mirror of
https://github.com/openai/codex.git
synced 2026-11-09 20:50:33 +00:00
Merge remote-tracking branch 'origin/main' into easong/mcp-CLI-interface
This commit is contained in:
easong-openai
+16
-8
@@ -134,14 +134,6 @@ If completing the user's task requires writing or modifying files, your code and
|
||||
- Do not use one-letter variable names unless explicitly requested.
|
||||
- NEVER output inline citations like "【F:README.md†L5-L14】" in your outputs. The CLI is not able to render these so they will just be broken in the UI. Instead, if you output valid filepaths, users will be able to click on them to open the files in their editor.
|
||||
|
||||
## Testing your work
|
||||
|
||||
If the codebase has tests or the ability to build or run, you should use them to verify that your work is complete. Generally, your testing philosophy should be to start as specific as possible to the code you changed so that you can catch issues efficiently, then make your way to broader tests as you build confidence. If there's no test for the code you changed, and if the adjacent patterns in the codebases show that there's a logical place for you to add a test, you may do so. However, do not add tests to codebases with no tests, or where the patterns don't indicate so.
|
||||
|
||||
Once you're confident in correctness, use formatting commands to ensure that your code is well formatted. These commands can take time so you should run them on as precise a target as possible. If there are issues you can iterate up to 3 times to get formatting right, but if you still can't manage it's better to save the user time and present them a correct solution where you call out the formatting in your final message. If the codebase does not have a formatter configured, do not add one.
|
||||
|
||||
For all of testing, running, building, and formatting, do not attempt to fix unrelated bugs. It is not your responsibility to fix them. (You may mention them to the user in your final message though.)
|
||||
|
||||
## Sandbox and approvals
|
||||
|
||||
The Codex CLI harness supports several different sandboxing, and approval configurations that the user can choose from.
|
||||
@@ -177,6 +169,22 @@ Note that when sandboxing is set to read-only, you'll need to request approval f
|
||||
|
||||
You will be told what filesystem sandboxing, network sandboxing, and approval mode are active in a developer or user message. If you are not told about this, assume that you are running with workspace-write, network sandboxing ON, and approval on-failure.
|
||||
|
||||
## Validating your work
|
||||
|
||||
If the codebase has tests or the ability to build or run, consider using them to verify that your work is complete.
|
||||
|
||||
When testing, your philosophy should be to start as specific as possible to the code you changed so that you can catch issues efficiently, then make your way to broader tests as you build confidence. If there's no test for the code you changed, and if the adjacent patterns in the codebases show that there's a logical place for you to add a test, you may do so. However, do not add tests to codebases with no tests.
|
||||
|
||||
Similarly, once you're confident in correctness, you can suggest or use formatting commands to ensure that your code is well formatted. If there are issues you can iterate up to 3 times to get formatting right, but if you still can't manage it's better to save the user time and present them a correct solution where you call out the formatting in your final message. If the codebase does not have a formatter configured, do not add one.
|
||||
|
||||
For all of testing, running, building, and formatting, do not attempt to fix unrelated bugs. It is not your responsibility to fix them. (You may mention them to the user in your final message though.)
|
||||
|
||||
Be mindful of whether to run validation commands proactively. In the absence of behavioral guidance:
|
||||
|
||||
- When running in non-interactive approval modes like **never** or **on-failure**, proactively run tests, lint and do whatever you need to ensure you've completed the task.
|
||||
- When working in interactive approval modes like **untrusted**, or **on-request**, hold off on running tests or lint commands until the user is ready for you to finalize your output, because these commands take time to run and slow down iteration. Instead suggest what you want to do next, and let the user confirm first.
|
||||
- When working on test-related tasks, such as adding tests, fixing tests, or reproducing a bug to verify behavior, you may proactively run tests regardless of approval mode. Use your judgement to decide whether this is a test-related task.
|
||||
|
||||
## Ambition vs. precision
|
||||
|
||||
For tasks that have no prior context (i.e. the user is starting something brand new), you should feel free to be ambitious and demonstrate creativity with your implementation.
|
||||
|
||||
+39
-82
@@ -1,6 +1,5 @@
|
||||
use std::io::BufRead;
|
||||
use std::path::Path;
|
||||
use std::sync::OnceLock;
|
||||
use std::time::Duration;
|
||||
|
||||
use bytes::Bytes;
|
||||
@@ -8,7 +7,6 @@ use codex_login::AuthManager;
|
||||
use codex_login::AuthMode;
|
||||
use eventsource_stream::Eventsource;
|
||||
use futures::prelude::*;
|
||||
use regex_lite::Regex;
|
||||
use reqwest::StatusCode;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
@@ -37,6 +35,7 @@ use crate::flags::CODEX_RS_SSE_FIXTURE;
|
||||
use crate::model_family::ModelFamily;
|
||||
use crate::model_provider_info::ModelProviderInfo;
|
||||
use crate::model_provider_info::WireApi;
|
||||
use crate::openai_model_info::get_model_info;
|
||||
use crate::openai_tools::create_tools_json_for_responses_api;
|
||||
use crate::protocol::TokenUsage;
|
||||
use crate::user_agent::get_codex_user_agent;
|
||||
@@ -54,8 +53,11 @@ struct ErrorResponse {
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct Error {
|
||||
r#type: Option<String>,
|
||||
code: Option<String>,
|
||||
message: Option<String>,
|
||||
|
||||
// Optional fields available on "usage_limit_reached" and "usage_not_included" errors
|
||||
plan_type: Option<String>,
|
||||
resets_in_seconds: Option<u64>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
@@ -89,6 +91,12 @@ impl ModelClient {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn get_model_context_window(&self) -> Option<u64> {
|
||||
self.config
|
||||
.model_context_window
|
||||
.or_else(|| get_model_info(&self.config.model_family).map(|info| info.context_window))
|
||||
}
|
||||
|
||||
/// Dispatches to either the Responses or Chat implementation depending on
|
||||
/// the provider config. Public callers always invoke `stream()` – the
|
||||
/// specialised helpers are private to avoid accidental misuse.
|
||||
@@ -142,9 +150,12 @@ impl ModelClient {
|
||||
}
|
||||
|
||||
let auth_manager = self.auth_manager.clone();
|
||||
let auth = auth_manager.as_ref().and_then(|m| m.auth());
|
||||
|
||||
let auth_mode = auth.as_ref().map(|a| a.mode);
|
||||
let auth_mode = auth_manager
|
||||
.as_ref()
|
||||
.and_then(|m| m.auth())
|
||||
.as_ref()
|
||||
.map(|a| a.mode);
|
||||
|
||||
let store = prompt.store && auth_mode != Some(AuthMode::ChatGPT);
|
||||
|
||||
@@ -211,15 +222,18 @@ impl ModelClient {
|
||||
let mut attempt = 0;
|
||||
let max_retries = self.provider.request_max_retries();
|
||||
|
||||
trace!(
|
||||
"POST to {}: {}",
|
||||
self.provider.get_full_url(&auth),
|
||||
serde_json::to_string(&payload)?
|
||||
);
|
||||
|
||||
loop {
|
||||
attempt += 1;
|
||||
|
||||
// Always fetch the latest auth in case a prior attempt refreshed the token.
|
||||
let auth = auth_manager.as_ref().and_then(|m| m.auth());
|
||||
|
||||
trace!(
|
||||
"POST to {}: {}",
|
||||
self.provider.get_full_url(&auth),
|
||||
serde_json::to_string(&payload)?
|
||||
);
|
||||
|
||||
let mut req_builder = self
|
||||
.provider
|
||||
.create_request_builder(&self.client, &auth)
|
||||
@@ -303,19 +317,20 @@ impl ModelClient {
|
||||
|
||||
if status == StatusCode::TOO_MANY_REQUESTS {
|
||||
let body = res.json::<ErrorResponse>().await.ok();
|
||||
if let Some(ErrorResponse {
|
||||
error:
|
||||
Error {
|
||||
r#type: Some(error_type),
|
||||
..
|
||||
},
|
||||
}) = body
|
||||
{
|
||||
if error_type == "usage_limit_reached" {
|
||||
if let Some(ErrorResponse { error }) = body {
|
||||
if error.r#type.as_deref() == Some("usage_limit_reached") {
|
||||
// Prefer the plan_type provided in the error message if present
|
||||
// because it's more up to date than the one encoded in the auth
|
||||
// token.
|
||||
let plan_type = error
|
||||
.plan_type
|
||||
.or_else(|| auth.and_then(|a| a.get_plan_type()));
|
||||
let resets_in_seconds = error.resets_in_seconds;
|
||||
return Err(CodexErr::UsageLimitReached(UsageLimitReachedError {
|
||||
plan_type: auth.and_then(|a| a.get_plan_type()),
|
||||
plan_type,
|
||||
resets_in_seconds,
|
||||
}));
|
||||
} else if error_type == "usage_not_included" {
|
||||
} else if error.r#type.as_deref() == Some("usage_not_included") {
|
||||
return Err(CodexErr::UsageNotIncluded);
|
||||
}
|
||||
}
|
||||
@@ -563,9 +578,8 @@ async fn process_sse<S>(
|
||||
if let Some(error) = error {
|
||||
match serde_json::from_value::<Error>(error.clone()) {
|
||||
Ok(error) => {
|
||||
let delay = try_parse_retry_after(&error);
|
||||
let message = error.message.unwrap_or_default();
|
||||
response_error = Some(CodexErr::Stream(message, delay));
|
||||
response_error = Some(CodexErr::Stream(message, None));
|
||||
}
|
||||
Err(e) => {
|
||||
debug!("failed to parse ErrorResponse: {e}");
|
||||
@@ -653,40 +667,6 @@ async fn stream_from_fixture(
|
||||
Ok(ResponseStream { rx_event })
|
||||
}
|
||||
|
||||
fn rate_limit_regex() -> &'static Regex {
|
||||
static RE: OnceLock<Regex> = OnceLock::new();
|
||||
|
||||
#[expect(clippy::unwrap_used)]
|
||||
RE.get_or_init(|| Regex::new(r"Please try again in (\d+(?:\.\d+)?)(s|ms)").unwrap())
|
||||
}
|
||||
|
||||
fn try_parse_retry_after(err: &Error) -> Option<Duration> {
|
||||
if err.code != Some("rate_limit_exceeded".to_string()) {
|
||||
return None;
|
||||
}
|
||||
|
||||
// parse the Please try again in 1.898s format using regex
|
||||
let re = rate_limit_regex();
|
||||
if let Some(message) = &err.message
|
||||
&& let Some(captures) = re.captures(message)
|
||||
{
|
||||
let seconds = captures.get(1);
|
||||
let unit = captures.get(2);
|
||||
|
||||
if let (Some(value), Some(unit)) = (seconds, unit) {
|
||||
let value = value.as_str().parse::<f64>().ok()?;
|
||||
let unit = unit.as_str();
|
||||
|
||||
if unit == "s" {
|
||||
return Some(Duration::from_secs_f64(value));
|
||||
} else if unit == "ms" {
|
||||
return Some(Duration::from_millis(value as u64));
|
||||
}
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -907,7 +887,7 @@ mod tests {
|
||||
msg,
|
||||
"Rate limit reached for gpt-5 in organization org-AAA on tokens per min (TPM): Limit 30000, Used 22999, Requested 12528. Please try again in 11.054s. Visit https://platform.openai.com/account/rate-limits to learn more."
|
||||
);
|
||||
assert_eq!(*delay, Some(Duration::from_secs_f64(11.054)));
|
||||
assert_eq!(*delay, None);
|
||||
}
|
||||
other => panic!("unexpected second event: {other:?}"),
|
||||
}
|
||||
@@ -1011,27 +991,4 @@ mod tests {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_try_parse_retry_after() {
|
||||
let err = Error {
|
||||
r#type: None,
|
||||
message: Some("Rate limit reached for gpt-5 in organization org- on tokens per min (TPM): Limit 1, Used 1, Requested 19304. Please try again in 28ms. Visit https://platform.openai.com/account/rate-limits to learn more.".to_string()),
|
||||
code: Some("rate_limit_exceeded".to_string()),
|
||||
};
|
||||
|
||||
let delay = try_parse_retry_after(&err);
|
||||
assert_eq!(delay, Some(Duration::from_millis(28)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_try_parse_retry_after_no_delay() {
|
||||
let err = Error {
|
||||
r#type: None,
|
||||
message: Some("Rate limit reached for gpt-5 in organization <ORG> on tokens per min (TPM): Limit 30000, Used 6899, Requested 24050. Please try again in 1.898s. Visit https://platform.openai.com/account/rate-limits to learn more.".to_string()),
|
||||
code: Some("rate_limit_exceeded".to_string()),
|
||||
};
|
||||
let delay = try_parse_retry_after(&err);
|
||||
assert_eq!(delay, Some(Duration::from_secs_f64(1.898)));
|
||||
}
|
||||
}
|
||||
|
||||
+35
-18
@@ -15,6 +15,7 @@ use codex_apply_patch::MaybeApplyPatchVerified;
|
||||
use codex_apply_patch::maybe_parse_apply_patch_verified;
|
||||
use codex_login::AuthManager;
|
||||
use codex_protocol::protocol::ConversationHistoryResponseEvent;
|
||||
use codex_protocol::protocol::TaskStartedEvent;
|
||||
use codex_protocol::protocol::TurnAbortReason;
|
||||
use codex_protocol::protocol::TurnAbortedEvent;
|
||||
use futures::prelude::*;
|
||||
@@ -62,6 +63,7 @@ use crate::exec_env::create_env;
|
||||
use crate::mcp_connection_manager::McpConnectionManager;
|
||||
use crate::mcp_tool_call::handle_mcp_tool_call;
|
||||
use crate::model_family::find_family_for_model;
|
||||
use crate::openai_model_info::get_model_info;
|
||||
use crate::openai_tools::ApplyPatchToolArgs;
|
||||
use crate::openai_tools::ToolsConfig;
|
||||
use crate::openai_tools::ToolsConfigParams;
|
||||
@@ -1079,6 +1081,9 @@ async fn submission_loop(
|
||||
let mut updated_config = (*config).clone();
|
||||
updated_config.model = effective_model.clone();
|
||||
updated_config.model_family = effective_family.clone();
|
||||
if let Some(model_info) = get_model_info(&effective_family) {
|
||||
updated_config.model_context_window = Some(model_info.context_window);
|
||||
}
|
||||
|
||||
let client = ModelClient::new(
|
||||
Arc::new(updated_config),
|
||||
@@ -1152,6 +1157,7 @@ async fn submission_loop(
|
||||
if let Err(items) = sess.inject_input(items) {
|
||||
// Derive a fresh TurnContext for this turn using the provided overrides.
|
||||
let provider = turn_context.client.get_provider();
|
||||
let auth_manager = turn_context.client.get_auth_manager();
|
||||
|
||||
// Derive a model family for the requested model; fall back to the session's.
|
||||
let model_family = find_family_for_model(&model)
|
||||
@@ -1161,12 +1167,15 @@ async fn submission_loop(
|
||||
let mut per_turn_config = (*config).clone();
|
||||
per_turn_config.model = model.clone();
|
||||
per_turn_config.model_family = model_family.clone();
|
||||
if let Some(model_info) = get_model_info(&model_family) {
|
||||
per_turn_config.model_context_window = Some(model_info.context_window);
|
||||
}
|
||||
|
||||
// Build a new client with per‑turn reasoning settings.
|
||||
// Reuse the same provider and session id; auth defaults to env/API key.
|
||||
let client = ModelClient::new(
|
||||
Arc::new(per_turn_config),
|
||||
None,
|
||||
auth_manager,
|
||||
provider,
|
||||
effort,
|
||||
summary,
|
||||
@@ -1369,7 +1378,9 @@ async fn run_task(
|
||||
}
|
||||
let event = Event {
|
||||
id: sub_id.clone(),
|
||||
msg: EventMsg::TaskStarted,
|
||||
msg: EventMsg::TaskStarted(TaskStartedEvent {
|
||||
model_context_window: turn_context.client.get_model_context_window(),
|
||||
}),
|
||||
};
|
||||
if sess.tx_event.send(event).await.is_err() {
|
||||
return;
|
||||
@@ -1769,11 +1780,6 @@ async fn try_run_turn(
|
||||
return Ok(output);
|
||||
}
|
||||
ResponseEvent::OutputTextDelta(delta) => {
|
||||
{
|
||||
let mut st = sess.state.lock_unchecked();
|
||||
st.history.append_assistant_text(&delta);
|
||||
}
|
||||
|
||||
let event = Event {
|
||||
id: sub_id.to_string(),
|
||||
msg: EventMsg::AgentMessageDelta(AgentMessageDeltaEvent { delta }),
|
||||
@@ -1816,9 +1822,12 @@ async fn run_compact_task(
|
||||
input: Vec<InputItem>,
|
||||
compact_instructions: String,
|
||||
) {
|
||||
let model_context_window = turn_context.client.get_model_context_window();
|
||||
let start_event = Event {
|
||||
id: sub_id.clone(),
|
||||
msg: EventMsg::TaskStarted,
|
||||
msg: EventMsg::TaskStarted(TaskStartedEvent {
|
||||
model_context_window,
|
||||
}),
|
||||
};
|
||||
if sess.tx_event.send(start_event).await.is_err() {
|
||||
return;
|
||||
@@ -2454,11 +2463,15 @@ async fn handle_container_exec_with_params(
|
||||
sandbox_type,
|
||||
sandbox_policy: &turn_context.sandbox_policy,
|
||||
codex_linux_sandbox_exe: &sess.codex_linux_sandbox_exe,
|
||||
stdout_stream: Some(StdoutStream {
|
||||
sub_id: sub_id.clone(),
|
||||
call_id: call_id.clone(),
|
||||
tx_event: sess.tx_event.clone(),
|
||||
}),
|
||||
stdout_stream: if exec_command_context.apply_patch.is_some() {
|
||||
None
|
||||
} else {
|
||||
Some(StdoutStream {
|
||||
sub_id: sub_id.clone(),
|
||||
call_id: call_id.clone(),
|
||||
tx_event: sess.tx_event.clone(),
|
||||
})
|
||||
},
|
||||
},
|
||||
)
|
||||
.await;
|
||||
@@ -2587,11 +2600,15 @@ async fn handle_sandbox_error(
|
||||
sandbox_type: SandboxType::None,
|
||||
sandbox_policy: &turn_context.sandbox_policy,
|
||||
codex_linux_sandbox_exe: &sess.codex_linux_sandbox_exe,
|
||||
stdout_stream: Some(StdoutStream {
|
||||
sub_id: sub_id.clone(),
|
||||
call_id: call_id.clone(),
|
||||
tx_event: sess.tx_event.clone(),
|
||||
}),
|
||||
stdout_stream: if exec_command_context.apply_patch.is_some() {
|
||||
None
|
||||
} else {
|
||||
Some(StdoutStream {
|
||||
sub_id: sub_id.clone(),
|
||||
call_id: call_id.clone(),
|
||||
tx_event: sess.tx_event.clone(),
|
||||
})
|
||||
},
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -28,49 +28,7 @@ impl ConversationHistory {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Merge adjacent assistant messages into a single history entry.
|
||||
// This prevents duplicates when a partial assistant message was
|
||||
// streamed into history earlier in the turn and the final full
|
||||
// message is recorded at turn end.
|
||||
match (&*item, self.items.last_mut()) {
|
||||
(
|
||||
ResponseItem::Message {
|
||||
role: new_role,
|
||||
content: new_content,
|
||||
..
|
||||
},
|
||||
Some(ResponseItem::Message {
|
||||
role: last_role,
|
||||
content: last_content,
|
||||
..
|
||||
}),
|
||||
) if new_role == "assistant" && last_role == "assistant" => {
|
||||
append_text_content(last_content, new_content);
|
||||
}
|
||||
_ => {
|
||||
self.items.push(item.clone());
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Append a text `delta` to the latest assistant message, creating a new
|
||||
/// assistant entry if none exists yet (e.g. first delta for this turn).
|
||||
pub(crate) fn append_assistant_text(&mut self, delta: &str) {
|
||||
match self.items.last_mut() {
|
||||
Some(ResponseItem::Message { role, content, .. }) if role == "assistant" => {
|
||||
append_text_delta(content, delta);
|
||||
}
|
||||
_ => {
|
||||
// Start a new assistant message with the delta.
|
||||
self.items.push(ResponseItem::Message {
|
||||
id: None,
|
||||
role: "assistant".to_string(),
|
||||
content: vec![codex_protocol::models::ContentItem::OutputText {
|
||||
text: delta.to_string(),
|
||||
}],
|
||||
});
|
||||
}
|
||||
self.items.push(item.clone());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -118,34 +76,6 @@ fn is_api_message(message: &ResponseItem) -> bool {
|
||||
}
|
||||
}
|
||||
|
||||
/// Helper to append the textual content from `src` into `dst` in place.
|
||||
fn append_text_content(
|
||||
dst: &mut Vec<codex_protocol::models::ContentItem>,
|
||||
src: &Vec<codex_protocol::models::ContentItem>,
|
||||
) {
|
||||
for c in src {
|
||||
if let codex_protocol::models::ContentItem::OutputText { text } = c {
|
||||
append_text_delta(dst, text);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Append a single text delta to the last OutputText item in `content`, or
|
||||
/// push a new OutputText item if none exists.
|
||||
fn append_text_delta(content: &mut Vec<codex_protocol::models::ContentItem>, delta: &str) {
|
||||
if let Some(codex_protocol::models::ContentItem::OutputText { text }) = content
|
||||
.iter_mut()
|
||||
.rev()
|
||||
.find(|c| matches!(c, codex_protocol::models::ContentItem::OutputText { .. }))
|
||||
{
|
||||
text.push_str(delta);
|
||||
} else {
|
||||
content.push(codex_protocol::models::ContentItem::OutputText {
|
||||
text: delta.to_string(),
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -171,49 +101,6 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merges_adjacent_assistant_messages() {
|
||||
let mut h = ConversationHistory::default();
|
||||
let a1 = assistant_msg("Hello");
|
||||
let a2 = assistant_msg(", world!");
|
||||
h.record_items([&a1, &a2]);
|
||||
|
||||
let items = h.contents();
|
||||
assert_eq!(
|
||||
items,
|
||||
vec![ResponseItem::Message {
|
||||
id: None,
|
||||
role: "assistant".to_string(),
|
||||
content: vec![ContentItem::OutputText {
|
||||
text: "Hello, world!".to_string()
|
||||
}]
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn append_assistant_text_creates_and_appends() {
|
||||
let mut h = ConversationHistory::default();
|
||||
h.append_assistant_text("Hello");
|
||||
h.append_assistant_text(", world");
|
||||
|
||||
// Now record a final full assistant message and verify it merges.
|
||||
let final_msg = assistant_msg("!");
|
||||
h.record_items([&final_msg]);
|
||||
|
||||
let items = h.contents();
|
||||
assert_eq!(
|
||||
items,
|
||||
vec![ResponseItem::Message {
|
||||
id: None,
|
||||
role: "assistant".to_string(),
|
||||
content: vec![ContentItem::OutputText {
|
||||
text: "Hello, world!".to_string()
|
||||
}]
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filters_non_api_messages() {
|
||||
let mut h = ConversationHistory::default();
|
||||
|
||||
@@ -110,6 +110,10 @@ impl ConversationManager {
|
||||
.ok_or_else(|| CodexErr::ConversationNotFound(conversation_id))
|
||||
}
|
||||
|
||||
pub async fn remove_conversation(&self, conversation_id: Uuid) {
|
||||
self.conversations.write().await.remove(&conversation_id);
|
||||
}
|
||||
|
||||
/// Fork an existing conversation by dropping the last `drop_last_messages`
|
||||
/// user/assistant messages from its transcript and starting a new
|
||||
/// conversation with identical configuration (unless overridden by the
|
||||
|
||||
+107
-9
@@ -128,27 +128,70 @@ pub enum CodexErr {
|
||||
#[derive(Debug)]
|
||||
pub struct UsageLimitReachedError {
|
||||
pub plan_type: Option<String>,
|
||||
pub resets_in_seconds: Option<u64>,
|
||||
}
|
||||
|
||||
impl std::fmt::Display for UsageLimitReachedError {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
// Base message differs slightly for legacy ChatGPT Plus plan users.
|
||||
if let Some(plan_type) = &self.plan_type
|
||||
&& plan_type == "plus"
|
||||
{
|
||||
write!(
|
||||
f,
|
||||
"You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing), or wait for limits to reset (every 5h and every week.)."
|
||||
"You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing) or try again"
|
||||
)?;
|
||||
if let Some(secs) = self.resets_in_seconds {
|
||||
let reset_duration = format_reset_duration(secs);
|
||||
write!(f, " in {reset_duration}.")?;
|
||||
} else {
|
||||
write!(f, " later.")?;
|
||||
}
|
||||
} else {
|
||||
write!(
|
||||
f,
|
||||
"You've hit your usage limit. Limits reset every 5h and every week."
|
||||
)?;
|
||||
write!(f, "You've hit your usage limit.")?;
|
||||
|
||||
if let Some(secs) = self.resets_in_seconds {
|
||||
let reset_duration = format_reset_duration(secs);
|
||||
write!(f, " Try again in {reset_duration}.")?;
|
||||
} else {
|
||||
write!(f, " Try again later.")?;
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
fn format_reset_duration(total_secs: u64) -> String {
|
||||
let days = total_secs / 86_400;
|
||||
let hours = (total_secs % 86_400) / 3_600;
|
||||
let minutes = (total_secs % 3_600) / 60;
|
||||
|
||||
let mut parts: Vec<String> = Vec::new();
|
||||
if days > 0 {
|
||||
let unit = if days == 1 { "day" } else { "days" };
|
||||
parts.push(format!("{} {}", days, unit));
|
||||
}
|
||||
if hours > 0 {
|
||||
let unit = if hours == 1 { "hour" } else { "hours" };
|
||||
parts.push(format!("{} {}", hours, unit));
|
||||
}
|
||||
if minutes > 0 {
|
||||
let unit = if minutes == 1 { "minute" } else { "minutes" };
|
||||
parts.push(format!("{} {}", minutes, unit));
|
||||
}
|
||||
|
||||
if parts.is_empty() {
|
||||
return "less than a minute".to_string();
|
||||
}
|
||||
|
||||
match parts.len() {
|
||||
1 => parts[0].clone(),
|
||||
2 => format!("{} {}", parts[0], parts[1]),
|
||||
_ => format!("{} {} {}", parts[0], parts[1], parts[2]),
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct EnvVarError {
|
||||
/// Name of the environment variable that is missing.
|
||||
@@ -181,6 +224,8 @@ impl CodexErr {
|
||||
pub fn get_error_message_ui(e: &CodexErr) -> String {
|
||||
match e {
|
||||
CodexErr::Sandbox(SandboxErr::Denied(_, _, stderr)) => stderr.to_string(),
|
||||
// Timeouts are not sandbox errors from a UX perspective; present them plainly
|
||||
CodexErr::Sandbox(SandboxErr::Timeout) => "error: command timed out".to_string(),
|
||||
_ => e.to_string(),
|
||||
}
|
||||
}
|
||||
@@ -193,19 +238,23 @@ mod tests {
|
||||
fn usage_limit_reached_error_formats_plus_plan() {
|
||||
let err = UsageLimitReachedError {
|
||||
plan_type: Some("plus".to_string()),
|
||||
resets_in_seconds: None,
|
||||
};
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing), or wait for limits to reset (every 5h and every week.)."
|
||||
"You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing) or try again later."
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn usage_limit_reached_error_formats_default_when_none() {
|
||||
let err = UsageLimitReachedError { plan_type: None };
|
||||
let err = UsageLimitReachedError {
|
||||
plan_type: None,
|
||||
resets_in_seconds: None,
|
||||
};
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"You've hit your usage limit. Limits reset every 5h and every week."
|
||||
"You've hit your usage limit. Try again later."
|
||||
);
|
||||
}
|
||||
|
||||
@@ -213,10 +262,59 @@ mod tests {
|
||||
fn usage_limit_reached_error_formats_default_for_other_plans() {
|
||||
let err = UsageLimitReachedError {
|
||||
plan_type: Some("pro".to_string()),
|
||||
resets_in_seconds: None,
|
||||
};
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"You've hit your usage limit. Limits reset every 5h and every week."
|
||||
"You've hit your usage limit. Try again later."
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn usage_limit_reached_includes_minutes_when_available() {
|
||||
let err = UsageLimitReachedError {
|
||||
plan_type: None,
|
||||
resets_in_seconds: Some(5 * 60),
|
||||
};
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"You've hit your usage limit. Try again in 5 minutes."
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn usage_limit_reached_includes_hours_and_minutes() {
|
||||
let err = UsageLimitReachedError {
|
||||
plan_type: Some("plus".to_string()),
|
||||
resets_in_seconds: Some(3 * 3600 + 32 * 60),
|
||||
};
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"You've hit your usage limit. Upgrade to Pro (https://openai.com/chatgpt/pricing) or try again in 3 hours 32 minutes."
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn usage_limit_reached_includes_days_hours_minutes() {
|
||||
let err = UsageLimitReachedError {
|
||||
plan_type: None,
|
||||
resets_in_seconds: Some(2 * 86_400 + 3 * 3600 + 5 * 60),
|
||||
};
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"You've hit your usage limit. Try again in 2 days 3 hours 5 minutes."
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn usage_limit_reached_less_than_minute() {
|
||||
let err = UsageLimitReachedError {
|
||||
plan_type: None,
|
||||
resets_in_seconds: Some(30),
|
||||
};
|
||||
assert_eq!(
|
||||
err.to_string(),
|
||||
"You've hit your usage limit. Try again in less than a minute."
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -385,7 +385,9 @@ async fn find_closest_sha(cwd: &Path, branches: &[String], remotes: &[String]) -
|
||||
}
|
||||
|
||||
async fn diff_against_sha(cwd: &Path, sha: &GitSha) -> Option<String> {
|
||||
let output = run_git_command_with_timeout(&["diff", &sha.0], cwd).await?;
|
||||
let output =
|
||||
run_git_command_with_timeout(&["diff", "--no-textconv", "--no-ext-diff", &sha.0], cwd)
|
||||
.await?;
|
||||
// 0 is success and no diff.
|
||||
// 1 is success but there is a diff.
|
||||
let exit_ok = output.status.code().is_some_and(|c| c == 0 || c == 1);
|
||||
@@ -406,10 +408,21 @@ async fn diff_against_sha(cwd: &Path, sha: &GitSha) -> Option<String> {
|
||||
.collect();
|
||||
|
||||
if !untracked.is_empty() {
|
||||
// Use platform-appropriate null device and guard paths with `--`.
|
||||
let null_device: &str = if cfg!(windows) { "NUL" } else { "/dev/null" };
|
||||
let futures_iter = untracked.into_iter().map(|file| async move {
|
||||
let file_owned = file;
|
||||
let args_vec: Vec<&str> =
|
||||
vec!["diff", "--binary", "--no-index", "/dev/null", &file_owned];
|
||||
let args_vec: Vec<&str> = vec![
|
||||
"diff",
|
||||
"--no-textconv",
|
||||
"--no-ext-diff",
|
||||
"--binary",
|
||||
"--no-index",
|
||||
// -- ensures that filenames that start with - are not treated as options.
|
||||
"--",
|
||||
null_device,
|
||||
&file_owned,
|
||||
];
|
||||
run_git_command_with_timeout(&args_vec, cwd).await
|
||||
});
|
||||
let results = join_all(futures_iter).await;
|
||||
|
||||
@@ -17,6 +17,10 @@ use crate::error::EnvVarError;
|
||||
const DEFAULT_STREAM_IDLE_TIMEOUT_MS: u64 = 300_000;
|
||||
const DEFAULT_STREAM_MAX_RETRIES: u64 = 5;
|
||||
const DEFAULT_REQUEST_MAX_RETRIES: u64 = 4;
|
||||
/// Hard cap for user-configured `stream_max_retries`.
|
||||
const MAX_STREAM_MAX_RETRIES: u64 = 100;
|
||||
/// Hard cap for user-configured `request_max_retries`.
|
||||
const MAX_REQUEST_MAX_RETRIES: u64 = 100;
|
||||
|
||||
/// Wire protocol that the provider speaks. Most third-party services only
|
||||
/// implement the classic OpenAI Chat Completions JSON schema, whereas OpenAI
|
||||
@@ -207,12 +211,14 @@ impl ModelProviderInfo {
|
||||
pub fn request_max_retries(&self) -> u64 {
|
||||
self.request_max_retries
|
||||
.unwrap_or(DEFAULT_REQUEST_MAX_RETRIES)
|
||||
.min(MAX_REQUEST_MAX_RETRIES)
|
||||
}
|
||||
|
||||
/// Effective maximum number of stream reconnection attempts for this provider.
|
||||
pub fn stream_max_retries(&self) -> u64 {
|
||||
self.stream_max_retries
|
||||
.unwrap_or(DEFAULT_STREAM_MAX_RETRIES)
|
||||
.min(MAX_STREAM_MAX_RETRIES)
|
||||
}
|
||||
|
||||
/// Effective idle timeout for streaming responses.
|
||||
|
||||
@@ -12,6 +12,7 @@ use codex_login::CodexAuth;
|
||||
use core_test_support::load_default_config_for_test;
|
||||
use core_test_support::load_sse_fixture_with_id;
|
||||
use core_test_support::wait_for_event;
|
||||
use serde_json::json;
|
||||
use tempfile::TempDir;
|
||||
use wiremock::Mock;
|
||||
use wiremock::MockServer;
|
||||
@@ -66,7 +67,6 @@ fn write_auth_json(
|
||||
account_id: Option<&str>,
|
||||
) -> String {
|
||||
use base64::Engine as _;
|
||||
use serde_json::json;
|
||||
|
||||
let header = json!({ "alg": "none", "typ": "JWT" });
|
||||
let payload = json!({
|
||||
@@ -746,3 +746,151 @@ async fn env_var_overrides_loaded_auth() {
|
||||
fn create_dummy_codex_auth() -> CodexAuth {
|
||||
CodexAuth::create_dummy_chatgpt_auth_for_testing()
|
||||
}
|
||||
|
||||
/// Scenario:
|
||||
/// - Turn 1: user sends U1; model streams deltas then a final assistant message A.
|
||||
/// - Turn 2: user sends U2; model streams a delta then the same final assistant message A.
|
||||
/// - Turn 3: user sends U3; model responds (same SSE again, not important).
|
||||
///
|
||||
/// We assert that the `input` sent on each turn contains the expected conversation history
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn history_dedupes_streamed_and_final_messages_across_turns() {
|
||||
// Skip under Codex sandbox network restrictions (mirrors other tests).
|
||||
if std::env::var(CODEX_SANDBOX_NETWORK_DISABLED_ENV_VAR).is_ok() {
|
||||
println!(
|
||||
"Skipping test because it cannot execute when network is disabled in a Codex sandbox."
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// Mock server that will receive three sequential requests and return the same SSE stream
|
||||
// each time: a few deltas, then a final assistant message, then completed.
|
||||
let server = MockServer::start().await;
|
||||
|
||||
// Build a small SSE stream with deltas and a final assistant message.
|
||||
// We emit the same body for all 3 turns; ids vary but are unused by assertions.
|
||||
let sse_raw = r##"[
|
||||
{"type":"response.output_text.delta", "delta":"Hey "},
|
||||
{"type":"response.output_text.delta", "delta":"there"},
|
||||
{"type":"response.output_text.delta", "delta":"!\n"},
|
||||
{"type":"response.output_item.done", "item":{
|
||||
"type":"message", "role":"assistant",
|
||||
"content":[{"type":"output_text","text":"Hey there!\n"}]
|
||||
}},
|
||||
{"type":"response.completed", "response": {"id": "__ID__"}}
|
||||
]"##;
|
||||
let sse1 = core_test_support::load_sse_fixture_with_id_from_str(sse_raw, "resp1");
|
||||
|
||||
Mock::given(method("POST"))
|
||||
.and(path("/v1/responses"))
|
||||
.respond_with(
|
||||
ResponseTemplate::new(200)
|
||||
.insert_header("content-type", "text/event-stream")
|
||||
.set_body_raw(sse1.clone(), "text/event-stream"),
|
||||
)
|
||||
.expect(3) // respond identically to the three sequential turns
|
||||
.mount(&server)
|
||||
.await;
|
||||
|
||||
// Configure provider to point to mock server (Responses API) and use API key auth.
|
||||
let model_provider = ModelProviderInfo {
|
||||
base_url: Some(format!("{}/v1", server.uri())),
|
||||
..built_in_model_providers()["openai"].clone()
|
||||
};
|
||||
|
||||
// Init session with isolated codex home.
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let mut config = load_default_config_for_test(&codex_home);
|
||||
config.model_provider = model_provider;
|
||||
|
||||
let conversation_manager =
|
||||
ConversationManager::with_auth(CodexAuth::from_api_key("Test API Key"));
|
||||
let NewConversation {
|
||||
conversation: codex,
|
||||
..
|
||||
} = conversation_manager
|
||||
.new_conversation(config)
|
||||
.await
|
||||
.expect("create new conversation");
|
||||
|
||||
// Turn 1: user sends U1; wait for completion.
|
||||
codex
|
||||
.submit(Op::UserInput {
|
||||
items: vec![InputItem::Text { text: "U1".into() }],
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
|
||||
|
||||
// Turn 2: user sends U2; wait for completion.
|
||||
codex
|
||||
.submit(Op::UserInput {
|
||||
items: vec![InputItem::Text { text: "U2".into() }],
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
|
||||
|
||||
// Turn 3: user sends U3; wait for completion.
|
||||
codex
|
||||
.submit(Op::UserInput {
|
||||
items: vec![InputItem::Text { text: "U3".into() }],
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
|
||||
|
||||
// Inspect the three captured requests.
|
||||
let requests = server.received_requests().await.unwrap();
|
||||
assert_eq!(requests.len(), 3, "expected 3 requests (one per turn)");
|
||||
|
||||
// Replace full-array compare with tail-only raw JSON compare using a single hard-coded value.
|
||||
let r3_tail_expected = serde_json::json!([
|
||||
{
|
||||
"type": "message",
|
||||
"id": null,
|
||||
"role": "user",
|
||||
"content": [{"type":"input_text","text":"U1"}]
|
||||
},
|
||||
{
|
||||
"type": "message",
|
||||
"id": null,
|
||||
"role": "assistant",
|
||||
"content": [{"type":"output_text","text":"Hey there!\n"}]
|
||||
},
|
||||
{
|
||||
"type": "message",
|
||||
"id": null,
|
||||
"role": "user",
|
||||
"content": [{"type":"input_text","text":"U2"}]
|
||||
},
|
||||
{
|
||||
"type": "message",
|
||||
"id": null,
|
||||
"role": "assistant",
|
||||
"content": [{"type":"output_text","text":"Hey there!\n"}]
|
||||
},
|
||||
{
|
||||
"type": "message",
|
||||
"id": null,
|
||||
"role": "user",
|
||||
"content": [{"type":"input_text","text":"U3"}]
|
||||
}
|
||||
]);
|
||||
|
||||
let r3_input_array = requests[2]
|
||||
.body_json::<serde_json::Value>()
|
||||
.unwrap()
|
||||
.get("input")
|
||||
.and_then(|v| v.as_array())
|
||||
.cloned()
|
||||
.expect("r3 missing input array");
|
||||
// skipping earlier context and developer messages
|
||||
let tail_len = r3_tail_expected.as_array().unwrap().len();
|
||||
let actual_tail = &r3_input_array[r3_input_array.len() - tail_len..];
|
||||
assert_eq!(
|
||||
serde_json::Value::Array(actual_tail.to_vec()),
|
||||
r3_tail_expected,
|
||||
"request 3 tail mismatch",
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user