diff --git a/.gitignore b/.gitignore index 938c247..9125036 100644 --- a/.gitignore +++ b/.gitignore @@ -3,6 +3,7 @@ PROMPT.md PROMPT-*.md PROMPT.MD PROMPT-*.MD +artifact/ # Local Code Review .vscode/local-reviews diff --git a/src/Cargo.lock b/src/Cargo.lock index 3723388..1d65247 100644 --- a/src/Cargo.lock +++ b/src/Cargo.lock @@ -2965,7 +2965,9 @@ name = "reloaded-code-serdesai" version = "0.2.0" dependencies = [ "ahash", + "anyhow", "async-trait", + "chrono", "futures", "indexmap", "reloaded-code-agents", diff --git a/src/docs/src/architecture.md b/src/docs/src/architecture.md index eb9b552..8ce8db1 100644 --- a/src/docs/src/architecture.md +++ b/src/docs/src/architecture.md @@ -40,8 +40,8 @@ The foundation. Contains every tool implementation as a plain function - **Model catalog** - compact hash-table-based provider/model lookup - **Hook types** - `HookSet`, `HookSetBuilder`, tool hook types (`ToolHook`, `ToolOriginal`, `ToolHookFuture`, `ToolExecutor`, - `ToolCallContext`, `ToolRequest`), and session event types - (`SessionContext`, `EndReason`). See [Hooks]. + `ToolCallContext`, `ToolRequest`), and hook run event types + (`HookRunContext`, `EndReason`). See [Hooks](hooks). Core is **framework-agnostic**: it has no dependencies on any specific LLM framework. Your integration layer wraps these functions into framework-specific diff --git a/src/docs/src/examples.md b/src/docs/src/examples.md index 8c0c082..2a16139 100644 --- a/src/docs/src/examples.md +++ b/src/docs/src/examples.md @@ -13,6 +13,8 @@ Runnable examples live in the repository under each crate's `examples/` director | [serdesai-task] | Orchestrator delegates a read-only task to a reader sub-agent, with streamed transcript and tool-call logging. | `cargo run --example serdesai-task -p reloaded-code-serdesai` | | [serdesai-sandboxed] | Agent with `AllowedPathResolver` - file operations restricted to specific directories. | `cargo run --example serdesai-sandboxed -p reloaded-code-serdesai` | | [serdesai-sandboxed-bash] | Sandboxed shell execution with a bubblewrap `public_bot` profile (Linux only). | `cargo run --example serdesai-sandboxed-bash --features linux-bubblewrap -p reloaded-code-serdesai` | +| [serdesai-run-hook] | Single `RunHook` injecting a preamble via `RunConfig`, integrated with SerdesAI agent pipeline. | `cargo run --example serdesai-run-hook -p reloaded-code-serdesai --features mock` | +| [serdesai-run-chain] | Two `RunHook`s showing nesting order in the integrated SerdesAI agent pipeline. | `cargo run --example serdesai-run-chain -p reloaded-code-serdesai --features mock` | [serdesai-basic]: https://github.com/Reloaded-Project/ReloadedCode/blob/main/src/reloaded-code-serdesai/examples/serdesai-basic.rs [serdesai-agents]: https://github.com/Reloaded-Project/ReloadedCode/blob/main/src/reloaded-code-serdesai/examples/serdesai-agents.rs @@ -21,6 +23,8 @@ Runnable examples live in the repository under each crate's `examples/` director [serdesai-task]: https://github.com/Reloaded-Project/ReloadedCode/blob/main/src/reloaded-code-serdesai/examples/serdesai-task.rs [serdesai-sandboxed]: https://github.com/Reloaded-Project/ReloadedCode/blob/main/src/reloaded-code-serdesai/examples/serdesai-sandboxed.rs [serdesai-sandboxed-bash]: https://github.com/Reloaded-Project/ReloadedCode/blob/main/src/reloaded-code-serdesai/examples/serdesai-sandboxed-bash.rs +[serdesai-run-hook]: https://github.com/Reloaded-Project/ReloadedCode/blob/main/src/reloaded-code-serdesai/examples/hooks/run/serdesai-run-hook.rs +[serdesai-run-chain]: https://github.com/Reloaded-Project/ReloadedCode/blob/main/src/reloaded-code-serdesai/examples/hooks/run/serdesai-run-chain.rs ## Core Library diff --git a/src/docs/src/hooks.md b/src/docs/src/hooks.md index 7f81132..0143d1d 100644 --- a/src/docs/src/hooks.md +++ b/src/docs/src/hooks.md @@ -2,9 +2,8 @@ Hooks let your code see, change, or stop things the agent does. -!!! warning "Work in progress" - Backend wiring is not done yet. Core hooks, event types, and container - exist. [SerdesAI] dispatch code comes next. +Tool and run hooks are wired into the [SerdesAI] agent pipeline: registered +hooks intercept real tool calls and agent runs end to end. Tool hooks work like game mods. Each hook gets an `original` function. @@ -12,9 +11,17 @@ Each hook gets an `original` function. This lets you run code before and after the tool call in the same method. -## Example +## Examples -A hook can modify the request before the tool sees it. +### Observe and wrap a tool call + +A hook can modify the request before the tool sees it, or rewrite the +result after the real tool runs. + +The full example takes the result path: a real `read` runs, and the +hook scrubs the `API_KEY=` and `TOKEN=` values from the result before +the model sees them. Permission rules can allow or deny the call; they +cannot rewrite what the tool returns. `$HOME` in string arguments expands to the user's home directory: @@ -57,8 +64,18 @@ let hooks = HookSet::builder() .build(); ``` +Full example: [serdesai-tool-hook] +(`cargo run --example serdesai-tool-hook -p reloaded-code-serdesai --features mock`). + +### Block a tool call + To block or replace a tool call, do not call `original`. +The full example keeps state across calls: the hook records every file +the run reads, then denies a `write` to a file that was never read, so +the real `write` never executes. Permission rules cannot make a +decision depend on earlier calls. + A common case: prevent credential leaks by blocking read/write access to `.env` files. @@ -98,6 +115,138 @@ let hooks = HookSet::builder() .build(); ``` +Full example: [serdesai-tool-block] +(`cargo run --example serdesai-tool-block -p reloaded-code-serdesai --features mock`). + +### Stack hooks + +Hooks run in registration order. Each hook wraps the next one, so code after +`original.call(...)` runs in reverse order. + +The full example stacks an audit hook that logs the original `bash` +arguments with a hardening hook that injects a `timeout_ms` before the +real tool runs. + +`tool_hook` takes ownership of the hook. `shared_tool_hook` registers an +existing `Arc` when the same instance must be used in several +hook sets: + +```rust +use std::sync::Arc; +use reloaded_code_core::{ + HookSet, ToolCallContext, ToolHook, ToolHookFuture, ToolOriginal, ToolRequest, +}; + +struct AuditHook(&'static str); + +impl ToolHook for AuditHook { + fn hook<'a>( + &'a self, + ctx: &'a ToolCallContext<'a>, + req: ToolRequest, + original: ToolOriginal<'a>, + ) -> ToolHookFuture<'a> { + Box::pin(async move { + println!("{}: before {}", self.0, ctx.tool_name); + let output = original.call(ctx, req).await?; + println!("{}: after {}", self.0, ctx.tool_name); + Ok(output) + }) + } +} + +let shared: Arc = Arc::new(AuditHook("inner")); +let hooks = HookSet::builder() + .tool_hook(AuditHook("outer")) + .shared_tool_hook(shared) + .build(); +``` + +Full example: [serdesai-tool-chain] +(`cargo run --example serdesai-tool-chain -p reloaded-code-serdesai --features mock`). + +### Intercept a run + +Run hooks wrap the whole agent run. Mutate `RunConfig` to change the system +prompt, preambles, or parameters, then call `original` to continue: + +```rust +use reloaded_code_core::{ + HookRunContext, HookSet, PreambleMessage, PreambleRole, RunConfig, RunHook, + RunHookFuture, RunOriginal, +}; + +struct PreambleInjector; + +impl RunHook for PreambleInjector { + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + mut config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + Box::pin(async move { + config.preamble_messages.push(PreambleMessage { + role: PreambleRole::System, + content: "You are a helpful assistant.".into(), + }); + original.call(ctx, config).await + }) + } +} + +let hooks = HookSet::builder() + .run_hook(PreambleInjector) + .build(); +``` + +Full example: [serdesai-run-hook] +(`cargo run --example serdesai-run-hook -p reloaded-code-serdesai --features mock`). + +### Observe run start and end + +A `RunHook` observes without changing anything: log before calling +`original`, inspect the result after: + +```rust +use reloaded_code_core::{ + EndReason, HookRunContext, HookSet, RunConfig, RunHook, RunHookFuture, + RunOriginal, +}; + +struct RunObserver; + +impl RunHook for RunObserver { + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + Box::pin(async move { + println!("run starting for {}", ctx.agent_name); + let result = original.call(ctx, config).await; + let reason = match &result { + Ok(output) => output.reason, + Err(_) => EndReason::Failed, + }; + println!("run ended for {} ({:?})", ctx.agent_name, reason); + result + }) + } +} + +let hooks = HookSet::builder() + .run_hook(RunObserver) + .build(); +``` + +Code after `original` runs when the wrapped continuation finishes, +including failure: a failed run reports `EndReason::Failed` and the +error still propagates to the caller. An outer hook that skips +`original` never reaches this hook, so do not rely on it for cleanup +that must run on every path. + ## Available types ### Tool hook types @@ -111,12 +260,24 @@ let hooks = HookSet::builder() | [`ToolRequest`] | JSON arguments carried through the hook chain. | | [`ToolOutput`] | Tool call result wrapping content and truncation metadata. | +### Run hook types + +| Type | Purpose | +| ----------------- | ------------------------------------------------------------ | +| [`RunHook`] | Intercepts a run and may call [`RunOriginal`]. | +| [`RunOriginal`] | Pointer to next hook or the real run executor. | +| [`RunHookFuture`] | Boxed future returned by run hooks. | +| [`RunConfig`] | Mutable config a RunHook can change before calling original. | +| [`RunOutput`] | Framework-agnostic result of a completed run. | +| [`RunExecutor`] | Final callable used at the end of the run hook chain. | +| [`RunUsage`] | Token usage for a completed run. | + ### Container types -| Type | Purpose | -| ------------------ | ------------------------------------- | -| [`HookSet`] | Stores tool hooks and session events. | -| [`HookSetBuilder`] | Builder for [`HookSet`]. | +| Type | Purpose | +| ------------------ | ------------------------------------------------- | +| [`HookSet`] | Stores tool hooks, run hooks, and compact events. | +| [`HookSetBuilder`] | Builder for [`HookSet`]. | ## How tool hooks stack @@ -166,15 +327,12 @@ passes `HookSet::default()`. ## Design notes -- **Tool hooks, not before/after events.** A tool call has one action with - a function to call in the middle. Hooks fit better than events here. - -- **Lifecycle events.** Session start/end/compact have no result to wrap. - They stay as simple callbacks. - They tell you something happened. +- **Everything is a hook**: Observers are plain `RunHook`s. Code before + `original` is "start", code after is "end". They participate in the + same hook chain with the same ordering rules. - **Natural unwind order.** Hook code after `original.call(...)` runs in - reverse order. Later hooks run first after the tool. + reverse order. Later hooks run first after the operation. - **Blocking by omission.** A hook blocks or replaces a call by not calling `original`. @@ -191,4 +349,15 @@ passes `HookSet::default()`. [`ToolOutput`]: https://docs.rs/reloaded-code-core/latest/reloaded_code_core/struct.ToolOutput.html [`HookSet`]: https://docs.rs/reloaded-code-core/latest/reloaded_code_core/struct.HookSet.html [`HookSetBuilder`]: https://docs.rs/reloaded-code-core/latest/reloaded_code_core/struct.HookSetBuilder.html +[`RunHook`]: https://docs.rs/reloaded-code-core/latest/reloaded_code_core/trait.RunHook.html +[`RunOriginal`]: https://docs.rs/reloaded-code-core/latest/reloaded_code_core/struct.RunOriginal.html +[`RunHookFuture`]: https://docs.rs/reloaded-code-core/latest/reloaded_code_core/type.RunHookFuture.html +[`RunConfig`]: https://docs.rs/reloaded-code-core/latest/reloaded_code_core/struct.RunConfig.html +[`RunOutput`]: https://docs.rs/reloaded-code-core/latest/reloaded_code_core/struct.RunOutput.html +[`RunExecutor`]: https://docs.rs/reloaded-code-core/latest/reloaded_code_core/trait.RunExecutor.html +[`RunUsage`]: https://docs.rs/reloaded-code-core/latest/reloaded_code_core/struct.RunUsage.html [SerdesAI]: https://crates.io/crates/serdes-ai +[serdesai-tool-hook]: https://github.com/Reloaded-Project/ReloadedCode/blob/main/src/reloaded-code-serdesai/examples/hooks/tool/serdesai-tool-hook.rs +[serdesai-tool-block]: https://github.com/Reloaded-Project/ReloadedCode/blob/main/src/reloaded-code-serdesai/examples/hooks/tool/serdesai-tool-block.rs +[serdesai-tool-chain]: https://github.com/Reloaded-Project/ReloadedCode/blob/main/src/reloaded-code-serdesai/examples/hooks/tool/serdesai-tool-chain.rs +[serdesai-run-hook]: https://github.com/Reloaded-Project/ReloadedCode/blob/main/src/reloaded-code-serdesai/examples/hooks/run/serdesai-run-hook.rs diff --git a/src/reloaded-code-agents/src/runtime/builder.rs b/src/reloaded-code-agents/src/runtime/builder.rs index f770e09..55c3824 100644 --- a/src/reloaded-code-agents/src/runtime/builder.rs +++ b/src/reloaded-code-agents/src/runtime/builder.rs @@ -82,7 +82,7 @@ impl AgentRuntimeBuilder { self } - /// Sets the hook set for tool interception and session lifecycle. + /// Sets the hook set for tool interception and run lifecycle. #[inline] pub fn hooks(mut self, hooks: HookSet) -> Self { self.hooks = hooks; @@ -181,21 +181,19 @@ mod tests { } #[test] - fn builder_overrides_task_settings() -> TestResult { - let runtime = AgentRuntimeBuilder::new().max_task_depth(5).build()?; - - assert_eq!(runtime.task_settings(), TaskSettings::with_max_depth(5)); - Ok(()) - } - - #[test] - fn builder_defaults_to_empty_catalog_defaults_and_default_tools() -> TestResult { + fn builder_defaults_and_overrides() -> TestResult { + // default: empty catalog, default AgentDefaults, default task settings, default tools let runtime = AgentRuntimeBuilder::new().build()?; assert_eq!(runtime.catalog().iter().count(), 0); assert_eq!(runtime.defaults(), &AgentDefaults::default()); assert_eq!(runtime.task_settings(), TaskSettings::default()); assert_eq!(runtime.tools(), default_tools().as_slice()); + + // override: max_task_depth(5) replaces default task settings + let runtime = AgentRuntimeBuilder::new().max_task_depth(5).build()?; + + assert_eq!(runtime.task_settings(), TaskSettings::with_max_depth(5)); Ok(()) } @@ -307,17 +305,39 @@ mod tests { } #[test] - fn builder_default_hooks_are_empty() -> TestResult { + fn builder_hooks() -> TestResult { + // default: no hooks registered let runtime = AgentRuntimeBuilder::new().build()?; assert!(runtime.hooks().is_empty()); - Ok(()) - } - #[test] - fn builder_hooks_sets_hook_set() -> TestResult { - let custom_hooks = HookSet::builder().build(); - let runtime = AgentRuntimeBuilder::new().hooks(custom_hooks).build()?; + // explicit empty HookSet stays empty + let runtime = AgentRuntimeBuilder::new() + .hooks(HookSet::builder().build()) + .build()?; assert!(runtime.hooks().is_empty()); + + // run hook survives build and populates hook set + struct NoopRun; + impl reloaded_code_core::RunHook for NoopRun { + fn hook<'a>( + &'a self, + ctx: &'a reloaded_code_core::HookRunContext<'a>, + config: reloaded_code_core::RunConfig, + original: reloaded_code_core::RunOriginal<'a>, + ) -> reloaded_code_core::RunHookFuture<'a> { + original.call(ctx, config) + } + } + + let runtime = AgentRuntimeBuilder::new() + .hooks( + reloaded_code_core::HookSet::builder() + .run_hook(NoopRun) + .build(), + ) + .build()?; + assert!(!runtime.hooks().is_empty()); + assert!(!runtime.hooks().run_hooks_is_empty()); Ok(()) } } diff --git a/src/reloaded-code-bubblewrap/src/profile/types.rs b/src/reloaded-code-bubblewrap/src/profile/types.rs index 86bfa9a..25ddb8c 100644 --- a/src/reloaded-code-bubblewrap/src/profile/types.rs +++ b/src/reloaded-code-bubblewrap/src/profile/types.rs @@ -17,12 +17,12 @@ use std::sync::Arc; /// /// Build this with [`crate::profile::Builder::build`]. /// -/// [`crate::profile::Builder::build`]: crate::profile::Builder::build -/// /// The build step validates profile-owned paths, resolves the `bwrap` binary, /// picks a visible host shell, and precomputes the static `bwrap` argv prefix. /// [`crate::wrap::wrap_command`] only needs to map the per-call working /// directory and append the shell command tail. +/// +/// [`crate::profile::Builder::build`]: crate::profile::Builder::build #[derive(Debug, Clone, PartialEq, Eq)] pub struct Profile { pub(crate) preset: Option, diff --git a/src/reloaded-code-core/src/context/mod.rs b/src/reloaded-code-core/src/context/mod.rs index 0f5c674..13b7859 100644 --- a/src/reloaded-code-core/src/context/mod.rs +++ b/src/reloaded-code-core/src/context/mod.rs @@ -61,8 +61,6 @@ pub const GIT_WORKFLOW: &str = include_str!("git_workflow.txt"); /// [`SystemPromptBuilder`] include tool guidance /// automatically. /// -/// [`SystemPromptBuilder`]: crate::SystemPromptBuilder -/// /// # Example /// /// ```rust @@ -78,6 +76,8 @@ pub const GIT_WORKFLOW: &str = include_str!("git_workflow.txt"); /// } /// } /// ``` +/// +/// [`SystemPromptBuilder`]: crate::SystemPromptBuilder pub trait ToolContext { /// Returns the tool name for section headers in generated system prompt. /// diff --git a/src/reloaded-code-core/src/custom_tool/mod.rs b/src/reloaded-code-core/src/custom_tool/mod.rs index b389f88..3dde5b3 100644 --- a/src/reloaded-code-core/src/custom_tool/mod.rs +++ b/src/reloaded-code-core/src/custom_tool/mod.rs @@ -17,8 +17,6 @@ //! - [`CustomToolRegistry`] - Registry of custom tool factories. //! - [`SharedToolRegistry`] - Shared wrapper around a registry for cheap cloning. //! -//! [`ToolContext`]: crate::ToolContext -//! //! # Usage //! //! ```rust @@ -78,6 +76,8 @@ //! registry.insert(MyFactory); //! assert!(registry.get("my_tool").is_some()); //! ``` +//! +//! [`ToolContext`]: crate::ToolContext pub use crate::tool_context::ToolBuildContext; pub use definition::CustomToolDefinition; diff --git a/src/reloaded-code-core/src/hooks/builder.rs b/src/reloaded-code-core/src/hooks/builder.rs index 9bf85de..b95ffdd 100644 --- a/src/reloaded-code-core/src/hooks/builder.rs +++ b/src/reloaded-code-core/src/hooks/builder.rs @@ -1,6 +1,6 @@ //! HookSetBuilder — builder for constructing a [`HookSet`]. -use crate::hooks::{HookSet, SessionCompactFn, SessionEndFn, SessionStartFn, ToolHook, INLINE_CAP}; +use crate::hooks::{HookSet, RunHook, SessionCompactFn, ToolHook, INLINE_CAP}; use std::fmt; use std::sync::Arc; use tinyvec::TinyVec; @@ -9,8 +9,7 @@ use tinyvec::TinyVec; #[derive(Default)] pub struct HookSetBuilder { pub(super) tool_hooks: Vec>, - pub(super) session_start: TinyVec<[Option; INLINE_CAP]>, - pub(super) session_end: TinyVec<[Option; INLINE_CAP]>, + pub(super) run_hooks: Vec>, pub(super) session_compact: TinyVec<[Option; INLINE_CAP]>, } @@ -41,27 +40,30 @@ impl HookSetBuilder { self } - /// Registers a session-start event. + /// Registers a compact event. Name preserved — compact is its own concept, distinct from "run". #[inline] #[must_use] - pub fn on_session_start(mut self, event: SessionStartFn) -> Self { - self.session_start.push(Some(event)); + pub fn on_session_compact(mut self, event: SessionCompactFn) -> Self { + self.session_compact.push(Some(event)); self } - /// Registers a session-end event. + /// Registers a game-style run hook. + /// + /// Hooks run in registration order. Each hook's `original` handle calls + /// the next registered hook, or the real run executor at the end of the chain. #[inline] #[must_use] - pub fn on_session_end(mut self, event: SessionEndFn) -> Self { - self.session_end.push(Some(event)); + pub fn run_hook(mut self, hook: impl RunHook) -> Self { + self.run_hooks.push(Arc::new(hook)); self } - /// Registers a session-compact event. + /// Registers an already shared game-style run hook. #[inline] #[must_use] - pub fn on_session_compact(mut self, event: SessionCompactFn) -> Self { - self.session_compact.push(Some(event)); + pub fn shared_run_hook(mut self, hook: Arc) -> Self { + self.run_hooks.push(hook); self } @@ -71,8 +73,7 @@ impl HookSetBuilder { pub fn build(self) -> HookSet { HookSet { tool_hooks: self.tool_hooks, - session_start: self.session_start, - session_end: self.session_end, + run_hooks: self.run_hooks, session_compact: self.session_compact, } } @@ -82,8 +83,7 @@ impl fmt::Debug for HookSetBuilder { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("HookSetBuilder") .field("tool_hooks", &self.tool_hooks.len()) - .field("session_start", &self.session_start.len()) - .field("session_end", &self.session_end.len()) + .field("run_hooks", &self.run_hooks.len()) .field("session_compact", &self.session_compact.len()) .finish() } @@ -92,6 +92,7 @@ impl fmt::Debug for HookSetBuilder { #[cfg(test)] mod tests { use super::*; + use crate::hooks::run_hook::{HookRunContext, RunConfig, RunHookFuture, RunOriginal}; use crate::hooks::tool_hook::{ToolCallContext, ToolHookFuture, ToolOriginal, ToolRequest}; #[test] @@ -126,4 +127,49 @@ mod tests { assert!(!hooks.tool_hooks_is_empty()); assert_eq!(hooks.tool_hooks().len(), 1); } + + #[test] + fn run_hook_registration_makes_hook_set_non_empty() { + struct NoopRun; + impl RunHook for NoopRun { + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + original.call(ctx, config) + } + } + let hooks = HookSetBuilder::new().run_hook(NoopRun).build(); + assert!(!hooks.is_empty()); + assert!(!hooks.run_hooks_is_empty()); + assert_eq!(hooks.run_hooks().len(), 1); + } + + #[test] + fn shared_run_hook_registration() { + struct NoopRun; + impl RunHook for NoopRun { + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + original.call(ctx, config) + } + } + let shared: Arc = Arc::new(NoopRun); + let hooks = HookSetBuilder::new().shared_run_hook(shared).build(); + assert!(!hooks.run_hooks_is_empty()); + assert_eq!(hooks.run_hooks().len(), 1); + } + + #[test] + fn builder_debug_includes_run_hooks() { + let builder = HookSetBuilder::new(); + let debug = format!("{:?}", builder); + assert!(debug.contains("run_hooks")); + } } diff --git a/src/reloaded-code-core/src/hooks/hook_set.rs b/src/reloaded-code-core/src/hooks/hook_set.rs index e39827c..2d674c6 100644 --- a/src/reloaded-code-core/src/hooks/hook_set.rs +++ b/src/reloaded-code-core/src/hooks/hook_set.rs @@ -1,8 +1,8 @@ //! HookSet — container and dispatch for all registered hooks and lifecycle events. use crate::hooks::{ - EndReason, SessionCompactFn, SessionContext, SessionEndFn, SessionStartFn, ToolCallContext, - ToolExecutor, ToolHook, ToolHookFuture, ToolOriginal, ToolRequest, INLINE_CAP, + HookRunContext, RunConfig, RunExecutor, RunHook, RunHookFuture, RunOriginal, SessionCompactFn, + ToolCallContext, ToolExecutor, ToolHook, ToolHookFuture, ToolOriginal, ToolRequest, INLINE_CAP, }; use std::fmt; use std::sync::Arc; @@ -12,8 +12,7 @@ use tinyvec::TinyVec; #[derive(Clone, Default)] pub struct HookSet { pub(super) tool_hooks: Vec>, - pub(super) session_start: TinyVec<[Option; INLINE_CAP]>, - pub(super) session_end: TinyVec<[Option; INLINE_CAP]>, + pub(super) run_hooks: Vec>, pub(super) session_compact: TinyVec<[Option; INLINE_CAP]>, } @@ -22,10 +21,7 @@ impl HookSet { #[inline] #[must_use] pub fn is_empty(&self) -> bool { - self.tool_hooks.is_empty() - && self.session_start.is_empty() - && self.session_end.is_empty() - && self.session_compact.is_empty() + self.tool_hooks.is_empty() && self.run_hooks.is_empty() && self.session_compact.is_empty() } /// Returns `true` if no tool hooks are registered. @@ -35,6 +31,13 @@ impl HookSet { self.tool_hooks.is_empty() } + /// Returns `true` if no run hooks are registered. + #[inline] + #[must_use] + pub fn run_hooks_is_empty(&self) -> bool { + self.run_hooks.is_empty() + } + /// Returns registered tool hooks in dispatch order. #[inline] #[must_use] @@ -42,6 +45,13 @@ impl HookSet { &self.tool_hooks } + /// Returns registered run hooks in dispatch order. + #[inline] + #[must_use] + pub fn run_hooks(&self) -> &[Arc] { + &self.run_hooks + } + /// Returns a new builder for constructing a `HookSet`. #[inline] #[must_use] @@ -62,29 +72,32 @@ impl HookSet { if self.tool_hooks.is_empty() { return real_tool.execute(ctx, req); } - ToolOriginal::new(&self.tool_hooks, real_tool).call(ctx, req) } - /// Dispatches session-start events. - #[inline] - pub fn dispatch_session_start(&self, ctx: &SessionContext<'_>) { - for event in self.session_start.iter().flatten() { - event(ctx); - } - } - - /// Dispatches session-end events. + /// Dispatches a run through the hook chain. + /// + /// If no run hooks are registered, this calls the real run + /// executor directly. + /// + /// # Errors + /// Returns `ToolError` if the executor or any run hook in the chain returns an error. #[inline] - pub fn dispatch_session_end(&self, ctx: &SessionContext<'_>, reason: EndReason) { - for event in self.session_end.iter().flatten() { - event(ctx, reason); + pub fn dispatch_run<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + config: RunConfig, + real_run: &'a dyn RunExecutor, + ) -> RunHookFuture<'a> { + if self.run_hooks.is_empty() { + return real_run.execute(ctx, config); } + RunOriginal::new(&self.run_hooks, real_run).call(ctx, config) } - /// Dispatches session-compact events. + /// Dispatches compact events. Name preserved — compact is its own concept, distinct from "run". #[inline] - pub fn dispatch_session_compact(&self, ctx: &SessionContext<'_>) { + pub fn dispatch_session_compact(&self, ctx: &HookRunContext<'_>) { for event in self.session_compact.iter().flatten() { event(ctx); } @@ -95,8 +108,7 @@ impl fmt::Debug for HookSet { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("HookSet") .field("tool_hooks", &self.tool_hooks.len()) - .field("session_start", &self.session_start.len()) - .field("session_end", &self.session_end.len()) + .field("run_hooks", &self.run_hooks.len()) .field("session_compact", &self.session_compact.len()) .finish() } @@ -105,6 +117,9 @@ impl fmt::Debug for HookSet { #[cfg(test)] mod tests { use super::*; + use crate::hooks::run_hook::{ + EndReason, RunConfig, RunExecutor, RunHook, RunHookFuture, RunOriginal, RunOutput, RunUsage, + }; use crate::ToolOutput; use serde_json::json; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -119,6 +134,26 @@ mod tests { let hooks = HookSet::default(); assert!(hooks.is_empty()); assert!(hooks.tool_hooks_is_empty()); + assert!(hooks.run_hooks_is_empty()); + } + + #[test] + fn hook_set_with_run_hooks_is_not_empty() { + struct NoopRun; + impl RunHook for NoopRun { + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + original.call(ctx, config) + } + } + let hooks = HookSet::builder().run_hook(NoopRun).build(); + assert!(!hooks.is_empty()); + assert!(!hooks.run_hooks_is_empty()); + assert_eq!(hooks.run_hooks().len(), 1); } #[tokio::test] @@ -260,45 +295,273 @@ mod tests { assert_eq!(output.content, "blocked"); } - #[test] - fn session_events_dispatch() { - static STARTS: AtomicUsize = AtomicUsize::new(0); - static ENDS: AtomicUsize = AtomicUsize::new(0); - static COMPACTS: AtomicUsize = AtomicUsize::new(0); + // --- Run dispatch tests ---------------------------------------------------- + + #[tokio::test] + async fn dispatch_run_empty_calls_real_run_directly() { + struct RealRun; + + impl RunExecutor for RealRun { + fn execute<'a>( + &'a self, + _ctx: &'a HookRunContext<'a>, + config: RunConfig, + ) -> RunHookFuture<'a> { + let content = config.system_prompt.unwrap_or_else(|| "default".into()); + Box::pin(async move { + Ok(RunOutput { + content, + reason: EndReason::Completed, + usage: RunUsage::default(), + }) + }) + } + } - fn on_start(_ctx: &SessionContext<'_>) { - STARTS.fetch_add(1, Ordering::SeqCst); + let hooks = HookSet::default(); + let ctx = HookRunContext { + agent_name: "coder", + run_id: "r1", + model_name: "gpt-4o", + }; + let output = hooks + .dispatch_run(&ctx, RunConfig::default(), &RealRun) + .await + .unwrap(); + + assert_eq!(output.content, "default"); + } + + #[tokio::test] + async fn dispatch_run_hooks_wrap_real_run() { + struct Prefix; + struct RealRun; + + impl RunHook for Prefix { + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + mut config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + Box::pin(async move { + config.system_prompt = Some("overridden".into()); + let mut output = original.call(ctx, config).await?; + output.content.push_str("-post"); + Ok(output) + }) + } } - fn on_end(_ctx: &SessionContext<'_>, reason: EndReason) { - assert_eq!(reason, EndReason::Completed); - ENDS.fetch_add(1, Ordering::SeqCst); + impl RunExecutor for RealRun { + fn execute<'a>( + &'a self, + _ctx: &'a HookRunContext<'a>, + config: RunConfig, + ) -> RunHookFuture<'a> { + let content = config.system_prompt.unwrap_or_else(|| "default".into()); + Box::pin(async move { + Ok(RunOutput { + content, + reason: EndReason::Completed, + usage: RunUsage::default(), + }) + }) + } } - fn on_compact(_ctx: &SessionContext<'_>) { + let hooks = crate::hooks::builder::HookSetBuilder::new() + .run_hook(Prefix) + .build(); + let ctx = HookRunContext { + agent_name: "coder", + run_id: "r1", + model_name: "gpt-4o", + }; + let output = hooks + .dispatch_run(&ctx, RunConfig::default(), &RealRun) + .await + .unwrap(); + + assert_eq!(output.content, "overridden-post"); + assert_eq!(output.reason, EndReason::Completed); + } + + #[tokio::test] + async fn dispatch_run_hook_can_skip_without_calling_original() { + struct Skip; + struct RealRun; + + impl RunHook for Skip { + fn hook<'a>( + &'a self, + _ctx: &'a HookRunContext<'a>, + _config: RunConfig, + _original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + Box::pin(async { + Ok(RunOutput { + content: "skipped".into(), + reason: EndReason::Completed, + usage: RunUsage::default(), + }) + }) + } + } + + impl RunExecutor for RealRun { + fn execute<'a>( + &'a self, + _ctx: &'a HookRunContext<'a>, + _config: RunConfig, + ) -> RunHookFuture<'a> { + Box::pin(async { + Ok(RunOutput { + content: "should not run".into(), + reason: EndReason::Completed, + usage: RunUsage::default(), + }) + }) + } + } + + let hooks = crate::hooks::builder::HookSetBuilder::new() + .run_hook(Skip) + .build(); + let ctx = HookRunContext { + agent_name: "coder", + run_id: "r1", + model_name: "gpt-4o", + }; + let output = hooks + .dispatch_run(&ctx, RunConfig::default(), &RealRun) + .await + .unwrap(); + + assert_eq!(output.content, "skipped"); + } + + #[tokio::test] + async fn dispatch_run_two_hooks_unwind_order() { + use std::sync::Mutex; + static LOG: Mutex> = Mutex::new(Vec::new()); + + struct First; + struct Second; + + impl RunHook for First { + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + LOG.lock().unwrap().push("first-before".into()); + Box::pin(async move { + let output = original.call(ctx, config).await?; + LOG.lock().unwrap().push("first-after".into()); + Ok(output) + }) + } + } + + impl RunHook for Second { + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + LOG.lock().unwrap().push("second-before".into()); + Box::pin(async move { + let output = original.call(ctx, config).await?; + LOG.lock().unwrap().push("second-after".into()); + Ok(output) + }) + } + } + + struct RealRun; + impl RunExecutor for RealRun { + fn execute<'a>( + &'a self, + _ctx: &'a HookRunContext<'a>, + _config: RunConfig, + ) -> RunHookFuture<'a> { + Box::pin(async { + Ok(RunOutput { + content: "ok".into(), + reason: EndReason::Completed, + usage: RunUsage::default(), + }) + }) + } + } + + LOG.lock().unwrap().clear(); + let hooks = crate::hooks::builder::HookSetBuilder::new() + .run_hook(First) + .run_hook(Second) + .build(); + let ctx = HookRunContext { + agent_name: "t", + run_id: "r1", + model_name: "m", + }; + hooks + .dispatch_run(&ctx, RunConfig::default(), &RealRun) + .await + .unwrap(); + + let log = LOG.lock().unwrap(); + assert_eq!( + *log, + vec![ + "first-before".to_string(), + "second-before".to_string(), + "second-after".to_string(), + "first-after".to_string(), + ] + ); + } + + #[tokio::test] + async fn session_compact_dispatch_untouched() { + static COMPACTS: AtomicUsize = AtomicUsize::new(0); + + fn on_compact(_ctx: &HookRunContext<'_>) { COMPACTS.fetch_add(1, Ordering::SeqCst); } - STARTS.store(0, Ordering::SeqCst); - ENDS.store(0, Ordering::SeqCst); COMPACTS.store(0, Ordering::SeqCst); - let hooks = crate::hooks::builder::HookSetBuilder::new() - .on_session_start(on_start) - .on_session_end(on_end) .on_session_compact(on_compact) .build(); - let ctx = SessionContext { + let ctx = HookRunContext { agent_name: "coder", run_id: "r1", + model_name: "gpt-4o", }; - hooks.dispatch_session_start(&ctx); - hooks.dispatch_session_end(&ctx, EndReason::Completed); hooks.dispatch_session_compact(&ctx); - - assert_eq!(STARTS.load(Ordering::SeqCst), 1); - assert_eq!(ENDS.load(Ordering::SeqCst), 1); assert_eq!(COMPACTS.load(Ordering::SeqCst), 1); } + + #[test] + fn hook_set_debug_includes_run_hooks_count() { + struct NoopRun; + impl RunHook for NoopRun { + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + original.call(ctx, config) + } + } + let hooks = HookSet::builder().run_hook(NoopRun).build(); + let debug = format!("{:?}", hooks); + assert!(debug.contains("run_hooks: 1")); + } } diff --git a/src/reloaded-code-core/src/hooks/mod.rs b/src/reloaded-code-core/src/hooks/mod.rs index 9e4a490..bbbd286 100644 --- a/src/reloaded-code-core/src/hooks/mod.rs +++ b/src/reloaded-code-core/src/hooks/mod.rs @@ -1,4 +1,4 @@ -//! Hook infrastructure for tool hooks and session lifecycle events. +//! Hook infrastructure for tool hooks and run lifecycle hooks. //! //! # Public API //! @@ -10,9 +10,18 @@ //! - [`ToolRequest`] - JSON tool arguments //! - [`ToolExecutor`] - Final callable used at the end of the hook chain //! -//! Session event types: -//! - [`SessionContext`] - Context given to session lifecycle events -//! - [`EndReason`] - Why a session ended +//! Run hook types: +//! - [`RunHook`] - Intercepts a run and may call [`RunOriginal`] +//! - [`RunHookFuture`] - Boxed future returned by [`RunHook::hook`] +//! - [`RunOriginal`] - Managed trampoline to the next hook or real run executor +//! - [`RunConfig`] - Mutable config a RunHook can change before calling original +//! - [`RunOutput`] - Result of a completed run +//! - [`RunExecutor`] - Final callable used at the end of the run hook chain +//! - [`HookRunContext`] - Context given to hook run lifecycle events +//! - [`EndReason`] - Why a run ended +//! +//! Observers are plain hooks: code before `original` is "start", code +//! after is "end". They participate in the same hook chain. //! //! Container: //! - [`HookSet`] - Container for registered hooks and lifecycle events @@ -20,19 +29,20 @@ //! //! # Design //! -//! Tool hooks follow game-style hook semantics. Each hook receives an -//! `original` handle. Calling it invokes the next hook in the chain, or the -//! real tool when the chain is exhausted. Not calling it blocks or replaces the -//! tool call. Session hooks remain simple lifecycle events. +//! Tool hooks and run hooks follow game-style hook semantics. Each hook +//! receives an `original` handle. Calling it invokes the next hook in the +//! chain, or the real implementation when the chain is exhausted. Not calling +//! it blocks or replaces the call. Everything is built on top of the same +//! hook chain. pub use self::builder::HookSetBuilder; pub use self::hook_set::HookSet; -pub use self::session::*; +pub use self::run_hook::*; pub use self::tool_hook::*; mod builder; mod hook_set; -mod session; +mod run_hook; mod tool_hook; /// Max hooks per point before falling back to heap. diff --git a/src/reloaded-code-core/src/hooks/run_hook/mod.rs b/src/reloaded-code-core/src/hooks/run_hook/mod.rs new file mode 100644 index 0000000..34f23f2 --- /dev/null +++ b/src/reloaded-code-core/src/hooks/run_hook/mod.rs @@ -0,0 +1,395 @@ +//! Run hook types: intercept trait, config, output, and chain trampoline. +//! +//! # What a run is +//! +//! One run = one `agent.run()` call, start to finish. +//! The framework is headless: no persistent conversation, no branching, +//! no multi-session switching. One API call starts exactly one run. +//! +//! A run holds N steps. One step = one LLM request plus the tool calls +//! it triggers. A run with no tool calls is a single step. +//! +//! A run hook wraps that whole boundary. Code before `original` runs +//! before the first step: inject preamble messages, override the system +//! prompt or model settings. +//! +//! Code after `original` sees the finished [`RunOutput`]. Skipping +//! `original` skips the run and returns a synthetic result instead. +//! +//! Each run carries a `run_id` (see [`HookRunContext`]). Tool hooks +//! fire inside a run, once per tool call, under the same `run_id`. +//! +//! Next: see [`ToolHook`] for the innermost intercept point. +//! +//! [`ToolHook`]: crate::hooks::ToolHook + +use crate::ToolError; +use std::fmt; +use std::future::Future; +use std::pin::Pin; +use std::sync::Arc; + +/// Mutable config a RunHook can change before calling original. +#[derive(Default)] +pub struct RunConfig { + /// Override the agent's default system prompt. + pub system_prompt: Option, + /// Preamble messages injected before the user prompt. + pub preamble_messages: Vec, + /// Model settings overrides (temperature, top_p, etc.). + pub model_settings_overrides: Option, +} + +/// Boxed future returned by [`RunHook::hook`] and [`RunExecutor::execute`]. +pub type RunHookFuture<'a> = Pin> + Send + 'a>>; + +/// Managed trampoline to the next hook or real run executor. +/// +/// `RunOriginal` is consumed by [`call`], so normal hooks call +/// the continuation once. +/// +/// [`call`]: Self::call +pub struct RunOriginal<'a> { + chain: &'a [Arc], + index: usize, + real_run: &'a dyn RunExecutor, +} + +/// Compact event callback. Name preserved - compact is its own concept, distinct from "run". +pub type SessionCompactFn = for<'a> fn(&'a HookRunContext<'a>); + +/// Context given to hook run lifecycle events. +#[derive(Debug)] +pub struct HookRunContext<'a> { + /// Name of the agent running the hook. + pub agent_name: &'a str, + /// Unique identifier for the current run. + pub run_id: &'a str, + /// Name of the model being used for this run. + pub model_name: &'a str, +} + +/// Model-level settings that a RunHook can override. +#[derive(Default)] +pub struct ModelSettingsOverrides { + /// Temperature override. + pub temperature: Option, + /// Top-p override. + pub top_p: Option, +} + +/// Preamble message injected before the user's prompt. +#[derive(Debug, Clone)] +pub struct PreambleMessage { + /// Role of the preamble message. + pub role: PreambleRole, + /// Content of the preamble message. + pub content: String, +} + +/// Result of a completed run. Framework-agnostic distillation of the agent output. +#[derive(Debug)] +pub struct RunOutput { + /// The text output from the run. + pub content: String, + /// Why the run ended. + pub reason: EndReason, + /// Token usage consumed during the run. + pub usage: RunUsage, +} + +/// Result alias for run hook operations. Re-uses [`ToolError`]. +pub type RunResult = Result; + +/// Why a run ended. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum EndReason { + /// Run completed normally. + Completed, + /// Run was stopped externally. + Stopped, + /// Run failed (LLM error, length limit, content filter). + Failed, +} + +/// Role for a preamble message. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PreambleRole { + /// System-level instruction. + System, + /// User-level context. + User, +} + +/// Token usage for a completed run. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct RunUsage { + /// Tokens consumed in the prompt. + pub prompt_tokens: u64, + /// Tokens consumed in the completion. + pub completion_tokens: u64, +} + +/// Final callable used when the hook chain reaches the real run executor. +pub trait RunExecutor: Send + Sync { + /// Executes the real run. + /// + /// # Errors + /// Returns `ToolError` if the real run executor encounters an error. + fn execute<'a>(&'a self, ctx: &'a HookRunContext<'a>, config: RunConfig) -> RunHookFuture<'a>; +} + +/// Intercept hook for the full run lifecycle. +/// +/// Code before `original` = inject preamble, override config. +/// Skip `original` = skip the run (return a synthetic `RunOutput`). +/// Code after = observe the run result. +/// +/// `config` is owned (same as `ToolRequest` in `ToolHook`). Each hook +/// takes ownership, mutates, and passes to `original.call()`. The final +/// [`RunExecutor`] consumes it: strings move into the framework's run +/// options with zero clones. +pub trait RunHook: Send + Sync + 'static { + /// Intercepts a run. + /// + /// # Errors + /// Returns `ToolError` if the hook implementation or downstream executor fails. + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a>; +} + +impl<'a> RunOriginal<'a> { + /// Creates a trampoline over the provided hook chain and real run executor. + #[inline] + #[must_use] + pub fn new(chain: &'a [Arc], real_run: &'a dyn RunExecutor) -> Self { + Self { + chain, + index: 0, + real_run, + } + } + + /// Calls the next hook, or the real run executor when no hooks remain. + /// + /// # Errors + /// Returns `ToolError` if a downstream hook or the real executor returns an error. + #[inline] + pub fn call(self, ctx: &'a HookRunContext<'a>, config: RunConfig) -> RunHookFuture<'a> { + if let Some(hook) = self.chain.get(self.index) { + hook.hook( + ctx, + config, + Self { + chain: self.chain, + index: self.index + 1, + real_run: self.real_run, + }, + ) + } else { + self.real_run.execute(ctx, config) + } + } +} + +impl fmt::Debug for RunOriginal<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("RunOriginal") + .field("chain_len", &self.chain.len()) + .field("index", &self.index) + .finish_non_exhaustive() + } +} + +impl RunHook for F +where + F: for<'a> Fn(&'a HookRunContext<'a>, RunConfig, RunOriginal<'a>) -> RunHookFuture<'a> + + Send + + Sync + + 'static, +{ + #[inline] + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + self(ctx, config, original) + } +} + +impl RunExecutor for F +where + F: for<'a> Fn(&'a HookRunContext<'a>, RunConfig) -> RunHookFuture<'a> + Send + Sync, +{ + #[inline] + fn execute<'a>(&'a self, ctx: &'a HookRunContext<'a>, config: RunConfig) -> RunHookFuture<'a> { + self(ctx, config) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn run_config_populated_holds_values() { + let config = RunConfig { + system_prompt: Some("sys".into()), + preamble_messages: vec![PreambleMessage { + role: PreambleRole::User, + content: "ctx".into(), + }], + model_settings_overrides: Some(ModelSettingsOverrides { + temperature: Some(0.5), + top_p: Some(0.9), + }), + }; + assert_eq!(config.system_prompt.as_deref(), Some("sys")); + assert_eq!(config.preamble_messages.len(), 1); + assert_eq!( + config.model_settings_overrides.unwrap().temperature, + Some(0.5) + ); + } + + #[tokio::test] + async fn run_hook_closure_impl() { + struct RealRun; + impl RunExecutor for RealRun { + fn execute<'a>( + &'a self, + _ctx: &'a HookRunContext<'a>, + _config: RunConfig, + ) -> RunHookFuture<'a> { + Box::pin(async { + Ok(RunOutput { + content: "real".into(), + reason: EndReason::Completed, + usage: RunUsage::default(), + }) + }) + } + } + + struct MockHook; + impl RunHook for MockHook { + fn hook<'a>( + &'a self, + _ctx: &'a HookRunContext<'a>, + _config: RunConfig, + _original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + Box::pin(async { + Ok(RunOutput { + content: "mock".into(), + reason: EndReason::Completed, + usage: RunUsage::default(), + }) + }) + } + } + + let ctx = HookRunContext { + agent_name: "test", + run_id: "r1", + model_name: "gpt-4o", + }; + let hook: Arc = Arc::new(MockHook); + let output = hook + .hook(&ctx, RunConfig::default(), RunOriginal::new(&[], &RealRun)) + .await + .unwrap(); + assert_eq!(output.content, "mock"); + } + + #[tokio::test] + async fn run_original_calls_real_executor_when_chain_empty() { + struct RealRun; + impl RunExecutor for RealRun { + fn execute<'a>( + &'a self, + _ctx: &'a HookRunContext<'a>, + _config: RunConfig, + ) -> RunHookFuture<'a> { + Box::pin(async { + Ok(RunOutput { + content: "real".into(), + reason: EndReason::Completed, + usage: RunUsage::default(), + }) + }) + } + } + + let ctx = HookRunContext { + agent_name: "test", + run_id: "r1", + model_name: "gpt-4o", + }; + let original = RunOriginal::new(&[], &RealRun); + let output = original.call(&ctx, RunConfig::default()).await.unwrap(); + assert_eq!(output.content, "real"); + } + + #[tokio::test] + async fn run_original_debug_format() { + struct RealRun; + impl RunExecutor for RealRun { + fn execute<'a>( + &'a self, + _ctx: &'a HookRunContext<'a>, + _config: RunConfig, + ) -> RunHookFuture<'a> { + Box::pin(async { + Ok(RunOutput { + content: "".into(), + reason: EndReason::Completed, + usage: RunUsage::default(), + }) + }) + } + } + let chain: Vec> = vec![]; + let original = RunOriginal::new(&chain, &RealRun); + let debug = format!("{:?}", original); + assert!(debug.contains("RunOriginal")); + assert!(debug.contains("chain_len")); + } + + #[tokio::test] + async fn run_executor_fn_impl() { + struct FnExecutor; + impl RunExecutor for FnExecutor { + fn execute<'a>( + &'a self, + _ctx: &'a HookRunContext<'a>, + _config: RunConfig, + ) -> RunHookFuture<'a> { + Box::pin(async { + Ok(RunOutput { + content: "from-fn".into(), + reason: EndReason::Completed, + usage: RunUsage::default(), + }) + }) + } + } + + let ctx = HookRunContext { + agent_name: "test", + run_id: "r1", + model_name: "gpt-4o", + }; + let output = FnExecutor + .execute(&ctx, RunConfig::default()) + .await + .unwrap(); + assert_eq!(output.content, "from-fn"); + } +} diff --git a/src/reloaded-code-core/src/hooks/session.rs b/src/reloaded-code-core/src/hooks/session.rs deleted file mode 100644 index e73acb5..0000000 --- a/src/reloaded-code-core/src/hooks/session.rs +++ /dev/null @@ -1,50 +0,0 @@ -//! Session lifecycle event types. - -/// Session-compact event callback. -pub type SessionCompactFn = for<'a> fn(&'a SessionContext<'a>); - -/// Session-end event callback. -pub type SessionEndFn = for<'a> fn(&'a SessionContext<'a>, EndReason); - -/// Session-start event callback. -pub type SessionStartFn = for<'a> fn(&'a SessionContext<'a>); - -/// Why a session ended. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum EndReason { - /// Session completed normally. - Completed, - /// Session was stopped externally. - Stopped, -} - -/// Context given to session lifecycle events. -#[derive(Debug)] -pub struct SessionContext<'a> { - /// Name of the agent running the session. - pub agent_name: &'a str, - /// Unique identifier for the current run. - pub run_id: &'a str, -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn session_context_fields_are_accessible() { - let ctx = SessionContext { - agent_name: "orchestrator", - run_id: "r3", - }; - assert_eq!(ctx.agent_name, "orchestrator"); - assert_eq!(ctx.run_id, "r3"); - } - - #[test] - fn end_reason_variants_exist() { - assert_eq!(EndReason::Completed, EndReason::Completed); - assert_eq!(EndReason::Stopped, EndReason::Stopped); - assert_ne!(EndReason::Completed, EndReason::Stopped); - } -} diff --git a/src/reloaded-code-core/src/hooks/tool_hook.rs b/src/reloaded-code-core/src/hooks/tool_hook/mod.rs similarity index 71% rename from src/reloaded-code-core/src/hooks/tool_hook.rs rename to src/reloaded-code-core/src/hooks/tool_hook/mod.rs index 5aa8ae8..6738181 100644 --- a/src/reloaded-code-core/src/hooks/tool_hook.rs +++ b/src/reloaded-code-core/src/hooks/tool_hook/mod.rs @@ -1,4 +1,40 @@ -//! Tool hook types -- traits, futures, and chain trampoline. +//! Tool hook types: intercept trait, context, request, and chain trampoline. +//! +//! # What a tool call is +//! +//! One tool call = one invocation of a single tool. The model requests +//! the tool with JSON arguments, the tool runs, a result goes back to +//! the model. Each request during a run is its own call. +//! +//! Tool hooks fire inside a run, once per tool call, under the same +//! `run_id` as the enclosing run. +//! +//! [`ToolCallContext`] names the call: `tool_name` for the tool being +//! called, `agent_name` for the agent making the call, `run_id` for +//! the enclosing run. +//! +//! A tool hook wraps that single call. Code before +//! [`ToolOriginal::call`] sees the raw [`ToolRequest`]: inspect the +//! JSON arguments or rewrite them. +//! +//! Code after [`ToolOriginal::call`] sees the result returned by the +//! next hook in the chain, or the real tool if none remain: an inner +//! hook may wrap or replace it. Skipping `original` blocks the call: +//! the real tool never runs and the hook's return value becomes the +//! result. +//! +//! [`ToolOriginal`] is consumed by [`ToolOriginal::call`], so each hook +//! can continue the chain at most once. There is no built-in retry: a +//! hook that wants to block or replace the call returns its own result +//! without calling the continuation. +//! +//! Multiple hooks run in registration order, outer-to-inner: the first +//! registered hook is outermost, the last one sits directly on the +//! real tool. +//! +//! Next: see [`RunHook`] for the whole-run intercept point. +//! +//! [`RunHook`]: crate::hooks::RunHook use crate::{ToolOutput, ToolResult}; use serde_json::Value; @@ -23,9 +59,9 @@ pub type ToolHookFuture<'a> = Pin /// Managed trampoline to the next hook or real tool. /// -/// `ToolOriginal` is consumed by [`call`], so normal hooks call -/// the continuation once. Hooks that intentionally retry can clone the -/// request before calling and perform retries around one continuation call. +/// `ToolOriginal` is consumed by [`call`], so each hook can continue +/// the chain at most once. There is no built-in retry: block or replace +/// the call by returning a result without invoking the continuation. /// /// [`call`]: Self::call pub struct ToolOriginal<'a> { diff --git a/src/reloaded-code-core/src/models/catalog/mod.rs b/src/reloaded-code-core/src/models/catalog/mod.rs index 4df3109..b82a05d 100644 --- a/src/reloaded-code-core/src/models/catalog/mod.rs +++ b/src/reloaded-code-core/src/models/catalog/mod.rs @@ -85,8 +85,6 @@ //! Collision estimates use the birthday-bound approximation described by //! [Preshing]: //! -//! [Preshing]: https://preshing.com/20110504/hash-collision-probabilities/ -//! //! `p(at least one collision) ~= 1 - exp(-n * (n - 1) / (2 * 2^48))` //! //! where `n` is the number of inserted keys. @@ -223,6 +221,8 @@ //! `ProviderModelTable` keys point to shared `model_entries` and optional //! `model_config_entries` rows. If multiple provider models share the same //! model metadata, the metadata is stored once and reused by index. +//! +//! [Preshing]: https://preshing.com/20110504/hash-collision-probabilities/ use crate::internal::hash64::Hash64; use crate::models::ProviderType; diff --git a/src/reloaded-code-core/src/system_prompt.rs b/src/reloaded-code-core/src/system_prompt.rs index 20be324..eac09c2 100644 --- a/src/reloaded-code-core/src/system_prompt.rs +++ b/src/reloaded-code-core/src/system_prompt.rs @@ -125,8 +125,6 @@ impl SystemPromptBuilder { /// * `name` - Section header (e.g., "Git Workflow", "GitHub CLI") /// * `context` - Context string content (e.g., [`GIT_WORKFLOW`]) /// - /// [`GIT_WORKFLOW`]: crate::context::GIT_WORKFLOW - /// /// # Examples /// /// Adding both git and GitHub CLI context: @@ -156,6 +154,8 @@ impl SystemPromptBuilder { /// assert!(prompt.contains("## Git Workflow")); /// assert!(!prompt.contains("## GitHub CLI")); /// ``` + /// + /// [`GIT_WORKFLOW`]: crate::context::GIT_WORKFLOW #[inline] pub fn add_context(mut self, name: &'static str, context: &'static str) -> Self { self.supplemental.push((name, context)); diff --git a/src/reloaded-code-models-dev/src/api/catalog_sources.rs b/src/reloaded-code-models-dev/src/api/catalog_sources.rs index e990542..811c349 100644 --- a/src/reloaded-code-models-dev/src/api/catalog_sources.rs +++ b/src/reloaded-code-models-dev/src/api/catalog_sources.rs @@ -3,8 +3,6 @@ //! This module parses models.dev `api.json`, maps provider/model metadata into //! transient core builder inputs, and immediately constructs a [`ModelCatalog`]. //! -//! [`ModelCatalog`]: reloaded_code_core::models::ModelCatalog -//! //! Mapping policy: //! - missing limits default to `0`; //! - model modalities are mapped from `modalities.input[]`/`modalities.output[]` @@ -14,6 +12,8 @@ //! [`Modality::empty()`]; //! - model rows remain provider-scoped; shared configurations are deduplicated by //! core during catalog build. +//! +//! [`ModelCatalog`]: reloaded_code_core::models::ModelCatalog use super::schema::{parse_api_json, ApiModelEntry, ApiModelLimit, ApiModelModalities}; use crate::cache::payload::{CachedModelRow, CachedProviderRow, CatalogCachePayload}; diff --git a/src/reloaded-code-models-dev/src/catalog/mod.rs b/src/reloaded-code-models-dev/src/catalog/mod.rs index b49c404..1b16ed7 100644 --- a/src/reloaded-code-models-dev/src/catalog/mod.rs +++ b/src/reloaded-code-models-dev/src/catalog/mod.rs @@ -102,8 +102,6 @@ impl ModelsDevCatalog { /// - Custom deployment scenarios /// - Isolated cache locations /// - /// [`load`]: Self::load - /// /// # Parameters /// /// * `path` - The path to the cache file. Parent directories will be @@ -148,6 +146,8 @@ impl ModelsDevCatalog { /// # Ok(()) /// # } /// ``` + /// + /// [`load`]: Self::load #[maybe_async::maybe_async] pub async fn load_at(path: impl AsRef) -> Result { sync::load_catalog_at_path(path.as_ref()).await diff --git a/src/reloaded-code-provider-config/README.md b/src/reloaded-code-provider-config/README.md new file mode 100644 index 0000000..7f0c3b6 --- /dev/null +++ b/src/reloaded-code-provider-config/README.md @@ -0,0 +1,29 @@ +# reloaded-code-provider-config + +YAML-based custom provider configuration for ReloadedCode. + +Parse provider definitions from YAML files, merge multiple sources, and +convert them into catalog types for `ModelCatalog::build()`. + +## Install + +```toml +[dependencies] +reloaded-code-provider-config = "0.1.0" +``` + +## Usage + +```rust +use reloaded_code_provider_config::ProviderConfigLoader; + +let loaded = ProviderConfigLoader::with_default_paths()?.load()?; +for (key, config) in &loaded.providers { + let count = config.models.as_ref().map_or(0, |m| m.len()); + println!("{key}: {count} model(s)"); +} +``` + +See the [project documentation] for details. + +[project documentation]: https://github.com/Reloaded-Project/ReloadedCode diff --git a/src/reloaded-code-serdesai/Cargo.toml b/src/reloaded-code-serdesai/Cargo.toml index 09ac1bd..a4e41a3 100644 --- a/src/reloaded-code-serdesai/Cargo.toml +++ b/src/reloaded-code-serdesai/Cargo.toml @@ -87,6 +87,10 @@ reqwest = { workspace = true } # Used for IndexMap in build.rs (permission config) indexmap = { workspace = true } +# Error conversion for hook dispatch +anyhow = "1.0" +chrono = "0.4" + [dev-dependencies] serial_test = { workspace = true } tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } @@ -98,3 +102,28 @@ temp-env = { workspace = true } rstest = { workspace = true } # models.dev catalog loader for examples reloaded-code-models-dev = { workspace = true } + +[[example]] +name = "serdesai-run-hook" +path = "examples/hooks/run/serdesai-run-hook.rs" +required-features = ["mock"] + +[[example]] +name = "serdesai-run-chain" +path = "examples/hooks/run/serdesai-run-chain.rs" +required-features = ["mock"] + +[[example]] +name = "serdesai-tool-hook" +path = "examples/hooks/tool/serdesai-tool-hook.rs" +required-features = ["mock"] + +[[example]] +name = "serdesai-tool-block" +path = "examples/hooks/tool/serdesai-tool-block.rs" +required-features = ["mock"] + +[[example]] +name = "serdesai-tool-chain" +path = "examples/hooks/tool/serdesai-tool-chain.rs" +required-features = ["mock"] diff --git a/src/reloaded-code-serdesai/examples/hooks/README.MD b/src/reloaded-code-serdesai/examples/hooks/README.MD new file mode 100644 index 0000000..012899a --- /dev/null +++ b/src/reloaded-code-serdesai/examples/hooks/README.MD @@ -0,0 +1,58 @@ +# Hooks + +Hooks let your code see or change what an agent does. + +## Run hooks + +- `RunHook` trait + - Full control over `RunConfig` (system prompt, preambles, params). + - Intercept the run, modify config, then call `original` to continue. + - Skip `original.call()` to block or replace the run entirely. + - Observe start and end: code before `original` is "start", code + after is "end" (logging, metrics). + +### Example programs + +- serdesai-run-hook + - Single `RunHook` that injects a system preamble via `RunConfig`. + - `cargo run --example serdesai-run-hook -p reloaded-code-serdesai --features mock` + +- serdesai-run-chain + - Two `RunHook`s showing nesting order. + - Registration order A -> B gives execution A-before, B-before, executor, B-after, A-after. + - `cargo run --example serdesai-run-chain -p reloaded-code-serdesai --features mock` + +## Tool hooks + +- `ToolHook` trait + - Full control over tool arguments and results. + - Inspect or rewrite arguments, call `original` to run the real tool, + then wrap the result. + - Skip `original.call()` to block the call and return your own response. + +### Example programs + +- serdesai-tool-hook + - Single `ToolHook` that lets a real `read` of a secrets fixture run, + then scrubs the `API_KEY=` and `TOKEN=` values from the result + before the model sees it. + - `cargo run --example serdesai-tool-hook -p reloaded-code-serdesai --features mock` + +- serdesai-tool-block + - Stateful `ToolHook` that records each file the run reads, then skips + `original` on a `write` to a never-read file; the real `write` never + executes and the hook-supplied response completes the run. + - `cargo run --example serdesai-tool-block -p reloaded-code-serdesai --features mock` + +- serdesai-tool-chain + - Two `ToolHook`s around one real `bash` call: an audit hook logs the + original arguments and run context, and a hardening hook injects a + `timeout_ms` into the arguments before the real tool runs; the + second is registered via `shared_tool_hook`. + - `cargo run --example serdesai-tool-chain -p reloaded-code-serdesai --features mock` + +## Shared code + +`shared.rs` holds common setup: mock `ModelCatalog`, dummy credentials, `build_agent_context`, `mock_model`, and agent config fixtures. Each example pulls it in with `#[path = "../shared.rs"] mod shared;`. `shared.rs` is not a standalone example. + +Run examples build their agent with `agent_config`; tool examples use `agent_config_with_tools`, whose permission rules allow the named standard tools so the agent has real tools to intercept. diff --git a/src/reloaded-code-serdesai/examples/hooks/run/serdesai-run-chain.rs b/src/reloaded-code-serdesai/examples/hooks/run/serdesai-run-chain.rs new file mode 100644 index 0000000..f30246b --- /dev/null +++ b/src/reloaded-code-serdesai/examples/hooks/run/serdesai-run-chain.rs @@ -0,0 +1,81 @@ +//! Multiple `RunHook`s with a real SerdesAI agent and mock model. +//! +//! Registers two hooks via `AgentRuntimeBuilder::hooks()` and demonstrates +//! the expected nesting order: A-before -> B-before -> Executor -> B-after -> A-after. +//! +//! Expected output: +//! [FirstHook] before +//! [SecondHook] before +//! [SecondHook] after +//! [FirstHook] after +//! Output: Mock response +//! +//! Run with: +//! cargo run --example serdesai-run-chain -p reloaded-code-serdesai --features mock + +use reloaded_code_agents::AgentCatalog; +use reloaded_code_core::{HookRunContext, HookSet, RunConfig, RunHook, RunHookFuture, RunOriginal}; + +#[path = "../shared.rs"] +mod shared; + +struct FirstHook; + +struct SecondHook; + +impl RunHook for FirstHook { + fn hook<'a>( + &'a self, + _ctx: &'a HookRunContext<'a>, + config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + Box::pin(async move { + println!("[FirstHook] before"); + let output = original.call(_ctx, config).await?; + println!("[FirstHook] after"); + Ok(output) + }) + } +} + +impl RunHook for SecondHook { + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + Box::pin(async move { + println!("[SecondHook] before"); + let output = original.call(ctx, config).await?; + println!("[SecondHook] after"); + Ok(output) + }) + } +} + +#[tokio::main] +async fn main() -> Result<(), Box> { + let hooks = HookSet::builder() + .run_hook(FirstHook) + .run_hook(SecondHook) + .build(); + + let catalog = AgentCatalog::from_entries([shared::agent_config( + "chain-demo", + "chain demo", + "You are a chain demo agent.", + )]); + + let build_context = shared::build_agent_context(catalog, hooks); + + let model = shared::mock_model(); + let agent = build_context + .with_model_override(model) + .build("chain-demo")?; + + let response = agent.run("Say hello.", ()).await?; + println!("Output: {}", response.output()); + Ok(()) +} diff --git a/src/reloaded-code-serdesai/examples/hooks/run/serdesai-run-hook.rs b/src/reloaded-code-serdesai/examples/hooks/run/serdesai-run-hook.rs new file mode 100644 index 0000000..75f4728 --- /dev/null +++ b/src/reloaded-code-serdesai/examples/hooks/run/serdesai-run-hook.rs @@ -0,0 +1,69 @@ +//! Single `RunHook` with a real SerdesAI agent and mock model. +//! +//! This example registers a `RunHook` via `AgentRuntimeBuilder::hooks()`, +//! builds an agent with `AgentBuildContext::with_model_override()` using a +//! mock model, and runs it. The hook injects a system preamble via +//! `RunConfig` and prints a confirmation message. +//! +//! Expected output: +//! Built agent with 0 tools. +//! [PreambleInjector] injecting preamble for agent=hook-demo +//! Output: Mock response +//! +//! Run with: +//! cargo run --example serdesai-run-hook -p reloaded-code-serdesai --features mock + +use reloaded_code_agents::AgentCatalog; +use reloaded_code_core::{ + HookRunContext, HookSet, PreambleMessage, PreambleRole, RunConfig, RunHook, RunHookFuture, + RunOriginal, +}; + +#[path = "../shared.rs"] +mod shared; + +struct PreambleInjector; + +impl RunHook for PreambleInjector { + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + mut config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + Box::pin(async move { + println!( + "[PreambleInjector] injecting preamble for agent={}", + ctx.agent_name + ); + config.preamble_messages.push(PreambleMessage { + role: PreambleRole::System, + content: "You are a helpful assistant.".into(), + }); + original.call(ctx, config).await + }) + } +} + +#[tokio::main] +async fn main() -> Result<(), Box> { + let hooks = HookSet::builder().run_hook(PreambleInjector).build(); + + let catalog = AgentCatalog::from_entries([shared::agent_config( + "hook-demo", + "demo agent", + "You are a demo agent.", + )]); + + let build_context = shared::build_agent_context(catalog, hooks); + + let model = shared::mock_model(); + let agent = build_context + .with_model_override(model) + .build("hook-demo")?; + println!("Built agent with {} tools.", agent.tools().len()); + + let response = agent.run("Say hello.", ()).await?; + println!("Output: {}", response.output()); + Ok(()) +} diff --git a/src/reloaded-code-serdesai/examples/hooks/shared.rs b/src/reloaded-code-serdesai/examples/hooks/shared.rs new file mode 100644 index 0000000..bc06149 --- /dev/null +++ b/src/reloaded-code-serdesai/examples/hooks/shared.rs @@ -0,0 +1,223 @@ +// Fixture module included into each example binary via `#[path]`; binaries +// use different subsets of these helpers, so unused ones are expected. +#![allow(dead_code)] + +use reloaded_code_agents::{ + AgentCatalog, AgentConfig, AgentDefaults, AgentMode, AgentRuntimeBuilder, PermissionRule, +}; +use reloaded_code_core::models::{ + Modality, ModelCatalog, ModelInfo, ProviderIdx, ProviderInfo, ProviderModelSource, + ProviderSource, ProviderType, +}; +use reloaded_code_core::permissions::PermissionAction; +use reloaded_code_core::{CredentialResolver, HookSet, resolve_workspace_root}; +use reloaded_code_serdesai::AgentBuildContext; +use reloaded_code_serdesai::mock::Streamed; +use serdes_ai_models::MockModel; +use std::path::{Path, PathBuf}; +use std::sync::Arc; +use tempfile::TempDir; + +const DEFAULT_MODEL_ID: &str = "openrouter/cutie/patootie"; +/// Contents of the `service.env` fixture. The redaction demo scrubs the +/// `API_KEY=` and `TOKEN=` lines, so the benign `LOG_LEVEL` line must stay +/// to show that plain lines survive the scrub. +const SERVICE_ENV: &str = "\ +# Demo service configuration. +API_KEY=sk-demo-9f41c2a7b8e04d65 +TOKEN=tok-demo-6b83d15a92f0 +LOG_LEVEL=debug +"; + +/// Hermetic temp workspace for tool-hook examples. +/// +/// File tools resolve paths inside [`TempWorkspace::root`] only, so example +/// runs cannot read or write the repository checkout. Dropping the fixture +/// deletes the workspace, so keep it alive for the whole agent run. +pub struct TempWorkspace { + /// Temp directory guard; dropping it deletes the workspace and its files. + pub dir: TempDir, + /// Workspace root, ready to pass to [`AgentBuildContext::new`] or + /// [`build_agent_context_in_workspace`]. + pub root: Arc, + /// Config fixture whose `API_KEY=`/`TOKEN=` lines the redaction demo scrubs. + pub secrets_file: PathBuf, + /// Write target that no example reads first; the fixture never creates it. + pub unread_target: PathBuf, +} + +/// Builds an `AgentConfig` fixture whose permission rules allow the named tools. +/// +/// Agents attach only the standard tools their permission rules explicitly +/// allow, so tool-using examples need this variant instead of +/// [`agent_config`]. +/// +/// # Arguments +/// +/// - `name` - the agent name. +/// - `description` - the agent description. +/// - `prompt` - the agent system prompt. +/// - `tools` - names of the standard tools the agent may call. +pub fn agent_config_with_tools( + name: &str, + description: &str, + prompt: &str, + tools: &[&str], +) -> AgentConfig { + let permission = tools + .iter() + .map(|tool| { + ( + (*tool).into(), + PermissionRule::Action(PermissionAction::Allow), + ) + }) + .collect(); + AgentConfig { + permission, + ..agent_config(name, description, prompt) + } +} + +/// Builds an `AgentBuildContext` wired to mock models and credentials. +/// +/// # Arguments +/// +/// - `catalog` - the agent catalog to attach to the runtime. +/// - `hooks` - the hook set to install on the runtime. +pub fn build_agent_context(catalog: AgentCatalog, hooks: HookSet) -> AgentBuildContext { + let runtime = AgentRuntimeBuilder::new() + .catalog(catalog) + .defaults(AgentDefaults::with_model(DEFAULT_MODEL_ID)) + .hooks(hooks) + .build() + .expect("runtime should build"); + + AgentBuildContext::new( + Arc::new(runtime), + Arc::new(model_catalog()), + mock_credentials(), + workspace_root(), + ) +} + +/// Builds an [`AgentBuildContext`] wired to mock models and credentials, +/// exposing `workspace_root` to tools instead of the repository root. +/// +/// Tool examples pass [`TempWorkspace::root`] so `read` and `write` calls +/// stay inside the temp workspace. +/// +/// # Arguments +/// +/// - `catalog` - the agent catalog to attach to the runtime. +/// - `hooks` - the hook set to install on the runtime. +/// - `workspace_root` - project directory exposed to tools. +/// +/// # Panics +/// +/// Panics when the agent runtime fails to build. +pub fn build_agent_context_in_workspace( + catalog: AgentCatalog, + hooks: HookSet, + workspace_root: Arc, +) -> AgentBuildContext { + let runtime = AgentRuntimeBuilder::new() + .catalog(catalog) + .defaults(AgentDefaults::with_model(DEFAULT_MODEL_ID)) + .hooks(hooks) + .build() + .expect("runtime should build"); + + AgentBuildContext::new( + Arc::new(runtime), + Arc::new(model_catalog()), + mock_credentials(), + workspace_root, + ) +} + +/// Returns a mock model that streams deterministic output. +pub fn mock_model() -> Streamed { + Streamed::new(MockModel::new("mock-model")) +} + +/// Creates a hermetic temp workspace containing the example fixture files. +/// +/// Tool grants still come from [`agent_config_with_tools`]; this fixture only +/// owns the directory and its files. +/// +/// # Panics +/// +/// Panics when the temp directory or a fixture file cannot be created. +pub fn temp_workspace() -> TempWorkspace { + let dir = TempDir::new().expect("create temp workspace"); + let root: Arc = Arc::from(dir.path()); + let secrets_file = dir.path().join("service.env"); + std::fs::write(&secrets_file, SERVICE_ENV).expect("write service.env fixture"); + let unread_target = dir.path().join("draft.md"); + TempWorkspace { + dir, + root, + secrets_file, + unread_target, + } +} + +/// Builds an `AgentConfig` fixture. +/// +/// # Arguments +/// +/// - `name` - the agent name. +/// - `description` - the agent description. +/// - `prompt` - the agent system prompt. +pub fn agent_config(name: &str, description: &str, prompt: &str) -> AgentConfig { + AgentConfig { + name: name.into(), + mode: AgentMode::Primary, + description: description.into(), + model: None, + hidden: false, + temperature: None, + top_p: None, + permission: Default::default(), + options: Default::default(), + tool_settings: Default::default(), + prompt: prompt.into(), + } +} + +/// Returns a credential resolver with a dummy OpenRouter key. +pub fn mock_credentials() -> Arc { + let mut creds = CredentialResolver::new(); + creds.set_override("OPENROUTER_API_KEY", "dummy-key-for-mock"); + Arc::new(creds) +} + +/// Returns a model catalog with a single OpenRouter mock model. +pub fn model_catalog() -> ModelCatalog { + let providers = vec![ProviderSource::new( + "openrouter", + ProviderInfo { + api_url: "https://openrouter.ai/api/v1".into(), + env_vars: vec!["OPENROUTER_API_KEY".into()], + api_type: ProviderType::OpenRouter, + }, + )]; + let info = ModelInfo { + modalities: Modality::TEXT, + max_input: 128_000, + max_output: 16_384, + temperature: Some(1.0), + top_p: Some(0.95), + }; + let models: Vec> = [("cutie/patootie", info)] + .into_iter() + .map(|(key, i)| ProviderModelSource::new(ProviderIdx::new(0), key, i)) + .collect(); + ModelCatalog::build(&providers, &models).expect("catalog fixture should build") +} + +/// Resolves the repository workspace root. +pub fn workspace_root() -> Arc { + Arc::from(resolve_workspace_root().expect("resolve workspace root")) +} diff --git a/src/reloaded-code-serdesai/examples/hooks/tool/serdesai-tool-block.rs b/src/reloaded-code-serdesai/examples/hooks/tool/serdesai-tool-block.rs new file mode 100644 index 0000000..84e9756 --- /dev/null +++ b/src/reloaded-code-serdesai/examples/hooks/tool/serdesai-tool-block.rs @@ -0,0 +1,150 @@ +//! `ToolHook` denies a `write` to a file the run never read. +//! +//! - Turn 1: real `read` of `service.env`. +//! - Turn 2: `write` to `draft.md`, never read. +//! - Hook tracks read files per run in a `Mutex` set keyed by +//! `(run_id, path)`; denies unseen writes without calling +//! [`ToolOriginal`], so the file never lands on disk. +//! - Paths compared raw; real deployments would canonicalize. +//! - Permission rules cannot depend on earlier calls; stateful hooks can. +//! +//! Expected output: +//! Built agent with 2 tools. +//! [ReadBeforeWrite] read recorded: service.env +//! [ReadBeforeWrite] denying write to never-read file: draft.md +//! [ReadBeforeWrite] the real write was not executed +//! [ReadBeforeWrite] draft.md exists after the run: false +//! Output: Run finished. +//! +//! 1: # Demo service configuration. +//! 2: API_KEY=sk-demo-9f41c2a7b8e04d65 +//! 3: TOKEN=tok-demo-6b83d15a92f0 +//! 4: LOG_LEVEL=debug +//! [blocked by hook] write to draft.md denied: no read of that file happened this run +//! +//! Run with: +//! cargo run --example serdesai-tool-block -p reloaded-code-serdesai --features mock + +use reloaded_code_agents::AgentCatalog; +use reloaded_code_core::{ + HookSet, ToolCallContext, ToolHook, ToolHookFuture, ToolOriginal, ToolOutput, ToolRequest, +}; +use reloaded_code_serdesai::mock::two_tools_then_text; +use serde_json::json; +use std::collections::HashSet; +use std::path::PathBuf; +use std::sync::Mutex; + +#[path = "../shared.rs"] +mod shared; + +/// Workspace fixture the scripted model reads first. +const READ_SOURCE: &str = "service.env"; +/// Write target the scripted model never reads first. +const WRITE_TARGET: &str = "draft.md"; + +struct ReadBeforeWrite { + /// Files each run has read, keyed by `(run_id, path)` so a read in one + /// run cannot authorize a write in another. Interior mutability because + /// the shared hook instance fires for every tool call, `read` and + /// `write` alike. + read_files: Mutex>, +} + +impl ToolHook for ReadBeforeWrite { + fn hook<'a>( + &'a self, + ctx: &'a ToolCallContext<'a>, + req: ToolRequest, + original: ToolOriginal<'a>, + ) -> ToolHookFuture<'a> { + Box::pin(async move { + let target = req + .args + .get("file_path") + .and_then(|value| value.as_str()) + .map(PathBuf::from); + + match (ctx.tool_name, target) { + ("read", Some(path)) => { + // Record every read attempt, successful or not, before + // releasing the lock; nothing is held across the await. + self.read_files + .lock() + .expect("read_files should not be poisoned") + .insert((ctx.run_id.to_string(), path.clone())); + println!("[ReadBeforeWrite] read recorded: {}", path.display()); + original.call(ctx, req).await + } + ("write", Some(path)) => { + let was_read = self + .read_files + .lock() + .expect("read_files should not be poisoned") + .contains(&(ctx.run_id.to_string(), path.clone())); + if was_read { + return original.call(ctx, req).await; + } + println!( + "[ReadBeforeWrite] denying write to never-read file: {}", + path.display() + ); + // Skipping `original` is what blocks the call: the real + // `write` sits behind it and is never reached. + println!("[ReadBeforeWrite] the real write was not executed"); + Ok(ToolOutput::new(format!( + "[blocked by hook] write to {} denied: no read of that file happened this run", + path.display() + ))) + } + _ => original.call(ctx, req).await, + } + }) + } +} + +#[tokio::main] +async fn main() -> Result<(), Box> { + let workspace = shared::temp_workspace(); + let hook = ReadBeforeWrite { + read_files: Mutex::new(HashSet::new()), + }; + let hooks = HookSet::builder().tool_hook(hook).build(); + + let catalog = AgentCatalog::from_entries([shared::agent_config_with_tools( + "tool-block-demo", + "tool block demo", + "You are a tool block demo agent.", + &["read", "write"], + )]); + + let build_context = + shared::build_agent_context_in_workspace(catalog, hooks, workspace.root.clone()); + + let model = two_tools_then_text( + ("read", json!({"file_path": READ_SOURCE})), + ( + "write", + json!({"file_path": WRITE_TARGET, "content": "Draft notes."}), + ), + "Run finished.", + ); + let agent = build_context + .with_model_override(model) + .build("tool-block-demo")?; + println!("Built agent with {} tools.", agent.tools().len()); + + let response = agent + .run("Read the config, then write the draft.", ()) + .await?; + + // The denial must be observable on disk, not just in the transcript. + assert!( + !workspace.unread_target.exists(), + "the denied write must not create {}", + workspace.unread_target.display() + ); + println!("[ReadBeforeWrite] {WRITE_TARGET} exists after the run: false"); + println!("Output: {}", response.output()); + Ok(()) +} diff --git a/src/reloaded-code-serdesai/examples/hooks/tool/serdesai-tool-chain.rs b/src/reloaded-code-serdesai/examples/hooks/tool/serdesai-tool-chain.rs new file mode 100644 index 0000000..146f6fa --- /dev/null +++ b/src/reloaded-code-serdesai/examples/hooks/tool/serdesai-tool-chain.rs @@ -0,0 +1,117 @@ +//! Two `ToolHook`s around one real `bash` call. +//! +//! - Outer audit hook: logs args and run context. +//! - Inner hardening hook: injects `timeout_ms`, then calls the real tool. +//! - `echo` writes nothing; no file I/O. +//! - Timeout never fires; effect shows in printed args plus execution. +//! - Permission rules allow/deny; only hooks can log or rewrite args. +//! +//! Expected output: +//! Built agent with 1 tools. +//! [AuditHook] tool=bash agent=tool-chain-demo run_id= args={"command":"echo reloaded tool hooks"} +//! [HardeningHook] original args={"command":"echo reloaded tool hooks"} +//! [HardeningHook] hardened args={"command":"echo reloaded tool hooks","timeout_ms":5000} +//! Output: Tool call finished. +//! +//! reloaded tool hooks +//! +//! Run with: +//! cargo run --example serdesai-tool-chain -p reloaded-code-serdesai --features mock + +use reloaded_code_agents::AgentCatalog; +use reloaded_code_core::{ + HookSet, ToolCallContext, ToolHook, ToolHookFuture, ToolOriginal, ToolRequest, +}; +use reloaded_code_serdesai::mock::tool_then_text; +use serde_json::json; +use std::sync::Arc; + +#[path = "../shared.rs"] +mod shared; + +/// Fixed trivial command the scripted model runs; it writes nothing. +const COMMAND: &str = "echo reloaded tool hooks"; +/// stdout of [`COMMAND`]; only a real bash execution can put it in the +/// model-facing transcript. +const EXPECTED_ECHO: &str = "reloaded tool hooks"; +/// Timeout the hardening hook injects, in milliseconds. +const HARDENED_TIMEOUT_MS: u32 = 5_000; + +struct AuditHook; + +struct HardeningHook; + +impl ToolHook for AuditHook { + fn hook<'a>( + &'a self, + ctx: &'a ToolCallContext<'a>, + req: ToolRequest, + original: ToolOriginal<'a>, + ) -> ToolHookFuture<'a> { + Box::pin(async move { + println!( + "[AuditHook] tool={} agent={} run_id={} args={}", + ctx.tool_name, ctx.agent_name, ctx.run_id, req.args + ); + original.call(ctx, req).await + }) + } +} + +impl ToolHook for HardeningHook { + fn hook<'a>( + &'a self, + ctx: &'a ToolCallContext<'a>, + req: ToolRequest, + original: ToolOriginal<'a>, + ) -> ToolHookFuture<'a> { + Box::pin(async move { + // Clone so the original arguments stay intact for the log line; + // only the hardened clone reaches the real tool. + let mut hardened = req.clone(); + if let Some(args) = hardened.args.as_object_mut() { + args.insert("timeout_ms".into(), json!(HARDENED_TIMEOUT_MS)); + } + println!("[HardeningHook] original args={}", req.args); + println!("[HardeningHook] hardened args={}", hardened.args); + original.call(ctx, hardened).await + }) + } +} + +#[tokio::main] +async fn main() -> Result<(), Box> { + // Registration order is outer-to-inner: the audit hook sees the model's + // original arguments, the hardening hook transforms them right before + // the real bash tool. + let hardening: Arc = Arc::new(HardeningHook); + let hooks = HookSet::builder() + .tool_hook(AuditHook) + .shared_tool_hook(hardening) + .build(); + + let catalog = AgentCatalog::from_entries([shared::agent_config_with_tools( + "tool-chain-demo", + "tool chain demo", + "You are a tool chain demo agent.", + &["bash"], + )]); + + let build_context = shared::build_agent_context(catalog, hooks); + + let model = tool_then_text("bash", json!({"command": COMMAND}), "Tool call finished."); + let agent = build_context + .with_model_override(model) + .build("tool-chain-demo")?; + println!("Built agent with {} tools.", agent.tools().len()); + + let response = agent.run("Run the demo command.", ()).await?; + // Only a real bash execution can echo this into the transcript, so the + // assert fails the run if the hardened call never reached the tool. + assert!( + response.output().contains(EXPECTED_ECHO), + "the hardened bash call should have executed exactly once" + ); + println!("Output: {}", response.output()); + Ok(()) +} diff --git a/src/reloaded-code-serdesai/examples/hooks/tool/serdesai-tool-hook.rs b/src/reloaded-code-serdesai/examples/hooks/tool/serdesai-tool-hook.rs new file mode 100644 index 0000000..3f3764a --- /dev/null +++ b/src/reloaded-code-serdesai/examples/hooks/tool/serdesai-tool-hook.rs @@ -0,0 +1,163 @@ +//! `ToolHook` scrubs secrets from a real `read` result. +//! +//! - Real `read` of `service.env` in a temp workspace. +//! - Hook rewrites secret values to `[REDACTED]` before the model sees them. +//! - Permission rules allow/deny; only a hook can rewrite results. +//! +//! Expected output: +//! Built agent with 1 tools. +//! [SecretRedactor] scrubbed 2 secret values from tool=read of service.env +//! Output: Tool call finished. +//! +//! 1: # Demo service configuration. +//! 2: API_KEY=[REDACTED] +//! 3: TOKEN=[REDACTED] +//! 4: LOG_LEVEL=debug +//! +//! Run with: +//! cargo run --example serdesai-tool-hook -p reloaded-code-serdesai --features mock + +use reloaded_code_agents::AgentCatalog; +use reloaded_code_core::{ + HookSet, ToolCallContext, ToolHook, ToolHookFuture, ToolOriginal, ToolRequest, +}; +use reloaded_code_serdesai::mock::tool_then_text; +use serde_json::json; + +#[path = "../shared.rs"] +mod shared; + +/// Value prefixes of the fixture's secret lines; the final transcript +/// check uses them to prove the raw values never reach the model. +const RAW_SECRET_PREFIXES: [&str; 2] = ["sk-demo-", "tok-demo-"]; +/// Replacement written in place of each scrubbed secret value. +const REDACTED: &str = "[REDACTED]"; +/// Workspace fixture the scripted model reads. +const SECRETS_FILE: &str = "service.env"; +/// Assignment keys whose values get scrubbed from tool results. +/// +/// Deliberately example-local: a fixed key list, not a secret catalog or a +/// general scanning policy. +const SECRET_KEYS: [&str; 2] = ["API_KEY", "TOKEN"]; + +struct SecretRedactor; + +impl ToolHook for SecretRedactor { + fn hook<'a>( + &'a self, + ctx: &'a ToolCallContext<'a>, + req: ToolRequest, + original: ToolOriginal<'a>, + ) -> ToolHookFuture<'a> { + Box::pin(async move { + let mut output = original.call(ctx, req).await?; + let (scrubbed, count) = scrub_secrets(&output.content); + // The fixture has exactly one assignment line per secret key, so + // any other count means the scrub or the fixture drifted. + assert_eq!( + count, + SECRET_KEYS.len(), + "expected to scrub one value per secret key" + ); + println!( + "[SecretRedactor] scrubbed {count} secret values from tool={} of {SECRETS_FILE}", + ctx.tool_name + ); + output.content = scrubbed; + Ok(output) + }) + } +} + +#[tokio::main] +async fn main() -> Result<(), Box> { + let workspace = shared::temp_workspace(); + let hooks = HookSet::builder().tool_hook(SecretRedactor).build(); + + let catalog = AgentCatalog::from_entries([shared::agent_config_with_tools( + "tool-hook-demo", + "tool hook demo", + "You are a tool hook demo agent.", + &["read"], + )]); + + let build_context = + shared::build_agent_context_in_workspace(catalog, hooks, workspace.root.clone()); + + let model = tool_then_text( + "read", + json!({"file_path": SECRETS_FILE}), + "Tool call finished.", + ); + let agent = build_context + .with_model_override(model) + .build("tool-hook-demo")?; + println!("Built agent with {} tools.", agent.tools().len()); + + let response = agent.run("Read the service configuration.", ()).await?; + // The scrubbed values must reach the model, not just be computed: a raw + // fixture prefix in the transcript means propagation dropped the rewrite. + for prefix in RAW_SECRET_PREFIXES { + assert!( + !response.output().contains(prefix), + "the model-facing transcript must not contain the raw secret value {prefix:?}" + ); + } + println!("Output: {}", response.output()); + Ok(()) +} + +/// Replaces the values of `KEY=`-assignment lines with `[REDACTED]`. +/// +/// Returns the scrubbed text plus the number of values replaced. Lines that +/// assign to none of [`SECRET_KEYS`] pass through unchanged, which is why the +/// benign `LOG_LEVEL` line survives in the printed output. +fn scrub_secrets(content: &str) -> (String, usize) { + let mut count = 0; + let mut scrubbed = String::with_capacity(content.len()); + for line in content.lines() { + match scrub_line(line) { + Some(redacted) => { + count += 1; + scrubbed.push_str(&redacted); + } + None => scrubbed.push_str(line), + } + scrubbed.push('\n'); + } + // `lines()` drops the shape of the final newline; restore it so the + // scrubbed result keeps the original content's exact tail. + if !content.ends_with('\n') { + scrubbed.pop(); + } + (scrubbed, count) +} + +/// Scrubs one line when it assigns to a secret key, else returns `None`. +/// +/// Tolerates the read tool's `{number}: ` line prefix so numbered output +/// stays aligned with the file's line numbers. +fn scrub_line(line: &str) -> Option { + let (prefix, body) = split_number_prefix(line); + let key = SECRET_KEYS + .iter() + .find(|key| body.starts_with(*key) && body[key.len()..].starts_with('='))?; + Some(format!("{prefix}{key}={REDACTED}")) +} + +/// Splits a leading, possibly padded `{number}: ` prefix off a line. +/// +/// The read tool pads line numbers to a fixed width, so the digits can be +/// preceded by spaces. Returns empty-string prefix when the line has no +/// such prefix. +fn split_number_prefix(line: &str) -> (&str, &str) { + let padding = line.len() - line.trim_start().len(); + let rest = &line[padding..]; + if let Some(index) = rest.find(": ") { + let digits = &rest[..index]; + if !digits.is_empty() && digits.bytes().all(|byte| byte.is_ascii_digit()) { + return line.split_at(padding + index + 2); + } + } + ("", line) +} diff --git a/src/reloaded-code-serdesai/src/agent_ext.rs b/src/reloaded-code-serdesai/src/agent_ext.rs index 1b6d152..5c26ffe 100644 --- a/src/reloaded-code-serdesai/src/agent_ext.rs +++ b/src/reloaded-code-serdesai/src/agent_ext.rs @@ -22,22 +22,57 @@ use crate::AgentBuildError; use async_trait::async_trait; +use reloaded_code_core::hooks::{ + HookSet, ToolCallContext, ToolExecutor as CoreToolExecutor, ToolHookFuture, ToolRequest, +}; use serde_json::Value as JsonValue; use serdes_ai::agent::ToolExecutor; use serdes_ai::tools::{RunContext as ToolsRunContext, Tool, ToolError, ToolReturn}; use serdes_ai::{AgentBuilder, RunContext as AgentRunContext}; +use std::sync::Arc; + +/// Bridges a SerdesAI `ToolExecutor` back to the core `ToolExecutor` trait so +/// [`HookSet::dispatch_tool`] can call the real tool at the end of the hook chain. +/// +/// Borrows the original execution context to avoid cloning strings and settings. +struct CoreToolBridge<'a, Deps> { + inner: &'a dyn serdes_ai::agent::ToolExecutor, + ctx: &'a AgentRunContext, + captured: &'a CapturedToolResult, +} /// Adapter for boxed trait object tools, similar to [`ToolAsExecutor`] but /// for dynamically dispatched tools where the concrete type is not known /// at compile time. -struct DynToolAsExecutor(Box + Send + Sync>); +pub(crate) struct DynToolAsExecutor(pub(crate) Box + Send + Sync>); + +/// Wraps a SerdesAI [`ToolExecutor`] so that [`HookSet::dispatch_tool`] is called +/// before the inner executor when tool hooks are registered. Pass-through when +/// no tool hooks are present. +pub(crate) struct HookedToolExecutor { + inner: Arc + Send + Sync>, + hooks: HookSet, + agent_name: String, + tool_name: &'static str, +} /// Adapter that wraps a [`Tool`] to implement [`ToolExecutor`]. /// /// This bridges the gap between `serdes_ai::tools::Tool` (which uses /// `tools::RunContext`) and `serdes_ai::agent::ToolExecutor` (which uses /// `agent::RunContext`). -struct ToolAsExecutor(T); +pub(crate) struct ToolAsExecutor(T); + +/// Original tool result captured by [`CoreToolBridge`] while the hook chain +/// runs. Lets [`HookedToolExecutor`] restore the untouched `ToolReturn` or +/// `ToolError` after dispatch, so JSON shapes, truncated markers, image +/// content, `tool_call_id`, and structured validation errors reach the model +/// exactly as the no-hook path would deliver them. +#[derive(Default)] +struct CapturedToolResult { + return_value: std::sync::Mutex>, + error: std::sync::Mutex>, +} /// Extension trait for [`AgentBuilder`] to add tools that implement [`Tool`]. pub trait AgentBuilderExt { @@ -93,14 +128,78 @@ pub trait ToolResultExt { /// Maps a [`ToolError`] to /// [`AgentBuildError::ToolSettingsValidation`]. /// - /// [`ToolError`]: reloaded_code_core::ToolError - /// /// # Errors /// - Returns [`AgentBuildError::ToolSettingsValidation`] when the original result /// contains a [`ToolError`], preserving the tool name and original error. + /// + /// [`ToolError`]: reloaded_code_core::ToolError fn with_tool(self, tool: &'static str) -> Result; } +impl HookedToolExecutor { + pub(crate) fn new + 'static>( + tool: T, + hooks: &HookSet, + agent_name: &str, + tool_name: &'static str, + ) -> Self + where + Deps: Send + Sync + 'static, + { + Self { + inner: Arc::new(ToolAsExecutor(tool)), + hooks: hooks.clone(), + agent_name: agent_name.into(), + tool_name, + } + } + + pub(crate) fn from_dyn( + executor: Box + Send + Sync>, + hooks: &HookSet, + agent_name: &str, + tool_name: &'static str, + ) -> Self { + Self { + inner: Arc::from(executor), + hooks: hooks.clone(), + agent_name: agent_name.into(), + tool_name, + } + } +} + +impl<'a, Deps: Send + Sync + 'static> CoreToolExecutor for CoreToolBridge<'a, Deps> { + fn execute<'b>( + &'b self, + _ctx: &'b ToolCallContext<'b>, + req: ToolRequest, + ) -> ToolHookFuture<'b> { + let inner = self.inner; + let ctx = self.ctx; + let captured = self.captured; + let args = req.args; + Box::pin(async move { + match inner.execute(args, ctx).await { + Ok(tool_return) => { + // Hooks see the text projection; the original is restored + // after dispatch when the output is untouched. + let output = crate::convert::return_to_output(&tool_return); + *captured.return_value.lock().expect("capture lock") = Some(tool_return); + Ok(output) + } + Err(err) => { + // Hooks see a core error projection; the original is + // restored after dispatch when the error is untouched. + let core_err = crate::convert::serdes_error_to_core(&err); + *captured.error.lock().expect("capture lock") = Some(err); + Err(core_err) + } + } + }) + } +} + #[async_trait] impl ToolExecutor for DynToolAsExecutor { async fn execute( @@ -120,6 +219,76 @@ impl ToolExecutor for DynToolAsExecutor } } +// Derived from serdes-ai trait signatures: +// - [`AgentBuilder::tool_with_executor`] stores a `dyn serdes_ai::agent::ToolExecutor`. +// - [`ToolExecutor::execute`] receives `(args: JsonValue, ctx: &RunContext)` and returns `Result`. +// - [`HookSet::dispatch_tool`] needs a `dyn reloaded_code_core::hooks::ToolExecutor`, +// whose [`execute`](reloaded_code_core::hooks::ToolExecutor::execute) takes `(ctx: &ToolCallContext, req: ToolRequest)` and returns a pinned future. +// +// [`HookedToolExecutor`] wraps the stored serdes-ai executor. When hooks are present, +// it adapts args to [`ToolRequest`], borrows the incoming [`AgentRunContext`] by reference, +// and passes a [`CoreToolBridge`] as the core [`ToolExecutor`](reloaded_code_core::hooks::ToolExecutor). The bridge delegates back +// to the original serdes-ai executor inside the hook chain, so hooks can intercept +// or wrap the real tool call. +// +// When no hooks are registered, this passes through directly to `inner.execute(args, ctx)` +// with no extra allocations. +#[async_trait] +impl serdes_ai::agent::ToolExecutor + for HookedToolExecutor +{ + async fn execute( + &self, + args: JsonValue, + ctx: &AgentRunContext, + ) -> Result { + if self.hooks.tool_hooks_is_empty() { + return self.inner.execute(args, ctx).await; + } + let tool_ctx = ToolCallContext { + tool_name: self.tool_name, + agent_name: &self.agent_name, + run_id: &ctx.run_id, + }; + let tool_req = ToolRequest::new(args); + let captured = CapturedToolResult::default(); + let bridge = CoreToolBridge { + inner: &*self.inner, + ctx, + captured: &captured, + }; + let result = self.hooks.dispatch_tool(&tool_ctx, tool_req, &bridge).await; + match result { + Ok(output) => { + // Untouched pass-through: hand back the original `ToolReturn` + // so image content, `tool_call_id`, and exact JSON survive. + if let Some(original) = captured.return_value.lock().expect("capture lock").take() + && output_matches_original(&output, &original) + { + return Ok(original); + } + Ok(crate::convert::output_to_return(output)) + } + Err(core_err) => { + // Untouched inner failure: hand back the original `ToolError` + // so structured validation details survive. Hook-produced + // errors (different message) still convert through the core + // mapping. + if let Some(original) = captured.error.lock().expect("capture lock").take() + && crate::convert::serdes_error_to_core(&original).to_string() + == core_err.to_string() + { + return Err(original); + } + Err(crate::convert::core_error_to_serdes( + self.tool_name, + core_err, + )) + } + } + } +} + #[async_trait] impl> ToolExecutor for ToolAsExecutor { async fn execute( @@ -167,3 +336,10 @@ impl ToolResultExt for Result { self.map_err(|source| AgentBuildError::ToolSettingsValidation { tool, source }) } } + +/// True when the hook-chain output is byte-identical to the text projection +/// of the original tool return, meaning no hook modified it. +fn output_matches_original(output: &reloaded_code_core::ToolOutput, original: &ToolReturn) -> bool { + let projection = crate::convert::return_to_output(original); + output.content == projection.content && output.truncated == projection.truncated +} diff --git a/src/reloaded-code-serdesai/src/agent_runtime/build.rs b/src/reloaded-code-serdesai/src/agent_runtime/build.rs index 68fa5dc..9a4257d 100644 --- a/src/reloaded-code-serdesai/src/agent_runtime/build.rs +++ b/src/reloaded-code-serdesai/src/agent_runtime/build.rs @@ -8,7 +8,7 @@ use super::model::resolve_model; use super::provider_bridge::build_serdes_model; -use crate::agent_ext::{AgentBuilderExt, ToolResultExt}; +use crate::agent_ext::{HookedToolExecutor, ToolResultExt}; use crate::task::{TaskHandle, TaskTool}; use crate::tools::CustomToolAdapter; use crate::{ @@ -26,14 +26,15 @@ use reloaded_code_core::context::ToolPrompt; use reloaded_code_core::permissions::Ruleset; use reloaded_code_core::tool_context::ToolBuildContext; use reloaded_code_core::tool_metadata::{ - edit as edit_meta, glob as glob_meta, grep as grep_meta, read as read_meta, + bash as bash_meta, edit as edit_meta, glob as glob_meta, grep as grep_meta, read as read_meta, + task as task_meta, todo_read as todo_read_meta, todo_write as todo_write_meta, webfetch as webfetch_meta, write as write_meta, }; use reloaded_code_core::tools::{ GlobSettings, GrepFormattingSettings, GrepSettings, ReadSettings, WebFetchSettings, }; use reloaded_code_core::{ - CredentialLookup, SharedToolRegistry, ToolCatalogEntry, ToolCatalogKind, ToolError, + CredentialLookup, HookSet, SharedToolRegistry, ToolCatalogEntry, ToolCatalogKind, ToolError, models::ModelCatalog, }; use serdes_ai::AgentBuilder; @@ -138,6 +139,15 @@ impl PreparedBuild<'_> { /// Attaches the standard runtime tools and prompt contexts without finalizing the builder. /// +/// # Arguments +/// - `builder`: The [`AgentBuilder`] to attach tools onto. +/// - `prepared`: Catalog-prepared build data (resolved model, tools, prompt, etc.). +/// - `task_handle`: Optional handle for Task delegation support. +/// - `workspace_root`: Project root directory exposed to tools. +/// - `bash_sandbox`: Optional pre-built sandbox profile for [`BashTool`]. +/// - `custom_tool_registry`: Registry of user-defined portable custom tools. +/// - `hooks`: The [`HookSet`] to wrap tool executors with hook dispatch. +/// /// # Errors /// /// Returns [`AgentBuildError::UnsupportedToolKind`] when the runtime catalog contains an @@ -149,6 +159,11 @@ impl PreparedBuild<'_> { /// Returns [`AgentBuildError::CustomToolCreateFailed`] when a custom-tool /// factory cannot create its portable tool object. /// +/// Returns [`AgentBuildError::CustomToolNameMismatch`] when a custom tool's +/// name (from [`reloaded_code_core::CustomTool`] or +/// [`reloaded_code_core::CustomToolDefinition`]) does not match the catalog +/// entry name. +/// /// Returns [`AgentBuildError::ToolSettingsValidation`] when resolver creation or settings /// building fails for any tool, including: /// - [`ToolError::InvalidPath`] if the workspace root cannot be canonicalized @@ -161,6 +176,7 @@ pub(super) fn attach_standard_tools<'a, C>( workspace_root: &Path, bash_sandbox: Option<&Arc>, custom_tool_registry: &SharedToolRegistry, + hooks: &HookSet, ) -> Result<(AgentBuilder<(), String>, SystemPromptBuilder), AgentBuildError> where C: CredentialLookup + Send + Sync + 'static, @@ -195,28 +211,62 @@ where build_resolver_for_tool(&build_context, permission_config, read_meta::NAME) .with_tool(read_meta::NAME)?; let settings = build_read_settings(&prepared.tool_settings.read)?; - builder = - builder.tool(prompt_builder.track(ReadTool::with_settings(resolver, settings))); + let tool = ReadTool::with_settings(resolver, settings); + builder = builder.tool_with_executor( + serdes_ai::Tool::<()>::definition(&tool), + HookedToolExecutor::new( + prompt_builder.track(tool), + hooks, + prepared.agent_name.as_ref(), + read_meta::NAME, + ), + ); } ToolCatalogKind::Write => { let resolver = build_resolver_for_tool(&build_context, permission_config, write_meta::NAME) .with_tool(write_meta::NAME)?; - builder = builder.tool(prompt_builder.track(WriteTool::new(resolver))); + let tool = WriteTool::new(resolver); + builder = builder.tool_with_executor( + serdes_ai::Tool::<()>::definition(&tool), + HookedToolExecutor::new( + prompt_builder.track(tool), + hooks, + prepared.agent_name.as_ref(), + write_meta::NAME, + ), + ); } ToolCatalogKind::Edit => { let resolver = build_resolver_for_tool(&build_context, permission_config, edit_meta::NAME) .with_tool(edit_meta::NAME)?; - builder = builder.tool(prompt_builder.track(EditTool::new(resolver))); + let tool = EditTool::new(resolver); + builder = builder.tool_with_executor( + serdes_ai::Tool::<()>::definition(&tool), + HookedToolExecutor::new( + prompt_builder.track(tool), + hooks, + prepared.agent_name.as_ref(), + edit_meta::NAME, + ), + ); } ToolCatalogKind::Glob => { let resolver = build_resolver_for_tool(&build_context, permission_config, glob_meta::NAME) .with_tool(glob_meta::NAME)?; let settings = build_glob_settings(&prepared.tool_settings.glob)?; - builder = - builder.tool(prompt_builder.track(GlobTool::with_settings(resolver, settings))); + let tool = GlobTool::with_settings(resolver, settings); + builder = builder.tool_with_executor( + serdes_ai::Tool::<()>::definition(&tool), + HookedToolExecutor::new( + prompt_builder.track(tool), + hooks, + prepared.agent_name.as_ref(), + glob_meta::NAME, + ), + ); } ToolCatalogKind::Grep => { let resolver = @@ -224,11 +274,16 @@ where .with_tool(grep_meta::NAME)?; let (search_settings, formatting_settings) = build_grep_settings(&prepared.tool_settings.grep)?; - builder = builder.tool(prompt_builder.track(GrepTool::with_settings( - resolver, - search_settings, - formatting_settings, - ))); + let tool = GrepTool::with_settings(resolver, search_settings, formatting_settings); + builder = builder.tool_with_executor( + serdes_ai::Tool::<()>::definition(&tool), + HookedToolExecutor::new( + prompt_builder.track(tool), + hooks, + prepared.agent_name.as_ref(), + grep_meta::NAME, + ), + ); } ToolCatalogKind::Bash => { let settings = &prepared.tool_settings.bash; @@ -240,27 +295,69 @@ where if let Some(profile) = bash_sandbox { tool = tool.with_linux_bwrap(profile.clone()); } - builder = builder.tool(prompt_builder.track(tool)); + builder = builder.tool_with_executor( + serdes_ai::Tool::<()>::definition(&tool), + HookedToolExecutor::new( + prompt_builder.track(tool), + hooks, + prepared.agent_name.as_ref(), + bash_meta::NAME, + ), + ); } ToolCatalogKind::WebFetch => { let settings = build_webfetch_settings(&prepared.tool_settings.webfetch)?; - builder = builder.tool(prompt_builder.track(WebFetchTool::with_settings(settings))); + let tool = WebFetchTool::with_settings(settings); + builder = builder.tool_with_executor( + serdes_ai::Tool::<()>::definition(&tool), + HookedToolExecutor::new( + prompt_builder.track(tool), + hooks, + prepared.agent_name.as_ref(), + webfetch_meta::NAME, + ), + ); } ToolCatalogKind::TodoRead => { - builder = builder.tool(prompt_builder.track(todo_read.clone())) + builder = builder.tool_with_executor( + serdes_ai::Tool::<()>::definition(&todo_read), + HookedToolExecutor::new( + prompt_builder.track(todo_read.clone()), + hooks, + prepared.agent_name.as_ref(), + todo_read_meta::NAME, + ), + ); } ToolCatalogKind::TodoWrite => { - builder = builder.tool(prompt_builder.track(todo_write.clone())) + builder = builder.tool_with_executor( + serdes_ai::Tool::<()>::definition(&todo_write), + HookedToolExecutor::new( + prompt_builder.track(todo_write.clone()), + hooks, + prepared.agent_name.as_ref(), + todo_write_meta::NAME, + ), + ); } ToolCatalogKind::Task => { if let Some(task_handle) = task_handle && !prepared.callable_target_summaries.is_empty() { - builder = builder.tool(prompt_builder.track(TaskTool::new( + let tool = TaskTool::new( prepared.agent_name.as_ref(), prepared.callable_target_summaries.clone(), (*task_handle).clone(), - ))); + ); + builder = builder.tool_with_executor( + serdes_ai::Tool::<()>::definition(&tool), + HookedToolExecutor::new( + prompt_builder.track(tool), + hooks, + prepared.agent_name.as_ref(), + task_meta::NAME, + ), + ); } } ToolCatalogKind::Custom => { @@ -302,8 +399,19 @@ where } let serdes_definition = crate::convert::custom_definition_to_serdes(definition); - builder = - builder.tool_dyn(serdes_definition, Box::new(CustomToolAdapter::new(tool))); + let tool_adapter = CustomToolAdapter::new(tool); + let tool_name = reloaded_code_core::ToolContext::name(&tool_adapter); + let executor = + Box::new(crate::agent_ext::DynToolAsExecutor(Box::new(tool_adapter))); + builder = builder.tool_with_executor( + serdes_definition, + HookedToolExecutor::from_dyn( + executor, + hooks, + prepared.agent_name.as_ref(), + tool_name, + ), + ); } _ => { return Err(AgentBuildError::UnsupportedToolKind { @@ -406,7 +514,9 @@ fn build_webfetch_settings( #[cfg(test)] mod tests { - use super::{AgentBuildError, attach_standard_tools, prepare_build}; + use super::AgentBuildError; + use super::{attach_standard_tools, prepare_build}; + use crate::agent_runtime::test_stubs::{agent, allow_tools, catalog, credentials}; use ahash::AHashMap; use indexmap::IndexMap; use reloaded_code_agents::{ @@ -414,18 +524,14 @@ mod tests { AgentToolSettings, PermissionRule, }; use reloaded_code_core::context::{ToolContext, ToolPrompt}; - use reloaded_code_core::models::{ - Modality, ModelCatalog, ModelInfo, ProviderIdx, ProviderInfo, ProviderModelSource, - ProviderSource, ProviderType, - }; - use reloaded_code_core::permissions::{ExpandError, PermissionAction}; + use reloaded_code_core::permissions::ExpandError; use reloaded_code_core::tool_metadata::{ bash as bash_meta, glob as glob_meta, grep as grep_meta, read as read_meta, }; use reloaded_code_core::{ - CredentialResolver, CustomTool, CustomToolDefinition, CustomToolFuture, SharedToolRegistry, - ToolBuildContext, ToolCatalogEntry, ToolCatalogKind, ToolError, ToolFactory, ToolOutput, - ToolResult, ToolRunContext, + CredentialResolver, CustomTool, CustomToolDefinition, CustomToolFuture, HookSet, + SharedToolRegistry, ToolBuildContext, ToolCatalogEntry, ToolCatalogKind, ToolError, + ToolFactory, ToolOutput, ToolResult, ToolRunContext, }; use serdes_ai::AgentBuilder; use serdes_ai_models::MockModel; @@ -458,33 +564,13 @@ mod tests { &workspace_root, None, registry, + &HookSet::default(), )?; let prompt = prompt_builder.build(); let agent = builder.system_prompt(prompt.clone()).build(); Ok((agent, prompt)) } - /// Creates a minimal agent config with no model or sampling overrides. - fn agent( - name: &str, - permission: IndexMap, - prompt: &str, - ) -> AgentConfig { - AgentConfig { - name: name.into(), - mode: AgentMode::Primary, - description: format!("{name} description").into(), - model: None, - hidden: false, - temperature: None, - top_p: None, - permission, - options: AHashMap::new(), - tool_settings: AgentToolSettings::default(), - prompt: prompt.into(), - } - } - /// Creates an agent config with explicit model and sampling settings. fn agent_with_sampling( name: &str, @@ -495,59 +581,14 @@ mod tests { prompt: &str, ) -> AgentConfig { AgentConfig { - name: name.into(), mode: AgentMode::All, - description: format!("{name} description").into(), model: Some(model.into()), - hidden: false, temperature, top_p, - permission, - options: AHashMap::new(), - tool_settings: AgentToolSettings::default(), - prompt: prompt.into(), + ..agent(name, AgentMode::Primary, permission, prompt) } } - /// Creates permission rules that allow the specified tools. - fn allow_tools(names: &[&str]) -> IndexMap { - names - .iter() - .map(|n| ((*n).into(), PermissionRule::Action(PermissionAction::Allow))) - .collect() - } - - /// Creates a model catalog with two OpenRouter models for testing. - fn catalog() -> ModelCatalog { - let providers = vec![ProviderSource::new( - "openrouter", - ProviderInfo { - api_url: "https://openrouter.ai/api/v1".into(), - env_vars: vec!["OPENROUTER_API_KEY".into()], - api_type: ProviderType::OpenRouter, - }, - )]; - let info = ModelInfo { - modalities: Modality::TEXT, - max_input: 128_000, - max_output: 16_384, - temperature: Some(1.0), - top_p: Some(0.95), - }; - let models: Vec> = - [("openai/gpt-4.1-mini", info), ("openai/gpt-4o", info)] - .into_iter() - .map(|(key, i)| ProviderModelSource::new(ProviderIdx::new(0), key, i)) - .collect(); - ModelCatalog::build(&providers, &models).expect("catalog fixture should build") - } - - fn credentials() -> CredentialResolver { - let mut credentials = CredentialResolver::without_env(); - credentials.set_override("OPENROUTER_API_KEY", "openrouter-key"); - credentials - } - /// Builds a test runtime with one custom tool and read permission. fn custom_tool_runtime( agent_name: &str, @@ -557,6 +598,7 @@ mod tests { AgentRuntimeBuilder::new() .catalog(AgentCatalog::from_entries([agent( agent_name, + AgentMode::Primary, allow_tools(&[read_meta::NAME, custom_name]), "prompt", )])) @@ -597,10 +639,11 @@ mod tests { .catalog(AgentCatalog::from_entries([ agent( "with-tools", + AgentMode::Primary, allow_tools(&[read_meta::NAME, bash_meta::NAME]), "prompt", ), - agent("no-tools", IndexMap::new(), "prompt"), + agent("no-tools", AgentMode::Primary, IndexMap::new(), "prompt"), ])) .defaults(AgentDefaults::with_model("openrouter/openai/gpt-4.1-mini")) .build()?; @@ -714,6 +757,7 @@ mod tests { let runtime_true = AgentRuntimeBuilder::new() .catalog(AgentCatalog::from_entries([agent( "numbered", + AgentMode::Primary, allow_tools(&[read_meta::NAME, grep_meta::NAME]), "prompt", )])) @@ -786,6 +830,7 @@ mod tests { let runtime = AgentRuntimeBuilder::new() .catalog(AgentCatalog::from_entries([agent( "tester", + AgentMode::Primary, allow_tools(&[read_meta::NAME, "custom_missing"]), "prompt", )])) diff --git a/src/reloaded-code-serdesai/src/agent_runtime/mod.rs b/src/reloaded-code-serdesai/src/agent_runtime/mod.rs index 4d52bd2..b128cf5 100644 --- a/src/reloaded-code-serdesai/src/agent_runtime/mod.rs +++ b/src/reloaded-code-serdesai/src/agent_runtime/mod.rs @@ -6,6 +6,7 @@ //! //! # Public API //! - [`AgentBuildContext`] - Shared context that builds runnable agents by name. +//! - [`HookedAgent`] - Built agent wrapper that dispatches through run hooks. //! - [`AgentBuildError`] - Build-time failures. pub use build::AgentBuildError; @@ -13,7 +14,7 @@ pub use reloaded_code_agents::{ AgentDefaults, AgentRuntime, AgentRuntimeBuilder, ModelResolutionError, ResolvedModel, resolve_model_with_catalog, }; -pub use task::AgentBuildContext; +pub use task::{AgentBuildContext, HookedAgent, HookedAgentRunResult}; pub(crate) use task::{TaskBuildContext, build_agent}; mod build; @@ -21,4 +22,4 @@ mod model; mod provider_bridge; mod task; #[cfg(test)] -mod test_stubs; +pub(crate) mod test_stubs; diff --git a/src/reloaded-code-serdesai/src/agent_runtime/task.rs b/src/reloaded-code-serdesai/src/agent_runtime/task.rs index c295a8f..39e04f9 100644 --- a/src/reloaded-code-serdesai/src/agent_runtime/task.rs +++ b/src/reloaded-code-serdesai/src/agent_runtime/task.rs @@ -2,20 +2,28 @@ //! //! # Public API //! - [`AgentBuildContext`] - Reusable shared inputs for building runnable agents. +//! - [`HookedAgent`] - Built agent wrapper that dispatches through run hooks. #[cfg(not(all(feature = "linux-bubblewrap", target_os = "linux")))] use super::build::Profile; use super::build::{AgentBuildError, attach_standard_tools, prepare_build}; use crate::task::TaskHandle; +use futures::Stream; use reloaded_code_agents::AgentRuntime; #[cfg(all(feature = "linux-bubblewrap", target_os = "linux"))] use reloaded_code_bubblewrap::{CreateSandboxError, Preset, Profile, TempSandboxDirs}; +use reloaded_code_core::hooks::{ + EndReason, HookRunContext, HookSet, ModelSettingsOverrides, PreambleRole, RunConfig, + RunExecutor, RunHookFuture, RunOutput, RunUsage, +}; use reloaded_code_core::{CredentialLookup, CredentialResolver, models::ModelCatalog}; -use serdes_ai::{Agent, AgentBuilder}; +use serdes_ai::core::ModelRequest; +use serdes_ai::{Agent, AgentBuilder, RunOptions}; #[cfg(any(test, feature = "mock"))] use serdes_ai_models::BoxedModel; use std::path::Path; -use std::sync::Arc; +use std::pin::Pin; +use std::sync::{Arc, Mutex}; /// Reusable shared inputs for building runnable SerdesAI agents. /// @@ -28,6 +36,60 @@ pub struct AgentBuildContext>, } +/// Lightweight newtype around a built SerdesAI `Agent` that dispatches +/// `run()` and `run_stream()` through the core `HookSet::dispatch_run` hook +/// chain when run hooks are registered. Passes through directly when no hooks +/// are present for zero overhead. +pub struct HookedAgent { + inner: Agent<(), String>, + hooks: HookSet, + agent_name: String, + model_name: String, +} + +/// Result type returned by `HookedAgent::run`. Provides `.output()` and +/// `.into_output()` so existing call sites compile without changes. +pub struct HookedAgentRunResult { + content: String, +} + +/// RunExecutor that calls the inner SerdesAI agent synchronously (non-stream). +/// +/// Applies `RunConfig::preamble_messages` and `system_prompt` to the prompt +/// text before calling the agent, because the built agent does not support +/// runtime mutation of those fields. Applies `model_settings_overrides` to +/// the per-run model settings via [`RunOptions`], merged over the agent's +/// configured settings. +/// +/// On inner failure the hook chain sees a [`ToolError::Execution`] projection +/// while the original [`AgentRunError`] is parked in `error`; the dispatch +/// site in `run_with_extras` restores the original when the failure reaches +/// the caller untouched. +/// +/// [`ToolError::Execution`]: reloaded_code_core::ToolError::Execution +/// [`AgentRunError`]: serdes_ai::agent::AgentRunError +struct SerdesRunExecutor<'a> { + agent: &'a Agent<(), String>, + prompt: String, + deps: (), + /// Slot the executor fills with the inner response's run metadata. + extras: Arc>>, + /// Slot the executor fills with the inner run's original failure. + error: Arc>>, +} + +/// Inner-agent run metadata captured alongside the output so streaming +/// callers can emit a faithful `RunComplete` event. +#[derive(Default)] +struct AgentRunExtras { + /// Run identifier assigned by the inner agent, or the wrapper's + /// identifier when a hook replaced the run without calling `original`. + run_id: String, + /// Complete message history from the inner run; empty when a hook + /// replaced the run without calling `original`. + messages: Vec, +} + /// Shared owned state for builds that may happen later during Task delegation. #[derive(Clone)] pub(crate) struct TaskBuildContext @@ -57,13 +119,13 @@ where /// For sandboxed builds on Linux with the `linux-bubblewrap` feature, use /// `new_with_sandbox` or `new_with_temp_sandbox` instead. /// - /// [`BashTool`]: crate::BashTool - /// /// # Arguments /// - `runtime`: Shared agent runtime holding the catalog and defaults. /// - `model_catalog`: Available models for agent resolution. /// - `credentials`: Credential lookup used to authenticate model requests. /// - `workspace_root`: Project directory exposed to tools. + /// + /// [`BashTool`]: crate::BashTool pub fn new( runtime: Arc, model_catalog: Arc, @@ -96,13 +158,15 @@ where /// - `model_catalog`: Available models for agent resolution. /// - `credentials`: Credential lookup used to authenticate model requests. /// - `workspace_root`: Project directory exposed to tools. - /// - `profile`: Pre-built sandbox profile for [`BashTool`](crate::BashTool). + /// - `profile`: Pre-built sandbox profile for [`BashTool`]. /// - `sandbox_tmpdir`: Optional owning temp directories that keep the /// profile's backing storage alive for the context's lifetime. /// /// # Platform /// /// Only available on Linux with the `linux-bubblewrap` feature enabled. + /// + /// [`BashTool`]: crate::BashTool #[cfg(all(feature = "linux-bubblewrap", target_os = "linux"))] pub fn new_with_sandbox( runtime: Arc, @@ -179,7 +243,7 @@ where /// - `name`: Catalog entry name to build. /// /// # Returns - /// - `Ok(`[`Agent`]`)`: A fully constructed agent ready to run. + /// - `Ok(`[`HookedAgent`]`)`: A fully constructed agent ready to run. /// /// # Errors /// - Returns [`AgentBuildError::UnknownAgent`] when `name` is not in the @@ -194,10 +258,12 @@ where /// contains a tool kind this adapter cannot materialise. /// - Returns [`AgentBuildError::UnknownCustomTool`] when a custom tool /// entry names a tool absent from the custom-tool registry. + /// - Returns [`AgentBuildError::CustomToolNameMismatch`] when a custom + /// tool's name does not match its catalog entry name. /// - Returns [`AgentBuildError::CustomToolCreateFailed`] when a /// custom-tool factory cannot create its portable tool object. #[inline] - pub fn build(&self, name: &str) -> Result, AgentBuildError> { + pub fn build(&self, name: &str) -> Result { build_agent(Arc::clone(&self.context), name, 0) } @@ -240,6 +306,199 @@ where } } +impl HookedAgent { + /// Creates the wrapper from a built agent and its runtime metadata. + pub(crate) fn new( + inner: Agent<(), String>, + hooks: HookSet, + agent_name: String, + model_name: String, + ) -> Self { + Self { + inner, + hooks, + agent_name, + model_name, + } + } + + /// Returns attached tool definitions (delegates to inner agent). + pub fn tools(&self) -> Vec<&serdes_ai::ToolDefinition> { + self.inner.tools() + } + + /// Runs the agent with the given prompt, dispatching through run hooks. + /// + /// When no run hooks are registered this delegates directly to the inner + /// agent for zero overhead. Otherwise it builds a `RunConfig`, runs the + /// hook chain, applies any `preamble_messages` or `system_prompt` + /// mutations to the prompt text, applies `model_settings_overrides` to + /// the per-run model settings, and returns the result. + /// + /// The run-hook context carries a wrapper-generated `run_id`. The inner + /// agent assigns its own id for tool hooks; SerdesAI `RunOptions` has no + /// field to override it, so the two identifiers cannot be unified here. + /// + /// # Errors + /// + /// - Returns the inner agent's [`serdes_ai::agent::AgentRunError`] + /// unchanged when the inner agent fails (direct run or hooked run) + /// and the failure reaches the caller untouched. + /// - Returns [`serdes_ai::agent::AgentRunError::Other`] when a run hook + /// returns or substitutes its own error during dispatch. + pub async fn run( + &self, + prompt: impl Into, + deps: (), + ) -> Result { + let (result, _extras) = self.run_with_extras(prompt.into(), deps).await?; + Ok(result) + } + + /// Shared implementation behind `run` and `run_stream`. + /// + /// Returns the run result plus the inner agent's run id and message + /// history, which `run_stream` needs for its synthetic `RunComplete` + /// event. + /// + /// # Errors + /// + /// - Returns the inner agent's [`serdes_ai::agent::AgentRunError`] + /// unchanged when the inner agent fails (direct run or hooked run) + /// and the failure reaches the caller untouched. + /// - Returns [`serdes_ai::agent::AgentRunError::Other`] when a run hook + /// returns or substitutes its own error during dispatch. + async fn run_with_extras( + &self, + prompt: String, + deps: (), + ) -> Result<(HookedAgentRunResult, AgentRunExtras), serdes_ai::agent::AgentRunError> { + if self.hooks.run_hooks_is_empty() { + let response = self.inner.run(prompt, deps).await?; + let serdes_ai::agent::AgentRunResult { + output, + run_id, + messages, + .. + } = response; + let extras = AgentRunExtras { run_id, messages }; + return Ok((HookedAgentRunResult { content: output }, extras)); + } + + // Wrapper-assigned run id for the hook context. The inner agent + // generates its own id for the real run and tool hooks; see the + // `run` doc comment. + let run_id = serdes_ai::agent::generate_run_id(); + let ctx = HookRunContext { + agent_name: &self.agent_name, + run_id: &run_id, + model_name: &self.model_name, + }; + let config = RunConfig::default(); + + let extras_slot = Arc::new(Mutex::new(None)); + let error_slot = Arc::new(Mutex::new(None)); + let executor = SerdesRunExecutor { + agent: &self.inner, + prompt, + deps, + extras: Arc::clone(&extras_slot), + error: Arc::clone(&error_slot), + }; + + // A dispatch failure carries a `ToolError`; restore the inner + // agent's original `AgentRunError` when it propagated untouched, + // and only label the error hook-origin otherwise. + let output = match self.hooks.dispatch_run(&ctx, config, &executor).await { + Ok(output) => output, + Err(dispatched) => return Err(restore_run_error(dispatched, &error_slot)), + }; + + // A hook that skipped `original` leaves the slot empty; fall back to + // the wrapper id so downstream events still carry a stable identifier. + let extras = extras_slot + .lock() + .expect("extras slot should not be poisoned") + .take() + .unwrap_or(AgentRunExtras { + run_id, + messages: Vec::new(), + }); + + Ok((HookedAgentRunResult::from_run_output(output), extras)) + } + + /// Runs the agent in streaming mode. + /// + /// When no run hooks are registered this delegates directly to the inner + /// agent's `run_stream`. When hooks are present it reuses the hooked + /// non-stream path and emits a synthetic stream containing the final + /// text output plus a `RunComplete` event carrying the real run id and + /// message history. + /// + /// # Errors + /// + /// - Returns the inner agent's [`serdes_ai::agent::AgentRunError`] + /// unchanged when the inner agent fails (direct stream or hooked run) + /// and the failure reaches the caller untouched. + /// - Returns [`serdes_ai::agent::AgentRunError::Other`] when the prompt is not + /// representable as text, or when a run hook returns or substitutes its + /// own error during dispatch. + pub async fn run_stream( + &self, + prompt: impl Into, + deps: (), + ) -> Result< + Pin< + Box< + dyn Stream< + Item = Result, + > + Send, + >, + >, + serdes_ai::agent::AgentRunError, + > { + let prompt = prompt.into(); + if self.hooks.run_hooks_is_empty() { + let stream = self.inner.run_stream(prompt, deps).await?; + return Ok(Box::pin(stream)); + } + let text = prompt.as_text().ok_or_else(|| { + serdes_ai::agent::AgentRunError::Other(anyhow::anyhow!( + "run hooks require a text prompt; image or multi-part prompts are unsupported" + )) + })?; + let (result, extras) = self.run_with_extras(text.to_string(), deps).await?; + let text = result.output().to_string(); + let events = vec![ + Ok(serdes_ai::AgentStreamEvent::TextDelta { text }), + Ok(serdes_ai::AgentStreamEvent::OutputReady), + Ok(serdes_ai::AgentStreamEvent::RunComplete { + run_id: extras.run_id, + messages: extras.messages, + }), + ]; + Ok(Box::pin(futures::stream::iter(events))) + } +} + +impl HookedAgentRunResult { + /// Returns the text output. + pub fn output(&self) -> &str { + &self.content + } + /// Consumes self and returns the owned text output. + pub fn into_output(self) -> String { + self.content + } + + fn from_run_output(output: RunOutput) -> Self { + Self { + content: output.content, + } + } +} + impl TaskBuildContext where C: CredentialLookup + Send + Sync + 'static, @@ -260,9 +519,11 @@ where /// - `model_catalog`: Available models for agent resolution. /// - `credentials`: Credential lookup used to authenticate model requests. /// - `workspace_root`: Project directory exposed to tools. - /// - `bash_sandbox`: Pre-built sandbox profile for [`BashTool`](crate::BashTool). + /// - `bash_sandbox`: Pre-built sandbox profile for [`BashTool`]. /// - `_sandbox_tmpdir`: Optional owning temp directories that keep the /// profile's backing storage alive. + /// + /// [`BashTool`]: crate::BashTool #[cfg(all(feature = "linux-bubblewrap", target_os = "linux"))] pub(crate) fn new_with_sandbox( runtime: Arc, @@ -312,6 +573,70 @@ where } } +impl<'a> RunExecutor for SerdesRunExecutor<'a> { + fn execute<'b>(&'b self, _ctx: &'b HookRunContext<'b>, config: RunConfig) -> RunHookFuture<'b> { + let agent = self.agent; + let mut prompt = self.prompt.clone(); + + // Apply RunConfig modifications that can be expressed by prepending + // to the prompt text. Order: system prompt, preamble messages in + // configured order, then the original prompt. + let mut sections: Vec = Vec::new(); + if let Some(sys) = &config.system_prompt { + sections.push(sys.clone()); + } + for msg in &config.preamble_messages { + match msg.role { + PreambleRole::System => sections.push(format!("[System] {}", msg.content)), + PreambleRole::User => sections.push(format!("[User] {}", msg.content)), + } + } + if !sections.is_empty() { + prompt = format!("{}\n\n{prompt}", sections.join("\n\n")); + } + + let extras = Arc::clone(&self.extras); + let error = Arc::clone(&self.error); + let run_options = run_options_with_overrides(agent, config.model_settings_overrides); + #[allow(clippy::let_unit_value)] + let deps = self.deps; + #[allow(clippy::unit_arg)] + Box::pin(async move { + let result = if let Some(options) = run_options { + agent.run_with_options(prompt, deps, options).await + } else { + agent.run(prompt, deps).await + }; + let response = match result { + Ok(response) => response, + Err(err) => { + // Hooks see the `ToolError` projection; the original is + // restored after dispatch when it propagates untouched. + let projection = run_error_projection(&err); + *error.lock().expect("run error slot should not be poisoned") = Some(err); + return Err(projection); + } + }; + // Read borrowed fields before moving run_id and messages out. + let content = response.output().to_string(); + let usage = RunUsage { + prompt_tokens: response.usage.request_tokens, + completion_tokens: response.usage.response_tokens, + }; + let inner_extras = AgentRunExtras { + run_id: response.run_id, + messages: response.messages, + }; + *extras.lock().expect("extras slot should not be poisoned") = Some(inner_extras); + Ok(RunOutput { + content, + reason: EndReason::Completed, + usage, + }) + }) + } +} + /// Builds one runnable agent using the shared build context. /// /// # Arguments @@ -321,7 +646,7 @@ where /// - `current_depth`: Current Task delegation depth (0 for top-level calls). /// /// # Returns -/// - `Ok(`[`Agent`]`)`: A fully constructed agent ready to run. +/// - `Ok(`[`HookedAgent`]`)`: A fully constructed agent ready to run. /// /// # Errors /// - Returns [`AgentBuildError::UnknownAgent`] when `name` is not in the @@ -336,13 +661,15 @@ where /// contains a tool kind this adapter cannot materialise. /// - Returns [`AgentBuildError::UnknownCustomTool`] when a custom tool entry /// names a tool absent from the custom-tool registry. +/// - Returns [`AgentBuildError::CustomToolNameMismatch`] when a custom +/// tool's name does not match its catalog entry name. /// - Returns [`AgentBuildError::CustomToolCreateFailed`] when a custom-tool /// factory cannot create its portable tool object. pub(crate) fn build_agent( context: Arc>, name: &str, current_depth: u8, -) -> Result, AgentBuildError> +) -> Result where C: CredentialLookup + Send + Sync + 'static, { @@ -383,104 +710,108 @@ where &context.workspace_root, sandbox_ref, context.runtime.custom_tool_registry(), + context.runtime().hooks(), )?; - Ok(builder.system_prompt(prompt_builder.build()).build()) + let agent = builder.system_prompt(prompt_builder.build()).build(); + let hooks = context.runtime().hooks().clone(); + let model_name = prepared.model().name().to_string(); + Ok(HookedAgent::new(agent, hooks, name.to_string(), model_name)) +} + +/// Recovers the failure from a failed run dispatch. +/// +/// Restores the captured inner-agent [`AgentRunError`] when the hook chain +/// propagated its projection untouched. Any other dispatched error reached +/// the caller through a hook returning or substituting its own error, so it +/// is labeled as hook-origin. +/// +/// [`AgentRunError`]: serdes_ai::agent::AgentRunError +fn restore_run_error( + dispatched: reloaded_code_core::ToolError, + captured: &Mutex>, +) -> serdes_ai::agent::AgentRunError { + let captured = captured + .lock() + .expect("run error slot should not be poisoned") + .take(); + match captured { + Some(inner) if run_error_projection(&inner).to_string() == dispatched.to_string() => inner, + _ => { + serdes_ai::agent::AgentRunError::Other(anyhow::anyhow!("run hook error: {dispatched}")) + } + } +} + +/// Builds per-run [`RunOptions`] that merge [`ModelSettingsOverrides`] over +/// the agent's configured settings. +/// +/// Returns `None` when no override field is set, so those runs keep the plain +/// [`Agent::run`] behavior. An overridden field replaces only that field; +/// all others keep the agent's values, because a provided +/// `RunOptions::model_settings` replaces the agent's settings wholesale. +/// +/// Every [`ModelSettingsOverrides`] field is bound explicitly (no rest +/// pattern), so adding a field fails compilation here; extend this function +/// to apply the new field or reject it with +/// [`ToolError::validation_for`][reloaded_code_core::ToolError::validation_for] +/// naming `model_settings_overrides`. +fn run_options_with_overrides( + agent: &Agent<(), String>, + overrides: Option, +) -> Option { + let ModelSettingsOverrides { temperature, top_p } = overrides?; + if temperature.is_none() && top_p.is_none() { + return None; + } + let mut settings = agent.model_settings().clone(); + if let Some(temperature) = temperature { + settings.temperature = Some(f64::from(temperature)); + } + if let Some(top_p) = top_p { + settings.top_p = Some(f64::from(top_p)); + } + Some(RunOptions::default().model_settings(settings)) +} + +/// Deterministic [`ToolError`] projection of an inner-agent run failure that +/// the run hook chain carries. The dispatch site recognizes an untouched +/// projection and restores the original [`AgentRunError`] afterward. +/// +/// [`ToolError`]: reloaded_code_core::ToolError +/// [`AgentRunError`]: serdes_ai::agent::AgentRunError +fn run_error_projection(err: &serdes_ai::agent::AgentRunError) -> reloaded_code_core::ToolError { + reloaded_code_core::ToolError::Execution(err.to_string()) } #[cfg(test)] mod tests { use super::*; - use ahash::AHashMap; - use indexmap::IndexMap; - use reloaded_code_agents::{ - AgentCatalog, AgentConfig, AgentDefaults, AgentMode, AgentRuntimeBuilder, - AgentToolSettings, PermissionRule, + use crate::agent_runtime::test_stubs::{ + agent, allow_tools, catalog, credentials, pattern_task, workspace_root, }; - use reloaded_code_core::CredentialResolver; - use reloaded_code_core::models::{ - Modality, ModelCatalog, ModelInfo, ProviderIdx, ProviderInfo, ProviderModelSource, - ProviderSource, ProviderType, + use crate::mock::{FunctionModel, two_tools_then_text}; + use reloaded_code_agents::{AgentCatalog, AgentDefaults, AgentMode, AgentRuntimeBuilder}; + use reloaded_code_core::ToolOutput; + use reloaded_code_core::hooks::{ + ModelSettingsOverrides, PreambleMessage, PreambleRole, RunHook, RunOriginal, + ToolCallContext, ToolHook, ToolHookFuture, ToolOriginal, ToolRequest, }; use reloaded_code_core::permissions::{ExpandError, PermissionAction}; use reloaded_code_core::tool_metadata::{ read as read_meta, task as task_meta, write as write_meta, }; + use serde_json::json; + use serdes_ai::core::{ModelResponse, ModelSettings}; + use std::collections::HashSet; + use std::path::PathBuf; + use std::sync::Mutex; + use tempfile::TempDir; type TestResult = Result<(), ExpandError>; - fn agent( - name: &str, - mode: AgentMode, - permission: IndexMap, - prompt: &str, - ) -> AgentConfig { - AgentConfig { - name: name.into(), - mode, - description: format!("{name} description").into(), - model: None, - hidden: false, - temperature: None, - top_p: None, - permission, - options: AHashMap::new(), - tool_settings: AgentToolSettings::default(), - prompt: prompt.into(), - } - } - - fn allow_tools(names: &[&str]) -> IndexMap { - names - .iter() - .map(|n| ((*n).into(), PermissionRule::Action(PermissionAction::Allow))) - .collect() - } - - fn pattern_task(patterns: &[(&str, PermissionAction)]) -> IndexMap { - let mut map = IndexMap::new(); - for (pattern, action) in patterns { - map.insert(pattern.to_string(), *action); - } - IndexMap::from([(task_meta::NAME.into(), PermissionRule::Pattern(map))]) - } - - fn catalog() -> ModelCatalog { - let providers = vec![ProviderSource::new( - "openrouter", - ProviderInfo { - api_url: "https://openrouter.ai/api/v1".into(), - env_vars: vec!["OPENROUTER_API_KEY".into()], - api_type: ProviderType::OpenRouter, - }, - )]; - let info = ModelInfo { - modalities: Modality::TEXT, - max_input: 128_000, - max_output: 16_384, - temperature: Some(1.0), - top_p: Some(0.95), - }; - let models: Vec> = - [("openai/gpt-4.1-mini", info), ("openai/gpt-4o", info)] - .into_iter() - .map(|(key, i)| ProviderModelSource::new(ProviderIdx::new(0), key, i)) - .collect(); - ModelCatalog::build(&providers, &models).expect("catalog fixture should build") - } - - fn credentials() -> Arc> { - let mut resolver = CredentialResolver::without_env(); - resolver.set_override("OPENROUTER_API_KEY", "test-key"); - Arc::new(resolver) - } - - fn workspace_root() -> Arc { - Arc::from(reloaded_code_core::resolve_workspace_root().expect("workspace root")) - } - #[test] fn build_agent_skips_task_tool_when_no_targets_are_callable() -> TestResult { - let credentials = credentials(); + let credentials = Arc::new(credentials()); let model_catalog = Arc::new(catalog()); let runtime = AgentRuntimeBuilder::new() @@ -496,18 +827,12 @@ mod tests { .defaults(AgentDefaults::with_model("openrouter/openai/gpt-4.1-mini")) .build()?; - let context = Arc::new(TaskBuildContext { - runtime: Arc::new(runtime), + let context = Arc::new(TaskBuildContext::new_for_test( + Arc::new(runtime), model_catalog, credentials, - workspace_root: workspace_root(), - #[cfg(any(test, feature = "mock"))] - model_override: None, - #[cfg(all(feature = "linux-bubblewrap", target_os = "linux"))] - bash_sandbox: None, - #[cfg(all(feature = "linux-bubblewrap", target_os = "linux"))] - _sandbox_tmpdir: None, - }); + workspace_root(), + )); let agent = build_agent(context, "caller", 0).expect("build should succeed"); let names: Vec<_> = agent.tools().iter().map(|t| t.name()).collect(); @@ -517,7 +842,7 @@ mod tests { #[test] fn build_agent_attaches_task_when_callable_targets_exist() -> TestResult { - let credentials = credentials(); + let credentials = Arc::new(credentials()); let model_catalog = Arc::new(catalog()); let runtime = AgentRuntimeBuilder::new() @@ -538,18 +863,12 @@ mod tests { .defaults(AgentDefaults::with_model("openrouter/openai/gpt-4.1-mini")) .build()?; - let context = Arc::new(TaskBuildContext { - runtime: Arc::new(runtime), + let context = Arc::new(TaskBuildContext::new_for_test( + Arc::new(runtime), model_catalog, credentials, - workspace_root: workspace_root(), - #[cfg(any(test, feature = "mock"))] - model_override: None, - #[cfg(all(feature = "linux-bubblewrap", target_os = "linux"))] - bash_sandbox: None, - #[cfg(all(feature = "linux-bubblewrap", target_os = "linux"))] - _sandbox_tmpdir: None, - }); + workspace_root(), + )); let agent = build_agent(context, "caller", 0).expect("build should succeed"); let names: Vec<_> = agent.tools().iter().map(|t| t.name()).collect(); @@ -559,48 +878,10 @@ mod tests { } #[test] - fn build_agent_attaches_task_when_task_permission_is_target_scoped() -> TestResult { - let credentials = credentials(); - let model_catalog = Arc::new(catalog()); - - let runtime = AgentRuntimeBuilder::new() - .catalog(AgentCatalog::from_entries([ - agent( - "caller", - AgentMode::Primary, - pattern_task(&[ - ("*", PermissionAction::Deny), - ("reader", PermissionAction::Allow), - ]), - "prompt", - ), - agent("reader", AgentMode::Subagent, allow_tools(&[]), "prompt"), - ])) - .defaults(AgentDefaults::with_model("openrouter/openai/gpt-4.1-mini")) - .build()?; - - let context = Arc::new(TaskBuildContext { - runtime: Arc::new(runtime), - model_catalog, - credentials, - workspace_root: workspace_root(), - #[cfg(any(test, feature = "mock"))] - model_override: None, - #[cfg(all(feature = "linux-bubblewrap", target_os = "linux"))] - bash_sandbox: None, - #[cfg(all(feature = "linux-bubblewrap", target_os = "linux"))] - _sandbox_tmpdir: None, - }); - - let agent = build_agent(context, "caller", 0).expect("build should succeed"); - let names: Vec<_> = agent.tools().iter().map(|t| t.name()).collect(); - assert_eq!(names, vec![task_meta::NAME]); - Ok(()) - } - - #[test] - fn build_agent_attaches_task_when_permission_task_is_absent() -> TestResult { - let credentials = credentials(); + fn build_agent_attaches_task_according_to_task_permission() -> TestResult { + // Task permission absent: delegation defaults to every non-Primary + // target, so the Task tool attaches alongside the allowed `read`. + let credentials = Arc::new(credentials()); let model_catalog = Arc::new(catalog()); let runtime = AgentRuntimeBuilder::new() @@ -616,59 +897,52 @@ mod tests { .defaults(AgentDefaults::with_model("openrouter/openai/gpt-4.1-mini")) .build()?; - let context = Arc::new(TaskBuildContext { - runtime: Arc::new(runtime), - model_catalog, - credentials, - workspace_root: workspace_root(), - #[cfg(any(test, feature = "mock"))] - model_override: None, - #[cfg(all(feature = "linux-bubblewrap", target_os = "linux"))] - bash_sandbox: None, - #[cfg(all(feature = "linux-bubblewrap", target_os = "linux"))] - _sandbox_tmpdir: None, - }); + let context = Arc::new(TaskBuildContext::new_for_test( + Arc::new(runtime), + model_catalog.clone(), + credentials.clone(), + workspace_root(), + )); - let agent = build_agent(context, "caller", 0).expect("build should succeed"); - let names: Vec<_> = agent.tools().iter().map(|t| t.name()).collect(); + let built = build_agent(context, "caller", 0).expect("build should succeed"); + let names: Vec<_> = built.tools().iter().map(|t| t.name()).collect(); assert!(names.contains(&read_meta::NAME)); assert!(names.contains(&task_meta::NAME)); - Ok(()) - } - - #[test] - fn agent_build_context_omits_task_tool_when_no_targets_are_callable() -> TestResult { - let model_catalog = Arc::new(catalog()); - let credentials = credentials(); + // Pattern-scoped Task permission: only the `reader` target is + // callable and no other tool is allowed, so Task attaches alone. let runtime = AgentRuntimeBuilder::new() .catalog(AgentCatalog::from_entries([ agent( "caller", AgentMode::Primary, - allow_tools(&[read_meta::NAME]), + pattern_task(&[ + ("*", PermissionAction::Deny), + ("reader", PermissionAction::Allow), + ]), "prompt", ), - agent("other", AgentMode::Primary, allow_tools(&[]), "prompt"), + agent("reader", AgentMode::Subagent, allow_tools(&[]), "prompt"), ])) .defaults(AgentDefaults::with_model("openrouter/openai/gpt-4.1-mini")) .build()?; - let context = AgentBuildContext::new( + let context = Arc::new(TaskBuildContext::new_for_test( Arc::new(runtime), - model_catalog, - credentials, + model_catalog.clone(), + credentials.clone(), workspace_root(), - ); - let agent = context.build("caller").expect("build should succeed"); - let names: Vec<_> = agent.tools().iter().map(|t| t.name()).collect(); - assert!(!names.contains(&task_meta::NAME)); + )); + + let built = build_agent(context, "caller", 0).expect("build should succeed"); + let names: Vec<_> = built.tools().iter().map(|t| t.name()).collect(); + assert_eq!(names, vec![task_meta::NAME]); Ok(()) } #[test] fn build_agent_omits_task_tool_at_max_depth() -> TestResult { - let credentials = credentials(); + let credentials = Arc::new(credentials()); let model_catalog = Arc::new(catalog()); let runtime = AgentRuntimeBuilder::new() @@ -690,18 +964,12 @@ mod tests { .max_task_depth(1) .build()?; - let context = Arc::new(TaskBuildContext { - runtime: Arc::new(runtime), + let context = Arc::new(TaskBuildContext::new_for_test( + Arc::new(runtime), model_catalog, credentials, - workspace_root: workspace_root(), - #[cfg(any(test, feature = "mock"))] - model_override: None, - #[cfg(all(feature = "linux-bubblewrap", target_os = "linux"))] - bash_sandbox: None, - #[cfg(all(feature = "linux-bubblewrap", target_os = "linux"))] - _sandbox_tmpdir: None, - }); + workspace_root(), + )); let agent = build_agent(context, "caller", 1).expect("build should succeed"); let names: Vec<_> = agent.tools().iter().map(|t| t.name()).collect(); @@ -710,40 +978,642 @@ mod tests { Ok(()) } - #[test] - fn agent_build_context_omits_task_tool_when_max_depth_is_zero() -> TestResult { - let model_catalog = Arc::new(catalog()); - let credentials = credentials(); + /// Denies `write` calls to files the run has not `read`. + /// + /// The shared hook instance fires for every tool call of the run, so the + /// set of read files lives behind interior mutability. Reads record their + /// target keyed by `(run_id, path)` so authorization cannot leak across + /// runs; writes to never-read targets get an explanatory result without + /// calling `original`, which is what short-circuits the real tool. + struct ReadBeforeWriteHook { + read_files: Mutex>, + } + + impl ToolHook for ReadBeforeWriteHook { + fn hook<'a>( + &'a self, + ctx: &'a ToolCallContext<'a>, + req: ToolRequest, + original: ToolOriginal<'a>, + ) -> ToolHookFuture<'a> { + Box::pin(async move { + let target = req + .args + .get("file_path") + .and_then(|value| value.as_str()) + .map(PathBuf::from); + + match (ctx.tool_name, target) { + (read_meta::NAME, Some(path)) => { + // Record the read before continuing so the lock is + // never held across the await. + self.read_files + .lock() + .expect("read_files should not be poisoned") + .insert((ctx.run_id.to_string(), path)); + original.call(ctx, req).await + } + (write_meta::NAME, Some(path)) => { + let was_read = self + .read_files + .lock() + .expect("read_files should not be poisoned") + .contains(&(ctx.run_id.to_string(), path.clone())); + if was_read { + return original.call(ctx, req).await; + } + // Skipping `original` blocks the call: the real + // `write` sits behind it and is never reached. + Ok(ToolOutput::new(format!( + "[blocked by hook] write to {} denied: no read of that file \ + happened this run", + path.display() + ))) + } + _ => original.call(ctx, req).await, + } + }) + } + } + + #[tokio::test] + async fn tool_hook_denies_write_to_never_read_file_during_agent_run() { + // Workspace fixture: the model reads `service.env`, then tries to + // write `draft.md`, a file the run never reads. + let workspace = TempDir::new().expect("create temp workspace"); + let read_file = workspace.path().join("service.env"); + std::fs::write(&read_file, "LOG_LEVEL=debug\n").expect("write read fixture"); + let unread_target = workspace.path().join("draft.md"); + + let hooks = HookSet::builder() + .tool_hook(ReadBeforeWriteHook { + read_files: Mutex::new(HashSet::new()), + }) + .build(); let runtime = AgentRuntimeBuilder::new() - .catalog(AgentCatalog::from_entries([ - agent( - "caller", - AgentMode::All, - allow_tools(&[task_meta::NAME, read_meta::NAME]), - "prompt", - ), - agent( - "target", - AgentMode::All, - allow_tools(&[write_meta::NAME]), - "prompt", - ), - ])) + .catalog(AgentCatalog::from_entries([agent( + "caller", + AgentMode::Primary, + allow_tools(&[read_meta::NAME, write_meta::NAME]), + "prompt", + )])) .defaults(AgentDefaults::with_model("openrouter/openai/gpt-4.1-mini")) - .max_task_depth(0) - .build()?; + .hooks(hooks) + .build() + .expect("runtime should build"); + // Script the model: first turn reads the fixture, second turn writes + // the never-read target, final turn echoes the collected tool returns. let context = AgentBuildContext::new( Arc::new(runtime), - model_catalog, - credentials, + Arc::new(catalog()), + Arc::new(credentials()), + Arc::from(workspace.path()), + ) + .with_model_override(two_tools_then_text( + (read_meta::NAME, json!({"file_path": "service.env"})), + ( + write_meta::NAME, + json!({"file_path": "draft.md", "content": "Draft notes."}), + ), + "Run finished.", + )); + let hooked = context.build("caller").expect("build should succeed"); + + let result = hooked + .run("Read the config, then write the draft.", ()) + .await + .expect("run should complete"); + + // The real `read` executed: its result is the only source of the + // fixture content in the final answer. + assert!( + result.output().contains("LOG_LEVEL=debug"), + "real read result should reach the model: {}", + result.output() + ); + + // The denied `write` never executed; the hook's response reached the + // model in its place and the target file was never created. + assert!( + result.output().contains("[blocked by hook]"), + "hook denial should reach the model: {}", + result.output() + ); + assert!( + !unread_target.exists(), + "the denied write must not create {}", + unread_target.display() + ); + } + + /// Model settings overrides: applied per run, merged over the agent's + /// configured settings, with prompt-prepend behavior untouched. + + /// Run hook that installs fixed model settings overrides before + /// delegating to `original`. + struct OverridingRunHook { + temperature: Option, + top_p: Option, + } + + impl RunHook for OverridingRunHook { + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + mut config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + config.model_settings_overrides = Some(ModelSettingsOverrides { + temperature: self.temperature, + top_p: self.top_p, + }); + original.call(ctx, config) + } + } + + /// Run hook that injects prompt sections plus a temperature override + /// before delegating to `original`. + struct PromptAndSettingsOverrideRunHook; + + impl RunHook for PromptAndSettingsOverrideRunHook { + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + mut config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + config.system_prompt = Some("agent system override".into()); + config.preamble_messages = vec![ + PreambleMessage { + role: PreambleRole::System, + content: "sys note".into(), + }, + PreambleMessage { + role: PreambleRole::User, + content: "user note".into(), + }, + ]; + config.model_settings_overrides = Some(ModelSettingsOverrides { + temperature: Some(0.9), + top_p: None, + }); + original.call(ctx, config) + } + } + + /// Builds a hooked agent with agent-level settings temperature 0.3 and + /// top_p 0.8, running a model that records the [`ModelSettings`] of every + /// request and echoes the last user prompt. + fn hooked_agent_with_settings_capture( + hook: impl RunHook + 'static, + ) -> (HookedAgent, Arc>>) { + let captured: Arc>> = Arc::new(Mutex::new(Vec::new())); + let seen = Arc::clone(&captured); + let model = FunctionModel::new(move |messages, settings| { + seen.lock() + .expect("captured settings should not be poisoned") + .push(settings.clone()); + let last_user = messages + .iter() + .rev() + .flat_map(|m| m.user_prompts()) + .next() + .and_then(|prompt| prompt.as_text()) + .unwrap_or_default() + .to_string(); + ModelResponse::text(last_user) + }); + + let hooks = HookSet::builder().run_hook(hook).build(); + let mut defaults = AgentDefaults::with_model("openrouter/openai/gpt-4.1-mini"); + defaults.temperature = Some(0.3); + defaults.top_p = Some(0.8); + let runtime = AgentRuntimeBuilder::new() + .catalog(AgentCatalog::from_entries([agent( + "caller", + AgentMode::Primary, + allow_tools(&[]), + "prompt", + )])) + .defaults(defaults) + .hooks(hooks) + .build() + .expect("runtime should build"); + + let context = AgentBuildContext::new( + Arc::new(runtime), + Arc::new(catalog()), + Arc::new(credentials()), workspace_root(), + ) + .with_model_override(model); + let hooked = context.build("caller").expect("build should succeed"); + (hooked, captured) + } + + #[tokio::test] + async fn model_settings_override_replaces_only_the_overridden_setting_in_request() { + let (hooked, captured) = hooked_agent_with_settings_capture(OverridingRunHook { + temperature: Some(0.9), + top_p: None, + }); + + hooked.run("hello", ()).await.expect("run should complete"); + + let seen = captured + .lock() + .expect("captured settings should not be poisoned"); + assert_eq!(seen.len(), 1, "one model request should have been made"); + assert_eq!(seen[0].temperature, Some(f64::from(0.9_f32))); + assert_eq!( + seen[0].top_p, + Some(f64::from(0.8_f32)), + "agent-configured top_p should be retained" ); - let agent = context.build("caller").expect("build should succeed"); - let names: Vec<_> = agent.tools().iter().map(|t| t.name()).collect(); - assert!(!names.contains(&task_meta::NAME)); - assert!(names.contains(&read_meta::NAME)); - Ok(()) + drop(seen); + + // Mirror direction: a top_p-only override replaces top_p and keeps + // the agent-configured temperature. + let (hooked, captured) = hooked_agent_with_settings_capture(OverridingRunHook { + temperature: None, + top_p: Some(0.6), + }); + + hooked.run("hello", ()).await.expect("run should complete"); + + let seen = captured + .lock() + .expect("captured settings should not be poisoned"); + assert_eq!(seen.len(), 1, "one model request should have been made"); + assert_eq!(seen[0].top_p, Some(f64::from(0.6_f32))); + assert_eq!( + seen[0].temperature, + Some(f64::from(0.3_f32)), + "agent-configured temperature should be retained" + ); + } + + #[tokio::test] + async fn run_without_model_settings_overrides_uses_agent_configured_settings() { + // Absent overrides: `RunConfig::default()` flows through untouched. + let (hooked, captured) = hooked_agent_with_settings_capture(PassthroughRunHook); + hooked.run("hello", ()).await.expect("run should complete"); + + // All-None overrides: no field is set, so agent settings apply as-is. + let (hooked, captured_empty) = hooked_agent_with_settings_capture(OverridingRunHook { + temperature: None, + top_p: None, + }); + hooked.run("hello", ()).await.expect("run should complete"); + + let expected = ModelSettings { + temperature: Some(f64::from(0.3_f32)), + top_p: Some(f64::from(0.8_f32)), + ..ModelSettings::default() + }; + for run in [captured, captured_empty] { + let seen = run + .lock() + .expect("captured settings should not be poisoned"); + assert_eq!(seen.len(), 1, "one model request should have been made"); + assert_eq!( + seen[0], expected, + "no-override runs must use the agent's configured settings" + ); + } + } + + #[tokio::test] + async fn prompt_sections_are_unchanged_when_model_settings_overrides_are_present() { + let (hooked, captured) = + hooked_agent_with_settings_capture(PromptAndSettingsOverrideRunHook); + + let output = hooked + .run("base prompt", ()) + .await + .expect("run should complete") + .into_output(); + + // The echoed prompt still leads with system prompt, preamble + // messages in configured order, then the original prompt. + assert_eq!( + output, + "agent system override\n\n[System] sys note\n\n[User] user note\n\nbase prompt" + ); + + // The same run carried the override, proving prompt handling is + // untouched while model settings change. + let seen = captured + .lock() + .expect("captured settings should not be poisoned"); + assert_eq!(seen[0].temperature, Some(f64::from(0.9_f32))); + assert_eq!( + seen[0].top_p, + Some(f64::from(0.8_f32)), + "agent-configured top_p should be retained" + ); + } + + /// Model whose every request fails, so the inner agent run surfaces a + /// real `AgentRunError::Model` failure. + struct FailingModel { + profile: serdes_ai_models::ModelProfile, + } + + impl FailingModel { + fn new() -> Self { + Self { + profile: serdes_ai_models::ModelProfile::default(), + } + } + } + + #[async_trait::async_trait] + impl serdes_ai_models::Model for FailingModel { + fn name(&self) -> &str { + "failing-model" + } + + fn system(&self) -> &str { + "test" + } + + fn profile(&self) -> &serdes_ai_models::ModelProfile { + &self.profile + } + + async fn request( + &self, + _messages: &[ModelRequest], + _settings: &serdes_ai::core::ModelSettings, + _params: &serdes_ai_models::ModelRequestParameters, + ) -> Result { + Err(serdes_ai_models::ModelError::api("upstream exploded")) + } + + async fn request_stream( + &self, + _messages: &[ModelRequest], + _settings: &serdes_ai::core::ModelSettings, + _params: &serdes_ai_models::ModelRequestParameters, + ) -> Result { + Err(serdes_ai_models::ModelError::api("upstream exploded")) + } + } + + /// Run hook that delegates straight to `original`, standing in for any + /// observer hook that never interferes with the run. + struct PassthroughRunHook; + + impl RunHook for PassthroughRunHook { + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + original.call(ctx, config) + } + } + + /// Run hook that skips `original` and fails on its own. + struct FailingRunHook; + + impl RunHook for FailingRunHook { + fn hook<'a>( + &'a self, + _ctx: &'a HookRunContext<'a>, + _config: RunConfig, + _original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + Box::pin(async { + Err(reloaded_code_core::ToolError::Execution( + "hook rejected the run".into(), + )) + }) + } + } + + /// Run hook that observes the original run's failure and substitutes its + /// own error instead of propagating it. + struct SubstitutingRunHook; + + impl RunHook for SubstitutingRunHook { + fn hook<'a>( + &'a self, + ctx: &'a HookRunContext<'a>, + config: RunConfig, + original: RunOriginal<'a>, + ) -> RunHookFuture<'a> { + Box::pin(async move { + match original.call(ctx, config).await { + Err(_) => Err(reloaded_code_core::ToolError::Execution( + "policy veto".into(), + )), + ok => ok, + } + }) + } + } + + #[tokio::test] + async fn run_failure_keeps_original_error_variant_when_hook_propagates_it_untouched() { + // The hook calls `original`, so the inner model failure flows + // through the whole chain before reaching the caller. + let hooks = HookSet::builder().run_hook(PassthroughRunHook).build(); + + let runtime = AgentRuntimeBuilder::new() + .catalog(AgentCatalog::from_entries([agent( + "caller", + AgentMode::Primary, + allow_tools(&[]), + "prompt", + )])) + .defaults(AgentDefaults::with_model("openrouter/openai/gpt-4.1-mini")) + .hooks(hooks) + .build() + .expect("runtime should build"); + + let context = AgentBuildContext::new( + Arc::new(runtime), + Arc::new(catalog()), + Arc::new(credentials()), + workspace_root(), + ) + .with_model_override(FailingModel::new()); + let hooked = context.build("caller").expect("build should succeed"); + + let err = hooked + .run("trigger the failure", ()) + .await + .err() + .expect("run should fail"); + + assert!( + matches!( + err, + serdes_ai::agent::AgentRunError::Model(serdes_ai_models::ModelError::Api { .. }) + ), + "inner model failure should keep its variant, got: {err:?}" + ); + assert!( + !err.to_string().contains("run hook error"), + "untouched inner failure must not be labeled as a hook error: {err}" + ); + } + + #[tokio::test] + async fn run_failure_keeps_original_error_variant_when_model_settings_overrides_are_present() { + // The override routes the run through `run_with_options`; its + // failures must keep the same untouched-projection handling as + // plain runs. + let hooks = HookSet::builder() + .run_hook(OverridingRunHook { + temperature: Some(0.9), + top_p: None, + }) + .build(); + + let runtime = AgentRuntimeBuilder::new() + .catalog(AgentCatalog::from_entries([agent( + "caller", + AgentMode::Primary, + allow_tools(&[]), + "prompt", + )])) + .defaults(AgentDefaults::with_model("openrouter/openai/gpt-4.1-mini")) + .hooks(hooks) + .build() + .expect("runtime should build"); + + let context = AgentBuildContext::new( + Arc::new(runtime), + Arc::new(catalog()), + Arc::new(credentials()), + workspace_root(), + ) + .with_model_override(FailingModel::new()); + let hooked = context.build("caller").expect("build should succeed"); + + let err = hooked + .run("trigger the failure", ()) + .await + .err() + .expect("run should fail"); + + assert!( + matches!( + err, + serdes_ai::agent::AgentRunError::Model(serdes_ai_models::ModelError::Api { .. }) + ), + "inner model failure should keep its variant, got: {err:?}" + ); + assert!( + !err.to_string().contains("run hook error"), + "untouched inner failure must not be labeled as a hook error: {err}" + ); + } + + #[tokio::test] + async fn run_failure_is_labeled_hook_error_when_hook_returns_its_own_error() { + // The hook skips `original` and fails on its own, so the model is + // never invoked and the failure can only be hook-origin. + let hooks = HookSet::builder().run_hook(FailingRunHook).build(); + + let runtime = AgentRuntimeBuilder::new() + .catalog(AgentCatalog::from_entries([agent( + "caller", + AgentMode::Primary, + allow_tools(&[]), + "prompt", + )])) + .defaults(AgentDefaults::with_model("openrouter/openai/gpt-4.1-mini")) + .hooks(hooks) + .build() + .expect("runtime should build"); + + let context = AgentBuildContext::new( + Arc::new(runtime), + Arc::new(catalog()), + Arc::new(credentials()), + workspace_root(), + ) + .with_model_override(crate::mock::MockModel::new("unused").with_text_response("unused")); + let hooked = context.build("caller").expect("build should succeed"); + + let err = hooked + .run("trigger the hook failure", ()) + .await + .err() + .expect("run should fail"); + + match err { + serdes_ai::agent::AgentRunError::Other(source) => { + let message = source.to_string(); + assert!( + message.contains("run hook error"), + "hook-origin failure should be labeled as such: {message}" + ); + assert!( + message.contains("hook rejected the run"), + "dispatched hook error should be preserved: {message}" + ); + } + other => panic!("hook-substituted failure should surface as Other, got: {other:?}"), + } + } + + #[tokio::test] + async fn run_failure_is_labeled_hook_error_when_hook_substitutes_its_own_error() { + // The hook calls `original`, sees the model failure, then returns a + // different error: the inner failure was observed but replaced, so + // the surfaced failure is hook-origin, not the inner one. + let hooks = HookSet::builder().run_hook(SubstitutingRunHook).build(); + + let runtime = AgentRuntimeBuilder::new() + .catalog(AgentCatalog::from_entries([agent( + "caller", + AgentMode::Primary, + allow_tools(&[]), + "prompt", + )])) + .defaults(AgentDefaults::with_model("openrouter/openai/gpt-4.1-mini")) + .hooks(hooks) + .build() + .expect("runtime should build"); + + let context = AgentBuildContext::new( + Arc::new(runtime), + Arc::new(catalog()), + Arc::new(credentials()), + workspace_root(), + ) + .with_model_override(FailingModel::new()); + let hooked = context.build("caller").expect("build should succeed"); + + let err = hooked + .run("trigger the failure", ()) + .await + .err() + .expect("run should fail"); + + match err { + serdes_ai::agent::AgentRunError::Other(source) => { + let message = source.to_string(); + assert!( + message.contains("run hook error"), + "hook-substituted failure should be labeled hook-origin: {message}" + ); + assert!( + message.contains("policy veto"), + "hook's substituted error should be preserved: {message}" + ); + } + other => { + panic!("hook-substituted failure must not surface the inner error, got: {other:?}") + } + } } } diff --git a/src/reloaded-code-serdesai/src/agent_runtime/test_stubs.rs b/src/reloaded-code-serdesai/src/agent_runtime/test_stubs.rs index 99433f2..7f6eea1 100644 --- a/src/reloaded-code-serdesai/src/agent_runtime/test_stubs.rs +++ b/src/reloaded-code-serdesai/src/agent_runtime/test_stubs.rs @@ -1,10 +1,24 @@ -//! Shared test stubs for SerdesAI custom tool tests. +//! Shared test stubs and fixtures for agent-runtime tests. +//! +//! Fixtures here are shared by the test modules of [`crate::agent_runtime`], +//! [`crate::agent_runtime::build`], and [`crate::task`]; keep them inert (no +//! environment reads, no network) so any test module can reuse them. +use ahash::AHashMap; +use indexmap::IndexMap; +use reloaded_code_agents::{AgentConfig, AgentMode, AgentToolSettings, PermissionRule}; use reloaded_code_core::context::{ToolContext, ToolPrompt}; +use reloaded_code_core::models::{ + Modality, ModelCatalog, ModelInfo, ProviderIdx, ProviderInfo, ProviderModelSource, + ProviderSource, ProviderType, +}; +use reloaded_code_core::permissions::PermissionAction; +use reloaded_code_core::tool_metadata::task as task_meta; use reloaded_code_core::{ - CustomTool, CustomToolDefinition, CustomToolFuture, ToolBuildContext, ToolFactory, ToolOutput, - ToolResult, ToolRunContext, + CredentialResolver, CustomTool, CustomToolDefinition, CustomToolFuture, ToolBuildContext, + ToolFactory, ToolOutput, ToolResult, ToolRunContext, }; +use std::path::Path; use std::sync::Arc; /// A `ToolFactory` that creates a portable [`SerdesTestTool`]. @@ -91,3 +105,87 @@ impl CustomTool for SerdesTestTool { Box::pin(async move { Ok(ToolOutput::new(self.response)) }) } } + +// ============================================================================ +// Shared test fixtures +// ============================================================================ + +/// Builds an [`AgentConfig`] fixture with the given mode, permission rules, +/// and system prompt. +pub(crate) fn agent( + name: &str, + mode: AgentMode, + permission: IndexMap, + prompt: &str, +) -> AgentConfig { + AgentConfig { + name: name.into(), + mode, + description: format!("{name} description").into(), + model: None, + hidden: false, + temperature: None, + top_p: None, + permission, + options: AHashMap::new(), + tool_settings: AgentToolSettings::default(), + prompt: prompt.into(), + } +} + +/// Builds permission rules that allow exactly the named tools. +pub(crate) fn allow_tools(names: &[&str]) -> IndexMap { + names + .iter() + .map(|n| ((*n).into(), PermissionRule::Action(PermissionAction::Allow))) + .collect() +} + +/// Builds a two-model OpenRouter catalog fixture. +pub(crate) fn catalog() -> ModelCatalog { + let providers = vec![ProviderSource::new( + "openrouter", + ProviderInfo { + api_url: "https://openrouter.ai/api/v1".into(), + env_vars: vec!["OPENROUTER_API_KEY".into()], + api_type: ProviderType::OpenRouter, + }, + )]; + let info = ModelInfo { + modalities: Modality::TEXT, + max_input: 128_000, + max_output: 16_384, + temperature: Some(1.0), + top_p: Some(0.95), + }; + let models: Vec> = + [("openai/gpt-4.1-mini", info), ("openai/gpt-4o", info)] + .into_iter() + .map(|(key, i)| ProviderModelSource::new(ProviderIdx::new(0), key, i)) + .collect(); + ModelCatalog::build(&providers, &models).expect("catalog fixture should build") +} + +/// Builds a credential resolver with an inert OpenRouter override. +pub(crate) fn credentials() -> CredentialResolver { + let mut resolver = CredentialResolver::without_env(); + resolver.set_override("OPENROUTER_API_KEY", "test-key"); + resolver +} + +/// Builds permission rules where the `task` tool dispatches on target-name +/// patterns; later patterns win, mirroring rule precedence. +pub(crate) fn pattern_task( + patterns: &[(&str, PermissionAction)], +) -> IndexMap { + let mut map = IndexMap::new(); + for (pattern, action) in patterns { + map.insert(pattern.to_string(), *action); + } + IndexMap::from([(task_meta::NAME.into(), PermissionRule::Pattern(map))]) +} + +/// Resolves the repository workspace root, wrapped for context structs. +pub(crate) fn workspace_root() -> Arc { + Arc::from(reloaded_code_core::resolve_workspace_root().expect("workspace root")) +} diff --git a/src/reloaded-code-serdesai/src/convert.rs b/src/reloaded-code-serdesai/src/convert.rs index 80af3b8..903e873 100644 --- a/src/reloaded-code-serdesai/src/convert.rs +++ b/src/reloaded-code-serdesai/src/convert.rs @@ -8,9 +8,16 @@ use reloaded_code_core::{ CustomToolDefinition, ToolError as CoreError, ToolOutput, ToolResult as CoreResult, }; -use serde_json::json; +use serde_json::{Value as JsonValue, json}; use serdes_ai::tools::{ToolDefinition, ToolError as SerdesError, ToolReturn}; +/// Placeholder shown to tool hooks when the real tool returned image +/// content, which the core [`ToolOutput`] text model cannot represent. +pub(crate) const IMAGE_RETURN_PLACEHOLDER: &str = "[tool returned image content]"; +/// Placeholder shown to tool hooks when the real tool returned multiple +/// items, which the core [`ToolOutput`] text model cannot represent. +pub(crate) const MULTIPLE_RETURN_PLACEHOLDER: &str = "[tool returned multiple items]"; + /// Convert a portable [`CustomToolDefinition`] to a SerdesAI [`ToolDefinition`]. /// /// Fields map 1:1. The SerdesAI `outer_typed_dict_key` field is always `None` @@ -123,16 +130,6 @@ pub(crate) fn core_error_to_serdes(tool_name: &str, err: CoreError) -> SerdesErr } } -fn field_for_out_of_bounds(msg: &str) -> Option { - if msg.starts_with("offset ") || msg.starts_with("offset must") { - Some("offset".to_string()) - } else if msg.starts_with("limit ") || msg.starts_with("limit must") { - Some("limit".to_string()) - } else { - None - } -} - /// Convert [`ToolOutput`] to [`ToolReturn`] (serdesAI). /// /// - Non-truncated output: `ToolReturn::text(content)` @@ -141,7 +138,7 @@ fn field_for_out_of_bounds(msg: &str) -> Option { /// [`ToolOutput`]: reloaded_code_core::ToolOutput /// [`ToolReturn`]: serdes_ai::tools::ToolReturn #[inline] -fn output_to_return(output: ToolOutput) -> ToolReturn { +pub(crate) fn output_to_return(output: ToolOutput) -> ToolReturn { if output.truncated { ToolReturn::json(json!({ "content": output.content, @@ -152,6 +149,92 @@ fn output_to_return(output: ToolOutput) -> ToolReturn { } } +/// Convert a SerdesAI [`ToolReturn`] reference to a core [`ToolOutput`]. +/// +/// Used by the tool-hook bridge so the hook chain can consume the real tool +/// result and transform it before it is converted back to SerdesAI types. +/// +/// Variant handling: +/// - Text: content maps directly. +/// - JSON: `{"content": .., "truncated": true}` maps to a truncated output, +/// mirroring [`output_to_return`]; any other JSON keeps its serialized form. +/// - Image and multi-part returns map to placeholder text: the core output is +/// text-only, and the bridge restores the untouched original afterward. +/// - Error returns map to a text rendering of the error payload. +/// +/// [`ToolOutput`]: reloaded_code_core::ToolOutput +/// [`ToolReturn`]: serdes_ai::tools::ToolReturn +pub(crate) fn return_to_output(tool_return: &ToolReturn) -> ToolOutput { + use serdes_ai::core::messages::ToolReturnContent; + match &tool_return.content { + ToolReturnContent::Text { content } => ToolOutput::new(content.clone()), + ToolReturnContent::Json { content } => { + if let Some(output) = truncated_json_to_output(content) { + output + } else { + ToolOutput::new(content.to_string()) + } + } + ToolReturnContent::Image { .. } => ToolOutput::new(IMAGE_RETURN_PLACEHOLDER), + ToolReturnContent::Error { error } => ToolOutput::new(format!( + "[tool error return] {}: {}", + error.kind, error.message + )), + ToolReturnContent::Multiple { .. } => ToolOutput::new(MULTIPLE_RETURN_PLACEHOLDER), + } +} + +/// Convert a SerdesAI [`ToolError`][serdes] to a core [`ToolError`][core]. +/// +/// Inverse of [`core_error_to_serdes`], used by the tool-hook bridge so hook +/// chains see structured validation failures instead of flattened strings. +/// Round-trips exactly when the error is untouched; the bridge also restores +/// the untouched original error itself, so fidelity here only matters for +/// hook-modified errors. +/// +/// [core]: reloaded_code_core::ToolError +/// [serdes]: serdes_ai::tools::ToolError +pub(crate) fn serdes_error_to_core(err: &SerdesError) -> CoreError { + match err { + SerdesError::ValidationFailed { errors, .. } => { + let message = if errors.is_empty() { + err.to_string() + } else { + errors + .iter() + .map(|e| match &e.field { + Some(field) => format!("{field}: {}", e.message), + None => e.message.clone(), + }) + .collect::>() + .join("; ") + }; + CoreError::Validation { + field: errors.first().and_then(|e| e.field.clone()), + message, + } + } + _ => CoreError::Execution(err.to_string()), + } +} + +fn field_for_out_of_bounds(msg: &str) -> Option { + if msg.starts_with("offset ") || msg.starts_with("offset must") { + Some("offset".to_string()) + } else if msg.starts_with("limit ") || msg.starts_with("limit must") { + Some("limit".to_string()) + } else { + None + } +} + +/// Detects the truncated-marker convention written by [`output_to_return`]. +fn truncated_json_to_output(value: &JsonValue) -> Option { + let content = value.get("content")?.as_str()?; + let truncated = value.get("truncated")?.as_bool()?; + truncated.then(|| ToolOutput::truncated(content)) +} + #[cfg(test)] mod tests { use super::*; @@ -165,6 +248,54 @@ mod tests { assert_eq!(ret.as_text(), Some("hello world")); } + #[test] + fn text_return_round_trips_through_output() { + let ret = ToolReturn::text("plain result"); + let output = return_to_output(&ret); + assert_eq!(output.content, "plain result"); + assert!(!output.truncated); + } + + #[test] + fn truncated_output_round_trips_through_return() { + let output = ToolOutput::truncated("partial"); + let roundtrip = return_to_output(&output_to_return(output)); + assert_eq!(roundtrip.content, "partial"); + assert!(roundtrip.truncated); + } + + #[test] + fn arbitrary_json_return_keeps_serialized_form() { + let ret = ToolReturn::json(json!({"rows": [1, 2]})); + let output = return_to_output(&ret); + assert!(!output.truncated); + assert!(output.content.contains("\"rows\"")); + } + + #[test] + fn image_return_maps_to_placeholder_not_debug_dump() { + use serdes_ai::core::messages::ToolReturnContent; + let ret = ToolReturn { + content: ToolReturnContent::Image { + image: serdes_ai::core::messages::ImageContent::Url( + serdes_ai::core::messages::ImageUrl::new("https://example.invalid/x.png"), + ), + }, + tool_call_id: None, + }; + let output = return_to_output(&ret); + assert_eq!(output.content, IMAGE_RETURN_PLACEHOLDER); + } + + #[test] + fn serdes_validation_error_maps_to_core_validation() { + let serdes_err = SerdesError::validation_error("read", Some("path".into()), "bad path"); + assert!(matches!( + serdes_error_to_core(&serdes_err), + CoreError::Validation { .. } + )); + } + #[test] fn tool_output_converts_to_json_when_truncated() { let output = ToolOutput::truncated("partial content"); diff --git a/src/reloaded-code-serdesai/src/lib.rs b/src/reloaded-code-serdesai/src/lib.rs index 23e882d..103091f 100644 --- a/src/reloaded-code-serdesai/src/lib.rs +++ b/src/reloaded-code-serdesai/src/lib.rs @@ -28,7 +28,7 @@ pub use reloaded_code_core::{ TodoPriority, TodoState, TodoStatus, WebFetchOutput, }; // Re-export standalone tools and runtime helpers -pub use agent_runtime::{AgentBuildContext, AgentBuildError}; +pub use agent_runtime::{AgentBuildContext, AgentBuildError, HookedAgent, HookedAgentRunResult}; pub use reloaded_code_agents::{ AgentDefaults, AgentRuntime, AgentRuntimeBuilder, ModelResolutionError, ResolvedModel, resolve_model_with_catalog, diff --git a/src/reloaded-code-serdesai/src/mock.rs b/src/reloaded-code-serdesai/src/mock.rs index ad45c6e..c6edb4d 100644 --- a/src/reloaded-code-serdesai/src/mock.rs +++ b/src/reloaded-code-serdesai/src/mock.rs @@ -40,13 +40,9 @@ use serdes_ai_models::{ /// Wrapper adding [`request_stream`] support to any [`ModelTrait`] implementation. /// -/// [`request_stream`]: ModelTrait::request_stream -/// /// Delegates [`request`] directly to the inner model and converts the non-streaming /// response into a sequence of [`ModelResponseStreamEvent`]s for streaming callers. /// -/// [`request`]: ModelTrait::request -/// /// # Example /// /// ```rust,no_run @@ -55,6 +51,9 @@ use serdes_ai_models::{ /// /// let model = Streamed::new(FunctionModel::tool_call("glob", json!({"pattern": "*.rs"}))); /// ``` +/// +/// [`request_stream`]: ModelTrait::request_stream +/// [`request`]: ModelTrait::request #[derive(Clone, Debug)] pub struct Streamed { inner: T, @@ -156,7 +155,6 @@ pub fn tool_then_text( ) -> Streamed { let tool_name = tool_name.into(); let fallback_text = fallback_text.into(); - let tool_name_clone = tool_name.clone(); let model = FunctionModel::new(move |messages, _settings| { // Check whether the conversation already contains a tool return from a @@ -186,11 +184,71 @@ pub fn tool_then_text( ModelResponse::text(text) } else { // First call: emit a tool call so the agent executes the real tool. - ModelResponse::with_parts(vec![ - ModelResponsePart::text(format!("Calling {tool_name}...")), - ModelResponsePart::tool_call(tool_name_clone.clone(), args.clone()), - ]) - .with_finish_reason(FinishReason::ToolCall) + tool_call_response(&tool_name, &args) + } + }); + + Streamed::new(model) +} + +/// Scripts two tool calls followed by a text answer, generalising +/// [`tool_then_text`] to two-step flows: the first turn calls `first`, the +/// next turn calls `second`, and once both tool returns are in the +/// conversation the model answers with `final_text` followed by the real +/// tool returns, so callers can observe what the model received. +/// +/// # Arguments +/// +/// - `first` - name and JSON arguments of the tool called on the first turn. +/// - `second` - name and JSON arguments of the tool called on the second turn. +/// - `final_text` - text prefix for the closing response; the real tool +/// returns are appended after it. +/// +/// # Example +/// +/// ```rust,no_run +/// use reloaded_code_serdesai::mock::two_tools_then_text; +/// use serde_json::json; +/// +/// let model = two_tools_then_text( +/// ("read", json!({"file_path": "service.env"})), +/// ("write", json!({"file_path": "draft.md", "content": "notes"})), +/// "Run finished.", +/// ); +/// ``` +pub fn two_tools_then_text( + first: (impl Into, serde_json::Value), + second: (impl Into, serde_json::Value), + final_text: impl Into, +) -> Streamed { + let first_name = first.0.into(); + let first_args = first.1; + let second_name = second.0.into(); + let second_args = second.1; + let final_text = final_text.into(); + + let model = FunctionModel::new(move |messages, _settings| { + // Each finished tool call adds one tool return to the history, so + // their count tells which scripted turn is next. + let answered_calls = messages.iter().flat_map(|m| m.tool_returns()).count(); + + match answered_calls { + 0 => tool_call_response(&first_name, &first_args), + 1 => tool_call_response(&second_name, &second_args), + _ => { + let tool_results: String = messages + .iter() + .flat_map(|m| m.tool_returns()) + .map(extract_tool_return_text) + .collect::>() + .join("\n"); + let text = if tool_results.is_empty() { + final_text.clone() + } else { + format!("{final_text}\n\n{tool_results}") + }; + ModelResponse::text(text) + } } }); @@ -248,3 +306,13 @@ fn response_to_stream_events(response: ModelResponse) -> Vec ModelResponse { + ModelResponse::with_parts(vec![ + ModelResponsePart::text(format!("Calling {tool_name}...")), + ModelResponsePart::tool_call(tool_name, args.clone()), + ]) + .with_finish_reason(FinishReason::ToolCall) +} diff --git a/src/reloaded-code-serdesai/src/task/handle.rs b/src/reloaded-code-serdesai/src/task/handle.rs index effa3ee..eff779a 100644 --- a/src/reloaded-code-serdesai/src/task/handle.rs +++ b/src/reloaded-code-serdesai/src/task/handle.rs @@ -38,8 +38,6 @@ where /// - `input` — task payload including the [`subagent_type`] /// and prompt. /// - /// [`subagent_type`]: TaskInput::subagent_type - /// /// # Returns /// /// A [`TaskOutput`] wrapping the sub-agent's text response. @@ -54,6 +52,8 @@ where /// /// Returns [`ToolError::ExecutionFailed`] when the sub-agent fails to build or /// produce a response. + /// + /// [`subagent_type`]: TaskInput::subagent_type pub(crate) async fn execute( &self, caller_name: &str, @@ -154,83 +154,15 @@ where mod tests { use super::*; use crate::agent_runtime::TaskBuildContext; - use ahash::AHashMap; - use indexmap::IndexMap; + use crate::agent_runtime::test_stubs::{ + agent, allow_tools, catalog, credentials, pattern_task, workspace_root, + }; use reloaded_code_agents::{ AgentCatalog, AgentConfig, AgentDefaults, AgentMode, AgentRuntimeBuilder, - AgentToolSettings, PermissionRule, }; use reloaded_code_core::CredentialResolver; - use reloaded_code_core::models::{ - Modality, ModelCatalog, ModelInfo, ProviderIdx, ProviderInfo, ProviderModelSource, - ProviderSource, ProviderType, - }; - use reloaded_code_core::permissions::{ExpandError, PermissionAction}; - - fn agent( - name: &str, - mode: AgentMode, - permission: IndexMap, - ) -> AgentConfig { - AgentConfig { - name: name.into(), - mode, - description: format!("{name} description").into(), - model: None, - hidden: false, - temperature: None, - top_p: None, - permission, - options: AHashMap::new(), - tool_settings: AgentToolSettings::default(), - prompt: Default::default(), - } - } - - fn allow_tools(names: &[&str]) -> IndexMap { - names - .iter() - .map(|n| ((*n).into(), PermissionRule::Action(PermissionAction::Allow))) - .collect() - } - - fn pattern_task(patterns: &[(&str, PermissionAction)]) -> IndexMap { - let mut map = IndexMap::new(); - for (pattern, action) in patterns { - map.insert(pattern.to_string(), *action); - } - IndexMap::from([(task_meta::NAME.into(), PermissionRule::Pattern(map))]) - } - - fn catalog() -> ModelCatalog { - let providers = vec![ProviderSource::new( - "openrouter", - ProviderInfo { - api_url: "https://openrouter.ai/api/v1".into(), - env_vars: vec!["OPENROUTER_API_KEY".into()], - api_type: ProviderType::OpenRouter, - }, - )]; - let info = ModelInfo { - modalities: Modality::TEXT, - max_input: 128_000, - max_output: 16_384, - temperature: Some(1.0), - top_p: Some(0.95), - }; - let models: Vec> = - [("openai/gpt-4.1-mini", info), ("openai/gpt-4o", info)] - .into_iter() - .map(|(key, i)| ProviderModelSource::new(ProviderIdx::new(0), key, i)) - .collect(); - ModelCatalog::build(&providers, &models).expect("catalog fixture should build") - } - - fn credentials() -> Arc> { - let mut resolver = CredentialResolver::without_env(); - resolver.set_override("OPENROUTER_API_KEY", "test-key"); - Arc::new(resolver) - } + use reloaded_code_core::permissions::ExpandError; + use reloaded_code_core::permissions::PermissionAction; fn runtime_with_agents(agents: Vec) -> AgentRuntimeBuilder { AgentRuntimeBuilder::new() @@ -244,8 +176,8 @@ mod tests { Arc::new(TaskBuildContext::new_for_test( Arc::new(runtime.expect("test fixture should not fail pattern expansion")), Arc::new(catalog()), - credentials(), - Arc::from(reloaded_code_core::resolve_workspace_root().expect("workspace root")), + Arc::new(credentials()), + workspace_root(), )) } @@ -255,6 +187,7 @@ mod tests { "caller", AgentMode::All, allow_tools(&[task_meta::NAME]), + "", )]) .build(); let context = build_test_context(runtime); @@ -285,8 +218,13 @@ mod tests { #[tokio::test] async fn validate_target_rejects_primary_target() { let runtime = runtime_with_agents(vec![ - agent("caller", AgentMode::All, allow_tools(&[task_meta::NAME])), - agent("primary-agent", AgentMode::Primary, allow_tools(&[])), + agent( + "caller", + AgentMode::All, + allow_tools(&[task_meta::NAME]), + "", + ), + agent("primary-agent", AgentMode::Primary, allow_tools(&[]), ""), ]) .build(); let context = build_test_context(runtime); @@ -321,8 +259,9 @@ mod tests { "caller", AgentMode::All, pattern_task(&[("*", PermissionAction::Deny)]), + "", ), - agent("target", AgentMode::All, allow_tools(&[])), + agent("target", AgentMode::All, allow_tools(&[]), ""), ]) .build(); let context = build_test_context(runtime); @@ -355,8 +294,13 @@ mod tests { // Defense-in-depth: even if the Task tool were somehow present at max depth, // execute() rejects the call. let runtime = runtime_with_agents(vec![ - agent("caller", AgentMode::All, allow_tools(&[task_meta::NAME])), - agent("target", AgentMode::All, allow_tools(&[])), + agent( + "caller", + AgentMode::All, + allow_tools(&[task_meta::NAME]), + "", + ), + agent("target", AgentMode::All, allow_tools(&[]), ""), ]) .defaults(AgentDefaults::with_model("openrouter/openai/gpt-4.1-mini")) .max_task_depth(0) diff --git a/src/reloaded-code-serdesai/src/tools/custom.rs b/src/reloaded-code-serdesai/src/tools/custom.rs index 137c3a8..ef7b31b 100644 --- a/src/reloaded-code-serdesai/src/tools/custom.rs +++ b/src/reloaded-code-serdesai/src/tools/custom.rs @@ -4,15 +4,10 @@ //! [`AgentBuilder`] via //! [`AgentBuilderExt`](crate::agent_ext::AgentBuilderExt). //! -//! [`AgentBuilder`]: serdes_ai::AgentBuilder -//! //! The adapter is also used internally by the agent-runtime build layer, but //! it lives here so non-agent users can attach portable custom tools to a plain //! SerdesAI agent without going through [`AgentRuntimeBuilder`]. //! -//! [`AgentRuntimeBuilder`]: reloaded_code_agents::AgentRuntimeBuilder -//! [`AgentBuilderExt`]: crate::agent_ext::AgentBuilderExt -//! //! # Example //! //! ```no_run @@ -65,6 +60,10 @@ //! .build(); //! # Ok::<(), Box>(()) //! ``` +//! +//! [`AgentBuilder`]: serdes_ai::AgentBuilder +//! [`AgentRuntimeBuilder`]: reloaded_code_agents::AgentRuntimeBuilder +//! [`AgentBuilderExt`]: crate::agent_ext::AgentBuilderExt use async_trait::async_trait; use reloaded_code_core::context::ToolContext;