Merge 5f3612d071 into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2026-03-03 17:51:20 -08:00
committed by GitHub
53 changed files with 3414 additions and 411 deletions
+7
View File
@@ -1851,6 +1851,7 @@ dependencies = [
"notify",
"once_cell",
"openssl-sys",
"opentelemetry",
"opentelemetry_sdk",
"os_info",
"predicates",
@@ -1880,6 +1881,7 @@ dependencies = [
"toml 0.9.11+spec-1.1.0",
"toml_edit 0.24.0+spec-1.1.0",
"tracing",
"tracing-opentelemetry",
"tracing-subscriber",
"tracing-test",
"url",
@@ -1916,6 +1918,7 @@ dependencies = [
"codex-arg0",
"codex-cloud-requirements",
"codex-core",
"codex-otel",
"codex-protocol",
"codex-utils-absolute-path",
"codex-utils-cargo-bin",
@@ -1925,6 +1928,8 @@ dependencies = [
"codex-utils-sandbox-summary",
"core_test_support",
"libc",
"opentelemetry",
"opentelemetry_sdk",
"owo-colors",
"predicates",
"pretty_assertions",
@@ -1936,6 +1941,7 @@ dependencies = [
"tempfile",
"tokio",
"tracing",
"tracing-opentelemetry",
"tracing-subscriber",
"ts-rs",
"uuid",
@@ -1999,6 +2005,7 @@ dependencies = [
"sentry",
"tracing",
"tracing-subscriber",
"url",
]
[[package]]
@@ -1485,6 +1485,10 @@
}
]
},
"SkillsChangedNotification": {
"description": "Notification emitted when watched local skill files change.\n\nTreat this as an invalidation signal and re-run `skills/list` with the client's current parameters when refreshed skill metadata is needed.",
"type": "object"
},
"SubAgentSource": {
"oneOf": [
{
@@ -3202,6 +3206,26 @@
"title": "Thread/closedNotification",
"type": "object"
},
{
"properties": {
"method": {
"enum": [
"skills/changed"
],
"title": "Skills/changedNotificationMethod",
"type": "string"
},
"params": {
"$ref": "#/definitions/SkillsChangedNotification"
}
},
"required": [
"method",
"params"
],
"title": "Skills/changedNotification",
"type": "object"
},
{
"properties": {
"method": {
@@ -6039,6 +6039,26 @@
"title": "Thread/closedNotification",
"type": "object"
},
{
"properties": {
"method": {
"enum": [
"skills/changed"
],
"title": "Skills/changedNotificationMethod",
"type": "string"
},
"params": {
"$ref": "#/definitions/v2/SkillsChangedNotification"
}
},
"required": [
"method",
"params"
],
"title": "Skills/changedNotification",
"type": "object"
},
{
"properties": {
"method": {
@@ -12311,6 +12331,12 @@
],
"type": "object"
},
"SkillsChangedNotification": {
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Notification emitted when watched local skill files change.\n\nTreat this as an invalidation signal and re-run `skills/list` with the client's current parameters when refreshed skill metadata is needed.",
"title": "SkillsChangedNotification",
"type": "object"
},
"SkillsConfigWriteParams": {
"$schema": "http://json-schema.org/draft-07/schema#",
"properties": {
@@ -9977,6 +9977,26 @@
"title": "Thread/closedNotification",
"type": "object"
},
{
"properties": {
"method": {
"enum": [
"skills/changed"
],
"title": "Skills/changedNotificationMethod",
"type": "string"
},
"params": {
"$ref": "#/definitions/SkillsChangedNotification"
}
},
"required": [
"method",
"params"
],
"title": "Skills/changedNotification",
"type": "object"
},
{
"properties": {
"method": {
@@ -10972,6 +10992,12 @@
],
"type": "object"
},
"SkillsChangedNotification": {
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Notification emitted when watched local skill files change.\n\nTreat this as an invalidation signal and re-run `skills/list` with the client's current parameters when refreshed skill metadata is needed.",
"title": "SkillsChangedNotification",
"type": "object"
},
"SkillsConfigWriteParams": {
"$schema": "http://json-schema.org/draft-07/schema#",
"properties": {
@@ -0,0 +1,6 @@
{
"$schema": "http://json-schema.org/draft-07/schema#",
"description": "Notification emitted when watched local skill files change.\n\nTreat this as an invalidation signal and re-run `skills/list` with the client's current parameters when refreshed skill metadata is needed.",
"title": "SkillsChangedNotification",
"type": "object"
}
@@ -25,6 +25,7 @@ import type { ReasoningSummaryPartAddedNotification } from "./v2/ReasoningSummar
import type { ReasoningSummaryTextDeltaNotification } from "./v2/ReasoningSummaryTextDeltaNotification";
import type { ReasoningTextDeltaNotification } from "./v2/ReasoningTextDeltaNotification";
import type { ServerRequestResolvedNotification } from "./v2/ServerRequestResolvedNotification";
import type { SkillsChangedNotification } from "./v2/SkillsChangedNotification";
import type { TerminalInteractionNotification } from "./v2/TerminalInteractionNotification";
import type { ThreadArchivedNotification } from "./v2/ThreadArchivedNotification";
import type { ThreadClosedNotification } from "./v2/ThreadClosedNotification";
@@ -48,4 +49,4 @@ import type { WindowsWorldWritableWarningNotification } from "./v2/WindowsWorldW
/**
* Notification sent from the server to the client.
*/
export type ServerNotification = { "method": "error", "params": ErrorNotification } | { "method": "thread/started", "params": ThreadStartedNotification } | { "method": "thread/status/changed", "params": ThreadStatusChangedNotification } | { "method": "thread/archived", "params": ThreadArchivedNotification } | { "method": "thread/unarchived", "params": ThreadUnarchivedNotification } | { "method": "thread/closed", "params": ThreadClosedNotification } | { "method": "thread/name/updated", "params": ThreadNameUpdatedNotification } | { "method": "thread/tokenUsage/updated", "params": ThreadTokenUsageUpdatedNotification } | { "method": "turn/started", "params": TurnStartedNotification } | { "method": "turn/completed", "params": TurnCompletedNotification } | { "method": "turn/diff/updated", "params": TurnDiffUpdatedNotification } | { "method": "turn/plan/updated", "params": TurnPlanUpdatedNotification } | { "method": "item/started", "params": ItemStartedNotification } | { "method": "item/completed", "params": ItemCompletedNotification } | { "method": "rawResponseItem/completed", "params": RawResponseItemCompletedNotification } | { "method": "item/agentMessage/delta", "params": AgentMessageDeltaNotification } | { "method": "item/plan/delta", "params": PlanDeltaNotification } | { "method": "item/commandExecution/outputDelta", "params": CommandExecutionOutputDeltaNotification } | { "method": "item/commandExecution/terminalInteraction", "params": TerminalInteractionNotification } | { "method": "item/fileChange/outputDelta", "params": FileChangeOutputDeltaNotification } | { "method": "serverRequest/resolved", "params": ServerRequestResolvedNotification } | { "method": "item/mcpToolCall/progress", "params": McpToolCallProgressNotification } | { "method": "mcpServer/oauthLogin/completed", "params": McpServerOauthLoginCompletedNotification } | { "method": "account/updated", "params": AccountUpdatedNotification } | { "method": "account/rateLimits/updated", "params": AccountRateLimitsUpdatedNotification } | { "method": "app/list/updated", "params": AppListUpdatedNotification } | { "method": "item/reasoning/summaryTextDelta", "params": ReasoningSummaryTextDeltaNotification } | { "method": "item/reasoning/summaryPartAdded", "params": ReasoningSummaryPartAddedNotification } | { "method": "item/reasoning/textDelta", "params": ReasoningTextDeltaNotification } | { "method": "thread/compacted", "params": ContextCompactedNotification } | { "method": "model/rerouted", "params": ModelReroutedNotification } | { "method": "deprecationNotice", "params": DeprecationNoticeNotification } | { "method": "configWarning", "params": ConfigWarningNotification } | { "method": "fuzzyFileSearch/sessionUpdated", "params": FuzzyFileSearchSessionUpdatedNotification } | { "method": "fuzzyFileSearch/sessionCompleted", "params": FuzzyFileSearchSessionCompletedNotification } | { "method": "thread/realtime/started", "params": ThreadRealtimeStartedNotification } | { "method": "thread/realtime/itemAdded", "params": ThreadRealtimeItemAddedNotification } | { "method": "thread/realtime/outputAudio/delta", "params": ThreadRealtimeOutputAudioDeltaNotification } | { "method": "thread/realtime/error", "params": ThreadRealtimeErrorNotification } | { "method": "thread/realtime/closed", "params": ThreadRealtimeClosedNotification } | { "method": "windows/worldWritableWarning", "params": WindowsWorldWritableWarningNotification } | { "method": "windowsSandbox/setupCompleted", "params": WindowsSandboxSetupCompletedNotification } | { "method": "account/login/completed", "params": AccountLoginCompletedNotification };
export type ServerNotification = { "method": "error", "params": ErrorNotification } | { "method": "thread/started", "params": ThreadStartedNotification } | { "method": "thread/status/changed", "params": ThreadStatusChangedNotification } | { "method": "thread/archived", "params": ThreadArchivedNotification } | { "method": "thread/unarchived", "params": ThreadUnarchivedNotification } | { "method": "thread/closed", "params": ThreadClosedNotification } | { "method": "skills/changed", "params": SkillsChangedNotification } | { "method": "thread/name/updated", "params": ThreadNameUpdatedNotification } | { "method": "thread/tokenUsage/updated", "params": ThreadTokenUsageUpdatedNotification } | { "method": "turn/started", "params": TurnStartedNotification } | { "method": "turn/completed", "params": TurnCompletedNotification } | { "method": "turn/diff/updated", "params": TurnDiffUpdatedNotification } | { "method": "turn/plan/updated", "params": TurnPlanUpdatedNotification } | { "method": "item/started", "params": ItemStartedNotification } | { "method": "item/completed", "params": ItemCompletedNotification } | { "method": "rawResponseItem/completed", "params": RawResponseItemCompletedNotification } | { "method": "item/agentMessage/delta", "params": AgentMessageDeltaNotification } | { "method": "item/plan/delta", "params": PlanDeltaNotification } | { "method": "item/commandExecution/outputDelta", "params": CommandExecutionOutputDeltaNotification } | { "method": "item/commandExecution/terminalInteraction", "params": TerminalInteractionNotification } | { "method": "item/fileChange/outputDelta", "params": FileChangeOutputDeltaNotification } | { "method": "serverRequest/resolved", "params": ServerRequestResolvedNotification } | { "method": "item/mcpToolCall/progress", "params": McpToolCallProgressNotification } | { "method": "mcpServer/oauthLogin/completed", "params": McpServerOauthLoginCompletedNotification } | { "method": "account/updated", "params": AccountUpdatedNotification } | { "method": "account/rateLimits/updated", "params": AccountRateLimitsUpdatedNotification } | { "method": "app/list/updated", "params": AppListUpdatedNotification } | { "method": "item/reasoning/summaryTextDelta", "params": ReasoningSummaryTextDeltaNotification } | { "method": "item/reasoning/summaryPartAdded", "params": ReasoningSummaryPartAddedNotification } | { "method": "item/reasoning/textDelta", "params": ReasoningTextDeltaNotification } | { "method": "thread/compacted", "params": ContextCompactedNotification } | { "method": "model/rerouted", "params": ModelReroutedNotification } | { "method": "deprecationNotice", "params": DeprecationNoticeNotification } | { "method": "configWarning", "params": ConfigWarningNotification } | { "method": "fuzzyFileSearch/sessionUpdated", "params": FuzzyFileSearchSessionUpdatedNotification } | { "method": "fuzzyFileSearch/sessionCompleted", "params": FuzzyFileSearchSessionCompletedNotification } | { "method": "thread/realtime/started", "params": ThreadRealtimeStartedNotification } | { "method": "thread/realtime/itemAdded", "params": ThreadRealtimeItemAddedNotification } | { "method": "thread/realtime/outputAudio/delta", "params": ThreadRealtimeOutputAudioDeltaNotification } | { "method": "thread/realtime/error", "params": ThreadRealtimeErrorNotification } | { "method": "thread/realtime/closed", "params": ThreadRealtimeClosedNotification } | { "method": "windows/worldWritableWarning", "params": WindowsWorldWritableWarningNotification } | { "method": "windowsSandbox/setupCompleted", "params": WindowsSandboxSetupCompletedNotification } | { "method": "account/login/completed", "params": AccountLoginCompletedNotification };
@@ -0,0 +1,11 @@
// GENERATED CODE! DO NOT MODIFY BY HAND!
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
/**
* Notification emitted when watched local skill files change.
*
* Treat this as an invalidation signal and re-run `skills/list` with the
* client's current parameters when refreshed skill metadata is needed.
*/
export type SkillsChangedNotification = Record<string, never>;
@@ -150,6 +150,7 @@ export type { SkillInterface } from "./SkillInterface";
export type { SkillMetadata } from "./SkillMetadata";
export type { SkillScope } from "./SkillScope";
export type { SkillToolDependency } from "./SkillToolDependency";
export type { SkillsChangedNotification } from "./SkillsChangedNotification";
export type { SkillsConfigWriteParams } from "./SkillsConfigWriteParams";
export type { SkillsConfigWriteResponse } from "./SkillsConfigWriteResponse";
export type { SkillsListEntry } from "./SkillsListEntry";
@@ -753,6 +753,7 @@ server_notification_definitions! {
ThreadArchived => "thread/archived" (v2::ThreadArchivedNotification),
ThreadUnarchived => "thread/unarchived" (v2::ThreadUnarchivedNotification),
ThreadClosed => "thread/closed" (v2::ThreadClosedNotification),
SkillsChanged => "skills/changed" (v2::SkillsChangedNotification),
ThreadNameUpdated => "thread/name/updated" (v2::ThreadNameUpdatedNotification),
ThreadTokenUsageUpdated => "thread/tokenUsage/updated" (v2::ThreadTokenUsageUpdatedNotification),
TurnStarted => "turn/started" (v2::TurnStartedNotification),
@@ -3637,6 +3637,15 @@ pub struct ThreadClosedNotification {
pub thread_id: String,
}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
/// Notification emitted when watched local skill files change.
///
/// Treat this as an invalidation signal and re-run `skills/list` with the
/// client's current parameters when refreshed skill metadata is needed.
pub struct SkillsChangedNotification {}
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
+9
View File
@@ -148,6 +148,7 @@ Example with notification opt-out:
- `experimentalFeature/list` — list feature flags with stage metadata (`beta`, `underDevelopment`, `stable`, etc.), enabled/default-enabled state, and cursor pagination. For non-beta flags, `displayName`/`description`/`announcement` are `null`.
- `collaborationMode/list` — list available collaboration mode presets (experimental, no pagination). This response omits built-in developer instructions; clients should either pass `settings.developer_instructions: null` when setting a mode to use Codex's built-in instructions, or provide their own instructions explicitly.
- `skills/list` — list skills for one or more `cwd` values (optional `forceReload`).
- `skills/changed` — notification emitted when watched local skill files change.
- `skills/remote/list` — list public remote skills (**under development; do not call from production clients yet**).
- `skills/remote/export` — download a remote skill by `hazelnutId` into `skills` under `codex_home` (**under development; do not call from production clients yet**).
- `app/list` — list available apps.
@@ -841,6 +842,7 @@ Use `skills/list` to fetch the available skills (optionally scoped by `cwds`, wi
You can also add `perCwdExtraUserRoots` to scan additional absolute paths as `user` scope for specific `cwd` entries.
Entries whose `cwd` is not present in `cwds` are ignored.
`skills/list` might reuse a cached skills result per `cwd`; setting `forceReload` to `true` refreshes the result from disk.
The server also emits `skills/changed` notifications when watched local skill files change. Treat this as an invalidation signal and re-run `skills/list` with your current params when needed.
```json
{ "method": "skills/list", "id": 25, "params": {
@@ -876,6 +878,13 @@ Entries whose `cwd` is not present in `cwds` are ignored.
} }
```
```json
{
"method": "skills/changed",
"params": {}
}
```
To enable or disable a skill by path:
```json
@@ -61,6 +61,7 @@ use codex_app_server_protocol::ReasoningTextDeltaNotification;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::ServerNotification;
use codex_app_server_protocol::ServerRequestPayload;
use codex_app_server_protocol::SkillsChangedNotification;
use codex_app_server_protocol::TerminalInteractionNotification;
use codex_app_server_protocol::ThreadItem;
use codex_app_server_protocol::ThreadNameUpdatedNotification;
@@ -220,6 +221,15 @@ pub(crate) async fn apply_bespoke_event_handling(
.await;
handle_turn_complete(conversation_id, event_turn_id, &outgoing, &thread_state).await;
}
EventMsg::SkillsUpdateAvailable => {
if let ApiVersion::V2 = api_version {
outgoing
.send_server_notification(ServerNotification::SkillsChanged(
SkillsChangedNotification {},
))
.await;
}
}
EventMsg::Warning(_warning_event) => {}
EventMsg::ModelReroute(event) => {
if let ApiVersion::V2 = api_version {
@@ -1,18 +1,22 @@
use std::time::Duration;
use anyhow::Context;
use anyhow::Result;
use app_test_support::McpProcess;
use app_test_support::to_response;
use codex_app_server_protocol::JSONRPCResponse;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::SkillsChangedNotification;
use codex_app_server_protocol::SkillsListExtraRootsForCwd;
use codex_app_server_protocol::SkillsListParams;
use codex_app_server_protocol::SkillsListResponse;
use codex_app_server_protocol::ThreadStartParams;
use pretty_assertions::assert_eq;
use tempfile::TempDir;
use tokio::time::timeout;
const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10);
const WATCHER_TIMEOUT: Duration = Duration::from_secs(20);
fn write_skill(root: &TempDir, name: &str) -> Result<()> {
let skill_dir = root.path().join("skills").join(name);
@@ -214,3 +218,60 @@ async fn skills_list_uses_cached_result_until_force_reload() -> Result<()> {
);
Ok(())
}
#[tokio::test]
async fn skills_changed_notification_is_emitted_after_skill_change() -> Result<()> {
let codex_home = TempDir::new()?;
write_skill(&codex_home, "demo")?;
let mut mcp = McpProcess::new(codex_home.path()).await?;
timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??;
let thread_start_request_id = mcp
.send_thread_start_request(ThreadStartParams {
model: None,
model_provider: None,
service_tier: None,
cwd: None,
approval_policy: None,
sandbox: None,
config: None,
service_name: None,
base_instructions: None,
developer_instructions: None,
personality: None,
ephemeral: None,
dynamic_tools: None,
mock_experimental_field: None,
experimental_raw_events: false,
persist_extended_history: false,
})
.await?;
let _: JSONRPCResponse = timeout(
DEFAULT_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(thread_start_request_id)),
)
.await??;
let skill_path = codex_home
.path()
.join("skills")
.join("demo")
.join("SKILL.md");
std::fs::write(
&skill_path,
"---\nname: demo\ndescription: updated\n---\n\n# Updated\n",
)?;
let notification = timeout(
WATCHER_TIMEOUT,
mcp.read_stream_until_notification_message("skills/changed"),
)
.await??;
let params = notification
.params
.context("skills/changed params must be present")?;
let notification: SkillsChangedNotification = serde_json::from_value(params)?;
assert_eq!(notification, SkillsChangedNotification {});
Ok(())
}
+16 -3
View File
@@ -26,7 +26,9 @@ pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_o
pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_options_and_status;
pub use codex_core::connectors::list_cached_accessible_connectors_from_mcp_tools;
use codex_core::connectors::merge_connectors;
use codex_core::connectors::merge_plugin_apps;
pub use codex_core::connectors::with_app_enabled_state;
use codex_core::plugins::PluginsManager;
#[derive(Debug, Deserialize)]
struct DirectoryListResponse {
@@ -106,7 +108,10 @@ pub async fn list_cached_all_connectors(config: &Config) -> Option<Vec<AppInfo>>
}
let token_data = get_chatgpt_token_data()?;
let cache_key = all_connectors_cache_key(config, &token_data);
read_cached_all_connectors(&cache_key).map(filter_disallowed_connectors)
read_cached_all_connectors(&cache_key).map(|connectors| {
let connectors = merge_plugin_apps(connectors, plugin_apps_for_config(config));
filter_disallowed_connectors(connectors)
})
}
pub async fn list_all_connectors_with_options(
@@ -123,7 +128,8 @@ pub async fn list_all_connectors_with_options(
get_chatgpt_token_data().ok_or_else(|| anyhow::anyhow!("ChatGPT token not available"))?;
let cache_key = all_connectors_cache_key(config, &token_data);
if !force_refetch && let Some(cached_connectors) = read_cached_all_connectors(&cache_key) {
return Ok(filter_disallowed_connectors(cached_connectors));
let connectors = merge_plugin_apps(cached_connectors, plugin_apps_for_config(config));
return Ok(filter_disallowed_connectors(connectors));
}
let mut apps = list_directory_connectors(config).await?;
@@ -151,7 +157,8 @@ pub async fn list_all_connectors_with_options(
});
let connectors = filter_disallowed_connectors(connectors);
write_cached_all_connectors(cache_key, &connectors);
Ok(connectors)
let connectors = merge_plugin_apps(connectors, plugin_apps_for_config(config));
Ok(filter_disallowed_connectors(connectors))
}
fn all_connectors_cache_key(config: &Config, token_data: &TokenData) -> AllConnectorsCacheKey {
@@ -192,6 +199,12 @@ fn write_cached_all_connectors(cache_key: AllConnectorsCacheKey, connectors: &[A
});
}
fn plugin_apps_for_config(config: &Config) -> Vec<codex_core::plugins::AppConnectorId> {
PluginsManager::new(config.codex_home.clone())
.plugins_for_config(config)
.effective_apps()
}
pub fn merge_connectors_with_accessible(
connectors: Vec<AppInfo>,
accessible_connectors: Vec<AppInfo>,
+2
View File
@@ -161,6 +161,7 @@ core_test_support = { workspace = true }
ctor = { workspace = true }
insta = { workspace = true }
maplit = { workspace = true }
opentelemetry = { workspace = true }
predicates = { workspace = true }
pretty_assertions = { workspace = true }
test-case = "3.3.1"
@@ -170,6 +171,7 @@ opentelemetry_sdk = { workspace = true, features = [
] }
serial_test = { workspace = true }
tempfile = { workspace = true }
tracing-opentelemetry = { workspace = true }
tracing-subscriber = { workspace = true }
tracing-test = { workspace = true, features = ["no-env-filter"] }
walkdir = { workspace = true }
+333 -163
View File
@@ -70,6 +70,8 @@ use codex_hooks::HooksConfig;
use codex_network_proxy::NetworkProxy;
use codex_network_proxy::NetworkProxyAuditMetadata;
use codex_network_proxy::normalize_host;
use codex_otel::current_span_w3c_trace_context;
use codex_otel::set_parent_from_w3c_trace_context;
use codex_protocol::ThreadId;
use codex_protocol::approvals::ExecPolicyAmendment;
use codex_protocol::approvals::NetworkPolicyAmendment;
@@ -545,14 +547,21 @@ impl Codex {
/// Submit the `op` wrapped in a `Submission` with a unique ID.
pub async fn submit(&self, op: Op) -> CodexResult<String> {
let id = Uuid::now_v7().to_string();
let sub = Submission { id: id.clone(), op };
let sub = Submission {
id: id.clone(),
op,
trace: None,
};
self.submit_with_id(sub).await?;
Ok(id)
}
/// Use sparingly: prefer `submit()` so Codex is responsible for generating
/// unique IDs for each submission.
pub async fn submit_with_id(&self, sub: Submission) -> CodexResult<()> {
pub async fn submit_with_id(&self, mut sub: Submission) -> CodexResult<()> {
if sub.trace.is_none() {
sub.trace = current_span_w3c_trace_context();
}
self.tx_sub
.send(sub)
.await
@@ -3692,176 +3701,230 @@ async fn submission_loop(sess: Arc<Session>, config: Arc<Config>, rx_sub: Receiv
// To break out of this loop, send Op::Shutdown.
while let Ok(sub) = rx_sub.recv().await {
debug!(?sub, "Submission");
match sub.op.clone() {
Op::Interrupt => {
handlers::interrupt(&sess).await;
}
Op::CleanBackgroundTerminals => {
handlers::clean_background_terminals(&sess).await;
}
Op::RealtimeConversationStart(params) => {
if let Err(err) =
handle_realtime_conversation_start(&sess, sub.id.clone(), params).await
{
sess.send_event_raw(Event {
id: sub.id.clone(),
msg: EventMsg::Error(ErrorEvent {
message: err.to_string(),
codex_error_info: Some(CodexErrorInfo::Other),
}),
})
.await;
let dispatch_span = submission_dispatch_span(&sub);
let should_exit = async {
match sub.op.clone() {
Op::Interrupt => {
handlers::interrupt(&sess).await;
false
}
}
Op::RealtimeConversationAudio(params) => {
handle_realtime_conversation_audio(&sess, sub.id.clone(), params).await;
}
Op::RealtimeConversationText(params) => {
handle_realtime_conversation_text(&sess, sub.id.clone(), params).await;
}
Op::RealtimeConversationClose => {
handle_realtime_conversation_close(&sess, sub.id.clone()).await;
}
Op::OverrideTurnContext {
cwd,
approval_policy,
sandbox_policy,
windows_sandbox_level,
model,
effort,
summary,
service_tier,
collaboration_mode,
personality,
} => {
let collaboration_mode = if let Some(collab_mode) = collaboration_mode {
collab_mode
} else {
let state = sess.state.lock().await;
state.session_configuration.collaboration_mode.with_updates(
model.clone(),
effort,
None,
Op::CleanBackgroundTerminals => {
handlers::clean_background_terminals(&sess).await;
false
}
Op::RealtimeConversationStart(params) => {
if let Err(err) =
handle_realtime_conversation_start(&sess, sub.id.clone(), params).await
{
sess.send_event_raw(Event {
id: sub.id.clone(),
msg: EventMsg::Error(ErrorEvent {
message: err.to_string(),
codex_error_info: Some(CodexErrorInfo::Other),
}),
})
.await;
}
false
}
Op::RealtimeConversationAudio(params) => {
handle_realtime_conversation_audio(&sess, sub.id.clone(), params).await;
false
}
Op::RealtimeConversationText(params) => {
handle_realtime_conversation_text(&sess, sub.id.clone(), params).await;
false
}
Op::RealtimeConversationClose => {
handle_realtime_conversation_close(&sess, sub.id.clone()).await;
false
}
Op::OverrideTurnContext {
cwd,
approval_policy,
sandbox_policy,
windows_sandbox_level,
model,
effort,
summary,
service_tier,
collaboration_mode,
personality,
} => {
let collaboration_mode = if let Some(collab_mode) = collaboration_mode {
collab_mode
} else {
let state = sess.state.lock().await;
state.session_configuration.collaboration_mode.with_updates(
model.clone(),
effort,
None,
)
};
handlers::override_turn_context(
&sess,
sub.id.clone(),
SessionSettingsUpdate {
cwd,
approval_policy,
sandbox_policy,
windows_sandbox_level,
collaboration_mode: Some(collaboration_mode),
reasoning_summary: summary,
service_tier,
personality,
..Default::default()
},
)
};
handlers::override_turn_context(
&sess,
sub.id.clone(),
SessionSettingsUpdate {
cwd,
approval_policy,
sandbox_policy,
windows_sandbox_level,
collaboration_mode: Some(collaboration_mode),
reasoning_summary: summary,
service_tier,
personality,
..Default::default()
},
)
.await;
}
Op::UserInput { .. } | Op::UserTurn { .. } => {
handlers::user_input_or_turn(&sess, sub.id.clone(), sub.op).await;
}
Op::ExecApproval {
id: approval_id,
turn_id,
decision,
} => {
handlers::exec_approval(&sess, approval_id, turn_id, decision).await;
}
Op::PatchApproval { id, decision } => {
handlers::patch_approval(&sess, id, decision).await;
}
Op::UserInputAnswer { id, response } => {
handlers::request_user_input_response(&sess, id, response).await;
}
Op::DynamicToolResponse { id, response } => {
handlers::dynamic_tool_response(&sess, id, response).await;
}
Op::AddToHistory { text } => {
handlers::add_to_history(&sess, &config, text).await;
}
Op::GetHistoryEntryRequest { offset, log_id } => {
handlers::get_history_entry_request(&sess, &config, sub.id.clone(), offset, log_id)
.await;
}
Op::ListMcpTools => {
handlers::list_mcp_tools(&sess, &config, sub.id.clone()).await;
}
Op::RefreshMcpServers { config } => {
handlers::refresh_mcp_servers(&sess, config).await;
}
Op::ReloadUserConfig => {
handlers::reload_user_config(&sess).await;
}
Op::ListCustomPrompts => {
handlers::list_custom_prompts(&sess, sub.id.clone()).await;
}
Op::ListSkills { cwds, force_reload } => {
handlers::list_skills(&sess, sub.id.clone(), cwds, force_reload).await;
}
Op::ListRemoteSkills {
hazelnut_scope,
product_surface,
enabled,
} => {
handlers::list_remote_skills(
&sess,
&config,
sub.id.clone(),
false
}
Op::UserInput { .. } | Op::UserTurn { .. } => {
handlers::user_input_or_turn(&sess, sub.id.clone(), sub.op).await;
false
}
Op::ExecApproval {
id: approval_id,
turn_id,
decision,
} => {
handlers::exec_approval(&sess, approval_id, turn_id, decision).await;
false
}
Op::PatchApproval { id, decision } => {
handlers::patch_approval(&sess, id, decision).await;
false
}
Op::UserInputAnswer { id, response } => {
handlers::request_user_input_response(&sess, id, response).await;
false
}
Op::DynamicToolResponse { id, response } => {
handlers::dynamic_tool_response(&sess, id, response).await;
false
}
Op::AddToHistory { text } => {
handlers::add_to_history(&sess, &config, text).await;
false
}
Op::GetHistoryEntryRequest { offset, log_id } => {
handlers::get_history_entry_request(
&sess,
&config,
sub.id.clone(),
offset,
log_id,
)
.await;
false
}
Op::ListMcpTools => {
handlers::list_mcp_tools(&sess, &config, sub.id.clone()).await;
false
}
Op::RefreshMcpServers { config } => {
handlers::refresh_mcp_servers(&sess, config).await;
false
}
Op::ReloadUserConfig => {
handlers::reload_user_config(&sess).await;
false
}
Op::ListCustomPrompts => {
handlers::list_custom_prompts(&sess, sub.id.clone()).await;
false
}
Op::ListSkills { cwds, force_reload } => {
handlers::list_skills(&sess, sub.id.clone(), cwds, force_reload).await;
false
}
Op::ListRemoteSkills {
hazelnut_scope,
product_surface,
enabled,
)
.await;
}
Op::DownloadRemoteSkill { hazelnut_id } => {
handlers::export_remote_skill(&sess, &config, sub.id.clone(), hazelnut_id).await;
}
Op::Undo => {
handlers::undo(&sess, sub.id.clone()).await;
}
Op::Compact => {
handlers::compact(&sess, sub.id.clone()).await;
}
Op::DropMemories => {
handlers::drop_memories(&sess, &config, sub.id.clone()).await;
}
Op::UpdateMemories => {
handlers::update_memories(&sess, &config, sub.id.clone()).await;
}
Op::ThreadRollback { num_turns } => {
handlers::thread_rollback(&sess, sub.id.clone(), num_turns).await;
}
Op::SetThreadName { name } => {
handlers::set_thread_name(&sess, sub.id.clone(), name).await;
}
Op::RunUserShellCommand { command } => {
handlers::run_user_shell_command(&sess, sub.id.clone(), command).await;
}
Op::ResolveElicitation {
server_name,
request_id,
decision,
} => {
handlers::resolve_elicitation(&sess, server_name, request_id, decision).await;
}
Op::Shutdown => {
if handlers::shutdown(&sess, sub.id.clone()).await {
break;
} => {
handlers::list_remote_skills(
&sess,
&config,
sub.id.clone(),
hazelnut_scope,
product_surface,
enabled,
)
.await;
false
}
Op::DownloadRemoteSkill { hazelnut_id } => {
handlers::export_remote_skill(&sess, &config, sub.id.clone(), hazelnut_id)
.await;
false
}
Op::Undo => {
handlers::undo(&sess, sub.id.clone()).await;
false
}
Op::Compact => {
handlers::compact(&sess, sub.id.clone()).await;
false
}
Op::DropMemories => {
handlers::drop_memories(&sess, &config, sub.id.clone()).await;
false
}
Op::UpdateMemories => {
handlers::update_memories(&sess, &config, sub.id.clone()).await;
false
}
Op::ThreadRollback { num_turns } => {
handlers::thread_rollback(&sess, sub.id.clone(), num_turns).await;
false
}
Op::SetThreadName { name } => {
handlers::set_thread_name(&sess, sub.id.clone(), name).await;
false
}
Op::RunUserShellCommand { command } => {
handlers::run_user_shell_command(&sess, sub.id.clone(), command).await;
false
}
Op::ResolveElicitation {
server_name,
request_id,
decision,
} => {
handlers::resolve_elicitation(&sess, server_name, request_id, decision).await;
false
}
Op::Shutdown => handlers::shutdown(&sess, sub.id.clone()).await,
Op::Review { review_request } => {
handlers::review(&sess, &config, sub.id.clone(), review_request).await;
false
}
_ => false, // Ignore unknown ops; enum is non_exhaustive to allow extensions.
}
Op::Review { review_request } => {
handlers::review(&sess, &config, sub.id.clone(), review_request).await;
}
_ => {} // Ignore unknown ops; enum is non_exhaustive to allow extensions.
}
.instrument(dispatch_span)
.await;
if should_exit {
break;
}
}
debug!("Agent loop exited");
}
fn submission_dispatch_span(sub: &Submission) -> tracing::Span {
let dispatch_span = info_span!("submission_dispatch", submission.id = sub.id.as_str());
if let Some(trace) = sub.trace.as_ref()
&& !set_parent_from_w3c_trace_context(&dispatch_span, trace)
{
warn!(
submission.id = sub.id.as_str(),
"ignoring invalid submission trace carrier"
);
}
dispatch_span
}
/// Operation handlers
mod handlers {
use crate::codex::Session;
@@ -4896,10 +4959,15 @@ pub(crate) async fn run_turn(
Ok(mcp_tools) => mcp_tools,
Err(_) => return None,
};
connectors::with_app_enabled_state(
let plugin_apps = sess
.services
.plugins_manager
.plugins_for_config(&turn_context.config);
let connectors = connectors::merge_plugin_apps_with_accessible(
plugin_apps.effective_apps(),
connectors::accessible_connectors_from_mcp_tools(&mcp_tools),
&turn_context.config,
)
);
connectors::with_app_enabled_state(connectors, &turn_context.config)
} else {
Vec::new()
};
@@ -5619,8 +5687,16 @@ async fn built_tools(
effective_explicitly_enabled_connectors.extend(sess.get_connector_selection().await);
let connectors = if turn_context.features.enabled(Feature::Apps) {
Some(connectors::with_app_enabled_state(
let plugin_apps = sess
.services
.plugins_manager
.plugins_for_config(&turn_context.config);
let connectors = connectors::merge_plugin_apps_with_accessible(
plugin_apps.effective_apps(),
connectors::accessible_connectors_from_mcp_tools(&mcp_tools),
);
Some(connectors::with_app_enabled_state(
connectors,
&turn_context.config,
))
} else {
@@ -6619,9 +6695,17 @@ mod tests {
use codex_protocol::models::ResponseInputItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::openai_models::ModelsResponse;
use codex_protocol::protocol::Submission;
use codex_protocol::protocol::W3cTraceContext;
use opentelemetry::trace::TraceContextExt;
use opentelemetry::trace::TraceId;
use opentelemetry::trace::TracerProvider as _;
use opentelemetry_sdk::trace::SdkTracerProvider;
use std::path::Path;
use std::time::Duration;
use tokio::time::sleep;
use tracing_opentelemetry::OpenTelemetrySpanExt;
use tracing_subscriber::prelude::*;
use codex_protocol::mcp::CallToolResult as McpCallToolResult;
use pretty_assertions::assert_eq;
@@ -8097,6 +8181,12 @@ mod tests {
})
}
fn test_tracing_subscriber() -> impl tracing::Subscriber + Send + Sync {
let provider = SdkTracerProvider::builder().build();
let tracer = provider.tracer("codex-core-tests");
tracing_subscriber::registry().with(tracing_opentelemetry::layer().with_tracer(tracer))
}
async fn build_test_config(codex_home: &Path) -> Config {
ConfigBuilder::default()
.codex_home(codex_home.to_path_buf())
@@ -8427,6 +8517,86 @@ mod tests {
(session, turn_context)
}
#[tokio::test]
async fn submit_with_id_captures_current_span_trace_context() {
let (session, _turn_context) = make_session_and_context().await;
let (tx_sub, rx_sub) = async_channel::bounded(1);
let (_tx_event, rx_event) = async_channel::unbounded();
let (_agent_status_tx, agent_status) = watch::channel(AgentStatus::PendingInit);
let codex = Codex {
tx_sub,
rx_event,
agent_status,
session: Arc::new(session),
};
let subscriber = test_tracing_subscriber();
let _guard = tracing::subscriber::set_default(subscriber);
let request_parent = W3cTraceContext {
traceparent: Some("00-00000000000000000000000000000011-0000000000000022-01".into()),
tracestate: Some("vendor=value".into()),
};
let request_span = info_span!("app_server.request");
assert!(set_parent_from_w3c_trace_context(
&request_span,
&request_parent
));
let expected_trace = async {
let expected_trace =
current_span_w3c_trace_context().expect("current span should have trace context");
codex
.submit_with_id(Submission {
id: "sub-1".into(),
op: Op::Interrupt,
trace: None,
})
.await
.expect("submit should succeed");
expected_trace
}
.instrument(request_span)
.await;
let submitted = rx_sub.recv().await.expect("submission");
assert_eq!(submitted.trace, Some(expected_trace));
}
#[test]
fn submission_dispatch_span_prefers_submission_trace_context() {
let subscriber = test_tracing_subscriber();
let _guard = tracing::subscriber::set_default(subscriber);
let ambient_parent = W3cTraceContext {
traceparent: Some("00-00000000000000000000000000000033-0000000000000044-01".into()),
tracestate: None,
};
let ambient_span = info_span!("ambient");
assert!(set_parent_from_w3c_trace_context(
&ambient_span,
&ambient_parent
));
let submission_trace = W3cTraceContext {
traceparent: Some("00-00000000000000000000000000000055-0000000000000066-01".into()),
tracestate: Some("vendor=value".into()),
};
let dispatch_span = ambient_span.in_scope(|| {
submission_dispatch_span(&Submission {
id: "sub-1".into(),
op: Op::Interrupt,
trace: Some(submission_trace),
})
});
let trace_id = dispatch_span.context().span().span_context().trace_id();
assert_eq!(
trace_id,
TraceId::from_hex("00000000000000000000000000000055").expect("trace id")
);
}
pub(crate) async fn make_session_and_context_with_dynamic_tools_and_rx(
dynamic_tools: Vec<DynamicToolSpec>,
) -> (
+47 -3
View File
@@ -154,6 +154,7 @@ pub(crate) async fn run_codex_thread_one_shot(
.send(Submission {
id: "shutdown".to_string(),
op: Op::Shutdown {},
trace: None,
})
.await;
child_cancel.cancel();
@@ -298,11 +299,11 @@ async fn forward_ops(
cancel_token_ops: CancellationToken,
) {
loop {
let op: Op = match rx_ops.recv().or_cancel(&cancel_token_ops).await {
Ok(Ok(Submission { id: _, op })) => op,
let submission = match rx_ops.recv().or_cancel(&cancel_token_ops).await {
Ok(Ok(submission)) => submission,
Ok(Err(_)) | Err(_) => break,
};
let _ = codex.submit(op).await;
let _ = codex.submit_with_id(submission).await;
}
}
@@ -550,4 +551,47 @@ mod tests {
"expected Shutdown op after cancellation"
);
}
#[tokio::test]
async fn forward_ops_preserves_submission_trace_context() {
let (tx_sub, rx_sub) = bounded(SUBMISSION_CHANNEL_CAPACITY);
let (_tx_events, rx_events) = bounded(SUBMISSION_CHANNEL_CAPACITY);
let (_agent_status_tx, agent_status) = watch::channel(AgentStatus::PendingInit);
let (session, _ctx, _rx_evt) = crate::codex::make_session_and_context_with_rx().await;
let codex = Arc::new(Codex {
tx_sub,
rx_event: rx_events,
agent_status,
session,
});
let (tx_ops, rx_ops) = bounded(1);
let cancel = CancellationToken::new();
let forward = tokio::spawn(forward_ops(Arc::clone(&codex), rx_ops, cancel));
let submission = Submission {
id: "sub-1".to_string(),
op: Op::Interrupt,
trace: Some(codex_protocol::protocol::W3cTraceContext {
traceparent: Some(
"00-1234567890abcdef1234567890abcdef-1234567890abcdef-01".to_string(),
),
tracestate: Some("vendor=state".to_string()),
}),
};
tx_ops.send(submission.clone()).await.unwrap();
drop(tx_ops);
let forwarded = timeout(Duration::from_secs(1), rx_sub.recv())
.await
.expect("forward_ops hung")
.expect("forwarded submission missing");
assert_eq!(submission.id, forwarded.id);
assert_eq!(submission.op, forwarded.op);
assert_eq!(submission.trace, forwarded.trace);
timeout(Duration::from_secs(1), forward)
.await
.expect("forward_ops did not exit")
.expect("forward_ops join error");
}
}
+106
View File
@@ -1,4 +1,5 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::env;
use std::path::PathBuf;
use std::sync::LazyLock;
@@ -29,6 +30,7 @@ use crate::mcp::auth::compute_auth_statuses;
use crate::mcp::with_codex_apps_mcp;
use crate::mcp_connection_manager::McpConnectionManager;
use crate::mcp_connection_manager::codex_apps_tools_cache_key;
use crate::plugins::AppConnectorId;
use crate::token_data::TokenData;
pub const CONNECTORS_CACHE_TTL: Duration = Duration::from_secs(3600);
@@ -353,6 +355,48 @@ pub fn merge_connectors(
merged
}
pub fn merge_plugin_apps(
connectors: Vec<AppInfo>,
plugin_apps: Vec<AppConnectorId>,
) -> Vec<AppInfo> {
let mut merged = connectors;
let mut connector_ids = merged
.iter()
.map(|connector| connector.id.clone())
.collect::<HashSet<_>>();
for connector_id in plugin_apps {
if connector_ids.insert(connector_id.0.clone()) {
merged.push(plugin_app_to_app_info(connector_id));
}
}
merged.sort_by(|left, right| {
right
.is_accessible
.cmp(&left.is_accessible)
.then_with(|| left.name.cmp(&right.name))
.then_with(|| left.id.cmp(&right.id))
});
merged
}
pub fn merge_plugin_apps_with_accessible(
plugin_apps: Vec<AppConnectorId>,
accessible_connectors: Vec<AppInfo>,
) -> Vec<AppInfo> {
let accessible_connector_ids: HashSet<&str> = accessible_connectors
.iter()
.map(|connector| connector.id.as_str())
.collect();
let plugin_connectors = plugin_apps
.into_iter()
.filter(|connector_id| accessible_connector_ids.contains(connector_id.0.as_str()))
.map(plugin_app_to_app_info)
.collect::<Vec<_>>();
merge_connectors(plugin_connectors, accessible_connectors)
}
pub fn with_app_enabled_state(mut connectors: Vec<AppInfo>, config: &Config) -> Vec<AppInfo> {
let apps_config = read_apps_config(config);
if let Some(apps_config) = apps_config.as_ref() {
@@ -575,6 +619,28 @@ where
accessible
}
fn plugin_app_to_app_info(connector_id: AppConnectorId) -> AppInfo {
// Leave the placeholder name as the connector id so merge_connectors() can
// replace it with canonical app metadata from directory fetches or
// connector_name values from codex_apps tool discovery.
let connector_id = connector_id.0;
let name = connector_id.clone();
AppInfo {
id: connector_id.clone(),
name: name.clone(),
description: None,
logo_url: None,
logo_url_dark: None,
distribution_channel: None,
branding: None,
app_metadata: None,
labels: None,
install_url: Some(connector_install_url(&name, &connector_id)),
is_accessible: false,
is_enabled: true,
}
}
fn normalize_connector_value(value: Option<&str>) -> Option<String> {
value
.map(str::trim)
@@ -647,6 +713,46 @@ mod tests {
}
}
#[test]
fn merge_connectors_replaces_plugin_placeholder_name_with_accessible_name() {
let plugin = plugin_app_to_app_info(AppConnectorId("calendar".to_string()));
let accessible = AppInfo {
id: "calendar".to_string(),
name: "Google Calendar".to_string(),
description: Some("Plan events".to_string()),
logo_url: Some("https://example.com/logo.png".to_string()),
logo_url_dark: Some("https://example.com/logo-dark.png".to_string()),
distribution_channel: Some("workspace".to_string()),
branding: None,
app_metadata: None,
labels: None,
install_url: None,
is_accessible: true,
is_enabled: true,
};
let merged = merge_connectors(vec![plugin], vec![accessible]);
assert_eq!(
merged,
vec![AppInfo {
id: "calendar".to_string(),
name: "Google Calendar".to_string(),
description: Some("Plan events".to_string()),
logo_url: Some("https://example.com/logo.png".to_string()),
logo_url_dark: Some("https://example.com/logo-dark.png".to_string()),
distribution_channel: Some("workspace".to_string()),
branding: None,
app_metadata: None,
labels: None,
install_url: Some(connector_install_url("calendar", "calendar")),
is_accessible: true,
is_enabled: true,
}]
);
assert_eq!(connector_mention_slug(&merged[0]), "google-calendar");
}
#[test]
fn app_tool_policy_uses_global_defaults_for_destructive_hints() {
let apps_config = AppsConfigToml {
+125 -58
View File
@@ -126,7 +126,7 @@ impl ExternalAgentConfigService {
items.push(ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::Config,
description: format!(
"Migrate {} into {}.",
"Migrate {} into {}",
source_settings.display(),
target_config.display()
),
@@ -153,7 +153,7 @@ impl ExternalAgentConfigService {
items.push(ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::Skills,
description: format!(
"Copy skill folders from {} to {}.",
"Copy skill folders from {} to {}",
source_skills.display(),
target_skills.display()
),
@@ -161,19 +161,23 @@ impl ExternalAgentConfigService {
});
}
let source_agents_md = repo_root.map_or_else(
|| self.claude_home.join("CLAUDE.md"),
|repo_root| repo_root.join("CLAUDE.md"),
);
let source_agents_md = if let Some(repo_root) = repo_root {
find_repo_agents_md_source(repo_root)?
} else {
let path = self.claude_home.join("CLAUDE.md");
is_non_empty_text_file(&path)?.then_some(path)
};
let target_agents_md = repo_root.map_or_else(
|| self.codex_home.join("AGENTS.md"),
|repo_root| repo_root.join("AGENTS.md"),
);
if source_agents_md.is_file() && is_missing_or_empty_text_file(&target_agents_md)? {
if let Some(source_agents_md) = source_agents_md
&& is_missing_or_empty_text_file(&target_agents_md)?
{
items.push(ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::AgentsMd,
description: format!(
"Import {} to {}.",
"Import {} to {}",
source_agents_md.display(),
target_agents_md.display()
),
@@ -283,7 +287,10 @@ impl ExternalAgentConfigService {
fn import_agents_md(&self, cwd: Option<&Path>) -> io::Result<()> {
let (source_agents_md, target_agents_md) = if let Some(repo_root) = find_repo_root(cwd)? {
(repo_root.join("CLAUDE.md"), repo_root.join("AGENTS.md"))
let Some(source_agents_md) = find_repo_agents_md_source(&repo_root)? else {
return Ok(());
};
(source_agents_md, repo_root.join("AGENTS.md"))
} else if cwd.is_some_and(|cwd| !cwd.as_os_str().is_empty()) {
return Ok(());
} else {
@@ -292,7 +299,9 @@ impl ExternalAgentConfigService {
self.codex_home.join("AGENTS.md"),
)
};
if !source_agents_md.is_file() || !is_missing_or_empty_text_file(&target_agents_md)? {
if !is_non_empty_text_file(&source_agents_md)?
|| !is_missing_or_empty_text_file(&target_agents_md)?
{
return Ok(());
}
@@ -376,6 +385,27 @@ fn is_missing_or_empty_text_file(path: &Path) -> io::Result<bool> {
Ok(fs::read_to_string(path)?.trim().is_empty())
}
fn is_non_empty_text_file(path: &Path) -> io::Result<bool> {
if !path.is_file() {
return Ok(false);
}
Ok(!fs::read_to_string(path)?.trim().is_empty())
}
fn find_repo_agents_md_source(repo_root: &Path) -> io::Result<Option<PathBuf>> {
for candidate in [
repo_root.join("CLAUDE.md"),
repo_root.join(".claude").join("CLAUDE.md"),
] {
if is_non_empty_text_file(&candidate)? {
return Ok(Some(candidate));
}
}
Ok(None)
}
fn copy_dir_recursive(source: &Path, target: &Path) -> io::Result<()> {
fs::create_dir_all(target)?;
@@ -487,7 +517,7 @@ fn build_config_from_external(settings: &JsonValue) -> io::Result<TomlValue> {
shell_policy.insert("inherit".to_string(), TomlValue::String("core".to_string()));
shell_policy.insert(
"set".to_string(),
TomlValue::Table(json_object_to_toml_table(env)?),
TomlValue::Table(json_object_to_env_toml_table(env)),
);
root.insert(
"shell_environment_policy".to_string(),
@@ -511,36 +541,25 @@ fn build_config_from_external(settings: &JsonValue) -> io::Result<TomlValue> {
Ok(TomlValue::Table(root))
}
fn json_object_to_toml_table(
fn json_object_to_env_toml_table(
object: &serde_json::Map<String, JsonValue>,
) -> io::Result<toml::map::Map<String, TomlValue>> {
) -> toml::map::Map<String, TomlValue> {
let mut table = toml::map::Map::new();
for (key, value) in object {
table.insert(key.clone(), json_to_toml_value(value)?);
if let Some(value) = json_env_value_to_string(value) {
table.insert(key.clone(), TomlValue::String(value));
}
}
Ok(table)
table
}
fn json_to_toml_value(value: &JsonValue) -> io::Result<TomlValue> {
fn json_env_value_to_string(value: &JsonValue) -> Option<String> {
match value {
JsonValue::Null => Ok(TomlValue::String("null".to_string())),
JsonValue::Bool(v) => Ok(TomlValue::Boolean(*v)),
JsonValue::Number(n) => {
if let Some(i) = n.as_i64() {
return Ok(TomlValue::Integer(i));
}
if let Some(f) = n.as_f64() {
return Ok(TomlValue::Float(f));
}
Err(invalid_data_error("unsupported JSON number"))
}
JsonValue::String(v) => Ok(TomlValue::String(v.clone())),
JsonValue::Array(values) => values
.iter()
.map(json_to_toml_value)
.collect::<io::Result<Vec<_>>>()
.map(TomlValue::Array),
JsonValue::Object(map) => json_object_to_toml_table(map).map(TomlValue::Table),
JsonValue::String(value) => Some(value.clone()),
JsonValue::Null => None,
JsonValue::Bool(value) => Some(value.to_string()),
JsonValue::Number(value) => Some(value.to_string()),
JsonValue::Array(_) | JsonValue::Object(_) => None,
}
}
@@ -576,7 +595,7 @@ fn merge_missing_toml_values(existing: &mut TomlValue, incoming: &TomlValue) ->
fn write_toml_file(path: &Path, value: &TomlValue) -> io::Result<()> {
let serialized = toml::to_string_pretty(value)
.map_err(|err| invalid_data_error(format!("failed to serialize config.toml: {err}")))?;
fs::write(path, format!("{serialized}\n"))
fs::write(path, format!("{}\n", serialized.trim_end()))
}
fn is_empty_toml_table(value: &TomlValue) -> bool {
@@ -638,7 +657,7 @@ mod tests {
ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::Config,
description: format!(
"Migrate {} into {}.",
"Migrate {} into {}",
claude_home.join("settings.json").display(),
codex_home.join("config.toml").display()
),
@@ -647,7 +666,7 @@ mod tests {
ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::Skills,
description: format!(
"Copy skill folders from {} to {}.",
"Copy skill folders from {} to {}",
claude_home.join("skills").display(),
agents_skills.display()
),
@@ -656,7 +675,7 @@ mod tests {
ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::AgentsMd,
description: format!(
"Import {} to {}.",
"Import {} to {}",
claude_home.join("CLAUDE.md").display(),
codex_home.join("AGENTS.md").display()
),
@@ -687,7 +706,7 @@ mod tests {
ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::AgentsMd,
description: format!(
"Import {} to {}.",
"Import {} to {}",
repo_root.join("CLAUDE.md").display(),
repo_root.join("AGENTS.md").display(),
),
@@ -696,7 +715,7 @@ mod tests {
ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::AgentsMd,
description: format!(
"Import {} to {}.",
"Import {} to {}",
repo_root.join("CLAUDE.md").display(),
repo_root.join("AGENTS.md").display(),
),
@@ -717,7 +736,7 @@ mod tests {
fs::create_dir_all(claude_home.join("skills").join("skill-a")).expect("create skills");
fs::write(
claude_home.join("settings.json"),
r#"{"model":"claude","permissions":{"ask":["git push"]},"env":{"FOO":"bar"},"sandbox":{"enabled":true,"network":{"allowLocalBinding":true}}}"#,
r#"{"model":"claude","permissions":{"ask":["git push"]},"env":{"FOO":"bar","CI":false,"MAX_RETRIES":3,"MY_TEAM":"codex","IGNORED":null,"LIST":["a","b"],"MAP":{"x":1}},"sandbox":{"enabled":true,"network":{"allowLocalBinding":true}}}"#,
)
.expect("write settings");
fs::write(
@@ -752,23 +771,10 @@ mod tests {
"Codex guidance"
);
let parsed_config: TomlValue = toml::from_str(
&fs::read_to_string(codex_home.join("config.toml")).expect("read config"),
)
.expect("parse config");
let expected_config: TomlValue = toml::from_str(
r#"
sandbox_mode = "workspace-write"
[shell_environment_policy]
inherit = "core"
[shell_environment_policy.set]
FOO = "bar"
"#,
)
.expect("parse expected");
assert_eq!(parsed_config, expected_config);
assert_eq!(
fs::read_to_string(codex_home.join("config.toml")).expect("read config"),
"sandbox_mode = \"workspace-write\"\n\n[shell_environment_policy]\ninherit = \"core\"\n\n[shell_environment_policy.set]\nCI = \"false\"\nFOO = \"bar\"\nMAX_RETRIES = \"3\"\nMY_TEAM = \"codex\"\n"
);
assert_eq!(
fs::read_to_string(agents_skills.join("skill-a").join("SKILL.md"))
.expect("read copied skill"),
@@ -917,4 +923,65 @@ mod tests {
"Codex guidance"
);
}
#[test]
fn detect_repo_prefers_non_empty_dot_claude_agents_source() {
let root = TempDir::new().expect("create tempdir");
let repo_root = root.path().join("repo");
fs::create_dir_all(repo_root.join(".git")).expect("create git");
fs::create_dir_all(repo_root.join(".claude")).expect("create dot claude");
fs::write(repo_root.join("CLAUDE.md"), " \n\t").expect("write empty root source");
fs::write(
repo_root.join(".claude").join("CLAUDE.md"),
"Claude code guidance",
)
.expect("write dot claude source");
let items = service_for_paths(root.path().join(".claude"), root.path().join(".codex"))
.detect(ExternalAgentConfigDetectOptions {
include_home: false,
cwds: Some(vec![repo_root.clone()]),
})
.expect("detect");
assert_eq!(
items,
vec![ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::AgentsMd,
description: format!(
"Import {} to {}",
repo_root.join(".claude").join("CLAUDE.md").display(),
repo_root.join("AGENTS.md").display(),
),
cwd: Some(repo_root),
}]
);
}
#[test]
fn import_repo_uses_non_empty_dot_claude_agents_source() {
let root = TempDir::new().expect("create tempdir");
let repo_root = root.path().join("repo");
fs::create_dir_all(repo_root.join(".git")).expect("create git");
fs::create_dir_all(repo_root.join(".claude")).expect("create dot claude");
fs::write(repo_root.join("CLAUDE.md"), "").expect("write empty root source");
fs::write(
repo_root.join(".claude").join("CLAUDE.md"),
"Claude code guidance",
)
.expect("write dot claude source");
service_for_paths(root.path().join(".claude"), root.path().join(".codex"))
.import(vec![ExternalAgentConfigMigrationItem {
item_type: ExternalAgentConfigMigrationItemType::AgentsMd,
description: String::new(),
cwd: Some(repo_root.clone()),
}])
.expect("import");
assert_eq!(
fs::read_to_string(repo_root.join("AGENTS.md")).expect("read target"),
"Codex guidance"
);
}
}
+160
View File
@@ -33,6 +33,10 @@ use tracing::warn;
const DEFAULT_SKILLS_DIR_NAME: &str = "skills";
const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json";
const DEFAULT_APP_CONFIG_FILE: &str = ".app.json";
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct AppConnectorId(pub String);
#[derive(Debug, Clone, PartialEq)]
pub struct LoadedPlugin {
@@ -42,6 +46,7 @@ pub struct LoadedPlugin {
pub enabled: bool,
pub skill_roots: Vec<PathBuf>,
pub mcp_servers: HashMap<String, McpServerConfig>,
pub apps: Vec<AppConnectorId>,
pub error: Option<String>,
}
@@ -80,6 +85,21 @@ impl PluginLoadOutcome {
}
mcp_servers
}
pub fn effective_apps(&self) -> Vec<AppConnectorId> {
let mut apps = Vec::new();
let mut seen_connector_ids = std::collections::HashSet::new();
for plugin in self.plugins.iter().filter(|plugin| plugin.is_active()) {
for connector_id in &plugin.apps {
if seen_connector_ids.insert(connector_id.clone()) {
apps.push(connector_id.clone());
}
}
}
apps
}
}
pub struct PluginsManager {
@@ -227,6 +247,18 @@ struct PluginMcpFile {
mcp_servers: HashMap<String, JsonValue>,
}
#[derive(Debug, Default, Deserialize)]
#[serde(rename_all = "camelCase")]
struct PluginAppFile {
#[serde(default)]
apps: HashMap<String, PluginAppConfig>,
}
#[derive(Debug, Default, Deserialize)]
struct PluginAppConfig {
id: String,
}
pub(crate) fn load_plugins_from_layer_stack(
config_layer_stack: &ConfigLayerStack,
store: &PluginStore,
@@ -299,6 +331,7 @@ fn load_plugin(config_name: String, plugin: &PluginConfig, store: &PluginStore)
enabled: plugin.enabled,
skill_roots: Vec::new(),
mcp_servers: HashMap::new(),
apps: Vec::new(),
error: None,
};
@@ -341,6 +374,10 @@ fn load_plugin(config_name: String, plugin: &PluginConfig, store: &PluginStore)
}
}
loaded_plugin.mcp_servers = mcp_servers;
loaded_plugin.apps = load_apps_from_file(
plugin_root.as_path(),
&plugin_root.as_path().join(DEFAULT_APP_CONFIG_FILE),
);
loaded_plugin
}
@@ -364,6 +401,39 @@ fn default_mcp_config_paths(plugin_root: &Path) -> Vec<PathBuf> {
paths
}
fn load_apps_from_file(plugin_root: &Path, app_config_path: &Path) -> Vec<AppConnectorId> {
let Ok(contents) = fs::read_to_string(app_config_path) else {
return Vec::new();
};
let parsed = match serde_json::from_str::<PluginAppFile>(&contents) {
Ok(parsed) => parsed,
Err(err) => {
warn!(
path = %app_config_path.display(),
"failed to parse plugin app config: {err}"
);
return Vec::new();
}
};
let mut apps: Vec<PluginAppConfig> = parsed.apps.into_values().collect();
apps.sort_unstable_by(|left, right| left.id.cmp(&right.id));
apps.into_iter()
.filter_map(|app| {
if app.id.trim().is_empty() {
warn!(
plugin = %plugin_root.display(),
"plugin app config is missing an app id"
);
None
} else {
Some(AppConnectorId(app.id))
}
})
.collect()
}
fn load_mcp_servers_from_file(plugin_root: &Path, mcp_config_path: &Path) -> PluginMcpDiscovery {
let Ok(contents) = fs::read_to_string(mcp_config_path) else {
return PluginMcpDiscovery::default();
@@ -550,6 +620,16 @@ mod tests {
}
}"#,
);
write_file(
&plugin_root.join(".app.json"),
r#"{
"apps": {
"example": {
"id": "connector_example"
}
}
}"#,
);
let outcome =
load_plugins_from_config(&plugin_config_toml(true, true), codex_home.path()).await;
@@ -582,6 +662,7 @@ mod tests {
oauth_resource: None,
},
)]),
apps: vec![AppConnectorId("connector_example".to_string())],
error: None,
}]
);
@@ -590,6 +671,10 @@ mod tests {
vec![plugin_root.join("skills")]
);
assert_eq!(outcome.effective_mcp_servers().len(), 1);
assert_eq!(
outcome.effective_apps(),
vec![AppConnectorId("connector_example".to_string())]
);
}
#[tokio::test]
@@ -628,6 +713,7 @@ mod tests {
enabled: false,
skill_roots: Vec::new(),
mcp_servers: HashMap::new(),
apps: Vec::new(),
error: None,
}]
);
@@ -635,6 +721,80 @@ mod tests {
assert!(outcome.effective_mcp_servers().is_empty());
}
#[tokio::test]
async fn effective_apps_dedupes_connector_ids_across_plugins() {
let codex_home = TempDir::new().unwrap();
let plugin_a_root = codex_home
.path()
.join("plugins/cache")
.join("test/plugin-a/local");
let plugin_b_root = codex_home
.path()
.join("plugins/cache")
.join("test/plugin-b/local");
write_file(
&plugin_a_root.join(".codex-plugin/plugin.json"),
r#"{"name":"plugin-a"}"#,
);
write_file(
&plugin_a_root.join(".app.json"),
r#"{
"apps": {
"example": {
"id": "connector_example"
}
}
}"#,
);
write_file(
&plugin_b_root.join(".codex-plugin/plugin.json"),
r#"{"name":"plugin-b"}"#,
);
write_file(
&plugin_b_root.join(".app.json"),
r#"{
"apps": {
"chat": {
"id": "connector_example"
},
"gmail": {
"id": "connector_gmail"
}
}
}"#,
);
let mut root = toml::map::Map::new();
let mut features = toml::map::Map::new();
features.insert("plugins".to_string(), Value::Boolean(true));
root.insert("features".to_string(), Value::Table(features));
let mut plugins = toml::map::Map::new();
let mut plugin_a = toml::map::Map::new();
plugin_a.insert("enabled".to_string(), Value::Boolean(true));
plugins.insert("plugin-a@test".to_string(), Value::Table(plugin_a));
let mut plugin_b = toml::map::Map::new();
plugin_b.insert("enabled".to_string(), Value::Boolean(true));
plugins.insert("plugin-b@test".to_string(), Value::Table(plugin_b));
root.insert("plugins".to_string(), Value::Table(plugins));
let config_toml =
toml::to_string(&Value::Table(root)).expect("plugin test config should serialize");
let outcome = load_plugins_from_config(&config_toml, codex_home.path()).await;
assert_eq!(
outcome.effective_apps(),
vec![
AppConnectorId("connector_example".to_string()),
AppConnectorId("connector_gmail".to_string()),
]
);
}
#[test]
fn plugin_namespace_for_skill_path_uses_manifest_name() {
let codex_home = TempDir::new().unwrap();
+1
View File
@@ -2,6 +2,7 @@ mod manager;
mod manifest;
mod store;
pub use manager::AppConnectorId;
pub use manager::LoadedPlugin;
pub use manager::PluginInstallError;
pub use manager::PluginLoadOutcome;
+11 -7
View File
@@ -52,9 +52,13 @@ fn render_js_repl_instructions(config: &Config) -> Option<String> {
"- Use `js_repl` for Node-backed JavaScript with top-level await in a persistent kernel.\n",
);
section.push_str("- `js_repl` is a freeform/custom tool. Direct `js_repl` calls must send raw JavaScript tool input (optionally with first-line `// codex-js-repl: timeout_ms=15000`). Do not wrap code in JSON (for example `{\"code\":\"...\"}`), quotes, or markdown code fences.\n");
section.push_str("- Helpers: `codex.tmpDir` and `codex.tool(name, args?)`.\n");
section.push_str("- `codex.tool` executes a normal tool call and resolves to the raw tool output object. Use it for shell and non-shell tools alike.\n");
section.push_str("- To share generated images with the model, write a file under `codex.tmpDir`, call `await codex.tool(\"view_image\", { path: \"/absolute/path\" })`, then delete the file.\n");
section.push_str(
"- Helpers: `codex.tmpDir`, `codex.tool(name, args?)`, and `codex.emitImage(imageLike)`.\n",
);
section.push_str("- `codex.tool` executes a normal tool call and resolves to the raw tool output object. Use it for shell and non-shell tools alike. Nested tool outputs stay inside JavaScript unless you emit them explicitly.\n");
section.push_str("- `codex.emitImage(...)` adds exactly one image to the outer `js_repl` function output. It accepts a direct image URL, a single `input_image` item, an object like `{ bytes, mimeType }`, or a raw tool response object with exactly one image and no text. It rejects mixed text-and-image content.\n");
section.push_str("- Example of sharing an in-memory Playwright screenshot: `await codex.emitImage({ bytes: await page.screenshot({ type: \"jpeg\", quality: 85 }), mimeType: \"image/jpeg\" })`.\n");
section.push_str("- Example of sharing a local image tool result: `await codex.emitImage(codex.tool(\"view_image\", { path: \"/absolute/path\" }))`.\n");
if config.features.enabled(Feature::ImageDetailOriginal) {
section.push_str("- When generating or converting images for `view_image` in `js_repl`, prefer JPEG at 85% quality unless lossless quality is strictly required; other formats can be used if the user requests them. This keeps uploads smaller and reduces the chance of hitting image size caps.\n");
}
@@ -67,7 +71,7 @@ fn render_js_repl_instructions(config: &Config) -> Option<String> {
.push_str("- MCP tools (if any) can also be called by name via `codex.tool(...)`.\n");
}
section.push_str("- Avoid direct access to `process.stdout` / `process.stderr` / `process.stdin`; it can corrupt the JSON line protocol. Use `console.log` and `codex.tool(...)`.");
section.push_str("- Avoid direct access to `process.stdout` / `process.stderr` / `process.stdin`; it can corrupt the JSON line protocol. Use `console.log`, `codex.tool(...)`, and `codex.emitImage(...)`.");
Some(section)
}
@@ -478,7 +482,7 @@ mod tests {
let res = get_user_instructions(&cfg, None)
.await
.expect("js_repl instructions expected");
let expected = "## JavaScript REPL (Node)\n- Use `js_repl` for Node-backed JavaScript with top-level await in a persistent kernel.\n- `js_repl` is a freeform/custom tool. Direct `js_repl` calls must send raw JavaScript tool input (optionally with first-line `// codex-js-repl: timeout_ms=15000`). Do not wrap code in JSON (for example `{\"code\":\"...\"}`), quotes, or markdown code fences.\n- Helpers: `codex.tmpDir` and `codex.tool(name, args?)`.\n- `codex.tool` executes a normal tool call and resolves to the raw tool output object. Use it for shell and non-shell tools alike.\n- To share generated images with the model, write a file under `codex.tmpDir`, call `await codex.tool(\"view_image\", { path: \"/absolute/path\" })`, then delete the file.\n- Top-level bindings persist across cells. If you hit `SyntaxError: Identifier 'x' has already been declared`, reuse the binding, pick a new name, wrap in `{ ... }` for block scope, or reset the kernel with `js_repl_reset`.\n- Top-level static import declarations (for example `import x from \"pkg\"`) are currently unsupported in `js_repl`; use dynamic imports with `await import(\"pkg\")` instead.\n- Avoid direct access to `process.stdout` / `process.stderr` / `process.stdin`; it can corrupt the JSON line protocol. Use `console.log` and `codex.tool(...)`.";
let expected = "## JavaScript REPL (Node)\n- Use `js_repl` for Node-backed JavaScript with top-level await in a persistent kernel.\n- `js_repl` is a freeform/custom tool. Direct `js_repl` calls must send raw JavaScript tool input (optionally with first-line `// codex-js-repl: timeout_ms=15000`). Do not wrap code in JSON (for example `{\"code\":\"...\"}`), quotes, or markdown code fences.\n- Helpers: `codex.tmpDir`, `codex.tool(name, args?)`, and `codex.emitImage(imageLike)`.\n- `codex.tool` executes a normal tool call and resolves to the raw tool output object. Use it for shell and non-shell tools alike. Nested tool outputs stay inside JavaScript unless you emit them explicitly.\n- `codex.emitImage(...)` adds exactly one image to the outer `js_repl` function output. It accepts a direct image URL, a single `input_image` item, an object like `{ bytes, mimeType }`, or a raw tool response object with exactly one image and no text. It rejects mixed text-and-image content.\n- Example of sharing an in-memory Playwright screenshot: `await codex.emitImage({ bytes: await page.screenshot({ type: \"jpeg\", quality: 85 }), mimeType: \"image/jpeg\" })`.\n- Example of sharing a local image tool result: `await codex.emitImage(codex.tool(\"view_image\", { path: \"/absolute/path\" }))`.\n- Top-level bindings persist across cells. If you hit `SyntaxError: Identifier 'x' has already been declared`, reuse the binding, pick a new name, wrap in `{ ... }` for block scope, or reset the kernel with `js_repl_reset`.\n- Top-level static import declarations (for example `import x from \"pkg\"`) are currently unsupported in `js_repl`; use dynamic imports with `await import(\"pkg\")` instead.\n- Avoid direct access to `process.stdout` / `process.stderr` / `process.stdin`; it can corrupt the JSON line protocol. Use `console.log`, `codex.tool(...)`, and `codex.emitImage(...)`.";
assert_eq!(res, expected);
}
@@ -497,7 +501,7 @@ mod tests {
let res = get_user_instructions(&cfg, None)
.await
.expect("js_repl instructions expected");
let expected = "## JavaScript REPL (Node)\n- Use `js_repl` for Node-backed JavaScript with top-level await in a persistent kernel.\n- `js_repl` is a freeform/custom tool. Direct `js_repl` calls must send raw JavaScript tool input (optionally with first-line `// codex-js-repl: timeout_ms=15000`). Do not wrap code in JSON (for example `{\"code\":\"...\"}`), quotes, or markdown code fences.\n- Helpers: `codex.tmpDir` and `codex.tool(name, args?)`.\n- `codex.tool` executes a normal tool call and resolves to the raw tool output object. Use it for shell and non-shell tools alike.\n- To share generated images with the model, write a file under `codex.tmpDir`, call `await codex.tool(\"view_image\", { path: \"/absolute/path\" })`, then delete the file.\n- Top-level bindings persist across cells. If you hit `SyntaxError: Identifier 'x' has already been declared`, reuse the binding, pick a new name, wrap in `{ ... }` for block scope, or reset the kernel with `js_repl_reset`.\n- Top-level static import declarations (for example `import x from \"pkg\"`) are currently unsupported in `js_repl`; use dynamic imports with `await import(\"pkg\")` instead.\n- Do not call tools directly; use `js_repl` + `codex.tool(...)` for all tool calls, including shell commands.\n- MCP tools (if any) can also be called by name via `codex.tool(...)`.\n- Avoid direct access to `process.stdout` / `process.stderr` / `process.stdin`; it can corrupt the JSON line protocol. Use `console.log` and `codex.tool(...)`.";
let expected = "## JavaScript REPL (Node)\n- Use `js_repl` for Node-backed JavaScript with top-level await in a persistent kernel.\n- `js_repl` is a freeform/custom tool. Direct `js_repl` calls must send raw JavaScript tool input (optionally with first-line `// codex-js-repl: timeout_ms=15000`). Do not wrap code in JSON (for example `{\"code\":\"...\"}`), quotes, or markdown code fences.\n- Helpers: `codex.tmpDir`, `codex.tool(name, args?)`, and `codex.emitImage(imageLike)`.\n- `codex.tool` executes a normal tool call and resolves to the raw tool output object. Use it for shell and non-shell tools alike. Nested tool outputs stay inside JavaScript unless you emit them explicitly.\n- `codex.emitImage(...)` adds exactly one image to the outer `js_repl` function output. It accepts a direct image URL, a single `input_image` item, an object like `{ bytes, mimeType }`, or a raw tool response object with exactly one image and no text. It rejects mixed text-and-image content.\n- Example of sharing an in-memory Playwright screenshot: `await codex.emitImage({ bytes: await page.screenshot({ type: \"jpeg\", quality: 85 }), mimeType: \"image/jpeg\" })`.\n- Example of sharing a local image tool result: `await codex.emitImage(codex.tool(\"view_image\", { path: \"/absolute/path\" }))`.\n- Top-level bindings persist across cells. If you hit `SyntaxError: Identifier 'x' has already been declared`, reuse the binding, pick a new name, wrap in `{ ... }` for block scope, or reset the kernel with `js_repl_reset`.\n- Top-level static import declarations (for example `import x from \"pkg\"`) are currently unsupported in `js_repl`; use dynamic imports with `await import(\"pkg\")` instead.\n- Do not call tools directly; use `js_repl` + `codex.tool(...)` for all tool calls, including shell commands.\n- MCP tools (if any) can also be called by name via `codex.tool(...)`.\n- Avoid direct access to `process.stdout` / `process.stderr` / `process.stdin`; it can corrupt the JSON line protocol. Use `console.log`, `codex.tool(...)`, and `codex.emitImage(...)`.";
assert_eq!(res, expected);
}
@@ -516,7 +520,7 @@ mod tests {
let res = get_user_instructions(&cfg, None)
.await
.expect("js_repl instructions expected");
let expected = "## JavaScript REPL (Node)\n- Use `js_repl` for Node-backed JavaScript with top-level await in a persistent kernel.\n- `js_repl` is a freeform/custom tool. Direct `js_repl` calls must send raw JavaScript tool input (optionally with first-line `// codex-js-repl: timeout_ms=15000`). Do not wrap code in JSON (for example `{\"code\":\"...\"}`), quotes, or markdown code fences.\n- Helpers: `codex.tmpDir` and `codex.tool(name, args?)`.\n- `codex.tool` executes a normal tool call and resolves to the raw tool output object. Use it for shell and non-shell tools alike.\n- To share generated images with the model, write a file under `codex.tmpDir`, call `await codex.tool(\"view_image\", { path: \"/absolute/path\" })`, then delete the file.\n- When generating or converting images for `view_image` in `js_repl`, prefer JPEG at 85% quality unless lossless quality is strictly required; other formats can be used if the user requests them. This keeps uploads smaller and reduces the chance of hitting image size caps.\n- Top-level bindings persist across cells. If you hit `SyntaxError: Identifier 'x' has already been declared`, reuse the binding, pick a new name, wrap in `{ ... }` for block scope, or reset the kernel with `js_repl_reset`.\n- Top-level static import declarations (for example `import x from \"pkg\"`) are currently unsupported in `js_repl`; use dynamic imports with `await import(\"pkg\")` instead.\n- Avoid direct access to `process.stdout` / `process.stderr` / `process.stdin`; it can corrupt the JSON line protocol. Use `console.log` and `codex.tool(...)`.";
let expected = "## JavaScript REPL (Node)\n- Use `js_repl` for Node-backed JavaScript with top-level await in a persistent kernel.\n- `js_repl` is a freeform/custom tool. Direct `js_repl` calls must send raw JavaScript tool input (optionally with first-line `// codex-js-repl: timeout_ms=15000`). Do not wrap code in JSON (for example `{\"code\":\"...\"}`), quotes, or markdown code fences.\n- Helpers: `codex.tmpDir`, `codex.tool(name, args?)`, and `codex.emitImage(imageLike)`.\n- `codex.tool` executes a normal tool call and resolves to the raw tool output object. Use it for shell and non-shell tools alike. Nested tool outputs stay inside JavaScript unless you emit them explicitly.\n- `codex.emitImage(...)` adds exactly one image to the outer `js_repl` function output. It accepts a direct image URL, a single `input_image` item, an object like `{ bytes, mimeType }`, or a raw tool response object with exactly one image and no text. It rejects mixed text-and-image content.\n- Example of sharing an in-memory Playwright screenshot: `await codex.emitImage({ bytes: await page.screenshot({ type: \"jpeg\", quality: 85 }), mimeType: \"image/jpeg\" })`.\n- Example of sharing a local image tool result: `await codex.emitImage(codex.tool(\"view_image\", { path: \"/absolute/path\" }))`.\n- When generating or converting images for `view_image` in `js_repl`, prefer JPEG at 85% quality unless lossless quality is strictly required; other formats can be used if the user requests them. This keeps uploads smaller and reduces the chance of hitting image size caps.\n- Top-level bindings persist across cells. If you hit `SyntaxError: Identifier 'x' has already been declared`, reuse the binding, pick a new name, wrap in `{ ... }` for block scope, or reset the kernel with `js_repl_reset`.\n- Top-level static import declarations (for example `import x from \"pkg\"`) are currently unsupported in `js_repl`; use dynamic imports with `await import(\"pkg\")` instead.\n- Avoid direct access to `process.stdout` / `process.stderr` / `process.stdin`; it can corrupt the JSON line protocol. Use `console.log`, `codex.tool(...)`, and `codex.emitImage(...)`.";
assert_eq!(res, expected);
}
-3
View File
@@ -78,9 +78,6 @@ impl SessionTask for RegularTask {
let sess = session.clone_session();
let run_turn_span = trace_span!("run_turn");
sess.set_server_reasoning_included(false).await;
sess.services
.otel_manager
.apply_traceparent_parent(&run_turn_span);
let prewarmed_client_session = self.take_prewarmed_session().await;
run_turn(
sess,
+316 -1
View File
@@ -115,7 +115,10 @@ function isDeniedBuiltin(specifier) {
/** @type {Map<string, (msg: any) => void>} */
const pendingTool = new Map();
/** @type {Map<string, (msg: any) => void>} */
const pendingEmitImage = new Map();
let toolCounter = 0;
let emitImageCounter = 0;
const tmpDir = process.env.CODEX_JS_TMP_DIR || process.cwd();
const nodeModuleDirEnv = process.env.CODEX_JS_REPL_NODE_MODULE_DIRS ?? "";
const moduleSearchBases = (() => {
@@ -480,8 +483,255 @@ function withCapturedConsole(ctx, fn) {
});
}
function isPlainObject(value) {
return Boolean(value) && typeof value === "object" && !Array.isArray(value);
}
function toByteArray(value) {
if (value instanceof Uint8Array) {
return value;
}
if (value instanceof ArrayBuffer) {
return new Uint8Array(value);
}
if (ArrayBuffer.isView(value)) {
return new Uint8Array(value.buffer, value.byteOffset, value.byteLength);
}
return null;
}
function encodeByteImage(bytes, mimeType, detail) {
if (bytes.byteLength === 0) {
throw new Error("codex.emitImage expected non-empty bytes");
}
if (typeof mimeType !== "string" || !mimeType) {
throw new Error("codex.emitImage expected a non-empty mimeType");
}
const image_url = `data:${mimeType};base64,${Buffer.from(bytes).toString("base64")}`;
return { image_url, detail };
}
function parseImageDetail(detail) {
if (typeof detail === "undefined") {
return undefined;
}
if (typeof detail !== "string" || !detail) {
throw new Error("codex.emitImage expected detail to be a non-empty string");
}
if (
detail !== "auto" &&
detail !== "low" &&
detail !== "high" &&
detail !== "original"
) {
throw new Error(
'codex.emitImage expected detail to be one of "auto", "low", "high", or "original"',
);
}
return detail;
}
function parseInputImageItem(value) {
if (!isPlainObject(value) || value.type !== "input_image") {
return null;
}
if (typeof value.image_url !== "string" || !value.image_url) {
throw new Error("codex.emitImage expected a non-empty image_url");
}
return {
images: [{ image_url: value.image_url, detail: parseImageDetail(value.detail) }],
textCount: 0,
};
}
function parseContentItems(items) {
if (!Array.isArray(items)) {
return null;
}
const images = [];
let textCount = 0;
for (const item of items) {
if (!isPlainObject(item) || typeof item.type !== "string") {
throw new Error("codex.emitImage received malformed content items");
}
if (item.type === "input_image") {
if (typeof item.image_url !== "string" || !item.image_url) {
throw new Error("codex.emitImage expected a non-empty image_url");
}
images.push({
image_url: item.image_url,
detail: parseImageDetail(item.detail),
});
continue;
}
if (item.type === "input_text" || item.type === "output_text") {
textCount += 1;
continue;
}
throw new Error(
`codex.emitImage does not support content item type "${item.type}"`,
);
}
return { images, textCount };
}
function parseByteImageValue(value) {
if (!isPlainObject(value) || !("bytes" in value)) {
return null;
}
const bytes = toByteArray(value.bytes);
if (!bytes) {
throw new Error(
"codex.emitImage expected bytes to be Buffer, Uint8Array, ArrayBuffer, or ArrayBufferView",
);
}
const detail = parseImageDetail(value.detail);
return encodeByteImage(bytes, value.mimeType, detail);
}
function parseToolOutput(output) {
if (typeof output === "string") {
return {
images: [],
textCount: output.length > 0 ? 1 : 0,
};
}
const parsedItems = parseContentItems(output);
if (parsedItems) {
return parsedItems;
}
throw new Error("codex.emitImage received an unsupported tool output shape");
}
function normalizeMcpImageData(data, mimeType) {
if (typeof data !== "string" || !data) {
throw new Error("codex.emitImage expected MCP image data");
}
if (data.startsWith("data:")) {
return data;
}
const normalizedMimeType =
typeof mimeType === "string" && mimeType ? mimeType : "application/octet-stream";
return `data:${normalizedMimeType};base64,${data}`;
}
function parseMcpToolResult(result) {
if (typeof result === "string") {
return { images: [], textCount: result.length > 0 ? 1 : 0 };
}
if (!isPlainObject(result)) {
throw new Error("codex.emitImage received an unsupported MCP result");
}
if ("Err" in result) {
const error = result.Err;
return { images: [], textCount: typeof error === "string" && error ? 1 : 0 };
}
if (!("Ok" in result)) {
throw new Error("codex.emitImage received an unsupported MCP result");
}
const ok = result.Ok;
if (!isPlainObject(ok) || !Array.isArray(ok.content)) {
throw new Error("codex.emitImage received malformed MCP content");
}
const images = [];
let textCount = 0;
for (const item of ok.content) {
if (!isPlainObject(item) || typeof item.type !== "string") {
throw new Error("codex.emitImage received malformed MCP content");
}
if (item.type === "image") {
images.push({
image_url: normalizeMcpImageData(item.data, item.mimeType ?? item.mime_type),
});
continue;
}
if (item.type === "text") {
textCount += 1;
continue;
}
throw new Error(
`codex.emitImage does not support MCP content type "${item.type}"`,
);
}
return { images, textCount };
}
function requireSingleImage(parsed) {
if (parsed.textCount > 0) {
throw new Error("codex.emitImage does not accept mixed text and image content");
}
if (parsed.images.length !== 1) {
throw new Error("codex.emitImage expected exactly one image");
}
return parsed.images[0];
}
function normalizeEmitImageValue(value) {
if (typeof value === "string") {
if (!value) {
throw new Error("codex.emitImage expected a non-empty image URL");
}
return { image_url: value };
}
const directItem = parseInputImageItem(value);
if (directItem) {
return requireSingleImage(directItem);
}
const byteImage = parseByteImageValue(value);
if (byteImage) {
return byteImage;
}
const directItems = parseContentItems(value);
if (directItems) {
return requireSingleImage(directItems);
}
if (!isPlainObject(value)) {
throw new Error("codex.emitImage received an unsupported value");
}
if (value.type === "message") {
return requireSingleImage(parseContentItems(value.content));
}
if (
value.type === "function_call_output" ||
value.type === "custom_tool_call_output"
) {
return requireSingleImage(parseToolOutput(value.output));
}
if (value.type === "mcp_tool_call_output") {
return requireSingleImage(parseMcpToolResult(value.result));
}
if ("output" in value) {
return requireSingleImage(parseToolOutput(value.output));
}
if ("content" in value) {
return requireSingleImage(parseContentItems(value.content));
}
throw new Error("codex.emitImage received an unsupported value");
}
async function handleExec(message) {
activeExecId = message.id;
const pendingBackgroundTasks = new Set();
const tool = (toolName, args) => {
if (typeof toolName !== "string" || !toolName) {
return Promise.reject(new Error("codex.tool expects a tool name string"));
@@ -512,13 +762,57 @@ async function handleExec(message) {
});
});
};
const emitImage = (imageLike) => {
const operation = (async () => {
const normalized = normalizeEmitImageValue(await imageLike);
const id = `${message.id}-emit-image-${emitImageCounter++}`;
const payload = {
type: "emit_image",
id,
exec_id: message.id,
image_url: normalized.image_url,
detail: normalized.detail ?? null,
};
send(payload);
return new Promise((resolve, reject) => {
pendingEmitImage.set(id, (res) => {
if (!res.ok) {
reject(new Error(res.error || "emitImage failed"));
return;
}
resolve();
});
});
})();
const observation = { observed: false };
const trackedOperation = operation.then(
() => ({ ok: true, error: null, observation }),
(error) => ({ ok: false, error, observation }),
);
pendingBackgroundTasks.add(trackedOperation);
return {
then(onFulfilled, onRejected) {
observation.observed = true;
return operation.then(onFulfilled, onRejected);
},
catch(onRejected) {
observation.observed = true;
return operation.catch(onRejected);
},
finally(onFinally) {
observation.observed = true;
return operation.finally(onFinally);
},
};
};
try {
const code = typeof message.code === "string" ? message.code : "";
const { source, nextBindings } = await buildModuleSource(code);
let output = "";
context.codex = { tmpDir, tool };
context.codex = { tmpDir, tool, emitImage };
context.tmpDir = tmpDir;
await withCapturedConsole(context, async (logs) => {
@@ -558,6 +852,15 @@ async function handleExec(message) {
});
await module.evaluate();
if (pendingBackgroundTasks.size > 0) {
const backgroundResults = await Promise.all([...pendingBackgroundTasks]);
const firstUnhandledBackgroundError = backgroundResults.find(
(result) => !result.ok && !result.observation.observed,
);
if (firstUnhandledBackgroundError) {
throw firstUnhandledBackgroundError.error;
}
}
previousModule = module;
previousBindings = nextBindings;
output = logs.join("\n");
@@ -593,6 +896,14 @@ function handleToolResult(message) {
}
}
function handleEmitImageResult(message) {
const resolver = pendingEmitImage.get(message.id);
if (resolver) {
pendingEmitImage.delete(message.id);
resolver(message);
}
}
let queue = Promise.resolve();
process.on("uncaughtException", (error) => {
@@ -622,5 +933,9 @@ input.on("line", (line) => {
}
if (message.type === "run_tool_result") {
handleToolResult(message);
return;
}
if (message.type === "emit_image_result") {
handleEmitImageResult(message);
}
});
+550 -65
View File
@@ -12,6 +12,7 @@ use codex_protocol::ThreadId;
use codex_protocol::models::ContentItem;
use codex_protocol::models::FunctionCallOutputContentItem;
use codex_protocol::models::FunctionCallOutputPayload;
use codex_protocol::models::ImageDetail;
use codex_protocol::models::ResponseInputItem;
use serde::Deserialize;
use serde::Serialize;
@@ -35,6 +36,7 @@ use crate::codex::Session;
use crate::codex::TurnContext;
use crate::exec::ExecExpiration;
use crate::exec_env::create_env;
use crate::features::Feature;
use crate::function_tool::FunctionCallError;
use crate::sandboxing::CommandSpec;
use crate::sandboxing::SandboxManager;
@@ -372,18 +374,14 @@ impl JsReplManager {
Some(state.cancel.clone())
}
async fn record_exec_tool_call_content_items(
async fn record_exec_content_item(
exec_tool_calls: &Arc<Mutex<HashMap<String, ExecToolCalls>>>,
exec_id: &str,
content_items: Vec<FunctionCallOutputContentItem>,
content_item: FunctionCallOutputContentItem,
) {
if content_items.is_empty() {
return;
}
let mut calls = exec_tool_calls.lock().await;
if let Some(state) = calls.get_mut(exec_id) {
state.content_items.extend(content_items);
state.content_items.push(content_item);
}
}
@@ -1131,6 +1129,49 @@ impl JsReplManager {
exec_contexts.lock().await.remove(&id);
JsReplManager::clear_exec_tool_calls_map(&exec_tool_calls, &id).await;
}
KernelToHost::EmitImage(req) => {
let exec_id = req.exec_id.clone();
let emit_id = req.id.clone();
let response =
if let Some(ctx) = exec_contexts.lock().await.get(&exec_id).cloned() {
let content_item = emitted_image_content_item(
ctx.turn.as_ref(),
req.image_url,
req.detail,
);
JsReplManager::record_exec_content_item(
&exec_tool_calls,
&exec_id,
content_item,
)
.await;
HostToKernel::EmitImageResult(EmitImageResult {
id: emit_id,
ok: true,
error: None,
})
} else {
HostToKernel::EmitImageResult(EmitImageResult {
id: emit_id,
ok: false,
error: Some("js_repl exec context not found".to_string()),
})
};
if let Err(err) = JsReplManager::write_message(&stdin, &response).await {
let snapshot =
JsReplManager::kernel_debug_snapshot(&child, &recent_stderr).await;
warn!(
exec_id = %exec_id,
emit_id = %req.id,
error = %err,
kernel_pid = ?snapshot.pid,
kernel_status = %snapshot.status,
kernel_stderr_tail = %snapshot.stderr_tail,
"failed to reply to kernel emit_image request"
);
}
}
KernelToHost::RunTool(req) => {
let Some(reset_cancel) =
JsReplManager::begin_exec_tool_call(&exec_tool_calls, &req.exec_id).await
@@ -1176,11 +1217,7 @@ impl JsReplManager {
response: None,
error: Some("js_repl execution reset".to_string()),
},
result = JsReplManager::run_tool_request(
ctx,
req,
Arc::clone(&exec_tool_calls_for_task),
) => result,
result = JsReplManager::run_tool_request(ctx, req) => result,
}
}
None => RunToolResult {
@@ -1274,11 +1311,7 @@ impl JsReplManager {
}
}
async fn run_tool_request(
exec: ExecContext,
req: RunToolRequest,
exec_tool_calls: Arc<Mutex<HashMap<String, ExecToolCalls>>>,
) -> RunToolResult {
async fn run_tool_request(exec: ExecContext, req: RunToolRequest) -> RunToolResult {
if is_js_repl_internal_tool(&req.tool_name) {
let error = "js_repl cannot invoke itself".to_string();
let summary = Self::summarize_tool_call_error(&error);
@@ -1351,15 +1384,6 @@ impl JsReplManager {
.await
{
Ok(response) => {
if let Some(items) = response_content_items(&response) {
Self::record_exec_tool_call_content_items(
&exec_tool_calls,
&req.exec_id,
items,
)
.await;
}
let summary = Self::summarize_tool_call_response(&response);
match serde_json::to_value(response) {
Ok(value) => {
@@ -1432,24 +1456,23 @@ impl JsReplManager {
}
}
fn response_content_items(
response: &ResponseInputItem,
) -> Option<Vec<FunctionCallOutputContentItem>> {
match response {
ResponseInputItem::FunctionCallOutput { output, .. }
| ResponseInputItem::CustomToolCallOutput { output, .. } => output
.content_items()
.map(<[FunctionCallOutputContentItem]>::to_vec),
ResponseInputItem::McpToolCallOutput { result, .. } => match result {
Ok(result) => FunctionCallOutputPayload::from(result)
.content_items()
.map(<[FunctionCallOutputContentItem]>::to_vec),
Err(_) => None,
},
ResponseInputItem::Message { .. } => None,
fn emitted_image_content_item(
turn: &TurnContext,
image_url: String,
detail: Option<ImageDetail>,
) -> FunctionCallOutputContentItem {
FunctionCallOutputContentItem::InputImage {
image_url,
detail: detail.or_else(|| default_output_image_detail_for_turn(turn)),
}
}
fn default_output_image_detail_for_turn(turn: &TurnContext) -> Option<ImageDetail> {
(turn.config.features.enabled(Feature::ImageDetailOriginal)
&& turn.model_info.supports_image_detail_original)
.then_some(ImageDetail::Original)
}
fn build_exec_result_content_items(
output: String,
content_items: Vec<FunctionCallOutputContentItem>,
@@ -1497,6 +1520,7 @@ enum KernelToHost {
error: Option<String>,
},
RunTool(RunToolRequest),
EmitImage(EmitImageRequest),
}
#[derive(Clone, Debug, Serialize)]
@@ -1509,6 +1533,7 @@ enum HostToKernel {
timeout_ms: Option<u64>,
},
RunToolResult(RunToolResult),
EmitImageResult(EmitImageResult),
}
#[derive(Clone, Debug, Deserialize)]
@@ -1529,6 +1554,23 @@ struct RunToolResult {
error: Option<String>,
}
#[derive(Clone, Debug, Deserialize)]
struct EmitImageRequest {
id: String,
exec_id: String,
image_url: String,
#[serde(default)]
detail: Option<ImageDetail>,
}
#[derive(Clone, Debug, Serialize)]
struct EmitImageResult {
id: String,
ok: bool,
#[serde(default)]
error: Option<String>,
}
#[derive(Debug)]
enum ExecResultMessage {
Ok {
@@ -1667,6 +1709,7 @@ mod tests {
use super::*;
use crate::codex::make_session_and_context;
use crate::codex::make_session_and_context_with_dynamic_tools_and_rx;
use crate::features::Feature;
use crate::protocol::AskForApproval;
use crate::protocol::EventMsg;
use crate::protocol::SandboxPolicy;
@@ -1676,6 +1719,7 @@ mod tests {
use codex_protocol::dynamic_tools::DynamicToolSpec;
use codex_protocol::models::FunctionCallOutputContentItem;
use codex_protocol::models::FunctionCallOutputPayload;
use codex_protocol::models::ImageDetail;
use codex_protocol::models::ResponseInputItem;
use codex_protocol::openai_models::InputModality;
use pretty_assertions::assert_eq;
@@ -1923,6 +1967,44 @@ mod tests {
);
}
#[tokio::test]
async fn emitted_image_content_item_preserves_explicit_detail() {
let (_session, turn) = make_session_and_context().await;
let content_item = emitted_image_content_item(
&turn,
"data:image/png;base64,AAA".to_string(),
Some(ImageDetail::Low),
);
assert_eq!(
content_item,
FunctionCallOutputContentItem::InputImage {
image_url: "data:image/png;base64,AAA".to_string(),
detail: Some(ImageDetail::Low),
}
);
}
#[tokio::test]
async fn emitted_image_content_item_uses_turn_original_detail_when_enabled() {
let (_session, mut turn) = make_session_and_context().await;
Arc::make_mut(&mut turn.config)
.features
.enable(Feature::ImageDetailOriginal)
.expect("test config should allow feature update");
turn.model_info.supports_image_detail_original = true;
let content_item =
emitted_image_content_item(&turn, "data:image/png;base64,AAA".to_string(), None);
assert_eq!(
content_item,
FunctionCallOutputContentItem::InputImage {
image_url: "data:image/png;base64,AAA".to_string(),
detail: Some(ImageDetail::Original),
}
);
}
#[test]
fn summarize_tool_call_response_for_multimodal_custom_output() {
let response = ResponseInputItem::CustomToolCallOutput {
@@ -2361,7 +2443,7 @@ console.log("cell-complete");
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn js_repl_can_attach_image_via_view_image_tool() -> anyhow::Result<()> {
async fn js_repl_does_not_auto_attach_image_via_view_image_tool() -> anyhow::Result<()> {
if !can_run_js_repl_runtime_tests().await {
return Ok(());
}
@@ -2398,7 +2480,65 @@ const png = Buffer.from(
await fs.writeFile(imagePath, png);
const out = await codex.tool("view_image", { path: imagePath });
console.log(out.type);
console.log(out.output?.body?.text ?? "");
"#;
let result = manager
.execute(
Arc::clone(&session),
turn,
tracker,
JsReplArgs {
code: code.to_string(),
timeout_ms: Some(15_000),
},
)
.await?;
assert!(result.output.contains("function_call_output"));
assert!(result.content_items.is_empty());
assert!(session.get_pending_input().await.is_empty());
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn js_repl_can_emit_image_via_view_image_tool() -> anyhow::Result<()> {
if !can_run_js_repl_runtime_tests().await {
return Ok(());
}
let (session, mut turn) = make_session_and_context().await;
if !turn
.model_info
.input_modalities
.contains(&InputModality::Image)
{
return Ok(());
}
turn.approval_policy
.set(AskForApproval::Never)
.expect("test setup should allow updating approval policy");
turn.sandbox_policy
.set(SandboxPolicy::DangerFullAccess)
.expect("test setup should allow updating sandbox policy");
let session = Arc::new(session);
let turn = Arc::new(turn);
*session.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default()));
let manager = turn.js_repl.manager().await?;
let code = r#"
const fs = await import("node:fs/promises");
const path = await import("node:path");
const imagePath = path.join(codex.tmpDir, "js-repl-view-image-explicit.png");
const png = Buffer.from(
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==",
"base64"
);
await fs.writeFile(imagePath, png);
const out = await codex.tool("view_image", { path: imagePath });
await codex.emitImage(out);
console.log(out.type);
"#;
let result = manager
@@ -2423,20 +2563,374 @@ console.log(out.output?.body?.text ?? "");
}]
.as_slice()
);
let [FunctionCallOutputContentItem::InputImage { image_url, detail }] =
result.content_items.as_slice()
else {
panic!("view_image should return exactly one input_image content item");
};
assert!(image_url.starts_with("data:image/png;base64,"));
assert_eq!(*detail, None);
assert!(session.get_pending_input().await.is_empty());
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn js_repl_can_attach_image_via_dynamic_tool_with_mixed_content() -> anyhow::Result<()> {
async fn js_repl_can_emit_image_from_bytes_and_mime_type() -> anyhow::Result<()> {
if !can_run_js_repl_runtime_tests().await {
return Ok(());
}
let (session, turn) = make_session_and_context().await;
if !turn
.model_info
.input_modalities
.contains(&InputModality::Image)
{
return Ok(());
}
let session = Arc::new(session);
let turn = Arc::new(turn);
*session.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default()));
let manager = turn.js_repl.manager().await?;
let code = r#"
const png = Buffer.from(
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==",
"base64"
);
await codex.emitImage({ bytes: png, mimeType: "image/png" });
"#;
let result = manager
.execute(
Arc::clone(&session),
turn,
tracker,
JsReplArgs {
code: code.to_string(),
timeout_ms: Some(15_000),
},
)
.await?;
assert_eq!(
result.content_items.as_slice(),
[FunctionCallOutputContentItem::InputImage {
image_url:
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg=="
.to_string(),
detail: None,
}]
.as_slice()
);
assert!(session.get_pending_input().await.is_empty());
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn js_repl_can_emit_multiple_images_in_one_cell() -> anyhow::Result<()> {
if !can_run_js_repl_runtime_tests().await {
return Ok(());
}
let (session, turn) = make_session_and_context().await;
if !turn
.model_info
.input_modalities
.contains(&InputModality::Image)
{
return Ok(());
}
let session = Arc::new(session);
let turn = Arc::new(turn);
*session.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default()));
let manager = turn.js_repl.manager().await?;
let code = r#"
await codex.emitImage(
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg=="
);
await codex.emitImage(
"data:image/gif;base64,R0lGODdhAQABAIAAAP///////ywAAAAAAQABAAACAkQBADs="
);
"#;
let result = manager
.execute(
Arc::clone(&session),
turn,
tracker,
JsReplArgs {
code: code.to_string(),
timeout_ms: Some(15_000),
},
)
.await?;
assert_eq!(
result.content_items.as_slice(),
[
FunctionCallOutputContentItem::InputImage {
image_url:
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg=="
.to_string(),
detail: None,
},
FunctionCallOutputContentItem::InputImage {
image_url:
"data:image/gif;base64,R0lGODdhAQABAIAAAP///////ywAAAAAAQABAAACAkQBADs="
.to_string(),
detail: None,
},
]
.as_slice()
);
assert!(session.get_pending_input().await.is_empty());
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn js_repl_waits_for_unawaited_emit_image_before_completion() -> anyhow::Result<()> {
if !can_run_js_repl_runtime_tests().await {
return Ok(());
}
let (session, turn) = make_session_and_context().await;
if !turn
.model_info
.input_modalities
.contains(&InputModality::Image)
{
return Ok(());
}
let session = Arc::new(session);
let turn = Arc::new(turn);
*session.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default()));
let manager = turn.js_repl.manager().await?;
let code = r#"
void codex.emitImage(
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg=="
);
console.log("cell-complete");
"#;
let result = manager
.execute(
Arc::clone(&session),
turn,
tracker,
JsReplArgs {
code: code.to_string(),
timeout_ms: Some(15_000),
},
)
.await?;
assert!(result.output.contains("cell-complete"));
assert_eq!(
result.content_items.as_slice(),
[FunctionCallOutputContentItem::InputImage {
image_url:
"data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg=="
.to_string(),
detail: None,
}]
.as_slice()
);
assert!(session.get_pending_input().await.is_empty());
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn js_repl_unawaited_emit_image_errors_fail_cell() -> anyhow::Result<()> {
if !can_run_js_repl_runtime_tests().await {
return Ok(());
}
let (session, turn) = make_session_and_context().await;
if !turn
.model_info
.input_modalities
.contains(&InputModality::Image)
{
return Ok(());
}
let session = Arc::new(session);
let turn = Arc::new(turn);
*session.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default()));
let manager = turn.js_repl.manager().await?;
let code = r#"
void codex.emitImage({ bytes: new Uint8Array(), mimeType: "image/png" });
console.log("cell-complete");
"#;
let err = manager
.execute(
Arc::clone(&session),
turn,
tracker,
JsReplArgs {
code: code.to_string(),
timeout_ms: Some(15_000),
},
)
.await
.expect_err("unawaited invalid emitImage should fail");
assert!(err.to_string().contains("expected non-empty bytes"));
assert!(session.get_pending_input().await.is_empty());
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn js_repl_caught_emit_image_error_does_not_fail_cell() -> anyhow::Result<()> {
if !can_run_js_repl_runtime_tests().await {
return Ok(());
}
let (session, turn) = make_session_and_context().await;
if !turn
.model_info
.input_modalities
.contains(&InputModality::Image)
{
return Ok(());
}
let session = Arc::new(session);
let turn = Arc::new(turn);
*session.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default()));
let manager = turn.js_repl.manager().await?;
let code = r#"
try {
await codex.emitImage({ bytes: new Uint8Array(), mimeType: "image/png" });
} catch (error) {
console.log(error.message);
}
console.log("cell-complete");
"#;
let result = manager
.execute(
Arc::clone(&session),
turn,
tracker,
JsReplArgs {
code: code.to_string(),
timeout_ms: Some(15_000),
},
)
.await?;
assert!(result.output.contains("expected non-empty bytes"));
assert!(result.output.contains("cell-complete"));
assert!(result.content_items.is_empty());
assert!(session.get_pending_input().await.is_empty());
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn js_repl_emit_image_requires_explicit_mime_type_for_bytes() -> anyhow::Result<()> {
if !can_run_js_repl_runtime_tests().await {
return Ok(());
}
let (session, turn) = make_session_and_context().await;
if !turn
.model_info
.input_modalities
.contains(&InputModality::Image)
{
return Ok(());
}
let session = Arc::new(session);
let turn = Arc::new(turn);
*session.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default()));
let manager = turn.js_repl.manager().await?;
let code = r#"
const png = Buffer.from(
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==",
"base64"
);
await codex.emitImage({ bytes: png });
"#;
let err = manager
.execute(
Arc::clone(&session),
turn,
tracker,
JsReplArgs {
code: code.to_string(),
timeout_ms: Some(15_000),
},
)
.await
.expect_err("missing mimeType should fail");
assert!(err.to_string().contains("expected a non-empty mimeType"));
assert!(session.get_pending_input().await.is_empty());
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn js_repl_emit_image_rejects_invalid_detail() -> anyhow::Result<()> {
if !can_run_js_repl_runtime_tests().await {
return Ok(());
}
let (session, turn) = make_session_and_context().await;
if !turn
.model_info
.input_modalities
.contains(&InputModality::Image)
{
return Ok(());
}
let session = Arc::new(session);
let turn = Arc::new(turn);
*session.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::default()));
let manager = turn.js_repl.manager().await?;
let code = r#"
const png = Buffer.from(
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==",
"base64"
);
await codex.emitImage({ bytes: png, mimeType: "image/png", detail: "ultra" });
"#;
let err = manager
.execute(
Arc::clone(&session),
turn,
tracker,
JsReplArgs {
code: code.to_string(),
timeout_ms: Some(15_000),
},
)
.await
.expect_err("invalid detail should fail");
assert!(err.to_string().contains("expected detail to be one of"));
assert!(session.get_pending_input().await.is_empty());
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn js_repl_emit_image_rejects_mixed_content() -> anyhow::Result<()> {
if !can_run_js_repl_runtime_tests().await {
return Ok(());
}
@@ -2466,7 +2960,7 @@ console.log(out.output?.body?.text ?? "");
let manager = turn.js_repl.manager().await?;
let code = r#"
const out = await codex.tool("inline_image", {});
console.log(out.type);
await codex.emitImage(out);
"#;
let image_url = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==";
@@ -2509,19 +3003,10 @@ console.log(out.type);
response_watcher,
);
response_watcher_result?;
let result = result?;
assert!(result.output.contains("function_call_output"));
assert_eq!(
result.content_items,
vec![
FunctionCallOutputContentItem::InputText {
text: "inline image note".to_string(),
},
FunctionCallOutputContentItem::InputImage {
image_url: image_url.to_string(),
detail: None,
},
]
let err = result.expect_err("mixed content should fail");
assert!(
err.to_string()
.contains("does not accept mixed text and image content")
);
assert!(session.get_pending_input().await.is_empty());
+15 -6
View File
@@ -23,8 +23,15 @@ pub struct AppsTestServer {
impl AppsTestServer {
pub async fn mount(server: &MockServer) -> Result<Self> {
Self::mount_with_connector_name(server, CONNECTOR_NAME).await
}
pub async fn mount_with_connector_name(
server: &MockServer,
connector_name: &str,
) -> Result<Self> {
mount_oauth_metadata(server).await;
mount_streamable_http_json_rpc(server).await;
mount_streamable_http_json_rpc(server, connector_name.to_string()).await;
Ok(Self {
chatgpt_base_url: server.uri(),
})
@@ -43,15 +50,17 @@ async fn mount_oauth_metadata(server: &MockServer) {
.await;
}
async fn mount_streamable_http_json_rpc(server: &MockServer) {
async fn mount_streamable_http_json_rpc(server: &MockServer, connector_name: String) {
Mock::given(method("POST"))
.and(path_regex("^/api/codex/apps/?$"))
.respond_with(CodexAppsJsonRpcResponder)
.respond_with(CodexAppsJsonRpcResponder { connector_name })
.mount(server)
.await;
}
struct CodexAppsJsonRpcResponder;
struct CodexAppsJsonRpcResponder {
connector_name: String,
}
impl Respond for CodexAppsJsonRpcResponder {
fn respond(&self, request: &Request) -> ResponseTemplate {
@@ -117,7 +126,7 @@ impl Respond for CodexAppsJsonRpcResponder {
},
"_meta": {
"connector_id": CONNECTOR_ID,
"connector_name": CONNECTOR_NAME
"connector_name": self.connector_name.clone()
}
},
{
@@ -133,7 +142,7 @@ impl Respond for CodexAppsJsonRpcResponder {
},
"_meta": {
"connector_id": CONNECTOR_ID,
"connector_name": CONNECTOR_NAME
"connector_name": self.connector_name.clone()
}
}
],
+123
View File
@@ -21,7 +21,9 @@ use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::config_types::Settings;
use codex_protocol::config_types::Verbosity;
use codex_protocol::models::ContentItem;
use codex_protocol::models::FunctionCallOutputContentItem;
use codex_protocol::models::FunctionCallOutputPayload;
use codex_protocol::models::ImageDetail;
use codex_protocol::models::LocalShellAction;
use codex_protocol::models::LocalShellExecAction;
use codex_protocol::models::LocalShellStatus;
@@ -485,6 +487,127 @@ async fn resume_replays_legacy_js_repl_image_rollout_shapes() {
assert!(legacy_image_index < new_user_index);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn resume_replays_image_tool_outputs_with_detail() {
skip_if_no_network!();
let image_url = "data:image/webp;base64,UklGRiIAAABXRUJQVlA4IBYAAAAwAQCdASoBAAEAAUAmJaACdLoB+AADsAD+8ut//NgVzXPv9//S4P0uD9Lg/9KQAAA=";
let function_call_id = "view-image-call";
let custom_call_id = "js-repl-call";
let rollout = vec![
RolloutLine {
timestamp: "2024-01-01T00:00:00.000Z".to_string(),
item: RolloutItem::SessionMeta(SessionMetaLine {
meta: SessionMeta {
id: ThreadId::default(),
timestamp: "2024-01-01T00:00:00Z".to_string(),
cwd: ".".into(),
originator: "test_originator".to_string(),
cli_version: "test_version".to_string(),
model_provider: Some("test-provider".to_string()),
..Default::default()
},
git: None,
}),
},
RolloutLine {
timestamp: "2024-01-01T00:00:01.000Z".to_string(),
item: RolloutItem::ResponseItem(ResponseItem::FunctionCall {
id: None,
name: "view_image".to_string(),
arguments: "{\"path\":\"/tmp/example.webp\"}".to_string(),
call_id: function_call_id.to_string(),
}),
},
RolloutLine {
timestamp: "2024-01-01T00:00:01.500Z".to_string(),
item: RolloutItem::ResponseItem(ResponseItem::FunctionCallOutput {
call_id: function_call_id.to_string(),
output: FunctionCallOutputPayload::from_content_items(vec![
FunctionCallOutputContentItem::InputImage {
image_url: image_url.to_string(),
detail: Some(ImageDetail::Original),
},
]),
}),
},
RolloutLine {
timestamp: "2024-01-01T00:00:02.000Z".to_string(),
item: RolloutItem::ResponseItem(ResponseItem::CustomToolCall {
id: None,
status: Some("completed".to_string()),
call_id: custom_call_id.to_string(),
name: "js_repl".to_string(),
input: "console.log('image flow')".to_string(),
}),
},
RolloutLine {
timestamp: "2024-01-01T00:00:02.500Z".to_string(),
item: RolloutItem::ResponseItem(ResponseItem::CustomToolCallOutput {
call_id: custom_call_id.to_string(),
output: FunctionCallOutputPayload::from_content_items(vec![
FunctionCallOutputContentItem::InputImage {
image_url: image_url.to_string(),
detail: Some(ImageDetail::Original),
},
]),
}),
},
];
let tmpdir = TempDir::new().unwrap();
let session_path = tmpdir
.path()
.join("resume-image-tool-outputs-with-detail.jsonl");
let mut file = std::fs::File::create(&session_path).unwrap();
for line in rollout {
writeln!(file, "{}", serde_json::to_string(&line).unwrap()).unwrap();
}
let server = MockServer::start().await;
let resp_mock = mount_sse_once(
&server,
sse(vec![ev_response_created("resp1"), ev_completed("resp1")]),
)
.await;
let codex_home = Arc::new(TempDir::new().unwrap());
let mut builder = test_codex().with_model("gpt-5.1");
let test = builder
.resume(&server, codex_home, session_path.clone())
.await
.expect("resume conversation");
test.submit_turn("after resume").await.unwrap();
let function_output = resp_mock
.single_request()
.function_call_output(function_call_id);
assert_eq!(
function_output.get("output"),
Some(&serde_json::json!([
{
"type": "input_image",
"image_url": image_url,
"detail": "original"
}
]))
);
let custom_output = resp_mock
.single_request()
.custom_tool_call_output(custom_call_id);
assert_eq!(
custom_output.get("output"),
Some(&serde_json::json!([
{
"type": "input_image",
"image_url": image_url,
"detail": "original"
}
]))
);
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn includes_conversation_id_and_model_headers_in_request() {
skip_if_no_network!();
+136
View File
@@ -7,11 +7,14 @@ use std::time::Instant;
use anyhow::Result;
use codex_core::CodexAuth;
use codex_core::features::Feature;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::Op;
use core_test_support::apps_test_server::AppsTestServer;
use core_test_support::responses::ev_completed;
use core_test_support::responses::ev_response_created;
use core_test_support::responses::mount_sse_once;
use core_test_support::responses::mount_sse_sequence;
use core_test_support::responses::sse;
use core_test_support::responses::start_mock_server;
use core_test_support::skip_if_no_network;
@@ -74,6 +77,35 @@ fn write_plugin_mcp_plugin(home: &TempDir, command: &str) {
.expect("write config");
}
fn write_plugin_app_plugin(home: &TempDir) {
let plugin_root = home.path().join("plugins/sample");
std::fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create plugin manifest dir");
std::fs::write(
plugin_root.join(".codex-plugin/plugin.json"),
r#"{"name":"sample"}"#,
)
.expect("write plugin manifest");
std::fs::write(
plugin_root.join(".app.json"),
r#"{
"apps": {
"calendar": {
"id": "calendar"
}
}
}"#,
)
.expect("write plugin app config");
std::fs::write(
home.path().join("config.toml"),
format!(
"[features]\nplugins = true\n\n[plugins.sample]\nenabled = true\npath = \"{}\"\n",
plugin_root.display()
),
)
.expect("write config");
}
async fn build_plugin_test_codex(
server: &MockServer,
codex_home: Arc<TempDir>,
@@ -88,6 +120,23 @@ async fn build_plugin_test_codex(
.codex)
}
fn tool_names(body: &serde_json::Value) -> Vec<String> {
body.get("tools")
.and_then(serde_json::Value::as_array)
.map(|tools| {
tools
.iter()
.filter_map(|tool| {
tool.get("name")
.or_else(|| tool.get("type"))
.and_then(serde_json::Value::as_str)
.map(str::to_string)
})
.collect()
})
.unwrap_or_default()
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn plugin_skills_append_to_instructions() -> Result<()> {
skip_if_no_network!(Ok(()));
@@ -138,6 +187,93 @@ async fn plugin_skills_append_to_instructions() -> Result<()> {
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn plugin_apps_expose_tools_after_canonical_name_mention() -> Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
let apps_server = AppsTestServer::mount_with_connector_name(&server, "Google Calendar").await?;
let mock = mount_sse_sequence(
&server,
vec![
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
sse(vec![ev_response_created("resp-2"), ev_completed("resp-2")]),
],
)
.await;
let codex_home = Arc::new(TempDir::new()?);
write_plugin_app_plugin(codex_home.as_ref());
#[allow(clippy::expect_used)]
let mut builder = test_codex()
.with_home(codex_home)
.with_auth(CodexAuth::from_api_key("Test API Key"))
.with_config(move |config| {
config
.features
.enable(Feature::Apps)
.expect("test config should allow feature update");
config
.features
.disable(Feature::AppsMcpGateway)
.expect("test config should allow feature update");
config.chatgpt_base_url = apps_server.chatgpt_base_url;
});
let codex = builder
.build(&server)
.await
.expect("create new conversation")
.codex;
codex
.submit(Op::UserInput {
items: vec![codex_protocol::user_input::UserInput::Text {
text: "hello".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
})
.await?;
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
codex
.submit(Op::UserInput {
items: vec![codex_protocol::user_input::UserInput::Text {
text: "Use $google-calendar and then call tools.".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
})
.await?;
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
let requests = mock.requests();
assert_eq!(requests.len(), 2, "expected two model requests");
let first_tools = tool_names(&requests[0].body_json());
assert!(
!first_tools
.iter()
.any(|name| name == "mcp__codex_apps__calendar_create_event"),
"app tools should stay hidden before plugin app mention: {first_tools:?}"
);
let second_tools = tool_names(&requests[1].body_json());
assert!(
second_tools
.iter()
.any(|name| name == "mcp__codex_apps__calendar_create_event"),
"calendar create tool should be available after plugin app mention: {second_tools:?}"
);
assert!(
second_tools
.iter()
.any(|name| name == "mcp__codex_apps__calendar_list_events"),
"calendar list tool should be available after plugin app mention: {second_tools:?}"
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 1)]
async fn plugin_mcp_tools_are_listed() -> Result<()> {
skip_if_no_network!(Ok(()));
+123 -3
View File
@@ -500,7 +500,7 @@ async fn view_image_tool_keeps_legacy_behavior_below_gpt5_3_codex() -> anyhow::R
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn js_repl_view_image_tool_attaches_local_image() -> anyhow::Result<()> {
async fn js_repl_emit_image_attaches_local_image() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
@@ -528,7 +528,7 @@ const png = Buffer.from(
);
await fs.writeFile(imagePath, png);
const out = await codex.tool("view_image", { path: imagePath });
console.log(out.output?.body?.text ?? "");
await codex.emitImage(out);
"#;
let first_response = sse(vec![
@@ -564,12 +564,29 @@ console.log(out.output?.body?.text ?? "");
})
.await?;
let mut tool_event = None;
wait_for_event_with_timeout(
&codex,
|event| matches!(event, EventMsg::TurnComplete(_)),
|event| match event {
EventMsg::ViewImageToolCall(_) => {
tool_event = Some(event.clone());
false
}
EventMsg::TurnComplete(_) => true,
_ => false,
},
Duration::from_secs(10),
)
.await;
let tool_event = match tool_event {
Some(EventMsg::ViewImageToolCall(event)) => event,
other => panic!("expected ViewImageToolCall event, got {other:?}"),
};
assert!(
tool_event.path.ends_with("js-repl-view-image.png"),
"unexpected image path: {}",
tool_event.path.display()
);
let req = mock.single_request();
let body = req.body_json();
@@ -600,6 +617,109 @@ console.log(out.output?.body?.text ?? "");
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn js_repl_view_image_requires_explicit_emit() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
let server = start_mock_server().await;
#[allow(clippy::expect_used)]
let mut builder = test_codex().with_config(|config| {
config
.features
.enable(Feature::JsRepl)
.expect("test config should allow feature update");
});
let TestCodex {
codex,
cwd,
session_configured,
..
} = builder.build(&server).await?;
let call_id = "js-repl-view-image-no-emit";
let js_input = r#"
const fs = await import("node:fs/promises");
const path = await import("node:path");
const imagePath = path.join(codex.tmpDir, "js-repl-view-image-no-emit.png");
const png = Buffer.from(
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4z8DwHwAFAAH/iZk9HQAAAABJRU5ErkJggg==",
"base64"
);
await fs.writeFile(imagePath, png);
const out = await codex.tool("view_image", { path: imagePath });
console.log(out.type);
"#;
let first_response = sse(vec![
ev_response_created("resp-1"),
ev_custom_tool_call(call_id, "js_repl", js_input),
ev_completed("resp-1"),
]);
responses::mount_sse_once(&server, first_response).await;
let second_response = sse(vec![
ev_assistant_message("msg-1", "done"),
ev_completed("resp-2"),
]);
let mock = responses::mount_sse_once(&server, second_response).await;
let session_model = session_configured.model.clone();
codex
.submit(Op::UserTurn {
items: vec![UserInput::Text {
text: "use js_repl to write an image but do not emit it".into(),
text_elements: Vec::new(),
}],
final_output_json_schema: None,
cwd: cwd.path().to_path_buf(),
approval_policy: AskForApproval::Never,
sandbox_policy: SandboxPolicy::DangerFullAccess,
model: session_model,
effort: None,
service_tier: None,
summary: None,
collaboration_mode: None,
personality: None,
})
.await?;
let mut tool_event = None;
wait_for_event_with_timeout(
&codex,
|event| match event {
EventMsg::ViewImageToolCall(_) => {
tool_event = Some(event.clone());
false
}
EventMsg::TurnComplete(_) => true,
_ => false,
},
Duration::from_secs(10),
)
.await;
let tool_event = match tool_event {
Some(EventMsg::ViewImageToolCall(event)) => event,
other => panic!("expected ViewImageToolCall event, got {other:?}"),
};
assert!(
tool_event.path.ends_with("js-repl-view-image-no-emit.png"),
"unexpected image path: {}",
tool_event.path.display()
);
let req = mock.single_request();
let custom_output = req.custom_tool_call_output(call_id);
let output_items = custom_output.get("output").and_then(Value::as_array);
assert!(
output_items.is_none_or(|items| items
.iter()
.all(|item| item.get("type").and_then(Value::as_str) != Some("input_image"))),
"nested view_image should not auto-populate js_repl output"
);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn view_image_tool_errors_when_path_is_directory() -> anyhow::Result<()> {
skip_if_no_network!(Ok(()));
+4
View File
@@ -21,6 +21,7 @@ clap = { workspace = true, features = ["derive"] }
codex-arg0 = { workspace = true }
codex-cloud-requirements = { workspace = true }
codex-core = { workspace = true }
codex-otel = { workspace = true }
codex-protocol = { workspace = true }
codex-utils-absolute-path = { workspace = true }
codex-utils-cli = { workspace = true }
@@ -55,10 +56,13 @@ codex-apply-patch = { workspace = true }
codex-utils-cargo-bin = { workspace = true }
core_test_support = { workspace = true }
libc = { workspace = true }
opentelemetry = { workspace = true }
opentelemetry_sdk = { workspace = true }
predicates = { workspace = true }
pretty_assertions = { workspace = true }
rmcp = { workspace = true }
tempfile = { workspace = true }
tracing-opentelemetry = { workspace = true }
uuid = { workspace = true }
walkdir = { workspace = true }
wiremock = { workspace = true }
+109 -1
View File
@@ -34,6 +34,8 @@ use codex_core::format_exec_policy_error_with_source;
use codex_core::git_info::get_git_repo_root;
use codex_core::models_manager::collaboration_mode_presets::CollaborationModesConfig;
use codex_core::models_manager::manager::RefreshStrategy;
use codex_otel::set_parent_from_context;
use codex_otel::traceparent_context_from_env;
use codex_protocol::approvals::ElicitationAction;
use codex_protocol::config_types::SandboxMode;
use codex_protocol::protocol::AskForApproval;
@@ -58,9 +60,12 @@ use std::path::PathBuf;
use std::sync::Arc;
use supports_color::Stream;
use tokio::sync::Mutex;
use tracing::Instrument;
use tracing::debug;
use tracing::error;
use tracing::field;
use tracing::info;
use tracing::info_span;
use tracing::warn;
use tracing_subscriber::EnvFilter;
use tracing_subscriber::prelude::*;
@@ -94,6 +99,32 @@ struct ThreadEventEnvelope {
suppress_output: bool,
}
struct ExecRunArgs {
command: Option<ExecCommand>,
config: Config,
cursor_ansi: bool,
dangerously_bypass_approvals_and_sandbox: bool,
exec_span: tracing::Span,
images: Vec<PathBuf>,
json_mode: bool,
last_message_file: Option<PathBuf>,
model_provider: Option<String>,
oss: bool,
output_schema_path: Option<PathBuf>,
prompt: Option<String>,
skip_git_repo_check: bool,
stderr_with_ansi: bool,
}
fn exec_root_span() -> tracing::Span {
info_span!(
"codex.exec",
otel.kind = "internal",
thread.id = field::Empty,
turn.id = field::Empty,
)
}
pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> {
if let Err(err) = set_default_originator("codex_exec".to_string()) {
tracing::warn!(?err, "Failed to set codex exec originator override {err:?}");
@@ -347,6 +378,48 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
.with(otel_logger_layer)
.try_init();
let exec_span = exec_root_span();
if let Some(context) = traceparent_context_from_env() {
set_parent_from_context(&exec_span, context);
}
run_exec_session(ExecRunArgs {
command,
config,
cursor_ansi,
dangerously_bypass_approvals_and_sandbox,
exec_span: exec_span.clone(),
images,
json_mode,
last_message_file,
model_provider,
oss,
output_schema_path,
prompt,
skip_git_repo_check,
stderr_with_ansi,
})
.instrument(exec_span)
.await
}
async fn run_exec_session(args: ExecRunArgs) -> anyhow::Result<()> {
let ExecRunArgs {
command,
config,
cursor_ansi,
dangerously_bypass_approvals_and_sandbox,
exec_span,
images,
json_mode,
last_message_file,
model_provider,
oss,
output_schema_path,
prompt,
skip_git_repo_check,
stderr_with_ansi,
} = args;
let mut event_processor: Box<dyn EventProcessor> = match json_mode {
true => Box::new(EventProcessorWithJsonOutput::new(last_message_file.clone())),
_ => Box::new(EventProcessorWithHumanOutput::create_with_ansi(
@@ -435,6 +508,9 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
} else {
thread_manager.start_thread(config.clone()).await?
};
let primary_thread_id_for_span = primary_thread_id.to_string();
exec_span.record("thread.id", primary_thread_id_for_span.as_str());
let (initial_operation, prompt_summary) = match (command, prompt, images) {
(Some(ExecCommand::Review(review_cli)), _, _) => {
let review_request = build_review_request(review_cli)?;
@@ -554,7 +630,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
});
}
match initial_operation {
let task_id = match initial_operation {
InitialOperation::UserTurn {
items,
output_schema,
@@ -583,6 +659,7 @@ pub async fn run_main(cli: Cli, arg0_paths: Arg0DispatchPaths) -> anyhow::Result
task_id
}
};
exec_span.record("turn.id", task_id.as_str());
// Run the loop until the task is complete.
// Track whether a fatal error was reported by the server so we can
@@ -934,13 +1011,44 @@ fn build_review_request(args: ReviewArgs) -> anyhow::Result<ReviewRequest> {
#[cfg(test)]
mod tests {
use super::*;
use codex_otel::set_parent_from_w3c_trace_context;
use opentelemetry::trace::TraceContextExt;
use opentelemetry::trace::TraceId;
use opentelemetry::trace::TracerProvider as _;
use opentelemetry_sdk::trace::SdkTracerProvider;
use pretty_assertions::assert_eq;
use tracing_opentelemetry::OpenTelemetrySpanExt;
fn test_tracing_subscriber() -> impl tracing::Subscriber + Send + Sync {
let provider = SdkTracerProvider::builder().build();
let tracer = provider.tracer("codex-exec-tests");
tracing_subscriber::registry().with(tracing_opentelemetry::layer().with_tracer(tracer))
}
#[test]
fn exec_defaults_analytics_to_enabled() {
assert_eq!(DEFAULT_ANALYTICS_ENABLED, true);
}
#[test]
fn exec_root_span_can_be_parented_from_trace_context() {
let subscriber = test_tracing_subscriber();
let _guard = tracing::subscriber::set_default(subscriber);
let parent = codex_protocol::protocol::W3cTraceContext {
traceparent: Some("00-00000000000000000000000000000077-0000000000000088-01".into()),
tracestate: Some("vendor=value".into()),
};
let exec_span = exec_root_span();
assert!(set_parent_from_w3c_trace_context(&exec_span, &parent));
let trace_id = exec_span.context().span().span_context().trace_id();
assert_eq!(
trace_id,
TraceId::from_hex("00000000000000000000000000000077").expect("trace id")
);
}
#[test]
fn builds_uncommitted_review_request() {
let request = build_review_request(ReviewArgs {
+1
View File
@@ -10,6 +10,7 @@ codex-protocol = { workspace = true }
sentry = { version = "0.46" }
tracing = { workspace = true }
tracing-subscriber = { workspace = true }
url = { workspace = true }
[dev-dependencies]
pretty_assertions = { workspace = true }
@@ -0,0 +1,247 @@
use std::collections::HashMap;
use url::Url;
const DEFAULT_OPENAI_BASE_URL: &str = "https://api.openai.com/v1";
const OPENAI_BASE_URL_ENV_VAR: &str = "OPENAI_BASE_URL";
pub const FEEDBACK_DIAGNOSTICS_ATTACHMENT_FILENAME: &str = "codex-connectivity-diagnostics.txt";
const PROXY_ENV_VARS: &[&str] = &[
"HTTP_PROXY",
"http_proxy",
"HTTPS_PROXY",
"https_proxy",
"ALL_PROXY",
"all_proxy",
];
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct FeedbackDiagnostics {
diagnostics: Vec<FeedbackDiagnostic>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct FeedbackDiagnostic {
pub headline: String,
pub details: Vec<String>,
}
impl FeedbackDiagnostics {
pub fn new(diagnostics: Vec<FeedbackDiagnostic>) -> Self {
Self { diagnostics }
}
pub fn collect_from_env() -> Self {
Self::collect_from_pairs(std::env::vars())
}
fn collect_from_pairs<I, K, V>(pairs: I) -> Self
where
I: IntoIterator<Item = (K, V)>,
K: Into<String>,
V: Into<String>,
{
let env = pairs
.into_iter()
.map(|(key, value)| (key.into(), value.into()))
.collect::<HashMap<_, _>>();
let mut diagnostics = Vec::new();
let proxy_details = PROXY_ENV_VARS
.iter()
.filter_map(|key| {
let value = env.get(*key)?.trim();
if value.is_empty() {
return None;
}
let detail = match sanitize_proxy_value(value) {
Some(sanitized) => format!("{key} = {sanitized}"),
None => format!("{key} = invalid value"),
};
Some(detail)
})
.collect::<Vec<_>>();
if !proxy_details.is_empty() {
diagnostics.push(FeedbackDiagnostic {
headline: "Proxy environment variables are set and may affect connectivity."
.to_string(),
details: proxy_details,
});
}
if let Some(value) = env.get(OPENAI_BASE_URL_ENV_VAR).map(String::as_str) {
let trimmed = value.trim();
if !trimmed.is_empty() && trimmed.trim_end_matches('/') != DEFAULT_OPENAI_BASE_URL {
let detail = match sanitize_url_for_display(trimmed) {
Some(sanitized) => format!("{OPENAI_BASE_URL_ENV_VAR} = {sanitized}"),
None => format!("{OPENAI_BASE_URL_ENV_VAR} = invalid value"),
};
diagnostics.push(FeedbackDiagnostic {
headline: "OPENAI_BASE_URL is set and may affect connectivity.".to_string(),
details: vec![detail],
});
}
}
Self { diagnostics }
}
pub fn is_empty(&self) -> bool {
self.diagnostics.is_empty()
}
pub fn diagnostics(&self) -> &[FeedbackDiagnostic] {
&self.diagnostics
}
pub fn attachment_text(&self) -> Option<String> {
if self.diagnostics.is_empty() {
return None;
}
let mut lines = vec!["Connectivity diagnostics".to_string(), String::new()];
for diagnostic in &self.diagnostics {
lines.push(format!("- {}", diagnostic.headline));
lines.extend(
diagnostic
.details
.iter()
.map(|detail| format!(" - {detail}")),
);
}
Some(lines.join("\n"))
}
}
pub fn sanitize_url_for_display(raw: &str) -> Option<String> {
let trimmed = raw.trim();
if trimmed.is_empty() {
return None;
}
let Ok(mut url) = Url::parse(trimmed) else {
return None;
};
let _ = url.set_username("");
let _ = url.set_password(None);
url.set_query(None);
url.set_fragment(None);
Some(url.to_string().trim_end_matches('/').to_string()).filter(|value| !value.is_empty())
}
fn sanitize_proxy_value(raw: &str) -> Option<String> {
if raw.contains("://") {
return sanitize_url_for_display(raw);
}
sanitize_url_for_display(&format!("http://{raw}"))
}
#[cfg(test)]
mod tests {
use pretty_assertions::assert_eq;
use super::FeedbackDiagnostic;
use super::FeedbackDiagnostics;
use super::sanitize_url_for_display;
#[test]
fn collect_from_pairs_reports_sanitized_diagnostics_and_attachment() {
let diagnostics = FeedbackDiagnostics::collect_from_pairs([
(
"HTTPS_PROXY",
"https://user:password@secure-proxy.example.com:443?secret=1",
),
("http_proxy", "proxy.example.com:8080"),
("all_proxy", "socks5h://all-proxy.example.com:1080"),
("OPENAI_BASE_URL", "https://example.com/v1?token=secret"),
]);
assert_eq!(
diagnostics,
FeedbackDiagnostics {
diagnostics: vec![
FeedbackDiagnostic {
headline:
"Proxy environment variables are set and may affect connectivity."
.to_string(),
details: vec![
"http_proxy = http://proxy.example.com:8080".to_string(),
"HTTPS_PROXY = https://secure-proxy.example.com".to_string(),
"all_proxy = socks5h://all-proxy.example.com:1080".to_string(),
],
},
FeedbackDiagnostic {
headline: "OPENAI_BASE_URL is set and may affect connectivity.".to_string(),
details: vec!["OPENAI_BASE_URL = https://example.com/v1".to_string()],
},
],
}
);
assert_eq!(
diagnostics.attachment_text(),
Some(
"Connectivity diagnostics\n\n- Proxy environment variables are set and may affect connectivity.\n - http_proxy = http://proxy.example.com:8080\n - HTTPS_PROXY = https://secure-proxy.example.com\n - all_proxy = socks5h://all-proxy.example.com:1080\n- OPENAI_BASE_URL is set and may affect connectivity.\n - OPENAI_BASE_URL = https://example.com/v1"
.to_string()
)
);
}
#[test]
fn collect_from_pairs_ignores_absent_and_default_values() {
for diagnostics in [
FeedbackDiagnostics::collect_from_pairs(Vec::<(String, String)>::new()),
FeedbackDiagnostics::collect_from_pairs([(
"OPENAI_BASE_URL",
"https://api.openai.com/v1/",
)]),
] {
assert_eq!(diagnostics, FeedbackDiagnostics::default());
assert_eq!(diagnostics.attachment_text(), None);
}
}
#[test]
fn collect_from_pairs_reports_invalid_values_without_echoing_them() {
let invalid_proxy = "not a valid\nproxy";
let invalid_base_url = "not a valid\nurl";
let diagnostics = FeedbackDiagnostics::collect_from_pairs([
("HTTP_PROXY", invalid_proxy),
("OPENAI_BASE_URL", invalid_base_url),
]);
assert_eq!(
diagnostics,
FeedbackDiagnostics {
diagnostics: vec![
FeedbackDiagnostic {
headline:
"Proxy environment variables are set and may affect connectivity."
.to_string(),
details: vec!["HTTP_PROXY = invalid value".to_string()],
},
FeedbackDiagnostic {
headline: "OPENAI_BASE_URL is set and may affect connectivity.".to_string(),
details: vec!["OPENAI_BASE_URL = invalid value".to_string()],
},
],
}
);
let attachment_text = diagnostics
.attachment_text()
.expect("invalid diagnostics should still render attachment text");
assert!(!attachment_text.contains(invalid_proxy));
assert!(!attachment_text.contains(invalid_base_url));
}
#[test]
fn sanitize_url_for_display_strips_credentials_query_and_fragment() {
let sanitized = sanitize_url_for_display(
"https://user:password@example.com:8443/v1?token=secret#fragment",
);
assert_eq!(sanitized, Some("https://example.com:8443/v1".to_string()));
}
}
+127 -19
View File
@@ -13,6 +13,8 @@ use anyhow::Result;
use anyhow::anyhow;
use codex_protocol::ThreadId;
use codex_protocol::protocol::SessionSource;
use feedback_diagnostics::FEEDBACK_DIAGNOSTICS_ATTACHMENT_FILENAME;
use feedback_diagnostics::FeedbackDiagnostics;
use tracing::Event;
use tracing::Level;
use tracing::field::Visit;
@@ -21,6 +23,8 @@ use tracing_subscriber::filter::Targets;
use tracing_subscriber::fmt::writer::MakeWriter;
use tracing_subscriber::registry::LookupSpan;
pub mod feedback_diagnostics;
const DEFAULT_MAX_BYTES: usize = 4 * 1024 * 1024; // 4 MiB
const SENTRY_DSN: &str =
"https://ae32ed50620d7a7792c1ce5df38b3e3e@o33249.ingest.us.sentry.io/4510195390611458";
@@ -88,7 +92,7 @@ impl CodexFeedback {
.with_filter(Targets::new().with_target(FEEDBACK_TAGS_TARGET, Level::TRACE))
}
pub fn snapshot(&self, session_id: Option<ThreadId>) -> CodexLogSnapshot {
pub fn snapshot(&self, session_id: Option<ThreadId>) -> FeedbackSnapshot {
let bytes = {
let guard = self.inner.ring.lock().expect("mutex poisoned");
guard.snapshot_bytes()
@@ -97,9 +101,10 @@ impl CodexFeedback {
let guard = self.inner.tags.lock().expect("mutex poisoned");
guard.clone()
};
CodexLogSnapshot {
FeedbackSnapshot {
bytes,
tags,
feedback_diagnostics: FeedbackDiagnostics::collect_from_env(),
thread_id: session_id
.map(|id| id.to_string())
.unwrap_or("no-active-thread-".to_string() + &ThreadId::new().to_string()),
@@ -199,17 +204,35 @@ impl RingBuffer {
}
}
pub struct CodexLogSnapshot {
pub struct FeedbackSnapshot {
bytes: Vec<u8>,
tags: BTreeMap<String, String>,
feedback_diagnostics: FeedbackDiagnostics,
pub thread_id: String,
}
impl CodexLogSnapshot {
impl FeedbackSnapshot {
pub(crate) fn as_bytes(&self) -> &[u8] {
&self.bytes
}
pub fn feedback_diagnostics(&self) -> &FeedbackDiagnostics {
&self.feedback_diagnostics
}
pub fn with_feedback_diagnostics(mut self, feedback_diagnostics: FeedbackDiagnostics) -> Self {
self.feedback_diagnostics = feedback_diagnostics;
self
}
pub fn feedback_diagnostics_attachment_text(&self, include_logs: bool) -> Option<String> {
if !include_logs {
return None;
}
self.feedback_diagnostics.attachment_text()
}
pub fn save_to_temp_file(&self) -> io::Result<PathBuf> {
let dir = std::env::temp_dir();
let filename = format!("codex-feedback-{}.log", self.thread_id);
@@ -224,18 +247,16 @@ impl CodexLogSnapshot {
classification: &str,
reason: Option<&str>,
include_logs: bool,
extra_log_files: &[PathBuf],
extra_attachment_paths: &[PathBuf],
session_source: Option<SessionSource>,
logs_override: Option<Vec<u8>>,
) -> Result<()> {
use std::collections::BTreeMap;
use std::fs;
use std::str::FromStr;
use std::sync::Arc;
use sentry::Client;
use sentry::ClientOptions;
use sentry::protocol::Attachment;
use sentry::protocol::Envelope;
use sentry::protocol::EnvelopeItem;
use sentry::protocol::Event;
@@ -309,16 +330,46 @@ impl CodexLogSnapshot {
}
envelope.add_item(EnvelopeItem::Event(event));
for attachment in
self.feedback_attachments(include_logs, extra_attachment_paths, logs_override)
{
envelope.add_item(EnvelopeItem::Attachment(attachment));
}
client.send_envelope(envelope);
client.flush(Some(Duration::from_secs(UPLOAD_TIMEOUT_SECS)));
Ok(())
}
fn feedback_attachments(
&self,
include_logs: bool,
extra_attachment_paths: &[PathBuf],
logs_override: Option<Vec<u8>>,
) -> Vec<sentry::protocol::Attachment> {
use sentry::protocol::Attachment;
let mut attachments = Vec::new();
if include_logs {
envelope.add_item(EnvelopeItem::Attachment(Attachment {
attachments.push(Attachment {
buffer: logs_override.unwrap_or_else(|| self.bytes.clone()),
filename: String::from("codex-logs.log"),
content_type: Some("text/plain".to_string()),
ty: None,
}));
});
}
for path in extra_log_files {
if let Some(text) = self.feedback_diagnostics_attachment_text(include_logs) {
attachments.push(Attachment {
buffer: text.into_bytes(),
filename: FEEDBACK_DIAGNOSTICS_ATTACHMENT_FILENAME.to_string(),
content_type: Some("text/plain".to_string()),
ty: None,
});
}
for path in extra_attachment_paths {
let data = match fs::read(path) {
Ok(data) => data,
Err(err) => {
@@ -330,22 +381,19 @@ impl CodexLogSnapshot {
continue;
}
};
let fname = path
let filename = path
.file_name()
.map(|s| s.to_string_lossy().to_string())
.unwrap_or_else(|| "extra-log.log".to_string());
let content_type = "text/plain".to_string();
envelope.add_item(EnvelopeItem::Attachment(Attachment {
attachments.push(Attachment {
buffer: data,
filename: fname,
content_type: Some(content_type),
filename,
content_type: Some("text/plain".to_string()),
ty: None,
}));
});
}
client.send_envelope(envelope);
client.flush(Some(Duration::from_secs(UPLOAD_TIMEOUT_SECS)));
Ok(())
attachments
}
}
@@ -430,7 +478,12 @@ impl Visit for FeedbackTagsVisitor {
#[cfg(test)]
mod tests {
use std::ffi::OsStr;
use std::fs;
use super::*;
use feedback_diagnostics::FeedbackDiagnostic;
use pretty_assertions::assert_eq;
use tracing_subscriber::layer::SubscriberExt;
use tracing_subscriber::util::SubscriberInitExt;
@@ -460,4 +513,59 @@ mod tests {
pretty_assertions::assert_eq!(snap.tags.get("model").map(String::as_str), Some("gpt-5"));
pretty_assertions::assert_eq!(snap.tags.get("cached").map(String::as_str), Some("true"));
}
#[test]
fn feedback_attachments_gate_connectivity_diagnostics() {
let extra_filename = format!("codex-feedback-extra-{}.jsonl", ThreadId::new());
let extra_path = std::env::temp_dir().join(&extra_filename);
fs::write(&extra_path, "rollout").expect("extra attachment should be written");
let snapshot_with_diagnostics = CodexFeedback::new()
.snapshot(None)
.with_feedback_diagnostics(FeedbackDiagnostics::new(vec![FeedbackDiagnostic {
headline: "OPENAI_BASE_URL is set and may affect connectivity.".to_string(),
details: vec!["OPENAI_BASE_URL = https://example.com/v1".to_string()],
}]));
let attachments_with_diagnostics = snapshot_with_diagnostics.feedback_attachments(
true,
std::slice::from_ref(&extra_path),
Some(vec![1]),
);
assert_eq!(
attachments_with_diagnostics
.iter()
.map(|attachment| attachment.filename.as_str())
.collect::<Vec<_>>(),
vec![
"codex-logs.log",
FEEDBACK_DIAGNOSTICS_ATTACHMENT_FILENAME,
extra_filename.as_str()
]
);
assert_eq!(attachments_with_diagnostics[0].buffer, vec![1]);
assert_eq!(
attachments_with_diagnostics[1].buffer,
b"Connectivity diagnostics\n\n- OPENAI_BASE_URL is set and may affect connectivity.\n - OPENAI_BASE_URL = https://example.com/v1".to_vec()
);
assert_eq!(attachments_with_diagnostics[2].buffer, b"rollout".to_vec());
assert_eq!(
OsStr::new(attachments_with_diagnostics[2].filename.as_str()),
OsStr::new(extra_filename.as_str())
);
let attachments_without_diagnostics = CodexFeedback::new()
.snapshot(None)
.feedback_attachments(true, &[], Some(vec![1]));
assert_eq!(
attachments_without_diagnostics
.iter()
.map(|attachment| attachment.filename.as_str())
.collect::<Vec<_>>(),
vec!["codex-logs.log"]
);
assert_eq!(attachments_without_diagnostics[0].buffer, vec![1]);
fs::remove_file(extra_path).expect("extra attachment should be removed");
}
}
@@ -115,6 +115,7 @@ pub async fn run_codex_tool_session(
}],
final_output_json_schema: None,
},
trace: None,
};
if let Err(e) = thread.submit_with_id(submission).await {
@@ -576,6 +576,7 @@ impl MessageProcessor {
.submit_with_id(Submission {
id: request_id_string,
op: codex_protocol::protocol::Op::Interrupt,
trace: None,
})
.await
{
-22
View File
@@ -7,7 +7,6 @@ use crate::trace_context::context_from_trace_headers;
use gethostname::gethostname;
use opentelemetry::Context;
use opentelemetry::KeyValue;
use opentelemetry::context::ContextGuard;
use opentelemetry::global;
use opentelemetry::trace::TracerProvider as _;
use opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge;
@@ -28,7 +27,6 @@ use opentelemetry_sdk::trace::BatchSpanProcessor;
use opentelemetry_sdk::trace::SdkTracerProvider;
use opentelemetry_sdk::trace::Tracer;
use opentelemetry_semantic_conventions as semconv;
use std::cell::RefCell;
use std::env;
use std::error::Error;
use std::sync::OnceLock;
@@ -43,10 +41,6 @@ const HOST_NAME_ATTRIBUTE: &str = "host.name";
const TRACEPARENT_ENV_VAR: &str = "TRACEPARENT";
const TRACESTATE_ENV_VAR: &str = "TRACESTATE";
static TRACEPARENT_CONTEXT: OnceLock<Option<Context>> = OnceLock::new();
thread_local! {
static TRACEPARENT_GUARD: RefCell<Option<ContextGuard>> = const { RefCell::new(None) };
}
pub struct OtelProvider {
pub logger: Option<SdkLoggerProvider>,
pub tracer_provider: Option<SdkTracerProvider>,
@@ -113,10 +107,6 @@ impl OtelProvider {
global::set_tracer_provider(provider);
global::set_text_map_propagator(TraceContextPropagator::new());
}
if tracer.is_some() {
attach_traceparent_context();
}
Ok(Some(Self {
logger,
tracer_provider,
@@ -176,18 +166,6 @@ pub fn traceparent_context_from_env() -> Option<Context> {
.clone()
}
fn attach_traceparent_context() {
TRACEPARENT_GUARD.with(|guard| {
let mut guard = guard.borrow_mut();
if guard.is_some() {
return;
}
if let Some(context) = traceparent_context_from_env() {
*guard = Some(context.attach());
}
});
}
fn load_traceparent_context() -> Option<Context> {
let traceparent = env::var(TRACEPARENT_ENV_VAR).ok()?;
let tracestate = env::var(TRACESTATE_ENV_VAR).ok();
-8
View File
@@ -15,7 +15,6 @@ use crate::metrics::names::WEBSOCKET_EVENT_COUNT_METRIC;
use crate::metrics::names::WEBSOCKET_EVENT_DURATION_METRIC;
use crate::metrics::names::WEBSOCKET_REQUEST_COUNT_METRIC;
use crate::metrics::names::WEBSOCKET_REQUEST_DURATION_METRIC;
use crate::otel_provider::traceparent_context_from_env;
use crate::sanitize_metric_tag_value;
use chrono::SecondsFormat;
use chrono::Utc;
@@ -41,7 +40,6 @@ use std::time::Duration;
use std::time::Instant;
use tokio::time::error::Elapsed;
use tracing::Span;
use tracing_opentelemetry::OpenTelemetrySpanExt;
pub use crate::OtelEventMetadata;
pub use crate::OtelManager;
@@ -92,12 +90,6 @@ impl OtelManager {
}
}
pub fn apply_traceparent_parent(&self, span: &Span) {
if let Some(context) = traceparent_context_from_env() {
let _ = span.set_parent(context);
}
}
pub fn record_responses(&self, handle_responses_span: &Span, event: &ResponseEvent) {
handle_responses_span.record("otel.name", OtelManager::responses_type(event));
+3
View File
@@ -82,6 +82,9 @@ pub struct Submission {
pub id: String,
/// Payload
pub op: Op,
/// Optional W3C trace carrier propagated across async submission handoffs.
#[serde(default, skip_serializing_if = "Option::is_none")]
pub trace: Option<W3cTraceContext>,
}
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)]
+143 -19
View File
@@ -1,6 +1,8 @@
use std::cell::RefCell;
use std::path::PathBuf;
use codex_feedback::feedback_diagnostics::FEEDBACK_DIAGNOSTICS_ATTACHMENT_FILENAME;
use codex_feedback::feedback_diagnostics::FeedbackDiagnostics;
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyModifiers;
@@ -19,6 +21,8 @@ use crate::app_event::FeedbackCategory;
use crate::app_event_sender::AppEventSender;
use crate::history_cell;
use crate::render::renderable::Renderable;
use crate::wrapping::RtOptions;
use crate::wrapping::word_wrap_lines;
use codex_protocol::protocol::SessionSource;
use super::CancellationEvent;
@@ -46,7 +50,7 @@ pub(crate) enum FeedbackAudience {
/// both logs and rollout with classification + metadata.
pub(crate) struct FeedbackNoteView {
category: FeedbackCategory,
snapshot: codex_feedback::CodexLogSnapshot,
snapshot: codex_feedback::FeedbackSnapshot,
rollout_path: Option<PathBuf>,
app_event_tx: AppEventSender,
include_logs: bool,
@@ -61,7 +65,7 @@ pub(crate) struct FeedbackNoteView {
impl FeedbackNoteView {
pub(crate) fn new(
category: FeedbackCategory,
snapshot: codex_feedback::CodexLogSnapshot,
snapshot: codex_feedback::FeedbackSnapshot,
rollout_path: Option<PathBuf>,
app_event_tx: AppEventSender,
include_logs: bool,
@@ -87,7 +91,7 @@ impl FeedbackNoteView {
} else {
Some(note.as_str())
};
let log_file_paths = if self.include_logs {
let attachment_paths = if self.include_logs {
self.rollout_path.iter().cloned().collect::<Vec<_>>()
} else {
Vec::new()
@@ -100,7 +104,7 @@ impl FeedbackNoteView {
classification,
reason_opt,
self.include_logs,
&log_file_paths,
&attachment_paths,
Some(SessionSource::Cli),
None,
);
@@ -217,21 +221,21 @@ impl BottomPaneView for FeedbackNoteView {
impl Renderable for FeedbackNoteView {
fn desired_height(&self, width: u16) -> u16 {
1u16 + self.input_height(width) + 3u16
self.intro_lines(width).len() as u16 + self.input_height(width) + 2u16
}
fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> {
if area.height < 2 || area.width <= 2 {
return None;
}
let intro_height = self.intro_lines(area.width).len() as u16;
let text_area_height = self.input_height(area.width).saturating_sub(1);
if text_area_height == 0 {
return None;
}
let top_line_count = 1u16; // title only
let textarea_rect = Rect {
x: area.x.saturating_add(2),
y: area.y.saturating_add(top_line_count).saturating_add(1),
y: area.y.saturating_add(intro_height).saturating_add(1),
width: area.width.saturating_sub(2),
height: text_area_height,
};
@@ -244,23 +248,26 @@ impl Renderable for FeedbackNoteView {
return;
}
let (title, placeholder) = feedback_title_and_placeholder(self.category);
let intro_lines = self.intro_lines(area.width);
let (_, placeholder) = feedback_title_and_placeholder(self.category);
let input_height = self.input_height(area.width);
// Title line
let title_area = Rect {
x: area.x,
y: area.y,
width: area.width,
height: 1,
};
let title_spans: Vec<Span<'static>> = vec![gutter(), title.bold()];
Paragraph::new(Line::from(title_spans)).render(title_area, buf);
for (offset, line) in intro_lines.iter().enumerate() {
Paragraph::new(line.clone()).render(
Rect {
x: area.x,
y: area.y.saturating_add(offset as u16),
width: area.width,
height: 1,
},
buf,
);
}
// Input line
let input_area = Rect {
x: area.x,
y: area.y.saturating_add(1),
y: area.y.saturating_add(intro_lines.len() as u16),
width: area.width,
height: input_height,
};
@@ -334,6 +341,56 @@ impl FeedbackNoteView {
let text_height = self.textarea.desired_height(usable_width).clamp(1, 8);
text_height.saturating_add(1).min(9)
}
fn intro_lines(&self, width: u16) -> Vec<Line<'static>> {
let (title, _) = feedback_title_and_placeholder(self.category);
let mut lines = vec![Line::from(vec![gutter(), title.bold()])];
if should_show_feedback_connectivity_details(
self.category,
self.snapshot.feedback_diagnostics(),
) {
lines.push(Line::from(vec![gutter()]));
lines.push(Line::from(vec![
gutter(),
"Connectivity diagnostics".bold(),
]));
lines.extend(self.diagnostics_lines(width));
}
lines
}
fn diagnostics_lines(&self, width: u16) -> Vec<Line<'static>> {
let width = usize::from(width.max(1));
let headline_options = RtOptions::new(width)
.initial_indent(Line::from(vec![gutter(), " - ".into()]))
.subsequent_indent(Line::from(vec![gutter(), " ".into()]));
let detail_options = RtOptions::new(width)
.initial_indent(Line::from(vec![gutter(), " - ".dim()]))
.subsequent_indent(Line::from(vec![gutter(), " ".into()]));
let mut lines = Vec::new();
for diagnostic in self.snapshot.feedback_diagnostics().diagnostics() {
lines.extend(word_wrap_lines(
[Line::from(diagnostic.headline.clone())],
headline_options.clone(),
));
for detail in &diagnostic.details {
lines.extend(word_wrap_lines(
[Line::from(detail.clone())],
detail_options.clone(),
));
}
}
lines
}
}
pub(crate) fn should_show_feedback_connectivity_details(
category: FeedbackCategory,
diagnostics: &FeedbackDiagnostics,
) -> bool {
category != FeedbackCategory::GoodResult && !diagnostics.is_empty()
}
fn gutter() -> Span<'static> {
@@ -485,6 +542,7 @@ pub(crate) fn feedback_upload_consent_params(
app_event_tx: AppEventSender,
category: FeedbackCategory,
rollout_path: Option<std::path::PathBuf>,
include_connectivity_diagnostics_attachment: bool,
) -> super::SelectionViewParams {
use super::popup_consts::standard_popup_hint_line;
let yes_action: super::SelectionAction = Box::new({
@@ -521,6 +579,15 @@ pub(crate) fn feedback_upload_consent_params(
{
header_lines.push(Line::from(vec![" • ".into(), name.into()]).into());
}
if include_connectivity_diagnostics_attachment {
header_lines.push(
Line::from(vec![
" • ".into(),
FEEDBACK_DIAGNOSTICS_ATTACHMENT_FILENAME.into(),
])
.into(),
);
}
super::SelectionViewParams {
footer_hint: Some(standard_popup_hint_line()),
@@ -537,7 +604,6 @@ pub(crate) fn feedback_upload_consent_params(
},
super::SelectionItem {
name: "No".to_string(),
description: Some("".to_string()),
actions: vec![no_action],
dismiss_on_select: true,
..Default::default()
@@ -555,6 +621,8 @@ mod tests {
use super::*;
use crate::app_event::AppEvent;
use crate::app_event_sender::AppEventSender;
use codex_feedback::feedback_diagnostics::FeedbackDiagnostic;
use pretty_assertions::assert_eq;
fn render(view: &FeedbackNoteView, width: u16) -> String {
let height = view.desired_height(width);
@@ -635,6 +703,62 @@ mod tests {
insta::assert_snapshot!("feedback_view_safety_check", rendered);
}
#[test]
fn feedback_view_with_connectivity_diagnostics() {
let (tx_raw, _rx) = tokio::sync::mpsc::unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let diagnostics = FeedbackDiagnostics::new(vec![
FeedbackDiagnostic {
headline: "Proxy environment variables are set and may affect connectivity."
.to_string(),
details: vec!["HTTP_PROXY = http://proxy.example.com:8080".to_string()],
},
FeedbackDiagnostic {
headline: "OPENAI_BASE_URL is set and may affect connectivity.".to_string(),
details: vec!["OPENAI_BASE_URL = https://example.com/v1".to_string()],
},
]);
let snapshot = codex_feedback::CodexFeedback::new()
.snapshot(None)
.with_feedback_diagnostics(diagnostics);
let view = FeedbackNoteView::new(
FeedbackCategory::Bug,
snapshot,
None,
tx,
false,
FeedbackAudience::External,
);
let rendered = render(&view, 60);
insta::assert_snapshot!("feedback_view_with_connectivity_diagnostics", rendered);
}
#[test]
fn should_show_feedback_connectivity_details_only_for_non_good_result_with_diagnostics() {
let diagnostics = FeedbackDiagnostics::new(vec![FeedbackDiagnostic {
headline: "Proxy environment variables are set and may affect connectivity."
.to_string(),
details: vec!["HTTP_PROXY = http://proxy.example.com:8080".to_string()],
}]);
assert_eq!(
should_show_feedback_connectivity_details(FeedbackCategory::Bug, &diagnostics),
true
);
assert_eq!(
should_show_feedback_connectivity_details(FeedbackCategory::GoodResult, &diagnostics),
false
);
assert_eq!(
should_show_feedback_connectivity_details(
FeedbackCategory::BadResult,
&FeedbackDiagnostics::default()
),
false
);
}
#[test]
fn issue_url_available_for_bug_bad_result_safety_check_and_other() {
let bug_url = issue_url_for_category(
@@ -0,0 +1,17 @@
---
source: tui/src/bottom_pane/feedback_view.rs
assertion_line: 749
expression: rendered
---
▌ Tell us more (bug)
▌
▌ Connectivity diagnostics
▌ - Proxy environment variables are set and may affect
▌ connectivity.
▌ - HTTP_PROXY = http://proxy.example.com:8080
▌ - OPENAI_BASE_URL is set and may affect connectivity.
▌ - OPENAI_BASE_URL = https://example.com/v1
▌
▌ (optional) Write a short description to help us further
Press enter to confirm or esc to go back
+13 -1
View File
@@ -1284,8 +1284,16 @@ impl ChatWidget {
category: crate::app_event::FeedbackCategory,
include_logs: bool,
) {
// Build a fresh snapshot at the time of opening the note overlay.
let snapshot = self.feedback.snapshot(self.thread_id);
self.show_feedback_note(category, include_logs, snapshot);
}
fn show_feedback_note(
&mut self,
category: crate::app_event::FeedbackCategory,
include_logs: bool,
snapshot: codex_feedback::FeedbackSnapshot,
) {
let rollout = if include_logs {
self.current_rollout_path.clone()
} else {
@@ -1310,10 +1318,14 @@ impl ChatWidget {
}
pub(crate) fn open_feedback_consent(&mut self, category: crate::app_event::FeedbackCategory) {
let snapshot = self.feedback.snapshot(self.thread_id);
let params = crate::bottom_pane::feedback_upload_consent_params(
self.app_event_tx.clone(),
category,
self.current_rollout_path.clone(),
snapshot
.feedback_diagnostics_attachment_text(true)
.is_some(),
);
self.bottom_pane.show_selection_view(params);
self.request_redraw();
@@ -0,0 +1,15 @@
---
source: tui/src/chatwidget/tests.rs
expression: popup
---
Upload logs?
The following files will be sent:
• codex-logs.log
• codex-connectivity-diagnostics.txt
› 1. Yes Share the current Codex session logs with the team for
troubleshooting.
2. No
Press enter to confirm or esc to go back
@@ -6,6 +6,7 @@ expression: popup
The following files will be sent:
• codex-logs.log
• codex-connectivity-diagnostics.txt
› 1. Yes Share the current Codex session logs with the team for
troubleshooting.
+21 -2
View File
@@ -7092,13 +7092,32 @@ async fn feedback_selection_popup_snapshot() {
async fn feedback_upload_consent_popup_snapshot() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
// Open the consent popup directly for a chosen category.
chat.open_feedback_consent(crate::app_event::FeedbackCategory::Bug);
chat.show_selection_view(crate::bottom_pane::feedback_upload_consent_params(
chat.app_event_tx.clone(),
crate::app_event::FeedbackCategory::Bug,
chat.current_rollout_path.clone(),
true,
));
let popup = render_bottom_popup(&chat, 80);
assert_snapshot!("feedback_upload_consent_popup", popup);
}
#[tokio::test]
async fn feedback_good_result_consent_popup_includes_connectivity_diagnostics_filename() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await;
chat.show_selection_view(crate::bottom_pane::feedback_upload_consent_params(
chat.app_event_tx.clone(),
crate::app_event::FeedbackCategory::GoodResult,
chat.current_rollout_path.clone(),
true,
));
let popup = render_bottom_popup(&chat, 80);
assert_snapshot!("feedback_good_result_consent_popup", popup);
}
#[tokio::test]
async fn reasoning_popup_escape_returns_to_model_popup() {
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(Some("gpt-5.1-codex-max")).await;
+1 -1
View File
@@ -29,6 +29,7 @@ codex-utils-string = { workspace = true }
dunce = "1.0"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
tempfile = "3"
windows = { version = "0.58", features = [
"Win32_Foundation",
"Win32_NetworkManagement_WindowsFirewall",
@@ -79,7 +80,6 @@ version = "0.52"
[dev-dependencies]
pretty_assertions = { workspace = true }
tempfile = "3"
[build-dependencies]
winres = "0.1"
@@ -6,6 +6,8 @@ mod windows_impl {
use crate::env::ensure_non_interactive_pager;
use crate::env::inherit_path_env;
use crate::env::normalize_null_device_env;
use crate::helper_materialization::resolve_helper_for_launch;
use crate::helper_materialization::HelperExecutable;
use crate::identity::require_logon_sandbox_creds;
use crate::logging::log_failure;
use crate::logging::log_note;
@@ -110,17 +112,9 @@ mod windows_impl {
}
}
/// Locates `codex-command-runner.exe` next to the current binary.
fn find_runner_exe() -> PathBuf {
if let Ok(exe) = std::env::current_exe() {
if let Some(dir) = exe.parent() {
let candidate = dir.join("codex-command-runner.exe");
if candidate.exists() {
return candidate;
}
}
}
PathBuf::from("codex-command-runner.exe")
/// Resolves the command runner path, preferring CODEX_HOME/.sandbox/bin.
fn find_runner_exe(codex_home: &Path, log_dir: Option<&Path>) -> PathBuf {
resolve_helper_for_launch(HelperExecutable::CommandRunner, codex_home, log_dir)
}
/// Generates a unique named-pipe path used to communicate with the runner process.
@@ -286,7 +280,7 @@ mod windows_impl {
)?;
// Launch runner as sandbox user via CreateProcessWithLogonW.
let runner_exe = find_runner_exe();
let runner_exe = find_runner_exe(codex_home, logs_base_dir);
let runner_cmdline = runner_exe
.to_str()
.map(|s| s.to_string())
@@ -340,6 +334,16 @@ mod windows_impl {
// Suppress WER/UI popups from the runner process so we can collect exit codes.
let _ = unsafe { SetErrorMode(0x0001 | 0x0002) }; // SEM_FAILCRITICALERRORS | SEM_NOGPFAULTERRORBOX
log_note(
&format!(
"runner launch: exe={} cmdline={} cwd={}",
runner_exe.display(),
runner_full_cmd,
cwd.display()
),
logs_base_dir,
);
// Ensure command line buffer is mutable and includes the exe as argv[0].
let spawn_res = unsafe {
CreateProcessWithLogonW(
@@ -362,6 +366,14 @@ mod windows_impl {
};
if spawn_res == 0 {
let err = unsafe { GetLastError() } as i32;
log_note(
&format!(
"runner launch failed before process start: exe={} cmdline={} error={err}",
runner_exe.display(),
runner_full_cmd
),
logs_base_dir,
);
return Err(anyhow::anyhow!("CreateProcessWithLogonW failed: {}", err));
}
@@ -0,0 +1,351 @@
use anyhow::anyhow;
use anyhow::Context;
use anyhow::Result;
use std::collections::HashMap;
use std::fs;
use std::io::Write;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Mutex;
use std::sync::OnceLock;
use tempfile::NamedTempFile;
use crate::logging::log_note;
use crate::sandbox_bin_dir;
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub(crate) enum HelperExecutable {
CommandRunner,
}
impl HelperExecutable {
fn file_name(self) -> &'static str {
match self {
Self::CommandRunner => "codex-command-runner.exe",
}
}
fn label(self) -> &'static str {
match self {
Self::CommandRunner => "command-runner",
}
}
}
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
enum CopyOutcome {
Reused,
ReCopied,
}
static HELPER_PATH_CACHE: OnceLock<Mutex<HashMap<String, PathBuf>>> = OnceLock::new();
pub(crate) fn helper_bin_dir(codex_home: &Path) -> PathBuf {
sandbox_bin_dir(codex_home)
}
pub(crate) fn legacy_lookup(kind: HelperExecutable) -> PathBuf {
if let Ok(exe) = std::env::current_exe() {
if let Some(dir) = exe.parent() {
let candidate = dir.join(kind.file_name());
if candidate.exists() {
return candidate;
}
}
}
PathBuf::from(kind.file_name())
}
pub(crate) fn resolve_helper_for_launch(
kind: HelperExecutable,
codex_home: &Path,
log_dir: Option<&Path>,
) -> PathBuf {
match copy_helper_if_needed(kind, codex_home, log_dir) {
Ok(path) => {
log_note(
&format!(
"helper launch resolution: using copied {} path {}",
kind.label(),
path.display()
),
log_dir,
);
path
}
Err(err) => {
let fallback = legacy_lookup(kind);
log_note(
&format!(
"helper copy failed for {}: {err:#}; falling back to legacy path {}",
kind.label(),
fallback.display()
),
log_dir,
);
fallback
}
}
}
pub(crate) fn copy_helper_if_needed(
kind: HelperExecutable,
codex_home: &Path,
log_dir: Option<&Path>,
) -> Result<PathBuf> {
let cache_key = format!("{}|{}", kind.file_name(), codex_home.display());
if let Some(path) = cached_helper_path(&cache_key) {
log_note(
&format!(
"helper copy: using in-memory cache for {} -> {}",
kind.label(),
path.display()
),
log_dir,
);
return Ok(path);
}
let source = sibling_source_path(kind)?;
let destination = helper_bin_dir(codex_home).join(kind.file_name());
log_note(
&format!(
"helper copy: validating {} source={} destination={}",
kind.label(),
source.display(),
destination.display()
),
log_dir,
);
let outcome = copy_from_source_if_needed(&source, &destination)?;
let action = match outcome {
CopyOutcome::Reused => "reused",
CopyOutcome::ReCopied => "recopied",
};
log_note(
&format!(
"helper copy: {} {} source={} destination={}",
action,
kind.label(),
source.display(),
destination.display()
),
log_dir,
);
store_helper_path(cache_key, destination.clone());
Ok(destination)
}
fn cached_helper_path(cache_key: &str) -> Option<PathBuf> {
let cache = HELPER_PATH_CACHE.get_or_init(|| Mutex::new(HashMap::new()));
let guard = cache.lock().ok()?;
guard.get(cache_key).cloned()
}
fn store_helper_path(cache_key: String, path: PathBuf) {
let cache = HELPER_PATH_CACHE.get_or_init(|| Mutex::new(HashMap::new()));
if let Ok(mut guard) = cache.lock() {
guard.insert(cache_key, path);
}
}
fn sibling_source_path(kind: HelperExecutable) -> Result<PathBuf> {
let exe = std::env::current_exe().context("resolve current executable for helper lookup")?;
let dir = exe
.parent()
.ok_or_else(|| anyhow!("current executable has no parent directory"))?;
let candidate = dir.join(kind.file_name());
if candidate.exists() {
Ok(candidate)
} else {
Err(anyhow!(
"helper not found next to current executable: {}",
candidate.display()
))
}
}
fn copy_from_source_if_needed(source: &Path, destination: &Path) -> Result<CopyOutcome> {
if destination_is_fresh(source, destination)? {
return Ok(CopyOutcome::Reused);
}
let destination_dir = destination
.parent()
.ok_or_else(|| anyhow!("helper destination has no parent: {}", destination.display()))?;
fs::create_dir_all(destination_dir).with_context(|| {
format!(
"create helper destination directory {}",
destination_dir.display()
)
})?;
let temp_path = NamedTempFile::new_in(destination_dir)
.with_context(|| {
format!(
"create temporary helper file in {}",
destination_dir.display()
)
})?
.into_temp_path();
let temp_path_buf = temp_path.to_path_buf();
let mut source_file = fs::File::open(source)
.with_context(|| format!("open helper source for read {}", source.display()))?;
let mut temp_file = fs::OpenOptions::new()
.write(true)
.truncate(true)
.open(&temp_path_buf)
.with_context(|| format!("open temporary helper file {}", temp_path_buf.display()))?;
// Write into a temp file created inside `.sandbox-bin` so the copied helper keeps the
// destination directory's inherited ACLs instead of reusing the source file's descriptor.
std::io::copy(&mut source_file, &mut temp_file).with_context(|| {
format!(
"copy helper from {} to {}",
source.display(),
temp_path_buf.display()
)
})?;
temp_file
.flush()
.with_context(|| format!("flush temporary helper file {}", temp_path_buf.display()))?;
drop(temp_file);
if destination.exists() {
fs::remove_file(destination).with_context(|| {
format!("remove stale helper destination {}", destination.display())
})?;
}
match fs::rename(&temp_path_buf, destination) {
Ok(()) => Ok(CopyOutcome::ReCopied),
Err(rename_err) => {
if destination_is_fresh(source, destination)? {
Ok(CopyOutcome::Reused)
} else {
Err(rename_err).with_context(|| {
format!(
"rename helper temp file {} to {}",
temp_path_buf.display(),
destination.display()
)
})
}
}
}
}
fn destination_is_fresh(source: &Path, destination: &Path) -> Result<bool> {
let source_meta = fs::metadata(source)
.with_context(|| format!("read helper source metadata {}", source.display()))?;
let destination_meta = match fs::metadata(destination) {
Ok(meta) => meta,
Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(false),
Err(err) => {
return Err(err)
.with_context(|| format!("read helper destination metadata {}", destination.display()));
}
};
if source_meta.len() != destination_meta.len() {
return Ok(false);
}
let source_modified = source_meta
.modified()
.with_context(|| format!("read helper source mtime {}", source.display()))?;
let destination_modified = destination_meta
.modified()
.with_context(|| format!("read helper destination mtime {}", destination.display()))?;
Ok(destination_modified >= source_modified)
}
#[cfg(test)]
mod tests {
use super::destination_is_fresh;
use super::helper_bin_dir;
use super::copy_from_source_if_needed;
use super::CopyOutcome;
use pretty_assertions::assert_eq;
use std::fs;
use std::path::Path;
use std::path::PathBuf;
use tempfile::TempDir;
#[test]
fn copy_from_source_if_needed_copies_missing_destination() {
let tmp = TempDir::new().expect("tempdir");
let source = tmp.path().join("source.exe");
let destination = tmp.path().join("bin").join("helper.exe");
fs::write(&source, b"runner-v1").expect("write source");
let outcome = copy_from_source_if_needed(&source, &destination).expect("copy helper");
assert_eq!(CopyOutcome::ReCopied, outcome);
assert_eq!(b"runner-v1".as_slice(), fs::read(&destination).expect("read destination"));
}
#[test]
fn destination_is_fresh_uses_size_and_mtime() {
let tmp = TempDir::new().expect("tempdir");
let source = tmp.path().join("source.exe");
let destination = tmp.path().join("destination.exe");
fs::write(&destination, b"same-size").expect("write destination");
std::thread::sleep(std::time::Duration::from_secs(1));
fs::write(&source, b"same-size").expect("write source");
assert!(!destination_is_fresh(&source, &destination).expect("stale metadata"));
fs::write(&destination, b"same-size").expect("rewrite destination");
assert!(destination_is_fresh(&source, &destination).expect("fresh metadata"));
}
#[test]
fn copy_from_source_if_needed_reuses_fresh_destination() {
let tmp = TempDir::new().expect("tempdir");
let source = tmp.path().join("source.exe");
let destination = tmp.path().join("bin").join("helper.exe");
fs::write(&source, b"runner-v1").expect("write source");
copy_from_source_if_needed(&source, &destination).expect("initial copy");
let outcome =
copy_from_source_if_needed(&source, &destination).expect("revalidate helper");
assert_eq!(CopyOutcome::Reused, outcome);
assert_eq!(b"runner-v1".as_slice(), fs::read(&destination).expect("read destination"));
}
#[test]
fn helper_bin_dir_is_under_sandbox_bin() {
let codex_home = Path::new(r"C:\Users\example\.codex");
assert_eq!(
PathBuf::from(r"C:\Users\example\.codex\.sandbox-bin"),
helper_bin_dir(codex_home)
);
}
#[test]
fn copy_runner_into_shared_bin_dir() {
let tmp = TempDir::new().expect("tempdir");
let codex_home = tmp.path().join("codex-home");
let source_dir = tmp.path().join("sibling-source");
fs::create_dir_all(&source_dir).expect("create source dir");
let runner_source = source_dir.join("codex-command-runner.exe");
let runner_destination = helper_bin_dir(&codex_home).join("codex-command-runner.exe");
fs::write(&runner_source, b"runner").expect("runner");
let runner_outcome =
copy_from_source_if_needed(&runner_source, &runner_destination).expect("runner copy");
assert_eq!(CopyOutcome::ReCopied, runner_outcome);
assert_eq!(
b"runner".as_slice(),
fs::read(&runner_destination).expect("read runner")
);
}
}
+1 -1
View File
@@ -130,7 +130,7 @@ pub fn require_logon_sandbox_creds(
codex_home: &Path,
) -> Result<SandboxCreds> {
let sandbox_dir = crate::setup::sandbox_dir(codex_home);
let needed_read = gather_read_roots(command_cwd, policy);
let needed_read = gather_read_roots(command_cwd, policy, codex_home);
let needed_write = gather_write_roots(policy, policy_cwd, command_cwd, env_map);
// NOTE: Do not add CODEX_HOME/.sandbox to `needed_write`; it must remain non-writable by the
// restricted capability token. The setup helper's `lock_sandbox_dir` is responsible for
+3
View File
@@ -11,6 +11,7 @@ windows_modules!(
cap,
dpapi,
env,
helper_materialization,
hide_users,
identity,
logging,
@@ -85,6 +86,8 @@ pub use setup::run_setup_refresh;
#[cfg(target_os = "windows")]
pub use setup::run_setup_refresh_with_extra_read_roots;
#[cfg(target_os = "windows")]
pub use setup::sandbox_bin_dir;
#[cfg(target_os = "windows")]
pub use setup::sandbox_dir;
#[cfg(target_os = "windows")]
pub use setup::sandbox_secrets_dir;
@@ -18,6 +18,7 @@ use codex_windows_sandbox::log_note;
use codex_windows_sandbox::path_mask_allows;
use codex_windows_sandbox::protect_workspace_agents_dir;
use codex_windows_sandbox::protect_workspace_codex_dir;
use codex_windows_sandbox::sandbox_bin_dir;
use codex_windows_sandbox::sandbox_dir;
use codex_windows_sandbox::sandbox_secrets_dir;
use codex_windows_sandbox::string_from_sid_bytes;
@@ -245,6 +246,8 @@ fn lock_sandbox_dir(
real_user: &str,
sandbox_group_sid: &[u8],
sandbox_group_access_mode: i32,
sandbox_group_mask: u32,
real_user_mask: u32,
_log: &mut File,
) -> Result<()> {
std::fs::create_dir_all(dir)?;
@@ -254,7 +257,7 @@ fn lock_sandbox_dir(
let entries = [
(
sandbox_group_sid.to_vec(),
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE,
sandbox_group_mask,
sandbox_group_access_mode,
),
(
@@ -267,11 +270,7 @@ fn lock_sandbox_dir(
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE,
GRANT_ACCESS,
),
(
real_sid,
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE,
GRANT_ACCESS,
),
(real_sid, real_user_mask, GRANT_ACCESS),
];
unsafe {
let mut eas: Vec<EXPLICIT_ACCESS_W> = Vec::new();
@@ -740,6 +739,25 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
}
});
lock_sandbox_dir(
&sandbox_bin_dir(&payload.codex_home),
&payload.real_user,
&sandbox_group_sid,
GRANT_ACCESS,
FILE_GENERIC_READ | FILE_GENERIC_EXECUTE,
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE,
log,
)
.map_err(|err| {
anyhow::Error::new(SetupFailure::new(
SetupErrorCode::HelperSandboxLockFailed,
format!(
"lock sandbox bin dir {} failed: {err}",
sandbox_bin_dir(&payload.codex_home).display()
),
))
})?;
if refresh_only {
log_line(
log,
@@ -756,6 +774,8 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
&payload.real_user,
&sandbox_group_sid,
GRANT_ACCESS,
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE,
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE,
log,
)
.map_err(|err| {
@@ -772,6 +792,8 @@ fn run_setup_full(payload: &Payload, log: &mut File, sbx_dir: &Path) -> Result<(
&payload.real_user,
&sandbox_group_sid,
DENY_ACCESS,
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE | DELETE,
FILE_GENERIC_READ | FILE_GENERIC_WRITE | FILE_GENERIC_EXECUTE,
log,
)
.map_err(|err| {
@@ -11,6 +11,7 @@ use std::process::Stdio;
use crate::allow::compute_allow_paths;
use crate::allow::AllowDenyPaths;
use crate::helper_materialization::helper_bin_dir;
use crate::logging::log_note;
use crate::path_normalization::canonical_path_key;
use crate::policy::SandboxPolicy;
@@ -55,6 +56,10 @@ pub fn sandbox_dir(codex_home: &Path) -> PathBuf {
codex_home.join(".sandbox")
}
pub fn sandbox_bin_dir(codex_home: &Path) -> PathBuf {
codex_home.join(".sandbox-bin")
}
pub fn sandbox_secrets_dir(codex_home: &Path) -> PathBuf {
codex_home.join(".sandbox-secrets")
}
@@ -93,7 +98,7 @@ pub fn run_setup_refresh_with_extra_read_roots(
codex_home: &Path,
extra_read_roots: Vec<PathBuf>,
) -> Result<()> {
let mut read_roots = gather_read_roots(command_cwd, policy);
let mut read_roots = gather_read_roots(command_cwd, policy, codex_home);
read_roots.extend(extra_read_roots);
run_setup_refresh_inner(
policy,
@@ -276,13 +281,20 @@ fn profile_read_roots(user_profile: &Path) -> Vec<PathBuf> {
.collect()
}
pub(crate) fn gather_read_roots(command_cwd: &Path, policy: &SandboxPolicy) -> Vec<PathBuf> {
pub(crate) fn gather_read_roots(
command_cwd: &Path,
policy: &SandboxPolicy,
codex_home: &Path,
) -> Vec<PathBuf> {
let mut roots: Vec<PathBuf> = Vec::new();
if let Ok(exe) = std::env::current_exe() {
if let Some(dir) = exe.parent() {
roots.push(dir.to_path_buf());
}
}
let helper_dir = helper_bin_dir(codex_home);
let _ = std::fs::create_dir_all(&helper_dir);
roots.push(helper_dir);
for p in [
PathBuf::from(r"C:\Windows"),
PathBuf::from(r"C:\Program Files"),
@@ -583,7 +595,7 @@ fn build_payload_roots(
let mut read_roots = if let Some(roots) = read_roots_override {
canonical_existing(&roots)
} else {
gather_read_roots(command_cwd, policy)
gather_read_roots(command_cwd, policy, codex_home)
};
let write_root_set: HashSet<PathBuf> = write_roots.iter().cloned().collect();
read_roots.retain(|root| !write_root_set.contains(root));
@@ -591,11 +603,14 @@ fn build_payload_roots(
}
fn filter_sensitive_write_roots(mut roots: Vec<PathBuf>, codex_home: &Path) -> Vec<PathBuf> {
// Never grant capability write access to CODEX_HOME or anything under CODEX_HOME/.sandbox.
// These locations contain sandbox control/state and must remain tamper-resistant.
// Never grant capability write access to CODEX_HOME or anything under CODEX_HOME/.sandbox,
// CODEX_HOME/.sandbox-bin, or CODEX_HOME/.sandbox-secrets. These locations contain sandbox
// control/state and helper binaries and must remain tamper-resistant.
let codex_home_key = canonical_path_key(codex_home);
let sbx_dir_key = canonical_path_key(&sandbox_dir(codex_home));
let sbx_dir_prefix = format!("{}/", sbx_dir_key.trim_end_matches('/'));
let sbx_bin_dir_key = canonical_path_key(&sandbox_bin_dir(codex_home));
let sbx_bin_dir_prefix = format!("{}/", sbx_bin_dir_key.trim_end_matches('/'));
let secrets_dir_key = canonical_path_key(&sandbox_secrets_dir(codex_home));
let secrets_dir_prefix = format!("{}/", secrets_dir_key.trim_end_matches('/'));
@@ -604,6 +619,8 @@ fn filter_sensitive_write_roots(mut roots: Vec<PathBuf>, codex_home: &Path) -> V
key != codex_home_key
&& key != sbx_dir_key
&& !key.starts_with(&sbx_dir_prefix)
&& key != sbx_bin_dir_key
&& !key.starts_with(&sbx_bin_dir_prefix)
&& key != secrets_dir_key
&& !key.starts_with(&secrets_dir_prefix)
});
@@ -612,7 +629,10 @@ fn filter_sensitive_write_roots(mut roots: Vec<PathBuf>, codex_home: &Path) -> V
#[cfg(test)]
mod tests {
use super::gather_read_roots;
use super::profile_read_roots;
use crate::helper_materialization::helper_bin_dir;
use crate::policy::SandboxPolicy;
use pretty_assertions::assert_eq;
use std::collections::HashSet;
use std::fs;
@@ -649,4 +669,19 @@ mod tests {
assert_eq!(vec![missing_profile], roots);
}
#[test]
fn gather_read_roots_includes_helper_bin_dir() {
let tmp = TempDir::new().expect("tempdir");
let codex_home = tmp.path().join("codex-home");
let command_cwd = tmp.path().join("workspace");
fs::create_dir_all(&command_cwd).expect("create workspace");
let policy = SandboxPolicy::new_read_only_policy();
let roots = gather_read_roots(&command_cwd, &policy, &codex_home);
let expected =
dunce::canonicalize(helper_bin_dir(&codex_home)).expect("canonical helper dir");
assert!(roots.contains(&expected));
}
}
+6 -1
View File
@@ -65,8 +65,13 @@ For `CODEX_JS_REPL_NODE_MODULE_DIRS` and `js_repl_node_module_dirs`, module reso
- `codex.tmpDir`: per-session scratch directory path.
- `codex.tool(name, args?)`: executes a normal Codex tool call from inside `js_repl` (including shell tools like `shell` / `shell_command` when available).
- `codex.emitImage(imageLike)`: explicitly adds exactly one image to the outer `js_repl` function output.
- Each `codex.tool(...)` call emits a bounded summary at `info` level from the `codex_core::tools::js_repl` logger. At `trace` level, the same path also logs the exact raw response object or error string seen by JavaScript.
- To share generated images with the model, write a file under `codex.tmpDir`, call `await codex.tool("view_image", { path: "/absolute/path" })`, then delete the file.
- Nested `codex.tool(...)` outputs stay inside JavaScript unless you emit them explicitly.
- `codex.emitImage(...)` accepts a direct image URL, a single `input_image` item, an object like `{ bytes, mimeType }`, or a raw tool response object that contains exactly one image and no text.
- `codex.emitImage(...)` rejects mixed text-and-image content.
- Example of sharing an in-memory Playwright screenshot: `await codex.emitImage({ bytes: await page.screenshot({ type: "jpeg", quality: 85 }), mimeType: "image/jpeg" })`.
- Example of sharing a local image tool result: `await codex.emitImage(codex.tool("view_image", { path: "/absolute/path" }))`.
Avoid writing directly to `process.stdout` / `process.stderr` / `process.stdin`; the kernel uses a JSON-line transport over stdio.