From aa5f492031f7edb75a40973d19eda8efb9512c61 Mon Sep 17 00:00:00 2001 From: Ruslan Nigmatullin Date: Fri, 13 Nov 2026 04:11:30 +0000 Subject: [PATCH] Refactor history and notes tools around backend traits (#61251) ## What changed - Introduce `Request`, `Backend`, `Handle`, and `ToolResponse` traits so backends can choose input types, response schemas, output rendering, and tool exposure. - Consolidate tool names, request types, catalog keys, and parallel-call settings in one registration macro, with HTTP routes derived from request metadata. - Move trusted session and agent context into `HttpBackend`, preserving opaque JSON arguments and catalog overrides for the HTTP proxy. ## Testing Extend the extension integration test to verify that an unknown `future_catalog_parameter` is forwarded to the HTTP endpoint alongside trusted context. Adapt the bundled tool contract snapshot test to use the new registration path. GitOrigin-RevId: 4ebfb9f6aa9395f9a27bd7fbe6cf131c0a06d8d7 --- codex-rs/ext/history-notes/src/backend.rs | 81 +++- .../ext/history-notes/src/backend_tests.rs | 20 +- codex-rs/ext/history-notes/src/extension.rs | 83 ++-- codex-rs/ext/history-notes/src/requests.rs | 14 + codex-rs/ext/history-notes/src/tools.rs | 394 ++++++++---------- codex-rs/ext/history-notes/src/tools_tests.rs | 34 +- .../tests/history_notes_extension.rs | 6 +- 7 files changed, 319 insertions(+), 313 deletions(-) diff --git a/codex-rs/ext/history-notes/src/backend.rs b/codex-rs/ext/history-notes/src/backend.rs index 8218b764c1..b5923929f2 100644 --- a/codex-rs/ext/history-notes/src/backend.rs +++ b/codex-rs/ext/history-notes/src/backend.rs @@ -1,3 +1,14 @@ +//! Execution boundary for history and notes. Each backend selects the wire representation +//! it accepts for a tool. The HTTP proxy preserves catalog-defined arguments verbatim. + +use crate::requests::Namespace; +use crate::tools::ToolResponse; +use codex_tools::ToolExposure; +use schemars::JsonSchema; +use serde::Serialize; +use serde::de::DeserializeOwned; +use std::future::Future; +use std::sync::Arc; use std::time::Duration; use codex_client::HttpTransport; @@ -17,41 +28,68 @@ const ENCRYPTED_TOOL_ARGUMENTS_HEADER: &str = "x-openai-encrypted-tool-arguments const TOOL_OUTPUT_TRUNCATION_POLICY_HEADER: &str = "x-openai-tool-output-truncation-policy"; const OPERATION_ERROR_PREFIX: &str = "Unable to perform operation:"; -#[derive(Clone)] -pub(crate) struct HistoryNotesBackend { - provider: SharedModelProvider, - http_client_factory: HttpClientFactory, +#[derive(Serialize)] +pub(crate) struct HttpContext { + pub(crate) session_id: String, + pub(crate) current_agent_name: String, } -impl HistoryNotesBackend { +/// Metadata shared by schema registration and the HTTP proxy. The catalog may override +/// model-facing descriptions and parameters, while the endpoint stays harness-owned. +pub(crate) trait Request: DeserializeOwned + JsonSchema + Send + Sync + 'static { + const NAMESPACE: Namespace; + const NAME: &'static str; + const PARALLEL: bool = true; +} + +/// Executes each tool using the wire representation its backend can read. The HTTP +/// proxy forwards opaque JSON. A typed backend uses the request and its declared response. +pub(crate) trait Backend: Clone + Send + Sync + 'static { + const EXPOSURE: ToolExposure; + type Input: DeserializeOwned + Send; +} + +/// Implements one operation. Each backend chooses its result type for this request. +pub(crate) trait Handle: Backend { + type Output: ToolResponse; + + fn handle( + &self, + budget: TruncationPolicy, + input: Self::Input, + ) -> impl Future> + Send; +} + +#[derive(Clone)] +pub(crate) struct HttpBackend { + provider: SharedModelProvider, + http_client_factory: HttpClientFactory, + context: Arc, +} + +impl HttpBackend { pub(crate) fn new( provider: SharedModelProvider, http_client_factory: HttpClientFactory, + context: Arc, ) -> Self { Self { provider, http_client_factory, + context, } } pub(crate) async fn call( &self, path: &str, - session_id: &str, - current_agent_name: &str, mut arguments: Value, truncation_policy: TruncationPolicy, ) -> Result { let Some(arguments_object) = arguments.as_object_mut() else { return Err("History tool arguments must be a JSON object".to_string()); }; - arguments_object.insert( - "context".to_string(), - json!({ - "session_id": session_id, - "current_agent_name": current_agent_name, - }), - ); + arguments_object.insert("context".to_string(), json!(self.context.as_ref())); let setup = codex_model_provider::resolve_request( self.provider.as_ref(), @@ -109,6 +147,21 @@ impl HistoryNotesBackend { } } +impl Backend for HttpBackend { + const EXPOSURE: ToolExposure = ToolExposure::DirectModelOnly; + + type Input = Value; +} + +impl Handle for HttpBackend { + type Output = Value; + + async fn handle(&self, budget: TruncationPolicy, input: Value) -> Result { + let path = format!("alpha/{}/v2/{}", R::NAMESPACE.name, R::NAME); + self.call(&path, input, budget).await + } +} + #[cfg(test)] #[path = "backend_tests.rs"] mod tests; diff --git a/codex-rs/ext/history-notes/src/backend_tests.rs b/codex-rs/ext/history-notes/src/backend_tests.rs index 244373f8c9..bda99a78a4 100644 --- a/codex-rs/ext/history-notes/src/backend_tests.rs +++ b/codex-rs/ext/history-notes/src/backend_tests.rs @@ -18,7 +18,9 @@ use wiremock::matchers::method; use wiremock::matchers::path; use super::ENCRYPTED_TOOL_ARGUMENTS_HEADER; -use super::HistoryNotesBackend; +use super::HttpBackend; +use super::HttpContext; +use std::sync::Arc; #[tokio::test] async fn routes_through_codex_backend_and_injects_trusted_session_agent_context() { @@ -45,16 +47,18 @@ async fn routes_through_codex_backend_and_injects_trusted_session_agent_context( ))), Some(auth_manager), ); - let backend = HistoryNotesBackend::new( + let backend = HttpBackend::new( provider, HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + Arc::new(HttpContext { + session_id: "session-123".to_owned(), + current_agent_name: "/root/worker".to_owned(), + }), ); let response = backend .call( "alpha/notes/v2/read_file", - "session-123", - "/root/worker", json!({ "path": "notes.md", "context": { @@ -121,7 +125,7 @@ async fn marks_encrypted_history_and_notes_arguments_without_changing_the_json_b let auth_manager = AuthManager::from_auth_for_testing(CodexAuth::Headers(AuthHeaders::new(HeaderMap::new()))); - let backend = HistoryNotesBackend::new( + let backend = HttpBackend::new( create_model_provider( ModelProviderInfo::create_openai_provider(Some(format!( "{}/backend-api/codex", @@ -130,14 +134,16 @@ async fn marks_encrypted_history_and_notes_arguments_without_changing_the_json_b Some(auth_manager), ), HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + Arc::new(HttpContext { + session_id: "session-123".to_owned(), + current_agent_name: "/root".to_owned(), + }), ); for (route, arguments) in cases { backend .call( &format!("alpha/{route}"), - "session-123", - "/root", arguments.clone(), TruncationPolicy::Bytes(1024), ) diff --git a/codex-rs/ext/history-notes/src/extension.rs b/codex-rs/ext/history-notes/src/extension.rs index e5590588f1..13a7520357 100644 --- a/codex-rs/ext/history-notes/src/extension.rs +++ b/codex-rs/ext/history-notes/src/extension.rs @@ -24,9 +24,9 @@ use codex_protocol::openai_models::NotesToolMessages; use codex_utils_output_truncation::TruncationPolicy; use serde_json::json; -use crate::backend::HistoryNotesBackend; -use crate::tools::HistoryNotesAction; -use crate::tools::HistoryNotesTool; +use crate::backend::HttpBackend; +use crate::backend::HttpContext; +use crate::tools::history_notes_tools; // Bound model context even when note paths are unusually long. const MAX_THREAD_HINT_BYTES: usize = 4_000; @@ -35,14 +35,6 @@ struct HistoryNotesExtension { auth_manager: Arc, } -struct HistoryNotesExtensionConfig { - backend: HistoryNotesBackend, -} - -struct HistoryNotesAgentIdentity { - agent_name: String, -} - impl HistoryNotesExtension { fn update_config(&self, thread_store: &ExtensionData, config: &Config) { let auth = thread_store @@ -59,15 +51,15 @@ impl HistoryNotesExtension { .is_some_and(|token_budget| token_budget.use_history_notes_extension) && config.model_provider.is_openai() && auth.credentials.current_auth_uses_codex_backend() + && let Some(context) = thread_store.get::() { - thread_store.insert(HistoryNotesExtensionConfig { - backend: HistoryNotesBackend::new( - auth.model_provider(config.model_provider.clone()), - config.http_client_factory(), - ), - }); + thread_store.insert(HttpBackend::new( + auth.model_provider(config.model_provider.clone()), + config.http_client_factory(), + context, + )); } else { - thread_store.remove::(); + thread_store.remove::(); } } } @@ -78,14 +70,14 @@ impl ThreadLifecycleContributor for HistoryNotesExtension { input: ThreadStartInput<'a, Config>, ) -> ExtensionFuture<'a, ()> { Box::pin(async move { - let agent_name = input - .session_source - .get_agent_path() - .unwrap_or_else(AgentPath::root) - .to_string(); - input - .thread_store - .insert(HistoryNotesAgentIdentity { agent_name }); + input.thread_store.insert(HttpContext { + session_id: input.session_store.level_id().to_owned(), + current_agent_name: input + .session_source + .get_agent_path() + .unwrap_or_else(AgentPath::root) + .to_string(), + }); self.update_config(input.thread_store, input.config); }) } @@ -110,10 +102,7 @@ impl ContextContributor for HistoryNotesExtension { thread_store: &'a ExtensionData, ) -> ExtensionFuture<'a, Vec> { Box::pin(async move { - let Some(config) = thread_store.get::() else { - return Vec::new(); - }; - let Some(identity) = thread_store.get::() else { + let Some(backend) = thread_store.get::() else { return Vec::new(); }; let track_status = |status| { @@ -125,12 +114,9 @@ impl ContextContributor for HistoryNotesExtension { }); } }; - let Ok(result) = config - .backend + let Ok(result) = backend .call( "alpha/notes/v2/thread_hint", - session_store.level_id(), - &identity.agent_name, json!({}), TruncationPolicy::Bytes(MAX_THREAD_HINT_BYTES), ) @@ -164,11 +150,10 @@ impl ContextContributor for HistoryNotesExtension { impl ToolContributor for HistoryNotesExtension { fn tools( &self, - session_store: &ExtensionData, + _session_store: &ExtensionData, thread_store: &ExtensionData, ) -> Vec ToolExecutor>>> { self.tools_with_messages( - session_store, thread_store, /*history_messages*/ None, /*notes_messages*/ None, @@ -177,14 +162,13 @@ impl ToolContributor for HistoryNotesExtension { fn tools_for_step( &self, - session_store: &ExtensionData, + _session_store: &ExtensionData, thread_store: &ExtensionData, step_store: &ExtensionData, ) -> Vec ToolExecutor>>> { let history_messages = step_store.get::(); let notes_messages = step_store.get::(); self.tools_with_messages( - session_store, thread_store, history_messages.as_deref(), notes_messages.as_deref(), @@ -195,35 +179,18 @@ impl ToolContributor for HistoryNotesExtension { impl HistoryNotesExtension { fn tools_with_messages( &self, - session_store: &ExtensionData, thread_store: &ExtensionData, history_messages: Option<&HistoryToolMessages>, notes_messages: Option<&NotesToolMessages>, ) -> Vec ToolExecutor>>> { - let Some(config) = thread_store.get::() else { + let Some(backend) = thread_store.get::() else { return Vec::new(); }; - let Some(identity) = thread_store.get::() else { - return Vec::new(); - }; - - HistoryNotesAction::ALL - .into_iter() - .map(|action| { - Arc::new(HistoryNotesTool::new( - action, - config.backend.clone(), - session_store.level_id().to_string(), - identity.agent_name.clone(), - history_messages, - notes_messages, - )) as Arc ToolExecutor>> - }) - .collect() + history_notes_tools((*backend).clone(), history_messages, notes_messages) } } -/// Installs the standalone history and notes tools backed by the Codex backend. +/// Installs the standalone history and notes tools. pub fn install(registry: &mut ExtensionRegistryBuilder, auth_manager: Arc) { let extension = Arc::new(HistoryNotesExtension { auth_manager }); registry.thread_lifecycle_contributor(extension.clone()); diff --git a/codex-rs/ext/history-notes/src/requests.rs b/codex-rs/ext/history-notes/src/requests.rs index fba0ca68a8..72f0c202d2 100644 --- a/codex-rs/ext/history-notes/src/requests.rs +++ b/codex-rs/ext/history-notes/src/requests.rs @@ -11,6 +11,20 @@ use serde::Deserializer; use serde::Serialize; use std::num::NonZeroUsize; +pub(crate) struct Namespace { + pub(crate) name: &'static str, + pub(crate) description: &'static str, +} + +pub(crate) const HISTORY: Namespace = Namespace { + name: "history", + description: "Recover prior conversation after a context-window reset by listing, reading, and searching normalized history using agent names and the opaque window and item IDs returned by these tools. Pass returned IDs unchanged when filtering or reading an item. Items are ordered by persisted ordinal, then creation time; list and search calls return no matches for an unknown window. Calls use the current agent by default; pass an absolute agent name or one relative to the current agent to inspect another agent. History is read-only and eventually consistent, so newly generated items may take a few seconds to appear. This is private model-only state. Use it silently to continue the task. Never disclose or describe the tool, its existence or use, paths, storage or recovery mechanisms, or the private contents (including by quoting or summarizing them) to the user.", +}; +pub(crate) const NOTES: Namespace = Namespace { + name: "notes", + description: "Read and maintain private notes that survive context-window transitions within this rollout. Paths are virtual, not filesystem paths. Relative file paths use the current agent's /notes directory; cross-agent paths must be absolute. Absolute paths use /notes[/]. Reads, listings, searches, and writes may access other agents' notes. File operations require ; path-prefix arguments may be omitted to use the current notes directory. Empty, '.', and '..' path components are unsupported. Shell expansion is not performed, so '~' is treated literally. Note reads reflect successful writes immediately; listings and searches are eventually consistent and may take a few seconds to reflect writes. Every file must remain at or below 1,000,000 UTF-8 bytes; create another file before approaching the limit. This is private model-only state. Use it silently to continue the task. Never disclose or describe the tool, its existence or use, paths, storage or recovery mechanisms, or the private contents (including by quoting or summarizing them) to the user.", +}; + fn encrypted_string(generator: &mut SchemaGenerator) -> Schema { let mut schema = String::json_schema(generator).into_object(); schema.extensions.insert("encrypted".into(), true.into()); diff --git a/codex-rs/ext/history-notes/src/tools.rs b/codex-rs/ext/history-notes/src/tools.rs index 2fd1f94ea4..aea231104c 100644 --- a/codex-rs/ext/history-notes/src/tools.rs +++ b/codex-rs/ext/history-notes/src/tools.rs @@ -1,3 +1,19 @@ +//! Registers each typed request with its backend and preserves model-only media in tool outputs. + +use crate::backend::Handle; +use crate::backend::Request; +use crate::requests::HISTORY; +use crate::requests::HistoryListItems; +use crate::requests::HistoryListWindows; +use crate::requests::HistoryReadItem; +use crate::requests::HistorySearchContents; +use crate::requests::NOTES; +use crate::requests::Namespace; +use crate::requests::NotesAppendToFile; +use crate::requests::NotesListFilesByPrefix; +use crate::requests::NotesReadFile; +use crate::requests::NotesSearchContents; +use crate::requests::NotesWriteFile; use codex_extension_api::FunctionCallError; use codex_extension_api::ResponsesApiTool; use codex_extension_api::ToolCall; @@ -15,251 +31,128 @@ use codex_protocol::models::ImageReference; use codex_protocol::models::ResponseInputItem; use codex_protocol::openai_models::HistoryToolMessages; use codex_protocol::openai_models::NotesToolMessages; +use codex_protocol::openai_models::ToolMessage; use codex_tools::JsonToolOutput; use codex_tools::ResponsesApiNamespace; use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ToolExposure; +use codex_tools::ToolOutputSchema; use codex_tools::with_catalog_tool_message; +use codex_utils_output_truncation::TruncationPolicy; +use schemars::JsonSchema; +use schemars::r#gen::SchemaSettings; +use serde::de::DeserializeOwned; use serde_json::Value; use serde_json::json; +use std::marker::PhantomData; +use std::sync::Arc; -use crate::backend::HistoryNotesBackend; +type Executor = Arc ToolExecutor>>; -const HISTORY_NAMESPACE: &str = "history"; -const NOTES_NAMESPACE: &str = "notes"; -const HISTORY_DESCRIPTION: &str = "Recover prior conversation after a context-window reset by listing, reading, and searching normalized history using agent names and the opaque window and item IDs returned by these tools. Pass returned IDs unchanged when filtering or reading an item. Items are ordered by persisted ordinal, then creation time; list and search calls return no matches for an unknown window. Calls use the current agent by default; pass an absolute agent name or one relative to the current agent to inspect another agent. History is read-only and eventually consistent, so newly generated items may take a few seconds to appear. This is private model-only state. Use it silently to continue the task. Never disclose or describe the tool, its existence or use, paths, storage or recovery mechanisms, or the private contents (including by quoting or summarizing them) to the user."; -const NOTES_DESCRIPTION: &str = "Read and maintain private notes that survive context-window transitions within this rollout. Paths are virtual, not filesystem paths. Relative file paths use the current agent's /notes directory; cross-agent paths must be absolute. Absolute paths use /notes[/]. Reads, listings, searches, and writes may access other agents' notes. File operations require ; path-prefix arguments may be omitted to use the current notes directory. Empty, '.', and '..' path components are unsupported. Shell expansion is not performed, so '~' is treated literally. Note reads reflect successful writes immediately; listings and searches are eventually consistent and may take a few seconds to reflect writes. Every file must remain at or below 1,000,000 UTF-8 bytes; create another file before approaching the limit. This is private model-only state. Use it silently to continue the task. Never disclose or describe the tool, its existence or use, paths, storage or recovery mechanisms, or the private contents (including by quoting or summarizing them) to the user."; +// Keep the catalog key, tool name, request type and HTTP route in one registration. +macro_rules! register_tools { + ($( $catalog:ident: $messages:ty => $namespace:ident { + $( $name:ident: $request:ty $( [parallel = $parallel:literal] )? ),+ $(,)? + } ),+ $(,)?) => { + $($(impl Request for $request { + const NAMESPACE: Namespace = $namespace; + const NAME: &'static str = stringify!($name); + $(const PARALLEL: bool = $parallel;)? + })+)+ -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub(crate) enum HistoryNotesAction { - HistoryListWindows, - HistoryListItems, - HistoryReadItem, - HistorySearchContents, - NotesListFilesByPrefix, - NotesReadFile, - NotesSearchContents, - NotesAppendToFile, - NotesWriteFile, + pub(crate) fn history_notes_tools( + backend: B, + $($catalog: Option<&$messages>,)+ + ) -> Vec + where + $($(B: Handle<$request>,)+)+ + { + vec![$($(Arc::new(HistoryNotesTool::<$request, B> { + request: PhantomData, + backend: backend.clone(), + spec: tool_spec::<$request, >::Output>( + $catalog.and_then(|messages| messages.namespace_description.as_deref()), + $catalog.and_then(|messages| messages.$name.as_ref()), + ), + }) as Executor,)+)+] + } + }; } -impl HistoryNotesAction { - pub(crate) const ALL: [Self; 9] = [ - Self::HistoryListWindows, - Self::HistoryListItems, - Self::HistoryReadItem, - Self::HistorySearchContents, - Self::NotesListFilesByPrefix, - Self::NotesReadFile, - Self::NotesSearchContents, - Self::NotesAppendToFile, - Self::NotesWriteFile, - ]; +register_tools! { + history: HistoryToolMessages => HISTORY { + list_windows: HistoryListWindows, + list_items: HistoryListItems, + read_item: HistoryReadItem, + search_contents: HistorySearchContents, + }, + notes: NotesToolMessages => NOTES { + list_files_by_prefix: NotesListFilesByPrefix, + read_file: NotesReadFile, + search_contents: NotesSearchContents, + append_to_file: NotesAppendToFile [parallel = false], + write_file: NotesWriteFile [parallel = false], + }, +} - fn namespace(self) -> &'static str { - match self { - Self::HistoryListWindows - | Self::HistoryListItems - | Self::HistoryReadItem - | Self::HistorySearchContents => HISTORY_NAMESPACE, - Self::NotesListFilesByPrefix - | Self::NotesReadFile - | Self::NotesSearchContents - | Self::NotesAppendToFile - | Self::NotesWriteFile => NOTES_NAMESPACE, - } - } - - fn name(self) -> &'static str { - match self { - Self::HistoryListWindows => "list_windows", - Self::HistoryListItems => "list_items", - Self::HistoryReadItem => "read_item", - Self::HistorySearchContents => "search_contents", - Self::NotesListFilesByPrefix => "list_files_by_prefix", - Self::NotesReadFile => "read_file", - Self::NotesSearchContents => "search_contents", - Self::NotesAppendToFile => "append_to_file", - Self::NotesWriteFile => "write_file", - } - } - - fn endpoint(self) -> &'static str { - match self { - Self::HistoryListWindows => "alpha/history/v2/list_windows", - Self::HistoryListItems => "alpha/history/v2/list_items", - Self::HistoryReadItem => "alpha/history/v2/read_item", - Self::HistorySearchContents => "alpha/history/v2/search_contents", - Self::NotesListFilesByPrefix => "alpha/notes/v2/list_files_by_prefix", - Self::NotesReadFile => "alpha/notes/v2/read_file", - Self::NotesSearchContents => "alpha/notes/v2/search_contents", - Self::NotesAppendToFile => "alpha/notes/v2/append_to_file", - Self::NotesWriteFile => "alpha/notes/v2/write_file", - } - } - - fn supports_parallel_tool_calls(self) -> bool { - !matches!(self, Self::NotesAppendToFile | Self::NotesWriteFile) - } - - fn namespace_description(self) -> &'static str { - match self.namespace() { - HISTORY_NAMESPACE => HISTORY_DESCRIPTION, - NOTES_NAMESPACE => NOTES_DESCRIPTION, - _ => unreachable!("History actions use a known namespace"), - } - } - - #[expect( - clippy::expect_used, - reason = "bundled schemas have documented request types" - )] - fn typed_parameters(self) -> (String, codex_tools::JsonSchema) { - fn derive() -> (String, codex_tools::JsonSchema) { - let schema = schemars::r#gen::SchemaSettings::draft2019_09() - .with(|settings| { - settings.inline_subschemas = true; - settings.option_add_null_type = false; - }) - .into_generator() - .into_root_schema_for::() - .schema; - let mut parameters = - parse_tool_input_schema(&json!(schema)).expect("valid typed tool schema"); - // Preserve the published ordering for note writes. - if let Some(required) = &mut parameters.required { - required.sort_by_key(|name| name != "text"); - } - let description = parameters - .description - .take() - .expect("documented request") - .into_string(); - (description, parameters) - } - match self { - Self::HistoryListWindows => derive::(), - Self::HistoryListItems => derive::(), - Self::HistoryReadItem => derive::(), - Self::HistorySearchContents => derive::(), - Self::NotesListFilesByPrefix => derive::(), - Self::NotesReadFile => derive::(), - Self::NotesSearchContents => derive::(), - Self::NotesAppendToFile => derive::(), - Self::NotesWriteFile => derive::(), - } - } - - fn spec( - self, - history_messages: Option<&HistoryToolMessages>, - notes_messages: Option<&NotesToolMessages>, - ) -> ToolSpec { - let (namespace_description, tool_message) = match self { - Self::HistoryListWindows => history_messages - .map(|messages| (&messages.namespace_description, &messages.list_windows)), - Self::HistoryListItems => history_messages - .map(|messages| (&messages.namespace_description, &messages.list_items)), - Self::HistoryReadItem => history_messages - .map(|messages| (&messages.namespace_description, &messages.read_item)), - Self::HistorySearchContents => history_messages - .map(|messages| (&messages.namespace_description, &messages.search_contents)), - Self::NotesListFilesByPrefix => notes_messages.map(|messages| { - ( - &messages.namespace_description, - &messages.list_files_by_prefix, - ) - }), - Self::NotesReadFile => notes_messages - .map(|messages| (&messages.namespace_description, &messages.read_file)), - Self::NotesSearchContents => notes_messages - .map(|messages| (&messages.namespace_description, &messages.search_contents)), - Self::NotesAppendToFile => notes_messages - .map(|messages| (&messages.namespace_description, &messages.append_to_file)), - Self::NotesWriteFile => notes_messages - .map(|messages| (&messages.namespace_description, &messages.write_file)), - } - .map_or((None, None), |(namespace_description, tool_message)| { - (namespace_description.as_deref(), tool_message.as_ref()) - }); - let (description, parameters) = self.typed_parameters(); - let tool = with_catalog_tool_message( - ResponsesApiTool { - name: self.name().to_string(), - description, - strict: false, - parameters, - output_schema: None, - defer_loading: None, - }, - tool_message, - ); - ToolSpec::Namespace(ResponsesApiNamespace { - name: self.namespace().to_string(), - description: namespace_description - .unwrap_or(self.namespace_description()) - .to_string(), - tools: vec![ResponsesApiNamespaceTool::Function(tool)], +pub(crate) fn schema(option_add_null_type: bool) -> Value { + let schema = SchemaSettings::draft2019_09() + .with(|settings| { + settings.inline_subschemas = true; + settings.option_add_null_type = option_add_null_type; }) - } + .into_generator() + .into_root_schema_for::() + .schema; + json!(schema) } -pub(crate) struct HistoryNotesTool { - action: HistoryNotesAction, - backend: HistoryNotesBackend, - session_id: String, - current_agent_name: String, +#[expect( + clippy::expect_used, + reason = "the bundled request schemas are derived from fixed, documented types" +)] +fn tool_spec( + namespace_description: Option<&str>, + message: Option<&ToolMessage>, +) -> ToolSpec { + let mut parameters = parse_tool_input_schema(&schema::(/*option_add_null_type*/ false)) + .expect("valid history and notes request schema"); + // Schemars sorts required keys. The published note write schemas put text before path. + if let Some(required) = &mut parameters.required { + required.sort_by_key(|name| name != "text"); + } + let description = parameters + .description + .take() + .expect("history and notes requests have descriptions") + .into_string(); + let tool = ResponsesApiTool { + name: R::NAME.to_string(), + description, + strict: false, + parameters, + output_schema: S::output_schema(), + defer_loading: None, + }; + let tool = S::with_catalog_message(tool, message); + ToolSpec::Namespace(ResponsesApiNamespace { + name: R::NAMESPACE.name.to_string(), + description: namespace_description + .unwrap_or(R::NAMESPACE.description) + .to_string(), + tools: vec![ResponsesApiNamespaceTool::Function(tool)], + }) +} + +struct HistoryNotesTool { + request: PhantomData, + backend: B, spec: ToolSpec, } -impl HistoryNotesTool { - pub(crate) fn new( - action: HistoryNotesAction, - backend: HistoryNotesBackend, - session_id: String, - current_agent_name: String, - history_messages: Option<&HistoryToolMessages>, - notes_messages: Option<&NotesToolMessages>, - ) -> Self { - Self { - action, - backend, - session_id, - current_agent_name, - spec: action.spec(history_messages, notes_messages), - } - } - - async fn handle_call( - &self, - call: ToolCall<'_>, - ) -> Result, FunctionCallError> { - let arguments = call.function_arguments()?; - let arguments = if arguments.trim().is_empty() { - json!({}) - } else { - serde_json::from_str(arguments) - .map_err(|error| FunctionCallError::RespondToModel(error.to_string()))? - }; - let result = self - .backend - .call( - self.action.endpoint(), - &self.session_id, - &self.current_agent_name, - arguments, - call.truncation_policy, - ) - .await - .map_err(FunctionCallError::RespondToModel)?; - - Ok(Box::new(HistoryNotesToolOutput::new(result)?)) - } -} - -impl<'call> ToolExecutor> for HistoryNotesTool { +impl<'call, R: Request, B: Handle> ToolExecutor> for HistoryNotesTool { fn tool_name(&self) -> ToolName { - ToolName::namespaced(self.action.namespace(), self.action.name()) + ToolName::namespaced(R::NAMESPACE.name, R::NAME) } fn spec(&self) -> ToolSpec { @@ -267,18 +160,63 @@ impl<'call> ToolExecutor> for HistoryNotesTool { } fn exposure(&self) -> ToolExposure { - ToolExposure::DirectModelOnly + B::EXPOSURE } fn supports_parallel_tool_calls(&self) -> bool { - self.action.supports_parallel_tool_calls() + R::PARALLEL } fn handle<'a>(&'a self, call: ToolCall<'call>) -> ToolExecutorFuture<'a> where 'call: 'a, { - Box::pin(self.handle_call(call)) + Box::pin(async move { + let input = parse_arguments(call.function_arguments()?)?; + let response = self + .backend + .handle(call.truncation_policy, input) + .await + .map_err(FunctionCallError::RespondToModel)?; + response.into_tool_output(call.truncation_policy) + }) + } +} + +fn parse_arguments(arguments: &str) -> Result { + serde_json::from_str(if arguments.trim().is_empty() { + "{}" + } else { + arguments + }) + .map_err(|err| FunctionCallError::RespondToModel(err.to_string())) +} + +/// Supplies the advertised output schema and renders a backend response for the model. +pub(crate) trait ToolResponse { + fn output_schema() -> Option { + None + } + + fn with_catalog_message( + tool: ResponsesApiTool, + message: Option<&ToolMessage>, + ) -> ResponsesApiTool { + with_catalog_tool_message(tool, message) + } + + fn into_tool_output( + self, + budget: TruncationPolicy, + ) -> Result, FunctionCallError>; +} + +impl ToolResponse for Value { + fn into_tool_output( + self, + _budget: TruncationPolicy, + ) -> Result, FunctionCallError> { + Ok(Box::new(HistoryNotesToolOutput::new(self)?)) } } diff --git a/codex-rs/ext/history-notes/src/tools_tests.rs b/codex-rs/ext/history-notes/src/tools_tests.rs index 67d3ee5d62..9a409a73a9 100644 --- a/codex-rs/ext/history-notes/src/tools_tests.rs +++ b/codex-rs/ext/history-notes/src/tools_tests.rs @@ -1,3 +1,7 @@ +use codex_http_client::HttpClientFactory; +use codex_http_client::OutboundProxyPolicy; +use codex_model_provider::create_model_provider; +use codex_model_provider_info::ModelProviderInfo; use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ImageDetail; @@ -7,8 +11,12 @@ use codex_tools::ToolOutput; use codex_tools::ToolPayload; use pretty_assertions::assert_eq; use serde_json::json; +use std::sync::Arc; use super::HistoryNotesToolOutput; +use super::history_notes_tools; +use crate::backend::HttpBackend; +use crate::backend::HttpContext; #[test] fn preserves_encrypted_history_output() { @@ -156,8 +164,8 @@ fn rejects_malformed_attachments_instead_of_silently_dropping_them() { #[test] fn sparse_catalog_messages_preserve_defaults_and_explicit_empty_descriptions() { - let action = super::HistoryNotesAction::NotesReadFile; - let mut expected = action.spec(/*history_messages*/ None, /*notes_messages*/ None); + let spec = super::tool_spec::; + let mut expected = spec(/*namespace_description*/ None, /*message*/ None); let codex_tools::ToolSpec::Namespace(namespace) = &mut expected else { panic!("expected namespace"); }; @@ -175,14 +183,30 @@ fn sparse_catalog_messages_preserve_defaults_and_explicit_empty_descriptions() { ..Default::default() }; assert_eq!( - action.spec(/*history_messages*/ None, Some(&messages)), + spec( + messages.namespace_description.as_deref(), + messages.read_file.as_ref() + ), expected ); } #[test] fn bundled_tool_contracts() { - let specs = super::HistoryNotesAction::ALL - .map(|action| action.spec(/*history_messages*/ None, /*notes_messages*/ None)); + let backend = HttpBackend::new( + create_model_provider( + ModelProviderInfo::create_openai_provider(/*base_url*/ None), + /*auth_manager*/ None, + ), + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + Arc::new(HttpContext { + session_id: "session-123".to_owned(), + current_agent_name: "/root".to_owned(), + }), + ); + let specs: Vec<_> = history_notes_tools(backend, /*history*/ None, /*notes*/ None) + .iter() + .map(|tool| tool.spec()) + .collect(); insta::assert_snapshot!(serde_json::to_string_pretty(&specs).unwrap()); } diff --git a/codex-rs/ext/history-notes/tests/history_notes_extension.rs b/codex-rs/ext/history-notes/tests/history_notes_extension.rs index e5db8b2ad2..937c1ef27f 100644 --- a/codex-rs/ext/history-notes/tests/history_notes_extension.rs +++ b/codex-rs/ext/history-notes/tests/history_notes_extension.rs @@ -175,7 +175,10 @@ async fn installed_extension_exposes_and_invokes_history_notes_tools() -> TestRe .expect("notes.read_file should be exposed"); let call = tool_call( ToolName::namespaced("notes", "read_file"), - json!({"path": "notes.md"}), + json!({ + "path": "notes.md", + "future_catalog_parameter": 42, + }), ); let output = read_file.handle(call.clone()).await?; let ResponseInputItem::FunctionCallOutput { output, .. } = @@ -212,6 +215,7 @@ async fn installed_extension_exposes_and_invokes_history_notes_tools() -> TestRe serde_json::from_slice::(&requests[0].body)?, json!({ "path": "notes.md", + "future_catalog_parameter": 42, "context": { "session_id": "session-123", "current_agent_name": "/root/worker",