Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions codex-rs/core/src/context/token_budget_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ pub(crate) struct TokenBudgetContext {
first_window_id: Uuid,
previous_window_id: Option<Uuid>,
window_id: Uuid,
tokens_left: i64,
mcp_result: Option<String>,
}

impl TokenBudgetContext {
Expand All @@ -17,14 +17,14 @@ impl TokenBudgetContext {
first_window_id: Uuid,
previous_window_id: Option<Uuid>,
window_id: Uuid,
tokens_left: i64,
mcp_result: Option<String>,
) -> Self {
Self {
thread_id,
first_window_id,
previous_window_id,
window_id,
tokens_left,
mcp_result,
}
}
}
Expand All @@ -49,9 +49,13 @@ impl ContextualUserFragment for TokenBudgetContext {
.previous_window_id
.map_or_else(|| "none".to_string(), |window_id| window_id.to_string());
let window_id = self.window_id;
let tokens_left = self.tokens_left;
let mcp_result = self
.mcp_result
.as_deref()
.map(|result| format!("\n{result}"))
.unwrap_or_default();
format!(
"Thread id {thread_id}.\nFirst context window id {first_window_id}.\nPrevious context window id {previous_window_id}.\nCurrent context window id {window_id}.\nYou have {tokens_left} tokens left in this context window."
"Thread id {thread_id}.\nFirst context window id {first_window_id}.\nPrevious context window id {previous_window_id}.\nCurrent context window id {window_id}.{mcp_result}"
Comment thread
pakrym-oai marked this conversation as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve the initial token budget in full context

When TokenBudget is enabled, the full-context fragment used to tell the model how many tokens were available at the start of each window; after this replacement it only includes window IDs unless later threshold reminders fire or the model explicitly calls get_context_remaining. On the first request in a fresh window the model no longer has the budget signal this feature is meant to provide, so keep the computed model_context_window text and append the MCP hint separately.

Useful? React with 👍 / 👎.

)
}
}
Expand Down
27 changes: 25 additions & 2 deletions codex-rs/core/src/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3216,15 +3216,38 @@ impl Session {
}
// This is full-context metadata. Steady-state context diffs should not re-emit it.
if turn_context.config.features.enabled(Feature::TokenBudget)
&& let Some(model_context_window) = turn_context.model_context_window()
&& turn_context.model_context_window().is_some()
{
let mcp_result = self
.call_tool(
"notes",
"thread_hint",
Comment on lines +3221 to +3224

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Mark memory polluted after background MCP hints

When memories.disable_on_external_context is enabled and mcp_history is a configured MCP server, normal MCP tool calls mark the thread's memory mode as polluted before using that external context, but this background call injects MCP-provided context without doing so. That leaves memory writes enabled for a thread that has already consumed external MCP context, so route this through the same pollution check used by model-initiated MCP calls before adding the hint.

Useful? React with 👍 / 👎.

/*arguments*/ None,
Some(serde_json::json!({
"threadId": self.thread_id().to_string(),
})),
Comment on lines +3226 to +3228

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include sandbox metadata in the hidden MCP call

For an mcp_history server that advertises the codex/sandbox-state-meta capability, normal MCP tool calls include the current sandbox cwd and permission state in request metadata, but this direct call sends only threadId. Such a server will see different metadata for this hidden thread_hint invocation and can fail or compute a hint for the wrong environment; build the request meta with the same sandbox-state augmentation used by regular MCP calls.

Useful? React with 👍 / 👎.

)
.await

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Bound the optional history lookup latency

When a configured mcp_history server is slow or still initializing, this awaits the optional thread_hint call before the first model request for every full-context window; the normal MCP startup/tool timeouts can be many seconds or minutes before .ok() discards the result. That makes user turns and compaction follow-ups appear hung due to an optional hint, so wrap this lookup in a short prototype-specific timeout or avoid blocking context construction on it.

Useful? React with 👍 / 👎.

.ok()
.and_then(|result| {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip MCP error results before injecting hints

If mcp_history/thread_hint returns an application-level MCP failure (is_error: true) with text content, call_tool still resolves successfully and this closure injects that error text as a thread hint. In cases like authorization or backend errors, the model receives a failure message inside developer context instead of omitting the optional hint; check result.is_error before reading content.

Useful? React with 👍 / 👎.

let text = result
.content
.iter()
.filter_map(|content| {
content.get("text").and_then(serde_json::Value::as_str)
})
.filter(|text| !text.is_empty())
.collect::<Vec<_>>()
.join("\n");
Comment on lines +3240 to +3241

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0 Badge Cap mcp_history text before injecting it

When a configured mcp_history/thread_hint returns a large hint, this code concatenates every text block and then stores the whole string in the developer context with no token or byte limit. That creates a new injected context fragment that can exceed the 1k-token manual-review threshold, or even the 10k-token hard limit, and can consume or overflow the model context before the turn starts; truncate or reject the hint before adding it to TokenBudgetContext.

AGENTS.md reference: AGENTS.md:L98-L100

Useful? React with 👍 / 👎.

(!text.is_empty()).then_some(text)
});
developer_sections.push(
crate::context::TokenBudgetContext::new(
self.thread_id(),
auto_compact_window_ids.first_window_id,
auto_compact_window_ids.previous_window_id,
auto_compact_window_ids.window_id,
model_context_window,
mcp_result,
)
.render(),
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
source: core/tests/suite/token_budget.rs
assertion_line: 539
assertion_line: 588
expression: snapshot
---
Scenario: New context window tool installs fresh full context before the next follow-up request.
Expand All @@ -9,7 +9,7 @@ Scenario: New context window tool installs fresh full context before the next fo
00:message/developer[3]:
[01] <PERMISSIONS_INSTRUCTIONS>
[02] <SKILLS_INSTRUCTIONS>
[03] <token_budget>\nThread id <THREAD_ID>.\nFirst context window id <FIRST_WINDOW_ID>.\nPrevious context window id <FIRST_WINDOW_ID>.\nCurrent context window id <WINDOW_ID>.\nYou have 121600 tokens left in this context window.\n</token_budget>
[03] <token_budget>\nThread id <THREAD_ID>.\nFirst context window id <FIRST_WINDOW_ID>.\nPrevious context window id <FIRST_WINDOW_ID>.\nCurrent context window id <WINDOW_ID>.\n</token_budget>
01:message/user:<ENVIRONMENT_CONTEXT:cwd=<CWD>>
02:function_call/update_plan
03:function_call_output:Plan updated
130 changes: 100 additions & 30 deletions codex-rs/core/tests/suite/token_budget.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
use anyhow::Result;
use codex_config::types::McpServerConfig;
use codex_config::types::McpServerTransportConfig;
use codex_features::Feature;
use codex_model_provider_info::built_in_model_providers;
use codex_protocol::protocol::EventMsg;
Expand All @@ -17,15 +19,18 @@ use core_test_support::responses::mount_sse_sequence;
use core_test_support::responses::sse;
use core_test_support::responses::start_mock_server;
use core_test_support::skip_if_no_network;
use core_test_support::stdio_server_bin;
use core_test_support::test_codex::local;
use core_test_support::test_codex::test_codex;
use core_test_support::wait_for_event;
use core_test_support::wait_for_mcp_server;
use pretty_assertions::assert_eq;
use serde_json::Value;
use serde_json::json;
use std::collections::HashMap;
use std::time::Duration;

const CONFIGURED_CONTEXT_WINDOW: i64 = 128_000;
const EFFECTIVE_CONTEXT_WINDOW: i64 = CONFIGURED_CONTEXT_WINDOW * 95 / 100;

fn token_budget_texts(request: &ResponsesRequest) -> Vec<String> {
request
Expand All @@ -38,11 +43,10 @@ fn token_budget_texts(request: &ResponsesRequest) -> Vec<String> {
fn token_budget_window_ids(
text: &str,
thread_id: codex_protocol::ThreadId,
tokens_left: i64,
) -> (String, Option<String>, String) {
let captures = assert_regex_match(
&format!(
r"^<token_budget>\nThread id {thread_id}\.\nFirst context window id ([0-9a-f-]{{36}})\.\nPrevious context window id (none|[0-9a-f-]{{36}})\.\nCurrent context window id ([0-9a-f-]{{36}})\.\nYou have {tokens_left} tokens left in this context window\.\n</token_budget>$"
r"^<token_budget>\nThread id {thread_id}\.\nFirst context window id ([0-9a-f-]{{36}})\.\nPrevious context window id (none|[0-9a-f-]{{36}})\.\nCurrent context window id ([0-9a-f-]{{36}})\.\n</token_budget>$"
),
text,
);
Expand Down Expand Up @@ -112,11 +116,8 @@ async fn token_budget_context_is_only_emitted_with_full_context() -> Result<()>
let thread_id = test.session_configured.thread_id;
let initial_token_budget = token_budget_texts(&requests[0]);
assert_eq!(initial_token_budget.len(), 1);
let (first_window_id, previous_window_id, window_id) = token_budget_window_ids(
&initial_token_budget[0],
thread_id,
EFFECTIVE_CONTEXT_WINDOW,
);
let (first_window_id, previous_window_id, window_id) =
token_budget_window_ids(&initial_token_budget[0], thread_id);
assert_eq!(previous_window_id, None);
assert_eq!(first_window_id, window_id);
assert_eq!(
Expand All @@ -128,6 +129,89 @@ async fn token_budget_context_is_only_emitted_with_full_context() -> Result<()>
Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn token_budget_context_injects_plain_thread_hint_text() -> Result<()> {
skip_if_no_network!(Ok(()));

let server = start_mock_server().await;
let rmcp_test_server_bin = stdio_server_bin()?;
let test = test_codex()
.with_config(move |config| {
config.model_context_window = Some(CONFIGURED_CONTEXT_WINDOW);
config
.features
.enable(Feature::TokenBudget)
.expect("test config should allow token budget");
let mut servers = config.mcp_servers.get().clone();
servers.insert(
"notes".to_string(),
McpServerConfig {
transport: McpServerTransportConfig::Stdio {
command: rmcp_test_server_bin,
args: Vec::new(),
env: None,
env_vars: Vec::new(),
cwd: None,
},
environment_id: "local".to_string(),
enabled: true,
required: false,
supports_parallel_tool_calls: false,
disabled_reason: None,
startup_timeout_sec: Some(Duration::from_secs(10)),
tool_timeout_sec: None,
default_tools_approval_mode: None,
enabled_tools: None,
disabled_tools: None,
scopes: None,
oauth: None,
oauth_resource: None,
tools: HashMap::new(),
},
);
config
.mcp_servers
.set(servers)
.expect("test mcp servers should accept any configuration");
})
.build(&server)
.await?;
wait_for_mcp_server(&test.codex, "notes").await?;
let responses = mount_sse_sequence(
&server,
vec![sse(vec![
ev_response_created("resp-1"),
ev_completed("resp-1"),
])],
)
.await;

test.submit_turn("inject the history hint").await?;

let request = responses.single_request();
let thread_id = test.session_configured.thread_id;
let token_budgets = token_budget_texts(&request);
assert_eq!(token_budgets.len(), 1);
let captures = assert_regex_match(
&format!(
r"^<token_budget>\nThread id {thread_id}\.\nFirst context window id ([0-9a-f-]{{36}})\.\nPrevious context window id none\.\nCurrent context window id ([0-9a-f-]{{36}})\.\nmanual history hint for thread {thread_id}\nunstructured notes/thread_hint fixture result\n</token_budget>$"
),
&token_budgets[0],
);
assert_eq!(
captures.get(1).expect("first window id capture").as_str(),
captures.get(2).expect("current window id capture").as_str()
);
assert!(
!tool_names(&request)
.iter()
.any(|name| name == "mcp__notes__thread_hint"),
"thread_hint should be hidden from model tool exposure"
);

Ok(())
}

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn token_budget_remaining_context_emits_on_first_threshold_crossing() -> Result<()> {
skip_if_no_network!(Ok(()));
Expand Down Expand Up @@ -177,7 +261,7 @@ async fn token_budget_remaining_context_emits_on_first_threshold_crossing() -> R
let thread_id = test.session_configured.thread_id;
let full_context = token_budget_texts(&requests[0]);
assert_eq!(full_context.len(), 1);
token_budget_window_ids(&full_context[0], thread_id, /*tokens_left*/ 9_500);
token_budget_window_ids(&full_context[0], thread_id);
let full_context = full_context[0].clone();
let threshold_25 =
"<token_budget>\nYou have 7000 tokens left in this context window.\n</token_budget>"
Expand Down Expand Up @@ -270,7 +354,7 @@ async fn get_context_remaining_returns_token_budget_remaining_fragment() -> Resu
.to_string();
let token_budgets = token_budget_texts(&requests[1]);
assert_eq!(token_budgets.len(), 2);
token_budget_window_ids(&token_budgets[0], thread_id, /*tokens_left*/ 9_500);
token_budget_window_ids(&token_budgets[0], thread_id);
assert_eq!(token_budgets[1], remaining_context);
assert_eq!(
requests[2].function_call_output_content_and_success(call_id),
Expand Down Expand Up @@ -394,22 +478,14 @@ async fn token_budget_context_uses_new_window_after_compaction() -> Result<()> {
let initial_token_budget = token_budget_texts(&requests[0]);
assert_eq!(initial_token_budget.len(), 1);
let (initial_first_window_id, initial_previous_window_id, initial_window_id) =
token_budget_window_ids(
&initial_token_budget[0],
thread_id,
EFFECTIVE_CONTEXT_WINDOW,
);
token_budget_window_ids(&initial_token_budget[0], thread_id);
let post_compaction_token_budget = token_budget_texts(&requests[2]);
assert_eq!(post_compaction_token_budget.len(), 1);
let (
post_compaction_first_window_id,
post_compaction_previous_window_id,
post_compaction_window_id,
) = token_budget_window_ids(
&post_compaction_token_budget[0],
thread_id,
EFFECTIVE_CONTEXT_WINDOW,
);
) = token_budget_window_ids(&post_compaction_token_budget[0], thread_id);
assert_eq!(initial_previous_window_id, None);
assert_eq!(initial_first_window_id, initial_window_id);
assert_eq!(post_compaction_first_window_id, initial_first_window_id);
Expand Down Expand Up @@ -480,18 +556,12 @@ async fn new_context_tool_starts_new_window_before_follow_up() -> Result<()> {
let thread_id = test.session_configured.thread_id;
let initial_token_budget = token_budget_texts(&requests[0]);
assert_eq!(initial_token_budget.len(), 1);
let (initial_first_window_id, _, initial_window_id) = token_budget_window_ids(
&initial_token_budget[0],
thread_id,
EFFECTIVE_CONTEXT_WINDOW,
);
let (initial_first_window_id, _, initial_window_id) =
token_budget_window_ids(&initial_token_budget[0], thread_id);
let new_window_token_budget = token_budget_texts(&requests[2]);
assert_eq!(new_window_token_budget.len(), 1);
let (new_first_window_id, new_previous_window_id, new_window_id) = token_budget_window_ids(
&new_window_token_budget[0],
thread_id,
EFFECTIVE_CONTEXT_WINDOW,
);
let (new_first_window_id, new_previous_window_id, new_window_id) =
token_budget_window_ids(&new_window_token_budget[0], thread_id);
assert_eq!(new_first_window_id, initial_first_window_id);
assert_eq!(
new_previous_window_id.as_deref(),
Expand Down
35 changes: 35 additions & 0 deletions codex-rs/rmcp-client/src/bin/test_stdio_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use rmcp::model::JsonObject;
use rmcp::model::ListResourceTemplatesResult;
use rmcp::model::ListResourcesResult;
use rmcp::model::ListToolsResult;
use rmcp::model::Meta;
use rmcp::model::PaginatedRequestParams;
use rmcp::model::RawResource;
use rmcp::model::RawResourceTemplate;
Expand Down Expand Up @@ -70,9 +71,27 @@ impl TestToolServer {
);
sandbox_meta_tool.annotations = Some(ToolAnnotations::new().read_only(true));

#[expect(clippy::expect_used)]
let thread_hint_schema: JsonObject = serde_json::from_value(json!({
"type": "object",
"properties": {},
"additionalProperties": false
}))
.expect("thread_hint tool schema should deserialize");
let mut thread_hint_tool = Tool::new(
Cow::Borrowed("thread_hint"),
Cow::Borrowed("Return an unstructured history hint for a thread."),
Arc::new(thread_hint_schema),
);
thread_hint_tool.annotations = Some(ToolAnnotations::new().read_only(true));
let mut thread_hint_meta = Meta::new();
thread_hint_meta.insert("ui".to_string(), json!({ "visibility": [] }));
thread_hint_tool.meta = Some(thread_hint_meta);

let tools = vec![
Self::echo_tool(),
Self::echo_dash_tool(),
thread_hint_tool,
Self::client_capabilities_tool(),
Self::cwd_tool(),
Self::sync_tool(),
Expand Down Expand Up @@ -537,6 +556,22 @@ impl ServerHandler for TestToolServer {
.map_err(|err| McpError::internal_error(err.to_string(), None))?;
Ok(Self::structured_result(json!({ "cwd": cwd })))
}
"thread_hint" => {
let thread_id = context
.meta
.0
.get("threadId")
.and_then(serde_json::Value::as_str)
.ok_or_else(|| {
McpError::invalid_params("missing threadId metadata".to_string(), None)
})?;
Ok(CallToolResult::success(vec![
rmcp::model::Content::text(format!(
"manual history hint for thread {thread_id}"
)),
rmcp::model::Content::text("unstructured notes/thread_hint fixture result"),
]))
}
"echo" | "echo-tool" => {
let args: EchoArgs = match request.arguments {
Some(arguments) => serde_json::from_value(serde_json::Value::Object(
Expand Down
Loading