From ff588f0d253bce7749e51f9b1009e5e2d67bfb69 Mon Sep 17 00:00:00 2001 From: Adam Perry Date: Thu, 4 Jun 2026 23:14:25 -0700 Subject: [PATCH] Address instruction preload review gaps --- .../app-server/tests/suite/v2/thread_fork.rs | 60 +++++++++++++++++++ .../tests/suite/v2/thread_resume.rs | 57 ++++++++++++++++++ codex-rs/mcp-server/tests/suite/codex_tool.rs | 19 +++++- 3 files changed, 133 insertions(+), 3 deletions(-) diff --git a/codex-rs/app-server/tests/suite/v2/thread_fork.rs b/codex-rs/app-server/tests/suite/v2/thread_fork.rs index a335b441ff..98c5188aa2 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_fork.rs @@ -36,6 +36,7 @@ use codex_protocol::protocol::RolloutItem; use codex_rollout::append_rollout_item_to_path; use codex_rollout::append_thread_name; use codex_rollout::read_session_meta_line; +use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use serde_json::Value; use serde_json::json; @@ -249,6 +250,65 @@ async fn thread_fork_creates_new_thread_and_emits_started() -> Result<()> { Ok(()) } +#[tokio::test] +async fn thread_fork_loads_instruction_sources() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; + let workspace = TempDir::new()?; + let project_agents = workspace.path().join("AGENTS.md"); + std::fs::write(&project_agents, "project instructions")?; + + let conversation_id = create_fake_rollout( + codex_home.path(), + "2025-01-05T12-00-00", + "2025-01-05T12:00:00Z", + "Saved user message", + Some("mock_provider"), + /*git_info*/ None, + )?; + let original_path = codex_home + .path() + .join("sessions") + .join("2025") + .join("01") + .join("05") + .join(format!( + "rollout-2025-01-05T12-00-00-{conversation_id}.jsonl" + )); + let mut session_meta = read_session_meta_line(&original_path).await?; + session_meta.meta.cwd = workspace.path().to_path_buf(); + append_rollout_item_to_path(&original_path, &RolloutItem::SessionMeta(session_meta)).await?; + + let mut mcp = TestAppServer::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let fork_id = mcp + .send_thread_fork_request(ThreadForkParams { + thread_id: conversation_id, + ..Default::default() + }) + .await?; + let fork_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(fork_id)), + ) + .await??; + let ThreadForkResponse { + instruction_sources, + .. + } = to_response::(fork_resp)?; + + assert_eq!( + instruction_sources, + vec![AbsolutePathBuf::try_from(std::fs::canonicalize( + project_agents + )?)?] + ); + + Ok(()) +} + #[tokio::test] async fn thread_fork_inherits_explicit_source_name_from_session_index() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index 4ae68b0e71..9bc416741d 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -332,6 +332,63 @@ async fn thread_resume_running_thread_uses_cached_instruction_sources() -> Resul Ok(()) } +#[tokio::test] +async fn thread_resume_cold_thread_loads_instruction_sources() -> Result<()> { + let server = create_mock_responses_server_repeating_assistant("Done").await; + let codex_home = TempDir::new()?; + create_config_toml(codex_home.path(), &server.uri())?; + let workspace = TempDir::new()?; + let project_agents = workspace.path().join("AGENTS.md"); + std::fs::write(&project_agents, "project instructions")?; + + let conversation_id = create_fake_rollout( + codex_home.path(), + "2025-01-05T12-00-00", + "2025-01-05T12:00:00Z", + "Saved user message", + Some("mock_provider"), + /*git_info*/ None, + )?; + let rollout_file = rollout_path(codex_home.path(), "2025-01-05T12-00-00", &conversation_id); + let contents = std::fs::read_to_string(&rollout_file)?; + let mut lines = contents.lines(); + let session_meta = lines + .next() + .ok_or_else(|| anyhow::anyhow!("fake rollout missing session meta"))?; + let mut session_meta: serde_json::Value = serde_json::from_str(session_meta)?; + session_meta["payload"]["cwd"] = json!(workspace.path()); + let remaining = lines.collect::>().join("\n"); + std::fs::write(&rollout_file, format!("{session_meta}\n{remaining}\n"))?; + + let mut mcp = TestAppServer::new(codex_home.path()).await?; + timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??; + + let resume_id = mcp + .send_thread_resume_request(ThreadResumeParams { + thread_id: conversation_id, + ..Default::default() + }) + .await?; + let resume_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(resume_id)), + ) + .await??; + let ThreadResumeResponse { + instruction_sources, + .. + } = to_response::(resume_resp)?; + + assert_eq!( + instruction_sources, + vec![AbsolutePathBuf::try_from(std::fs::canonicalize( + project_agents + )?)?] + ); + + Ok(()) +} + #[tokio::test] async fn turn_start_updates_runtime_workspace_roots_for_loaded_thread() -> Result<()> { let server = create_mock_responses_server_repeating_assistant("Done").await; diff --git a/codex-rs/mcp-server/tests/suite/codex_tool.rs b/codex-rs/mcp-server/tests/suite/codex_tool.rs index d9f290c3c5..b8e0fd20a4 100644 --- a/codex-rs/mcp-server/tests/suite/codex_tool.rs +++ b/codex-rs/mcp-server/tests/suite/codex_tool.rs @@ -351,17 +351,17 @@ async fn patch_approval_triggers_elicitation() -> anyhow::Result<()> { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_codex_tool_passes_base_instructions() { +async fn test_codex_tool_passes_base_and_user_instructions() { skip_if_no_network!(); // Apparently `#[tokio::test]` must return `()`, so we create a helper // function that returns `Result` so we can use `?` in favor of `unwrap`. - if let Err(err) = codex_tool_passes_base_instructions().await { + if let Err(err) = codex_tool_passes_base_and_user_instructions().await { panic!("failure: {err}"); } } -async fn codex_tool_passes_base_instructions() -> anyhow::Result<()> { +async fn codex_tool_passes_base_and_user_instructions() -> anyhow::Result<()> { #![expect(clippy::expect_used, clippy::unwrap_used)] let server = @@ -373,10 +373,16 @@ async fn codex_tool_passes_base_instructions() -> anyhow::Result<()> { create_config_toml(codex_home.path(), &server.uri())?; let mut mcp_process = McpProcess::new(codex_home.path()).await?; timeout(DEFAULT_READ_TIMEOUT, mcp_process.initialize()).await??; + let workspace = TempDir::new()?; + std::fs::write( + workspace.path().join("AGENTS.md"), + "Follow the project instructions.", + )?; // Send a "codex" tool request, which should hit the responses endpoint. let codex_request_id = mcp_process .send_codex_tool_call(CodexToolCallParam { + cwd: Some(workspace.path().to_string_lossy().to_string()), prompt: "How are you?".to_string(), base_instructions: Some("You are a helpful assistant.".to_string()), developer_instructions: Some("Foreshadow upcoming tool calls.".to_string()), @@ -442,6 +448,13 @@ async fn codex_tool_passes_base_instructions() -> anyhow::Result<()> { developer_contents.contains(&"Foreshadow upcoming tool calls."), "expected developer instructions in developer messages, got {developer_contents:?}" ); + assert!( + request["input"] + .to_string() + .contains("Follow the project instructions."), + "expected AGENTS.md instructions in model input, got {}", + request["input"] + ); Ok(()) }