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 d49545f70d..14aaddae38 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,10 +9,8 @@ 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; @@ -223,181 +221,6 @@ 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 encrypted_query = format!("enc_query_{}", "x".repeat(1_001)); diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 969a6298ff..047a054b89 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -42,10 +42,6 @@ use codex_protocol::turn_input::TurnStartOptions; #[cfg(test)] use crate::session::completed_session_loop_termination; -pub(crate) struct GuardianReadOnlyHistoryTools( - pub(crate) Vec codex_tools::ToolExecutor>>>, -); - /// 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. @@ -85,27 +81,6 @@ 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 { @@ -148,7 +123,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, + thread_extension_init: codex_extension_api::ExtensionDataInit::default(), 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 082bb8f0b2..d5729387d6 100644 --- a/codex-rs/core/src/tools/spec_plan.rs +++ b/codex-rs/core/src/tools/spec_plan.rs @@ -153,18 +153,6 @@ 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, - model_info, - history_tools.0.iter().cloned(), - &mut registry, - ); - } Vec::new() } else { let registered_mcp_tools = session.services.mcp_handler_cache.append_mcp_tools(