Skip to content

Commit c6fcec5

Browse files
authored
fix: always send full instructions when using the Responses API (#1207)
This fixes a longstanding error in the Rust CLI where `codex.rs` contained an errant `is_first_turn` check that would exclude the user instructions for subsequent "turns" of a conversation when using the responses API (i.e., when `previous_response_id` existed). While here, renames `Prompt.instructions` to `Prompt.user_instructions` since we now have quite a few levels of instructions floating around. Also removed an unnecessary use of `clone()` in `Prompt.get_full_instructions()`.
1 parent 6fcc528 commit c6fcec5

File tree

3 files changed

+22
-36
lines changed

3 files changed

+22
-36
lines changed

codex-rs/core/src/client_common.rs

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ pub struct Prompt {
2525
pub prev_id: Option<String>,
2626
/// Optional instructions from the user to amend to the built-in agent
2727
/// instructions.
28-
pub instructions: Option<String>,
28+
pub user_instructions: Option<String>,
2929
/// Whether to store response on server side (disable_response_storage = !store).
3030
pub store: bool,
3131

@@ -37,21 +37,14 @@ pub struct Prompt {
3737

3838
impl Prompt {
3939
pub(crate) fn get_full_instructions(&self, model: &str) -> Cow<str> {
40-
[
41-
Some(Cow::Borrowed(BASE_INSTRUCTIONS)),
42-
self.instructions.as_ref().map(|s| Cow::Owned(s.clone())),
43-
if model.starts_with("gpt-4.1") {
44-
Some(Cow::Borrowed(APPLY_PATCH_TOOL_INSTRUCTIONS))
45-
} else {
46-
None
47-
},
48-
]
49-
.iter()
50-
.filter_map(|s| s.as_ref())
51-
.map(|cow| cow.as_ref())
52-
.collect::<Vec<_>>()
53-
.join("\n")
54-
.into()
40+
let mut sections: Vec<&str> = vec![BASE_INSTRUCTIONS];
41+
if let Some(ref user) = self.user_instructions {
42+
sections.push(user);
43+
}
44+
if model.starts_with("gpt-4.1") {
45+
sections.push(APPLY_PATCH_TOOL_INSTRUCTIONS);
46+
}
47+
Cow::Owned(sections.join("\n"))
5548
}
5649
}
5750

codex-rs/core/src/codex.rs

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ use crate::models::ReasoningItemReasoningSummary;
5959
use crate::models::ResponseInputItem;
6060
use crate::models::ResponseItem;
6161
use crate::models::ShellToolCallParams;
62-
use crate::project_doc::create_full_instructions;
62+
use crate::project_doc::get_user_instructions;
6363
use crate::protocol::AgentMessageEvent;
6464
use crate::protocol::AgentReasoningEvent;
6565
use crate::protocol::ApplyPatchApprovalRequestEvent;
@@ -104,7 +104,7 @@ impl Codex {
104104
let (tx_sub, rx_sub) = async_channel::bounded(64);
105105
let (tx_event, rx_event) = async_channel::bounded(64);
106106

107-
let instructions = create_full_instructions(&config).await;
107+
let instructions = get_user_instructions(&config).await;
108108
let configure_session = Op::ConfigureSession {
109109
provider: config.model_provider.clone(),
110110
model: config.model.clone(),
@@ -990,9 +990,8 @@ async fn run_turn(
990990
input: Vec<ResponseItem>,
991991
) -> CodexResult<Vec<ProcessedResponseItem>> {
992992
// Decide whether to use server-side storage (previous_response_id) or disable it
993-
let (prev_id, store, is_first_turn) = {
993+
let (prev_id, store) = {
994994
let state = sess.state.lock().unwrap();
995-
let is_first_turn = state.previous_response_id.is_none();
996995
let store = state.zdr_transcript.is_none();
997996
let prev_id = if store {
998997
state.previous_response_id.clone()
@@ -1001,20 +1000,14 @@ async fn run_turn(
10011000
// back, but trying to use it results in a 400.
10021001
None
10031002
};
1004-
(prev_id, store, is_first_turn)
1005-
};
1006-
1007-
let instructions = if is_first_turn {
1008-
sess.instructions.clone()
1009-
} else {
1010-
None
1003+
(prev_id, store)
10111004
};
10121005

10131006
let extra_tools = sess.mcp_connection_manager.list_all_tools();
10141007
let prompt = Prompt {
10151008
input,
10161009
prev_id,
1017-
instructions,
1010+
user_instructions: sess.instructions.clone(),
10181011
store,
10191012
extra_tools,
10201013
};

codex-rs/core/src/project_doc.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ const PROJECT_DOC_SEPARATOR: &str = "\n\n--- project-doc ---\n\n";
2525

2626
/// Combines `Config::instructions` and `AGENTS.md` (if present) into a single
2727
/// string of instructions.
28-
pub(crate) async fn create_full_instructions(config: &Config) -> Option<String> {
28+
pub(crate) async fn get_user_instructions(config: &Config) -> Option<String> {
2929
match find_project_doc(config).await {
3030
Ok(Some(project_doc)) => match &config.instructions {
3131
Some(original_instructions) => Some(format!(
@@ -168,7 +168,7 @@ mod tests {
168168
async fn no_doc_file_returns_none() {
169169
let tmp = tempfile::tempdir().expect("tempdir");
170170

171-
let res = create_full_instructions(&make_config(&tmp, 4096, None)).await;
171+
let res = get_user_instructions(&make_config(&tmp, 4096, None)).await;
172172
assert!(
173173
res.is_none(),
174174
"Expected None when AGENTS.md is absent and no system instructions provided"
@@ -182,7 +182,7 @@ mod tests {
182182
let tmp = tempfile::tempdir().expect("tempdir");
183183
fs::write(tmp.path().join("AGENTS.md"), "hello world").unwrap();
184184

185-
let res = create_full_instructions(&make_config(&tmp, 4096, None))
185+
let res = get_user_instructions(&make_config(&tmp, 4096, None))
186186
.await
187187
.expect("doc expected");
188188

@@ -201,7 +201,7 @@ mod tests {
201201
let huge = "A".repeat(LIMIT * 2); // 2 KiB
202202
fs::write(tmp.path().join("AGENTS.md"), &huge).unwrap();
203203

204-
let res = create_full_instructions(&make_config(&tmp, LIMIT, None))
204+
let res = get_user_instructions(&make_config(&tmp, LIMIT, None))
205205
.await
206206
.expect("doc expected");
207207

@@ -233,7 +233,7 @@ mod tests {
233233
let mut cfg = make_config(&repo, 4096, None);
234234
cfg.cwd = nested;
235235

236-
let res = create_full_instructions(&cfg).await.expect("doc expected");
236+
let res = get_user_instructions(&cfg).await.expect("doc expected");
237237
assert_eq!(res, "root level doc");
238238
}
239239

@@ -243,7 +243,7 @@ mod tests {
243243
let tmp = tempfile::tempdir().expect("tempdir");
244244
fs::write(tmp.path().join("AGENTS.md"), "something").unwrap();
245245

246-
let res = create_full_instructions(&make_config(&tmp, 0, None)).await;
246+
let res = get_user_instructions(&make_config(&tmp, 0, None)).await;
247247
assert!(
248248
res.is_none(),
249249
"With limit 0 the function should return None"
@@ -259,7 +259,7 @@ mod tests {
259259

260260
const INSTRUCTIONS: &str = "base instructions";
261261

262-
let res = create_full_instructions(&make_config(&tmp, 4096, Some(INSTRUCTIONS)))
262+
let res = get_user_instructions(&make_config(&tmp, 4096, Some(INSTRUCTIONS)))
263263
.await
264264
.expect("should produce a combined instruction string");
265265

@@ -276,7 +276,7 @@ mod tests {
276276

277277
const INSTRUCTIONS: &str = "some instructions";
278278

279-
let res = create_full_instructions(&make_config(&tmp, 4096, Some(INSTRUCTIONS))).await;
279+
let res = get_user_instructions(&make_config(&tmp, 4096, Some(INSTRUCTIONS))).await;
280280

281281
assert_eq!(res, Some(INSTRUCTIONS.to_string()));
282282
}

0 commit comments

Comments
 (0)