Merge 0a4d9e60d5 into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2026-02-06 22:15:15 -08:00
committed by GitHub
65 changed files with 2315 additions and 504 deletions
+2
View File
@@ -1239,6 +1239,7 @@ dependencies = [
"core_test_support",
"futures",
"os_info",
"owo-colors",
"pretty_assertions",
"rmcp",
"serde",
@@ -2006,6 +2007,7 @@ dependencies = [
"codex-protocol",
"codex-utils-cargo-bin",
"codex-utils-home-dir",
"codex-utils-pty",
"futures",
"keyring",
"oauth2",
@@ -1064,11 +1064,23 @@
"type": "string"
},
"MessagePhase": {
"enum": [
"commentary",
"final_answer"
],
"type": "string"
"description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.",
"oneOf": [
{
"description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.",
"enum": [
"commentary"
],
"type": "string"
},
{
"description": "The assistant's terminal answer text for the current turn.",
"enum": [
"final_answer"
],
"type": "string"
}
]
},
"ModeKind": {
"description": "Initial collaboration mode to use when the TUI starts.",
@@ -3169,11 +3169,23 @@
]
},
"MessagePhase": {
"enum": [
"commentary",
"final_answer"
],
"type": "string"
"description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.",
"oneOf": [
{
"description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.",
"enum": [
"commentary"
],
"type": "string"
},
{
"description": "The assistant's terminal answer text for the current turn.",
"enum": [
"final_answer"
],
"type": "string"
}
]
},
"ModeKind": {
"description": "Initial collaboration mode to use when the TUI starts.",
@@ -4685,6 +4697,7 @@
"type": "object"
},
{
"description": "Assistant-authored message payload used in turn-item streams.\n\n`phase` is optional because not all providers/models emit it. Consumers should use it when present, but retain legacy completion semantics when it is `None`.",
"properties": {
"content": {
"items": {
@@ -4695,6 +4708,17 @@
"id": {
"type": "string"
},
"phase": {
"anyOf": [
{
"$ref": "#/definitions/MessagePhase"
},
{
"type": "null"
}
],
"description": "Optional phase metadata carried through from `ResponseItem::Message`.\n\nThis is currently used by TUI rendering to distinguish mid-turn commentary from a final answer and avoid status-indicator jitter."
},
"type": {
"enum": [
"AgentMessage"
@@ -3948,11 +3948,23 @@
"type": "string"
},
"MessagePhase": {
"enum": [
"commentary",
"final_answer"
],
"type": "string"
"description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.",
"oneOf": [
{
"description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.",
"enum": [
"commentary"
],
"type": "string"
},
{
"description": "The assistant's terminal answer text for the current turn.",
"enum": [
"final_answer"
],
"type": "string"
}
]
},
"ModeKind": {
"description": "Initial collaboration mode to use when the TUI starts.",
@@ -6686,6 +6698,7 @@
"type": "object"
},
{
"description": "Assistant-authored message payload used in turn-item streams.\n\n`phase` is optional because not all providers/models emit it. Consumers should use it when present, but retain legacy completion semantics when it is `None`.",
"properties": {
"content": {
"items": {
@@ -6696,6 +6709,17 @@
"id": {
"type": "string"
},
"phase": {
"anyOf": [
{
"$ref": "#/definitions/MessagePhase"
},
{
"type": "null"
}
],
"description": "Optional phase metadata carried through from `ResponseItem::Message`.\n\nThis is currently used by TUI rendering to distinguish mid-turn commentary from a final answer and avoid status-indicator jitter."
},
"type": {
"enum": [
"AgentMessage"
@@ -6064,11 +6064,23 @@
]
},
"MessagePhase": {
"enum": [
"commentary",
"final_answer"
],
"type": "string"
"description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.",
"oneOf": [
{
"description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.",
"enum": [
"commentary"
],
"type": "string"
},
{
"description": "The assistant's terminal answer text for the current turn.",
"enum": [
"final_answer"
],
"type": "string"
}
]
},
"ModeKind": {
"description": "Initial collaboration mode to use when the TUI starts.",
@@ -9146,6 +9158,7 @@
"type": "object"
},
{
"description": "Assistant-authored message payload used in turn-item streams.\n\n`phase` is optional because not all providers/models emit it. Consumers should use it when present, but retain legacy completion semantics when it is `None`.",
"properties": {
"content": {
"items": {
@@ -9156,6 +9169,17 @@
"id": {
"type": "string"
},
"phase": {
"anyOf": [
{
"$ref": "#/definitions/MessagePhase"
},
{
"type": "null"
}
],
"description": "Optional phase metadata carried through from `ResponseItem::Message`.\n\nThis is currently used by TUI rendering to distinguish mid-turn commentary from a final answer and avoid status-indicator jitter."
},
"type": {
"enum": [
"AgentMessage"
@@ -10518,17 +10542,6 @@
}
]
},
"apps": {
"anyOf": [
{
"$ref": "#/definitions/v2/AppsConfig"
},
{
"type": "null"
}
],
"default": null
},
"compact_prompt": {
"type": [
"string",
@@ -12198,11 +12211,23 @@
"type": "string"
},
"MessagePhase": {
"enum": [
"commentary",
"final_answer"
],
"type": "string"
"description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.",
"oneOf": [
{
"description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.",
"enum": [
"commentary"
],
"type": "string"
},
{
"description": "The assistant's terminal answer text for the current turn.",
"enum": [
"final_answer"
],
"type": "string"
}
]
},
"ModeKind": {
"description": "Initial collaboration mode to use when the TUI starts.",
@@ -3169,11 +3169,23 @@
]
},
"MessagePhase": {
"enum": [
"commentary",
"final_answer"
],
"type": "string"
"description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.",
"oneOf": [
{
"description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.",
"enum": [
"commentary"
],
"type": "string"
},
{
"description": "The assistant's terminal answer text for the current turn.",
"enum": [
"final_answer"
],
"type": "string"
}
]
},
"ModeKind": {
"description": "Initial collaboration mode to use when the TUI starts.",
@@ -4685,6 +4697,7 @@
"type": "object"
},
{
"description": "Assistant-authored message payload used in turn-item streams.\n\n`phase` is optional because not all providers/models emit it. Consumers should use it when present, but retain legacy completion semantics when it is `None`.",
"properties": {
"content": {
"items": {
@@ -4695,6 +4708,17 @@
"id": {
"type": "string"
},
"phase": {
"anyOf": [
{
"$ref": "#/definitions/MessagePhase"
},
{
"type": "null"
}
],
"description": "Optional phase metadata carried through from `ResponseItem::Message`.\n\nThis is currently used by TUI rendering to distinguish mid-turn commentary from a final answer and avoid status-indicator jitter."
},
"type": {
"enum": [
"AgentMessage"
@@ -271,11 +271,23 @@
"type": "string"
},
"MessagePhase": {
"enum": [
"commentary",
"final_answer"
],
"type": "string"
"description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.",
"oneOf": [
{
"description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.",
"enum": [
"commentary"
],
"type": "string"
},
{
"description": "The assistant's terminal answer text for the current turn.",
"enum": [
"final_answer"
],
"type": "string"
}
]
},
"NewConversationParams": {
"properties": {
@@ -3169,11 +3169,23 @@
]
},
"MessagePhase": {
"enum": [
"commentary",
"final_answer"
],
"type": "string"
"description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.",
"oneOf": [
{
"description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.",
"enum": [
"commentary"
],
"type": "string"
},
{
"description": "The assistant's terminal answer text for the current turn.",
"enum": [
"final_answer"
],
"type": "string"
}
]
},
"ModeKind": {
"description": "Initial collaboration mode to use when the TUI starts.",
@@ -4685,6 +4697,7 @@
"type": "object"
},
{
"description": "Assistant-authored message payload used in turn-item streams.\n\n`phase` is optional because not all providers/models emit it. Consumers should use it when present, but retain legacy completion semantics when it is `None`.",
"properties": {
"content": {
"items": {
@@ -4695,6 +4708,17 @@
"id": {
"type": "string"
},
"phase": {
"anyOf": [
{
"$ref": "#/definitions/MessagePhase"
},
{
"type": "null"
}
],
"description": "Optional phase metadata carried through from `ResponseItem::Message`.\n\nThis is currently used by TUI rendering to distinguish mid-turn commentary from a final answer and avoid status-indicator jitter."
},
"type": {
"enum": [
"AgentMessage"
@@ -3169,11 +3169,23 @@
]
},
"MessagePhase": {
"enum": [
"commentary",
"final_answer"
],
"type": "string"
"description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.",
"oneOf": [
{
"description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.",
"enum": [
"commentary"
],
"type": "string"
},
{
"description": "The assistant's terminal answer text for the current turn.",
"enum": [
"final_answer"
],
"type": "string"
}
]
},
"ModeKind": {
"description": "Initial collaboration mode to use when the TUI starts.",
@@ -4685,6 +4697,7 @@
"type": "object"
},
{
"description": "Assistant-authored message payload used in turn-item streams.\n\n`phase` is optional because not all providers/models emit it. Consumers should use it when present, but retain legacy completion semantics when it is `None`.",
"properties": {
"content": {
"items": {
@@ -4695,6 +4708,17 @@
"id": {
"type": "string"
},
"phase": {
"anyOf": [
{
"$ref": "#/definitions/MessagePhase"
},
{
"type": "null"
}
],
"description": "Optional phase metadata carried through from `ResponseItem::Message`.\n\nThis is currently used by TUI rendering to distinguish mid-turn commentary from a final answer and avoid status-indicator jitter."
},
"type": {
"enum": [
"AgentMessage"
@@ -78,17 +78,6 @@
}
]
},
"apps": {
"anyOf": [
{
"$ref": "#/definitions/AppsConfig"
},
{
"type": "null"
}
],
"default": null
},
"compact_prompt": {
"type": [
"string",
@@ -238,11 +238,23 @@
"type": "string"
},
"MessagePhase": {
"enum": [
"commentary",
"final_answer"
],
"type": "string"
"description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.",
"oneOf": [
{
"description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.",
"enum": [
"commentary"
],
"type": "string"
},
{
"description": "The assistant's terminal answer text for the current turn.",
"enum": [
"final_answer"
],
"type": "string"
}
]
},
"ReasoningItemContent": {
"oneOf": [
@@ -247,11 +247,23 @@
"type": "string"
},
"MessagePhase": {
"enum": [
"commentary",
"final_answer"
],
"type": "string"
"description": "Classifies an assistant message as interim commentary or final answer text.\n\nProviders do not emit this consistently, so callers must treat `None` as \"phase unknown\" and keep compatibility behavior for legacy models.",
"oneOf": [
{
"description": "Mid-turn assistant text (for example preamble/progress narration).\n\nAdditional tool calls or assistant output may follow before turn completion.",
"enum": [
"commentary"
],
"type": "string"
},
{
"description": "The assistant's terminal answer text for the current turn.",
"enum": [
"final_answer"
],
"type": "string"
}
]
},
"Personality": {
"enum": [
@@ -2,5 +2,20 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
import type { AgentMessageContent } from "./AgentMessageContent";
import type { MessagePhase } from "./MessagePhase";
export type AgentMessageItem = { id: string, content: Array<AgentMessageContent>, };
/**
* Assistant-authored message payload used in turn-item streams.
*
* `phase` is optional because not all providers/models emit it. Consumers
* should use it when present, but retain legacy completion semantics when it
* is `None`.
*/
export type AgentMessageItem = { id: string, content: Array<AgentMessageContent>,
/**
* Optional phase metadata carried through from `ResponseItem::Message`.
*
* This is currently used by TUI rendering to distinguish mid-turn
* commentary from a final answer and avoid status-indicator jitter.
*/
phase?: MessagePhase, };
@@ -2,4 +2,10 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
/**
* Classifies an assistant message as interim commentary or final answer text.
*
* Providers do not emit this consistently, so callers must treat `None` as
* "phase unknown" and keep compatibility behavior for legacy models.
*/
export type MessagePhase = "commentary" | "final_answer";
@@ -8,11 +8,10 @@ import type { Verbosity } from "../Verbosity";
import type { WebSearchMode } from "../WebSearchMode";
import type { JsonValue } from "../serde_json/JsonValue";
import type { AnalyticsConfig } from "./AnalyticsConfig";
import type { AppsConfig } from "./AppsConfig";
import type { AskForApproval } from "./AskForApproval";
import type { ProfileV2 } from "./ProfileV2";
import type { SandboxMode } from "./SandboxMode";
import type { SandboxWorkspaceWrite } from "./SandboxWorkspaceWrite";
import type { ToolsV2 } from "./ToolsV2";
export type Config = { model: string | null, review_model: string | null, model_context_window: bigint | null, model_auto_compact_token_limit: bigint | null, model_provider: string | null, approval_policy: AskForApproval | null, sandbox_mode: SandboxMode | null, sandbox_workspace_write: SandboxWorkspaceWrite | null, forced_chatgpt_workspace_id: string | null, forced_login_method: ForcedLoginMethod | null, web_search: WebSearchMode | null, tools: ToolsV2 | null, profile: string | null, profiles: { [key in string]?: ProfileV2 }, instructions: string | null, developer_instructions: string | null, compact_prompt: string | null, model_reasoning_effort: ReasoningEffort | null, model_reasoning_summary: ReasoningSummary | null, model_verbosity: Verbosity | null, analytics: AnalyticsConfig | null, apps: AppsConfig | null, } & ({ [key in string]?: number | string | boolean | Array<JsonValue> | { [key in string]?: JsonValue } | null });
export type Config = {model: string | null, review_model: string | null, model_context_window: bigint | null, model_auto_compact_token_limit: bigint | null, model_provider: string | null, approval_policy: AskForApproval | null, sandbox_mode: SandboxMode | null, sandbox_workspace_write: SandboxWorkspaceWrite | null, forced_chatgpt_workspace_id: string | null, forced_login_method: ForcedLoginMethod | null, web_search: WebSearchMode | null, tools: ToolsV2 | null, profile: string | null, profiles: { [key in string]?: ProfileV2 }, instructions: string | null, developer_instructions: string | null, compact_prompt: string | null, model_reasoning_effort: ReasoningEffort | null, model_reasoning_summary: ReasoningSummary | null, model_verbosity: Verbosity | null, analytics: AnalyticsConfig | null} & ({ [key in string]?: number | string | boolean | Array<JsonValue> | { [key in string]?: JsonValue } | null });
+52 -2
View File
@@ -238,7 +238,10 @@ fn filter_client_request_ts(out_dir: &Path, experimental_methods: &[&str]) -> Re
.collect();
let new_body = filtered_arms.join(" | ");
content = format!("{prefix}{new_body}{suffix}");
content = prune_unused_type_imports(content, &new_body);
let import_usage_scope = split_type_alias(&content)
.map(|(_, body, _)| body)
.unwrap_or_else(|| new_body.clone());
content = prune_unused_type_imports(content, &import_usage_scope);
fs::write(&path, content).with_context(|| format!("Failed to write {}", path.display()))?;
Ok(())
@@ -296,7 +299,10 @@ fn filter_experimental_fields_in_ts_file(
let prefix = &content[..open_brace + 1];
let suffix = &content[close_brace..];
content = format!("{prefix}{new_inner}{suffix}");
content = prune_unused_type_imports(content, &new_inner);
let import_usage_scope = split_type_alias(&content)
.map(|(_, body, _)| body)
.unwrap_or_else(|| new_inner.clone());
content = prune_unused_type_imports(content, &import_usage_scope);
fs::write(path, content).with_context(|| format!("Failed to write {}", path.display()))?;
Ok(())
}
@@ -1745,6 +1751,50 @@ mod tests {
Ok(())
}
#[test]
fn experimental_type_fields_ts_filter_keeps_imports_used_in_intersection_suffix() -> Result<()>
{
let output_dir = std::env::temp_dir().join(format!("codex_ts_filter_{}", Uuid::now_v7()));
fs::create_dir_all(&output_dir)?;
struct TempDirGuard(PathBuf);
impl Drop for TempDirGuard {
fn drop(&mut self) {
let _ = fs::remove_dir_all(&self.0);
}
}
let _guard = TempDirGuard(output_dir.clone());
let path = output_dir.join("Config.ts");
let content = r#"import type { JsonValue } from "../serde_json/JsonValue";
import type { Keep } from "./Keep";
export type Config = { stableField: Keep, unstableField: string | null } & ({ [key in string]?: number | string | boolean | Array<JsonValue> | { [key in string]?: JsonValue } | null });
"#;
fs::write(&path, content)?;
static CUSTOM_FIELD: crate::experimental_api::ExperimentalField =
crate::experimental_api::ExperimentalField {
type_name: "Config",
field_name: "unstableField",
reason: "custom/unstableField",
};
filter_experimental_type_fields_ts(&output_dir, &[&CUSTOM_FIELD])?;
let filtered = fs::read_to_string(&path)?;
assert_eq!(filtered.contains("unstableField"), false);
assert_eq!(
filtered.contains(r#"import type { JsonValue } from "../serde_json/JsonValue";"#),
true
);
assert_eq!(
filtered.contains(r#"import type { Keep } from "./Keep";"#),
true
);
Ok(())
}
#[test]
fn stable_schema_filter_removes_mock_experimental_method() -> Result<()> {
let output_dir = std::env::temp_dir().join(format!("codex_schema_{}", Uuid::now_v7()));
@@ -404,7 +404,7 @@ const fn default_enabled() -> bool {
true
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, ExperimentalApi)]
#[serde(rename_all = "snake_case")]
#[ts(export_to = "v2/")]
pub struct Config {
@@ -430,6 +430,7 @@ pub struct Config {
pub model_reasoning_summary: Option<ReasoningSummary>,
pub model_verbosity: Option<Verbosity>,
pub analytics: Option<AnalyticsConfig>,
#[experimental("config/read.apps")]
#[serde(default)]
pub apps: Option<AppsConfig>,
#[serde(default, flatten)]
@@ -3249,6 +3250,7 @@ mod tests {
text: "world".to_string(),
},
],
phase: None,
});
assert_eq!(
+1
View File
@@ -35,6 +35,7 @@ codex-utils-json-to-toml = { workspace = true }
chrono = { workspace = true }
clap = { workspace = true, features = ["derive"] }
futures = { workspace = true }
owo-colors = { workspace = true, features = ["supports-colors"] }
serde = { workspace = true, features = ["derive"] }
serde_json = { workspace = true }
tempfile = { workspace = true }
+36 -1
View File
@@ -5,6 +5,9 @@ use crate::outgoing_message::OutgoingMessage;
use codex_app_server_protocol::JSONRPCMessage;
use futures::SinkExt;
use futures::StreamExt;
use owo_colors::OwoColorize;
use owo_colors::Stream;
use owo_colors::Style;
use std::collections::HashMap;
use std::io::ErrorKind;
use std::io::Result as IoResult;
@@ -33,6 +36,36 @@ use tracing::warn;
/// plenty for an interactive CLI.
pub(crate) const CHANNEL_CAPACITY: usize = 128;
fn colorize(text: &str, style: Style) -> String {
text.if_supports_color(Stream::Stderr, |value| value.style(style))
.to_string()
}
#[allow(clippy::print_stderr)]
fn print_websocket_startup_banner(addr: SocketAddr) {
let title = colorize("codex app-server (WebSockets)", Style::new().bold().cyan());
let listening_label = colorize("listening on:", Style::new().dimmed());
let listen_url = colorize(&format!("ws://{addr}"), Style::new().green());
let note_label = colorize("note:", Style::new().dimmed());
eprintln!("{title}");
eprintln!(" {listening_label} {listen_url}");
if addr.ip().is_loopback() {
eprintln!(
" {note_label} binds localhost only (use SSH port-forwarding for remote access)"
);
} else {
eprintln!(
" {note_label} this is a raw WS server; consider running behind TLS/auth for real remote use"
);
}
}
#[allow(clippy::print_stderr)]
fn print_websocket_connection(peer_addr: SocketAddr) {
let connected_label = colorize("websocket client connected from", Style::new().dimmed());
eprintln!("{connected_label} {peer_addr}");
}
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum AppServerTransport {
Stdio,
@@ -191,13 +224,15 @@ pub(crate) async fn start_websocket_acceptor(
) -> IoResult<JoinHandle<()>> {
let listener = TcpListener::bind(bind_address).await?;
let local_addr = listener.local_addr()?;
print_websocket_startup_banner(local_addr);
info!("app-server websocket listening on ws://{local_addr}");
let connection_counter = Arc::new(AtomicU64::new(1));
Ok(tokio::spawn(async move {
loop {
match listener.accept().await {
Ok((stream, _peer_addr)) => {
Ok((stream, peer_addr)) => {
print_websocket_connection(peer_addr);
let connection_id =
ConnectionId(connection_counter.fetch_add(1, Ordering::Relaxed));
let transport_event_tx_for_connection = transport_event_tx.clone();
@@ -407,6 +407,9 @@ sandbox_mode = "read-only"
model_provider = "mock_provider"
[features]
remote_models = false
[model_providers.mock_provider]
name = "Mock provider"
base_url = "{server_uri}/v1"
+2
View File
@@ -172,6 +172,8 @@ pub struct ResponsesApiRequest<'a> {
pub struct ResponseCreateWsRequest {
pub model: String,
pub instructions: String,
#[serde(skip_serializing_if = "Option::is_none")]
pub previous_response_id: Option<String>,
pub input: Vec<ResponseItem>,
pub tools: Vec<Value>,
pub tool_choice: String,
+6
View File
@@ -239,6 +239,9 @@
"responses_websockets": {
"type": "boolean"
},
"responses_websockets_v2": {
"type": "boolean"
},
"runtime_metrics": {
"type": "boolean"
},
@@ -1279,6 +1282,9 @@
"responses_websockets": {
"type": "boolean"
},
"responses_websockets_v2": {
"type": "boolean"
},
"runtime_metrics": {
"type": "boolean"
},
+112 -14
View File
@@ -89,6 +89,8 @@ use reqwest::StatusCode;
use serde_json::Value;
use std::time::Duration;
use tokio::sync::mpsc;
use tokio::sync::oneshot;
use tokio::sync::oneshot::error::TryRecvError;
use tokio::task::JoinHandle;
use tokio_tungstenite::tungstenite::Error;
use tokio_tungstenite::tungstenite::Message;
@@ -117,6 +119,7 @@ pub const X_CODEX_TURN_STATE_HEADER: &str = "x-codex-turn-state";
pub const X_CODEX_TURN_METADATA_HEADER: &str = "x-codex-turn-metadata";
pub const X_RESPONSESAPI_INCLUDE_TIMING_METRICS_HEADER: &str =
"x-responsesapi-include-timing-metrics";
const RESPONSES_WEBSOCKETS_V2_BETA_HEADER_VALUE: &str = "responses_websockets=2026-02-06";
/// Session-scoped state shared by all [`ModelClient`] clones.
///
@@ -129,6 +132,7 @@ struct ModelClientState {
session_source: SessionSource,
model_verbosity: Option<VerbosityConfig>,
enable_responses_websockets: bool,
enable_responses_websockets_v2: bool,
enable_request_compression: bool,
include_timing_metrics: bool,
beta_features_header: Option<String>,
@@ -238,6 +242,8 @@ pub struct ModelClientSession {
client: ModelClient,
connection: Option<ApiWebSocketConnection>,
websocket_last_items: Vec<ResponseItem>,
websocket_last_response_id: Option<String>,
websocket_last_response_id_rx: Option<oneshot::Receiver<String>>,
/// Turn state for sticky routing.
///
/// This is an `OnceLock` that stores the turn state value received from the server
@@ -264,6 +270,7 @@ impl ModelClient {
session_source: SessionSource,
model_verbosity: Option<VerbosityConfig>,
enable_responses_websockets: bool,
enable_responses_websockets_v2: bool,
enable_request_compression: bool,
include_timing_metrics: bool,
beta_features_header: Option<String>,
@@ -276,6 +283,7 @@ impl ModelClient {
session_source,
model_verbosity,
enable_responses_websockets,
enable_responses_websockets_v2,
enable_request_compression,
include_timing_metrics,
beta_features_header,
@@ -295,6 +303,8 @@ impl ModelClient {
client: self.clone(),
connection: None,
websocket_last_items: Vec::new(),
websocket_last_response_id: None,
websocket_last_response_id_rx: None,
turn_state: Arc::new(OnceLock::new()),
}
}
@@ -479,6 +489,10 @@ impl ModelClient {
self.state.provider.supports_websockets && self.state.enable_responses_websockets
}
fn responses_websockets_v2_enabled(&self) -> bool {
self.state.enable_responses_websockets_v2
}
/// Returns whether websocket transport has been permanently disabled for this session.
///
/// Once set by fallback activation, subsequent turns must stay on HTTP transport.
@@ -544,9 +558,14 @@ impl ModelClient {
headers.extend(build_conversation_headers(Some(
self.state.conversation_id.to_string(),
)));
let responses_websockets_beta_header = if self.responses_websockets_v2_enabled() {
RESPONSES_WEBSOCKETS_V2_BETA_HEADER_VALUE
} else {
OPENAI_BETA_RESPONSES_WEBSOCKETS
};
headers.insert(
OPENAI_BETA_HEADER,
HeaderValue::from_static(OPENAI_BETA_RESPONSES_WEBSOCKETS),
HeaderValue::from_static(responses_websockets_beta_header),
);
if self.state.include_timing_metrics {
headers.insert(
@@ -789,18 +808,37 @@ impl ModelClientSession {
}
}
fn prepare_websocket_request(
fn refresh_websocket_last_response_id(&mut self) {
if let Some(mut receiver) = self.websocket_last_response_id_rx.take() {
match receiver.try_recv() {
Ok(response_id) if !response_id.is_empty() => {
self.websocket_last_response_id = Some(response_id);
}
Ok(_) | Err(TryRecvError::Closed) => {
self.websocket_last_response_id = None;
}
Err(TryRecvError::Empty) => {
self.websocket_last_response_id_rx = Some(receiver);
}
}
}
}
fn websocket_previous_response_id(&mut self) -> Option<String> {
self.refresh_websocket_last_response_id();
self.websocket_last_response_id
.clone()
.filter(|id| !id.is_empty())
}
fn prepare_websocket_create_request(
&self,
model_slug: &str,
api_prompt: &ApiPrompt,
options: &ApiResponsesOptions,
input: Vec<ResponseItem>,
previous_response_id: Option<String>,
) -> ResponsesWsRequest {
if let Some(append_items) = self.get_incremental_items(&api_prompt.input) {
return ResponsesWsRequest::ResponseAppend(ResponseAppendWsRequest {
input: append_items,
});
}
let ApiResponsesOptions {
reasoning,
include,
@@ -814,7 +852,8 @@ impl ModelClientSession {
let payload = ResponseCreateWsRequest {
model: model_slug.to_string(),
instructions: api_prompt.instructions.clone(),
input: api_prompt.input.clone(),
previous_response_id,
input,
tools: api_prompt.tools.clone(),
tool_choice: "auto".to_string(),
parallel_tool_calls: api_prompt.parallel_tool_calls,
@@ -829,6 +868,43 @@ impl ModelClientSession {
ResponsesWsRequest::ResponseCreate(payload)
}
fn prepare_websocket_request(
&mut self,
model_slug: &str,
api_prompt: &ApiPrompt,
options: &ApiResponsesOptions,
) -> ResponsesWsRequest {
let responses_websockets_v2_enabled = self.client.responses_websockets_v2_enabled();
let incremental_items = self.get_incremental_items(&api_prompt.input);
if let Some(append_items) = incremental_items {
if responses_websockets_v2_enabled
&& let Some(previous_response_id) = self.websocket_previous_response_id()
{
return self.prepare_websocket_create_request(
model_slug,
api_prompt,
options,
append_items,
Some(previous_response_id),
);
}
if !responses_websockets_v2_enabled {
return ResponsesWsRequest::ResponseAppend(ResponseAppendWsRequest {
input: append_items,
});
}
}
self.prepare_websocket_create_request(
model_slug,
api_prompt,
options,
api_prompt.input.clone(),
None,
)
}
/// Returns a websocket connection for this turn, reusing preconnect when possible.
///
/// This method first tries to adopt the session-level preconnect slot, then falls back to a
@@ -863,6 +939,9 @@ impl ModelClientSession {
if needs_new {
self.client.clear_preconnect();
self.websocket_last_items.clear();
self.websocket_last_response_id = None;
self.websocket_last_response_id_rx = None;
let turn_state = options
.turn_state
.clone()
@@ -1023,9 +1102,8 @@ impl ModelClientSession {
turn_metadata_header,
compression,
);
let request = self.prepare_websocket_request(&model_info.slug, &api_prompt, &options);
let connection = match self
match self
.websocket_connection(
otel_manager,
client_setup.api_provider,
@@ -1035,7 +1113,7 @@ impl ModelClientSession {
)
.await
{
Ok(connection) => connection,
Ok(_) => {}
Err(ApiError::Transport(
unauthorized_transport @ TransportError::Http { status, .. },
)) if status == StatusCode::UNAUTHORIZED => {
@@ -1043,13 +1121,33 @@ impl ModelClientSession {
continue;
}
Err(err) => return Err(map_api_error(err)),
};
}
let stream_result = connection
let request = self.prepare_websocket_request(&model_info.slug, &api_prompt, &options);
let stream_result = self
.connection
.as_ref()
.ok_or_else(|| {
map_api_error(ApiError::Stream(
"websocket connection is unavailable".to_string(),
))
})?
.stream_request(request)
.await
.map_err(map_api_error)?;
self.websocket_last_items = api_prompt.input.clone();
let (last_response_id_sender, last_response_id_receiver) = oneshot::channel();
self.websocket_last_response_id_rx = Some(last_response_id_receiver);
let mut last_response_id_sender = Some(last_response_id_sender);
let stream_result = stream_result.inspect(move |event| {
if let Ok(ResponseEvent::Completed { response_id, .. }) = event
&& !response_id.is_empty()
&& let Some(sender) = last_response_id_sender.take()
{
let _ = sender.send(response_id.clone());
}
});
return Ok(map_response_stream(stream_result, otel_manager.clone()));
}
+12 -3
View File
@@ -993,6 +993,7 @@ impl Session {
auth.and_then(CodexAuth::get_account_id),
auth.and_then(CodexAuth::get_account_email),
auth_mode,
crate::default_client::originator().value,
config.otel.log_user_prompt,
terminal::user_agent(),
session_configuration.session_source.clone(),
@@ -1072,7 +1073,9 @@ impl Session {
session_configuration.provider.clone(),
session_configuration.session_source.clone(),
config.model_verbosity,
config.features.enabled(Feature::ResponsesWebsockets),
config.features.enabled(Feature::ResponsesWebsockets)
|| config.features.enabled(Feature::ResponsesWebsocketsV2),
config.features.enabled(Feature::ResponsesWebsocketsV2),
config.features.enabled(Feature::EnableRequestCompression),
config.features.enabled(Feature::RuntimeMetrics),
Self::build_model_client_beta_features_header(config.as_ref()),
@@ -4500,6 +4503,7 @@ async fn emit_agent_message_in_plan_mode(
TurnItem::AgentMessage(codex_protocol::items::AgentMessageItem {
id: agent_message_id.clone(),
content: Vec::new(),
phase: None,
})
});
sess.emit_turn_item_started(turn_context, &start_item).await;
@@ -5793,6 +5797,7 @@ mod tests {
None,
Some("test@test.com".to_string()),
Some(TelemetryAuthMode::Chatgpt),
"test_originator".to_string(),
false,
"test".to_string(),
session_source,
@@ -5891,7 +5896,9 @@ mod tests {
session_configuration.provider.clone(),
session_configuration.session_source.clone(),
config.model_verbosity,
config.features.enabled(Feature::ResponsesWebsockets),
config.features.enabled(Feature::ResponsesWebsockets)
|| config.features.enabled(Feature::ResponsesWebsocketsV2),
config.features.enabled(Feature::ResponsesWebsocketsV2),
config.features.enabled(Feature::EnableRequestCompression),
config.features.enabled(Feature::RuntimeMetrics),
Session::build_model_client_beta_features_header(config.as_ref()),
@@ -6021,7 +6028,9 @@ mod tests {
session_configuration.provider.clone(),
session_configuration.session_source.clone(),
config.model_verbosity,
config.features.enabled(Feature::ResponsesWebsockets),
config.features.enabled(Feature::ResponsesWebsockets)
|| config.features.enabled(Feature::ResponsesWebsocketsV2),
config.features.enabled(Feature::ResponsesWebsocketsV2),
config.features.enabled(Feature::EnableRequestCompression),
config.features.enabled(Feature::RuntimeMetrics),
Session::build_model_client_beta_features_header(config.as_ref()),
+19 -17
View File
@@ -2739,25 +2739,27 @@ profile = "project"
}
#[test]
fn responses_websockets_feature_does_not_change_wire_api() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
let mut entries = BTreeMap::new();
entries.insert("responses_websockets".to_string(), true);
let cfg = ConfigToml {
features: Some(crate::features::FeaturesToml { entries }),
..Default::default()
};
fn responses_websocket_features_do_not_change_wire_api() -> std::io::Result<()> {
for feature_key in ["responses_websockets", "responses_websockets_v2"] {
let codex_home = TempDir::new()?;
let mut entries = BTreeMap::new();
entries.insert(feature_key.to_string(), true);
let cfg = ConfigToml {
features: Some(crate::features::FeaturesToml { entries }),
..Default::default()
};
let config = Config::load_from_base_config_with_overrides(
cfg,
ConfigOverrides::default(),
codex_home.path().to_path_buf(),
)?;
let config = Config::load_from_base_config_with_overrides(
cfg,
ConfigOverrides::default(),
codex_home.path().to_path_buf(),
)?;
assert_eq!(
config.model_provider.wire_api,
crate::model_provider_info::WireApi::Responses
);
assert_eq!(
config.model_provider.wire_api,
crate::model_provider_info::WireApi::Responses
);
}
Ok(())
}
+13 -3
View File
@@ -5,6 +5,7 @@ use codex_protocol::items::TurnItem;
use codex_protocol::items::UserMessageItem;
use codex_protocol::items::WebSearchItem;
use codex_protocol::models::ContentItem;
use codex_protocol::models::MessagePhase;
use codex_protocol::models::ReasoningItemContent;
use codex_protocol::models::ReasoningItemReasoningSummary;
use codex_protocol::models::ResponseItem;
@@ -69,7 +70,11 @@ fn parse_user_message(message: &[ContentItem]) -> Option<UserMessageItem> {
Some(UserMessageItem::new(&content))
}
fn parse_agent_message(id: Option<&String>, message: &[ContentItem]) -> AgentMessageItem {
fn parse_agent_message(
id: Option<&String>,
message: &[ContentItem],
phase: Option<MessagePhase>,
) -> AgentMessageItem {
let mut content: Vec<AgentMessageContent> = Vec::new();
for content_item in message.iter() {
match content_item {
@@ -85,18 +90,23 @@ fn parse_agent_message(id: Option<&String>, message: &[ContentItem]) -> AgentMes
}
}
let id = id.cloned().unwrap_or_else(|| Uuid::new_v4().to_string());
AgentMessageItem { id, content }
AgentMessageItem { id, content, phase }
}
pub fn parse_turn_item(item: &ResponseItem) -> Option<TurnItem> {
match item {
ResponseItem::Message {
role, content, id, ..
role,
content,
id,
phase,
..
} => match role.as_str() {
"user" => parse_user_message(content).map(TurnItem::UserMessage),
"assistant" => Some(TurnItem::AgentMessage(parse_agent_message(
id.as_ref(),
content,
phase.clone(),
))),
"system" => None,
_ => None,
+8
View File
@@ -127,6 +127,8 @@ pub enum Feature {
Personality,
/// Use the Responses API WebSocket transport for OpenAI by default.
ResponsesWebsockets,
/// Enable Responses API websocket v2 mode.
ResponsesWebsocketsV2,
}
impl Feature {
@@ -569,6 +571,12 @@ pub const FEATURES: &[FeatureSpec] = &[
stage: Stage::UnderDevelopment,
default_enabled: false,
},
FeatureSpec {
id: Feature::ResponsesWebsocketsV2,
key: "responses_websockets_v2",
stage: Stage::UnderDevelopment,
default_enabled: false,
},
];
/// Push a warning event if any under-development features are enabled.
+80 -9
View File
@@ -78,14 +78,17 @@ fn emit_skill_injected_metric(otel: Option<&OtelManager>, skill: &SkillMetadata,
);
}
/// Collect explicitly mentioned skills from `$name` text mentions.
/// Collect explicitly mentioned skills from structured and text mentions.
///
/// Text inputs are scanned once to extract `$skill-name` tokens, then we iterate `skills`
/// in their existing order to preserve prior ordering semantics. Explicit links are
/// resolved by path and plain names are only used when the match is unambiguous.
/// Structured `UserInput::Skill` selections are resolved first by path against
/// enabled skills. Text inputs are then scanned to extract `$skill-name` tokens, and we
/// iterate `skills` in their existing order to preserve prior ordering semantics.
/// Explicit links are resolved by path and plain names are only used when the match
/// is unambiguous.
///
/// Complexity: `O(S + T + N_t * S)` time, `O(S)` space, where:
/// `S` = number of skills, `T` = total text length, `N_t` = number of text inputs.
/// Complexity: `O(T + (N_s + N_t) * S)` time, `O(S + M)` space, where:
/// `S` = number of skills, `T` = total text length, `N_s` = number of structured skill inputs,
/// `N_t` = number of text inputs, `M` = max mentions parsed from a single text input.
pub(crate) fn collect_explicit_skill_mentions(
inputs: &[UserInput],
skills: &[SkillMetadata],
@@ -102,12 +105,33 @@ pub(crate) fn collect_explicit_skill_mentions(
let mut selected: Vec<SkillMetadata> = Vec::new();
let mut seen_names: HashSet<String> = HashSet::new();
let mut seen_paths: HashSet<PathBuf> = HashSet::new();
let mut blocked_plain_names: HashSet<String> = HashSet::new();
for input in inputs {
if let UserInput::Skill { name, path } = input {
blocked_plain_names.insert(name.clone());
if selection_context.disabled_paths.contains(path) || seen_paths.contains(path) {
continue;
}
if let Some(skill) = selection_context
.skills
.iter()
.find(|skill| skill.path.as_path() == path.as_path())
{
seen_paths.insert(skill.path.clone());
seen_names.insert(skill.name.clone());
selected.push(skill.clone());
}
}
}
for input in inputs {
if let UserInput::Text { text, .. } = input {
let mentioned_names = extract_tool_mentions(text);
select_skills_from_mentions(
&selection_context,
&blocked_plain_names,
&mentioned_names,
&mut seen_names,
&mut seen_paths,
@@ -254,6 +278,7 @@ pub(crate) fn extract_tool_mentions(text: &str) -> ToolMentions<'_> {
/// Select mentioned skills while preserving the order of `skills`.
fn select_skills_from_mentions(
selection_context: &SkillSelectionContext<'_>,
blocked_plain_names: &HashSet<String>,
mentions: &ToolMentions<'_>,
seen_names: &mut HashSet<String>,
seen_paths: &mut HashSet<PathBuf>,
@@ -296,6 +321,9 @@ fn select_skills_from_mentions(
continue;
}
if blocked_plain_names.contains(skill.name.as_str()) {
continue;
}
if !mentions.plain_names.contains(skill.name.as_str()) {
continue;
}
@@ -586,10 +614,10 @@ mod tests {
}
#[test]
fn collect_explicit_skill_mentions_ignores_structured_inputs() {
fn collect_explicit_skill_mentions_prioritizes_structured_inputs() {
let alpha = make_skill("alpha-skill", "/tmp/alpha");
let beta = make_skill("beta-skill", "/tmp/beta");
let skills = vec![alpha.clone(), beta];
let skills = vec![alpha.clone(), beta.clone()];
let inputs = vec![
UserInput::Text {
text: "please run $alpha-skill".to_string(),
@@ -604,7 +632,50 @@ mod tests {
let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts);
assert_eq!(selected, vec![alpha]);
assert_eq!(selected, vec![beta, alpha]);
}
#[test]
fn collect_explicit_skill_mentions_skips_invalid_structured_and_blocks_plain_fallback() {
let alpha = make_skill("alpha-skill", "/tmp/alpha");
let skills = vec![alpha];
let inputs = vec![
UserInput::Text {
text: "please run $alpha-skill".to_string(),
text_elements: Vec::new(),
},
UserInput::Skill {
name: "alpha-skill".to_string(),
path: PathBuf::from("/tmp/missing"),
},
];
let connector_counts = HashMap::new();
let selected = collect_mentions(&inputs, &skills, &HashSet::new(), &connector_counts);
assert_eq!(selected, Vec::new());
}
#[test]
fn collect_explicit_skill_mentions_skips_disabled_structured_and_blocks_plain_fallback() {
let alpha = make_skill("alpha-skill", "/tmp/alpha");
let skills = vec![alpha];
let inputs = vec![
UserInput::Text {
text: "please run $alpha-skill".to_string(),
text_elements: Vec::new(),
},
UserInput::Skill {
name: "alpha-skill".to_string(),
path: PathBuf::from("/tmp/alpha"),
},
];
let disabled = HashSet::from([PathBuf::from("/tmp/alpha")]);
let connector_counts = HashMap::new();
let selected = collect_mentions(&inputs, &skills, &disabled, &connector_counts);
assert_eq!(selected, Vec::new());
}
#[test]
+6
View File
@@ -80,6 +80,7 @@ async fn responses_stream_includes_subagent_header_on_review() {
None,
Some("test@test.com".to_string()),
Some(auth_mode),
"test_originator".to_string(),
false,
"test".to_string(),
session_source.clone(),
@@ -94,6 +95,7 @@ async fn responses_stream_includes_subagent_header_on_review() {
false,
false,
false,
false,
None,
);
let mut client_session = client.new_session();
@@ -182,6 +184,7 @@ async fn responses_stream_includes_subagent_header_on_other() {
None,
Some("test@test.com".to_string()),
Some(auth_mode),
"test_originator".to_string(),
false,
"test".to_string(),
session_source.clone(),
@@ -196,6 +199,7 @@ async fn responses_stream_includes_subagent_header_on_other() {
false,
false,
false,
false,
None,
);
let mut client_session = client.new_session();
@@ -283,6 +287,7 @@ async fn responses_respects_model_info_overrides_from_config() {
None,
Some("test@test.com".to_string()),
auth_mode,
"test_originator".to_string(),
false,
"test".to_string(),
session_source.clone(),
@@ -297,6 +302,7 @@ async fn responses_respects_model_info_overrides_from_config() {
false,
false,
false,
false,
None,
);
let mut client_session = client.new_session();
@@ -1,4 +1,5 @@
use anyhow::Result;
use codex_core::features::Feature;
use core_test_support::responses::WebSocketConnectionConfig;
use core_test_support::responses::ev_assistant_message;
use core_test_support::responses::ev_completed;
@@ -13,6 +14,8 @@ use pretty_assertions::assert_eq;
use serde_json::Value;
use std::time::Duration;
const WS_V2_BETA_HEADER_VALUE: &str = "responses_websockets=2026-02-06";
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn websocket_test_codex_shell_chain() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -120,3 +123,70 @@ async fn websocket_first_turn_waits_for_inflight_preconnect() -> Result<()> {
server.shutdown().await;
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn websocket_v2_test_codex_shell_chain() -> Result<()> {
skip_if_no_network!(Ok(()));
let call_id = "shell-command-call";
let server = start_websocket_server(vec![vec![
vec![
ev_response_created("resp-1"),
ev_shell_command_call(call_id, "echo websocket"),
ev_completed("resp-1"),
],
vec![
ev_response_created("resp-2"),
ev_assistant_message("msg-1", "done"),
ev_completed("resp-2"),
],
]])
.await;
let mut builder = test_codex().with_config(|config| {
config.features.enable(Feature::ResponsesWebsocketsV2);
});
let test = builder.build_with_websocket_server(&server).await?;
test.submit_turn("run the echo command").await?;
let connection = server.single_connection();
assert_eq!(connection.len(), 2);
let first = connection
.first()
.expect("missing first request")
.body_json();
let second = connection
.get(1)
.expect("missing second request")
.body_json();
assert_eq!(first["type"].as_str(), Some("response.create"));
assert_eq!(second["type"].as_str(), Some("response.create"));
assert_eq!(second["previous_response_id"].as_str(), Some("resp-1"));
let create_items = second
.get("input")
.and_then(Value::as_array)
.expect("response.create input array");
assert!(!create_items.is_empty());
let output_item = create_items
.iter()
.find(|item| item.get("type").and_then(Value::as_str) == Some("function_call_output"))
.expect("function_call_output in create");
assert_eq!(
output_item.get("call_id").and_then(Value::as_str),
Some(call_id)
);
let handshake = server.single_handshake();
assert_eq!(
handshake.header("openai-beta"),
Some(WS_V2_BETA_HEADER_VALUE.to_string())
);
server.shutdown().await;
Ok(())
}
+2
View File
@@ -1259,6 +1259,7 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() {
None,
Some("test@test.com".to_string()),
auth_manager.auth_mode().map(TelemetryAuthMode::from),
"test_originator".to_string(),
false,
"test".to_string(),
SessionSource::Exec,
@@ -1273,6 +1274,7 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() {
false,
false,
false,
false,
None,
);
let mut client_session = client.new_session();
@@ -42,6 +42,7 @@ use tracing_test::traced_test;
const MODEL: &str = "gpt-5.2-codex";
const OPENAI_BETA_HEADER: &str = "OpenAI-Beta";
const OPENAI_BETA_RESPONSES_WEBSOCKETS: &str = "responses_websockets=2026-02-04";
const WS_V2_BETA_HEADER_VALUE: &str = "responses_websockets=2026-02-06";
struct WebsocketTestHarness {
_codex_home: TempDir,
@@ -456,6 +457,165 @@ async fn responses_websocket_creates_on_non_prefix() {
server.shutdown().await;
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn responses_websocket_v2_creates_with_previous_response_id_on_prefix() {
skip_if_no_network!();
let server = start_websocket_server(vec![vec![
vec![ev_response_created("resp-1"), ev_completed("resp-1")],
vec![ev_response_created("resp-2"), ev_completed("resp-2")],
]])
.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;
stream_until_complete(&mut session, &harness, &prompt_two).await;
let connection = server.single_connection();
assert_eq!(connection.len(), 2);
let first = connection.first().expect("missing request").body_json();
let second = connection.get(1).expect("missing request").body_json();
assert_eq!(first["type"].as_str(), Some("response.create"));
assert_eq!(second["type"].as_str(), Some("response.create"));
assert_eq!(second["previous_response_id"].as_str(), Some("resp-1"));
assert_eq!(
second["input"],
serde_json::to_value(&prompt_two.input[1..]).unwrap()
);
server.shutdown().await;
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn responses_websocket_v2_after_error_uses_full_create_without_previous_response_id() {
skip_if_no_network!();
let server = start_websocket_server(vec![
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"
}
}
})],
],
vec![vec![ev_response_created("resp-3"), ev_completed("resp-3")]],
])
.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")]);
let prompt_three = prompt_with_input(vec![
message_item("hello"),
message_item("second"),
message_item("third"),
]);
stream_until_complete(&mut session, &harness, &prompt_one).await;
let mut second_stream = session
.stream(
&prompt_two,
&harness.model_info,
&harness.otel_manager,
harness.effort,
harness.summary,
None,
)
.await
.expect("websocket stream failed");
let mut saw_error = false;
while let Some(event) = second_stream.next().await {
if event.is_err() {
saw_error = true;
break;
}
}
assert!(saw_error, "expected second websocket stream to error");
stream_until_complete(&mut session, &harness, &prompt_three).await;
assert_eq!(server.handshakes().len(), 2);
let connections = server.connections();
assert_eq!(connections.len(), 2);
let first_connection = connections.first().expect("missing first connection");
assert_eq!(first_connection.len(), 2);
let first = first_connection
.first()
.expect("missing first request")
.body_json();
let second = first_connection
.get(1)
.expect("missing second request")
.body_json();
let third = connections
.get(1)
.and_then(|connection| connection.first())
.expect("missing third request")
.body_json();
assert_eq!(first["type"].as_str(), Some("response.create"));
assert_eq!(second["type"].as_str(), Some("response.create"));
assert_eq!(second["previous_response_id"].as_str(), Some("resp-1"));
assert_eq!(third["type"].as_str(), Some("response.create"));
assert_eq!(third.get("previous_response_id"), None);
assert_eq!(
third["input"],
serde_json::to_value(&prompt_three.input).unwrap()
);
server.shutdown().await;
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn responses_websocket_v2_sets_openai_beta_header() {
skip_if_no_network!();
let server = start_websocket_server(vec![vec![vec![
ev_response_created("resp-1"),
ev_completed("resp-1"),
]]])
.await;
let harness = websocket_harness_with_v2(&server, true).await;
let mut session = harness.client.new_session();
let prompt = prompt_with_input(vec![message_item("hello")]);
stream_until_complete(&mut session, &harness, &prompt).await;
let handshake = server.single_handshake();
let openai_beta_header = handshake
.header(OPENAI_BETA_HEADER)
.expect("missing OpenAI-Beta header");
assert!(
openai_beta_header
.split(',')
.map(str::trim)
.any(|value| value == WS_V2_BETA_HEADER_VALUE)
);
assert!(
!openai_beta_header
.split(',')
.map(str::trim)
.any(|value| value == OPENAI_BETA_RESPONSES_WEBSOCKETS)
);
server.shutdown().await;
}
fn message_item(text: &str) -> ResponseItem {
ResponseItem::Message {
id: None,
@@ -498,6 +658,21 @@ async fn websocket_harness(server: &WebSocketTestServer) -> WebsocketTestHarness
async fn websocket_harness_with_runtime_metrics(
server: &WebSocketTestServer,
runtime_metrics_enabled: bool,
) -> WebsocketTestHarness {
websocket_harness_with_options(server, runtime_metrics_enabled, false).await
}
async fn websocket_harness_with_v2(
server: &WebSocketTestServer,
websocket_v2_enabled: bool,
) -> WebsocketTestHarness {
websocket_harness_with_options(server, false, websocket_v2_enabled).await
}
async fn websocket_harness_with_options(
server: &WebSocketTestServer,
runtime_metrics_enabled: bool,
websocket_v2_enabled: bool,
) -> WebsocketTestHarness {
let provider = websocket_provider(server);
let codex_home = TempDir::new().unwrap();
@@ -507,6 +682,9 @@ async fn websocket_harness_with_runtime_metrics(
if runtime_metrics_enabled {
config.features.enable(Feature::RuntimeMetrics);
}
if websocket_v2_enabled {
config.features.enable(Feature::ResponsesWebsocketsV2);
}
let config = Arc::new(config);
let model_info = ModelsManager::construct_model_info_offline(MODEL, &config);
let conversation_id = ThreadId::new();
@@ -524,6 +702,7 @@ async fn websocket_harness_with_runtime_metrics(
None,
Some("test@test.com".to_string()),
auth_manager.auth_mode().map(TelemetryAuthMode::from),
"test_originator".to_string(),
false,
"test".to_string(),
SessionSource::Exec,
@@ -538,6 +717,7 @@ async fn websocket_harness_with_runtime_metrics(
SessionSource::Exec,
config.model_verbosity,
true,
websocket_v2_enabled,
false,
runtime_metrics_enabled,
None,
+1 -1
View File
@@ -101,7 +101,7 @@ async fn live_skills_reload_refreshes_skill_cache_after_skill_change() -> Result
});
let test = builder.build(&server).await?;
let skill_path = std::fs::canonicalize(test.codex_home_path().join("skills/demo/SKILL.md"))?;
let skill_path = dunce::canonicalize(test.codex_home_path().join("skills/demo/SKILL.md"))?;
submit_skill_turn(&test, skill_path.clone(), "please use $demo").await?;
let first_request = responses
+1 -2
View File
@@ -17,7 +17,7 @@ It enforces an allow/deny policy and a "limited" mode intended for read-only net
Example config:
```toml
[network_proxy]
[network]
enabled = true
proxy_url = "http://127.0.0.1:3128"
admin_url = "http://127.0.0.1:8080"
@@ -35,7 +35,6 @@ dangerously_allow_non_loopback_proxy = false
dangerously_allow_non_loopback_admin = false
mode = "full" # default when unset; use "limited" for read-only mode
[network_proxy.policy]
# Hosts must match the allowlist (unless denied).
# If `allowed_domains` is empty, the proxy blocks requests until an allowlist is configured.
allowed_domains = ["*.openai.com"]
+21 -39
View File
@@ -11,7 +11,7 @@ use url::Url;
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
pub struct NetworkProxyConfig {
#[serde(default)]
pub network_proxy: NetworkProxySettings,
pub network: NetworkProxySettings,
}
#[derive(Debug, Clone, Serialize, Deserialize)]
@@ -37,7 +37,13 @@ pub struct NetworkProxySettings {
#[serde(default)]
pub mode: NetworkMode,
#[serde(default)]
pub policy: NetworkPolicy,
pub allowed_domains: Vec<String>,
#[serde(default)]
pub denied_domains: Vec<String>,
#[serde(default)]
pub allow_unix_sockets: Vec<String>,
#[serde(default)]
pub allow_local_binding: bool,
}
impl Default for NetworkProxySettings {
@@ -53,23 +59,14 @@ impl Default for NetworkProxySettings {
dangerously_allow_non_loopback_proxy: false,
dangerously_allow_non_loopback_admin: false,
mode: NetworkMode::default(),
policy: NetworkPolicy::default(),
allowed_domains: Vec::new(),
denied_domains: Vec::new(),
allow_unix_sockets: Vec::new(),
allow_local_binding: false,
}
}
}
#[derive(Debug, Clone, Serialize, Deserialize, Default)]
pub struct NetworkPolicy {
#[serde(default)]
pub allowed_domains: Vec<String>,
#[serde(default)]
pub denied_domains: Vec<String>,
#[serde(default)]
pub allow_unix_sockets: Vec<String>,
#[serde(default)]
pub allow_local_binding: bool,
}
#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)]
#[serde(rename_all = "lowercase")]
pub enum NetworkMode {
@@ -142,7 +139,7 @@ pub(crate) fn clamp_bind_addrs(
cfg.dangerously_allow_non_loopback_admin,
"admin API",
);
if cfg.policy.allow_unix_sockets.is_empty() {
if cfg.allow_unix_sockets.is_empty() {
return (http_addr, socks_addr, admin_addr);
}
@@ -179,26 +176,14 @@ pub struct RuntimeConfig {
}
pub fn resolve_runtime(cfg: &NetworkProxyConfig) -> Result<RuntimeConfig> {
let http_addr = resolve_addr(&cfg.network_proxy.proxy_url, 3128).with_context(|| {
format!(
"invalid network_proxy.proxy_url: {}",
cfg.network_proxy.proxy_url
)
})?;
let socks_addr = resolve_addr(&cfg.network_proxy.socks_url, 8081).with_context(|| {
format!(
"invalid network_proxy.socks_url: {}",
cfg.network_proxy.socks_url
)
})?;
let admin_addr = resolve_addr(&cfg.network_proxy.admin_url, 8080).with_context(|| {
format!(
"invalid network_proxy.admin_url: {}",
cfg.network_proxy.admin_url
)
})?;
let http_addr = resolve_addr(&cfg.network.proxy_url, 3128)
.with_context(|| format!("invalid network.proxy_url: {}", cfg.network.proxy_url))?;
let socks_addr = resolve_addr(&cfg.network.socks_url, 8081)
.with_context(|| format!("invalid network.socks_url: {}", cfg.network.socks_url))?;
let admin_addr = resolve_addr(&cfg.network.admin_url, 8080)
.with_context(|| format!("invalid network.admin_url: {}", cfg.network.admin_url))?;
let (http_addr, socks_addr, admin_addr) =
clamp_bind_addrs(http_addr, socks_addr, admin_addr, &cfg.network_proxy);
clamp_bind_addrs(http_addr, socks_addr, admin_addr, &cfg.network);
Ok(RuntimeConfig {
http_addr,
@@ -453,10 +438,7 @@ mod tests {
let cfg = NetworkProxySettings {
dangerously_allow_non_loopback_proxy: true,
dangerously_allow_non_loopback_admin: true,
policy: NetworkPolicy {
allow_unix_sockets: vec!["/tmp/docker.sock".to_string()],
..Default::default()
},
allow_unix_sockets: vec!["/tmp/docker.sock".to_string()],
..Default::default()
};
let http_addr = "0.0.0.0:3128".parse::<SocketAddr>().unwrap();
+2 -2
View File
@@ -688,7 +688,7 @@ mod tests {
use super::*;
use crate::config::NetworkMode;
use crate::config::NetworkPolicy;
use crate::config::NetworkProxySettings;
use crate::runtime::network_proxy_state_for_policy;
use pretty_assertions::assert_eq;
use rama_http::Method;
@@ -697,7 +697,7 @@ mod tests {
#[tokio::test]
async fn http_connect_accept_blocks_in_limited_mode() {
let policy = NetworkPolicy {
let policy = NetworkProxySettings {
allowed_domains: vec!["example.com".to_string()],
..Default::default()
};
+6 -6
View File
@@ -217,7 +217,7 @@ fn map_decider_decision(decision: NetworkDecision) -> NetworkDecision {
mod tests {
use super::*;
use crate::config::NetworkPolicy;
use crate::config::NetworkProxySettings;
use crate::reasons::REASON_DENIED;
use crate::reasons::REASON_NOT_ALLOWED_LOCAL;
use crate::state::network_proxy_state_for_policy;
@@ -228,7 +228,7 @@ mod tests {
#[tokio::test]
async fn evaluate_host_policy_invokes_decider_for_not_allowed() {
let state = network_proxy_state_for_policy(NetworkPolicy::default());
let state = network_proxy_state_for_policy(NetworkProxySettings::default());
let calls = Arc::new(AtomicUsize::new(0));
let decider: Arc<dyn NetworkPolicyDecider> = Arc::new({
let calls = calls.clone();
@@ -259,10 +259,10 @@ mod tests {
#[tokio::test]
async fn evaluate_host_policy_skips_decider_for_denied() {
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["example.com".to_string()],
denied_domains: vec!["blocked.com".to_string()],
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
let calls = Arc::new(AtomicUsize::new(0));
let decider: Arc<dyn NetworkPolicyDecider> = Arc::new({
@@ -299,10 +299,10 @@ mod tests {
#[tokio::test]
async fn evaluate_host_policy_skips_decider_for_not_allowed_local() {
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["example.com".to_string()],
allow_local_binding: false,
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
let calls = Arc::new(AtomicUsize::new(0));
let decider: Arc<dyn NetworkPolicyDecider> = Arc::new({
+5 -5
View File
@@ -66,7 +66,7 @@ impl NetworkProxyBuilder {
self.http_addr.unwrap_or(runtime.http_addr),
runtime.socks_addr,
self.admin_addr.unwrap_or(runtime.admin_addr),
&current_cfg.network_proxy,
&current_cfg.network,
);
Ok(NetworkProxy {
@@ -95,8 +95,8 @@ impl NetworkProxy {
pub async fn run(&self) -> Result<NetworkProxyHandle> {
let current_cfg = self.state.current_cfg().await?;
if !current_cfg.network_proxy.enabled {
warn!("network_proxy.enabled is false; skipping proxy listeners");
if !current_cfg.network.enabled {
warn!("network.enabled is false; skipping proxy listeners");
return Ok(NetworkProxyHandle::noop());
}
@@ -109,12 +109,12 @@ impl NetworkProxy {
self.http_addr,
self.policy_decider.clone(),
));
let socks_task = if current_cfg.network_proxy.enable_socks5 {
let socks_task = if current_cfg.network.enable_socks5 {
Some(tokio::spawn(socks5::run_socks5(
self.state.clone(),
self.socks_addr,
self.policy_decider.clone(),
current_cfg.network_proxy.enable_socks5_udp,
current_cfg.network.enable_socks5_udp,
)))
} else {
None
+67 -90
View File
@@ -161,15 +161,15 @@ impl NetworkProxyState {
self.reload_if_needed().await?;
let guard = self.state.read().await;
Ok((
guard.config.network_proxy.policy.allowed_domains.clone(),
guard.config.network_proxy.policy.denied_domains.clone(),
guard.config.network.allowed_domains.clone(),
guard.config.network.denied_domains.clone(),
))
}
pub async fn enabled(&self) -> Result<bool> {
self.reload_if_needed().await?;
let guard = self.state.read().await;
Ok(guard.config.network_proxy.enabled)
Ok(guard.config.network.enabled)
}
pub async fn force_reload(&self) -> Result<()> {
@@ -209,9 +209,9 @@ impl NetworkProxyState {
(
guard.deny_set.clone(),
guard.allow_set.clone(),
guard.config.network_proxy.policy.allow_local_binding,
guard.config.network_proxy.policy.allowed_domains.is_empty(),
guard.config.network_proxy.policy.allowed_domains.clone(),
guard.config.network.allow_local_binding,
guard.config.network.allowed_domains.is_empty(),
guard.config.network.allowed_domains.clone(),
)
};
@@ -305,7 +305,7 @@ impl NetworkProxyState {
Err(_) => return Ok(false),
};
let requested_canonical = std::fs::canonicalize(requested_abs.as_path()).ok();
for allowed in &guard.config.network_proxy.policy.allow_unix_sockets {
for allowed in &guard.config.network.allow_unix_sockets {
if allowed == path {
return Ok(true);
}
@@ -327,19 +327,19 @@ impl NetworkProxyState {
pub async fn method_allowed(&self, method: &str) -> Result<bool> {
self.reload_if_needed().await?;
let guard = self.state.read().await;
Ok(guard.config.network_proxy.mode.allows_method(method))
Ok(guard.config.network.mode.allows_method(method))
}
pub async fn allow_upstream_proxy(&self) -> Result<bool> {
self.reload_if_needed().await?;
let guard = self.state.read().await;
Ok(guard.config.network_proxy.allow_upstream_proxy)
Ok(guard.config.network.allow_upstream_proxy)
}
pub async fn network_mode(&self) -> Result<NetworkMode> {
self.reload_if_needed().await?;
let guard = self.state.read().await;
Ok(guard.config.network_proxy.mode)
Ok(guard.config.network.mode)
}
pub async fn set_network_mode(&self, mode: NetworkMode) -> Result<()> {
@@ -348,19 +348,19 @@ impl NetworkProxyState {
let (candidate, constraints) = {
let guard = self.state.read().await;
let mut candidate = guard.config.clone();
candidate.network_proxy.mode = mode;
candidate.network.mode = mode;
(candidate, guard.constraints.clone())
};
validate_policy_against_constraints(&candidate, &constraints)
.context("network_proxy.mode constrained by managed config")?;
.context("network.mode constrained by managed config")?;
let mut guard = self.state.write().await;
if guard.constraints != constraints {
drop(guard);
continue;
}
guard.config.network_proxy.mode = mode;
guard.config.network.mode = mode;
info!("updated network mode to {mode:?}");
return Ok(());
}
@@ -417,13 +417,13 @@ async fn host_resolves_to_non_public_ip(host: &str, port: u16) -> bool {
fn log_policy_changes(previous: &NetworkProxyConfig, next: &NetworkProxyConfig) {
log_domain_list_changes(
"allowlist",
&previous.network_proxy.policy.allowed_domains,
&next.network_proxy.policy.allowed_domains,
&previous.network.allowed_domains,
&next.network.allowed_domains,
);
log_domain_list_changes(
"denylist",
&previous.network_proxy.policy.denied_domains,
&next.network_proxy.policy.denied_domains,
&previous.network.denied_domains,
&next.network.denied_domains,
);
}
@@ -483,21 +483,14 @@ fn unix_timestamp() -> i64 {
#[cfg(test)]
pub(crate) fn network_proxy_state_for_policy(
policy: crate::config::NetworkPolicy,
mut network: crate::config::NetworkProxySettings,
) -> NetworkProxyState {
let config = NetworkProxyConfig {
network_proxy: crate::config::NetworkProxySettings {
enabled: true,
mode: NetworkMode::Full,
policy,
..crate::config::NetworkProxySettings::default()
},
};
network.enabled = true;
network.mode = NetworkMode::Full;
let config = NetworkProxyConfig { network };
let allow_set =
crate::policy::compile_globset(&config.network_proxy.policy.allowed_domains).unwrap();
let deny_set =
crate::policy::compile_globset(&config.network_proxy.policy.denied_domains).unwrap();
let allow_set = crate::policy::compile_globset(&config.network.allowed_domains).unwrap();
let deny_set = crate::policy::compile_globset(&config.network.denied_domains).unwrap();
let state = ConfigState {
config,
@@ -518,7 +511,6 @@ pub(crate) fn network_proxy_state_for_policy(
mod tests {
use super::*;
use crate::config::NetworkPolicy;
use crate::config::NetworkProxyConfig;
use crate::config::NetworkProxySettings;
use crate::policy::compile_globset;
@@ -528,10 +520,10 @@ mod tests {
#[tokio::test]
async fn host_blocked_denied_wins_over_allowed() {
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["example.com".to_string()],
denied_domains: vec!["example.com".to_string()],
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
assert_eq!(
@@ -542,9 +534,9 @@ mod tests {
#[tokio::test]
async fn host_blocked_requires_allowlist_match() {
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["example.com".to_string()],
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
assert_eq!(
@@ -561,9 +553,9 @@ mod tests {
#[tokio::test]
async fn host_blocked_subdomain_wildcards_exclude_apex() {
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["*.openai.com".to_string()],
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
assert_eq!(
@@ -578,10 +570,10 @@ mod tests {
#[tokio::test]
async fn host_blocked_rejects_loopback_when_local_binding_disabled() {
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["example.com".to_string()],
allow_local_binding: false,
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
assert_eq!(
@@ -596,10 +588,10 @@ mod tests {
#[tokio::test]
async fn host_blocked_rejects_loopback_when_allowlist_is_wildcard() {
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["*".to_string()],
allow_local_binding: false,
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
assert_eq!(
@@ -610,10 +602,10 @@ mod tests {
#[tokio::test]
async fn host_blocked_rejects_private_ip_literal_when_allowlist_is_wildcard() {
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["*".to_string()],
allow_local_binding: false,
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
assert_eq!(
@@ -624,10 +616,10 @@ mod tests {
#[tokio::test]
async fn host_blocked_allows_loopback_when_explicitly_allowlisted_and_local_binding_disabled() {
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["localhost".to_string()],
allow_local_binding: false,
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
assert_eq!(
@@ -638,10 +630,10 @@ mod tests {
#[tokio::test]
async fn host_blocked_allows_private_ip_literal_when_explicitly_allowlisted() {
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["10.0.0.1".to_string()],
allow_local_binding: false,
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
assert_eq!(
@@ -652,10 +644,10 @@ mod tests {
#[tokio::test]
async fn host_blocked_rejects_scoped_ipv6_literal_when_not_allowlisted() {
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["example.com".to_string()],
allow_local_binding: false,
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
assert_eq!(
@@ -666,10 +658,10 @@ mod tests {
#[tokio::test]
async fn host_blocked_allows_scoped_ipv6_literal_when_explicitly_allowlisted() {
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["fe80::1%lo0".to_string()],
allow_local_binding: false,
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
assert_eq!(
@@ -680,10 +672,10 @@ mod tests {
#[tokio::test]
async fn host_blocked_rejects_private_ip_literals_when_local_binding_disabled() {
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["example.com".to_string()],
allow_local_binding: false,
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
assert_eq!(
@@ -694,10 +686,10 @@ mod tests {
#[tokio::test]
async fn host_blocked_rejects_loopback_when_allowlist_empty() {
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec![],
allow_local_binding: false,
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
assert_eq!(
@@ -714,12 +706,9 @@ mod tests {
};
let config = NetworkProxyConfig {
network_proxy: NetworkProxySettings {
network: NetworkProxySettings {
enabled: true,
policy: NetworkPolicy {
allowed_domains: vec!["example.com".to_string(), "evil.com".to_string()],
..NetworkPolicy::default()
},
allowed_domains: vec!["example.com".to_string(), "evil.com".to_string()],
..NetworkProxySettings::default()
},
};
@@ -735,7 +724,7 @@ mod tests {
};
let config = NetworkProxyConfig {
network_proxy: NetworkProxySettings {
network: NetworkProxySettings {
enabled: true,
mode: NetworkMode::Full,
..NetworkProxySettings::default()
@@ -753,12 +742,9 @@ mod tests {
};
let config = NetworkProxyConfig {
network_proxy: NetworkProxySettings {
network: NetworkProxySettings {
enabled: true,
policy: NetworkPolicy {
allowed_domains: vec!["api.example.com".to_string()],
..NetworkPolicy::default()
},
allowed_domains: vec!["api.example.com".to_string()],
..NetworkProxySettings::default()
},
};
@@ -774,12 +760,9 @@ mod tests {
};
let config = NetworkProxyConfig {
network_proxy: NetworkProxySettings {
network: NetworkProxySettings {
enabled: true,
policy: NetworkPolicy {
allowed_domains: vec!["**.example.com".to_string()],
..NetworkPolicy::default()
},
allowed_domains: vec!["**.example.com".to_string()],
..NetworkProxySettings::default()
},
};
@@ -795,12 +778,9 @@ mod tests {
};
let config = NetworkProxyConfig {
network_proxy: NetworkProxySettings {
network: NetworkProxySettings {
enabled: true,
policy: NetworkPolicy {
denied_domains: vec![],
..NetworkPolicy::default()
},
denied_domains: vec![],
..NetworkProxySettings::default()
},
};
@@ -816,7 +796,7 @@ mod tests {
};
let config = NetworkProxyConfig {
network_proxy: NetworkProxySettings {
network: NetworkProxySettings {
enabled: true,
..NetworkProxySettings::default()
},
@@ -833,12 +813,9 @@ mod tests {
};
let config = NetworkProxyConfig {
network_proxy: NetworkProxySettings {
network: NetworkProxySettings {
enabled: true,
policy: NetworkPolicy {
allow_local_binding: true,
..NetworkPolicy::default()
},
allow_local_binding: true,
..NetworkProxySettings::default()
},
};
@@ -854,7 +831,7 @@ mod tests {
};
let config = NetworkProxyConfig {
network_proxy: NetworkProxySettings {
network: NetworkProxySettings {
enabled: true,
dangerously_allow_non_loopback_admin: true,
..NetworkProxySettings::default()
@@ -872,7 +849,7 @@ mod tests {
};
let config = NetworkProxyConfig {
network_proxy: NetworkProxySettings {
network: NetworkProxySettings {
enabled: true,
dangerously_allow_non_loopback_admin: true,
..NetworkProxySettings::default()
@@ -935,10 +912,10 @@ mod tests {
#[tokio::test]
async fn unix_socket_allowlist_is_respected_on_macos() {
let socket_path = "/tmp/example.sock".to_string();
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["example.com".to_string()],
allow_unix_sockets: vec![socket_path.clone()],
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
assert!(state.is_unix_socket_allowed(&socket_path).await.unwrap());
@@ -970,10 +947,10 @@ mod tests {
let real_s = real.to_str().unwrap().to_string();
let link_s = link.to_str().unwrap().to_string();
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["example.com".to_string()],
allow_unix_sockets: vec![real_s],
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
assert!(state.is_unix_socket_allowed(&link_s).await.unwrap());
@@ -983,10 +960,10 @@ mod tests {
#[tokio::test]
async fn unix_socket_allowlist_is_rejected_on_non_macos() {
let socket_path = "/tmp/example.sock".to_string();
let state = network_proxy_state_for_policy(NetworkPolicy {
let state = network_proxy_state_for_policy(NetworkProxySettings {
allowed_domains: vec!["example.com".to_string()],
allow_unix_sockets: vec![socket_path.clone()],
..NetworkPolicy::default()
..NetworkProxySettings::default()
});
assert!(!state.is_unix_socket_allowed(&socket_path).await.unwrap());
+88 -97
View File
@@ -8,7 +8,6 @@ use anyhow::Context;
use anyhow::Result;
use codex_app_server_protocol::ConfigLayerSource;
use codex_core::config::CONFIG_TOML_FILE;
use codex_core::config::Constrained;
use codex_core::config::ConstraintError;
use codex_core::config::find_codex_home;
use codex_core::config_loader::CloudRequirementsLoader;
@@ -56,8 +55,8 @@ pub(crate) async fn build_config_state() -> Result<ConfigState> {
let constraints = enforce_trusted_constraints(&config_layer_stack, &config)?;
let layer_mtimes = collect_layer_mtimes(&config_layer_stack);
let deny_set = compile_globset(&config.network_proxy.policy.denied_domains)?;
let allow_set = compile_globset(&config.network_proxy.policy.allowed_domains)?;
let deny_set = compile_globset(&config.network.denied_domains)?;
let allow_set = compile_globset(&config.network.allowed_domains)?;
Ok(ConfigState {
config,
allow_set,
@@ -94,22 +93,16 @@ fn collect_layer_mtimes(stack: &ConfigLayerStack) -> Vec<LayerMtime> {
#[derive(Debug, Default, Deserialize)]
struct PartialConfig {
#[serde(default)]
network_proxy: PartialNetworkProxyConfig,
network: PartialNetworkConfig,
}
#[derive(Debug, Default, Deserialize)]
struct PartialNetworkProxyConfig {
struct PartialNetworkConfig {
enabled: Option<bool>,
mode: Option<NetworkMode>,
allow_upstream_proxy: Option<bool>,
dangerously_allow_non_loopback_proxy: Option<bool>,
dangerously_allow_non_loopback_admin: Option<bool>,
#[serde(default)]
policy: PartialNetworkPolicy,
}
#[derive(Debug, Default, Deserialize)]
struct PartialNetworkPolicy {
#[serde(default)]
allowed_domains: Option<Vec<String>>,
#[serde(default)]
@@ -137,13 +130,13 @@ fn enforce_trusted_constraints(
layers: &codex_core::config_loader::ConfigLayerStack,
config: &NetworkProxyConfig,
) -> Result<NetworkProxyConstraints> {
let constraints = network_proxy_constraints_from_trusted_layers(layers)?;
let constraints = network_constraints_from_trusted_layers(layers)?;
validate_policy_against_constraints(config, &constraints)
.context("network proxy constraints")?;
Ok(constraints)
}
fn network_proxy_constraints_from_trusted_layers(
fn network_constraints_from_trusted_layers(
layers: &codex_core::config_loader::ConfigLayerStack,
) -> Result<NetworkProxyConstraints> {
let mut constraints = NetworkProxyConstraints::default();
@@ -163,38 +156,38 @@ fn network_proxy_constraints_from_trusted_layers(
.try_into()
.context("failed to deserialize trusted config layer")?;
if let Some(enabled) = partial.network_proxy.enabled {
if let Some(enabled) = partial.network.enabled {
constraints.enabled = Some(enabled);
}
if let Some(mode) = partial.network_proxy.mode {
if let Some(mode) = partial.network.mode {
constraints.mode = Some(mode);
}
if let Some(allow_upstream_proxy) = partial.network_proxy.allow_upstream_proxy {
if let Some(allow_upstream_proxy) = partial.network.allow_upstream_proxy {
constraints.allow_upstream_proxy = Some(allow_upstream_proxy);
}
if let Some(dangerously_allow_non_loopback_proxy) =
partial.network_proxy.dangerously_allow_non_loopback_proxy
partial.network.dangerously_allow_non_loopback_proxy
{
constraints.dangerously_allow_non_loopback_proxy =
Some(dangerously_allow_non_loopback_proxy);
}
if let Some(dangerously_allow_non_loopback_admin) =
partial.network_proxy.dangerously_allow_non_loopback_admin
partial.network.dangerously_allow_non_loopback_admin
{
constraints.dangerously_allow_non_loopback_admin =
Some(dangerously_allow_non_loopback_admin);
}
if let Some(allowed_domains) = partial.network_proxy.policy.allowed_domains {
if let Some(allowed_domains) = partial.network.allowed_domains {
constraints.allowed_domains = Some(allowed_domains);
}
if let Some(denied_domains) = partial.network_proxy.policy.denied_domains {
if let Some(denied_domains) = partial.network.denied_domains {
constraints.denied_domains = Some(denied_domains);
}
if let Some(allow_unix_sockets) = partial.network_proxy.policy.allow_unix_sockets {
if let Some(allow_unix_sockets) = partial.network.allow_unix_sockets {
constraints.allow_unix_sockets = Some(allow_unix_sockets);
}
if let Some(allow_local_binding) = partial.network_proxy.policy.allow_local_binding {
if let Some(allow_local_binding) = partial.network.allow_local_binding {
constraints.allow_local_binding = Some(allow_local_binding);
}
}
@@ -227,12 +220,19 @@ pub(crate) fn validate_policy_against_constraints(
}
}
let enabled = config.network_proxy.enabled;
fn validate<T>(
candidate: T,
validator: impl FnOnce(&T) -> std::result::Result<(), ConstraintError>,
) -> std::result::Result<(), ConstraintError> {
validator(&candidate)
}
let enabled = config.network.enabled;
if let Some(max_enabled) = constraints.enabled {
let _ = Constrained::new(enabled, move |candidate| {
validate(enabled, move |candidate| {
if *candidate && !max_enabled {
Err(invalid_value(
"network_proxy.enabled",
"network.enabled",
"true",
"false (disabled by managed config)",
))
@@ -243,10 +243,10 @@ pub(crate) fn validate_policy_against_constraints(
}
if let Some(max_mode) = constraints.mode {
let _ = Constrained::new(config.network_proxy.mode, move |candidate| {
validate(config.network.mode, move |candidate| {
if network_mode_rank(*candidate) > network_mode_rank(max_mode) {
Err(invalid_value(
"network_proxy.mode",
"network.mode",
format!("{candidate:?}"),
format!("{max_mode:?} or more restrictive"),
))
@@ -257,14 +257,14 @@ pub(crate) fn validate_policy_against_constraints(
}
let allow_upstream_proxy = constraints.allow_upstream_proxy;
let _ = Constrained::new(
config.network_proxy.allow_upstream_proxy,
validate(
config.network.allow_upstream_proxy,
move |candidate| match allow_upstream_proxy {
Some(true) | None => Ok(()),
Some(false) => {
if *candidate {
Err(invalid_value(
"network_proxy.allow_upstream_proxy",
"network.allow_upstream_proxy",
"true",
"false (disabled by managed config)",
))
@@ -276,14 +276,14 @@ pub(crate) fn validate_policy_against_constraints(
)?;
let allow_non_loopback_admin = constraints.dangerously_allow_non_loopback_admin;
let _ = Constrained::new(
config.network_proxy.dangerously_allow_non_loopback_admin,
validate(
config.network.dangerously_allow_non_loopback_admin,
move |candidate| match allow_non_loopback_admin {
Some(true) | None => Ok(()),
Some(false) => {
if *candidate {
Err(invalid_value(
"network_proxy.dangerously_allow_non_loopback_admin",
"network.dangerously_allow_non_loopback_admin",
"true",
"false (disabled by managed config)",
))
@@ -295,14 +295,14 @@ pub(crate) fn validate_policy_against_constraints(
)?;
let allow_non_loopback_proxy = constraints.dangerously_allow_non_loopback_proxy;
let _ = Constrained::new(
config.network_proxy.dangerously_allow_non_loopback_proxy,
validate(
config.network.dangerously_allow_non_loopback_proxy,
move |candidate| match allow_non_loopback_proxy {
Some(true) | None => Ok(()),
Some(false) => {
if *candidate {
Err(invalid_value(
"network_proxy.dangerously_allow_non_loopback_proxy",
"network.dangerously_allow_non_loopback_proxy",
"true",
"false (disabled by managed config)",
))
@@ -314,20 +314,17 @@ pub(crate) fn validate_policy_against_constraints(
)?;
if let Some(allow_local_binding) = constraints.allow_local_binding {
let _ = Constrained::new(
config.network_proxy.policy.allow_local_binding,
move |candidate| {
if *candidate && !allow_local_binding {
Err(invalid_value(
"network_proxy.policy.allow_local_binding",
"true",
"false (disabled by managed config)",
))
} else {
Ok(())
}
},
)?;
validate(config.network.allow_local_binding, move |candidate| {
if *candidate && !allow_local_binding {
Err(invalid_value(
"network.allow_local_binding",
"true",
"false (disabled by managed config)",
))
} else {
Ok(())
}
})?;
}
if let Some(allowed_domains) = &constraints.allowed_domains {
@@ -335,30 +332,27 @@ pub(crate) fn validate_policy_against_constraints(
.iter()
.map(|entry| DomainPattern::parse_for_constraints(entry))
.collect();
let _ = Constrained::new(
config.network_proxy.policy.allowed_domains.clone(),
move |candidate| {
let mut invalid = Vec::new();
for entry in candidate {
let candidate_pattern = DomainPattern::parse_for_constraints(entry);
if !managed_patterns
.iter()
.any(|managed| managed.allows(&candidate_pattern))
{
invalid.push(entry.clone());
}
validate(config.network.allowed_domains.clone(), move |candidate| {
let mut invalid = Vec::new();
for entry in candidate {
let candidate_pattern = DomainPattern::parse_for_constraints(entry);
if !managed_patterns
.iter()
.any(|managed| managed.allows(&candidate_pattern))
{
invalid.push(entry.clone());
}
if invalid.is_empty() {
Ok(())
} else {
Err(invalid_value(
"network_proxy.policy.allowed_domains",
format!("{invalid:?}"),
"subset of managed allowed_domains",
))
}
},
)?;
}
if invalid.is_empty() {
Ok(())
} else {
Err(invalid_value(
"network.allowed_domains",
format!("{invalid:?}"),
"subset of managed allowed_domains",
))
}
})?;
}
if let Some(denied_domains) = &constraints.denied_domains {
@@ -366,27 +360,24 @@ pub(crate) fn validate_policy_against_constraints(
.iter()
.map(|s| s.to_ascii_lowercase())
.collect();
let _ = Constrained::new(
config.network_proxy.policy.denied_domains.clone(),
move |candidate| {
let candidate_set: HashSet<String> =
candidate.iter().map(|s| s.to_ascii_lowercase()).collect();
let missing: Vec<String> = required_set
.iter()
.filter(|entry| !candidate_set.contains(*entry))
.cloned()
.collect();
if missing.is_empty() {
Ok(())
} else {
Err(invalid_value(
"network_proxy.policy.denied_domains",
"missing managed denied_domains entries",
format!("{missing:?}"),
))
}
},
)?;
validate(config.network.denied_domains.clone(), move |candidate| {
let candidate_set: HashSet<String> =
candidate.iter().map(|s| s.to_ascii_lowercase()).collect();
let missing: Vec<String> = required_set
.iter()
.filter(|entry| !candidate_set.contains(*entry))
.cloned()
.collect();
if missing.is_empty() {
Ok(())
} else {
Err(invalid_value(
"network.denied_domains",
"missing managed denied_domains entries",
format!("{missing:?}"),
))
}
})?;
}
if let Some(allow_unix_sockets) = &constraints.allow_unix_sockets {
@@ -394,8 +385,8 @@ pub(crate) fn validate_policy_against_constraints(
.iter()
.map(|s| s.to_ascii_lowercase())
.collect();
let _ = Constrained::new(
config.network_proxy.policy.allow_unix_sockets.clone(),
validate(
config.network.allow_unix_sockets.clone(),
move |candidate| {
let mut invalid = Vec::new();
for entry in candidate {
@@ -407,7 +398,7 @@ pub(crate) fn validate_policy_against_constraints(
Ok(())
} else {
Err(invalid_value(
"network_proxy.policy.allow_unix_sockets",
"network.allow_unix_sockets",
format!("{invalid:?}"),
"subset of managed allow_unix_sockets",
))
+1
View File
@@ -44,6 +44,7 @@ pub struct OtelEventMetadata {
pub(crate) auth_mode: Option<String>,
pub(crate) account_id: Option<String>,
pub(crate) account_email: Option<String>,
pub(crate) originator: String,
pub(crate) session_source: String,
pub(crate) model: String,
pub(crate) slug: String,
+15
View File
@@ -66,6 +66,7 @@ impl OtelManager {
account_id: Option<String>,
account_email: Option<String>,
auth_mode: Option<TelemetryAuthMode>,
originator: String,
log_user_prompts: bool,
terminal_type: String,
session_source: SessionSource,
@@ -76,6 +77,7 @@ impl OtelManager {
auth_mode: auth_mode.map(|m| m.to_string()),
account_id,
account_email,
originator,
session_source: session_source.to_string(),
model: model.to_owned(),
slug: slug.to_owned(),
@@ -134,6 +136,7 @@ impl OtelManager {
conversation.id = %self.metadata.conversation_id,
app.version = %self.metadata.app_version,
auth_mode = self.metadata.auth_mode,
originator = %self.metadata.originator,
user.account_id = self.metadata.account_id,
user.email = self.metadata.account_email,
terminal.type = %self.metadata.terminal_type,
@@ -198,6 +201,7 @@ impl OtelManager {
conversation.id = %self.metadata.conversation_id,
app.version = %self.metadata.app_version,
auth_mode = self.metadata.auth_mode,
originator = %self.metadata.originator,
user.account_id = self.metadata.account_id,
user.email = self.metadata.account_email,
terminal.type = %self.metadata.terminal_type,
@@ -229,6 +233,7 @@ impl OtelManager {
conversation.id = %self.metadata.conversation_id,
app.version = %self.metadata.app_version,
auth_mode = self.metadata.auth_mode,
originator = %self.metadata.originator,
user.account_id = self.metadata.account_id,
user.email = self.metadata.account_email,
terminal.type = %self.metadata.terminal_type,
@@ -330,6 +335,7 @@ impl OtelManager {
conversation.id = %self.metadata.conversation_id,
app.version = %self.metadata.app_version,
auth_mode = self.metadata.auth_mode,
originator = %self.metadata.originator,
user.account_id = self.metadata.account_id,
user.email = self.metadata.account_email,
terminal.type = %self.metadata.terminal_type,
@@ -407,6 +413,7 @@ impl OtelManager {
conversation.id = %self.metadata.conversation_id,
app.version = %self.metadata.app_version,
auth_mode = self.metadata.auth_mode,
originator = %self.metadata.originator,
user.account_id = self.metadata.account_id,
user.email = self.metadata.account_email,
terminal.type = %self.metadata.terminal_type,
@@ -440,6 +447,7 @@ impl OtelManager {
conversation.id = %self.metadata.conversation_id,
app.version = %self.metadata.app_version,
auth_mode = self.metadata.auth_mode,
originator = %self.metadata.originator,
user.account_id = self.metadata.account_id,
user.email = self.metadata.account_email,
terminal.type = %self.metadata.terminal_type,
@@ -455,6 +463,7 @@ impl OtelManager {
conversation.id = %self.metadata.conversation_id,
app.version = %self.metadata.app_version,
auth_mode = self.metadata.auth_mode,
originator = %self.metadata.originator,
user.account_id = self.metadata.account_id,
user.email = self.metadata.account_email,
terminal.type = %self.metadata.terminal_type,
@@ -478,6 +487,7 @@ impl OtelManager {
conversation.id = %self.metadata.conversation_id,
app.version = %self.metadata.app_version,
auth_mode = self.metadata.auth_mode,
originator = %self.metadata.originator,
user.account_id = self.metadata.account_id,
user.email = self.metadata.account_email,
terminal.type = %self.metadata.terminal_type,
@@ -503,6 +513,7 @@ impl OtelManager {
conversation.id = %self.metadata.conversation_id,
app.version = %self.metadata.app_version,
auth_mode = self.metadata.auth_mode,
originator = %self.metadata.originator,
user.account_id = self.metadata.account_id,
user.email = self.metadata.account_email,
terminal.type = %self.metadata.terminal_type,
@@ -538,6 +549,7 @@ impl OtelManager {
conversation.id = %self.metadata.conversation_id,
app.version = %self.metadata.app_version,
auth_mode = self.metadata.auth_mode,
originator = %self.metadata.originator,
user.account_id = self.metadata.account_id,
user.email = self.metadata.account_email,
terminal.type = %self.metadata.terminal_type,
@@ -562,6 +574,7 @@ impl OtelManager {
conversation.id = %self.metadata.conversation_id,
app.version = %self.metadata.app_version,
auth_mode = self.metadata.auth_mode,
originator = %self.metadata.originator,
user.account_id = self.metadata.account_id,
user.email = self.metadata.account_email,
terminal.type = %self.metadata.terminal_type,
@@ -617,6 +630,7 @@ impl OtelManager {
conversation.id = %self.metadata.conversation_id,
app.version = %self.metadata.app_version,
auth_mode = self.metadata.auth_mode,
originator = %self.metadata.originator,
user.account_id = self.metadata.account_id,
user.email = self.metadata.account_email,
terminal.type = %self.metadata.terminal_type,
@@ -654,6 +668,7 @@ impl OtelManager {
conversation.id = %self.metadata.conversation_id,
app.version = %self.metadata.app_version,
auth_mode = self.metadata.auth_mode,
originator = %self.metadata.originator,
user.account_id = self.metadata.account_id,
user.email = self.metadata.account_email,
terminal.type = %self.metadata.terminal_type,
@@ -23,6 +23,7 @@ fn manager_attaches_metadata_tags_to_metrics() -> Result<()> {
Some("account-id".to_string()),
None,
Some(TelemetryAuthMode::ApiKey),
"test_originator".to_string(),
true,
"tty".to_string(),
SessionSource::Cli,
@@ -77,6 +78,7 @@ fn manager_allows_disabling_metadata_tags() -> Result<()> {
Some("account-id".to_string()),
None,
Some(TelemetryAuthMode::ApiKey),
"test_originator".to_string(),
true,
"tty".to_string(),
SessionSource::Cli,
@@ -27,6 +27,7 @@ fn runtime_metrics_summary_collects_tool_api_and_streaming_metrics() -> Result<(
Some("account-id".to_string()),
None,
Some(TelemetryAuthMode::ApiKey),
"test_originator".to_string(),
true,
"tty".to_string(),
SessionSource::Cli,
+1
View File
@@ -76,6 +76,7 @@ fn manager_snapshot_metrics_collects_without_shutdown() -> Result<()> {
Some("account-id".to_string()),
None,
Some(TelemetryAuthMode::ApiKey),
"test_originator".to_string(),
true,
"tty".to_string(),
SessionSource::Cli,
+16
View File
@@ -1,3 +1,4 @@
use crate::models::MessagePhase;
use crate::models::WebSearchAction;
use crate::protocol::AgentMessageEvent;
use crate::protocol::AgentReasoningEvent;
@@ -40,9 +41,21 @@ pub enum AgentMessageContent {
}
#[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)]
/// Assistant-authored message payload used in turn-item streams.
///
/// `phase` is optional because not all providers/models emit it. Consumers
/// should use it when present, but retain legacy completion semantics when it
/// is `None`.
pub struct AgentMessageItem {
pub id: String,
pub content: Vec<AgentMessageContent>,
/// Optional phase metadata carried through from `ResponseItem::Message`.
///
/// This is currently used by TUI rendering to distinguish mid-turn
/// commentary from a final answer and avoid status-indicator jitter.
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
pub phase: Option<MessagePhase>,
}
#[derive(Debug, Clone, Deserialize, Serialize, TS, JsonSchema)]
@@ -172,10 +185,13 @@ impl AgentMessageItem {
Self {
id: uuid::Uuid::new_v4().to_string(),
content: content.to_vec(),
phase: None,
}
}
pub fn as_legacy_events(&self) -> Vec<EventMsg> {
// Legacy events only preserve visible assistant text; `phase` has no
// representation in the v1 event stream.
self.content
.iter()
.map(|c| match c {
+11 -1
View File
@@ -74,8 +74,17 @@ pub enum ContentItem {
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, JsonSchema, TS)]
#[serde(rename_all = "snake_case")]
/// Classifies an assistant message as interim commentary or final answer text.
///
/// Providers do not emit this consistently, so callers must treat `None` as
/// "phase unknown" and keep compatibility behavior for legacy models.
pub enum MessagePhase {
/// Mid-turn assistant text (for example preamble/progress narration).
///
/// Additional tool calls or assistant output may follow before turn
/// completion.
Commentary,
/// The assistant's terminal answer text for the current turn.
FinalAnswer,
}
@@ -93,7 +102,8 @@ pub enum ResponseItem {
#[ts(optional)]
end_turn: Option<bool>,
// Optional output-message phase (for example: "commentary", "final_answer").
// Do not use directly; availability can vary by provider and model.
// Availability varies by provider/model, so downstream consumers must
// preserve fallback behavior when this is absent.
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
phase: Option<MessagePhase>,
+1
View File
@@ -15,6 +15,7 @@ axum = { workspace = true, default-features = false, features = [
] }
codex-keyring-store = { workspace = true }
codex-protocol = { workspace = true }
codex-utils-pty = { workspace = true }
codex-utils-home-dir = { workspace = true }
futures = { workspace = true, default-features = false, features = ["std"] }
keyring = { workspace = true, features = ["crypto-rust"] }
+81 -5
View File
@@ -60,7 +60,10 @@ use crate::utils::create_env_for_mcp_server;
use crate::utils::run_with_timeout;
enum PendingTransport {
ChildProcess(TokioChildProcess),
ChildProcess {
transport: TokioChildProcess,
process_group_guard: Option<ProcessGroupGuard>,
},
StreamableHttp {
transport: StreamableHttpClientTransport<reqwest::Client>,
},
@@ -75,11 +78,71 @@ enum ClientState {
transport: Option<PendingTransport>,
},
Ready {
_process_group_guard: Option<ProcessGroupGuard>,
service: Arc<RunningService<RoleClient, LoggingClientHandler>>,
oauth: Option<OAuthPersistor>,
},
}
#[cfg(unix)]
const PROCESS_GROUP_TERM_GRACE_PERIOD: Duration = Duration::from_secs(2);
#[cfg(unix)]
struct ProcessGroupGuard {
process_group_id: u32,
}
#[cfg(not(unix))]
struct ProcessGroupGuard;
impl ProcessGroupGuard {
fn new(process_group_id: u32) -> Self {
#[cfg(unix)]
{
Self { process_group_id }
}
#[cfg(not(unix))]
{
let _ = process_group_id;
Self
}
}
#[cfg(unix)]
fn maybe_terminate_process_group(&self) {
let process_group_id = self.process_group_id;
let should_escalate =
match codex_utils_pty::process_group::terminate_process_group(process_group_id) {
Ok(exists) => exists,
Err(error) => {
warn!("Failed to terminate MCP process group {process_group_id}: {error}");
false
}
};
if should_escalate {
std::thread::spawn(move || {
std::thread::sleep(PROCESS_GROUP_TERM_GRACE_PERIOD);
if let Err(error) =
codex_utils_pty::process_group::kill_process_group(process_group_id)
{
warn!("Failed to kill MCP process group {process_group_id}: {error}");
}
});
}
}
#[cfg(not(unix))]
fn maybe_terminate_process_group(&self) {}
}
impl Drop for ProcessGroupGuard {
fn drop(&mut self) {
if cfg!(unix) {
self.maybe_terminate_process_group();
}
}
}
pub type Elicitation = CreateElicitationRequestParam;
pub type ElicitationResponse = CreateElicitationResult;
@@ -129,6 +192,8 @@ impl RmcpClient {
.env_clear()
.envs(envs)
.args(&args);
#[cfg(unix)]
command.process_group(0);
if let Some(cwd) = cwd {
command.current_dir(cwd);
}
@@ -136,6 +201,7 @@ impl RmcpClient {
let (transport, stderr) = TokioChildProcess::builder(command)
.stderr(Stdio::piped())
.spawn()?;
let process_group_guard = transport.id().map(ProcessGroupGuard::new);
if let Some(stderr) = stderr {
tokio::spawn(async move {
@@ -157,7 +223,10 @@ impl RmcpClient {
Ok(Self {
state: Mutex::new(ClientState::Connecting {
transport: Some(PendingTransport::ChildProcess(transport)),
transport: Some(PendingTransport::ChildProcess {
transport,
process_group_guard,
}),
}),
})
}
@@ -226,17 +295,22 @@ impl RmcpClient {
) -> Result<InitializeResult> {
let client_handler = LoggingClientHandler::new(params.clone(), send_elicitation);
let (transport, oauth_persistor) = {
let (transport, oauth_persistor, process_group_guard) = {
let mut guard = self.state.lock().await;
match &mut *guard {
ClientState::Connecting { transport } => match transport.take() {
Some(PendingTransport::ChildProcess(transport)) => (
Some(PendingTransport::ChildProcess {
transport,
process_group_guard,
}) => (
service::serve_client(client_handler.clone(), transport).boxed(),
None,
process_group_guard,
),
Some(PendingTransport::StreamableHttp { transport }) => (
service::serve_client(client_handler.clone(), transport).boxed(),
None,
None,
),
Some(PendingTransport::StreamableHttpWithOAuth {
transport,
@@ -244,6 +318,7 @@ impl RmcpClient {
}) => (
service::serve_client(client_handler.clone(), transport).boxed(),
Some(oauth_persistor),
None,
),
None => return Err(anyhow!("client already initializing")),
},
@@ -270,6 +345,7 @@ impl RmcpClient {
{
let mut guard = self.state.lock().await;
*guard = ClientState::Ready {
_process_group_guard: process_group_guard,
service: Arc::new(service),
oauth: oauth_persistor.clone(),
};
@@ -448,7 +524,7 @@ impl RmcpClient {
match &*guard {
ClientState::Ready {
oauth: Some(runtime),
service: _,
..
} => Some(runtime.clone()),
_ => None,
}
@@ -0,0 +1,88 @@
#![cfg(unix)]
use std::collections::HashMap;
use std::ffi::OsString;
use std::fs;
use std::path::Path;
use std::time::Duration;
use anyhow::Context;
use anyhow::Result;
use codex_rmcp_client::RmcpClient;
fn process_exists(pid: u32) -> bool {
std::process::Command::new("kill")
.arg("-0")
.arg(pid.to_string())
.stderr(std::process::Stdio::null())
.status()
.map(|status| status.success())
.unwrap_or(false)
}
async fn wait_for_pid_file(path: &Path) -> Result<u32> {
for _ in 0..50 {
match fs::read_to_string(path) {
Ok(content) => {
let pid = content
.trim()
.parse::<u32>()
.with_context(|| format!("failed to parse pid from {}", path.display()))?;
return Ok(pid);
}
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
tokio::time::sleep(Duration::from_millis(100)).await;
}
Err(error) => {
return Err(error).with_context(|| format!("failed to read {}", path.display()));
}
}
}
anyhow::bail!("timed out waiting for child pid file at {}", path.display());
}
async fn wait_for_process_exit(pid: u32) -> Result<()> {
for _ in 0..50 {
if !process_exists(pid) {
return Ok(());
}
tokio::time::sleep(Duration::from_millis(100)).await;
}
anyhow::bail!("process {pid} still running after timeout");
}
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn drop_kills_wrapper_process_group() -> Result<()> {
let temp_dir = tempfile::tempdir()?;
let child_pid_file = temp_dir.path().join("child.pid");
let child_pid_file_str = child_pid_file.to_string_lossy().into_owned();
let client = RmcpClient::new_stdio_client(
OsString::from("/bin/sh"),
vec![
OsString::from("-c"),
OsString::from(
"sleep 300 & child_pid=$!; echo \"$child_pid\" > \"$CHILD_PID_FILE\"; cat >/dev/null",
),
],
Some(HashMap::from([(
"CHILD_PID_FILE".to_string(),
child_pid_file_str,
)])),
&[],
None,
)
.await?;
let grandchild_pid = wait_for_pid_file(&child_pid_file).await?;
assert!(
process_exists(grandchild_pid),
"expected grandchild process {grandchild_pid} to be running before dropping client"
);
drop(client);
wait_for_process_exit(grandchild_pid).await
}
+2
View File
@@ -986,6 +986,7 @@ impl App {
auth_ref.and_then(CodexAuth::get_account_id),
auth_ref.and_then(CodexAuth::get_account_email),
auth_mode,
codex_core::default_client::originator().value,
config.otel.log_user_prompt,
codex_core::terminal::user_agent(),
SessionSource::Cli,
@@ -2789,6 +2790,7 @@ mod tests {
None,
None,
None,
"test_originator".to_string(),
false,
"test".to_string(),
SessionSource::Cli,
+230 -47
View File
@@ -157,6 +157,7 @@ use crate::app_event::AppEvent;
use crate::app_event::ConnectorsSnapshot;
use crate::app_event_sender::AppEventSender;
use crate::bottom_pane::LocalImageAttachment;
use crate::bottom_pane::MentionBinding;
use crate::bottom_pane::textarea::TextArea;
use crate::bottom_pane::textarea::TextAreaState;
use crate::clipboard_paste::normalize_pasted_path;
@@ -290,7 +291,8 @@ pub(crate) struct ChatComposer {
skills: Option<Vec<SkillMetadata>>,
connectors_snapshot: Option<ConnectorsSnapshot>,
dismissed_mention_popup_token: Option<String>,
mention_paths: HashMap<String, String>,
mention_bindings: HashMap<u64, ComposerMentionBinding>,
recent_submission_mention_bindings: Vec<MentionBinding>,
/// When enabled, `Enter` submits immediately and `Tab` requests queuing behavior.
steer_enabled: bool,
collaboration_modes_enabled: bool,
@@ -309,6 +311,12 @@ struct FooterFlash {
expires_at: Instant,
}
#[derive(Clone, Debug)]
struct ComposerMentionBinding {
mention: String,
path: String,
}
/// Popup state – at most one can be visible at any time.
enum ActivePopup {
None,
@@ -382,7 +390,8 @@ impl ChatComposer {
skills: None,
connectors_snapshot: None,
dismissed_mention_popup_token: None,
mention_paths: HashMap::new(),
mention_bindings: HashMap::new(),
recent_submission_mention_bindings: Vec::new(),
steer_enabled: false,
collaboration_modes_enabled: false,
config,
@@ -414,8 +423,21 @@ impl ChatComposer {
self.connectors_snapshot = connectors_snapshot;
}
pub(crate) fn take_mention_paths(&mut self) -> HashMap<String, String> {
std::mem::take(&mut self.mention_paths)
pub(crate) fn take_mention_bindings(&mut self) -> Vec<MentionBinding> {
let elements = self.current_mention_elements();
let mut ordered = Vec::new();
for (id, mention) in elements {
if let Some(binding) = self.mention_bindings.remove(&id)
&& binding.mention == mention
{
ordered.push(MentionBinding {
mention: binding.mention,
path: binding.path,
});
}
}
self.mention_bindings.clear();
ordered
}
/// Enables or disables "Steer" behavior for submission keys.
@@ -518,7 +540,12 @@ impl ChatComposer {
};
// Persistent ↑/↓ history is text-only (backwards-compatible and avoids persisting
// attachments), but local in-session ↑/↓ history can rehydrate elements and image paths.
self.set_text_content(entry.text, entry.text_elements, entry.local_image_paths);
self.set_text_content_with_mention_bindings(
entry.text,
entry.text_elements,
entry.local_image_paths,
entry.mention_bindings,
);
true
}
@@ -732,41 +759,41 @@ impl ChatComposer {
/// This is the "fresh draft" path: it clears pending paste payloads and
/// mention link targets. Callers restoring a previously submitted draft
/// that must keep `$name -> path` resolution should use
/// [`Self::set_text_content_with_mention_paths`] instead.
/// [`Self::set_text_content_with_mention_bindings`] instead.
pub(crate) fn set_text_content(
&mut self,
text: String,
text_elements: Vec<TextElement>,
local_image_paths: Vec<PathBuf>,
) {
self.set_text_content_with_mention_paths(
self.set_text_content_with_mention_bindings(
text,
text_elements,
local_image_paths,
HashMap::new(),
Vec::new(),
);
}
/// Replace the entire composer content while restoring mention link targets.
///
/// Mention popup insertion stores both visible text (for example `$file`)
/// and hidden `mention_paths` used to resolve the canonical target during
/// and hidden mention bindings used to resolve the canonical target during
/// submission. Use this method when restoring an interrupted or blocked
/// draft; if callers restore only text and images, mentions can appear
/// intact to users while resolving to the wrong target or dropping on
/// retry.
pub(crate) fn set_text_content_with_mention_paths(
pub(crate) fn set_text_content_with_mention_bindings(
&mut self,
text: String,
text_elements: Vec<TextElement>,
local_image_paths: Vec<PathBuf>,
mention_paths: HashMap<String, String>,
mention_bindings: Vec<MentionBinding>,
) {
// Clear any existing content, placeholders, and attachments first.
self.textarea.set_text_clearing_elements("");
self.pending_pastes.clear();
self.attached_images.clear();
self.mention_paths = mention_paths;
self.mention_bindings.clear();
self.textarea.set_text_with_elements(&text, &text_elements);
@@ -782,6 +809,8 @@ impl ChatComposer {
}
}
self.bind_mentions_from_snapshot(mention_bindings);
self.textarea.set_cursor(0);
self.sync_popups();
}
@@ -808,12 +837,14 @@ impl ChatComposer {
.iter()
.map(|img| img.path.clone())
.collect();
let mention_bindings = self.snapshot_mention_bindings();
self.set_text_content(String::new(), Vec::new(), Vec::new());
self.history.reset_navigation();
self.history.record_local_submission(HistoryEntry {
text: previous.clone(),
text_elements,
local_image_paths,
mention_bindings,
});
Some(previous)
}
@@ -845,6 +876,14 @@ impl ChatComposer {
.collect()
}
pub(crate) fn mention_bindings(&self) -> Vec<MentionBinding> {
self.snapshot_mention_bindings()
}
pub(crate) fn take_recent_submission_mention_bindings(&mut self) -> Vec<MentionBinding> {
std::mem::take(&mut self.recent_submission_mention_bindings)
}
fn prune_attached_images_for_submission(&mut self, text: &str, text_elements: &[TextElement]) {
if self.attached_images.is_empty() {
return;
@@ -1476,10 +1515,7 @@ impl ChatComposer {
if close_popup {
if let Some((insert_text, path)) = selected_mention {
if let Some(path) = path.as_deref() {
self.record_mention_path(&insert_text, path);
}
self.insert_selected_mention(&insert_text);
self.insert_selected_mention(&insert_text, path.as_deref());
}
self.active_popup = ActivePopup::None;
}
@@ -1786,7 +1822,7 @@ impl ChatComposer {
self.textarea.set_cursor(new_cursor);
}
fn insert_selected_mention(&mut self, insert_text: &str) {
fn insert_selected_mention(&mut self, insert_text: &str, path: Option<&str>) {
let cursor_offset = self.textarea.cursor();
let text = self.textarea.text();
let safe_cursor = Self::clamp_to_char_boundary(text, cursor_offset);
@@ -1807,28 +1843,30 @@ impl ChatComposer {
.unwrap_or(after_cursor.len());
let end_idx = safe_cursor + end_rel_idx;
let inserted = insert_text.to_string();
// Remove the active token and insert the selected mention as an atomic element.
self.textarea.replace_range(start_idx..end_idx, "");
self.textarea.set_cursor(start_idx);
let id = self.textarea.insert_element(insert_text);
let mut new_text =
String::with_capacity(text.len() - (end_idx - start_idx) + inserted.len() + 1);
new_text.push_str(&text[..start_idx]);
new_text.push_str(&inserted);
new_text.push(' ');
new_text.push_str(&text[end_idx..]);
if let (Some(path), Some(mention)) =
(path, Self::mention_name_from_insert_text(insert_text))
{
self.mention_bindings.insert(
id,
ComposerMentionBinding {
mention,
path: path.to_string(),
},
);
}
// Mention insertion rebuilds plain text, so drop existing elements.
self.textarea.set_text_clearing_elements(&new_text);
let new_cursor = start_idx.saturating_add(inserted.len()).saturating_add(1);
self.textarea.insert_str(" ");
let new_cursor = start_idx
.saturating_add(insert_text.len())
.saturating_add(1);
self.textarea.set_cursor(new_cursor);
}
fn record_mention_path(&mut self, insert_text: &str, path: &str) {
let Some(name) = Self::mention_name_from_insert_text(insert_text) else {
return;
};
self.mention_paths.insert(name, path.to_string());
}
fn mention_name_from_insert_text(insert_text: &str) -> Option<String> {
let name = insert_text.strip_prefix('$')?;
if name.is_empty() {
@@ -1845,6 +1883,67 @@ impl ChatComposer {
}
}
fn current_mention_elements(&self) -> Vec<(u64, String)> {
self.textarea
.text_element_snapshots()
.into_iter()
.filter_map(|snapshot| {
Self::mention_name_from_insert_text(snapshot.text.as_str())
.map(|mention| (snapshot.id, mention))
})
.collect()
}
fn snapshot_mention_bindings(&self) -> Vec<MentionBinding> {
let mut ordered = Vec::new();
for (id, mention) in self.current_mention_elements() {
if let Some(binding) = self.mention_bindings.get(&id)
&& binding.mention == mention
{
ordered.push(MentionBinding {
mention: binding.mention.clone(),
path: binding.path.clone(),
});
}
}
ordered
}
fn bind_mentions_from_snapshot(&mut self, mention_bindings: Vec<MentionBinding>) {
self.mention_bindings.clear();
if mention_bindings.is_empty() {
return;
}
let text = self.textarea.text().to_string();
let mut scan_from = 0usize;
for binding in mention_bindings {
let token = format!("${}", binding.mention);
let Some(range) =
find_next_mention_token_range(text.as_str(), token.as_str(), scan_from)
else {
continue;
};
let id = if let Some(id) = self.textarea.add_element_range(range.clone()) {
Some(id)
} else {
self.textarea.element_id_for_exact_range(range.clone())
};
if let Some(id) = id {
self.mention_bindings.insert(
id,
ComposerMentionBinding {
mention: binding.mention,
path: binding.path,
},
);
scan_from = range.end;
}
}
}
/// Prepare text for submission/queuing. Returns None if submission should be suppressed.
/// On success, clears pending paste payloads because placeholders have been expanded.
///
@@ -1856,6 +1955,7 @@ impl ChatComposer {
let mut text = self.textarea.text().to_string();
let original_input = text.clone();
let original_text_elements = self.textarea.text_elements();
let original_mention_bindings = self.snapshot_mention_bindings();
let original_local_image_paths = self
.attached_images
.iter()
@@ -1864,6 +1964,7 @@ impl ChatComposer {
let original_pending_pastes = self.pending_pastes.clone();
let mut text_elements = original_text_elements.clone();
let input_starts_with_space = original_input.starts_with(' ');
self.recent_submission_mention_bindings.clear();
self.textarea.set_text_clearing_elements("");
if !self.pending_pastes.is_empty() {
@@ -1909,10 +2010,11 @@ impl ChatComposer {
self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
history_cell::new_info_event(message, None),
)));
self.set_text_content(
self.set_text_content_with_mention_bindings(
original_input.clone(),
original_text_elements,
original_local_image_paths,
original_mention_bindings,
);
self.pending_pastes.clone_from(&original_pending_pastes);
self.textarea.set_cursor(original_input.len());
@@ -1929,10 +2031,11 @@ impl ChatComposer {
self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
history_cell::new_error_event(err.user_message()),
)));
self.set_text_content(
self.set_text_content_with_mention_bindings(
original_input.clone(),
original_text_elements,
original_local_image_paths,
original_mention_bindings,
);
self.pending_pastes.clone_from(&original_pending_pastes);
self.textarea.set_cursor(original_input.len());
@@ -1950,6 +2053,7 @@ impl ChatComposer {
if text.is_empty() && self.attached_images.is_empty() {
return None;
}
self.recent_submission_mention_bindings = original_mention_bindings.clone();
if record_history && (!text.is_empty() || !self.attached_images.is_empty()) {
let local_image_paths = self
.attached_images
@@ -1960,6 +2064,7 @@ impl ChatComposer {
text: text.clone(),
text_elements: text_elements.clone(),
local_image_paths,
mention_bindings: original_mention_bindings,
});
}
self.pending_pastes.clear();
@@ -2022,6 +2127,7 @@ impl ChatComposer {
let original_input = self.textarea.text().to_string();
let original_text_elements = self.textarea.text_elements();
let original_mention_bindings = self.snapshot_mention_bindings();
let original_local_image_paths = self
.attached_images
.iter()
@@ -2053,10 +2159,11 @@ impl ChatComposer {
}
} else {
// Restore text if submission was suppressed.
self.set_text_content(
self.set_text_content_with_mention_bindings(
original_input,
original_text_elements,
original_local_image_paths,
original_mention_bindings,
);
self.pending_pastes = original_pending_pastes;
(InputResult::None, true)
@@ -2245,10 +2352,11 @@ impl ChatComposer {
_ => unreachable!(),
};
if let Some(entry) = replace_entry {
self.set_text_content(
self.set_text_content_with_mention_bindings(
entry.text,
entry.text_elements,
entry.local_image_paths,
entry.mention_bindings,
);
return (InputResult::None, true);
}
@@ -2930,6 +3038,8 @@ impl ChatComposer {
insert_text: format!("${skill_name}"),
search_terms,
path: Some(skill.path.to_string_lossy().into_owned()),
category_tag: (skill.scope == codex_core::protocol::SkillScope::Repo)
.then(|| "[Repo]".to_string()),
});
}
}
@@ -2952,23 +3062,26 @@ impl ChatComposer {
insert_text: format!("${slug}"),
search_terms,
path: Some(format!("app://{connector_id}")),
category_tag: Some("[App]".to_string()),
});
}
}
let mut counts: HashMap<String, usize> = HashMap::new();
for mention in &mentions {
*counts.entry(mention.insert_text.clone()).or_insert(0) += 1;
}
for mention in &mut mentions {
if counts.get(&mention.insert_text).copied().unwrap_or(0) <= 1 {
mention.category_tag = None;
}
}
mentions
}
fn connector_brief_description(connector: &AppInfo) -> String {
let status_label = if connector.is_accessible {
"Connected"
} else {
"Can be installed"
};
match Self::connector_description(connector) {
Some(description) => format!("{status_label} - {description}"),
None => status_label.to_string(),
}
Self::connector_description(connector).unwrap_or_default()
}
fn connector_description(connector: &AppInfo) -> Option<String> {
@@ -3049,6 +3162,42 @@ fn is_mention_name_char(byte: u8) -> bool {
matches!(byte, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'_' | b'-')
}
fn find_next_mention_token_range(text: &str, token: &str, from: usize) -> Option<Range<usize>> {
if token.is_empty() || from >= text.len() {
return None;
}
let bytes = text.as_bytes();
let token_bytes = token.as_bytes();
let mut index = from;
while index < bytes.len() {
if bytes[index] != b'$' {
index += 1;
continue;
}
let end = index.saturating_add(token_bytes.len());
if end > bytes.len() {
return None;
}
if &bytes[index..end] != token_bytes {
index += 1;
continue;
}
if bytes
.get(end)
.is_none_or(|byte| !is_mention_name_char(*byte))
{
return Some(index..end);
}
index = end;
}
None
}
impl Renderable for ChatComposer {
fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> {
if !self.input_enabled {
@@ -5513,6 +5662,40 @@ mod tests {
assert_eq!(vec![path], imgs);
}
#[test]
fn submit_captures_recent_mention_bindings_before_clearing_textarea() {
let (tx, _rx) = unbounded_channel::<AppEvent>();
let sender = AppEventSender::new(tx);
let mut composer = ChatComposer::new(
true,
sender,
false,
"Ask Codex to do anything".to_string(),
false,
);
composer.set_steer_enabled(true);
let mention_bindings = vec![MentionBinding {
mention: "figma".to_string(),
path: "/tmp/user/figma/SKILL.md".to_string(),
}];
composer.set_text_content_with_mention_bindings(
"$figma please".to_string(),
Vec::new(),
Vec::new(),
mention_bindings.clone(),
);
let (result, _) =
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
assert!(matches!(result, InputResult::Submitted { .. }));
assert_eq!(
composer.take_recent_submission_mention_bindings(),
mention_bindings
);
assert!(composer.take_mention_bindings().is_empty());
}
#[test]
fn history_navigation_restores_image_attachments() {
let (tx, _rx) = unbounded_channel::<AppEvent>();
@@ -3,6 +3,8 @@ use std::path::PathBuf;
use crate::app_event::AppEvent;
use crate::app_event_sender::AppEventSender;
use crate::bottom_pane::MentionBinding;
use crate::mention_codec::decode_history_mentions;
use codex_core::protocol::Op;
use codex_protocol::user_input::TextElement;
@@ -11,6 +13,7 @@ pub(crate) struct HistoryEntry {
pub(crate) text: String,
pub(crate) text_elements: Vec<TextElement>,
pub(crate) local_image_paths: Vec<PathBuf>,
pub(crate) mention_bindings: Vec<MentionBinding>,
}
impl HistoryEntry {
@@ -19,14 +22,24 @@ impl HistoryEntry {
text: String::new(),
text_elements: Vec::new(),
local_image_paths: Vec::new(),
mention_bindings: Vec::new(),
}
}
pub(crate) fn from_text(text: String) -> Self {
let decoded = decode_history_mentions(&text);
Self {
text,
text: decoded.text,
text_elements: Vec::new(),
local_image_paths: Vec::new(),
mention_bindings: decoded
.mentions
.into_iter()
.map(|mention| MentionBinding {
mention: mention.mention,
path: mention.path,
})
.collect(),
}
}
}
@@ -220,6 +220,7 @@ impl CommandPopup {
match_indices: indices.map(|v| v.into_iter().map(|i| i + 1).collect()),
display_shortcut: None,
description: Some(description),
category_tag: None,
wrap_indent: None,
is_disabled: false,
disabled_reason: None,
@@ -125,6 +125,7 @@ impl WidgetRef for &FileSearchPopup {
.map(|v| v.iter().map(|&i| i as usize).collect()),
display_shortcut: None,
description: None,
category_tag: None,
wrap_indent: None,
is_disabled: false,
disabled_reason: None,
@@ -258,6 +258,7 @@ impl ListSelectionView {
display_shortcut: item.display_shortcut,
match_indices: None,
description,
category_tag: None,
wrap_indent,
is_disabled,
disabled_reason: item.disabled_reason.clone(),
+26 -10
View File
@@ -13,7 +13,6 @@
//!
//! Some UI is time-based rather than input-based, such as the transient "press again to quit"
//! hint. The pane schedules redraws so those hints can expire even when the UI is otherwise idle.
use std::collections::HashMap;
use std::path::PathBuf;
use crate::app_event::ConnectorsSnapshot;
@@ -55,6 +54,14 @@ pub(crate) struct LocalImageAttachment {
pub(crate) placeholder: String,
pub(crate) path: PathBuf,
}
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) struct MentionBinding {
/// Mention token text without the leading `$`.
pub(crate) mention: String,
/// Canonical mention target (for example `app://...` or absolute SKILL.md path).
pub(crate) path: String,
}
mod chat_composer;
mod chat_composer_history;
mod command_popup;
@@ -228,14 +235,19 @@ impl BottomPane {
self.request_redraw();
}
pub fn take_mention_paths(&mut self) -> HashMap<String, String> {
self.composer.take_mention_paths()
pub fn take_mention_bindings(&mut self) -> Vec<MentionBinding> {
self.composer.take_mention_bindings()
}
/// Clear pending attachments and mention paths e.g. when a slash command doesn't submit text.
pub fn take_recent_submission_mention_bindings(&mut self) -> Vec<MentionBinding> {
self.composer.take_recent_submission_mention_bindings()
}
/// Clear pending attachments and mention bindings e.g. when a slash command doesn't submit text.
pub(crate) fn drain_pending_submission_state(&mut self) {
let _ = self.take_recent_submission_images_with_placeholders();
let _ = self.take_mention_paths();
let _ = self.take_recent_submission_mention_bindings();
let _ = self.take_mention_bindings();
}
pub fn set_steer_enabled(&mut self, enabled: bool) {
@@ -419,7 +431,7 @@ impl BottomPane {
///
/// This is intended for fresh input where mention linkage does not need to
/// survive; it routes to `ChatComposer::set_text_content`, which resets
/// `mention_paths`.
/// mention bindings.
pub(crate) fn set_composer_text(
&mut self,
text: String,
@@ -437,18 +449,18 @@ impl BottomPane {
/// Use this when rehydrating a draft after a local validation/gating
/// failure (for example unsupported image submit) so previously selected
/// mention targets remain stable across retry.
pub(crate) fn set_composer_text_with_mention_paths(
pub(crate) fn set_composer_text_with_mention_bindings(
&mut self,
text: String,
text_elements: Vec<TextElement>,
local_image_paths: Vec<PathBuf>,
mention_paths: HashMap<String, String>,
mention_bindings: Vec<MentionBinding>,
) {
self.composer.set_text_content_with_mention_paths(
self.composer.set_text_content_with_mention_bindings(
text,
text_elements,
local_image_paths,
mention_paths,
mention_bindings,
);
self.request_redraw();
}
@@ -481,6 +493,10 @@ impl BottomPane {
self.composer.local_images()
}
pub(crate) fn composer_mention_bindings(&self) -> Vec<MentionBinding> {
self.composer.mention_bindings()
}
#[cfg(test)]
pub(crate) fn composer_local_image_paths(&self) -> Vec<PathBuf> {
self.composer.local_image_paths()
@@ -30,6 +30,7 @@ pub(crate) struct GenericDisplayRow {
pub display_shortcut: Option<KeyBinding>,
pub match_indices: Option<Vec<usize>>, // indices to bold (char positions)
pub description: Option<String>, // optional grey text after the name
pub category_tag: Option<String>, // optional right-side category label
pub disabled_reason: Option<String>, // optional disabled message
pub is_disabled: bool,
pub wrap_indent: Option<usize>, // optional indent for wrapped lines
@@ -337,6 +338,10 @@ fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> {
}
full_spans.push(desc.clone().dim());
}
if let Some(tag) = row.category_tag.as_deref().filter(|tag| !tag.is_empty()) {
full_spans.push(" ".into());
full_spans.push(tag.to_string().dim());
}
Line::from(full_spans)
}
+5 -1
View File
@@ -24,8 +24,11 @@ pub(crate) struct MentionItem {
pub(crate) insert_text: String,
pub(crate) search_terms: Vec<String>,
pub(crate) path: Option<String>,
pub(crate) category_tag: Option<String>,
}
const MENTION_NAME_TRUNCATE_LEN: usize = 24;
pub(crate) struct SkillPopup {
query: String,
mentions: Vec<MentionItem>,
@@ -94,13 +97,14 @@ impl SkillPopup {
.into_iter()
.map(|(idx, indices, _score)| {
let mention = &self.mentions[idx];
let name = truncate_text(&mention.display_name, 21);
let name = truncate_text(&mention.display_name, MENTION_NAME_TRUNCATE_LEN);
let description = mention.description.clone().unwrap_or_default();
GenericDisplayRow {
name,
match_indices: indices,
display_shortcut: None,
description: Some(description).filter(|desc| !desc.is_empty()),
category_tag: mention.category_tag.clone(),
is_disabled: false,
disabled_reason: None,
wrap_indent: None,
+60 -10
View File
@@ -25,9 +25,17 @@ fn is_word_separator(ch: char) -> bool {
#[derive(Debug, Clone)]
struct TextElement {
id: u64,
range: Range<usize>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct TextElementSnapshot {
pub(crate) id: u64,
pub(crate) range: Range<usize>,
pub(crate) text: String,
}
#[derive(Debug)]
pub(crate) struct TextArea {
text: String,
@@ -35,6 +43,7 @@ pub(crate) struct TextArea {
wrap_cache: RefCell<Option<WrapCache>>,
preferred_col: Option<usize>,
elements: Vec<TextElement>,
next_element_id: u64,
kill_buffer: String,
}
@@ -58,6 +67,7 @@ impl TextArea {
wrap_cache: RefCell::new(None),
preferred_col: None,
elements: Vec::new(),
next_element_id: 1,
kill_buffer: String::new(),
}
}
@@ -87,7 +97,11 @@ impl TextArea {
if start >= end {
continue;
}
self.elements.push(TextElement { range: start..end });
let id = self.next_element_id();
self.elements.push(TextElement {
id,
range: start..end,
});
}
self.elements.sort_by_key(|e| e.range.start);
}
@@ -766,6 +780,28 @@ impl TextArea {
.collect()
}
pub(crate) fn text_element_snapshots(&self) -> Vec<TextElementSnapshot> {
self.elements
.iter()
.filter_map(|element| {
self.text
.get(element.range.clone())
.map(|text| TextElementSnapshot {
id: element.id,
range: element.range.clone(),
text: text.to_string(),
})
})
.collect()
}
pub(crate) fn element_id_for_exact_range(&self, range: Range<usize>) -> Option<u64> {
self.elements
.iter()
.find(|element| element.range == range)
.map(|element| element.id)
}
/// Renames a single text element in-place, keeping it atomic.
///
/// Use this when the element payload is an identifier (e.g. a placeholder) that must be
@@ -835,41 +871,47 @@ impl TextArea {
true
}
pub fn insert_element(&mut self, text: &str) {
pub fn insert_element(&mut self, text: &str) -> u64 {
let start = self.clamp_pos_for_insertion(self.cursor_pos);
self.insert_str_at(start, text);
let end = start + text.len();
self.add_element(start..end);
let id = self.add_element(start..end);
// Place cursor at end of inserted element
self.set_cursor(end);
id
}
/// Mark an existing text range as an atomic element without changing the text.
///
/// This is used to convert already-typed tokens (like `/plan`) into elements
/// so they render and edit atomically. Overlapping or duplicate ranges are ignored.
pub fn add_element_range(&mut self, range: Range<usize>) {
pub fn add_element_range(&mut self, range: Range<usize>) -> Option<u64> {
let start = self.clamp_pos_to_char_boundary(range.start.min(self.text.len()));
let end = self.clamp_pos_to_char_boundary(range.end.min(self.text.len()));
if start >= end {
return;
return None;
}
if self
.elements
.iter()
.any(|e| e.range.start == start && e.range.end == end)
{
return;
return None;
}
if self
.elements
.iter()
.any(|e| start < e.range.end && end > e.range.start)
{
return;
return None;
}
self.elements.push(TextElement { range: start..end });
let id = self.next_element_id();
self.elements.push(TextElement {
id,
range: start..end,
});
self.elements.sort_by_key(|e| e.range.start);
Some(id)
}
pub fn remove_element_range(&mut self, range: Range<usize>) -> bool {
@@ -884,10 +926,18 @@ impl TextArea {
len_before != self.elements.len()
}
fn add_element(&mut self, range: Range<usize>) {
let elem = TextElement { range };
fn add_element(&mut self, range: Range<usize>) -> u64 {
let id = self.next_element_id();
let elem = TextElement { id, range };
self.elements.push(elem);
self.elements.sort_by_key(|e| e.range.start);
id
}
fn next_element_id(&mut self) -> u64 {
let id = self.next_element_id;
self.next_element_id = self.next_element_id.saturating_add(1);
id
}
fn find_element_containing(&self, pos: usize) -> Option<usize> {
+185 -41
View File
@@ -20,6 +20,11 @@
//! is in progress and while MCP server startup is in progress. Those lifecycles are tracked
//! independently (`agent_turn_running` and `mcp_startup_status`) and synchronized via
//! `update_task_running_state`.
//!
//! For preamble-capable models, assistant output may include commentary before
//! the final answer. During streaming we hide the status row to avoid duplicate
//! progress indicators; once commentary completes and stream queues drain, we
//! re-show it so users still see turn-in-progress state between output bursts.
use std::collections::HashMap;
use std::collections::HashSet;
use std::collections::VecDeque;
@@ -116,6 +121,8 @@ use codex_protocol::config_types::Personality;
use codex_protocol::config_types::Settings;
#[cfg(target_os = "windows")]
use codex_protocol::config_types::WindowsSandboxLevel;
use codex_protocol::items::AgentMessageItem;
use codex_protocol::models::MessagePhase;
use codex_protocol::models::local_image_label_text;
use codex_protocol::parse_command::ParsedCommand;
use codex_protocol::request_user_input::RequestUserInputEvent;
@@ -164,6 +171,7 @@ use crate::bottom_pane::ExperimentalFeaturesView;
use crate::bottom_pane::FeedbackAudience;
use crate::bottom_pane::InputResult;
use crate::bottom_pane::LocalImageAttachment;
use crate::bottom_pane::MentionBinding;
use crate::bottom_pane::QUIT_SHORTCUT_TIMEOUT;
use crate::bottom_pane::SelectionAction;
use crate::bottom_pane::SelectionItem;
@@ -210,6 +218,8 @@ mod skills;
use self::skills::collect_tool_mentions;
use self::skills::find_app_mentions;
use self::skills::find_skill_mentions_with_tool_mentions;
use crate::mention_codec::LinkedMention;
use crate::mention_codec::encode_history_mentions;
use crate::streaming::chunking::AdaptiveChunkingPolicy;
use crate::streaming::commit_tick::CommitTickScope;
use crate::streaming::commit_tick::run_commit_tick;
@@ -539,6 +549,8 @@ pub(crate) struct ChatWidget {
current_status_header: String,
// Previous status header to restore after a transient stream retry.
retry_status_header: Option<String>,
// Set when commentary output completes; once stream queues go idle we restore the status row.
pending_status_indicator_restore: bool,
thread_id: Option<ThreadId>,
thread_name: Option<String>,
forked_from: Option<ThreadId>,
@@ -639,7 +651,7 @@ pub(crate) struct UserMessage {
text: String,
local_images: Vec<LocalImageAttachment>,
text_elements: Vec<TextElement>,
mention_paths: HashMap<String, String>,
mention_bindings: Vec<MentionBinding>,
}
impl From<String> for UserMessage {
@@ -649,7 +661,7 @@ impl From<String> for UserMessage {
local_images: Vec::new(),
// Plain text conversion has no UI element ranges.
text_elements: Vec::new(),
mention_paths: HashMap::new(),
mention_bindings: Vec::new(),
}
}
}
@@ -661,7 +673,7 @@ impl From<&str> for UserMessage {
local_images: Vec::new(),
// Plain text conversion has no UI element ranges.
text_elements: Vec::new(),
mention_paths: HashMap::new(),
mention_bindings: Vec::new(),
}
}
}
@@ -687,7 +699,7 @@ pub(crate) fn create_initial_user_message(
text,
local_images,
text_elements,
mention_paths: HashMap::new(),
mention_bindings: Vec::new(),
})
}
}
@@ -701,14 +713,14 @@ fn remap_placeholders_for_message(message: UserMessage, next_label: &mut usize)
text,
text_elements,
local_images,
mention_paths,
mention_bindings,
} = message;
if local_images.is_empty() {
return UserMessage {
text,
text_elements,
local_images,
mention_paths,
mention_bindings,
};
}
@@ -763,7 +775,7 @@ fn remap_placeholders_for_message(message: UserMessage, next_label: &mut usize)
text: rebuilt,
local_images: remapped_images,
text_elements: rebuilt_elements,
mention_paths,
mention_bindings,
}
}
@@ -805,6 +817,37 @@ impl ChatWidget {
self.adaptive_chunking.reset();
}
fn stream_controllers_idle(&self) -> bool {
self.stream_controller
.as_ref()
.map(|controller| controller.queued_lines() == 0)
.unwrap_or(true)
&& self
.plan_stream_controller
.as_ref()
.map(|controller| controller.queued_lines() == 0)
.unwrap_or(true)
}
/// Restore the status indicator only after commentary completion is pending,
/// the turn is still running, and all stream queues have drained.
///
/// This gate prevents flicker while normal output is still actively
/// streaming, but still restores a visible "working" affordance when a
/// commentary block ends before the turn itself has completed.
fn maybe_restore_status_indicator_after_stream_idle(&mut self) {
if !self.pending_status_indicator_restore
|| !self.bottom_pane.is_task_running()
|| !self.stream_controllers_idle()
{
return;
}
self.bottom_pane.ensure_status_indicator();
self.set_status_header(self.current_status_header.clone());
self.pending_status_indicator_restore = false;
}
/// Update the status indicator header and details.
///
/// Passing `None` clears any existing details.
@@ -1178,21 +1221,29 @@ impl ChatWidget {
} else {
text
};
// Plan commit ticks can hide the status row; remember whether we streamed plan output so
// completion can restore it once stream queues are idle.
let should_restore_after_stream = self.plan_stream_controller.is_some();
self.plan_delta_buffer.clear();
self.plan_item_active = false;
self.saw_plan_item_this_turn = true;
if let Some(mut controller) = self.plan_stream_controller.take()
&& let Some(cell) = controller.finalize()
{
let finalized_streamed_cell =
if let Some(mut controller) = self.plan_stream_controller.take() {
controller.finalize()
} else {
None
};
if let Some(cell) = finalized_streamed_cell {
self.add_boxed_history(cell);
// 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.
return;
} else if !plan_text.is_empty() {
self.add_to_history(history_cell::new_proposed_plan(plan_text));
}
if plan_text.is_empty() {
return;
if should_restore_after_stream {
self.pending_status_indicator_restore = true;
self.maybe_restore_status_indicator_after_stream_idle();
}
self.add_to_history(history_cell::new_proposed_plan(plan_text));
}
fn on_agent_reasoning_delta(&mut self, delta: String) {
@@ -1253,6 +1304,7 @@ impl ChatWidget {
self.quit_shortcut_key = None;
self.update_task_running_state();
self.retry_status_header = None;
self.pending_status_indicator_restore = false;
self.bottom_pane.set_interrupt_hint_visible(true);
self.set_status_header(String::from("Working"));
self.full_reasoning_buffer.clear();
@@ -1294,6 +1346,7 @@ impl ChatWidget {
self.request_status_line_branch_refresh();
}
// Mark task stopped and request redraw now that all content is in history.
self.pending_status_indicator_restore = false;
self.agent_turn_running = false;
self.update_task_running_state();
self.running_commands.clear();
@@ -1525,6 +1578,7 @@ impl ChatWidget {
self.adaptive_chunking.reset();
self.stream_controller = None;
self.plan_stream_controller = None;
self.pending_status_indicator_restore = false;
self.request_status_line_branch_refresh();
self.maybe_show_pending_rate_limit_prompt();
}
@@ -1666,7 +1720,7 @@ impl ChatWidget {
text: self.bottom_pane.composer_text(),
text_elements: self.bottom_pane.composer_text_elements(),
local_images: self.bottom_pane.composer_local_images(),
mention_paths: HashMap::new(),
mention_bindings: self.bottom_pane.composer_mention_bindings(),
};
let mut to_merge: Vec<UserMessage> = self.queued_user_messages.drain(..).collect();
@@ -1678,7 +1732,7 @@ impl ChatWidget {
text: String::new(),
text_elements: Vec::new(),
local_images: Vec::new(),
mention_paths: HashMap::new(),
mention_bindings: Vec::new(),
};
let mut combined_offset = 0usize;
let mut next_image_label = 1usize;
@@ -1700,7 +1754,7 @@ impl ChatWidget {
elem
}));
combined.local_images.extend(message.local_images);
combined.mention_paths.extend(message.mention_paths);
combined.mention_bindings.extend(message.mention_bindings);
}
Some(combined)
@@ -1711,11 +1765,15 @@ impl ChatWidget {
text,
local_images,
text_elements,
mention_paths: _,
mention_bindings,
} = user_message;
let local_image_paths = local_images.into_iter().map(|img| img.path).collect();
self.bottom_pane
.set_composer_text(text, text_elements, local_image_paths);
self.bottom_pane.set_composer_text_with_mention_bindings(
text,
text_elements,
local_image_paths,
mention_bindings,
);
}
fn on_plan_update(&mut self, update: UpdatePlanArgs) {
@@ -2080,9 +2138,24 @@ impl ChatWidget {
if self.retry_status_header.is_none() {
self.retry_status_header = Some(self.current_status_header.clone());
}
self.bottom_pane.ensure_status_indicator();
self.set_status(message, additional_details);
}
/// Handle completion of an `AgentMessage` turn item.
///
/// Commentary completion sets a deferred restore flag so the status row
/// returns once stream queues are idle. Final-answer completion (or absent
/// phase for legacy models) clears the flag to preserve historical behavior.
fn on_agent_message_item_completed(&mut self, item: AgentMessageItem) {
self.pending_status_indicator_restore = match item.phase {
// Models that don't support preambles only output AgentMessageItems on turn completion.
Some(MessagePhase::FinalAnswer) | None => false,
Some(MessagePhase::Commentary) => true,
};
self.maybe_restore_status_indicator_after_stream_idle();
}
/// Periodic tick for stream commits. In smooth mode this preserves one-line pacing, while
/// catch-up mode drains larger batches to reduce queue lag.
pub(crate) fn on_commit_tick(&mut self) {
@@ -2103,9 +2176,8 @@ impl ChatWidget {
///
/// `scope` controls whether this call may commit in smooth mode or only when catch-up
/// is currently active. While lines are actively streaming we hide the status row to avoid
/// duplicate "in progress" affordances, but once all stream controllers go idle for this
/// turn we restore the status row if the task is still running so users keep a live
/// spinner/shimmer signal between preamble output and subsequent tool activity.
/// duplicate "in progress" affordances. Restoration is gated separately so we only re-show
/// the row after commentary completion once stream queues are idle.
fn run_commit_tick_with_scope(&mut self, scope: CommitTickScope) {
let now = Instant::now();
let outcome = run_commit_tick(
@@ -2121,10 +2193,7 @@ impl ChatWidget {
}
if outcome.has_controller && outcome.all_idle {
if self.bottom_pane.is_task_running() {
self.bottom_pane.ensure_status_indicator();
self.set_status_header(self.current_status_header.clone());
}
self.maybe_restore_status_indicator_after_stream_idle();
self.app_event_tx.send(AppEvent::StopCommitAnimation);
}
@@ -2555,6 +2624,7 @@ impl ChatWidget {
full_reasoning_buffer: String::new(),
current_status_header: String::from("Working"),
retry_status_header: None,
pending_status_indicator_restore: false,
thread_id: None,
thread_name: None,
forked_from: None,
@@ -2717,6 +2787,7 @@ impl ChatWidget {
full_reasoning_buffer: String::new(),
current_status_header: String::from("Working"),
retry_status_header: None,
pending_status_indicator_restore: false,
thread_id: None,
thread_name: None,
forked_from: None,
@@ -2868,6 +2939,7 @@ impl ChatWidget {
full_reasoning_buffer: String::new(),
current_status_header: String::from("Working"),
retry_status_header: None,
pending_status_indicator_restore: false,
thread_id: None,
thread_name: None,
forked_from: None,
@@ -3022,7 +3094,9 @@ impl ChatWidget {
.bottom_pane
.take_recent_submission_images_with_placeholders(),
text_elements,
mention_paths: self.bottom_pane.take_mention_paths(),
mention_bindings: self
.bottom_pane
.take_recent_submission_mention_bindings(),
};
if self.is_session_configured() && !self.is_plan_streaming_in_tui() {
// Submitted is only emitted when steer is enabled.
@@ -3045,7 +3119,9 @@ impl ChatWidget {
.bottom_pane
.take_recent_submission_images_with_placeholders(),
text_elements,
mention_paths: self.bottom_pane.take_mention_paths(),
mention_bindings: self
.bottom_pane
.take_recent_submission_mention_bindings(),
};
self.queue_user_message(user_message);
}
@@ -3414,7 +3490,7 @@ impl ChatWidget {
.bottom_pane
.take_recent_submission_images_with_placeholders(),
text_elements: prepared_elements,
mention_paths: self.bottom_pane.take_mention_paths(),
mention_bindings: self.bottom_pane.take_recent_submission_mention_bindings(),
};
if self.is_session_configured() {
self.reasoning_buffer.clear();
@@ -3551,13 +3627,18 @@ impl ChatWidget {
text,
local_images,
text_elements,
mention_paths,
mention_bindings,
} = user_message;
if text.is_empty() && local_images.is_empty() {
return;
}
if !local_images.is_empty() && !self.current_model_supports_images() {
self.restore_blocked_image_submission(text, text_elements, local_images, mention_paths);
self.restore_blocked_image_submission(
text,
text_elements,
local_images,
mention_bindings,
);
return;
}
@@ -3594,16 +3675,43 @@ impl ChatWidget {
});
}
let mentions = collect_tool_mentions(&text, &mention_paths);
let mentions = collect_tool_mentions(&text, &HashMap::new());
let bound_names: HashSet<String> = mention_bindings
.iter()
.map(|binding| binding.mention.clone())
.collect();
let mut skill_names_lower: HashSet<String> = HashSet::new();
let mut selected_skill_paths: HashSet<PathBuf> = HashSet::new();
if let Some(skills) = self.bottom_pane.skills() {
skill_names_lower = skills
.iter()
.map(|skill| skill.name.to_ascii_lowercase())
.collect();
for binding in &mention_bindings {
let path = binding
.path
.strip_prefix("skill://")
.unwrap_or(binding.path.as_str());
let path = Path::new(path);
if let Some(skill) = skills.iter().find(|skill| skill.path.as_path() == path)
&& selected_skill_paths.insert(skill.path.clone())
{
items.push(UserInput::Skill {
name: skill.name.clone(),
path: skill.path.clone(),
});
}
}
let skill_mentions = find_skill_mentions_with_tool_mentions(&mentions, skills);
for skill in skill_mentions {
if bound_names.contains(skill.name.as_str())
|| !selected_skill_paths.insert(skill.path.clone())
{
continue;
}
items.push(UserInput::Skill {
name: skill.name.clone(),
path: skill.path.clone(),
@@ -3611,9 +3719,33 @@ impl ChatWidget {
}
}
let mut selected_app_ids: HashSet<String> = HashSet::new();
if let Some(apps) = self.connectors_for_mentions() {
for binding in &mention_bindings {
let Some(app_id) = binding
.path
.strip_prefix("app://")
.filter(|id| !id.is_empty())
else {
continue;
};
if !selected_app_ids.insert(app_id.to_string()) {
continue;
}
if let Some(app) = apps.iter().find(|app| app.id == app_id) {
items.push(UserInput::Mention {
name: app.name.clone(),
path: binding.path.clone(),
});
}
}
let app_mentions = find_app_mentions(&mentions, apps, &skill_names_lower);
for app in app_mentions {
let slug = codex_core::connectors::connector_mention_slug(&app);
if bound_names.contains(&slug) || !selected_app_ids.insert(app.id.clone()) {
continue;
}
let app_id = app.id.as_str();
items.push(UserInput::Mention {
name: app.name.clone(),
@@ -3654,8 +3786,16 @@ impl ChatWidget {
// Persist the text to cross-session message history.
if !text.is_empty() {
let encoded_mentions = mention_bindings
.iter()
.map(|binding| LinkedMention {
mention: binding.mention.clone(),
path: binding.path.clone(),
})
.collect::<Vec<_>>();
let history_text = encode_history_mentions(&text, &encoded_mentions);
self.codex_op_tx
.send(Op::AddToHistory { text: text.clone() })
.send(Op::AddToHistory { text: history_text })
.unwrap_or_else(|e| {
tracing::error!("failed to send AddHistory op: {e}");
});
@@ -3678,7 +3818,7 @@ impl ChatWidget {
///
/// The blocked-image path intentionally keeps the draft in the composer so
/// users can remove attachments and retry. We must restore
/// `mention_paths` alongside visible text; restoring only `$name` tokens
/// mention bindings alongside visible text; restoring only `$name` tokens
/// makes the draft look correct while degrading mention resolution to
/// name-only heuristics on retry.
fn restore_blocked_image_submission(
@@ -3686,15 +3826,15 @@ impl ChatWidget {
text: String,
text_elements: Vec<TextElement>,
local_images: Vec<LocalImageAttachment>,
mention_paths: HashMap<String, String>,
mention_bindings: Vec<MentionBinding>,
) {
// Preserve the user's composed payload so they can retry after changing models.
let local_image_paths = local_images.iter().map(|img| img.path.clone()).collect();
self.bottom_pane.set_composer_text_with_mention_paths(
self.bottom_pane.set_composer_text_with_mention_bindings(
text,
text_elements,
local_image_paths,
mention_paths,
mention_bindings,
);
self.add_to_history(history_cell::new_warning_event(
self.image_inputs_not_supported_message(),
@@ -3892,8 +4032,12 @@ impl ChatWidget {
| EventMsg::ReasoningRawContentDelta(_)
| EventMsg::DynamicToolCallRequest(_) => {}
EventMsg::ItemCompleted(event) => {
if let codex_protocol::items::TurnItem::Plan(plan_item) = event.item {
self.on_plan_item_completed(plan_item.text);
let item = event.item;
if let codex_protocol::items::TurnItem::Plan(plan_item) = &item {
self.on_plan_item_completed(plan_item.text.clone());
}
if let codex_protocol::items::TurnItem::AgentMessage(item) = item {
self.on_agent_message_item_completed(item);
}
}
}
@@ -6436,7 +6580,7 @@ impl ChatWidget {
text,
local_images: Vec::new(),
text_elements: Vec::new(),
mention_paths: HashMap::new(),
mention_bindings: Vec::new(),
};
if should_queue {
self.queue_user_message(user_message);
+220 -16
View File
@@ -10,6 +10,7 @@ use crate::app_event::ExitMode;
use crate::app_event_sender::AppEventSender;
use crate::bottom_pane::FeedbackAudience;
use crate::bottom_pane::LocalImageAttachment;
use crate::bottom_pane::MentionBinding;
use crate::history_cell::UserHistoryCell;
use crate::test_backend::VT100Backend;
use crate::tui::FrameRequester;
@@ -40,6 +41,7 @@ use codex_core::protocol::ExecCommandSource;
use codex_core::protocol::ExecPolicyAmendment;
use codex_core::protocol::ExitedReviewModeEvent;
use codex_core::protocol::FileChange;
use codex_core::protocol::ItemCompletedEvent;
use codex_core::protocol::McpStartupCompleteEvent;
use codex_core::protocol::McpStartupStatus;
use codex_core::protocol::McpStartupUpdateEvent;
@@ -61,6 +63,7 @@ use codex_core::protocol::UndoCompletedEvent;
use codex_core::protocol::UndoStartedEvent;
use codex_core::protocol::ViewImageToolCallEvent;
use codex_core::protocol::WarningEvent;
use codex_core::skills::model::SkillMetadata;
use codex_otel::OtelManager;
use codex_otel::RuntimeMetricsSummary;
use codex_protocol::ThreadId;
@@ -69,6 +72,10 @@ use codex_protocol::config_types::CollaborationMode;
use codex_protocol::config_types::ModeKind;
use codex_protocol::config_types::Personality;
use codex_protocol::config_types::Settings;
use codex_protocol::items::AgentMessageContent;
use codex_protocol::items::AgentMessageItem;
use codex_protocol::items::TurnItem;
use codex_protocol::models::MessagePhase;
use codex_protocol::openai_models::ModelPreset;
use codex_protocol::openai_models::ReasoningEffortPreset;
use codex_protocol::openai_models::default_input_modalities;
@@ -77,6 +84,7 @@ use codex_protocol::plan_tool::PlanItemArg;
use codex_protocol::plan_tool::StepStatus;
use codex_protocol::plan_tool::UpdatePlanArgs;
use codex_protocol::protocol::CodexErrorInfo;
use codex_protocol::protocol::SkillScope;
use codex_protocol::user_input::TextElement;
use codex_protocol::user_input::UserInput;
use codex_utils_absolute_path::AbsolutePathBuf;
@@ -391,7 +399,82 @@ async fn submission_preserves_text_elements_and_local_images() {
}
#[tokio::test]
async fn blocked_image_restore_preserves_mention_paths() {
async fn submission_prefers_selected_duplicate_skill_path() {
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await;
let conversation_id = ThreadId::new();
let rollout_file = NamedTempFile::new().unwrap();
let configured = codex_core::protocol::SessionConfiguredEvent {
session_id: conversation_id,
forked_from_id: None,
thread_name: None,
model: "test-model".to_string(),
model_provider_id: "test-provider".to_string(),
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::ReadOnly,
cwd: PathBuf::from("/home/user/project"),
reasoning_effort: Some(ReasoningEffortConfig::default()),
history_log_id: 0,
history_entry_count: 0,
initial_messages: None,
rollout_path: Some(rollout_file.path().to_path_buf()),
};
chat.handle_codex_event(Event {
id: "initial".into(),
msg: EventMsg::SessionConfigured(configured),
});
drain_insert_history(&mut rx);
let repo_skill_path = PathBuf::from("/tmp/repo/figma/SKILL.md");
let user_skill_path = PathBuf::from("/tmp/user/figma/SKILL.md");
chat.set_skills(Some(vec![
SkillMetadata {
name: "figma".to_string(),
description: "Repo skill".to_string(),
short_description: None,
interface: None,
dependencies: None,
path: repo_skill_path,
scope: SkillScope::Repo,
},
SkillMetadata {
name: "figma".to_string(),
description: "User skill".to_string(),
short_description: None,
interface: None,
dependencies: None,
path: user_skill_path.clone(),
scope: SkillScope::User,
},
]));
chat.bottom_pane.set_composer_text_with_mention_bindings(
"please use $figma now".to_string(),
Vec::new(),
Vec::new(),
vec![MentionBinding {
mention: "figma".to_string(),
path: user_skill_path.to_string_lossy().into_owned(),
}],
);
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
let items = match next_submit_op(&mut op_rx) {
Op::UserTurn { items, .. } => items,
other => panic!("expected Op::UserTurn, got {other:?}"),
};
let selected_skill_paths = items
.iter()
.filter_map(|item| match item {
UserInput::Skill { path, .. } => Some(path.clone()),
_ => None,
})
.collect::<Vec<_>>();
assert_eq!(selected_skill_paths, vec![user_skill_path]);
}
#[tokio::test]
async fn blocked_image_restore_preserves_mention_bindings() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
let placeholder = "[Image #1]";
@@ -404,23 +487,33 @@ async fn blocked_image_restore_preserves_mention_paths() {
placeholder: placeholder.to_string(),
path: PathBuf::from("/tmp/blocked.png"),
}];
let mention_paths =
HashMap::from([("file".to_string(), "/tmp/skills/file/SKILL.md".to_string())]);
let mention_bindings = vec![MentionBinding {
mention: "file".to_string(),
path: "/tmp/skills/file/SKILL.md".to_string(),
}];
chat.restore_blocked_image_submission(
text.clone(),
text_elements.clone(),
text_elements,
local_images.clone(),
mention_paths.clone(),
mention_bindings.clone(),
);
let mention_start = text.find("$file").expect("mention token exists");
let expected_elements = vec![
TextElement::new((0..placeholder.len()).into(), Some(placeholder.to_string())),
TextElement::new(
(mention_start..mention_start + "$file".len()).into(),
Some("$file".to_string()),
),
];
assert_eq!(chat.bottom_pane.composer_text(), text);
assert_eq!(chat.bottom_pane.composer_text_elements(), text_elements);
assert_eq!(chat.bottom_pane.composer_text_elements(), expected_elements);
assert_eq!(
chat.bottom_pane.composer_local_image_paths(),
vec![local_images[0].path.clone()],
);
assert_eq!(chat.bottom_pane.take_mention_paths(), mention_paths);
assert_eq!(chat.bottom_pane.take_mention_bindings(), mention_bindings);
let cells = drain_insert_history(&mut rx);
let warning = cells
@@ -468,7 +561,7 @@ async fn interrupted_turn_restores_queued_messages_with_images_and_elements() {
path: first_images[0].clone(),
}],
text_elements: first_elements,
mention_paths: HashMap::new(),
mention_bindings: Vec::new(),
});
chat.queued_user_messages.push_back(UserMessage {
text: second_text,
@@ -477,7 +570,7 @@ async fn interrupted_turn_restores_queued_messages_with_images_and_elements() {
path: second_images[0].clone(),
}],
text_elements: second_elements,
mention_paths: HashMap::new(),
mention_bindings: Vec::new(),
});
chat.refresh_queued_user_messages();
@@ -545,7 +638,7 @@ async fn interrupted_turn_restore_keeps_active_mode_for_resubmission() {
text: "Implement the plan.".to_string(),
local_images: Vec::new(),
text_elements: Vec::new(),
mention_paths: HashMap::new(),
mention_bindings: Vec::new(),
});
chat.refresh_queued_user_messages();
@@ -605,7 +698,7 @@ async fn remap_placeholders_uses_attachment_labels() {
text,
text_elements: elements,
local_images: attachments,
mention_paths: HashMap::new(),
mention_bindings: Vec::new(),
};
let mut next_label = 3usize;
let remapped = remap_placeholders_for_message(message, &mut next_label);
@@ -666,7 +759,7 @@ async fn remap_placeholders_uses_byte_ranges_when_placeholder_missing() {
text,
text_elements: elements,
local_images: attachments,
mention_paths: HashMap::new(),
mention_bindings: Vec::new(),
};
let mut next_label = 3usize;
let remapped = remap_placeholders_for_message(message, &mut next_label);
@@ -896,6 +989,7 @@ fn test_otel_manager(config: &Config, model: &str) -> OtelManager {
None,
None,
None,
"test_originator".to_string(),
false,
"test".to_string(),
SessionSource::Cli,
@@ -985,6 +1079,7 @@ async fn make_chatwidget_manual(
full_reasoning_buffer: String::new(),
current_status_header: String::from("Working"),
retry_status_header: None,
pending_status_indicator_restore: false,
thread_id: None,
thread_name: None,
forked_from: None,
@@ -1869,6 +1964,28 @@ fn terminal_interaction(chat: &mut ChatWidget, call_id: &str, process_id: &str,
});
}
fn complete_assistant_message(
chat: &mut ChatWidget,
item_id: &str,
text: &str,
phase: Option<MessagePhase>,
) {
chat.handle_codex_event(Event {
id: format!("raw-{item_id}"),
msg: EventMsg::ItemCompleted(ItemCompletedEvent {
thread_id: ThreadId::new(),
turn_id: "turn-1".to_string(),
item: TurnItem::AgentMessage(AgentMessageItem {
id: item_id.to_string(),
content: vec![AgentMessageContent::Text {
text: text.to_string(),
}],
phase,
}),
}),
});
}
fn begin_exec(chat: &mut ChatWidget, call_id: &str, raw_cmd: &str) -> ExecCommandBeginEvent {
begin_exec_with_source(chat, call_id, raw_cmd, ExecCommandSource::Agent)
}
@@ -2014,15 +2131,16 @@ async fn enqueueing_history_prompt_multiple_times_is_stable() {
#[tokio::test]
async fn streaming_final_answer_keeps_task_running_state() {
let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(None).await;
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await;
chat.thread_id = Some(ThreadId::new());
chat.on_task_started();
chat.on_agent_message_delta("Final answer line\n".to_string());
chat.on_commit_tick();
drain_insert_history(&mut rx);
assert!(chat.bottom_pane.is_task_running());
assert!(chat.bottom_pane.status_widget().is_some());
assert!(!chat.bottom_pane.status_indicator_visible());
chat.bottom_pane
.set_composer_text("queued submission".to_string(), Vec::new(), Vec::new());
@@ -2044,7 +2162,26 @@ async fn streaming_final_answer_keeps_task_running_state() {
}
#[tokio::test]
async fn preamble_keeps_status_indicator_visible_until_exec_begin() {
async fn idle_commit_ticks_do_not_restore_status_without_commentary_completion() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
chat.on_task_started();
assert_eq!(chat.bottom_pane.status_indicator_visible(), true);
chat.on_agent_message_delta("Final answer line\n".to_string());
chat.on_commit_tick();
drain_insert_history(&mut rx);
assert_eq!(chat.bottom_pane.status_indicator_visible(), false);
assert_eq!(chat.bottom_pane.is_task_running(), true);
// A second idle tick should not toggle the row back on and cause jitter.
chat.on_commit_tick();
assert_eq!(chat.bottom_pane.status_indicator_visible(), false);
}
#[tokio::test]
async fn commentary_completion_restores_status_indicator_before_exec_begin() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
chat.on_task_started();
@@ -2054,12 +2191,45 @@ async fn preamble_keeps_status_indicator_visible_until_exec_begin() {
chat.on_commit_tick();
drain_insert_history(&mut rx);
assert_eq!(chat.bottom_pane.status_indicator_visible(), false);
complete_assistant_message(
&mut chat,
"msg-commentary",
"Preamble line\n",
Some(MessagePhase::Commentary),
);
assert_eq!(chat.bottom_pane.status_indicator_visible(), true);
assert_eq!(chat.bottom_pane.is_task_running(), true);
begin_exec(&mut chat, "call-1", "echo hi");
assert_eq!(chat.bottom_pane.status_indicator_visible(), true);
}
#[tokio::test]
async fn plan_completion_restores_status_indicator_after_streaming_plan_output() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
chat.set_feature_enabled(Feature::CollaborationModes, true);
let plan_mask =
collaboration_modes::mask_for_kind(chat.models_manager.as_ref(), ModeKind::Plan)
.expect("expected plan collaboration mask");
chat.set_collaboration_mask(plan_mask);
chat.on_task_started();
assert_eq!(chat.bottom_pane.status_indicator_visible(), true);
chat.on_plan_delta("- Step 1\n".to_string());
chat.on_commit_tick();
drain_insert_history(&mut rx);
assert_eq!(chat.bottom_pane.status_indicator_visible(), false);
assert_eq!(chat.bottom_pane.is_task_running(), true);
chat.on_plan_item_completed("- Step 1\n".to_string());
assert_eq!(chat.bottom_pane.status_indicator_visible(), true);
assert_eq!(chat.bottom_pane.is_task_running(), true);
}
#[tokio::test]
@@ -2068,11 +2238,17 @@ async fn preamble_keeps_working_status_snapshot() {
chat.thread_id = Some(ThreadId::new());
// Regression sequence: a preamble line is committed to history before any exec/tool event.
// The status row must remain visible so the spinner/shimmer still communicates "working".
// After commentary completes, the status row should be restored before subsequent work.
chat.on_task_started();
chat.on_agent_message_delta("Preamble line\n".to_string());
chat.on_commit_tick();
drain_insert_history(&mut rx);
complete_assistant_message(
&mut chat,
"msg-commentary-snapshot",
"Preamble line\n",
Some(MessagePhase::Commentary),
);
let height = chat.desired_height(80);
let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(80, height))
@@ -5087,6 +5263,34 @@ async fn stream_error_updates_status_indicator() {
assert_eq!(status.details(), Some(details));
}
#[tokio::test]
async fn stream_error_restores_hidden_status_indicator() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
chat.on_task_started();
chat.on_agent_message_delta("Preamble line\n".to_string());
chat.on_commit_tick();
drain_insert_history(&mut rx);
assert!(!chat.bottom_pane.status_indicator_visible());
let msg = "Reconnecting... 2/5";
let details = "Idle timeout waiting for SSE";
chat.handle_codex_event(Event {
id: "sub-1".into(),
msg: EventMsg::StreamError(StreamErrorEvent {
message: msg.to_string(),
codex_error_info: Some(CodexErrorInfo::Other),
additional_details: Some(details.to_string()),
}),
});
let status = chat
.bottom_pane
.status_widget()
.expect("status indicator should be visible");
assert_eq!(status.header(), msg);
assert_eq!(status.details(), Some(details));
}
#[tokio::test]
async fn warning_event_adds_warning_history_cell() {
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None).await;
+1
View File
@@ -82,6 +82,7 @@ pub mod live_wrap;
mod markdown;
mod markdown_render;
mod markdown_stream;
mod mention_codec;
mod model_migration;
mod notifications;
pub mod onboarding;
+240
View File
@@ -0,0 +1,240 @@
use std::collections::HashMap;
use std::collections::VecDeque;
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) struct LinkedMention {
pub(crate) mention: String,
pub(crate) path: String,
}
#[derive(Clone, Debug, PartialEq, Eq)]
pub(crate) struct DecodedHistoryText {
pub(crate) text: String,
pub(crate) mentions: Vec<LinkedMention>,
}
pub(crate) fn encode_history_mentions(text: &str, mentions: &[LinkedMention]) -> String {
if mentions.is_empty() || text.is_empty() {
return text.to_string();
}
let mut mentions_by_name: HashMap<&str, VecDeque<&str>> = HashMap::new();
for mention in mentions {
mentions_by_name
.entry(mention.mention.as_str())
.or_default()
.push_back(mention.path.as_str());
}
let bytes = text.as_bytes();
let mut out = String::with_capacity(text.len());
let mut index = 0usize;
while index < bytes.len() {
if bytes[index] == b'$' {
let name_start = index + 1;
if let Some(first) = bytes.get(name_start)
&& is_mention_name_char(*first)
{
let mut name_end = name_start + 1;
while let Some(next) = bytes.get(name_end)
&& is_mention_name_char(*next)
{
name_end += 1;
}
let name = &text[name_start..name_end];
if let Some(path) = mentions_by_name.get_mut(name).and_then(VecDeque::pop_front) {
out.push('[');
out.push('$');
out.push_str(name);
out.push_str("](");
out.push_str(path);
out.push(')');
index = name_end;
continue;
}
}
}
let Some(ch) = text[index..].chars().next() else {
break;
};
out.push(ch);
index += ch.len_utf8();
}
out
}
pub(crate) fn decode_history_mentions(text: &str) -> DecodedHistoryText {
let bytes = text.as_bytes();
let mut out = String::with_capacity(text.len());
let mut mentions = Vec::new();
let mut index = 0usize;
while index < bytes.len() {
if bytes[index] == b'['
&& let Some((name, path, end_index)) = parse_linked_tool_mention(text, bytes, index)
&& !is_common_env_var(name)
&& is_tool_path(path)
{
out.push('$');
out.push_str(name);
mentions.push(LinkedMention {
mention: name.to_string(),
path: path.to_string(),
});
index = end_index;
continue;
}
let Some(ch) = text[index..].chars().next() else {
break;
};
out.push(ch);
index += ch.len_utf8();
}
DecodedHistoryText {
text: out,
mentions,
}
}
fn parse_linked_tool_mention<'a>(
text: &'a str,
text_bytes: &[u8],
start: usize,
) -> Option<(&'a str, &'a str, usize)> {
let dollar_index = start + 1;
if text_bytes.get(dollar_index) != Some(&b'$') {
return None;
}
let name_start = dollar_index + 1;
let first_name_byte = text_bytes.get(name_start)?;
if !is_mention_name_char(*first_name_byte) {
return None;
}
let mut name_end = name_start + 1;
while let Some(next_byte) = text_bytes.get(name_end)
&& is_mention_name_char(*next_byte)
{
name_end += 1;
}
if text_bytes.get(name_end) != Some(&b']') {
return None;
}
let mut path_start = name_end + 1;
while let Some(next_byte) = text_bytes.get(path_start)
&& next_byte.is_ascii_whitespace()
{
path_start += 1;
}
if text_bytes.get(path_start) != Some(&b'(') {
return None;
}
let mut path_end = path_start + 1;
while let Some(next_byte) = text_bytes.get(path_end)
&& *next_byte != b')'
{
path_end += 1;
}
if text_bytes.get(path_end) != Some(&b')') {
return None;
}
let path = text[path_start + 1..path_end].trim();
if path.is_empty() {
return None;
}
let name = &text[name_start..name_end];
Some((name, path, path_end + 1))
}
fn is_mention_name_char(byte: u8) -> bool {
matches!(byte, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'_' | b'-')
}
fn is_common_env_var(name: &str) -> bool {
let upper = name.to_ascii_uppercase();
matches!(
upper.as_str(),
"PATH"
| "HOME"
| "USER"
| "SHELL"
| "PWD"
| "TMPDIR"
| "TEMP"
| "TMP"
| "LANG"
| "TERM"
| "XDG_CONFIG_HOME"
)
}
fn is_tool_path(path: &str) -> bool {
path.starts_with("app://")
|| path.starts_with("mcp://")
|| path.starts_with("skill://")
|| path
.rsplit(['/', '\\'])
.next()
.is_some_and(|name| name.eq_ignore_ascii_case("SKILL.md"))
}
#[cfg(test)]
mod tests {
use super::*;
use pretty_assertions::assert_eq;
#[test]
fn decode_history_mentions_restores_visible_tokens() {
let decoded = decode_history_mentions(
"Use [$figma](app://figma-1) and [$figma](/tmp/figma/SKILL.md).",
);
assert_eq!(decoded.text, "Use $figma and $figma.");
assert_eq!(
decoded.mentions,
vec![
LinkedMention {
mention: "figma".to_string(),
path: "app://figma-1".to_string(),
},
LinkedMention {
mention: "figma".to_string(),
path: "/tmp/figma/SKILL.md".to_string(),
},
]
);
}
#[test]
fn encode_history_mentions_links_bound_mentions_in_order() {
let text = "$figma then $figma then $other";
let encoded = encode_history_mentions(
text,
&[
LinkedMention {
mention: "figma".to_string(),
path: "app://figma-app".to_string(),
},
LinkedMention {
mention: "figma".to_string(),
path: "/tmp/figma/SKILL.md".to_string(),
},
],
);
assert_eq!(
encoded,
"[$figma](app://figma-app) then [$figma](/tmp/figma/SKILL.md) then $other"
);
}
}
+27
View File
@@ -117,6 +117,33 @@ pub fn kill_process_group_by_pid(_pid: u32) -> io::Result<()> {
Ok(())
}
#[cfg(unix)]
/// Send SIGTERM to a specific process group ID (best-effort).
///
/// Returns `Ok(true)` when SIGTERM was delivered to an existing group and
/// `Ok(false)` when the group no longer exists.
pub fn terminate_process_group(process_group_id: u32) -> io::Result<bool> {
use std::io::ErrorKind;
let pgid = process_group_id as libc::pid_t;
let result = unsafe { libc::killpg(pgid, libc::SIGTERM) };
if result == -1 {
let err = io::Error::last_os_error();
if err.kind() == ErrorKind::NotFound {
return Ok(false);
}
return Err(err);
}
Ok(true)
}
#[cfg(not(unix))]
/// No-op on non-Unix platforms.
pub fn terminate_process_group(_process_group_id: u32) -> io::Result<bool> {
Ok(false)
}
#[cfg(unix)]
/// Kill a specific process group ID (best-effort).
pub fn kill_process_group(process_group_id: u32) -> io::Result<()> {