Merge origin/main and keep V8 exec runtime behavior

This commit is contained in:
pakrym-oai
2026-03-10 19:29:53 -07:00
50 changed files with 1409 additions and 433 deletions
@@ -3015,10 +3015,16 @@
"description": "Identifier for the collab tool call.",
"type": "string"
},
"model": {
"type": "string"
},
"prompt": {
"description": "Initial prompt sent to the agent. Can be empty to prevent CoT leaking at the beginning.",
"type": "string"
},
"reasoning_effort": {
"$ref": "#/definitions/ReasoningEffort"
},
"sender_thread_id": {
"allOf": [
{
@@ -3037,7 +3043,9 @@
},
"required": [
"call_id",
"model",
"prompt",
"reasoning_effort",
"sender_thread_id",
"type"
],
@@ -9144,10 +9152,16 @@
"description": "Identifier for the collab tool call.",
"type": "string"
},
"model": {
"type": "string"
},
"prompt": {
"description": "Initial prompt sent to the agent. Can be empty to prevent CoT leaking at the beginning.",
"type": "string"
},
"reasoning_effort": {
"$ref": "#/definitions/ReasoningEffort"
},
"sender_thread_id": {
"allOf": [
{
@@ -9166,7 +9180,9 @@
},
"required": [
"call_id",
"model",
"prompt",
"reasoning_effort",
"sender_thread_id",
"type"
],
@@ -4378,10 +4378,16 @@
"description": "Identifier for the collab tool call.",
"type": "string"
},
"model": {
"type": "string"
},
"prompt": {
"description": "Initial prompt sent to the agent. Can be empty to prevent CoT leaking at the beginning.",
"type": "string"
},
"reasoning_effort": {
"$ref": "#/definitions/v2/ReasoningEffort"
},
"sender_thread_id": {
"allOf": [
{
@@ -4400,7 +4406,9 @@
},
"required": [
"call_id",
"model",
"prompt",
"reasoning_effort",
"sender_thread_id",
"type"
],
@@ -6180,10 +6180,16 @@
"description": "Identifier for the collab tool call.",
"type": "string"
},
"model": {
"type": "string"
},
"prompt": {
"description": "Initial prompt sent to the agent. Can be empty to prevent CoT leaking at the beginning.",
"type": "string"
},
"reasoning_effort": {
"$ref": "#/definitions/ReasoningEffort"
},
"sender_thread_id": {
"allOf": [
{
@@ -6202,7 +6208,9 @@
},
"required": [
"call_id",
"model",
"prompt",
"reasoning_effort",
"sender_thread_id",
"type"
],
@@ -1,6 +1,7 @@
// GENERATED CODE! DO NOT MODIFY BY HAND!
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { ReasoningEffort } from "./ReasoningEffort";
import type { ThreadId } from "./ThreadId";
export type CollabAgentSpawnBeginEvent = {
@@ -16,4 +17,4 @@ sender_thread_id: ThreadId,
* Initial prompt sent to the agent. Can be empty to prevent CoT leaking at the
* beginning.
*/
prompt: string, };
prompt: string, model: string, reasoning_effort: ReasoningEffort, };
+39 -19
View File
@@ -67,34 +67,22 @@ pub fn execute(
let scope = pin!(v8::TryCatch::new(scope));
let scope = &mut scope.init();
let module = compile_module(scope, CODE_MODE_MAIN_FILENAME, &code)?;
let Some(instantiated) = module.instantiate_module(scope, resolve_code_mode_module) else {
return Err(format_v8_exception(scope));
};
if !instantiated {
return Err("failed to instantiate code_mode module".to_string());
}
let Some(result) = module.evaluate(scope) else {
return Err(format_v8_exception(scope));
};
if result.is_promise() {
let promise = v8::Local::<v8::Promise>::try_from(result)
.map_err(|_| "code_mode module evaluation did not return a promise".to_string())?;
wait_for_module_promise(scope, module, promise)?;
} else {
scope.perform_microtask_checkpoint();
}
let execution_outcome = execute_main_module(scope, &code);
let content_items = read_content_items(scope)?;
let stored_values = read_stored_values(scope)?;
let Some(runtime_state) = scope.get_slot::<RuntimeState>() else {
return Err("code_mode runtime state missing".to_string());
};
let (success, error_text) = match execution_outcome {
Ok(()) => (true, None),
Err(error_text) => (false, Some(error_text)),
};
Ok(ExecutionResult {
content_items,
stored_values,
max_output_tokens_per_exec_call: runtime_state.max_output_tokens_per_exec_call,
success,
error_text,
})
}
@@ -174,6 +162,38 @@ fn compile_module<'s>(
.ok_or_else(|| "failed to compile code_mode module".to_string())
}
fn execute_main_module(
try_catch: &mut v8::PinnedRef<'_, v8::TryCatch<v8::HandleScope>>,
code: &str,
) -> Result<(), String> {
let source = v8_string(try_catch, code)?;
let identifier = v8_string(try_catch, CODE_MODE_MAIN_FILENAME)?;
let origin = script_origin(try_catch, identifier, true);
let mut source = v8::script_compiler::Source::new(source, Some(&origin));
let Some(module) = v8::script_compiler::compile_module(try_catch, &mut source) else {
return Err(format_v8_exception(try_catch));
};
let Some(instantiated) = module.instantiate_module(try_catch, resolve_code_mode_module) else {
return Err(format_v8_exception(try_catch));
};
if !instantiated {
return Err("failed to instantiate code_mode module".to_string());
}
let Some(result) = module.evaluate(try_catch) else {
return Err(format_v8_exception(try_catch));
};
if result.is_promise() {
let promise = v8::Local::<v8::Promise>::try_from(result)
.map_err(|_| "code_mode module evaluation did not return a promise".to_string())?;
wait_for_module_promise(try_catch, module, promise)?;
} else {
try_catch.perform_microtask_checkpoint();
}
Ok(())
}
fn create_tools_module<'s>(
scope: &mut v8::PinScope<'s, '_>,
enabled_tools: &[EnabledTool],
+2
View File
@@ -26,6 +26,8 @@ pub struct ExecutionResult {
pub content_items: Vec<JsonValue>,
pub stored_values: HashMap<String, JsonValue>,
pub max_output_tokens_per_exec_call: usize,
pub success: bool,
pub error_text: Option<String>,
}
const MUSL_UNSUPPORTED_REASON: &str = "code_mode is unavailable on musl Linux";
@@ -53,9 +53,6 @@ enum WsCommand {
message: Message,
tx_result: oneshot::Sender<Result<(), WsError>>,
},
Close {
tx_result: oneshot::Sender<Result<(), WsError>>,
},
}
impl WsStream {
@@ -80,11 +77,6 @@ impl WsStream {
break;
}
}
WsCommand::Close { tx_result } => {
let result = inner.close(None).await;
let _ = tx_result.send(result);
break;
}
}
}
message = inner.next() => {
@@ -144,11 +136,6 @@ impl WsStream {
.await
}
async fn close(&self) -> Result<(), WsError> {
self.request(|tx_result| WsCommand::Close { tx_result })
.await
}
async fn next(&mut self) -> Option<Result<Message, WsError>> {
self.rx_message.recv().await
}
@@ -242,26 +229,32 @@ impl ResponsesWebsocketConnection {
.await;
}
let mut guard = stream.lock().await;
let Some(ws_stream) = guard.as_mut() else {
let _ = tx_event
.send(Err(ApiError::Stream(
"websocket connection is closed".to_string(),
)))
.await;
return;
let result = {
let Some(ws_stream) = guard.as_mut() else {
let _ = tx_event
.send(Err(ApiError::Stream(
"websocket connection is closed".to_string(),
)))
.await;
return;
};
run_websocket_response_stream(
ws_stream,
tx_event.clone(),
request_body,
idle_timeout,
telemetry,
)
.await
};
if let Err(err) = run_websocket_response_stream(
ws_stream,
tx_event.clone(),
request_body,
idle_timeout,
telemetry,
)
.await
{
let _ = ws_stream.close().await;
*guard = None;
if let Err(err) = result {
// A terminal stream error should reach the caller immediately. Waiting for a
// graceful close handshake here can stall indefinitely and mask the error.
let failed_stream = guard.take();
drop(guard);
drop(failed_stream);
let _ = tx_event.send(Err(err)).await;
}
});
+10
View File
@@ -459,6 +459,9 @@
"skill_mcp_dependency_install": {
"type": "boolean"
},
"spawn_csv": {
"type": "boolean"
},
"sqlite": {
"type": "boolean"
},
@@ -1788,6 +1791,10 @@
"experimental_compact_prompt_file": {
"$ref": "#/definitions/AbsolutePathBuf"
},
"experimental_realtime_start_instructions": {
"description": "Experimental / do not use. Replaces the built-in realtime start instructions inserted into developer messages when realtime becomes active.",
"type": "string"
},
"experimental_realtime_ws_backend_prompt": {
"description": "Experimental / do not use. Overrides only the realtime conversation websocket transport instructions (the `Op::RealtimeConversation` `/ws` session.update instructions) without changing normal prompts.",
"type": "string"
@@ -1953,6 +1960,9 @@
"skill_mcp_dependency_install": {
"type": "boolean"
},
"spawn_csv": {
"type": "boolean"
},
"sqlite": {
"type": "boolean"
},
+1
View File
@@ -385,6 +385,7 @@ impl Codex {
if let SessionSource::SubAgent(SubAgentSource::ThreadSpawn { depth, .. }) = session_source
&& depth >= config.agent_max_depth
{
let _ = config.features.disable(Feature::SpawnCsv);
let _ = config.features.disable(Feature::Collab);
}
+32
View File
@@ -3965,6 +3965,7 @@ fn test_precedence_fixture_with_o3_profile() -> std::io::Result<()> {
personality: Some(Personality::Pragmatic),
chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(),
realtime_audio: RealtimeAudioConfig::default(),
experimental_realtime_start_instructions: None,
experimental_realtime_ws_base_url: None,
experimental_realtime_ws_model: None,
experimental_realtime_ws_backend_prompt: None,
@@ -4100,6 +4101,7 @@ fn test_precedence_fixture_with_gpt3_profile() -> std::io::Result<()> {
personality: Some(Personality::Pragmatic),
chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(),
realtime_audio: RealtimeAudioConfig::default(),
experimental_realtime_start_instructions: None,
experimental_realtime_ws_base_url: None,
experimental_realtime_ws_model: None,
experimental_realtime_ws_backend_prompt: None,
@@ -4233,6 +4235,7 @@ fn test_precedence_fixture_with_zdr_profile() -> std::io::Result<()> {
personality: Some(Personality::Pragmatic),
chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(),
realtime_audio: RealtimeAudioConfig::default(),
experimental_realtime_start_instructions: None,
experimental_realtime_ws_base_url: None,
experimental_realtime_ws_model: None,
experimental_realtime_ws_backend_prompt: None,
@@ -4352,6 +4355,7 @@ fn test_precedence_fixture_with_gpt5_profile() -> std::io::Result<()> {
personality: Some(Personality::Pragmatic),
chatgpt_base_url: "https://chatgpt.com/backend-api/".to_string(),
realtime_audio: RealtimeAudioConfig::default(),
experimental_realtime_start_instructions: None,
experimental_realtime_ws_base_url: None,
experimental_realtime_ws_model: None,
experimental_realtime_ws_backend_prompt: None,
@@ -5261,6 +5265,34 @@ async fn feature_requirements_reject_legacy_aliases() {
);
}
#[test]
fn experimental_realtime_start_instructions_load_from_config_toml() -> std::io::Result<()> {
let cfg: ConfigToml = toml::from_str(
r#"
experimental_realtime_start_instructions = "start instructions from config"
"#,
)
.expect("TOML deserialization should succeed");
assert_eq!(
cfg.experimental_realtime_start_instructions.as_deref(),
Some("start instructions from config")
);
let codex_home = TempDir::new()?;
let config = Config::load_from_base_config_with_overrides(
cfg,
ConfigOverrides::default(),
codex_home.path().to_path_buf(),
)?;
assert_eq!(
config.experimental_realtime_start_instructions.as_deref(),
Some("start instructions from config")
);
Ok(())
}
#[test]
fn experimental_realtime_ws_base_url_loads_from_config_toml() -> std::io::Result<()> {
let cfg: ConfigToml = toml::from_str(
+9
View File
@@ -470,6 +470,10 @@ pub struct Config {
/// context appended to websocket session instructions. An empty string
/// disables startup context injection entirely.
pub experimental_realtime_ws_startup_context: Option<String>,
/// Experimental / do not use. Replaces the built-in realtime start
/// instructions inserted into developer messages when realtime becomes
/// active.
pub experimental_realtime_start_instructions: Option<String>,
/// When set, restricts ChatGPT login to a specific workspace identifier.
pub forced_chatgpt_workspace_id: Option<String>,
@@ -1241,6 +1245,10 @@ pub struct ConfigToml {
/// context appended to websocket session instructions. An empty string
/// disables startup context injection entirely.
pub experimental_realtime_ws_startup_context: Option<String>,
/// Experimental / do not use. Replaces the built-in realtime start
/// instructions inserted into developer messages when realtime becomes
/// active.
pub experimental_realtime_start_instructions: Option<String>,
pub projects: Option<HashMap<String, ProjectConfig>>,
/// Controls the web search tool mode: disabled, cached, or live.
@@ -2426,6 +2434,7 @@ impl Config {
experimental_realtime_ws_model: cfg.experimental_realtime_ws_model,
experimental_realtime_ws_backend_prompt: cfg.experimental_realtime_ws_backend_prompt,
experimental_realtime_ws_startup_context: cfg.experimental_realtime_ws_startup_context,
experimental_realtime_start_instructions: cfg.experimental_realtime_start_instructions,
forced_chatgpt_workspace_id,
forced_login_method,
include_apply_patch_tool: include_apply_patch_tool_flag,
+11 -1
View File
@@ -75,7 +75,17 @@ pub(crate) fn build_realtime_update_item(
next.realtime_active,
) {
(Some(true), false) => Some(DeveloperInstructions::realtime_end_message("inactive")),
(Some(false), true) | (None, true) => Some(DeveloperInstructions::realtime_start_message()),
(Some(false), true) | (None, true) => Some(
if let Some(instructions) = next
.config
.experimental_realtime_start_instructions
.as_deref()
{
DeveloperInstructions::realtime_start_message_with_instructions(instructions)
} else {
DeveloperInstructions::realtime_start_message()
},
),
(Some(true), true) | (Some(false), false) => None,
(None, false) => previous_turn_settings
.and_then(|settings| settings.realtime_active)
+32
View File
@@ -138,6 +138,8 @@ pub enum Feature {
EnableRequestCompression,
/// Enable collab tools.
Collab,
/// Enable CSV-backed agent job tools.
SpawnCsv,
/// Enable apps.
Apps,
/// Enable plugins.
@@ -414,6 +416,9 @@ impl Features {
}
pub(crate) fn normalize_dependencies(&mut self) {
if self.enabled(Feature::SpawnCsv) && !self.enabled(Feature::Collab) {
self.enable(Feature::Collab);
}
if self.enabled(Feature::JsReplToolsOnly) && !self.enabled(Feature::JsRepl) {
tracing::warn!("js_repl_tools_only requires js_repl; disabling js_repl_tools_only");
self.disable(Feature::JsReplToolsOnly);
@@ -693,6 +698,12 @@ pub const FEATURES: &[FeatureSpec] = &[
},
default_enabled: false,
},
FeatureSpec {
id: Feature::SpawnCsv,
key: "spawn_csv",
stage: Stage::UnderDevelopment,
default_enabled: false,
},
FeatureSpec {
id: Feature::Apps,
key: "apps",
@@ -997,6 +1008,27 @@ mod tests {
assert_eq!(feature_for_key("collab"), Some(Feature::Collab));
}
#[test]
fn spawn_csv_is_under_development() {
assert_eq!(Feature::SpawnCsv.stage(), Stage::UnderDevelopment);
assert_eq!(Feature::SpawnCsv.default_enabled(), false);
}
#[test]
fn spawn_csv_normalization_enables_multi_agent_one_way() {
let mut spawn_csv_features = Features::with_defaults();
spawn_csv_features.enable(Feature::SpawnCsv);
spawn_csv_features.normalize_dependencies();
assert_eq!(spawn_csv_features.enabled(Feature::SpawnCsv), true);
assert_eq!(spawn_csv_features.enabled(Feature::Collab), true);
let mut collab_features = Features::with_defaults();
collab_features.enable(Feature::Collab);
collab_features.normalize_dependencies();
assert_eq!(collab_features.enabled(Feature::Collab), true);
assert_eq!(collab_features.enabled(Feature::SpawnCsv), false);
}
#[test]
fn apps_require_feature_flag_and_chatgpt_auth() {
let mut features = Features::with_defaults();
+1
View File
@@ -687,6 +687,7 @@ fn build_guardian_subagent_config(
)?);
}
for feature in [
Feature::SpawnCsv,
Feature::Collab,
Feature::WebSearchRequest,
Feature::WebSearchCached,
+1
View File
@@ -270,6 +270,7 @@ mod agent {
// Approval policy
agent_config.permissions.approval_policy = Constrained::allow_only(AskForApproval::Never);
// Consolidation runs as an internal sub-agent and must not recursively delegate.
let _ = agent_config.features.disable(Feature::SpawnCsv);
let _ = agent_config.features.disable(Feature::Collab);
// Sandbox policy
+1
View File
@@ -100,6 +100,7 @@ async fn start_review_conversation(
{
panic!("by construction Constrained<WebSearchMode> must always support Disabled: {err}");
}
let _ = sub_agent_config.features.disable(Feature::SpawnCsv);
let _ = sub_agent_config.features.disable(Feature::Collab);
// Set explicit review rubric for the sub-agent
+39 -17
View File
@@ -1,5 +1,6 @@
use std::collections::HashMap;
use std::sync::Arc;
use std::time::Duration;
use crate::client_common::tools::ToolSpec;
use crate::codex::Session;
@@ -8,6 +9,7 @@ use crate::config::Config;
use crate::features::Feature;
use crate::function_tool::FunctionCallError;
use crate::tools::ToolRouter;
use crate::tools::context::FunctionToolOutput;
use crate::tools::context::SharedTurnDiffTracker;
use crate::tools::context::ToolPayload;
use crate::tools::router::ToolCall;
@@ -50,7 +52,7 @@ pub(crate) fn instructions(config: &Config) -> Option<String> {
));
section.push_str("- Import nested tools from `tools.js`, for example `import { exec_command } from \"tools.js\"` or `import { tools } from \"tools.js\"`. Namespaced tools are also available from `tools/<namespace...>.js`; MCP tools use `tools/mcp/<server>.js`, for example `import { append_notebook_logs_chart } from \"tools/mcp/ologs.js\"`. `tools[name]` and identifier wrappers like `await exec_command(args)` remain available for compatibility. Nested tool calls resolve to their code-mode result values.\n");
section.push_str(&format!(
"- Import `{{ output_text, output_image, set_max_output_tokens_per_exec_call, store, load }}` from `@openai/code_mode` (or `\"openai/code_mode\"`). `output_text(value)` surfaces text back to the model and stringifies non-string objects with `JSON.stringify(...)` when possible. `output_image(imageUrl)` appends an `input_image` content item for `http(s)` or `data:` URLs. `store(key, value)` persists JSON-serializable values across `{PUBLIC_TOOL_NAME}` calls in the current session, and `load(key)` returns a cloned stored value or `undefined`. `set_max_output_tokens_per_exec_call(value)` sets the token budget used to truncate the final Rust-side result of the current `{PUBLIC_TOOL_NAME}` execution; the default is `10000`. This guards the overall `{PUBLIC_TOOL_NAME}` output, not individual nested tool invocations. When truncation happens, the final text uses the unified-exec style `Original token count:` / `Output:` wrapper and the usual `…N tokens truncated…` marker.\n",
"- Import `{{ output_text, output_image, set_max_output_tokens_per_exec_call, store, load }}` from `@openai/code_mode` (or `\"openai/code_mode\"`). `output_text(value)` surfaces text back to the model and stringifies non-string objects with `JSON.stringify(...)` when possible. `output_image(imageUrl)` appends an `input_image` content item for `http(s)` or `data:` URLs. `store(key, value)` persists JSON-serializable values across `{PUBLIC_TOOL_NAME}` calls in the current session, and `load(key)` returns a cloned stored value or `undefined`. `set_max_output_tokens_per_exec_call(value)` sets the token budget used to truncate the final Rust-side result of the current `{PUBLIC_TOOL_NAME}` execution; the default is `10000`. This guards the overall `{PUBLIC_TOOL_NAME}` output, not individual nested tool invocations. The returned content starts with a separate `Script completed` or `Script failed` text item that includes wall time. When truncation happens, the final text may include `Total output lines:` and the usual `…N tokens truncated…` marker.\n",
));
section.push_str(
"- Function tools require JSON object arguments. Freeform tools require raw strings.\n",
@@ -66,7 +68,7 @@ pub(crate) async fn execute(
turn: Arc<TurnContext>,
tracker: SharedTurnDiffTracker,
code: String,
) -> Result<Vec<FunctionCallOutputContentItem>, FunctionCallError> {
) -> Result<FunctionToolOutput, FunctionCallError> {
let exec = ExecContext {
session,
turn,
@@ -74,6 +76,7 @@ pub(crate) async fn execute(
};
let enabled_tools = build_enabled_tools(&exec).await;
let stored_values = exec.session.services.code_mode_store.stored_values().await;
let started_at = std::time::Instant::now();
let callback_exec = exec.clone();
let result = execute_code_mode(
code,
@@ -87,38 +90,57 @@ pub(crate) async fn execute(
.code_mode_store
.replace_stored_values(result.stored_values)
.await;
let items = output_content_items_from_json_values(result.content_items)
let mut items = output_content_items_from_json_values(result.content_items)
.map_err(FunctionCallError::RespondToModel)?;
Ok(truncate_code_mode_result(
if !result.success {
let error_text = result
.error_text
.unwrap_or_else(|| "JavaScript execution failed".to_string());
items.push(FunctionCallOutputContentItem::InputText {
text: format!("Script error:\n{error_text}"),
});
}
let mut items = truncate_code_mode_result(items, Some(result.max_output_tokens_per_exec_call));
prepend_script_status(&mut items, result.success, started_at.elapsed());
Ok(FunctionToolOutput::from_content(
items,
Some(result.max_output_tokens_per_exec_call),
Some(result.success),
))
}
fn prepend_script_status(
content_items: &mut Vec<FunctionCallOutputContentItem>,
success: bool,
wall_time: Duration,
) {
let wall_time_seconds = ((wall_time.as_secs_f32()) * 10.0).round() / 10.0;
let header = format!(
"{}\nWall time {wall_time_seconds:.1} seconds\nOutput:\n",
if success {
"Script completed"
} else {
"Script failed"
}
);
content_items.insert(0, FunctionCallOutputContentItem::InputText { text: header });
}
fn truncate_code_mode_result(
items: Vec<FunctionCallOutputContentItem>,
max_output_tokens_per_exec_call: Option<usize>,
) -> Vec<FunctionCallOutputContentItem> {
let max_output_tokens = resolve_max_tokens(max_output_tokens_per_exec_call);
let policy = TruncationPolicy::Tokens(max_output_tokens);
if items
.iter()
.all(|item| matches!(item, FunctionCallOutputContentItem::InputText { .. }))
{
let (mut truncated_items, original_token_count) =
formatted_truncate_text_content_items_with_policy(
&items,
TruncationPolicy::Tokens(max_output_tokens),
);
if let Some(original_token_count) = original_token_count
&& let Some(FunctionCallOutputContentItem::InputText { text }) =
truncated_items.first_mut()
{
*text = format!("Original token count: {original_token_count}\nOutput:\n{text}");
}
let (truncated_items, _) =
formatted_truncate_text_content_items_with_policy(&items, policy);
return truncated_items;
}
truncate_function_output_items_with_policy(&items, TruncationPolicy::Tokens(max_output_tokens))
truncate_function_output_items_with_policy(&items, policy)
}
async fn build_enabled_tools(exec: &ExecContext) -> Vec<EnabledTool> {
+3 -3
View File
@@ -159,7 +159,7 @@ pub struct ExecCommandToolOutput {
/// Raw bytes returned for this unified exec call before any truncation.
pub raw_output: Vec<u8>,
pub max_output_tokens: Option<usize>,
pub process_id: Option<String>,
pub process_id: Option<i32>,
pub exit_code: Option<i32>,
pub original_token_count: Option<usize>,
pub session_command: Option<Vec<String>>,
@@ -194,7 +194,7 @@ impl ToolOutput for ExecCommandToolOutput {
#[serde(skip_serializing_if = "Option::is_none")]
exit_code: Option<i32>,
#[serde(skip_serializing_if = "Option::is_none")]
session_id: Option<String>,
session_id: Option<i32>,
#[serde(skip_serializing_if = "Option::is_none")]
original_token_count: Option<usize>,
output: String,
@@ -204,7 +204,7 @@ impl ToolOutput for ExecCommandToolOutput {
chunk_id: (!self.chunk_id.is_empty()).then(|| self.chunk_id.clone()),
wall_time_seconds: self.wall_time.as_secs_f64(),
exit_code: self.exit_code,
session_id: self.process_id.clone(),
session_id: self.process_id,
original_token_count: self.original_token_count,
output: self.truncated_output(),
};
@@ -51,7 +51,6 @@ impl ToolHandler for CodeModeHandler {
}
};
let content_items = code_mode::execute(session, turn, tracker, code).await?;
Ok(FunctionToolOutput::from_content(content_items, Some(true)))
code_mode::execute(session, turn, tracker, code).await
}
}
@@ -157,6 +157,8 @@ mod spawn {
call_id: call_id.clone(),
sender_thread_id: session.conversation_id,
prompt: prompt.clone(),
model: args.model.clone().unwrap_or_default(),
reasoning_effort: args.reasoning_effort.unwrap_or_default(),
}
.into(),
)
@@ -972,6 +974,7 @@ fn apply_spawn_agent_runtime_overrides(
fn apply_spawn_agent_overrides(config: &mut Config, child_depth: i32) {
if child_depth >= config.agent_max_depth {
let _ = config.features.disable(Feature::SpawnCsv);
let _ = config.features.disable(Feature::Collab);
}
}
@@ -190,7 +190,7 @@ impl ToolHandler for UnifiedExecHandler {
)
{
let approval_policy = context.turn.approval_policy.value();
manager.release_process_id(&process_id).await;
manager.release_process_id(process_id).await;
return Err(FunctionCallError::RespondToModel(format!(
"approval policy is {approval_policy:?}; reject command — you cannot ask for escalated permissions if the approval policy is {approval_policy:?}"
)));
@@ -211,7 +211,7 @@ impl ToolHandler for UnifiedExecHandler {
) {
Ok(normalized) => normalized,
Err(err) => {
manager.release_process_id(&process_id).await;
manager.release_process_id(process_id).await;
return Err(FunctionCallError::RespondToModel(err));
}
};
@@ -228,7 +228,7 @@ impl ToolHandler for UnifiedExecHandler {
)
.await?
{
manager.release_process_id(&process_id).await;
manager.release_process_id(process_id).await;
return Ok(ExecCommandToolOutput {
event_call_id: String::new(),
chunk_id: String::new(),
@@ -271,7 +271,7 @@ impl ToolHandler for UnifiedExecHandler {
let args: WriteStdinArgs = parse_arguments(&arguments)?;
let response = manager
.write_stdin(WriteStdinRequest {
process_id: &args.session_id.to_string(),
process_id: args.session_id,
input: &args.chars,
yield_time_ms: args.yield_time_ms,
max_output_tokens: args.max_output_tokens,
+1 -1
View File
@@ -1621,7 +1621,7 @@ source: /[\s\S]+/
enabled_tool_names.join(", ")
};
let description = format!(
"Runs JavaScript in an embedded V8 runtime. This is a freeform tool: send raw JavaScript source text (no JSON/quotes/markdown fences). Direct tool calls remain available while `{PUBLIC_TOOL_NAME}` is enabled. Inside JavaScript, import nested tools from `tools.js`, for example `import {{ exec_command }} from \"tools.js\"` or `import {{ tools }} from \"tools.js\"`. Namespaced tools are also available from `tools/<namespace...>.js`; MCP tools use `tools/mcp/<server>.js`, for example `import {{ append_notebook_logs_chart }} from \"tools/mcp/ologs.js\"`. `tools[name]` and identifier wrappers like `await shell(args)` remain available for compatibility when the tool name is a valid JS identifier. Nested tool calls resolve to their code-mode result values. Import `{{ output_text, output_image, set_max_output_tokens_per_exec_call, store, load }}` from `\"@openai/code_mode\"` (or `\"openai/code_mode\"`); `output_text(value)` surfaces text back to the model and stringifies non-string objects when possible, `output_image(imageUrl)` appends an `input_image` content item for `http(s)` or `data:` URLs, `store(key, value)` persists JSON-serializable values across `{PUBLIC_TOOL_NAME}` calls in the current session, `load(key)` returns a cloned stored value or `undefined`, and `set_max_output_tokens_per_exec_call(value)` sets the token budget used to truncate the final Rust-side result of the current `{PUBLIC_TOOL_NAME}` execution. The default is `10000`. This guards the overall `{PUBLIC_TOOL_NAME}` output, not individual nested tool invocations. When truncation happens, the final text uses the unified-exec style `Original token count:` / `Output:` wrapper and the usual `…N tokens truncated…` marker. Function tools require JSON object arguments. Freeform tools require raw strings. `add_content(value)` remains available for compatibility with a content item, content-item array, or string. Structured nested-tool results should be converted to text first, for example with `JSON.stringify(...)`. Only content passed to `output_text(...)`, `output_image(...)`, or `add_content(value)` is surfaced back to the model. Enabled nested tools: {enabled_list}."
"Runs JavaScript in an embedded V8 runtime. This is a freeform tool: send raw JavaScript source text (no JSON/quotes/markdown fences). Direct tool calls remain available while `{PUBLIC_TOOL_NAME}` is enabled. Inside JavaScript, import nested tools from `tools.js`, for example `import {{ exec_command }} from \"tools.js\"` or `import {{ tools }} from \"tools.js\"`. Namespaced tools are also available from `tools/<namespace...>.js`; MCP tools use `tools/mcp/<server>.js`, for example `import {{ append_notebook_logs_chart }} from \"tools/mcp/ologs.js\"`. `tools[name]` and identifier wrappers like `await shell(args)` remain available for compatibility when the tool name is a valid JS identifier. Nested tool calls resolve to their code-mode result values. Import `{{ output_text, output_image, set_max_output_tokens_per_exec_call, store, load }}` from `\"@openai/code_mode\"` (or `\"openai/code_mode\"`); `output_text(value)` surfaces text back to the model and stringifies non-string objects when possible, `output_image(imageUrl)` appends an `input_image` content item for `http(s)` or `data:` URLs, `store(key, value)` persists JSON-serializable values across `{PUBLIC_TOOL_NAME}` calls in the current session, `load(key)` returns a cloned stored value or `undefined`, and `set_max_output_tokens_per_exec_call(value)` sets the token budget used to truncate the final Rust-side result of the current `{PUBLIC_TOOL_NAME}` execution. The default is `10000`. This guards the overall `{PUBLIC_TOOL_NAME}` output, not individual nested tool invocations. The returned content starts with a separate `Script completed` or `Script failed` text item that includes wall time. When truncation happens, the final text may include `Total output lines:` and the usual `…N tokens truncated…` marker. Function tools require JSON object arguments. Freeform tools require raw strings. `add_content(value)` remains available for compatibility with a content item, content-item array, or string. Structured nested-tool results should be converted to text first, for example with `JSON.stringify(...)`. Only content passed to `output_text(...)`, `output_image(...)`, or `add_content(value)` is surfaced back to the model. Enabled nested tools: {enabled_list}."
);
ToolSpec::Freeform(FreeformTool {
@@ -110,7 +110,7 @@ pub(crate) fn spawn_exit_watcher(
call_id: String,
command: Vec<String>,
cwd: PathBuf,
process_id: String,
process_id: i32,
transcript: Arc<Mutex<HeadTailBuffer>>,
started_at: Instant,
) {
@@ -129,7 +129,7 @@ pub(crate) fn spawn_exit_watcher(
call_id,
command,
cwd,
Some(process_id),
Some(process_id.to_string()),
transcript,
String::new(),
exit_code,
+1 -1
View File
@@ -7,7 +7,7 @@ pub(crate) enum UnifiedExecError {
CreateProcess { message: String },
// The model is trained on `session_id`, but internally we track a `process_id`.
#[error("Unknown process id {process_id}")]
UnknownProcessId { process_id: String },
UnknownProcessId { process_id: i32 },
#[error("failed to write to stdin")]
WriteToStdin,
#[error(
+15 -35
View File
@@ -86,7 +86,7 @@ impl UnifiedExecContext {
#[derive(Debug)]
pub(crate) struct ExecCommandRequest {
pub command: Vec<String>,
pub process_id: String,
pub process_id: i32,
pub yield_time_ms: u64,
pub max_output_tokens: Option<usize>,
pub workdir: Option<PathBuf>,
@@ -101,7 +101,7 @@ pub(crate) struct ExecCommandRequest {
#[derive(Debug)]
pub(crate) struct WriteStdinRequest<'a> {
pub process_id: &'a str,
pub process_id: i32,
pub input: &'a str,
pub yield_time_ms: u64,
pub max_output_tokens: Option<usize>,
@@ -109,14 +109,14 @@ pub(crate) struct WriteStdinRequest<'a> {
#[derive(Default)]
pub(crate) struct ProcessStore {
processes: HashMap<String, ProcessEntry>,
reserved_process_ids: HashSet<String>,
processes: HashMap<i32, ProcessEntry>,
reserved_process_ids: HashSet<i32>,
}
impl ProcessStore {
fn remove(&mut self, process_id: &str) -> Option<ProcessEntry> {
self.reserved_process_ids.remove(process_id);
self.processes.remove(process_id)
fn remove(&mut self, process_id: i32) -> Option<ProcessEntry> {
self.reserved_process_ids.remove(&process_id);
self.processes.remove(&process_id)
}
}
@@ -144,7 +144,7 @@ impl Default for UnifiedExecProcessManager {
struct ProcessEntry {
process: Arc<UnifiedExecProcess>,
call_id: String,
process_id: String,
process_id: i32,
command: Vec<String>,
tty: bool,
network_approval_id: Option<String>,
@@ -238,7 +238,7 @@ mod tests {
async fn write_stdin(
session: &Arc<Session>,
process_id: &str,
process_id: i32,
input: &str,
yield_time_ms: u64,
) -> Result<ExecCommandToolOutput, UnifiedExecError> {
@@ -294,11 +294,7 @@ mod tests {
let (session, turn) = test_session_and_turn().await;
let open_shell = exec_command(&session, &turn, "bash -i", 2_500).await?;
let process_id = open_shell
.process_id
.as_ref()
.expect("expected process_id")
.as_str();
let process_id = open_shell.process_id.expect("expected process_id");
write_stdin(
&session,
@@ -330,15 +326,11 @@ mod tests {
let (session, turn) = test_session_and_turn().await;
let shell_a = exec_command(&session, &turn, "bash -i", 2_500).await?;
let session_a = shell_a
.process_id
.as_ref()
.expect("expected process id")
.clone();
let session_a = shell_a.process_id.expect("expected process id");
write_stdin(
&session,
session_a.as_str(),
session_a,
"export CODEX_INTERACTIVE_SHELL_VAR=codex\n",
2_500,
)
@@ -358,11 +350,7 @@ mod tests {
let out_3 = write_stdin(
&session,
shell_a
.process_id
.as_ref()
.expect("expected process id")
.as_str(),
shell_a.process_id.expect("expected process id"),
"echo $CODEX_INTERACTIVE_SHELL_VAR\n",
2_500,
)
@@ -384,11 +372,7 @@ mod tests {
let (session, turn) = test_session_and_turn().await;
let open_shell = exec_command(&session, &turn, "bash -i", 2_500).await?;
let process_id = open_shell
.process_id
.as_ref()
.expect("expected process id")
.as_str();
let process_id = open_shell.process_id.expect("expected process id");
write_stdin(
&session,
@@ -501,11 +485,7 @@ mod tests {
let (session, turn) = test_session_and_turn().await;
let open_shell = exec_command(&session, &turn, "bash -i", 2_500).await?;
let process_id = open_shell
.process_id
.as_ref()
.expect("expected process id")
.as_str();
let process_id = open_shell.process_id.expect("expected process id");
write_stdin(&session, process_id, "exit\n", 2_500).await?;
@@ -98,41 +98,39 @@ struct PreparedProcessHandles {
cancellation_token: CancellationToken,
pause_state: Option<watch::Receiver<bool>>,
command: Vec<String>,
process_id: String,
process_id: i32,
tty: bool,
}
impl UnifiedExecProcessManager {
pub(crate) async fn allocate_process_id(&self) -> String {
pub(crate) async fn allocate_process_id(&self) -> i32 {
loop {
let mut store = self.process_store.lock().await;
let process_id = if should_use_deterministic_process_ids() {
// test or deterministic mode
let next = store
store
.reserved_process_ids
.iter()
.filter_map(|s| s.parse::<i32>().ok())
.copied()
.max()
.map(|m| std::cmp::max(m, 999) + 1)
.unwrap_or(1000);
next.to_string()
.unwrap_or(1000)
} else {
// production mode → random
rand::rng().random_range(1_000..100_000).to_string()
rand::rng().random_range(1_000..100_000)
};
if store.reserved_process_ids.contains(&process_id) {
continue;
}
store.reserved_process_ids.insert(process_id.clone());
store.reserved_process_ids.insert(process_id);
return process_id;
}
}
pub(crate) async fn release_process_id(&self, process_id: &str) {
pub(crate) async fn release_process_id(&self, process_id: i32) {
let removed = {
let mut store = self.process_store.lock().await;
store.remove(process_id)
@@ -172,7 +170,7 @@ impl UnifiedExecProcessManager {
(Arc::new(process), deferred_network_approval)
}
Err(err) => {
self.release_process_id(&request.process_id).await;
self.release_process_id(request.process_id).await;
return Err(err);
}
};
@@ -188,7 +186,7 @@ impl UnifiedExecProcessManager {
&request.command,
cwd.clone(),
ExecCommandSource::UnifiedExecStartup,
Some(request.process_id.clone()),
Some(request.process_id.to_string()),
);
emitter.emit(event_ctx, ToolEventStage::Begin).await;
@@ -227,7 +225,7 @@ impl UnifiedExecProcessManager {
let exit_code = process.exit_code();
let has_exited = process.has_exited() || exit_code.is_some();
let chunk_id = generate_chunk_id();
let process_id = request.process_id.clone();
let process_id = request.process_id;
if has_exited {
// Short‑lived command: emit ExecCommandEnd immediately using the
@@ -240,7 +238,7 @@ impl UnifiedExecProcessManager {
context.call_id.clone(),
request.command.clone(),
cwd.clone(),
Some(process_id),
Some(process_id.to_string()),
Arc::clone(&transcript),
text.clone(),
exit,
@@ -248,7 +246,7 @@ impl UnifiedExecProcessManager {
)
.await;
self.release_process_id(&request.process_id).await;
self.release_process_id(request.process_id).await;
finish_deferred_network_approval(
context.session.as_ref(),
deferred_network_approval.take(),
@@ -287,7 +285,7 @@ impl UnifiedExecProcessManager {
process_id: if has_exited {
None
} else {
Some(request.process_id.clone())
Some(request.process_id)
},
exit_code,
original_token_count: Some(original_token_count),
@@ -301,7 +299,7 @@ impl UnifiedExecProcessManager {
&self,
request: WriteStdinRequest<'_>,
) -> Result<ExecCommandToolOutput, UnifiedExecError> {
let process_id = request.process_id.to_string();
let process_id = request.process_id;
let PreparedProcessHandles {
writer_tx,
@@ -315,7 +313,7 @@ impl UnifiedExecProcessManager {
process_id,
tty,
..
} = self.prepare_process_handles(process_id.as_str()).await?;
} = self.prepare_process_handles(process_id).await?;
if !request.input.is_empty() {
if !tty {
@@ -359,7 +357,7 @@ impl UnifiedExecProcessManager {
// still alive or has exited and been removed from the store; we thread
// that through so the handler can tag TerminalInteraction with an
// appropriate process_id and exit_code.
let status = self.refresh_process_state(process_id.as_str()).await;
let status = self.refresh_process_state(process_id).await;
let (process_id, exit_code, event_call_id) = match status {
ProcessStatus::Alive {
exit_code,
@@ -372,7 +370,7 @@ impl UnifiedExecProcessManager {
}
ProcessStatus::Unknown => {
return Err(UnifiedExecError::UnknownProcessId {
process_id: request.process_id.to_string(),
process_id: request.process_id,
});
}
};
@@ -392,18 +390,18 @@ impl UnifiedExecProcessManager {
Ok(response)
}
async fn refresh_process_state(&self, process_id: &str) -> ProcessStatus {
async fn refresh_process_state(&self, process_id: i32) -> ProcessStatus {
let status = {
let mut store = self.process_store.lock().await;
let Some(entry) = store.processes.get(process_id) else {
let Some(entry) = store.processes.get(&process_id) else {
return ProcessStatus::Unknown;
};
let exit_code = entry.process.exit_code();
let process_id = entry.process_id.clone();
let process_id = entry.process_id;
if entry.process.has_exited() {
let Some(entry) = store.remove(&process_id) else {
let Some(entry) = store.remove(process_id) else {
return ProcessStatus::Unknown;
};
ProcessStatus::Exited {
@@ -426,16 +424,13 @@ impl UnifiedExecProcessManager {
async fn prepare_process_handles(
&self,
process_id: &str,
process_id: i32,
) -> Result<PreparedProcessHandles, UnifiedExecError> {
let mut store = self.process_store.lock().await;
let entry =
store
.processes
.get_mut(process_id)
.ok_or(UnifiedExecError::UnknownProcessId {
process_id: process_id.to_string(),
})?;
let entry = store
.processes
.get_mut(&process_id)
.ok_or(UnifiedExecError::UnknownProcessId { process_id })?;
entry.last_used = Instant::now();
let OutputHandles {
output_buffer,
@@ -458,7 +453,7 @@ impl UnifiedExecProcessManager {
cancellation_token,
pause_state,
command: entry.command.clone(),
process_id: entry.process_id.clone(),
process_id: entry.process_id,
tty: entry.tty,
})
}
@@ -481,7 +476,7 @@ impl UnifiedExecProcessManager {
command: &[String],
cwd: PathBuf,
started_at: Instant,
process_id: String,
process_id: i32,
tty: bool,
network_approval_id: Option<String>,
transcript: Arc<tokio::sync::Mutex<HeadTailBuffer>>,
@@ -489,7 +484,7 @@ impl UnifiedExecProcessManager {
let entry = ProcessEntry {
process: Arc::clone(&process),
call_id: context.call_id.clone(),
process_id: process_id.clone(),
process_id,
command: command.to_vec(),
tty,
network_approval_id,
@@ -499,7 +494,7 @@ impl UnifiedExecProcessManager {
let (number_processes, pruned_entry) = {
let mut store = self.process_store.lock().await;
let pruned_entry = Self::prune_processes_if_needed(&mut store);
store.processes.insert(process_id.clone(), entry);
store.processes.insert(process_id, entry);
(store.processes.len(), pruned_entry)
};
// prune_processes_if_needed runs while holding process_store; do async
@@ -526,7 +521,7 @@ impl UnifiedExecProcessManager {
context.call_id.clone(),
command.to_vec(),
cwd,
process_id.clone(),
process_id,
transcript,
started_at,
);
@@ -759,31 +754,31 @@ impl UnifiedExecProcessManager {
return None;
}
let meta: Vec<(String, Instant, bool)> = store
let meta: Vec<(i32, Instant, bool)> = store
.processes
.iter()
.map(|(id, entry)| (id.clone(), entry.last_used, entry.process.has_exited()))
.map(|(id, entry)| (*id, entry.last_used, entry.process.has_exited()))
.collect();
if let Some(process_id) = Self::process_id_to_prune_from_meta(&meta) {
return store.remove(&process_id);
return store.remove(process_id);
}
None
}
// Centralized pruning policy so we can easily swap strategies later.
fn process_id_to_prune_from_meta(meta: &[(String, Instant, bool)]) -> Option<String> {
fn process_id_to_prune_from_meta(meta: &[(i32, Instant, bool)]) -> Option<i32> {
if meta.is_empty() {
return None;
}
let mut by_recency = meta.to_vec();
by_recency.sort_by_key(|(_, last_used, _)| Reverse(*last_used));
let protected: HashSet<String> = by_recency
let protected: HashSet<i32> = by_recency
.iter()
.take(8)
.map(|(process_id, _, _)| process_id.clone())
.map(|(process_id, _, _)| *process_id)
.collect();
let mut lru = meta.to_vec();
@@ -793,7 +788,7 @@ impl UnifiedExecProcessManager {
.iter()
.find(|(process_id, _, exited)| !protected.contains(process_id) && *exited)
{
return Some(process_id.clone());
return Some(*process_id);
}
lru.into_iter()
@@ -824,7 +819,7 @@ enum ProcessStatus {
Alive {
exit_code: Option<i32>,
call_id: String,
process_id: String,
process_id: i32,
},
Exited {
exit_code: Option<i32>,
@@ -874,67 +869,64 @@ mod tests {
#[test]
fn pruning_prefers_exited_processes_outside_recently_used() {
let now = Instant::now();
let id = |n: i32| n.to_string();
let meta = vec![
(id(1), now - Duration::from_secs(40), false),
(id(2), now - Duration::from_secs(30), true),
(id(3), now - Duration::from_secs(20), false),
(id(4), now - Duration::from_secs(19), false),
(id(5), now - Duration::from_secs(18), false),
(id(6), now - Duration::from_secs(17), false),
(id(7), now - Duration::from_secs(16), false),
(id(8), now - Duration::from_secs(15), false),
(id(9), now - Duration::from_secs(14), false),
(id(10), now - Duration::from_secs(13), false),
(1, now - Duration::from_secs(40), false),
(2, now - Duration::from_secs(30), true),
(3, now - Duration::from_secs(20), false),
(4, now - Duration::from_secs(19), false),
(5, now - Duration::from_secs(18), false),
(6, now - Duration::from_secs(17), false),
(7, now - Duration::from_secs(16), false),
(8, now - Duration::from_secs(15), false),
(9, now - Duration::from_secs(14), false),
(10, now - Duration::from_secs(13), false),
];
let candidate = UnifiedExecProcessManager::process_id_to_prune_from_meta(&meta);
assert_eq!(candidate, Some(id(2)));
assert_eq!(candidate, Some(2));
}
#[test]
fn pruning_falls_back_to_lru_when_no_exited() {
let now = Instant::now();
let id = |n: i32| n.to_string();
let meta = vec![
(id(1), now - Duration::from_secs(40), false),
(id(2), now - Duration::from_secs(30), false),
(id(3), now - Duration::from_secs(20), false),
(id(4), now - Duration::from_secs(19), false),
(id(5), now - Duration::from_secs(18), false),
(id(6), now - Duration::from_secs(17), false),
(id(7), now - Duration::from_secs(16), false),
(id(8), now - Duration::from_secs(15), false),
(id(9), now - Duration::from_secs(14), false),
(id(10), now - Duration::from_secs(13), false),
(1, now - Duration::from_secs(40), false),
(2, now - Duration::from_secs(30), false),
(3, now - Duration::from_secs(20), false),
(4, now - Duration::from_secs(19), false),
(5, now - Duration::from_secs(18), false),
(6, now - Duration::from_secs(17), false),
(7, now - Duration::from_secs(16), false),
(8, now - Duration::from_secs(15), false),
(9, now - Duration::from_secs(14), false),
(10, now - Duration::from_secs(13), false),
];
let candidate = UnifiedExecProcessManager::process_id_to_prune_from_meta(&meta);
assert_eq!(candidate, Some(id(1)));
assert_eq!(candidate, Some(1));
}
#[test]
fn pruning_protects_recent_processes_even_if_exited() {
let now = Instant::now();
let id = |n: i32| n.to_string();
let meta = vec![
(id(1), now - Duration::from_secs(40), false),
(id(2), now - Duration::from_secs(30), false),
(id(3), now - Duration::from_secs(20), true),
(id(4), now - Duration::from_secs(19), false),
(id(5), now - Duration::from_secs(18), false),
(id(6), now - Duration::from_secs(17), false),
(id(7), now - Duration::from_secs(16), false),
(id(8), now - Duration::from_secs(15), false),
(id(9), now - Duration::from_secs(14), false),
(id(10), now - Duration::from_secs(13), true),
(1, now - Duration::from_secs(40), false),
(2, now - Duration::from_secs(30), false),
(3, now - Duration::from_secs(20), true),
(4, now - Duration::from_secs(19), false),
(5, now - Duration::from_secs(18), false),
(6, now - Duration::from_secs(17), false),
(7, now - Duration::from_secs(16), false),
(8, now - Duration::from_secs(15), false),
(9, now - Duration::from_secs(14), false),
(10, now - Duration::from_secs(13), true),
];
let candidate = UnifiedExecProcessManager::process_id_to_prune_from_meta(&meta);
// (10) is exited but among the last 8; we should drop the LRU outside that set.
assert_eq!(candidate, Some(id(1)));
assert_eq!(candidate, Some(1));
}
}
+13 -1
View File
@@ -416,6 +416,11 @@ pub struct WebSocketConnectionConfig {
/// Tests use this to force websocket setup into an in-flight state so first-turn warmup paths
/// can be exercised deterministically.
pub accept_delay: Option<Duration>,
/// Whether the server should send a websocket close frame after all scripted responses.
///
/// Tests can disable this to simulate a peer that surfaces a terminal event but never
/// completes the close handshake.
pub close_after_requests: bool,
}
pub struct WebSocketTestServer {
@@ -1168,6 +1173,7 @@ pub async fn start_websocket_server(connections: Vec<Vec<Vec<Value>>>) -> WebSoc
requests,
response_headers: Vec::new(),
accept_delay: None,
close_after_requests: true,
})
.collect();
start_websocket_server_with_headers(connections).await
@@ -1261,6 +1267,7 @@ pub async fn start_websocket_server_with_headers(
log.push(Vec::new());
log.len() - 1
};
let close_after_requests = connection.close_after_requests;
for request_events in connection.requests {
let Some(Ok(message)) = ws_stream.next().await else {
break;
@@ -1324,7 +1331,12 @@ pub async fn start_websocket_server_with_headers(
}
}
let _ = ws_stream.close(None).await;
if close_after_requests {
let _ = ws_stream.close(None).await;
} else {
let _ = shutdown_rx.await;
return;
}
if connections.lock().unwrap().is_empty() {
return;
+4 -4
View File
@@ -224,7 +224,7 @@ async fn report_agent_job_result_rejects_wrong_thread() -> Result<()> {
let mut builder = test_codex().with_config(|config| {
config
.features
.enable(Feature::Collab)
.enable(Feature::SpawnCsv)
.expect("test config should allow feature update");
config
.features
@@ -290,7 +290,7 @@ async fn spawn_agents_on_csv_runs_and_exports() -> Result<()> {
let mut builder = test_codex().with_config(|config| {
config
.features
.enable(Feature::Collab)
.enable(Feature::SpawnCsv)
.expect("test config should allow feature update");
config
.features
@@ -333,7 +333,7 @@ async fn spawn_agents_on_csv_dedupes_item_ids() -> Result<()> {
let mut builder = test_codex().with_config(|config| {
config
.features
.enable(Feature::Collab)
.enable(Feature::SpawnCsv)
.expect("test config should allow feature update");
config
.features
@@ -391,7 +391,7 @@ async fn spawn_agents_on_csv_stop_halts_future_items() -> Result<()> {
let mut builder = test_codex().with_config(|config| {
config
.features
.enable(Feature::Collab)
.enable(Feature::SpawnCsv)
.expect("test config should allow feature update");
config
.features
@@ -129,6 +129,7 @@ async fn websocket_first_turn_handles_handshake_delay_with_startup_prewarm() ->
response_headers: Vec::new(),
// Delay handshake so turn processing must tolerate websocket startup latency.
accept_delay: Some(Duration::from_millis(150)),
close_after_requests: true,
}])
.await;
@@ -653,6 +653,7 @@ async fn responses_websocket_emits_reasoning_included_event() {
requests: vec![vec![ev_response_created("resp-1"), ev_completed("resp-1")]],
response_headers: vec![("X-Reasoning-Included".to_string(), "true".to_string())],
accept_delay: None,
close_after_requests: true,
}])
.await;
@@ -725,6 +726,7 @@ async fn responses_websocket_emits_rate_limit_events() {
("X-Reasoning-Included".to_string(), "true".to_string()),
],
accept_delay: None,
close_after_requests: true,
}])
.await;
@@ -1369,6 +1371,65 @@ async fn responses_websocket_v2_after_error_uses_full_create_without_previous_re
server.shutdown().await;
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn responses_websocket_v2_surfaces_terminal_error_without_close_handshake() {
skip_if_no_network!();
let server = start_websocket_server_with_headers(vec![WebSocketConnectionConfig {
requests: vec![
vec![ev_response_created("resp-1"), ev_completed("resp-1")],
vec![json!({
"type": "response.failed",
"response": {
"error": {
"code": "invalid_prompt",
"message": "synthetic websocket failure"
}
}
})],
],
response_headers: Vec::new(),
accept_delay: None,
close_after_requests: false,
}])
.await;
let harness = websocket_harness_with_v2(&server, true).await;
let mut session = harness.client.new_session();
let prompt_one = prompt_with_input(vec![message_item("hello")]);
let prompt_two = prompt_with_input(vec![message_item("hello"), message_item("second")]);
stream_until_complete(&mut session, &harness, &prompt_one).await;
let mut second_stream = session
.stream(
&prompt_two,
&harness.model_info,
&harness.session_telemetry,
harness.effort,
harness.summary,
None,
None,
)
.await
.expect("websocket stream failed");
let saw_error = tokio::time::timeout(Duration::from_secs(2), async {
while let Some(event) = second_stream.next().await {
if event.is_err() {
return true;
}
}
false
})
.await
.expect("timed out waiting for terminal websocket error");
assert!(saw_error, "expected second websocket stream to error");
server.shutdown().await;
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn responses_websocket_v2_sets_openai_beta_header() {
skip_if_no_network!();
+141 -44
View File
@@ -24,14 +24,35 @@ use std::fs;
use std::time::Duration;
use wiremock::MockServer;
fn custom_tool_output_text_and_success(
fn custom_tool_output_items(req: &ResponsesRequest, call_id: &str) -> Vec<Value> {
req.custom_tool_call_output(call_id)
.get("output")
.and_then(Value::as_array)
.expect("custom tool output should be serialized as content items")
.clone()
}
fn text_item(items: &[Value], index: usize) -> &str {
items[index]
.get("text")
.and_then(Value::as_str)
.expect("content item should be input_text")
}
fn custom_tool_output_body_and_success(
req: &ResponsesRequest,
call_id: &str,
) -> (String, Option<bool>) {
let (output, success) = req
let (_, success) = req
.custom_tool_call_output_content_and_success(call_id)
.expect("custom tool output should be present");
(output.unwrap_or_default(), success)
let items = custom_tool_output_items(req, call_id);
let output = items
.iter()
.skip(1)
.filter_map(|item| item.get("text").and_then(Value::as_str))
.collect();
(output, success)
}
async fn run_code_mode_turn(
@@ -152,13 +173,16 @@ add_content(JSON.stringify(await exec_command({ cmd: "printf code_mode_exec_mark
.await?;
let req = second_mock.single_request();
let (output, success) = custom_tool_output_text_and_success(&req, "call-1");
assert_ne!(
success,
Some(false),
"exec call failed unexpectedly: {output}"
let items = custom_tool_output_items(&req, "call-1");
assert_eq!(items.len(), 2);
assert_regex_match(
concat!(
r"(?s)\A",
r"Script completed\nWall time \d+\.\d seconds\nOutput:\n\z"
),
text_item(&items, 0),
);
let parsed: Value = serde_json::from_str(&output)?;
let parsed: Value = serde_json::from_str(text_item(&items, 1))?;
assert!(
parsed
.get("chunk_id")
@@ -201,22 +225,66 @@ add_content(JSON.stringify(await exec_command({
.await?;
let req = second_mock.single_request();
let (output, success) = custom_tool_output_text_and_success(&req, "call-1");
assert_ne!(
success,
Some(false),
"exec call failed unexpectedly: {output}"
let items = custom_tool_output_items(&req, "call-1");
assert_eq!(items.len(), 2);
assert_regex_match(
concat!(
r"(?s)\A",
r"Script completed\nWall time \d+\.\d seconds\nOutput:\n\z"
),
text_item(&items, 0),
);
let expected_pattern = r#"(?sx)
\A
Original\ token\ count:\ \d+\n
Output:\n
Total\ output\ lines:\ 1\n
\n
\{"chunk_id".*…\d+\ tokens\ truncated….*
.*…\d+\ tokens\ truncated….*
\z
"#;
assert_regex_match(expected_pattern, &output);
assert_regex_match(expected_pattern, text_item(&items, 1));
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn code_mode_returns_accumulated_output_when_script_fails() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
let (_test, second_mock) = run_code_mode_turn(
&server,
"use code_mode to surface script failures",
r#"
add_content("before crash");
add_content("still before crash");
throw new Error("boom");
"#,
false,
)
.await?;
let req = second_mock.single_request();
let items = custom_tool_output_items(&req, "call-1");
assert_eq!(items.len(), 4);
assert_regex_match(
concat!(
r"(?s)\A",
r"Script failed\nWall time \d+\.\d seconds\nOutput:\n\z"
),
text_item(&items, 0),
);
assert_eq!(text_item(&items, 1), "before crash");
assert_eq!(text_item(&items, 2), "still before crash");
assert_regex_match(
r#"(?sx)
\A
Script\ error:\n
Error:\ boom\n
(?:\s+at\ .+\n?)+
\z
"#,
text_item(&items, 3),
);
Ok(())
}
@@ -239,7 +307,7 @@ output_text({ json: true });
.await?;
let req = second_mock.single_request();
let (output, success) = custom_tool_output_text_and_success(&req, "call-1");
let (output, success) = custom_tool_output_body_and_success(&req, "call-1");
assert_ne!(
success,
Some(false),
@@ -270,13 +338,25 @@ output_text(circular);
.await?;
let req = second_mock.single_request();
let (output, success) = custom_tool_output_text_and_success(&req, "call-1");
let items = custom_tool_output_items(&req, "call-1");
let (_, success) = req
.custom_tool_call_output_content_and_success("call-1")
.expect("custom tool output should be present");
assert_ne!(
success,
Some(true),
"circular stringify unexpectedly succeeded"
);
assert!(output.contains("Converting circular structure to JSON"));
assert_eq!(items.len(), 2);
assert_regex_match(
concat!(
r"(?s)\A",
r"Script failed\nWall time \d+\.\d seconds\nOutput:\n\z"
),
text_item(&items, 0),
);
assert!(text_item(&items, 1).contains("Script error:"));
assert!(text_item(&items, 1).contains("Converting circular structure to JSON"));
Ok(())
}
@@ -300,28 +380,34 @@ output_image("data:image/png;base64,AAA");
.await?;
let req = second_mock.single_request();
let (_, success) = custom_tool_output_text_and_success(&req, "call-1");
let items = custom_tool_output_items(&req, "call-1");
let (_, success) = custom_tool_output_body_and_success(&req, "call-1");
assert_ne!(
success,
Some(false),
"code_mode image output failed unexpectedly"
);
assert_eq!(items.len(), 3);
assert_regex_match(
concat!(
r"(?s)\A",
r"Script completed\nWall time \d+\.\d seconds\nOutput:\n\z"
),
text_item(&items, 0),
);
assert_eq!(
req.custom_tool_call_output("call-1"),
items[1],
serde_json::json!({
"type": "custom_tool_call_output",
"call_id": "call-1",
"output": [
{
"type": "input_image",
"image_url": "https://example.com/image.jpg"
},
{
"type": "input_image",
"image_url": "data:image/png;base64,AAA"
}
]
})
"type": "input_image",
"image_url": "https://example.com/image.jpg"
}),
);
assert_eq!(
items[2],
serde_json::json!({
"type": "input_image",
"image_url": "data:image/png;base64,AAA"
}),
);
Ok(())
@@ -344,11 +430,22 @@ async fn code_mode_can_apply_patch_via_nested_tool() -> Result<()> {
run_code_mode_turn(&server, "use exec to run apply_patch", &code, true).await?;
let req = second_mock.single_request();
let (output, success) = custom_tool_output_text_and_success(&req, "call-1");
let items = custom_tool_output_items(&req, "call-1");
let (_, success) = req
.custom_tool_call_output_content_and_success("call-1")
.expect("custom tool output should be present");
assert_ne!(
success,
Some(false),
"exec apply_patch call failed unexpectedly: {output}"
"exec apply_patch call failed unexpectedly: {items:?}"
);
assert_eq!(items.len(), 2);
assert_regex_match(
concat!(
r"(?s)\A",
r"Script completed\nWall time \d+\.\d seconds\nOutput:\n\z"
),
text_item(&items, 0),
);
let file_path = test.cwd_path().join(file_name);
@@ -380,7 +477,7 @@ add_content(
run_code_mode_turn_with_rmcp(&server, "use exec to run the rmcp echo tool", code).await?;
let req = second_mock.single_request();
let (output, success) = custom_tool_output_text_and_success(&req, "call-1");
let (output, success) = custom_tool_output_body_and_success(&req, "call-1");
assert_ne!(
success,
Some(false),
@@ -419,7 +516,7 @@ add_content(
run_code_mode_turn_with_rmcp(&server, "use exec to run the rmcp echo tool", code).await?;
let req = second_mock.single_request();
let (output, success) = custom_tool_output_text_and_success(&req, "call-1");
let (output, success) = custom_tool_output_body_and_success(&req, "call-1");
assert_ne!(
success,
Some(false),
@@ -463,7 +560,7 @@ add_content(
.await?;
let req = second_mock.single_request();
let (output, success) = custom_tool_output_text_and_success(&req, "call-1");
let (output, success) = custom_tool_output_body_and_success(&req, "call-1");
assert_ne!(
success,
Some(false),
@@ -504,7 +601,7 @@ add_content(
run_code_mode_turn_with_rmcp(&server, "use exec to call rmcp echo badly", code).await?;
let req = second_mock.single_request();
let (output, success) = custom_tool_output_text_and_success(&req, "call-1");
let (output, success) = custom_tool_output_body_and_success(&req, "call-1");
assert_ne!(
success,
Some(false),
@@ -561,7 +658,7 @@ add_content("stored");
let first_request = first_follow_up.single_request();
let (first_output, first_success) =
custom_tool_output_text_and_success(&first_request, "call-1");
custom_tool_output_body_and_success(&first_request, "call-1");
assert_ne!(
first_success,
Some(false),
@@ -599,7 +696,7 @@ add_content(JSON.stringify(load("nb")));
let second_request = second_follow_up.single_request();
let (second_output, second_success) =
custom_tool_output_text_and_success(&second_request, "call-2");
custom_tool_output_body_and_success(&second_request, "call-2");
assert_ne!(
second_success,
Some(false),
@@ -161,6 +161,25 @@ fn assert_request_contains_realtime_start(request: &responses::ResponsesRequest)
);
}
fn assert_request_contains_custom_realtime_start(
request: &responses::ResponsesRequest,
instructions: &str,
) {
let body = request.body_json().to_string();
assert!(
body.contains("<realtime_conversation>"),
"expected request to preserve the realtime wrapper"
);
assert!(
body.contains(instructions),
"expected request to use custom realtime start instructions"
);
assert!(
!body.contains("Realtime conversation started."),
"expected request to replace the default realtime start instructions"
);
}
fn assert_request_contains_realtime_end(request: &responses::ResponsesRequest) {
let body = request.body_json().to_string();
assert!(
@@ -1518,6 +1537,53 @@ async fn snapshot_request_shape_remote_pre_turn_compaction_restates_realtime_sta
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn remote_request_uses_custom_experimental_realtime_start_instructions() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = wiremock::MockServer::start().await;
let realtime_server = start_remote_realtime_server().await;
let custom_instructions = "custom realtime start instructions";
let mut builder = remote_realtime_test_codex_builder(&realtime_server).with_config({
let custom_instructions = custom_instructions.to_string();
move |config| {
config.experimental_realtime_start_instructions = Some(custom_instructions);
}
});
let test = builder.build(&server).await?;
let responses_mock = responses::mount_sse_once(
&server,
responses::sse(vec![
responses::ev_assistant_message("m1", "REMOTE_FIRST_REPLY"),
responses::ev_completed("r1"),
]),
)
.await;
start_realtime_conversation(test.codex.as_ref()).await?;
test.codex
.submit(Op::UserInput {
items: vec![UserInput::Text {
text: "USER_ONE".to_string(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
})
.await?;
wait_for_event(&test.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
assert_request_contains_custom_realtime_start(
&responses_mock.single_request(),
custom_instructions,
);
close_realtime_conversation(test.codex.as_ref()).await?;
realtime_server.shutdown().await;
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn snapshot_request_shape_remote_pre_turn_compaction_restates_realtime_end() -> Result<()> {
skip_if_no_network!(Ok(()));
+3
View File
@@ -103,6 +103,7 @@ async fn websocket_turn_state_persists_within_turn_and_resets_after() -> Result<
]],
response_headers: vec![(TURN_STATE_HEADER.to_string(), "ts-1".to_string())],
accept_delay: None,
close_after_requests: true,
},
WebSocketConnectionConfig {
requests: vec![vec![
@@ -112,6 +113,7 @@ async fn websocket_turn_state_persists_within_turn_and_resets_after() -> Result<
]],
response_headers: Vec::new(),
accept_delay: None,
close_after_requests: true,
},
WebSocketConnectionConfig {
requests: vec![vec![
@@ -121,6 +123,7 @@ async fn websocket_turn_state_persists_within_turn_and_resets_after() -> Result<
]],
response_headers: Vec::new(),
accept_delay: None,
close_after_requests: true,
},
])
.await;
@@ -698,6 +698,7 @@ impl EventProcessor for EventProcessorWithHumanOutput {
call_id,
sender_thread_id: _,
prompt,
..
}) => {
ts_msg!(
self,
@@ -34,6 +34,7 @@ use codex_protocol::ThreadId;
use codex_protocol::config_types::ModeKind;
use codex_protocol::mcp::CallToolResult;
use codex_protocol::models::WebSearchAction;
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
use codex_protocol::plan_tool::PlanItemArg;
use codex_protocol::plan_tool::StepStatus;
use codex_protocol::plan_tool::UpdatePlanArgs;
@@ -547,6 +548,8 @@ fn collab_spawn_begin_and_end_emit_item_events() {
call_id: "call-10".to_string(),
sender_thread_id,
prompt: prompt.clone(),
model: "gpt-5".to_string(),
reasoning_effort: ReasoningEffortConfig::default(),
}),
);
let begin_events = ep.collect_thread_events(&begin);
+5 -2
View File
@@ -520,9 +520,12 @@ impl DeveloperInstructions {
}
pub fn realtime_start_message() -> Self {
Self::realtime_start_message_with_instructions(REALTIME_START_INSTRUCTIONS.trim())
}
pub fn realtime_start_message_with_instructions(instructions: &str) -> Self {
DeveloperInstructions::new(format!(
"{REALTIME_CONVERSATION_OPEN_TAG}\n{}\n{REALTIME_CONVERSATION_CLOSE_TAG}",
REALTIME_START_INSTRUCTIONS.trim()
"{REALTIME_CONVERSATION_OPEN_TAG}\n{instructions}\n{REALTIME_CONVERSATION_CLOSE_TAG}"
))
}
+2
View File
@@ -3132,6 +3132,8 @@ pub struct CollabAgentSpawnBeginEvent {
/// Initial prompt sent to the agent. Can be empty to prevent CoT leaking at the
/// beginning.
pub prompt: String,
pub model: String,
pub reasoning_effort: ReasoningEffortConfig,
}
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)]
+36 -6
View File
@@ -101,6 +101,7 @@ use codex_protocol::protocol::AgentReasoningRawContentEvent;
use codex_protocol::protocol::ApplyPatchApprovalRequestEvent;
use codex_protocol::protocol::BackgroundEventEvent;
use codex_protocol::protocol::CodexErrorInfo;
use codex_protocol::protocol::CollabAgentSpawnBeginEvent;
use codex_protocol::protocol::CreditsSnapshot;
use codex_protocol::protocol::DeprecationNoticeEvent;
use codex_protocol::protocol::ErrorEvent;
@@ -579,6 +580,7 @@ pub(crate) struct ChatWidget {
// Latest completed user-visible Codex output that `/copy` should place on the clipboard.
last_copyable_output: Option<String>,
running_commands: HashMap<String, RunningCommand>,
pending_collab_spawn_requests: HashMap<String, multi_agents::SpawnRequestSummary>,
suppressed_exec_calls: HashSet<String>,
skills_all: Vec<ProtocolSkillMetadata>,
skills_initial_state: Option<HashMap<PathBuf, bool>>,
@@ -1466,6 +1468,7 @@ impl ChatWidget {
if self.plan_stream_controller.is_none() {
self.plan_stream_controller = Some(PlanStreamController::new(
self.last_rendered_width.get().map(|w| w.saturating_sub(4)),
&self.config.cwd,
));
}
if let Some(controller) = self.plan_stream_controller.as_mut()
@@ -1504,7 +1507,7 @@ impl ChatWidget {
// TODO: Replace streamed output with the final plan item text if plan streaming is
// removed or if we need to reconcile mismatches between streamed and final content.
} else if !plan_text.is_empty() {
self.add_to_history(history_cell::new_proposed_plan(plan_text));
self.add_to_history(history_cell::new_proposed_plan(plan_text, &self.config.cwd));
}
if should_restore_after_stream {
self.pending_status_indicator_restore = true;
@@ -1537,8 +1540,10 @@ impl ChatWidget {
// At the end of a reasoning block, record transcript-only content.
self.full_reasoning_buffer.push_str(&self.reasoning_buffer);
if !self.full_reasoning_buffer.is_empty() {
let cell =
history_cell::new_reasoning_summary_block(self.full_reasoning_buffer.clone());
let cell = history_cell::new_reasoning_summary_block(
self.full_reasoning_buffer.clone(),
&self.config.cwd,
);
self.add_boxed_history(cell);
}
self.reasoning_buffer.clear();
@@ -2778,6 +2783,7 @@ impl ChatWidget {
}
self.stream_controller = Some(StreamController::new(
self.last_rendered_width.get().map(|w| w.saturating_sub(2)),
&self.config.cwd,
));
}
if let Some(controller) = self.stream_controller.as_mut()
@@ -3243,6 +3249,7 @@ impl ChatWidget {
plan_stream_controller: None,
last_copyable_output: None,
running_commands: HashMap::new(),
pending_collab_spawn_requests: HashMap::new(),
suppressed_exec_calls: HashSet::new(),
last_unified_wait: None,
unified_exec_wait_streak: None,
@@ -3427,6 +3434,7 @@ impl ChatWidget {
plan_stream_controller: None,
last_copyable_output: None,
running_commands: HashMap::new(),
pending_collab_spawn_requests: HashMap::new(),
suppressed_exec_calls: HashSet::new(),
last_unified_wait: None,
unified_exec_wait_streak: None,
@@ -3603,6 +3611,7 @@ impl ChatWidget {
plan_stream_controller: None,
last_copyable_output: None,
running_commands: HashMap::new(),
pending_collab_spawn_requests: HashMap::new(),
suppressed_exec_calls: HashSet::new(),
last_unified_wait: None,
unified_exec_wait_streak: None,
@@ -4999,8 +5008,24 @@ impl ChatWidget {
}
EventMsg::ExitedReviewMode(review) => self.on_exited_review_mode(review),
EventMsg::ContextCompacted(_) => self.on_agent_message("Context compacted".to_owned()),
EventMsg::CollabAgentSpawnBegin(_) => {}
EventMsg::CollabAgentSpawnEnd(ev) => self.on_collab_event(multi_agents::spawn_end(ev)),
EventMsg::CollabAgentSpawnBegin(CollabAgentSpawnBeginEvent {
call_id,
model,
reasoning_effort,
..
}) => {
self.pending_collab_spawn_requests.insert(
call_id,
multi_agents::SpawnRequestSummary {
model,
reasoning_effort,
},
);
}
EventMsg::CollabAgentSpawnEnd(ev) => {
let spawn_request = self.pending_collab_spawn_requests.remove(&ev.call_id);
self.on_collab_event(multi_agents::spawn_end(ev, spawn_request.as_ref()));
}
EventMsg::CollabAgentInteractionBegin(_) => {}
EventMsg::CollabAgentInteractionEnd(ev) => {
self.on_collab_event(multi_agents::interaction_end(ev))
@@ -5135,7 +5160,12 @@ impl ChatWidget {
} else {
// Show explanation when there are no structured findings.
let mut rendered: Vec<ratatui::text::Line<'static>> = vec!["".into()];
append_markdown(&explanation, None, &mut rendered);
append_markdown(
&explanation,
None,
Some(self.config.cwd.as_path()),
&mut rendered,
);
let body_cell = AgentMessageCell::new(rendered, false);
self.app_event_tx
.send(AppEvent::InsertHistoryCell(Box::new(body_cell)));
+46
View File
@@ -58,9 +58,12 @@ use codex_protocol::protocol::AgentMessageDeltaEvent;
use codex_protocol::protocol::AgentMessageEvent;
use codex_protocol::protocol::AgentReasoningDeltaEvent;
use codex_protocol::protocol::AgentReasoningEvent;
use codex_protocol::protocol::AgentStatus;
use codex_protocol::protocol::ApplyPatchApprovalRequestEvent;
use codex_protocol::protocol::BackgroundEventEvent;
use codex_protocol::protocol::CodexErrorInfo;
use codex_protocol::protocol::CollabAgentSpawnBeginEvent;
use codex_protocol::protocol::CollabAgentSpawnEndEvent;
use codex_protocol::protocol::CreditsSnapshot;
use codex_protocol::protocol::Event;
use codex_protocol::protocol::EventMsg;
@@ -1838,6 +1841,7 @@ async fn make_chatwidget_manual(
plan_stream_controller: None,
last_copyable_output: None,
running_commands: HashMap::new(),
pending_collab_spawn_requests: HashMap::new(),
suppressed_exec_calls: HashSet::new(),
skills_all: Vec::new(),
skills_initial_state: None,
@@ -2011,6 +2015,48 @@ fn lines_to_single_string(lines: &[ratatui::text::Line<'static>]) -> String {
s
}
#[tokio::test]
async fn collab_spawn_end_shows_requested_model_and_effort() {
let (mut chat, mut rx, _ops) = make_chatwidget_manual(None).await;
let sender_thread_id = ThreadId::new();
let spawned_thread_id = ThreadId::new();
chat.handle_codex_event(Event {
id: "spawn-begin".into(),
msg: EventMsg::CollabAgentSpawnBegin(CollabAgentSpawnBeginEvent {
call_id: "call-spawn".to_string(),
sender_thread_id,
prompt: "Explore the repo".to_string(),
model: "gpt-5".to_string(),
reasoning_effort: ReasoningEffortConfig::High,
}),
});
chat.handle_codex_event(Event {
id: "spawn-end".into(),
msg: EventMsg::CollabAgentSpawnEnd(CollabAgentSpawnEndEvent {
call_id: "call-spawn".to_string(),
sender_thread_id,
new_thread_id: Some(spawned_thread_id),
new_agent_nickname: Some("Robie".to_string()),
new_agent_role: Some("explorer".to_string()),
prompt: "Explore the repo".to_string(),
status: AgentStatus::PendingInit,
}),
});
let cells = drain_insert_history(&mut rx);
let rendered = cells
.iter()
.map(|lines| lines_to_single_string(lines))
.collect::<Vec<_>>()
.join("\n");
assert!(
rendered.contains("Spawned Robie [explorer] (gpt-5 high)"),
"expected spawn line to include agent metadata and requested model, got {rendered:?}"
);
}
fn status_line_text(chat: &ChatWidget) -> Option<String> {
chat.status_line_text()
}
+63 -13
View File
@@ -375,14 +375,19 @@ impl HistoryCell for UserHistoryCell {
pub(crate) struct ReasoningSummaryCell {
_header: String,
content: String,
/// Session cwd used to render local file links inside the reasoning body.
cwd: PathBuf,
transcript_only: bool,
}
impl ReasoningSummaryCell {
pub(crate) fn new(header: String, content: String, transcript_only: bool) -> Self {
/// Create a reasoning summary cell that will render local file links relative to the session
/// cwd active when the summary was recorded.
pub(crate) fn new(header: String, content: String, cwd: &Path, transcript_only: bool) -> Self {
Self {
_header: header,
content,
cwd: cwd.to_path_buf(),
transcript_only,
}
}
@@ -392,6 +397,7 @@ impl ReasoningSummaryCell {
append_markdown(
&self.content,
Some((width as usize).saturating_sub(2)),
Some(self.cwd.as_path()),
&mut lines,
);
let summary_style = Style::default().dim().italic();
@@ -997,11 +1003,15 @@ pub(crate) fn padded_emoji(emoji: &str) -> String {
#[derive(Debug)]
struct TooltipHistoryCell {
tip: String,
cwd: PathBuf,
}
impl TooltipHistoryCell {
fn new(tip: String) -> Self {
Self { tip }
fn new(tip: String, cwd: &Path) -> Self {
Self {
tip,
cwd: cwd.to_path_buf(),
}
}
}
@@ -1016,6 +1026,7 @@ impl HistoryCell for TooltipHistoryCell {
append_markdown(
&format!("**Tip:** {}", self.tip),
Some(wrap_width),
Some(self.cwd.as_path()),
&mut lines,
);
@@ -1108,7 +1119,7 @@ pub(crate) fn new_session_info(
matches!(config.service_tier, Some(ServiceTier::Fast)),
)
})
.map(TooltipHistoryCell::new)
.map(|tip| TooltipHistoryCell::new(tip, &config.cwd))
{
parts.push(Box::new(tooltips));
}
@@ -2046,8 +2057,12 @@ pub(crate) fn new_plan_update(update: UpdatePlanArgs) -> PlanUpdateCell {
PlanUpdateCell { explanation, plan }
}
pub(crate) fn new_proposed_plan(plan_markdown: String) -> ProposedPlanCell {
ProposedPlanCell { plan_markdown }
/// Create a proposed-plan cell that snapshots the session cwd for later markdown rendering.
pub(crate) fn new_proposed_plan(plan_markdown: String, cwd: &Path) -> ProposedPlanCell {
ProposedPlanCell {
plan_markdown,
cwd: cwd.to_path_buf(),
}
}
pub(crate) fn new_proposed_plan_stream(
@@ -2063,6 +2078,8 @@ pub(crate) fn new_proposed_plan_stream(
#[derive(Debug)]
pub(crate) struct ProposedPlanCell {
plan_markdown: String,
/// Session cwd used to keep local file-link display aligned with live streamed plan rendering.
cwd: PathBuf,
}
#[derive(Debug)]
@@ -2081,7 +2098,12 @@ impl HistoryCell for ProposedPlanCell {
let plan_style = proposed_plan_style();
let wrap_width = width.saturating_sub(4).max(1) as usize;
let mut body: Vec<Line<'static>> = Vec::new();
append_markdown(&self.plan_markdown, Some(wrap_width), &mut body);
append_markdown(
&self.plan_markdown,
Some(wrap_width),
Some(self.cwd.as_path()),
&mut body,
);
if body.is_empty() {
body.push(Line::from("(empty)".dim().italic()));
}
@@ -2231,7 +2253,15 @@ pub(crate) fn new_image_generation_call(
PlainHistoryCell { lines }
}
pub(crate) fn new_reasoning_summary_block(full_reasoning_buffer: String) -> Box<dyn HistoryCell> {
/// Create the reasoning history cell emitted at the end of a reasoning block.
///
/// The helper snapshots `cwd` into the returned cell so local file links render the same way they
/// did while the turn was live, even if rendering happens after other app state has advanced.
pub(crate) fn new_reasoning_summary_block(
full_reasoning_buffer: String,
cwd: &Path,
) -> Box<dyn HistoryCell> {
let cwd = cwd.to_path_buf();
let full_reasoning_buffer = full_reasoning_buffer.trim();
if let Some(open) = full_reasoning_buffer.find("**") {
let after_open = &full_reasoning_buffer[(open + 2)..];
@@ -2242,9 +2272,12 @@ pub(crate) fn new_reasoning_summary_block(full_reasoning_buffer: String) -> Box<
if after_close_idx < full_reasoning_buffer.len() {
let header_buffer = full_reasoning_buffer[..after_close_idx].to_string();
let summary_buffer = full_reasoning_buffer[after_close_idx..].to_string();
// Preserve the session cwd so local file links render the same way in the
// collapsed reasoning block as they did while streaming live content.
return Box::new(ReasoningSummaryCell::new(
header_buffer,
summary_buffer,
&cwd,
false,
));
}
@@ -2253,6 +2286,7 @@ pub(crate) fn new_reasoning_summary_block(full_reasoning_buffer: String) -> Box<
Box::new(ReasoningSummaryCell::new(
"".to_string(),
full_reasoning_buffer.to_string(),
&cwd,
true,
))
}
@@ -2468,6 +2502,12 @@ mod tests {
.expect("config")
}
fn test_cwd() -> PathBuf {
// These tests only need a stable absolute cwd; using temp_dir() avoids baking Unix- or
// Windows-specific root semantics into the fixtures.
std::env::temp_dir()
}
fn render_lines(lines: &[Line<'static>]) -> Vec<String> {
lines
.iter()
@@ -3999,6 +4039,7 @@ mod tests {
fn reasoning_summary_block() {
let cell = new_reasoning_summary_block(
"**High level reasoning**\n\nDetailed reasoning goes here.".to_string(),
&test_cwd(),
);
let rendered_display = render_lines(&cell.display_lines(80));
@@ -4014,6 +4055,7 @@ mod tests {
let cell: Box<dyn HistoryCell> = Box::new(ReasoningSummaryCell::new(
"High level reasoning".to_string(),
summary.to_string(),
&test_cwd(),
false,
));
let width: u16 = 24;
@@ -4054,7 +4096,8 @@ mod tests {
#[test]
fn reasoning_summary_block_returns_reasoning_cell_when_feature_disabled() {
let cell = new_reasoning_summary_block("Detailed reasoning goes here.".to_string());
let cell =
new_reasoning_summary_block("Detailed reasoning goes here.".to_string(), &test_cwd());
let rendered = render_transcript(cell.as_ref());
assert_eq!(rendered, vec!["• Detailed reasoning goes here."]);
@@ -4067,6 +4110,7 @@ mod tests {
config.model_supports_reasoning_summaries = Some(true);
let cell = new_reasoning_summary_block(
"**High level reasoning**\n\nDetailed reasoning goes here.".to_string(),
&test_cwd(),
);
let rendered_display = render_lines(&cell.display_lines(80));
@@ -4075,8 +4119,10 @@ mod tests {
#[test]
fn reasoning_summary_block_falls_back_when_header_is_missing() {
let cell =
new_reasoning_summary_block("**High level reasoning without closing".to_string());
let cell = new_reasoning_summary_block(
"**High level reasoning without closing".to_string(),
&test_cwd(),
);
let rendered = render_transcript(cell.as_ref());
assert_eq!(rendered, vec!["• **High level reasoning without closing"]);
@@ -4084,14 +4130,17 @@ mod tests {
#[test]
fn reasoning_summary_block_falls_back_when_summary_is_missing() {
let cell =
new_reasoning_summary_block("**High level reasoning without closing**".to_string());
let cell = new_reasoning_summary_block(
"**High level reasoning without closing**".to_string(),
&test_cwd(),
);
let rendered = render_transcript(cell.as_ref());
assert_eq!(rendered, vec!["• High level reasoning without closing"]);
let cell = new_reasoning_summary_block(
"**High level reasoning without closing**\n\n ".to_string(),
&test_cwd(),
);
let rendered = render_transcript(cell.as_ref());
@@ -4102,6 +4151,7 @@ mod tests {
fn reasoning_summary_block_splits_header_and_summary_when_present() {
let cell = new_reasoning_summary_block(
"**High level plan**\n\nWe should fix the bug next.".to_string(),
&test_cwd(),
);
let rendered_display = render_lines(&cell.display_lines(80));
+17 -6
View File
@@ -1,10 +1,21 @@
use ratatui::text::Line;
use std::path::Path;
/// Render markdown into `lines` while resolving local file-link display relative to `cwd`.
///
/// Callers that already know the session working directory should pass it here so streamed and
/// non-streamed rendering show the same relative path text even if the process cwd differs.
pub(crate) fn append_markdown(
markdown_source: &str,
width: Option<usize>,
cwd: Option<&Path>,
lines: &mut Vec<Line<'static>>,
) {
let rendered = crate::markdown_render::render_markdown_text_with_width(markdown_source, width);
let rendered = crate::markdown_render::render_markdown_text_with_width_and_cwd(
markdown_source,
width,
cwd,
);
crate::render::line_utils::push_owned_lines(&rendered.lines, lines);
}
@@ -30,7 +41,7 @@ mod tests {
fn citations_render_as_plain_text() {
let src = "Before 【F:/x.rs†L1】\nAfter 【F:/x.rs†L3】\n";
let mut out = Vec::new();
append_markdown(src, None, &mut out);
append_markdown(src, None, None, &mut out);
let rendered = lines_to_strings(&out);
assert_eq!(
rendered,
@@ -46,7 +57,7 @@ mod tests {
// Basic sanity: indented code with surrounding blank lines should produce the indented line.
let src = "Before\n\n code 1\n\nAfter\n";
let mut out = Vec::new();
append_markdown(src, None, &mut out);
append_markdown(src, None, None, &mut out);
let lines = lines_to_strings(&out);
assert_eq!(lines, vec!["Before", "", " code 1", "", "After"]);
}
@@ -55,7 +66,7 @@ mod tests {
fn append_markdown_preserves_full_text_line() {
let src = "Hi! How can I help with codex-rs today? Want me to explore the repo, run tests, or work on a specific change?\n";
let mut out = Vec::new();
append_markdown(src, None, &mut out);
append_markdown(src, None, None, &mut out);
assert_eq!(
out.len(),
1,
@@ -76,7 +87,7 @@ mod tests {
#[test]
fn append_markdown_matches_tui_markdown_for_ordered_item() {
let mut out = Vec::new();
append_markdown("1. Tight item\n", None, &mut out);
append_markdown("1. Tight item\n", None, None, &mut out);
let lines = lines_to_strings(&out);
assert_eq!(lines, vec!["1. Tight item".to_string()]);
}
@@ -85,7 +96,7 @@ mod tests {
fn append_markdown_keeps_ordered_list_line_unsplit_in_context() {
let src = "Loose vs. tight list items:\n1. Tight item\n";
let mut out = Vec::new();
append_markdown(src, None, &mut out);
append_markdown(src, None, None, &mut out);
let lines = lines_to_strings(&out);
+323 -73
View File
@@ -1,8 +1,16 @@
//! Markdown rendering for the TUI transcript.
//!
//! This renderer intentionally treats local file links differently from normal web links. For
//! local paths, the displayed text comes from the destination, not the markdown label, so
//! transcripts show the real file target (including normalized location suffixes) and can shorten
//! absolute paths relative to a known working directory.
use crate::render::highlight::highlight_code_to_lines;
use crate::render::line_utils::line_to_static;
use crate::wrapping::RtOptions;
use crate::wrapping::adaptive_wrap_line;
use codex_utils_string::normalize_markdown_hash_location_suffix;
use dirs::home_dir;
use pulldown_cmark::CodeBlockKind;
use pulldown_cmark::CowStr;
use pulldown_cmark::Event;
@@ -16,7 +24,10 @@ use ratatui::text::Line;
use ratatui::text::Span;
use ratatui::text::Text;
use regex_lite::Regex;
use std::path::Path;
use std::path::PathBuf;
use std::sync::LazyLock;
use url::Url;
struct MarkdownStyles {
h1: Style,
@@ -79,11 +90,26 @@ pub fn render_markdown_text(input: &str) -> Text<'static> {
render_markdown_text_with_width(input, None)
}
/// Render markdown using the current process working directory for local file-link display.
pub(crate) fn render_markdown_text_with_width(input: &str, width: Option<usize>) -> Text<'static> {
let cwd = std::env::current_dir().ok();
render_markdown_text_with_width_and_cwd(input, width, cwd.as_deref())
}
/// Render markdown with an explicit working directory for local file links.
///
/// The `cwd` parameter controls how absolute local targets are shortened before display. Passing
/// the session cwd keeps full renders, history cells, and streamed deltas visually aligned even
/// when rendering happens away from the process cwd.
pub(crate) fn render_markdown_text_with_width_and_cwd(
input: &str,
width: Option<usize>,
cwd: Option<&Path>,
) -> Text<'static> {
let mut options = Options::empty();
options.insert(Options::ENABLE_STRIKETHROUGH);
let parser = Parser::new_ext(input, options);
let mut w = Writer::new(parser, width);
let mut w = Writer::new(parser, width, cwd);
w.run();
w.text
}
@@ -92,9 +118,11 @@ pub(crate) fn render_markdown_text_with_width(input: &str, width: Option<usize>)
struct LinkState {
destination: String,
show_destination: bool,
hidden_location_suffix: Option<String>,
label_start_span_idx: usize,
label_styled: bool,
/// Pre-rendered display text for local file links.
///
/// When this is present, the markdown label is intentionally suppressed so the rendered
/// transcript always reflects the real target path.
local_target_display: Option<String>,
}
fn should_render_link_destination(dest_url: &str) -> bool {
@@ -116,20 +144,6 @@ static HASH_LOCATION_SUFFIX_RE: LazyLock<Regex> =
Err(error) => panic!("invalid hash location regex: {error}"),
});
fn is_local_path_like_link(dest_url: &str) -> bool {
dest_url.starts_with("file://")
|| dest_url.starts_with('/')
|| dest_url.starts_with("~/")
|| dest_url.starts_with("./")
|| dest_url.starts_with("../")
|| dest_url.starts_with("\\\\")
|| matches!(
dest_url.as_bytes(),
[drive, b':', separator, ..]
if drive.is_ascii_alphabetic() && matches!(separator, b'/' | b'\\')
)
}
struct Writer<'a, I>
where
I: Iterator<Item = Event<'a>>,
@@ -148,6 +162,9 @@ where
code_block_lang: Option<String>,
code_block_buffer: String,
wrap_width: Option<usize>,
cwd: Option<PathBuf>,
line_ends_with_local_link_target: bool,
pending_local_link_soft_break: bool,
current_line_content: Option<Line<'static>>,
current_initial_indent: Vec<Span<'static>>,
current_subsequent_indent: Vec<Span<'static>>,
@@ -159,7 +176,7 @@ impl<'a, I> Writer<'a, I>
where
I: Iterator<Item = Event<'a>>,
{
fn new(iter: I, wrap_width: Option<usize>) -> Self {
fn new(iter: I, wrap_width: Option<usize>, cwd: Option<&Path>) -> Self {
Self {
iter,
text: Text::default(),
@@ -175,6 +192,9 @@ where
code_block_lang: None,
code_block_buffer: String::new(),
wrap_width,
cwd: cwd.map(Path::to_path_buf),
line_ends_with_local_link_target: false,
pending_local_link_soft_break: false,
current_line_content: None,
current_initial_indent: Vec::new(),
current_subsequent_indent: Vec::new(),
@@ -191,6 +211,7 @@ where
}
fn handle_event(&mut self, event: Event<'a>) {
self.prepare_for_event(&event);
match event {
Event::Start(tag) => self.start_tag(tag),
Event::End(tag) => self.end_tag(tag),
@@ -213,6 +234,23 @@ where
}
}
fn prepare_for_event(&mut self, event: &Event<'a>) {
if !self.pending_local_link_soft_break {
return;
}
// Local file links render from the destination at `TagEnd::Link`, so a Markdown soft break
// immediately before a descriptive `: ...` should stay inline instead of splitting the
// list item across two lines.
if matches!(event, Event::Text(text) if text.trim_start().starts_with(':')) {
self.pending_local_link_soft_break = false;
return;
}
self.pending_local_link_soft_break = false;
self.push_line(Line::default());
}
fn start_tag(&mut self, tag: Tag<'a>) {
match tag {
Tag::Paragraph => self.start_paragraph(),
@@ -324,6 +362,10 @@ where
}
fn text(&mut self, text: CowStr<'a>) {
if self.suppressing_local_link_label() {
return;
}
self.line_ends_with_local_link_target = false;
if self.pending_marker_line {
self.push_line(Line::default());
}
@@ -373,6 +415,10 @@ where
}
fn code(&mut self, code: CowStr<'a>) {
if self.suppressing_local_link_label() {
return;
}
self.line_ends_with_local_link_target = false;
if self.pending_marker_line {
self.push_line(Line::default());
self.pending_marker_line = false;
@@ -382,6 +428,10 @@ where
}
fn html(&mut self, html: CowStr<'a>, inline: bool) {
if self.suppressing_local_link_label() {
return;
}
self.line_ends_with_local_link_target = false;
self.pending_marker_line = false;
for (i, line) in html.lines().enumerate() {
if self.needs_newline {
@@ -398,10 +448,23 @@ where
}
fn hard_break(&mut self) {
if self.suppressing_local_link_label() {
return;
}
self.line_ends_with_local_link_target = false;
self.push_line(Line::default());
}
fn soft_break(&mut self) {
if self.suppressing_local_link_label() {
return;
}
if self.line_ends_with_local_link_target {
self.pending_local_link_soft_break = true;
self.line_ends_with_local_link_target = false;
return;
}
self.line_ends_with_local_link_target = false;
self.push_line(Line::default());
}
@@ -513,36 +576,13 @@ where
fn push_link(&mut self, dest_url: String) {
let show_destination = should_render_link_destination(&dest_url);
let label_styled = !show_destination;
let label_start_span_idx = self
.current_line_content
.as_ref()
.map(|line| line.spans.len())
.unwrap_or(0);
if label_styled {
self.push_inline_style(self.styles.code);
}
self.link = Some(LinkState {
show_destination,
hidden_location_suffix: if is_local_path_like_link(&dest_url) {
dest_url
.rsplit_once('#')
.and_then(|(_, fragment)| {
HASH_LOCATION_SUFFIX_RE
.is_match(fragment)
.then(|| format!("#{fragment}"))
})
.and_then(|suffix| normalize_markdown_hash_location_suffix(&suffix))
.or_else(|| {
COLON_LOCATION_SUFFIX_RE
.find(&dest_url)
.map(|m| m.as_str().to_string())
})
local_target_display: if is_local_path_like_link(&dest_url) {
render_local_link_target(&dest_url, self.cwd.as_deref())
} else {
None
},
label_start_span_idx,
label_styled,
destination: dest_url,
});
}
@@ -550,43 +590,34 @@ where
fn pop_link(&mut self) {
if let Some(link) = self.link.take() {
if link.show_destination {
if link.label_styled {
self.pop_inline_style();
}
self.push_span(" (".into());
self.push_span(Span::styled(link.destination, self.styles.link));
self.push_span(")".into());
} else if let Some(location_suffix) = link.hidden_location_suffix.as_deref() {
let label_text = self
.current_line_content
.as_ref()
.and_then(|line| {
line.spans.get(link.label_start_span_idx..).map(|spans| {
spans
.iter()
.map(|span| span.content.as_ref())
.collect::<String>()
})
})
.unwrap_or_default();
if label_text
.rsplit_once('#')
.is_some_and(|(_, fragment)| HASH_LOCATION_SUFFIX_RE.is_match(fragment))
|| COLON_LOCATION_SUFFIX_RE.find(&label_text).is_some()
{
// The label already carries a location suffix; don't duplicate it.
} else {
self.push_span(Span::styled(location_suffix.to_string(), self.styles.code));
} else if let Some(local_target_display) = link.local_target_display {
if self.pending_marker_line {
self.push_line(Line::default());
}
if link.label_styled {
self.pop_inline_style();
}
} else if link.label_styled {
self.pop_inline_style();
// Local file links are rendered as code-like path text so the transcript shows the
// resolved target instead of arbitrary caller-provided label text.
let style = self
.inline_styles
.last()
.copied()
.unwrap_or_default()
.patch(self.styles.code);
self.push_span(Span::styled(local_target_display, style));
self.line_ends_with_local_link_target = true;
}
}
}
fn suppressing_local_link_label(&self) -> bool {
self.link
.as_ref()
.and_then(|link| link.local_target_display.as_ref())
.is_some()
}
fn flush_current_line(&mut self) {
if let Some(line) = self.current_line_content.take() {
let style = self.current_line_style;
@@ -610,6 +641,7 @@ where
self.current_initial_indent.clear();
self.current_subsequent_indent.clear();
self.current_line_in_code_block = false;
self.line_ends_with_local_link_target = false;
}
}
@@ -631,6 +663,7 @@ where
self.current_line_style = style;
self.current_line_content = Some(line);
self.current_line_in_code_block = self.in_code_block;
self.line_ends_with_local_link_target = false;
self.pending_marker_line = false;
}
@@ -687,6 +720,223 @@ where
}
}
fn is_local_path_like_link(dest_url: &str) -> bool {
dest_url.starts_with("file://")
|| dest_url.starts_with('/')
|| dest_url.starts_with("~/")
|| dest_url.starts_with("./")
|| dest_url.starts_with("../")
|| dest_url.starts_with("\\\\")
|| matches!(
dest_url.as_bytes(),
[drive, b':', separator, ..]
if drive.is_ascii_alphabetic() && matches!(separator, b'/' | b'\\')
)
}
/// Parse a local link target into normalized path text plus an optional location suffix.
///
/// This accepts the path shapes Codex emits today: `file://` URLs, absolute and relative paths,
/// `~/...`, Windows paths, and `#L..C..` or `:line:col` suffixes.
fn render_local_link_target(dest_url: &str, cwd: Option<&Path>) -> Option<String> {
let (path_text, location_suffix) = parse_local_link_target(dest_url)?;
let mut rendered = display_local_link_path(&path_text, cwd);
if let Some(location_suffix) = location_suffix {
rendered.push_str(&location_suffix);
}
Some(rendered)
}
/// Split a local-link destination into `(normalized_path_text, location_suffix)`.
///
/// The returned path text never includes a trailing `#L..` or `:line[:col]` suffix. Path
/// normalization expands `~/...` when possible and rewrites path separators into display-stable
/// forward slashes. The suffix, when present, is returned separately in normalized markdown form.
///
/// Returns `None` only when the destination looks like a `file://` URL but cannot be parsed into a
/// local path. Plain path-like inputs always return `Some(...)` even if they are relative.
fn parse_local_link_target(dest_url: &str) -> Option<(String, Option<String>)> {
if dest_url.starts_with("file://") {
let url = Url::parse(dest_url).ok()?;
let path_text = file_url_to_local_path_text(&url)?;
let location_suffix = url
.fragment()
.and_then(normalize_hash_location_suffix_fragment);
return Some((path_text, location_suffix));
}
let mut path_text = dest_url;
let mut location_suffix = None;
// Prefer `#L..` style fragments when both forms are present so URLs like `path#L10` do not
// get misparsed as a plain path ending in `:10`.
if let Some((candidate_path, fragment)) = dest_url.rsplit_once('#')
&& let Some(normalized) = normalize_hash_location_suffix_fragment(fragment)
{
path_text = candidate_path;
location_suffix = Some(normalized);
}
if location_suffix.is_none()
&& let Some(suffix) = extract_colon_location_suffix(path_text)
{
let path_len = path_text.len().saturating_sub(suffix.len());
path_text = &path_text[..path_len];
location_suffix = Some(suffix);
}
Some((expand_local_link_path(path_text), location_suffix))
}
/// Normalize a hash fragment like `L12` or `L12C3-L14C9` into the display suffix we render.
///
/// Returns `None` for fragments that are not location references. This deliberately ignores other
/// `#...` fragments so non-location hashes stay part of the path text.
fn normalize_hash_location_suffix_fragment(fragment: &str) -> Option<String> {
HASH_LOCATION_SUFFIX_RE
.is_match(fragment)
.then(|| format!("#{fragment}"))
.and_then(|suffix| normalize_markdown_hash_location_suffix(&suffix))
}
/// Extract a trailing `:line`, `:line:col`, or range suffix from a plain path-like string.
///
/// The suffix must occur at the end of the input; embedded colons elsewhere in the path are left
/// alone. This is what keeps Windows drive letters like `C:/...` from being misread as locations.
fn extract_colon_location_suffix(path_text: &str) -> Option<String> {
COLON_LOCATION_SUFFIX_RE
.find(path_text)
.filter(|matched| matched.end() == path_text.len())
.map(|matched| matched.as_str().to_string())
}
/// Expand home-relative paths and normalize separators for display.
///
/// If `~/...` cannot be expanded because the home directory is unavailable, the original text still
/// goes through separator normalization and is returned as-is otherwise.
fn expand_local_link_path(path_text: &str) -> String {
// Expand `~/...` eagerly so home-relative links can participate in the same normalization and
// cwd-relative shortening path as absolute links.
if let Some(rest) = path_text.strip_prefix("~/")
&& let Some(home) = home_dir()
{
return normalize_local_link_path_text(&home.join(rest).to_string_lossy());
}
normalize_local_link_path_text(path_text)
}
/// Convert a `file://` URL into the normalized local-path text used for transcript rendering.
///
/// This prefers `Url::to_file_path()` for standard file URLs. When that rejects Windows-oriented
/// encodings, we reconstruct a display path from the host/path parts so UNC paths and drive-letter
/// URLs still render sensibly.
fn file_url_to_local_path_text(url: &Url) -> Option<String> {
if let Ok(path) = url.to_file_path() {
return Some(normalize_local_link_path_text(&path.to_string_lossy()));
}
// Fall back to string reconstruction for cases `to_file_path()` rejects, especially UNC-style
// hosts and Windows drive paths encoded in URL form.
let mut path_text = url.path().to_string();
if let Some(host) = url.host_str()
&& !host.is_empty()
&& host != "localhost"
{
path_text = format!("//{host}{path_text}");
} else if matches!(
path_text.as_bytes(),
[b'/', drive, b':', b'/', ..] if drive.is_ascii_alphabetic()
) {
path_text.remove(0);
}
Some(normalize_local_link_path_text(&path_text))
}
/// Normalize local-path text into the transcript display form.
///
/// Display normalization is intentionally lexical: it does not touch the filesystem, resolve
/// symlinks, or collapse `.` / `..`. It only converts separators to forward slashes and rewrites
/// UNC-style `\\\\server\\share` inputs into `//server/share` so later prefix checks operate on a
/// stable representation.
fn normalize_local_link_path_text(path_text: &str) -> String {
// Render all local link paths with forward slashes so display and prefix stripping are stable
// across mixed Windows and Unix-style inputs.
if let Some(rest) = path_text.strip_prefix("\\\\") {
format!("//{}", rest.replace('\\', "/").trim_start_matches('/'))
} else {
path_text.replace('\\', "/")
}
}
fn is_absolute_local_link_path(path_text: &str) -> bool {
path_text.starts_with('/')
|| path_text.starts_with("//")
|| matches!(
path_text.as_bytes(),
[drive, b':', b'/', ..] if drive.is_ascii_alphabetic()
)
}
/// Remove trailing separators from a local path without destroying root semantics.
///
/// Roots like `/`, `//`, and `C:/` stay intact so callers can still distinguish "the root itself"
/// from "a path under the root".
fn trim_trailing_local_path_separator(path_text: &str) -> &str {
if path_text == "/" || path_text == "//" {
return path_text;
}
if matches!(path_text.as_bytes(), [drive, b':', b'/'] if drive.is_ascii_alphabetic()) {
return path_text;
}
path_text.trim_end_matches('/')
}
/// Strip `cwd_text` from the start of `path_text` when `path_text` is strictly underneath it.
///
/// Returns the relative remainder without a leading slash. If the path equals the cwd exactly, this
/// returns `None` so callers can keep rendering the full path instead of collapsing it to an empty
/// string.
fn strip_local_path_prefix<'a>(path_text: &'a str, cwd_text: &str) -> Option<&'a str> {
let path_text = trim_trailing_local_path_separator(path_text);
let cwd_text = trim_trailing_local_path_separator(cwd_text);
if path_text == cwd_text {
return None;
}
// Treat filesystem roots specially so `/tmp/x` under `/` becomes `tmp/x` instead of being
// left unchanged by the generic prefix-stripping branch.
if cwd_text == "/" || cwd_text == "//" {
return path_text.strip_prefix('/');
}
path_text
.strip_prefix(cwd_text)
.and_then(|rest| rest.strip_prefix('/'))
}
/// Choose the visible path text for a local link after normalization.
///
/// Relative paths stay relative. Absolute paths are shortened against `cwd` only when they are
/// lexically underneath it; otherwise the absolute path is preserved. This is display logic only,
/// not filesystem canonicalization.
fn display_local_link_path(path_text: &str, cwd: Option<&Path>) -> String {
let path_text = normalize_local_link_path_text(path_text);
if !is_absolute_local_link_path(&path_text) {
return path_text;
}
if let Some(cwd) = cwd {
// Only shorten absolute paths that are under the provided session cwd; otherwise preserve
// the original absolute target for clarity.
let cwd_text = normalize_local_link_path_text(&cwd.to_string_lossy());
if let Some(stripped) = strip_local_path_prefix(&path_text, &cwd_text) {
return stripped.to_string();
}
}
path_text
}
#[cfg(test)]
mod markdown_render_tests {
include!("markdown_render_tests.rs");
+128 -40
View File
@@ -3,12 +3,18 @@ use ratatui::style::Stylize;
use ratatui::text::Line;
use ratatui::text::Span;
use ratatui::text::Text;
use std::path::Path;
use crate::markdown_render::COLON_LOCATION_SUFFIX_RE;
use crate::markdown_render::HASH_LOCATION_SUFFIX_RE;
use crate::markdown_render::render_markdown_text;
use crate::markdown_render::render_markdown_text_with_width_and_cwd;
use insta::assert_snapshot;
fn render_markdown_text_for_cwd(input: &str, cwd: &Path) -> Text<'static> {
render_markdown_text_with_width_and_cwd(input, None, Some(cwd))
}
#[test]
fn empty() {
assert_eq!(render_markdown_text(""), Text::default());
@@ -661,8 +667,9 @@ fn load_location_suffix_regexes() {
#[test]
fn file_link_hides_destination() {
let text = render_markdown_text(
let text = render_markdown_text_for_cwd(
"[codex-rs/tui/src/markdown_render.rs](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs".cyan()]));
assert_eq!(text, expected);
@@ -670,97 +677,101 @@ fn file_link_hides_destination() {
#[test]
fn file_link_appends_line_number_when_label_lacks_it() {
let text = render_markdown_text(
let text = render_markdown_text_for_cwd(
"[markdown_render.rs](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter([
"markdown_render.rs".cyan(),
":74".cyan(),
]));
let expected = Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs:74".cyan()]));
assert_eq!(text, expected);
}
#[test]
fn file_link_uses_label_for_line_number() {
let text = render_markdown_text(
"[markdown_render.rs:74](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74)",
fn file_link_keeps_absolute_paths_outside_cwd() {
let text = render_markdown_text_for_cwd(
"[README.md:74](/Users/example/code/codex/README.md:74)",
Path::new("/Users/example/code/codex/codex-rs/tui"),
);
let expected = Text::from(Line::from_iter(["markdown_render.rs:74".cyan()]));
let expected = Text::from(Line::from_iter(["/Users/example/code/codex/README.md:74".cyan()]));
assert_eq!(text, expected);
}
#[test]
fn file_link_appends_hash_anchor_when_label_lacks_it() {
let text = render_markdown_text(
let text = render_markdown_text_for_cwd(
"[markdown_render.rs](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter([
"markdown_render.rs".cyan(),
":74:3".cyan(),
]));
let expected =
Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs:74:3".cyan()]));
assert_eq!(text, expected);
}
#[test]
fn file_link_uses_label_for_hash_anchor() {
let text = render_markdown_text(
fn file_link_uses_target_path_for_hash_anchor() {
let text = render_markdown_text_for_cwd(
"[markdown_render.rs#L74C3](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter(["markdown_render.rs#L74C3".cyan()]));
let expected =
Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs:74:3".cyan()]));
assert_eq!(text, expected);
}
#[test]
fn file_link_appends_range_when_label_lacks_it() {
let text = render_markdown_text(
let text = render_markdown_text_for_cwd(
"[markdown_render.rs](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74:3-76:9)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter([
"markdown_render.rs".cyan(),
":74:3-76:9".cyan(),
]));
let expected =
Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan()]));
assert_eq!(text, expected);
}
#[test]
fn file_link_uses_label_for_range() {
let text = render_markdown_text(
fn file_link_uses_target_path_for_range() {
let text = render_markdown_text_for_cwd(
"[markdown_render.rs:74:3-76:9](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74:3-76:9)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter(["markdown_render.rs:74:3-76:9".cyan()]));
let expected =
Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan()]));
assert_eq!(text, expected);
}
#[test]
fn file_link_appends_hash_range_when_label_lacks_it() {
let text = render_markdown_text(
let text = render_markdown_text_for_cwd(
"[markdown_render.rs](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3-L76C9)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter([
"markdown_render.rs".cyan(),
":74:3-76:9".cyan(),
]));
let expected =
Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan()]));
assert_eq!(text, expected);
}
#[test]
fn multiline_file_link_label_after_styled_prefix_does_not_panic() {
let text = render_markdown_text(
let text = render_markdown_text_for_cwd(
"**bold** plain [foo\nbar](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from_iter([
Line::from_iter(["bold".bold(), " plain ".into(), "foo".cyan()]),
Line::from_iter(["bar".cyan(), ":74:3".cyan()]),
]);
let expected = Text::from(Line::from_iter([
"bold".bold(),
" plain ".into(),
"codex-rs/tui/src/markdown_render.rs:74:3".cyan(),
]));
assert_eq!(text, expected);
}
#[test]
fn file_link_uses_label_for_hash_range() {
let text = render_markdown_text(
fn file_link_uses_target_path_for_hash_range() {
let text = render_markdown_text_for_cwd(
"[markdown_render.rs#L74C3-L76C9](file:///Users/example/code/codex/codex-rs/tui/src/markdown_render.rs#L74C3-L76C9)",
Path::new("/Users/example/code/codex"),
);
let expected = Text::from(Line::from_iter(["markdown_render.rs#L74C3-L76C9".cyan()]));
let expected =
Text::from(Line::from_iter(["codex-rs/tui/src/markdown_render.rs:74:3-76:9".cyan()]));
assert_eq!(text, expected);
}
@@ -778,8 +789,9 @@ fn url_link_shows_destination() {
#[test]
fn markdown_render_file_link_snapshot() {
let text = render_markdown_text(
let text = render_markdown_text_for_cwd(
"See [markdown_render.rs:74](/Users/example/code/codex/codex-rs/tui/src/markdown_render.rs:74).",
Path::new("/Users/example/code/codex"),
);
let rendered = text
.lines
@@ -796,6 +808,82 @@ fn markdown_render_file_link_snapshot() {
assert_snapshot!(rendered);
}
#[test]
fn unordered_list_local_file_link_stays_inline_with_following_text() {
let text = render_markdown_text_with_width_and_cwd(
"- [binary](/Users/example/code/codex/codex-rs/README.md:93): core is the agent/business logic, tui is the terminal UI, exec is the headless automation surface, and cli is the top-level multitool binary.",
Some(72),
Some(Path::new("/Users/example/code/codex")),
);
let rendered = text
.lines
.iter()
.map(|line| {
line.spans
.iter()
.map(|span| span.content.as_ref())
.collect::<String>()
})
.collect::<Vec<_>>();
assert_eq!(
rendered,
vec![
"- codex-rs/README.md:93: core is the agent/business logic, tui is the",
" terminal UI, exec is the headless automation surface, and cli is the",
" top-level multitool binary.",
]
);
}
#[test]
fn unordered_list_local_file_link_soft_break_before_colon_stays_inline() {
let text = render_markdown_text_with_width_and_cwd(
"- [binary](/Users/example/code/codex/codex-rs/README.md:93)\n : core is the agent/business logic.",
Some(72),
Some(Path::new("/Users/example/code/codex")),
);
let rendered = text
.lines
.iter()
.map(|line| {
line.spans
.iter()
.map(|span| span.content.as_ref())
.collect::<String>()
})
.collect::<Vec<_>>();
assert_eq!(
rendered,
vec!["- codex-rs/README.md:93: core is the agent/business logic.",]
);
}
#[test]
fn consecutive_unordered_list_local_file_links_do_not_detach_paths() {
let text = render_markdown_text_with_width_and_cwd(
"- [binary](/Users/example/code/codex/codex-rs/README.md:93)\n : cli is the top-level multitool binary.\n- [expectations](/Users/example/code/codex/codex-rs/core/README.md:1)\n : codex-core owns the real runtime behavior.",
Some(72),
Some(Path::new("/Users/example/code/codex")),
);
let rendered = text
.lines
.iter()
.map(|line| {
line.spans
.iter()
.map(|span| span.content.as_ref())
.collect::<String>()
})
.collect::<Vec<_>>();
assert_eq!(
rendered,
vec![
"- codex-rs/README.md:93: cli is the top-level multitool binary.",
"- codex-rs/core/README.md:1: codex-core owns the real runtime behavior.",
]
);
}
#[test]
fn code_block_known_lang_has_syntax_colors() {
let text = render_markdown_text("```rust\nfn main() {}\n```\n");
+35 -13
View File
@@ -1,4 +1,6 @@
use ratatui::text::Line;
use std::path::Path;
use std::path::PathBuf;
use crate::markdown;
@@ -8,14 +10,22 @@ pub(crate) struct MarkdownStreamCollector {
buffer: String,
committed_line_count: usize,
width: Option<usize>,
cwd: PathBuf,
}
impl MarkdownStreamCollector {
pub fn new(width: Option<usize>) -> Self {
/// Create a collector that renders markdown using `cwd` for local file-link display.
///
/// The collector snapshots `cwd` into owned state because stream commits can happen long after
/// construction. The same `cwd` should be reused for the entire stream lifecycle; mixing
/// different working directories within one stream would make the same link render with
/// different path prefixes across incremental commits.
pub fn new(width: Option<usize>, cwd: &Path) -> Self {
Self {
buffer: String::new(),
committed_line_count: 0,
width,
cwd: cwd.to_path_buf(),
}
}
@@ -41,7 +51,7 @@ impl MarkdownStreamCollector {
return Vec::new();
};
let mut rendered: Vec<Line<'static>> = Vec::new();
markdown::append_markdown(&source, self.width, &mut rendered);
markdown::append_markdown(&source, self.width, Some(self.cwd.as_path()), &mut rendered);
let mut complete_line_count = rendered.len();
if complete_line_count > 0
&& crate::render::line_utils::is_blank_line_spaces_only(
@@ -82,7 +92,7 @@ impl MarkdownStreamCollector {
tracing::trace!("markdown finalize (raw source):\n---\n{source}\n---");
let mut rendered: Vec<Line<'static>> = Vec::new();
markdown::append_markdown(&source, self.width, &mut rendered);
markdown::append_markdown(&source, self.width, Some(self.cwd.as_path()), &mut rendered);
let out = if self.committed_line_count >= rendered.len() {
Vec::new()
@@ -96,12 +106,19 @@ impl MarkdownStreamCollector {
}
}
#[cfg(test)]
fn test_cwd() -> PathBuf {
// These tests only need a stable absolute cwd; using temp_dir() avoids baking Unix- or
// Windows-specific root semantics into the fixtures.
std::env::temp_dir()
}
#[cfg(test)]
pub(crate) fn simulate_stream_markdown_for_tests(
deltas: &[&str],
finalize: bool,
) -> Vec<Line<'static>> {
let mut collector = MarkdownStreamCollector::new(None);
let mut collector = MarkdownStreamCollector::new(None, &test_cwd());
let mut out = Vec::new();
for d in deltas {
collector.push_delta(d);
@@ -122,7 +139,7 @@ mod tests {
#[tokio::test]
async fn no_commit_until_newline() {
let mut c = super::MarkdownStreamCollector::new(None);
let mut c = super::MarkdownStreamCollector::new(None, &super::test_cwd());
c.push_delta("Hello, world");
let out = c.commit_complete_lines();
assert!(out.is_empty(), "should not commit without newline");
@@ -133,7 +150,7 @@ mod tests {
#[tokio::test]
async fn finalize_commits_partial_line() {
let mut c = super::MarkdownStreamCollector::new(None);
let mut c = super::MarkdownStreamCollector::new(None, &super::test_cwd());
c.push_delta("Line without newline");
let out = c.finalize_and_drain();
assert_eq!(out.len(), 1);
@@ -253,7 +270,7 @@ mod tests {
async fn heading_starts_on_new_line_when_following_paragraph() {
// Stream a paragraph line, then a heading on the next line.
// Expect two distinct rendered lines: "Hello." and "Heading".
let mut c = super::MarkdownStreamCollector::new(None);
let mut c = super::MarkdownStreamCollector::new(None, &super::test_cwd());
c.push_delta("Hello.\n");
let out1 = c.commit_complete_lines();
let s1: Vec<String> = out1
@@ -309,7 +326,7 @@ mod tests {
// Paragraph without trailing newline, then a chunk that starts with the newline
// and the heading text, then a final newline. The collector should first commit
// only the paragraph line, and later commit the heading as its own line.
let mut c = super::MarkdownStreamCollector::new(None);
let mut c = super::MarkdownStreamCollector::new(None, &super::test_cwd());
c.push_delta("Sounds good!");
// No commit yet
assert!(c.commit_complete_lines().is_empty());
@@ -354,7 +371,8 @@ mod tests {
// Sanity check raw markdown rendering for a simple line does not produce spurious extras.
let mut rendered: Vec<ratatui::text::Line<'static>> = Vec::new();
crate::markdown::append_markdown("Hello.\n", None, &mut rendered);
let test_cwd = super::test_cwd();
crate::markdown::append_markdown("Hello.\n", None, Some(test_cwd.as_path()), &mut rendered);
let rendered_strings: Vec<String> = rendered
.iter()
.map(|l| {
@@ -414,7 +432,8 @@ mod tests {
let streamed_str = lines_to_plain_strings(&streamed);
let mut rendered_all: Vec<ratatui::text::Line<'static>> = Vec::new();
crate::markdown::append_markdown(input, None, &mut rendered_all);
let test_cwd = super::test_cwd();
crate::markdown::append_markdown(input, None, Some(test_cwd.as_path()), &mut rendered_all);
let rendered_all_str = lines_to_plain_strings(&rendered_all);
assert_eq!(
@@ -520,7 +539,8 @@ mod tests {
let full: String = deltas.iter().copied().collect();
let mut rendered_all: Vec<ratatui::text::Line<'static>> = Vec::new();
crate::markdown::append_markdown(&full, None, &mut rendered_all);
let test_cwd = super::test_cwd();
crate::markdown::append_markdown(&full, None, Some(test_cwd.as_path()), &mut rendered_all);
let rendered_all_strs = lines_to_plain_strings(&rendered_all);
assert_eq!(
@@ -608,7 +628,8 @@ mod tests {
// Compute a full render for diagnostics only.
let full: String = deltas.iter().copied().collect();
let mut rendered_all: Vec<ratatui::text::Line<'static>> = Vec::new();
crate::markdown::append_markdown(&full, None, &mut rendered_all);
let test_cwd = super::test_cwd();
crate::markdown::append_markdown(&full, None, Some(test_cwd.as_path()), &mut rendered_all);
// Also assert exact expected plain strings for clarity.
let expected = vec![
@@ -635,7 +656,8 @@ mod tests {
let streamed_strs = lines_to_plain_strings(&streamed);
let full: String = deltas.iter().copied().collect();
let mut rendered: Vec<ratatui::text::Line<'static>> = Vec::new();
crate::markdown::append_markdown(&full, None, &mut rendered);
let test_cwd = super::test_cwd();
crate::markdown::append_markdown(&full, None, Some(test_cwd.as_path()), &mut rendered);
let rendered_strs = lines_to_plain_strings(&rendered);
assert_eq!(streamed_strs, rendered_strs, "full:\n---\n{full}\n---");
}
+74 -21
View File
@@ -2,6 +2,7 @@ use crate::history_cell::PlainHistoryCell;
use crate::render::line_utils::prefix_lines;
use crate::text_formatting::truncate_text;
use codex_protocol::ThreadId;
use codex_protocol::openai_models::ReasoningEffort as ReasoningEffortConfig;
use codex_protocol::protocol::AgentStatus;
use codex_protocol::protocol::CollabAgentInteractionEndEvent;
use codex_protocol::protocol::CollabAgentRef;
@@ -36,6 +37,12 @@ struct AgentLabel<'a> {
role: Option<&'a str>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct SpawnRequestSummary {
pub(crate) model: String,
pub(crate) reasoning_effort: ReasoningEffortConfig,
}
pub(crate) fn agent_picker_status_dot_spans(is_closed: bool) -> Vec<Span<'static>> {
let dot = if is_closed {
"•".into()
@@ -74,7 +81,10 @@ pub(crate) fn sort_agent_picker_threads(agent_threads: &mut [(ThreadId, AgentPic
});
}
pub(crate) fn spawn_end(ev: CollabAgentSpawnEndEvent) -> PlainHistoryCell {
pub(crate) fn spawn_end(
ev: CollabAgentSpawnEndEvent,
spawn_request: Option<&SpawnRequestSummary>,
) -> PlainHistoryCell {
let CollabAgentSpawnEndEvent {
call_id: _,
sender_thread_id: _,
@@ -93,6 +103,7 @@ pub(crate) fn spawn_end(ev: CollabAgentSpawnEndEvent) -> PlainHistoryCell {
nickname: new_agent_nickname.as_deref(),
role: new_agent_role.as_deref(),
},
spawn_request,
),
None => title_text("Agent spawn failed"),
};
@@ -122,6 +133,7 @@ pub(crate) fn interaction_end(ev: CollabAgentInteractionEndEvent) -> PlainHistor
nickname: receiver_agent_nickname.as_deref(),
role: receiver_agent_role.as_deref(),
},
None,
);
let mut details = Vec::new();
@@ -141,7 +153,7 @@ pub(crate) fn waiting_begin(ev: CollabWaitingBeginEvent) -> PlainHistoryCell {
let receiver_agents = merge_wait_receivers(&receiver_thread_ids, receiver_agents);
let title = match receiver_agents.as_slice() {
[receiver] => title_with_agent("Waiting for", agent_label_from_ref(receiver)),
[receiver] => title_with_agent("Waiting for", agent_label_from_ref(receiver), None),
[] => title_text("Waiting for agents"),
_ => title_text(format!("Waiting for {} agents", receiver_agents.len())),
};
@@ -187,6 +199,7 @@ pub(crate) fn close_end(ev: CollabCloseEndEvent) -> PlainHistoryCell {
nickname: receiver_agent_nickname.as_deref(),
role: receiver_agent_role.as_deref(),
},
None,
),
Vec::new(),
)
@@ -209,6 +222,7 @@ pub(crate) fn resume_begin(ev: CollabResumeBeginEvent) -> PlainHistoryCell {
nickname: receiver_agent_nickname.as_deref(),
role: receiver_agent_role.as_deref(),
},
None,
),
Vec::new(),
)
@@ -232,6 +246,7 @@ pub(crate) fn resume_end(ev: CollabResumeEndEvent) -> PlainHistoryCell {
nickname: receiver_agent_nickname.as_deref(),
role: receiver_agent_role.as_deref(),
},
None,
),
vec![status_summary_line(&status)],
)
@@ -249,9 +264,14 @@ fn title_text(title: impl Into<String>) -> Line<'static> {
title_spans_line(vec![Span::from(title.into()).bold()])
}
fn title_with_agent(prefix: &str, agent: AgentLabel<'_>) -> Line<'static> {
fn title_with_agent(
prefix: &str,
agent: AgentLabel<'_>,
spawn_request: Option<&SpawnRequestSummary>,
) -> Line<'static> {
let mut spans = vec![Span::from(format!("{prefix} ")).bold()];
spans.extend(agent_label_spans(agent));
spans.extend(spawn_request_spans(spawn_request));
title_spans_line(spans)
}
@@ -298,6 +318,25 @@ fn agent_label_spans(agent: AgentLabel<'_>) -> Vec<Span<'static>> {
spans
}
fn spawn_request_spans(spawn_request: Option<&SpawnRequestSummary>) -> Vec<Span<'static>> {
let Some(spawn_request) = spawn_request else {
return Vec::new();
};
let model = spawn_request.model.trim();
if model.is_empty() && spawn_request.reasoning_effort == ReasoningEffortConfig::default() {
return Vec::new();
}
let details = if model.is_empty() {
format!("({})", spawn_request.reasoning_effort)
} else {
format!("({model} {})", spawn_request.reasoning_effort)
};
vec![Span::from(" ").dim(), Span::from(details).magenta()]
}
fn prompt_line(prompt: &str) -> Option<Line<'static>> {
let trimmed = prompt.trim();
if trimmed.is_empty() {
@@ -460,15 +499,21 @@ mod tests {
let bob_id = ThreadId::from_string("00000000-0000-0000-0000-000000000003")
.expect("valid bob thread id");
let spawn = spawn_end(CollabAgentSpawnEndEvent {
call_id: "call-spawn".to_string(),
sender_thread_id,
new_thread_id: Some(robie_id),
new_agent_nickname: Some("Robie".to_string()),
new_agent_role: Some("explorer".to_string()),
prompt: "Compute 11! and reply with just the integer result.".to_string(),
status: AgentStatus::PendingInit,
});
let spawn = spawn_end(
CollabAgentSpawnEndEvent {
call_id: "call-spawn".to_string(),
sender_thread_id,
new_thread_id: Some(robie_id),
new_agent_nickname: Some("Robie".to_string()),
new_agent_role: Some("explorer".to_string()),
prompt: "Compute 11! and reply with just the integer result.".to_string(),
status: AgentStatus::PendingInit,
},
Some(&SpawnRequestSummary {
model: "gpt-5".to_string(),
reasoning_effort: ReasoningEffortConfig::High,
}),
);
let send = interaction_end(CollabAgentInteractionEndEvent {
call_id: "call-send".to_string(),
@@ -540,15 +585,21 @@ mod tests {
.expect("valid sender thread id");
let robie_id = ThreadId::from_string("00000000-0000-0000-0000-000000000002")
.expect("valid robie thread id");
let cell = spawn_end(CollabAgentSpawnEndEvent {
call_id: "call-spawn".to_string(),
sender_thread_id,
new_thread_id: Some(robie_id),
new_agent_nickname: Some("Robie".to_string()),
new_agent_role: Some("explorer".to_string()),
prompt: String::new(),
status: AgentStatus::PendingInit,
});
let cell = spawn_end(
CollabAgentSpawnEndEvent {
call_id: "call-spawn".to_string(),
sender_thread_id,
new_thread_id: Some(robie_id),
new_agent_nickname: Some("Robie".to_string()),
new_agent_role: Some("explorer".to_string()),
prompt: String::new(),
status: AgentStatus::PendingInit,
},
Some(&SpawnRequestSummary {
model: "gpt-5".to_string(),
reasoning_effort: ReasoningEffortConfig::High,
}),
);
let lines = cell.display_lines(200);
let title = &lines[0];
@@ -558,6 +609,8 @@ mod tests {
assert_eq!(title.spans[4].content.as_ref(), "[explorer]");
assert_eq!(title.spans[4].style.fg, None);
assert!(!title.spans[4].style.add_modifier.contains(Modifier::DIM));
assert_eq!(title.spans[6].content.as_ref(), "(gpt-5 high)");
assert_eq!(title.spans[6].style.fg, Some(Color::Magenta));
}
fn cell_to_text(cell: &PlainHistoryCell) -> String {
@@ -3,4 +3,4 @@ source: tui/src/markdown_render_tests.rs
assertion_line: 714
expression: rendered
---
See markdown_render.rs:74.
See codex-rs/tui/src/markdown_render.rs:74.
@@ -2,7 +2,7 @@
source: tui/src/multi_agents.rs
expression: snapshot
---
• Spawned Robie [explorer]
• Spawned Robie [explorer] (gpt-5 high)
└ Compute 11! and reply with just the integer result.
• Sent input to Robie [explorer]
+24 -6
View File
@@ -4,6 +4,7 @@ use crate::render::line_utils::prefix_lines;
use crate::style::proposed_plan_style;
use ratatui::prelude::Stylize;
use ratatui::text::Line;
use std::path::Path;
use std::time::Duration;
use std::time::Instant;
@@ -18,9 +19,13 @@ pub(crate) struct StreamController {
}
impl StreamController {
pub(crate) fn new(width: Option<usize>) -> Self {
/// Create a controller whose markdown renderer shortens local file links relative to `cwd`.
///
/// The controller snapshots the path into stream state so later commit ticks and finalization
/// render against the same session cwd that was active when streaming started.
pub(crate) fn new(width: Option<usize>, cwd: &Path) -> Self {
Self {
state: StreamState::new(width),
state: StreamState::new(width, cwd),
finishing_after_drain: false,
header_emitted: false,
}
@@ -115,9 +120,14 @@ pub(crate) struct PlanStreamController {
}
impl PlanStreamController {
pub(crate) fn new(width: Option<usize>) -> Self {
/// Create a plan-stream controller whose markdown renderer shortens local file links relative
/// to `cwd`.
///
/// The controller snapshots the path into stream state so later commit ticks and finalization
/// render against the same session cwd that was active when streaming started.
pub(crate) fn new(width: Option<usize>, cwd: &Path) -> Self {
Self {
state: StreamState::new(width),
state: StreamState::new(width, cwd),
header_emitted: false,
top_padding_emitted: false,
}
@@ -232,6 +242,13 @@ impl PlanStreamController {
#[cfg(test)]
mod tests {
use super::*;
use std::path::PathBuf;
fn test_cwd() -> PathBuf {
// These tests only need a stable absolute cwd; using temp_dir() avoids baking Unix- or
// Windows-specific root semantics into the fixtures.
std::env::temp_dir()
}
fn lines_to_plain_strings(lines: &[ratatui::text::Line<'_>]) -> Vec<String> {
lines
@@ -248,7 +265,7 @@ mod tests {
#[tokio::test]
async fn controller_loose_vs_tight_with_commit_ticks_matches_full() {
let mut ctrl = StreamController::new(None);
let mut ctrl = StreamController::new(None, &test_cwd());
let mut lines = Vec::new();
// Exact deltas from the session log (section: Loose vs. tight list items)
@@ -346,7 +363,8 @@ mod tests {
// Full render of the same source
let source: String = deltas.iter().copied().collect();
let mut rendered: Vec<ratatui::text::Line<'static>> = Vec::new();
crate::markdown::append_markdown(&source, None, &mut rendered);
let test_cwd = test_cwd();
crate::markdown::append_markdown(&source, None, Some(test_cwd.as_path()), &mut rendered);
let rendered_strs = lines_to_plain_strings(&rendered);
assert_eq!(streamed, rendered_strs);
+15 -4
View File
@@ -10,6 +10,7 @@
//! arrival timestamp so policy code can reason about oldest queued age without peeking into text.
use std::collections::VecDeque;
use std::path::Path;
use std::time::Duration;
use std::time::Instant;
@@ -33,10 +34,13 @@ pub(crate) struct StreamState {
}
impl StreamState {
/// Creates an empty stream state with an optional target wrap width.
pub(crate) fn new(width: Option<usize>) -> Self {
/// Create stream state whose markdown collector renders local file links relative to `cwd`.
///
/// Controllers are expected to pass the session cwd here once and keep it stable for the
/// lifetime of the active stream.
pub(crate) fn new(width: Option<usize>, cwd: &Path) -> Self {
Self {
collector: MarkdownStreamCollector::new(width),
collector: MarkdownStreamCollector::new(width, cwd),
queued_lines: VecDeque::new(),
has_seen_delta: false,
}
@@ -102,10 +106,17 @@ impl StreamState {
mod tests {
use super::*;
use pretty_assertions::assert_eq;
use std::path::PathBuf;
fn test_cwd() -> PathBuf {
// These tests only need a stable absolute cwd; using temp_dir() avoids baking Unix- or
// Windows-specific root semantics into the fixtures.
std::env::temp_dir()
}
#[test]
fn drain_n_clamps_to_available_lines() {
let mut state = StreamState::new(None);
let mut state = StreamState::new(None, &test_cwd());
state.enqueue(vec![Line::from("one")]);
let drained = state.drain_n(8);