[codex] prototype mcp_history thread hint injection#29259
Conversation
84f75b3 to
ace5d9c
Compare
pakrym-oai
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b496dfeca3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .collect::<Vec<_>>() | ||
| .join("\n"); |
There was a problem hiding this comment.
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 👍 / 👎.
| .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}" |
There was a problem hiding this comment.
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 👍 / 👎.
| ) | ||
| .await | ||
| .ok() | ||
| .and_then(|result| { |
There was a problem hiding this comment.
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 👍 / 👎.
| "threadId": self.thread_id().to_string(), | ||
| })), | ||
| ) | ||
| .await |
There was a problem hiding this comment.
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 👍 / 👎.
| let mcp_result = self | ||
| .call_tool( | ||
| "mcp_history", | ||
| "thread_hint", |
There was a problem hiding this comment.
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 👍 / 👎.
| Some(serde_json::json!({ | ||
| "threadId": self.thread_id().to_string(), | ||
| })), |
There was a problem hiding this comment.
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 👍 / 👎.
1 participant
Why
Prototype whether the harness can invoke the
mcp_historyMCP while constructing full initial context and expose its thread hint to the model without requiring a model-issued tool call.The prototype builds on the context-window lineage added by #29256 and is now based directly on
main.What changed
mcp_history/thread_hintwith no arguments while building the full<token_budget>context.threadIdthrough MCP request metadata, matching the normal MCP tool-call path.contentresult and append it inside<token_budget>when the call succeeds.Prototype limitations
Validation
just test -p codex-core token_budget