Address instruction preload review gaps

This commit is contained in:
Adam Perry
2026-06-04 23:14:25 -07:00
parent 681e5678e7
commit ff588f0d25
3 changed files with 133 additions and 3 deletions

View File

@@ -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::<ThreadForkResponse>(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;

View File

@@ -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::<Vec<_>>().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::<ThreadResumeResponse>(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;

View File

@@ -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(())
}