diff --git a/crates/tinytools/README.md b/crates/tinytools/README.md index 65f2754..8a101ff 100644 --- a/crates/tinytools/README.md +++ b/crates/tinytools/README.md @@ -35,7 +35,77 @@ impl Tool for Echo { `Tool` describes a callable capability. Its result is a `ToolResult` block list with a reported-error flag and optional markdown rendering. `ToolSpec` is the model-visible declaration. `ToolRunContext` exposes only tool-relevant run -facts: workspace, thread id, and output cap. +facts: workspace, thread id, and output cap — plus `host_extension()`, the +same type-erased escape hatch `Tool::host_extension` offers, so a tool written +against one specific harness can downcast to that harness's full context. + +## Rich tool returns + +`ToolContent` has four block kinds: `Text`, `Json`, `Image`, and `File`. +`Image` carries a MIME `media_type` plus `ImageData::{Base64, Url}`; `File` +carries a display `name`, a `media_type`, and `FileData::{Base64, Url, Path}`. +`ToolResult::text()`, `output()`, and `output_for_llm()` render `Text` +verbatim, pretty-print `Json`, skip `Json` from `text()` specifically (as +before), and render a short placeholder for `Image`/`File` — `[image +image/png]`, `[file report.pdf (application/pdf)]` — so a model still gets a +sensible turn even when a renderer does not special-case the new block kinds. + +`ToolResult` carries three more optional surfaces beyond `content` and +`markdown_formatted`: + +- `follow_up: Vec` — content the caller should present to the + model as a *separate* user message after the tool result (a screenshot, a + generated document). It is never included in `text()`, `output()`, or + `output_for_llm()`; a host that wants to honour it reads the field directly. + Attach it with `with_follow_up(..)`. +- `metadata: Option` — host-only data (trace ids, raw + provider payloads) that is never shown to the model. Attach it with + `with_metadata(..)`. +- `control: Option` — loop-control hints a harness may honour: + `return_direct: Option`, `terminate`, `goto: Option`, and + `state_update: Option`. Set them with the builders + `return_direct()`, `dont_return_direct()`, `terminate()`, `with_goto(..)`, + and `with_state_update(..)`, which lazily create the `ToolControl`. + `return_direct` is tri-state, not a defaulted `bool`: a call that only used + `with_goto(..)`, `with_state_update(..)`, or `terminate()` leaves it `None` + rather than an implicit `false`, so it cannot silently suppress a tool's + static `true` default — see "Static and per-call return-direct" below. + +`ToolResult::retry(message)` and `ToolResult::failed(message)` both set +`is_error`, same as `error(message)`, but additionally tag +`error_kind: Option` as `Retry` or `Failed` — Pydantic AI's +`ModelRetry` versus a permanent tool failure — so a harness can decide whether +to loop the model back in or surface the failure as final. + +Every new field is `#[serde(default)]` and, where it can be empty or absent, +`skip_serializing_if`, so a `ToolResult` persisted before these fields existed +still decodes, and a plain result's wire shape is unchanged. + +## Static and per-call return-direct + +`Tool::return_direct()` is a static, per-tool default (`false`) for a tool +whose entire purpose is to hand the model's answer straight back — a +final-answer or handoff tool overrides it to `true`. `ToolResult::control`'s +`return_direct: Option` is the per-*call* override on `ToolControl`; a +harness should prefer `Some(..)` on the result it just received over the +tool's static declaration, and fall back to the static declaration when it is +`None`. `None` is the outcome of a call that never touched `return_direct` — +including one that only used `with_goto(..)`, `with_state_update(..)`, or +`terminate()` — so it must not be read as an explicit override. Call +`return_direct()` for `Some(true)`, or `dont_return_direct()` for `Some(false)` +to force the call to *not* return directly even when the tool's static +declaration is `true`. + +## Replay after a crash + +`ToolPolicy`'s `ToolRuntime` carries `replay: ToolReplay`, mirroring pi's +`replay` classification: whether an orphaned in-flight call for a tool may be +safely re-executed after a crash. It defaults to `ToolReplay::Never`; a tool +that is idempotent or otherwise safe to repeat declares `ToolReplay::Safe` +through its `ToolPolicy`. This lives on the existing declarative policy +surface rather than as a new `Tool` trait method, consistent with how every +other runtime requirement (timeout, retries, cancellation, sandboxing) is +already expressed there. `ToolPolicy` is the complete host-readable declaration around a call: diff --git a/crates/tinytools/src/context/test.rs b/crates/tinytools/src/context/test.rs index ad3a9b6..2278968 100644 --- a/crates/tinytools/src/context/test.rs +++ b/crates/tinytools/src/context/test.rs @@ -56,3 +56,35 @@ fn an_implementor_is_readable_through_the_trait_object() { Some(PathBuf::from("/tmp/worktree")) ); } + +/// A context carrying a host-owned payload behind the erased hook. +struct Hosted { + tag: HostTag, +} + +#[derive(Debug, PartialEq)] +struct HostTag(&'static str); + +impl ToolRunContext for Hosted { + fn host_extension(&self) -> Option<&(dyn std::any::Any + Send + Sync)> { + Some(&self.tag) + } +} + +#[test] +fn the_default_host_extension_is_absent() { + let erased: &dyn ToolRunContext = &Bare; + assert!(erased.host_extension().is_none()); +} + +#[test] +fn a_host_recovers_its_own_context_by_downcasting() { + let hosted = Hosted { + tag: HostTag("call-7"), + }; + let erased: &dyn ToolRunContext = &hosted; + let tag = erased + .host_extension() + .and_then(|any| any.downcast_ref::()); + assert_eq!(tag, Some(&HostTag("call-7"))); +} diff --git a/crates/tinytools/src/context/types.rs b/crates/tinytools/src/context/types.rs index b14a60a..4357594 100644 --- a/crates/tinytools/src/context/types.rs +++ b/crates/tinytools/src/context/types.rs @@ -61,4 +61,17 @@ pub trait ToolRunContext: Send + Sync { fn workspace_policy_id(&self) -> Option<&str> { self.workspace().map(|w| w.policy_id.as_str()) } + + /// The host's own context object, erased, for a tool written against a + /// specific harness that needs more than the portable facts above. + /// + /// The same escape hatch as [`Tool::host_extension`][crate::Tool::host_extension]: + /// this crate has no business naming the harness's context type, so a + /// host returns `Some(self)` and a tool that knows which host it runs + /// under downcasts. Every other implementor returns `None` and pays + /// nothing. A tool that only needs the workspace, thread id, or output + /// cap should keep using the typed methods. + fn host_extension(&self) -> Option<&(dyn std::any::Any + Send + Sync)> { + None + } } diff --git a/crates/tinytools/src/lib.rs b/crates/tinytools/src/lib.rs index d0b04c5..22e1d5a 100644 --- a/crates/tinytools/src/lib.rs +++ b/crates/tinytools/src/lib.rs @@ -122,9 +122,9 @@ pub use naming::{ }; pub use permission::PermissionLevel; pub use policy::{ - ToolAccess, ToolDisplay, ToolPolicy, ToolRuntime, ToolSideEffects, WorkspaceAccess, + ToolAccess, ToolDisplay, ToolPolicy, ToolReplay, ToolRuntime, ToolSideEffects, WorkspaceAccess, }; -pub use result::{ToolContent, ToolResult}; +pub use result::{FileData, ImageData, ToolContent, ToolControl, ToolErrorKind, ToolResult}; pub use spec::ToolSpec; pub use tool::{Tool, ToolExposure}; pub use workspace::{SandboxMode, WorkspaceDescriptor}; diff --git a/crates/tinytools/src/policy/mod.rs b/crates/tinytools/src/policy/mod.rs index 80dd263..417348b 100644 --- a/crates/tinytools/src/policy/mod.rs +++ b/crates/tinytools/src/policy/mod.rs @@ -7,7 +7,7 @@ mod types; pub use types::{ - ToolAccess, ToolDisplay, ToolPolicy, ToolRuntime, ToolSideEffects, WorkspaceAccess, + ToolAccess, ToolDisplay, ToolPolicy, ToolReplay, ToolRuntime, ToolSideEffects, WorkspaceAccess, }; #[cfg(test)] diff --git a/crates/tinytools/src/policy/test.rs b/crates/tinytools/src/policy/test.rs index 05c10b1..1669d58 100644 --- a/crates/tinytools/src/policy/test.rs +++ b/crates/tinytools/src/policy/test.rs @@ -2,7 +2,9 @@ #![allow(clippy::expect_used, clippy::panic, clippy::unwrap_used)] -use super::{ToolAccess, ToolDisplay, ToolPolicy, ToolRuntime, ToolSideEffects, WorkspaceAccess}; +use super::{ + ToolAccess, ToolDisplay, ToolPolicy, ToolReplay, ToolRuntime, ToolSideEffects, WorkspaceAccess, +}; use crate::{SandboxMode, ToolTimeout}; #[test] @@ -106,6 +108,7 @@ fn policy_round_trips_through_its_stable_json_shape() { "cancelable": false, "sandbox": "required", "streaming": false, + "replay": "never", }, "access": { "workspace": "any", @@ -140,6 +143,7 @@ fn fully_populated_policy_has_a_pinned_json_wire_shape() { sandbox: SandboxMode::Required, max_result_bytes: Some(8_192), streaming: true, + replay: ToolReplay::Safe, }, access: ToolAccess { workspace: WorkspaceAccess::Scoped, @@ -170,6 +174,7 @@ fn fully_populated_policy_has_a_pinned_json_wire_shape() { "sandbox": "required", "max_result_bytes": 8192, "streaming": true, + "replay": "safe", }, "access": { "workspace": "scoped", @@ -190,3 +195,34 @@ fn fully_populated_policy_has_a_pinned_json_wire_shape() { policy ); } + +#[test] +fn replay_defaults_to_never() { + assert_eq!(ToolRuntime::default().replay, ToolReplay::Never); +} + +#[test] +fn replay_round_trips_through_json() { + let runtime = ToolRuntime { + replay: ToolReplay::Safe, + ..ToolRuntime::default() + }; + let encoded = serde_json::to_string(&runtime).expect("serializable"); + assert!(encoded.contains("\"replay\":\"safe\"")); + let back: ToolRuntime = serde_json::from_str(&encoded).expect("deserializable"); + assert_eq!(back.replay, ToolReplay::Safe); +} + +#[test] +fn legacy_runtime_json_without_replay_defaults_to_never() { + // A `ToolRuntime` persisted before `replay` existed should still decode. + let literal = r#"{ + "timeout": { "mode": "inherit" }, + "idempotent": false, + "cancelable": false, + "sandbox": "disabled", + "streaming": false + }"#; + let decoded: ToolRuntime = serde_json::from_str(literal).expect("deserializable"); + assert_eq!(decoded.replay, ToolReplay::Never); +} diff --git a/crates/tinytools/src/policy/types.rs b/crates/tinytools/src/policy/types.rs index 41dcc1b..6deee60 100644 --- a/crates/tinytools/src/policy/types.rs +++ b/crates/tinytools/src/policy/types.rs @@ -90,6 +90,24 @@ impl ToolDisplay { } } +/// Whether an orphaned in-flight call may be safely re-executed after a +/// crash. +/// +/// A host that persists an in-flight call and recovers after a crash has to +/// decide whether to replay it. Pi's `replay` classification is the reference +/// design: most tools are not safe to blindly re-run (a payment, a send), so +/// [`Self::Never`] is the default and a tool must opt into [`Self::Safe`]. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ToolReplay { + /// An orphaned call must not be re-executed after a crash. + #[default] + Never, + /// An orphaned call may be safely re-executed after a crash — the tool is + /// idempotent or otherwise safe to repeat. + Safe, +} + /// Runtime requirements a tool declares for safe execution. /// /// A host decides how to apply these requirements. In particular, this type @@ -117,6 +135,10 @@ pub struct ToolRuntime { pub max_result_bytes: Option, /// Whether the tool can emit streaming result fragments. pub streaming: bool, + /// Whether an orphaned in-flight call for this tool may be safely + /// re-executed after a crash. See [`ToolReplay`]. + #[serde(default)] + pub replay: ToolReplay, } /// Access requirements a tool declares before a host exposes or runs it. diff --git a/crates/tinytools/src/result/mod.rs b/crates/tinytools/src/result/mod.rs index 46decef..f5a6b9b 100644 --- a/crates/tinytools/src/result/mod.rs +++ b/crates/tinytools/src/result/mod.rs @@ -2,7 +2,7 @@ mod types; -pub use types::{ToolContent, ToolResult}; +pub use types::{FileData, ImageData, ToolContent, ToolControl, ToolErrorKind, ToolResult}; #[cfg(test)] mod test; diff --git a/crates/tinytools/src/result/test.rs b/crates/tinytools/src/result/test.rs index 4ddd50e..d84e596 100644 --- a/crates/tinytools/src/result/test.rs +++ b/crates/tinytools/src/result/test.rs @@ -5,7 +5,7 @@ use serde_json::json; -use super::{ToolContent, ToolResult}; +use super::{FileData, ImageData, ToolContent, ToolControl, ToolErrorKind, ToolResult}; #[test] fn success_carries_one_text_block() { @@ -44,8 +44,7 @@ fn mixed_content_joins_in_order() { text: "line2".into(), }, ], - is_error: false, - markdown_formatted: None, + ..ToolResult::default() }; assert_eq!(r.text(), "line1\nline2"); let output = r.output(); @@ -58,8 +57,7 @@ fn mixed_content_joins_in_order() { fn empty_content_renders_empty() { let r = ToolResult { content: vec![], - is_error: false, - markdown_formatted: None, + ..ToolResult::default() }; assert!(r.text().is_empty()); assert!(r.output().is_empty()); @@ -115,14 +113,280 @@ fn content_blocks_are_tagged_by_type() { match serde_json::from_str::(&text).expect("deserializable") { ToolContent::Text { text } => assert_eq!(text, "test"), - ToolContent::Json { .. } => unreachable!("tagged as text"), + other => unreachable!("tagged as text, got {other:?}"), } match serde_json::from_str::(&data).expect("deserializable") { ToolContent::Json { data } => assert_eq!(data["x"], 1), - ToolContent::Text { .. } => unreachable!("tagged as json"), + other => unreachable!("tagged as json, got {other:?}"), } } +#[test] +fn image_block_round_trips_through_json() { + let block = ToolContent::Image { + media_type: "image/png".into(), + data: ImageData::Base64("aGVsbG8=".into()), + }; + let encoded = serde_json::to_value(&block).expect("serializable"); + assert_eq!( + encoded, + json!({ + "type": "image", + "media_type": "image/png", + "data": { "kind": "base64", "value": "aGVsbG8=" }, + }) + ); + let decoded: ToolContent = serde_json::from_value(encoded).expect("deserializable"); + match decoded { + ToolContent::Image { media_type, data } => { + assert_eq!(media_type, "image/png"); + assert!(matches!(data, ImageData::Base64(b) if b == "aGVsbG8=")); + } + other => unreachable!("tagged as image, got {other:?}"), + } +} + +#[test] +fn image_block_supports_url_data() { + let block = ToolContent::Image { + media_type: "image/jpeg".into(), + data: ImageData::Url("https://example.com/a.jpg".into()), + }; + let encoded = serde_json::to_string(&block).expect("serializable"); + let decoded: ToolContent = serde_json::from_str(&encoded).expect("deserializable"); + match decoded { + ToolContent::Image { data, .. } => { + assert!(matches!(data, ImageData::Url(u) if u == "https://example.com/a.jpg")); + } + other => unreachable!("tagged as image, got {other:?}"), + } +} + +#[test] +fn file_block_round_trips_through_json() { + let block = ToolContent::File { + name: "report.pdf".into(), + media_type: "application/pdf".into(), + data: FileData::Path("/tmp/report.pdf".into()), + }; + let encoded = serde_json::to_value(&block).expect("serializable"); + assert_eq!( + encoded, + json!({ + "type": "file", + "name": "report.pdf", + "media_type": "application/pdf", + "data": { "kind": "path", "value": "/tmp/report.pdf" }, + }) + ); + let decoded: ToolContent = serde_json::from_value(encoded).expect("deserializable"); + match decoded { + ToolContent::File { + name, + media_type, + data, + } => { + assert_eq!(name, "report.pdf"); + assert_eq!(media_type, "application/pdf"); + assert!(matches!(data, FileData::Path(p) if p == "/tmp/report.pdf")); + } + other => unreachable!("tagged as file, got {other:?}"), + } +} + +#[test] +fn file_block_supports_base64_and_url_data() { + let base64 = FileData::Base64("aGk=".into()); + let url = FileData::Url("https://example.com/a.csv".into()); + for data in [base64, url] { + let block = ToolContent::File { + name: "a".into(), + media_type: "text/csv".into(), + data, + }; + let encoded = serde_json::to_string(&block).expect("serializable"); + let _: ToolContent = serde_json::from_str(&encoded).expect("deserializable"); + } +} + +#[test] +fn text_and_output_render_placeholders_for_image_and_file_blocks() { + let r = ToolResult { + content: vec![ + ToolContent::Text { + text: "before".into(), + }, + ToolContent::Image { + media_type: "image/png".into(), + data: ImageData::Base64("Zm9v".into()), + }, + ToolContent::File { + name: "notes.txt".into(), + media_type: "text/plain".into(), + data: FileData::Url("https://example.com/notes.txt".into()), + }, + ], + ..ToolResult::default() + }; + assert_eq!( + r.text(), + "before\n[image image/png]\n[file notes.txt (text/plain)]" + ); + assert_eq!(r.output(), r.text()); +} + +#[test] +fn with_image_appends_an_image_block() { + let r = + ToolResult::success("caption").with_image("image/png", ImageData::Base64("Zm9v".into())); + assert_eq!(r.content.len(), 2); + assert!(r.text().ends_with("[image image/png]")); +} + +#[test] +fn with_follow_up_is_not_included_in_text_or_output() { + let r = ToolResult::success("primary").with_follow_up(vec![ToolContent::Text { + text: "secondary".into(), + }]); + assert_eq!(r.text(), "primary"); + assert_eq!(r.output(), "primary"); + assert_eq!(r.follow_up.len(), 1); +} + +#[test] +fn follow_up_is_omitted_from_wire_shape_when_empty() { + let r = ToolResult::success("plain"); + let encoded = serde_json::to_value(&r).expect("serializable"); + assert!(encoded.get("follow_up").is_none()); +} + +#[test] +fn follow_up_round_trips_when_present() { + let r = ToolResult::success("primary").with_follow_up(vec![ToolContent::Text { + text: "secondary".into(), + }]); + let encoded = serde_json::to_string(&r).expect("serializable"); + let back: ToolResult = serde_json::from_str(&encoded).expect("deserializable"); + assert_eq!(back.follow_up.len(), 1); +} + +#[test] +fn with_metadata_is_never_rendered_but_round_trips() { + let r = ToolResult::success("primary").with_metadata(json!({"trace_id": "abc"})); + assert_eq!(r.text(), "primary"); + assert!(!r.output().contains("trace_id")); + let encoded = serde_json::to_string(&r).expect("serializable"); + let back: ToolResult = serde_json::from_str(&encoded).expect("deserializable"); + assert_eq!(back.metadata, Some(json!({"trace_id": "abc"}))); +} + +#[test] +fn metadata_is_omitted_from_wire_shape_when_absent() { + let r = ToolResult::success("plain"); + let encoded = serde_json::to_value(&r).expect("serializable"); + assert!(encoded.get("metadata").is_none()); +} + +#[test] +fn control_builders_set_the_expected_fields() { + let r = ToolResult::success("done") + .return_direct() + .terminate() + .with_goto("next_node") + .with_state_update(json!({"count": 1})); + let control = r.control.as_ref().expect("control set"); + assert_eq!(control.return_direct, Some(true)); + assert!(control.terminate); + assert_eq!(control.goto.as_deref(), Some("next_node")); + assert_eq!(control.state_update, Some(json!({"count": 1}))); +} + +#[test] +fn control_is_omitted_from_wire_shape_when_absent() { + let r = ToolResult::success("plain"); + let encoded = serde_json::to_value(&r).expect("serializable"); + assert!(encoded.get("control").is_none()); +} + +#[test] +fn control_round_trips_through_json() { + let r = ToolResult::success("done").return_direct().with_goto("n"); + let encoded = serde_json::to_string(&r).expect("serializable"); + let back: ToolResult = serde_json::from_str(&encoded).expect("deserializable"); + let control = back.control.expect("control set"); + assert_eq!(control.return_direct, Some(true)); + assert!(!control.terminate); + assert_eq!(control.goto.as_deref(), Some("n")); +} + +#[test] +fn retry_and_failed_both_set_is_error_but_distinct_error_kind() { + let retry = ToolResult::retry("try again"); + assert!(retry.is_error); + assert_eq!(retry.error_kind, Some(ToolErrorKind::Retry)); + assert_eq!(retry.text(), "try again"); + + let failed = ToolResult::failed("do not retry"); + assert!(failed.is_error); + assert_eq!(failed.error_kind, Some(ToolErrorKind::Failed)); + assert_eq!(failed.text(), "do not retry"); +} + +#[test] +fn plain_error_leaves_error_kind_unset() { + let r = ToolResult::error("failed"); + assert_eq!(r.error_kind, None); +} + +#[test] +fn error_kind_round_trips_through_json() { + let r = ToolResult::retry("again"); + let encoded = serde_json::to_string(&r).expect("serializable"); + let back: ToolResult = serde_json::from_str(&encoded).expect("deserializable"); + assert_eq!(back.error_kind, Some(ToolErrorKind::Retry)); +} + +#[test] +fn legacy_json_without_new_fields_still_deserializes() { + // A transcript persisted before these fields existed should still decode, + // with every new field taking its default. + let literal = r#"{"content":[{"type":"text","text":"hi"}],"is_error":false}"#; + let decoded: ToolResult = serde_json::from_str(literal).expect("deserializable"); + assert!(decoded.follow_up.is_empty()); + assert_eq!(decoded.metadata, None); + assert!(decoded.control.is_none()); + assert_eq!(decoded.error_kind, None); +} + +#[test] +fn default_control_round_trips_to_all_false_and_none() { + let control = ToolControl::default(); + assert_eq!(control.return_direct, None); + assert!(!control.terminate); + assert_eq!(control.goto, None); + assert_eq!(control.state_update, None); +} + +#[test] +fn a_control_created_by_another_builder_leaves_return_direct_unset() { + // A call that only used `with_goto`/`with_state_update`/`terminate` must + // not silently express an opinion on `return_direct`: a harness falls + // back to the tool's static default only when this stays `None`. + let r = ToolResult::success("done") + .with_goto("next") + .with_state_update(json!({"count": 1})) + .terminate(); + let control = r.control.as_ref().expect("control set"); + assert_eq!(control.return_direct, None); +} + +#[test] +fn dont_return_direct_forces_false_regardless_of_a_static_default() { + let r = ToolResult::success("done").dont_return_direct(); + let control = r.control.as_ref().expect("control set"); + assert_eq!(control.return_direct, Some(false)); +} + #[test] fn output_for_llm_prefers_markdown_when_requested() { let r = ToolResult::success_with_markdown(json!({"items": [{"id": 1}, {"id": 2}]}), "- 1\n- 2"); diff --git a/crates/tinytools/src/result/types.rs b/crates/tinytools/src/result/types.rs index e6cca0e..13518b5 100644 --- a/crates/tinytools/src/result/types.rs +++ b/crates/tinytools/src/result/types.rs @@ -20,7 +20,7 @@ use serde::{Deserialize, Serialize}; /// [`Self::is_error`] is a *reported* failure — the tool ran and said no — and /// is distinct from the `Err` arm of [`Tool::execute`][crate::Tool::execute], /// which means the tool could not run at all. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, Serialize, Deserialize)] pub struct ToolResult { /// List of content blocks returned by the tool. pub content: Vec, @@ -41,6 +41,31 @@ pub struct ToolResult { skip_serializing_if = "Option::is_none" )] pub markdown_formatted: Option, + /// Content the caller should present to the model as a *separate* user + /// message after the tool result, rather than folding it into the result + /// itself — a screenshot a vision-capable model should look at, a document + /// a follow-up turn should read. + /// + /// This is deliberately not part of [`Self::content`]: the tool-result + /// message answers the call, while follow-up content is context handed to + /// the model *afterwards*. [`Self::text`], [`Self::output`] and + /// [`Self::output_for_llm`] never include it — a host that wants to honour + /// it reads this field directly and decides how to place it on the wire. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub follow_up: Vec, + /// Host-only metadata: never shown to the model, but available to the host + /// for events, persistence, or telemetry. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub metadata: Option, + /// Loop-control hints a harness may honour, such as ending the loop + /// immediately or steering a graph. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub control: Option, + /// Distinguishes a reported failure the model should retry from one it + /// should not. `None` (the historical shape) means the caller has not + /// classified the failure either way. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub error_kind: Option, } impl ToolResult { @@ -48,8 +73,7 @@ impl ToolResult { pub fn success(text: impl Into) -> Self { Self { content: vec![ToolContent::Text { text: text.into() }], - is_error: false, - markdown_formatted: None, + ..Self::default() } } @@ -63,17 +87,39 @@ impl ToolResult { text: message.into(), }], is_error: true, - markdown_formatted: None, + ..Self::default() } } + /// A reported failure the model should be told to retry, distinct from a + /// permanent one — Pydantic AI's `ModelRetry`. + /// + /// Sets [`Self::is_error`] and tags [`Self::error_kind`] as + /// [`ToolErrorKind::Retry`]; a harness that reads the tag can choose to + /// coach the model to try again rather than giving up. + pub fn retry(message: impl Into) -> Self { + let mut result = Self::error(message); + result.error_kind = Some(ToolErrorKind::Retry); + result + } + + /// A reported failure that must not be retried — permanent, distinct from + /// [`Self::retry`]. + /// + /// Sets [`Self::is_error`] and tags [`Self::error_kind`] as + /// [`ToolErrorKind::Failed`]. + pub fn failed(message: impl Into) -> Self { + let mut result = Self::error(message); + result.error_kind = Some(ToolErrorKind::Failed); + result + } + /// A successful result carrying a single JSON block. #[must_use] pub fn json(data: serde_json::Value) -> Self { Self { content: vec![ToolContent::Json { data }], - is_error: false, - markdown_formatted: None, + ..Self::default() } } @@ -83,8 +129,8 @@ impl ToolResult { pub fn success_with_markdown(data: serde_json::Value, markdown: impl Into) -> Self { Self { content: vec![ToolContent::Json { data }], - is_error: false, markdown_formatted: Some(markdown.into()), + ..Self::default() } } @@ -95,6 +141,84 @@ impl ToolResult { self } + /// Appends content the caller should present to the model as a separate, + /// follow-up message. See [`Self::follow_up`]. + #[must_use] + pub fn with_follow_up(mut self, content: impl IntoIterator) -> Self { + self.follow_up.extend(content); + self + } + + /// Appends an image block to [`Self::content`]. + #[must_use] + pub fn with_image(mut self, media_type: impl Into, data: ImageData) -> Self { + self.content.push(ToolContent::Image { + media_type: media_type.into(), + data, + }); + self + } + + /// Attaches (or replaces) host-only metadata never shown to the model. + #[must_use] + pub fn with_metadata(mut self, metadata: serde_json::Value) -> Self { + self.metadata = Some(metadata); + self + } + + /// Marks the result as one the harness should return directly to the + /// caller without further model interaction, overriding the tool's + /// static [`Tool::return_direct`][crate::Tool::return_direct] default for + /// this call. + #[must_use] + pub fn return_direct(mut self) -> Self { + self.control_mut().return_direct = Some(true); + self + } + + /// Marks the result as one that should *not* be returned directly, even + /// if the tool declares a static `true` + /// [`Tool::return_direct`][crate::Tool::return_direct] default. + /// + /// Use this to explicitly disable the static default for a single call. + /// Combining another control builder such as [`Self::with_goto`] or + /// [`Self::with_state_update`] with this call leaves the per-call + /// override unset (`None`), which is not the same as calling this + /// method: an unset override falls back to the tool's static default, + /// while this method forces `false` regardless of that default. + #[must_use] + pub fn dont_return_direct(mut self) -> Self { + self.control_mut().return_direct = Some(false); + self + } + + /// Marks the result as one that should end the agent loop. + #[must_use] + pub fn terminate(mut self) -> Self { + self.control_mut().terminate = true; + self + } + + /// Requests that the harness route to a named node or step next. + #[must_use] + pub fn with_goto(mut self, node: impl Into) -> Self { + self.control_mut().goto = Some(node.into()); + self + } + + /// Attaches a state update the harness may fold into its graph or session + /// state. + #[must_use] + pub fn with_state_update(mut self, update: serde_json::Value) -> Self { + self.control_mut().state_update = Some(update); + self + } + + /// Returns the [`ToolControl`], creating a default one if absent. + fn control_mut(&mut self) -> &mut ToolControl { + self.control.get_or_insert_with(ToolControl::default) + } + /// The markdown rendering when present and non-blank, otherwise /// [`Self::output`]. /// @@ -112,37 +236,53 @@ impl ToolResult { self.output() } - /// The text blocks alone, newline-joined. JSON blocks are skipped. + /// The text blocks alone, newline-joined, with a short placeholder in + /// place of non-text blocks other than JSON, which is skipped entirely. #[must_use] pub fn text(&self) -> String { self.content .iter() - .filter_map(|c| match c { - ToolContent::Text { text } => Some(text.as_str()), - ToolContent::Json { .. } => None, - }) + .filter_map(ToolContent::text_or_placeholder) .collect::>() .join("\n") } /// Every block rendered and newline-joined, with JSON blocks - /// pretty-printed. This is what a model sees when no markdown rendering is + /// pretty-printed and other non-text blocks rendered as a short + /// placeholder. This is what a model sees when no markdown rendering is /// preferred. #[must_use] pub fn output(&self) -> String { self.content .iter() - .map(|c| match c { - ToolContent::Text { text } => text.clone(), - ToolContent::Json { data } => { - serde_json::to_string_pretty(data).unwrap_or_default() - } - }) + .map(ToolContent::render) .collect::>() .join("\n") } } +/// How image bytes are referenced in a [`ToolContent::Image`] block. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(tag = "kind", content = "value", rename_all = "lowercase")] +pub enum ImageData { + /// Base64-encoded image bytes, inline. + Base64(String), + /// A URL the host may fetch the image from. + Url(String), +} + +/// How file bytes are referenced in a [`ToolContent::File`] block. +#[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(tag = "kind", content = "value", rename_all = "lowercase")] +pub enum FileData { + /// Base64-encoded file bytes, inline. + Base64(String), + /// A URL the host may fetch the file from. + Url(String), + /// A path on a filesystem the host and tool both have access to. + Path(String), +} + /// A single content block within a [`ToolResult`]. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(tag = "type", rename_all = "lowercase")] @@ -157,4 +297,92 @@ pub enum ToolContent { /// The JSON body. data: serde_json::Value, }, + /// Image bytes or a reference to them. + Image { + /// The image's MIME type, e.g. `image/png`. + media_type: String, + /// The image bytes or reference. + data: ImageData, + }, + /// File bytes or a reference to them. + File { + /// The file's display name. + name: String, + /// The file's MIME type. + media_type: String, + /// The file bytes or reference. + data: FileData, + }, +} + +impl ToolContent { + /// Renders this block as a model would see it: text verbatim, JSON + /// pretty-printed, and a short placeholder for an image or file. + #[must_use] + pub fn render(&self) -> String { + match self { + Self::Text { text } => text.clone(), + Self::Json { data } => serde_json::to_string_pretty(data).unwrap_or_default(), + Self::Image { media_type, .. } => format!("[image {media_type}]"), + Self::File { + name, media_type, .. + } => format!("[file {name} ({media_type})]"), + } + } + + /// Like [`Self::render`], but returns `None` for a JSON block so + /// [`ToolResult::text`] can skip it entirely rather than rendering it. + #[must_use] + fn text_or_placeholder(&self) -> Option { + match self { + Self::Json { .. } => None, + other => Some(other.render()), + } + } +} + +/// Distinguishes a reported tool failure the model should retry from one it +/// should not. +/// +/// Modelled on Pydantic AI's `ModelRetry` versus a permanent tool failure: both +/// set [`ToolResult::is_error`], but a harness that reads this tag can decide +/// whether to loop the model back in or surface the failure as final. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ToolErrorKind { + /// The failure is transient or correctable; ask the model to try again. + Retry, + /// The failure is permanent; do not retry. + Failed, +} + +/// Loop-control hints a harness may honour after a tool call. +/// +/// These are hints, not enforcement — same as [`crate::ToolPolicy`], a harness +/// decides whether and how to act on them. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +pub struct ToolControl { + /// Per-call override for returning this result directly to the caller + /// without further model interaction. + /// + /// Tri-state, not a defaulted `bool`: `None` means this call did not + /// express an opinion, so a harness should fall back to the tool's + /// static [`Tool::return_direct`][crate::Tool::return_direct] default + /// rather than treating an absent override as an explicit `false`. A + /// call that only used [`ToolResult::with_goto`], + /// [`ToolResult::with_state_update`], or [`ToolResult::terminate`] — none + /// of which touch this field — must not silently suppress a tool's + /// static `true` declaration. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub return_direct: Option, + /// End the agent loop after this call. + #[serde(default)] + pub terminate: bool, + /// Route to a named node or step next, for a harness with a graph or + /// state machine underneath it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub goto: Option, + /// A state update the harness may fold into its graph or session state. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub state_update: Option, } diff --git a/crates/tinytools/src/tool/test.rs b/crates/tinytools/src/tool/test.rs index 54cc924..37fe452 100644 --- a/crates/tinytools/src/tool/test.rs +++ b/crates/tinytools/src/tool/test.rs @@ -96,6 +96,7 @@ fn the_declaration_defaults_are_the_conservative_answer() { assert!(!tool.is_concurrency_safe(&Value::Null)); assert!(!tool.external_effect()); assert!(!tool.external_effect_with_args(&Value::Null)); + assert!(!tool.return_direct()); assert!(tool.max_result_size_chars().is_none()); assert_eq!(tool.timeout_policy(&Value::Null), ToolTimeout::Inherit); assert!(tool.host_extension().is_none()); @@ -326,3 +327,39 @@ fn overridden_declarations_are_visible_through_a_trait_object() { assert!(erased.external_effect()); assert_eq!(erased.timeout_policy(&Value::Null), ToolTimeout::Unbounded); } + +/// A tool whose entire job is to hand the model's answer straight back, so it +/// overrides the static [`Tool::return_direct`] default. +struct FinalAnswerTool; + +#[async_trait] +impl Tool for FinalAnswerTool { + fn name(&self) -> &str { + "final_answer" + } + + fn description(&self) -> &str { + "Ends the loop with the model's answer" + } + + fn parameters_schema(&self) -> Value { + json!({ "type": "object" }) + } + + async fn execute(&self, _args: Value) -> anyhow::Result { + Ok(ToolResult::success("done")) + } + + fn return_direct(&self) -> bool { + true + } +} + +#[test] +fn a_tool_can_declare_a_static_return_direct_default() { + let tool = FinalAnswerTool; + assert!(tool.return_direct()); + + let erased: &dyn Tool = &FinalAnswerTool; + assert!(erased.return_direct()); +} diff --git a/crates/tinytools/src/tool/types.rs b/crates/tinytools/src/tool/types.rs index 91f0702..9185fc8 100644 --- a/crates/tinytools/src/tool/types.rs +++ b/crates/tinytools/src/tool/types.rs @@ -303,4 +303,19 @@ pub trait Tool: Send + Sync { .detail .or_else(|| context_detail_from_args(args)) } + + /// Whether every successful call to this tool should be returned directly + /// to the caller without further model interaction, as a static, + /// per-tool default. + /// + /// This is the *tool's* declared default — a per-call override lives on + /// [`ToolResult::control`][crate::ToolResult::control] via + /// [`ToolResult::return_direct`][crate::ToolResult::return_direct], which + /// a harness should prefer when a result sets it. Most tools return + /// `false`; a tool whose entire purpose is to hand the model's answer + /// straight back — a final-answer or handoff tool — overrides this to + /// `true` so a harness need not special-case it by name. + fn return_direct(&self) -> bool { + false + } } diff --git a/docs/plans/tinytools-vocabulary.md b/docs/plans/tinytools-vocabulary.md index 9d9c92e..77fd4d3 100644 --- a/docs/plans/tinytools-vocabulary.md +++ b/docs/plans/tinytools-vocabulary.md @@ -85,7 +85,47 @@ sequence for the next module added to this crate. remaining template-only instructions (a deleted example, a nonexistent error-type variant) rather than leaving them to bit-rot. -## Task 6: Full verification +## Task 7: Rich `ToolResult`, replay classification, and a host escape hatch + +**Files:** `crates/tinytools/src/result/*`, `crates/tinytools/src/policy/*`, +`crates/tinytools/src/context/*`, `crates/tinytools/README.md`, +`docs/specs/tinytools-vocabulary.md` + +This task documents the sequence actually followed (post-hoc, added while +addressing review feedback on the pull request that landed it), for the +extension specified in +[`../specs/tinytools-vocabulary.md`](../specs/tinytools-vocabulary.md#extension-rich-results-replay-classification-and-a-host-escape-hatch). + +1. Add `ToolContent::Image` / `ToolContent::File` and their `ImageData` / + `FileData` payload types, each literal-wire tested for both directions. +2. Add `ToolResult::follow_up`, `::metadata`, `::control: Option`, + and `::error_kind: Option`, each `#[serde(default)]` and, for + the ones that can be empty or absent, `skip_serializing_if`, so a + `ToolResult` persisted before these fields existed still decodes and a + plain result's wire shape is unchanged. +3. Add `ToolControl` (`return_direct`, `terminate`, `goto`, `state_update`) + and the builders `return_direct()`, `terminate()`, `with_goto(..)`, + `with_state_update(..)` that lazily create it. +4. Add `ToolReplay` and `ToolRuntime::replay`, defaulting to `Never`. +5. Add `ToolRunContext::host_extension`, defaulting to `None`. +6. Bump `[workspace.package].version` from `0.2.0` to `0.3.0`: the new fields + on public structs (`ToolResult`, `ToolRuntime`) are additive on the wire + but break an external struct literal that does not use `..Default::default()` + or `..Self::default()`, which `AGENTS.md`'s versioning policy treats as a + non-additive, minor-bump-worthy change pre-1.0. +7. Fix `ToolControl::return_direct` to `Option` (review finding, both + CodeRabbit and Codex): a call that only used `with_goto`, + `with_state_update`, or `terminate` created a `ToolControl` whose + `return_direct` defaulted to `false`, so a harness following the + documented "prefer the per-call value" rule would silently suppress a + tool's static `true` declaration even though the call never touched + `return_direct`. Add `dont_return_direct()` as the explicit `Some(false)` + builder, and add regression tests asserting the field stays `None` when no + builder touches it. +8. Update `crates/tinytools/README.md`'s "Static and per-call return-direct" + section and field list to describe the tri-state semantics. + +## Task 8: Full verification All items below were run and passed locally as of this commit, and CI re-verifies the same commands on every push: diff --git a/docs/specs/tinytools-vocabulary.md b/docs/specs/tinytools-vocabulary.md index b1bba1b..a45dd00 100644 --- a/docs/specs/tinytools-vocabulary.md +++ b/docs/specs/tinytools-vocabulary.md @@ -140,6 +140,56 @@ section on the trait itself in `crates/tinytools/src/tool/types.rs`, and `crates/tinytools/Cargo.toml`) and this specification stay aligned with the public surface as it evolves. +## Extension: rich results, replay classification, and a host escape hatch + +Landed after the crate's initial acceptance (tracked in +[`../plans/tinytools-vocabulary.md`](../plans/tinytools-vocabulary.md#task-7-rich-toolresult-replay-classification-and-a-host-escape-hatch)), +this section specifies the additions below. All are additive to the wire shape +(new fields default and are omitted when absent) but not source-compatible — +see "Versioning" — so they shipped as a `0.2.0` → `0.3.0` minor bump. + +- **`ToolContent::Image` / `ToolContent::File`** extend the block-list result + with image and file blocks (`ImageData` / `FileData`, each `Base64`, `Url`, + or `Path`), alongside the existing `Text` and `Json` blocks. +- **`ToolResult::follow_up: Vec`** carries content a caller should + present to the model as a *separate* message after the tool result — a + screenshot or document the next turn should read — rather than folding it + into the result itself. It is deliberately excluded from `text()`, + `output()`, and `output_for_llm()`; a host that wants to honour it reads the + field directly. +- **`ToolResult::metadata: Option`** is host-only data + (trace ids, raw provider payloads) never shown to the model. +- **`ToolResult::control: Option`** carries loop-control hints a + harness may honour: `terminate`, `goto: Option`, + `state_update: Option`, and `return_direct: + Option`. `return_direct` is tri-state, not a defaulted `bool`: `None` + means this call did not express an opinion and a harness falls back to the + tool's static `Tool::return_direct` default; `Some(true)` / + `Some(false)` are explicit per-call overrides. This tri-state is load + bearing — a call that only used `with_goto`, `with_state_update`, or + `terminate` must not be read as silently disabling a tool's static `true` + declaration. See "Static and per-call return-direct" in + `crates/tinytools/README.md`. +- **`ToolResult::error_kind: Option`** distinguishes a reported + failure the model should retry (`Retry`, set by `ToolResult::retry`) from + one it should not (`Failed`, set by `ToolResult::failed`) — modelled on + Pydantic AI's `ModelRetry` versus a permanent failure. `None` (the + historical shape) means the caller did not classify the failure. +- **`ToolRuntime::replay: ToolReplay`** classifies whether an orphaned + in-flight call for a tool may be safely re-executed after a crash, + mirroring pi's `replay` classification. It defaults to `ToolReplay::Never`; + an idempotent tool declares `ToolReplay::Safe` through its `ToolPolicy`. + This lives on the existing declarative policy surface rather than a new + `Tool` trait method, consistent with how every other runtime requirement + (timeout, retries, cancellation, sandboxing) is already expressed there. +- **`ToolRunContext::host_extension`** is the same escape hatch as + `Tool::host_extension`: this crate has no business naming the harness's + context type, so a host implementor returns `Some(self)` as + `&(dyn Any + Send + Sync)` and a tool that knows which host it runs under + downcasts. Every other implementor returns `None` and pays nothing. A tool + that only needs the workspace, thread id, or output cap keeps using the + typed methods. + ## Open questions - Whether `ToolResult`/`ToolContent` should ever adopt an actual MCP