From b6ad8ea1fcd5562a7d3003887930be7d84824307 Mon Sep 17 00:00:00 2001 From: jif Date: Tue, 27 Oct 2026 10:59:18 +0000 Subject: [PATCH] Test goal provenance in existing workers' Guardian reviews (#57430) ## What changed Add a parameterized app-server test that sets a paused goal after a worker has spawned and completed its first Guardian review, then asks the worker to check again. Verify that a user-origin goal appears in the subsequent review's root authorization context as user-set, while an automatic-origin goal is excluded. Also assert that the initial review does not contain the later goal. GitOrigin-RevId: 1d67c4a5481a717eac706b64ee714e7da2fc5511 --- .../tests/suite/v2/guardian_goal_tests.rs | 117 +++++++++++++++++- 1 file changed, 116 insertions(+), 1 deletion(-) diff --git a/codex-rs/app-server/tests/suite/v2/guardian_goal_tests.rs b/codex-rs/app-server/tests/suite/v2/guardian_goal_tests.rs index 358114e4de..fb6e707a70 100644 --- a/codex-rs/app-server/tests/suite/v2/guardian_goal_tests.rs +++ b/codex-rs/app-server/tests/suite/v2/guardian_goal_tests.rs @@ -1,10 +1,125 @@ -//! Goal API edits and clears enter model history across loaded and cold-resume paths. +//! Goal API edits reach model history and existing workers' Guardian authorization. use super::*; use codex_app_server_protocol::ThreadGoalSetResponse; use pretty_assertions::assert_eq; use test_case::test_case; +/// A goal set after spawning a worker must reach its reviewer only with user provenance. +#[test_case("user"; "user goal")] +#[test_case("automatic"; "automatic goal is not authorization")] +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn worker_guardian_sees_goal_set_after_spawn(origin: &str) -> Result<()> { + skip_if_no_network!(Ok(())); + let state = Arc::new(MockResponsesState { + root_worker: true, + mcp_server_name: Some(TEST_SERVER_NAME), + ..Default::default() + }); + let listener = TcpListener::bind("127.0.0.1:0").await?; + let responses_url = format!("http://{}", listener.local_addr()?); + let router = Router::new() + .route("/v1/responses", post(parent_response)) + .with_state(Arc::clone(&state)); + let responses_server = tokio::spawn(async move { axum::serve(listener, router).await }); + let (mcp_url, mcp_server) = start_mcp_server(/*sensitive_action*/ None).await?; + let codex_home = TempDir::new()?; + MockResponsesConfig::new(&responses_url) + .with_provider_config("supports_websockets = false") + .with_approval_policy("on-request") + .with_root_config("approvals_reviewer = \"auto_review\"") + .with_extra_config(&format!( + "[mcp_servers.{TEST_SERVER_NAME}]\nurl = \"{mcp_url}/mcp\"\ndefault_tools_approval_mode = \"prompt\"" + )) + .enable_feature(Feature::GuardianApproval) + .enable_feature(Feature::Collab) + .enable_feature(Feature::MultiAgentV2) + .enable_feature(Feature::Goals) + .disable_feature(Feature::GuardianV2) + .disable_feature(Feature::EnableRequestCompression) + .write(codex_home.path())?; + let mut app = TestAppServer::builder() + .with_codex_home(codex_home.path()) + .build_initialized_with_timeout(TIMEOUT) + .await?; + let root = app.start_thread(ThreadStartParams::default()).await?.thread; + state.allow_guardian_review.notify_one(); + let id = app + .send_turn_start_request(TurnStartParams { + thread_id: root.id.clone(), + input: vec![UserInput::Text { + text: USER_CONTEXT.to_owned(), + text_elements: vec![], + }], + ..Default::default() + }) + .await?; + let _: TurnStartResponse = app.read_response(id).await?; + let review: ItemGuardianApprovalReviewStartedNotification = timeout( + TIMEOUT, + app.read_notification("item/autoApprovalReview/started"), + ) + .await??; + assert_ne!(review.thread_id, root.id); + loop { + let completed: TurnCompletedNotification = + timeout(TIMEOUT, app.read_notification("turn/completed")).await??; + if completed.thread_id == review.thread_id { + assert_eq!(completed.turn.status, TurnStatus::Completed); + break; + } + } + wait_for_guardian_reviews(&state, /*expected*/ 2).await?; + + // The worker's fork and first review predate the goal. Keep the goal paused so + // only the explicit follow-up below starts work, without an autonomous goal turn. + let objective = "Publish the approved report using the configured MCP tool."; + let id = app + .send_raw_request( + "thread/goal/set", + Some(json!({ + "threadId": root.id, "objective": objective, "status": "paused", "origin": origin, + })), + ) + .await?; + let _: ThreadGoalSetResponse = app.read_response(id).await?; + let id = app + .send_turn_start_request(TurnStartParams { + thread_id: root.id, + input: vec![UserInput::Text { + text: "Ask the worker to check again.".to_owned(), + text_elements: vec![], + }], + ..Default::default() + }) + .await?; + let _: TurnStartResponse = app.read_response(id).await?; + wait_for_guardian_reviews(&state, /*expected*/ 3).await?; + { + let requests = state.guardian_requests.lock().expect("Guardian requests"); + assert!(!requests[0].to_string().contains(objective)); + let text = requests[2]["input"].to_string(); + assert_eq!(text.contains(objective), origin == "user", "{text}"); + if origin == "user" { + let (_, root_context) = text + .split_once(">>> ROOT CONVERSATION START") + .expect("root authorization context"); + let (root_context, _) = root_context + .split_once(">>> ROOT CONVERSATION END") + .expect("root authorization context end"); + assert!( + root_context.contains("User set the goal:"), + "{root_context}" + ); + assert!(root_context.contains(objective), "{root_context}"); + } + } + app.shutdown_gracefully().await?; + mcp_server.abort(); + responses_server.abort(); + Ok(()) +} + #[test_case(false; "loaded")] #[test_case(true; "unloaded with path resume")] #[tokio::test]