diff --git a/codex-rs/app-server/tests/suite/v2/history_notes_extension.rs b/codex-rs/app-server/tests/suite/v2/history_notes_extension.rs index 8415cc42f6..83e9d99623 100644 --- a/codex-rs/app-server/tests/suite/v2/history_notes_extension.rs +++ b/codex-rs/app-server/tests/suite/v2/history_notes_extension.rs @@ -9,8 +9,10 @@ use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::UserInput; use codex_config::types::AuthCredentialsStoreMode; +use codex_features::Feature; use core_test_support::load_default_config_for_test; use core_test_support::responses; +use core_test_support::skip_if_wine_exec; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; @@ -245,6 +247,181 @@ async fn app_server_uses_configured_notes_backend_for_context_window_hints( Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn guardian_can_read_parent_history_without_inheriting_notes_tools() -> Result<()> { + // TODO: Remove after Guardian accepts executor-native cwd across host operating systems. + skip_if_wine_exec!( + Ok(()), + "Guardian approval currently rejects a Windows executor cwd on the Linux host" + ); + + let server = responses::start_mock_server().await; + Mock::given(method("POST")) + .and(path("/backend-api/codex/alpha/notes/v2/thread_hint")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({"text": ""}))) + .expect(1) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path("/backend-api/codex/alpha/history/v2/list_items")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "results": [], + "n_returned": 0, + }))) + .expect(1) + .mount(&server) + .await; + let response_mock = responses::mount_sse_sequence( + &server, + vec![ + responses::sse(vec![ + responses::ev_response_created("resp-parent-command"), + responses::ev_function_call( + "parent-shell-call", + "exec_command", + &json!({ + "cmd": "echo guardian", + "sandbox_permissions": "require_escalated", + "justification": "Review a command using parent history.", + }) + .to_string(), + ), + responses::ev_completed("resp-parent-command"), + ]), + responses::sse(vec![ + responses::ev_response_created("resp-guardian-history"), + responses::ev_function_call_with_namespace( + "guardian-history-call", + "history", + "list_items", + &json!({"role": "user"}).to_string(), + ), + responses::ev_completed("resp-guardian-history"), + ]), + responses::sse(vec![ + responses::ev_response_created("resp-guardian-review"), + responses::ev_assistant_message( + "guardian-review", + &json!({ + "outcome": "deny", + "rationale": "The original user instructions do not authorize this command.", + }) + .to_string(), + ), + responses::ev_completed("resp-guardian-review"), + ]), + responses::sse(vec![ + responses::ev_response_created("resp-parent-done"), + responses::ev_assistant_message("parent-done", "Done"), + responses::ev_completed("resp-parent-done"), + ]), + ], + ) + .await; + + let codex_home = TempDir::new()?; + MockResponsesConfig::new(&server.uri()) + .with_model_provider("openai-custom") + .with_provider_name("OpenAI") + .with_provider_base_url(&format!("{}/backend-api/codex", server.uri())) + .with_provider_config("supports_websockets = false\nrequires_openai_auth = true") + .with_approval_policy("on-request") + .with_root_config("approvals_reviewer = \"auto_review\"") + .enable_feature(Feature::GuardianApproval) + .with_extra_config( + "[features.token_budget]\nenabled = true\nuse_history_notes_extension = true", + ) + .write(codex_home.path())?; + write_chatgpt_auth( + codex_home.path(), + ChatGptAuthFixture::new("access-chatgpt"), + AuthCredentialsStoreMode::File, + )?; + + let mut app_server = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .with_env_overrides(&[("OPENAI_API_KEY", None)]) + .build_initialized() + .await?; + let thread = app_server + .start_thread(ThreadStartParams::default()) + .await? + .thread; + timeout( + DEFAULT_READ_TIMEOUT, + app_server.start_turn_and_wait_for_completion(TurnStartParams { + thread_id: thread.id.clone(), + input: vec![UserInput::Text { + text: "Review a command using the original user instructions".to_string(), + text_elements: Vec::new(), + }], + ..Default::default() + }), + ) + .await??; + + let requests = response_mock.requests(); + assert_eq!(requests.len(), 4); + let guardian_request = &requests[1]; + let guardian_body = guardian_request.body_json(); + assert_eq!( + guardian_body["client_metadata"]["x-openai-subagent"], + "guardian" + ); + let guardian_tools = guardian_body.get("tools").or_else(|| { + guardian_body["input"] + .as_array()? + .iter() + .find(|item| item["type"] == "additional_tools")? + .get("tools") + }); + let guardian_tools = json!({"tools": guardian_tools.expect("Guardian tool definitions")}); + for tool_name in ["list_windows", "list_items", "read_item", "search_contents"] { + assert!( + responses::namespace_child_tool(&guardian_tools, "history", tool_name).is_some(), + "Guardian should expose history.{tool_name}" + ); + } + for tool_name in [ + "list_files_by_prefix", + "read_file", + "search_contents", + "append_to_file", + "write_file", + ] { + assert!( + requests[0].tool_by_name("notes", tool_name).is_some(), + "the parent should retain notes.{tool_name}" + ); + assert!( + responses::namespace_child_tool(&guardian_tools, "notes", tool_name).is_none(), + "Guardian must not inherit notes.{tool_name}" + ); + } + assert_eq!( + requests[2].function_call_output_text("guardian-history-call"), + Some(json!({"results": [], "n_returned": 0}).to_string()) + ); + + let backend_requests = server.received_requests().await.expect("recorded requests"); + let history_request = backend_requests + .iter() + .find(|request| request.url.path() == "/backend-api/codex/alpha/history/v2/list_items") + .expect("Guardian history request"); + assert_eq!( + history_request.body_json::()?, + json!({ + "role": "user", + "context": { + "session_id": thread.id, + "current_agent_name": "/root", + }, + }) + ); + + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn history_notes_and_async_message_emit_control_tool_analytics() -> Result<()> { let calls = [ diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 3bd74e0afa..0a74f925e1 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -42,6 +42,10 @@ use codex_protocol::turn_input::TurnStartOptions; #[cfg(test)] use crate::session::completed_session_loop_termination; +pub(crate) struct GuardianReadOnlyHistoryTools( + pub(crate) Vec>>, +); + /// Start an interactive sub-Codex thread and return its runtime and IO channels. /// /// Delegates never request approvals, and the returned IO yields their public events. @@ -81,6 +85,27 @@ pub(crate) async fn run_codex_thread_interactive( }; let session_source = SessionSource::SubAgent(subagent_source.clone()); let is_guardian_reviewer = crate::guardian::is_basic_session_source(&session_source); + let mut thread_extension_init = codex_extension_api::ExtensionDataInit::default(); + if is_guardian_reviewer { + let history_tools = crate::tools::spec_plan::extension_tool_executors( + parent_session.as_ref(), + parent_ctx.extension_data.as_ref(), + ) + .filter(|executor| { + let name = executor.tool_name(); + matches!( + (name.namespace.as_deref(), name.name.as_str()), + ( + Some("history"), + "list_windows" | "list_items" | "read_item" | "search_contents" + ) + ) + }) + .collect::>(); + if !history_tools.is_empty() { + thread_extension_init.insert(GuardianReadOnlyHistoryTools(history_tools)); + } + } let extensions = if is_guardian_reviewer { codex_extension_api::empty_extension_registry() } else { @@ -123,7 +148,7 @@ pub(crate) async fn run_codex_thread_interactive( parent_rollout_thread_trace: codex_rollout_trace::ThreadTraceContext::disabled(), parent_trace: None, environment_selections: parent_environments.to_selections(), - thread_extension_init: codex_extension_api::ExtensionDataInit::default(), + thread_extension_init, client_mcp_extensions: parent_session.services.client_mcp_extensions.clone(), reserved_thread_id: None, analytics_events_client: Some(parent_session.services.analytics_events_client.clone()), diff --git a/codex-rs/core/src/tools/spec_plan.rs b/codex-rs/core/src/tools/spec_plan.rs index e159828c63..dad0ba3487 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -142,6 +142,17 @@ pub(crate) fn build_tool_router( add_core_tool_sources(&context, &mut registry); let hosted_specs = if crate::guardian::is_basic_session_source(&turn_context.session_source) { + if let Some(history_tools) = session + .services + .thread_extension_data + .get::() + { + append_extension_tool_executors( + turn_context, + history_tools.0.iter().cloned(), + &mut registry, + ); + } Vec::new() } else { let registered_mcp_tools = session.services.mcp_handler_cache.append_mcp_tools(