diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index c1ab56b653..03ad23ff77 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -612,10 +612,6 @@ "arrayvec_0.7.6": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"bencher\",\"req\":\"^0.1.4\"},{\"default_features\":false,\"name\":\"borsh\",\"optional\":true,\"req\":\"^1.2.0\"},{\"kind\":\"dev\",\"name\":\"matches\",\"req\":\"^0.1\"},{\"default_features\":false,\"name\":\"serde\",\"optional\":true,\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"serde_test\",\"req\":\"^1.0\"},{\"default_features\":false,\"name\":\"zeroize\",\"optional\":true,\"req\":\"^1.4\"}],\"features\":{\"default\":[\"std\"],\"std\":[]}}", "ascii-canvas_3.0.0": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"diff\",\"req\":\"^0.1\"},{\"name\":\"term\",\"req\":\"^0.7\"}],\"features\":{}}", "ascii_1.1.0": "{\"dependencies\":[{\"name\":\"serde\",\"optional\":true,\"req\":\"^1.0.25\"},{\"name\":\"serde_test\",\"optional\":true,\"req\":\"^1.0\"}],\"features\":{\"alloc\":[],\"default\":[\"std\"],\"std\":[\"alloc\"]}}", - "askama_0.15.4": "{\"dependencies\":[{\"default_features\":false,\"name\":\"askama_macros\",\"optional\":true,\"req\":\"=0.15.4\"},{\"kind\":\"dev\",\"name\":\"assert_matches\",\"req\":\"^1.5.0\"},{\"kind\":\"dev\",\"name\":\"criterion\",\"req\":\"^0.8\"},{\"name\":\"itoa\",\"req\":\"^1.0.11\"},{\"default_features\":false,\"name\":\"percent-encoding\",\"optional\":true,\"req\":\"^2.1.0\"},{\"default_features\":false,\"name\":\"serde\",\"optional\":true,\"req\":\"^1.0\"},{\"default_features\":false,\"name\":\"serde_json\",\"optional\":true,\"req\":\"^1.0\"}],\"features\":{\"alloc\":[\"askama_macros?/alloc\",\"serde?/alloc\",\"serde_json?/alloc\",\"percent-encoding?/alloc\"],\"code-in-doc\":[\"askama_macros?/code-in-doc\"],\"config\":[\"askama_macros?/config\"],\"default\":[\"config\",\"derive\",\"std\",\"urlencode\"],\"derive\":[\"dep:askama_macros\",\"dep:askama_macros\"],\"full\":[\"default\",\"code-in-doc\",\"serde_json\"],\"nightly-spans\":[\"askama_macros/nightly-spans\"],\"serde_json\":[\"std\",\"askama_macros?/serde_json\",\"dep:serde\",\"dep:serde_json\"],\"std\":[\"alloc\",\"askama_macros?/std\",\"serde?/std\",\"serde_json?/std\",\"percent-encoding?/std\"],\"urlencode\":[\"askama_macros?/urlencode\",\"dep:percent-encoding\"]}}", - "askama_derive_0.15.4": "{\"dependencies\":[{\"name\":\"basic-toml\",\"optional\":true,\"req\":\"^0.1.1\"},{\"kind\":\"dev\",\"name\":\"console\",\"req\":\"^0.16.0\"},{\"kind\":\"dev\",\"name\":\"criterion\",\"req\":\"^0.8\"},{\"name\":\"memchr\",\"req\":\"^2\"},{\"name\":\"parser\",\"package\":\"askama_parser\",\"req\":\"=0.15.4\"},{\"kind\":\"dev\",\"name\":\"prettyplease\",\"req\":\"^0.2.20\"},{\"default_features\":false,\"name\":\"proc-macro2\",\"req\":\"^1\"},{\"default_features\":false,\"name\":\"pulldown-cmark\",\"optional\":true,\"req\":\"^0.13.0\"},{\"default_features\":false,\"name\":\"quote\",\"req\":\"^1\"},{\"name\":\"rustc-hash\",\"req\":\"^2.0.0\"},{\"name\":\"serde\",\"optional\":true,\"req\":\"^1.0\"},{\"name\":\"serde_derive\",\"optional\":true,\"req\":\"^1.0\"},{\"kind\":\"dev\",\"name\":\"similar\",\"req\":\"^2.6.0\"},{\"default_features\":false,\"features\":[\"clone-impls\",\"derive\",\"full\",\"parsing\",\"printing\"],\"name\":\"syn\",\"req\":\"^2.0.3\"}],\"features\":{\"alloc\":[],\"code-in-doc\":[\"dep:pulldown-cmark\"],\"config\":[\"external-sources\",\"dep:basic-toml\",\"dep:serde\",\"dep:serde_derive\",\"parser/config\"],\"default\":[\"alloc\",\"code-in-doc\",\"config\",\"external-sources\",\"proc-macro\",\"serde_json\",\"std\",\"urlencode\"],\"external-sources\":[],\"nightly-spans\":[],\"proc-macro\":[\"proc-macro2/proc-macro\"],\"serde_json\":[],\"std\":[\"alloc\"],\"urlencode\":[]}}", - "askama_macros_0.15.4": "{\"dependencies\":[{\"default_features\":false,\"features\":[\"external-sources\",\"proc-macro\"],\"name\":\"askama_derive\",\"package\":\"askama_derive\",\"req\":\"=0.15.4\"}],\"features\":{\"alloc\":[\"askama_derive/alloc\"],\"code-in-doc\":[\"askama_derive/code-in-doc\"],\"config\":[\"askama_derive/config\"],\"default\":[\"config\",\"derive\",\"std\",\"urlencode\"],\"derive\":[],\"full\":[\"default\",\"code-in-doc\",\"serde_json\"],\"nightly-spans\":[\"askama_derive/nightly-spans\"],\"serde_json\":[\"askama_derive/serde_json\"],\"std\":[\"askama_derive/std\"],\"urlencode\":[\"askama_derive/urlencode\"]}}", - "askama_parser_0.15.4": "{\"dependencies\":[{\"kind\":\"dev\",\"name\":\"criterion\",\"req\":\"^0.8\"},{\"name\":\"rustc-hash\",\"req\":\"^2.0.0\"},{\"name\":\"serde\",\"optional\":true,\"req\":\"^1.0\"},{\"name\":\"serde_derive\",\"optional\":true,\"req\":\"^1.0\"},{\"name\":\"unicode-ident\",\"req\":\"^1.0.12\"},{\"features\":[\"simd\"],\"name\":\"winnow\",\"req\":\"^0.7.0\"}],\"features\":{\"config\":[\"dep:serde\",\"dep:serde_derive\"]}}", "asn1-rs-derive_0.6.0": "{\"dependencies\":[{\"name\":\"proc-macro2\",\"req\":\"^1.0\"},{\"name\":\"quote\",\"req\":\"^1.0\"},{\"features\":[\"full\"],\"name\":\"syn\",\"req\":\"^2.0\"},{\"name\":\"synstructure\",\"req\":\"^0.13\"}],\"features\":{}}", "asn1-rs-impl_0.2.0": "{\"dependencies\":[{\"name\":\"proc-macro2\",\"req\":\"^1\"},{\"name\":\"quote\",\"req\":\"^1\"},{\"name\":\"syn\",\"req\":\"^2.0\"}],\"features\":{}}", "asn1-rs_0.7.1": "{\"dependencies\":[{\"name\":\"asn1-rs-derive\",\"req\":\"^0.6\"},{\"name\":\"asn1-rs-impl\",\"req\":\"^0.2\"},{\"name\":\"bitvec\",\"optional\":true,\"req\":\"^1.0\"},{\"name\":\"colored\",\"optional\":true,\"req\":\"^3.0\"},{\"kind\":\"dev\",\"name\":\"colored\",\"req\":\"^3.0\"},{\"name\":\"cookie-factory\",\"optional\":true,\"req\":\"^0.3.0\"},{\"name\":\"displaydoc\",\"req\":\"^0.2.2\"},{\"kind\":\"dev\",\"name\":\"hex-literal\",\"req\":\"^0.4\"},{\"default_features\":false,\"features\":[\"std\"],\"name\":\"nom\",\"req\":\"^7.0\"},{\"name\":\"num-bigint\",\"optional\":true,\"req\":\"^0.4\"},{\"name\":\"num-traits\",\"req\":\"^0.2.14\"},{\"kind\":\"dev\",\"name\":\"pem\",\"req\":\"^3.0\"},{\"name\":\"rusticata-macros\",\"req\":\"^4.0\"},{\"name\":\"thiserror\",\"req\":\"^2.0.0\"},{\"features\":[\"macros\",\"parsing\",\"formatting\"],\"name\":\"time\",\"optional\":true,\"req\":\"^0.3\"},{\"kind\":\"dev\",\"name\":\"trybuild\",\"req\":\"^1.0\"}],\"features\":{\"bigint\":[\"num-bigint\"],\"bits\":[\"bitvec\"],\"datetime\":[\"time\"],\"debug\":[\"std\",\"colored\"],\"default\":[\"std\"],\"serialize\":[\"cookie-factory\"],\"std\":[],\"trace\":[\"debug\"]}}", diff --git a/codex-cli/package.json b/codex-cli/package.json index 3d1a2dcc2c..ee2c2a7c7d 100644 --- a/codex-cli/package.json +++ b/codex-cli/package.json @@ -18,5 +18,5 @@ "url": "git+https://github.com/openai/codex.git", "directory": "codex-cli" }, - "packageManager": "pnpm@10.28.2+sha512.41872f037ad22f7348e3b1debbaf7e867cfd448f2726d9cf74c08f19507c31d2c8e7a11525b983febc2df640b5438dee6023ebb1f84ed43cc2d654d2bc326264" + "packageManager": "pnpm@10.29.3+sha512.498e1fb4cca5aa06c1dcf2611e6fafc50972ffe7189998c409e90de74566444298ffe43e6cd2acdc775ba1aa7cc5e092a8b7054c811ba8c5770f84693d33d2dc" } diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 189d9468d2..20099b02eb 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -458,58 +458,6 @@ dependencies = [ "term", ] -[[package]] -name = "askama" -version = "0.15.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "08e1676b346cadfec169374f949d7490fd80a24193d37d2afce0c047cf695e57" -dependencies = [ - "askama_macros", - "itoa", - "percent-encoding", - "serde", - "serde_json", -] - -[[package]] -name = "askama_derive" -version = "0.15.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7661ff56517787343f376f75db037426facd7c8d3049cef8911f1e75016f3a37" -dependencies = [ - "askama_parser", - "basic-toml", - "memchr", - "proc-macro2", - "quote", - "rustc-hash 2.1.1", - "serde", - "serde_derive", - "syn 2.0.114", -] - -[[package]] -name = "askama_macros" -version = "0.15.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "713ee4dbfd1eb719c2dab859465b01fa1d21cb566684614a713a6b7a99a4e47b" -dependencies = [ - "askama_derive", -] - -[[package]] -name = "askama_parser" -version = "0.15.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1d62d674238a526418b30c0def480d5beadb9d8964e7f38d635b03bf639c704c" -dependencies = [ - "rustc-hash 2.1.1", - "serde", - "serde_derive", - "unicode-ident", - "winnow", -] - [[package]] name = "asn1-rs" version = "0.7.1" @@ -1406,6 +1354,7 @@ dependencies = [ "strum_macros 0.27.2", "tempfile", "thiserror 2.0.18", + "tracing", "ts-rs", "uuid", ] @@ -1662,7 +1611,6 @@ version = "0.0.0" dependencies = [ "anyhow", "arc-swap", - "askama", "assert_cmd", "assert_matches", "async-channel", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 7ad0566efd..e27b08b7f4 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -145,7 +145,6 @@ allocative = "0.3.3" ansi-to-tui = "7.0.0" anyhow = "1" arboard = { version = "3", features = ["wayland-data-control"] } -askama = "0.15.4" assert_cmd = "2" assert_matches = "1.5.0" async-channel = "2.3.1" diff --git a/codex-rs/app-server-protocol/Cargo.toml b/codex-rs/app-server-protocol/Cargo.toml index 2ab6f291f6..f5df7af034 100644 --- a/codex-rs/app-server-protocol/Cargo.toml +++ b/codex-rs/app-server-protocol/Cargo.toml @@ -25,6 +25,7 @@ strum_macros = { workspace = true } thiserror = { workspace = true } ts-rs = { workspace = true } inventory = { workspace = true } +tracing = { workspace = true } uuid = { workspace = true, features = ["serde", "v7"] } [dev-dependencies] diff --git a/codex-rs/app-server-protocol/schema/json/ClientRequest.json b/codex-rs/app-server-protocol/schema/json/ClientRequest.json index 37ce9c89f4..9fc2a99141 100644 --- a/codex-rs/app-server-protocol/schema/json/ClientRequest.json +++ b/codex-rs/app-server-protocol/schema/json/ClientRequest.json @@ -69,13 +69,46 @@ "type": "object" }, "AskForApproval": { - "enum": [ - "untrusted", - "on-failure", - "on-request", - "never" - ], - "type": "string" + "oneOf": [ + { + "enum": [ + "untrusted", + "on-failure", + "on-request", + "never" + ], + "type": "string" + }, + { + "additionalProperties": false, + "properties": { + "reject": { + "properties": { + "mcp_elicitations": { + "type": "boolean" + }, + "rules": { + "type": "boolean" + }, + "sandbox_approval": { + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + } + ] }, "AskForApproval2": { "description": "Determines the conditions under which the user is consulted to approve running the command proposed by Codex.", @@ -101,6 +134,20 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "Fine-grained rejection controls for approval prompts.\n\nWhen a field is `true`, prompts of that category are automatically rejected instead of shown to the user.", + "properties": { + "reject": { + "$ref": "#/definitions/RejectConfig" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval2", + "type": "object" + }, { "description": "Never ask the user to approve commands. Failures are immediately returned to the model, and never escalated to the user for approval.", "enum": [ @@ -1466,6 +1513,28 @@ } ] }, + "RejectConfig": { + "properties": { + "mcp_elicitations": { + "description": "Reject MCP elicitation prompts.", + "type": "boolean" + }, + "rules": { + "description": "Reject prompts triggered by execpolicy `prompt` rules.", + "type": "boolean" + }, + "sandbox_approval": { + "description": "Reject approval prompts related to sandbox escalation.", + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + }, "RemoveConversationListenerParams": { "properties": { "subscriptionId": { diff --git a/codex-rs/app-server-protocol/schema/json/EventMsg.json b/codex-rs/app-server-protocol/schema/json/EventMsg.json index 5ca7c850f8..a85d02853b 100644 --- a/codex-rs/app-server-protocol/schema/json/EventMsg.json +++ b/codex-rs/app-server-protocol/schema/json/EventMsg.json @@ -117,6 +117,20 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "Fine-grained rejection controls for approval prompts.\n\nWhen a field is `true`, prompts of that category are automatically rejected instead of shown to the user.", + "properties": { + "reject": { + "$ref": "#/definitions/RejectConfig" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + }, { "description": "Never ask the user to approve commands. Failures are immediately returned to the model, and never escalated to the user for approval.", "enum": [ @@ -3769,6 +3783,28 @@ } ] }, + "RejectConfig": { + "properties": { + "mcp_elicitations": { + "description": "Reject MCP elicitation prompts.", + "type": "boolean" + }, + "rules": { + "description": "Reject prompts triggered by execpolicy `prompt` rules.", + "type": "boolean" + }, + "sandbox_approval": { + "description": "Reject approval prompts related to sandbox escalation.", + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + }, "RemoteSkillSummary": { "properties": { "description": { diff --git a/codex-rs/app-server-protocol/schema/json/ServerNotification.json b/codex-rs/app-server-protocol/schema/json/ServerNotification.json index f3e71b70e8..f18531303e 100644 --- a/codex-rs/app-server-protocol/schema/json/ServerNotification.json +++ b/codex-rs/app-server-protocol/schema/json/ServerNotification.json @@ -454,6 +454,20 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "Fine-grained rejection controls for approval prompts.\n\nWhen a field is `true`, prompts of that category are automatically rejected instead of shown to the user.", + "properties": { + "reject": { + "$ref": "#/definitions/RejectConfig" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + }, { "description": "Never ask the user to approve commands. Failures are immediately returned to the model, and never escalated to the user for approval.", "enum": [ @@ -5169,6 +5183,28 @@ ], "type": "object" }, + "RejectConfig": { + "properties": { + "mcp_elicitations": { + "description": "Reject MCP elicitation prompts.", + "type": "boolean" + }, + "rules": { + "description": "Reject prompts triggered by execpolicy `prompt` rules.", + "type": "boolean" + }, + "sandbox_approval": { + "description": "Reject approval prompts related to sandbox escalation.", + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + }, "RemoteSkillSummary": { "properties": { "description": { diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 06b0c773f8..39b21d0a54 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -221,6 +221,20 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "Fine-grained rejection controls for approval prompts.\n\nWhen a field is `true`, prompts of that category are automatically rejected instead of shown to the user.", + "properties": { + "reject": { + "$ref": "#/definitions/RejectConfig" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + }, { "description": "Never ask the user to approve commands. Failures are immediately returned to the model, and never escalated to the user for approval.", "enum": [ @@ -6949,6 +6963,28 @@ } ] }, + "RejectConfig": { + "properties": { + "mcp_elicitations": { + "description": "Reject MCP elicitation prompts.", + "type": "boolean" + }, + "rules": { + "description": "Reject prompts triggered by execpolicy `prompt` rules.", + "type": "boolean" + }, + "sandbox_approval": { + "description": "Reject approval prompts related to sandbox escalation.", + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + }, "RemoteSkillSummary": { "properties": { "description": { @@ -10763,13 +10799,46 @@ "type": "object" }, "AskForApproval": { - "enum": [ - "untrusted", - "on-failure", - "on-request", - "never" - ], - "type": "string" + "oneOf": [ + { + "enum": [ + "untrusted", + "on-failure", + "on-request", + "never" + ], + "type": "string" + }, + { + "additionalProperties": false, + "properties": { + "reject": { + "properties": { + "mcp_elicitations": { + "type": "boolean" + }, + "rules": { + "type": "boolean" + }, + "sandbox_approval": { + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + } + ] }, "AuthMode": { "description": "Authentication mode for OpenAI-backed providers.", diff --git a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationParams.json b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationParams.json index cd52a492e5..b712352776 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationParams.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationParams.json @@ -25,6 +25,20 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "Fine-grained rejection controls for approval prompts.\n\nWhen a field is `true`, prompts of that category are automatically rejected instead of shown to the user.", + "properties": { + "reject": { + "$ref": "#/definitions/RejectConfig" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + }, { "description": "Never ask the user to approve commands. Failures are immediately returned to the model, and never escalated to the user for approval.", "enum": [ @@ -114,6 +128,28 @@ }, "type": "object" }, + "RejectConfig": { + "properties": { + "mcp_elicitations": { + "description": "Reject MCP elicitation prompts.", + "type": "boolean" + }, + "rules": { + "description": "Reject prompts triggered by execpolicy `prompt` rules.", + "type": "boolean" + }, + "sandbox_approval": { + "description": "Reject approval prompts related to sandbox escalation.", + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + }, "SandboxMode": { "enum": [ "read-only", diff --git a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json index c7018d5b7a..d7f1e1d837 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ForkConversationResponse.json @@ -117,6 +117,20 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "Fine-grained rejection controls for approval prompts.\n\nWhen a field is `true`, prompts of that category are automatically rejected instead of shown to the user.", + "properties": { + "reject": { + "$ref": "#/definitions/RejectConfig" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + }, { "description": "Never ask the user to approve commands. Failures are immediately returned to the model, and never escalated to the user for approval.", "enum": [ @@ -3769,6 +3783,28 @@ } ] }, + "RejectConfig": { + "properties": { + "mcp_elicitations": { + "description": "Reject MCP elicitation prompts.", + "type": "boolean" + }, + "rules": { + "description": "Reject prompts triggered by execpolicy `prompt` rules.", + "type": "boolean" + }, + "sandbox_approval": { + "description": "Reject approval prompts related to sandbox escalation.", + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + }, "RemoteSkillSummary": { "properties": { "description": { diff --git a/codex-rs/app-server-protocol/schema/json/v1/GetUserSavedConfigResponse.json b/codex-rs/app-server-protocol/schema/json/v1/GetUserSavedConfigResponse.json index beec3a22cc..2f6d8b3b5f 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/GetUserSavedConfigResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/GetUserSavedConfigResponse.json @@ -29,6 +29,20 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "Fine-grained rejection controls for approval prompts.\n\nWhen a field is `true`, prompts of that category are automatically rejected instead of shown to the user.", + "properties": { + "reject": { + "$ref": "#/definitions/RejectConfig" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + }, { "description": "Never ask the user to approve commands. Failures are immediately returned to the model, and never escalated to the user for approval.", "enum": [ @@ -140,6 +154,28 @@ } ] }, + "RejectConfig": { + "properties": { + "mcp_elicitations": { + "description": "Reject MCP elicitation prompts.", + "type": "boolean" + }, + "rules": { + "description": "Reject prompts triggered by execpolicy `prompt` rules.", + "type": "boolean" + }, + "sandbox_approval": { + "description": "Reject approval prompts related to sandbox escalation.", + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + }, "SandboxMode": { "enum": [ "read-only", diff --git a/codex-rs/app-server-protocol/schema/json/v1/NewConversationParams.json b/codex-rs/app-server-protocol/schema/json/v1/NewConversationParams.json index 327ecd23b2..c9c8caaa26 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/NewConversationParams.json +++ b/codex-rs/app-server-protocol/schema/json/v1/NewConversationParams.json @@ -25,6 +25,20 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "Fine-grained rejection controls for approval prompts.\n\nWhen a field is `true`, prompts of that category are automatically rejected instead of shown to the user.", + "properties": { + "reject": { + "$ref": "#/definitions/RejectConfig" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + }, { "description": "Never ask the user to approve commands. Failures are immediately returned to the model, and never escalated to the user for approval.", "enum": [ @@ -34,6 +48,28 @@ } ] }, + "RejectConfig": { + "properties": { + "mcp_elicitations": { + "description": "Reject MCP elicitation prompts.", + "type": "boolean" + }, + "rules": { + "description": "Reject prompts triggered by execpolicy `prompt` rules.", + "type": "boolean" + }, + "sandbox_approval": { + "description": "Reject approval prompts related to sandbox escalation.", + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + }, "SandboxMode": { "enum": [ "read-only", diff --git a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationParams.json b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationParams.json index 9f14c22084..a45e1e60ee 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationParams.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationParams.json @@ -25,6 +25,20 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "Fine-grained rejection controls for approval prompts.\n\nWhen a field is `true`, prompts of that category are automatically rejected instead of shown to the user.", + "properties": { + "reject": { + "$ref": "#/definitions/RejectConfig" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + }, { "description": "Never ask the user to approve commands. Failures are immediately returned to the model, and never escalated to the user for approval.", "enum": [ @@ -437,6 +451,28 @@ } ] }, + "RejectConfig": { + "properties": { + "mcp_elicitations": { + "description": "Reject MCP elicitation prompts.", + "type": "boolean" + }, + "rules": { + "description": "Reject prompts triggered by execpolicy `prompt` rules.", + "type": "boolean" + }, + "sandbox_approval": { + "description": "Reject approval prompts related to sandbox escalation.", + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + }, "ResponseItem": { "oneOf": [ { diff --git a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json index 2d7cc7e701..9cbd492d99 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v1/ResumeConversationResponse.json @@ -117,6 +117,20 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "Fine-grained rejection controls for approval prompts.\n\nWhen a field is `true`, prompts of that category are automatically rejected instead of shown to the user.", + "properties": { + "reject": { + "$ref": "#/definitions/RejectConfig" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + }, { "description": "Never ask the user to approve commands. Failures are immediately returned to the model, and never escalated to the user for approval.", "enum": [ @@ -3769,6 +3783,28 @@ } ] }, + "RejectConfig": { + "properties": { + "mcp_elicitations": { + "description": "Reject MCP elicitation prompts.", + "type": "boolean" + }, + "rules": { + "description": "Reject prompts triggered by execpolicy `prompt` rules.", + "type": "boolean" + }, + "sandbox_approval": { + "description": "Reject approval prompts related to sandbox escalation.", + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + }, "RemoteSkillSummary": { "properties": { "description": { diff --git a/codex-rs/app-server-protocol/schema/json/v1/SendUserTurnParams.json b/codex-rs/app-server-protocol/schema/json/v1/SendUserTurnParams.json index aa112e7319..a213f6077e 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/SendUserTurnParams.json +++ b/codex-rs/app-server-protocol/schema/json/v1/SendUserTurnParams.json @@ -29,6 +29,20 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "Fine-grained rejection controls for approval prompts.\n\nWhen a field is `true`, prompts of that category are automatically rejected instead of shown to the user.", + "properties": { + "reject": { + "$ref": "#/definitions/RejectConfig" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + }, { "description": "Never ask the user to approve commands. Failures are immediately returned to the model, and never escalated to the user for approval.", "enum": [ @@ -225,6 +239,28 @@ } ] }, + "RejectConfig": { + "properties": { + "mcp_elicitations": { + "description": "Reject MCP elicitation prompts.", + "type": "boolean" + }, + "rules": { + "description": "Reject prompts triggered by execpolicy `prompt` rules.", + "type": "boolean" + }, + "sandbox_approval": { + "description": "Reject approval prompts related to sandbox escalation.", + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + }, "SandboxPolicy": { "description": "Determines execution restrictions for model shell commands.", "oneOf": [ diff --git a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json index efd614139d..fd66f6060e 100644 --- a/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json +++ b/codex-rs/app-server-protocol/schema/json/v1/SessionConfiguredNotification.json @@ -117,6 +117,20 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "Fine-grained rejection controls for approval prompts.\n\nWhen a field is `true`, prompts of that category are automatically rejected instead of shown to the user.", + "properties": { + "reject": { + "$ref": "#/definitions/RejectConfig" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + }, { "description": "Never ask the user to approve commands. Failures are immediately returned to the model, and never escalated to the user for approval.", "enum": [ @@ -3769,6 +3783,28 @@ } ] }, + "RejectConfig": { + "properties": { + "mcp_elicitations": { + "description": "Reject MCP elicitation prompts.", + "type": "boolean" + }, + "rules": { + "description": "Reject prompts triggered by execpolicy `prompt` rules.", + "type": "boolean" + }, + "sandbox_approval": { + "description": "Reject approval prompts related to sandbox escalation.", + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + }, "RemoteSkillSummary": { "properties": { "description": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json index d934df7f58..293fe7dc31 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigReadResponse.json @@ -47,13 +47,46 @@ "type": "object" }, "AskForApproval": { - "enum": [ - "untrusted", - "on-failure", - "on-request", - "never" - ], - "type": "string" + "oneOf": [ + { + "enum": [ + "untrusted", + "on-failure", + "on-request", + "never" + ], + "type": "string" + }, + { + "additionalProperties": false, + "properties": { + "reject": { + "properties": { + "mcp_elicitations": { + "type": "boolean" + }, + "rules": { + "type": "boolean" + }, + "sandbox_approval": { + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + } + ] }, "Config": { "additionalProperties": true, diff --git a/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json index 3ba96a852a..4d8a967375 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ConfigRequirementsReadResponse.json @@ -2,13 +2,46 @@ "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { "AskForApproval": { - "enum": [ - "untrusted", - "on-failure", - "on-request", - "never" - ], - "type": "string" + "oneOf": [ + { + "enum": [ + "untrusted", + "on-failure", + "on-request", + "never" + ], + "type": "string" + }, + { + "additionalProperties": false, + "properties": { + "reject": { + "properties": { + "mcp_elicitations": { + "type": "boolean" + }, + "rules": { + "type": "boolean" + }, + "sandbox_approval": { + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + } + ] }, "ConfigRequirements": { "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkParams.json index 5638468a4c..2b832d5bf1 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkParams.json @@ -2,13 +2,46 @@ "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { "AskForApproval": { - "enum": [ - "untrusted", - "on-failure", - "on-request", - "never" - ], - "type": "string" + "oneOf": [ + { + "enum": [ + "untrusted", + "on-failure", + "on-request", + "never" + ], + "type": "string" + }, + { + "additionalProperties": false, + "properties": { + "reject": { + "properties": { + "mcp_elicitations": { + "type": "boolean" + }, + "rules": { + "type": "boolean" + }, + "sandbox_approval": { + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + } + ] }, "SandboxMode": { "enum": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json index f7db8aa66d..2dfc446ac2 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadForkResponse.json @@ -6,13 +6,46 @@ "type": "string" }, "AskForApproval": { - "enum": [ - "untrusted", - "on-failure", - "on-request", - "never" - ], - "type": "string" + "oneOf": [ + { + "enum": [ + "untrusted", + "on-failure", + "on-request", + "never" + ], + "type": "string" + }, + { + "additionalProperties": false, + "properties": { + "reject": { + "properties": { + "mcp_elicitations": { + "type": "boolean" + }, + "rules": { + "type": "boolean" + }, + "sandbox_approval": { + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + } + ] }, "ByteRange": { "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json index fc6593b186..29d6fbc6d7 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeParams.json @@ -2,13 +2,46 @@ "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { "AskForApproval": { - "enum": [ - "untrusted", - "on-failure", - "on-request", - "never" - ], - "type": "string" + "oneOf": [ + { + "enum": [ + "untrusted", + "on-failure", + "on-request", + "never" + ], + "type": "string" + }, + { + "additionalProperties": false, + "properties": { + "reject": { + "properties": { + "mcp_elicitations": { + "type": "boolean" + }, + "rules": { + "type": "boolean" + }, + "sandbox_approval": { + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + } + ] }, "ContentItem": { "oneOf": [ diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json index 198b91f054..2343766326 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadResumeResponse.json @@ -6,13 +6,46 @@ "type": "string" }, "AskForApproval": { - "enum": [ - "untrusted", - "on-failure", - "on-request", - "never" - ], - "type": "string" + "oneOf": [ + { + "enum": [ + "untrusted", + "on-failure", + "on-request", + "never" + ], + "type": "string" + }, + { + "additionalProperties": false, + "properties": { + "reject": { + "properties": { + "mcp_elicitations": { + "type": "boolean" + }, + "rules": { + "type": "boolean" + }, + "sandbox_approval": { + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + } + ] }, "ByteRange": { "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json index 1574c0710e..6b9c1568f0 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartParams.json @@ -2,13 +2,46 @@ "$schema": "http://json-schema.org/draft-07/schema#", "definitions": { "AskForApproval": { - "enum": [ - "untrusted", - "on-failure", - "on-request", - "never" - ], - "type": "string" + "oneOf": [ + { + "enum": [ + "untrusted", + "on-failure", + "on-request", + "never" + ], + "type": "string" + }, + { + "additionalProperties": false, + "properties": { + "reject": { + "properties": { + "mcp_elicitations": { + "type": "boolean" + }, + "rules": { + "type": "boolean" + }, + "sandbox_approval": { + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + } + ] }, "DynamicToolSpec": { "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json index 00a619927c..1b617a8508 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/ThreadStartResponse.json @@ -6,13 +6,46 @@ "type": "string" }, "AskForApproval": { - "enum": [ - "untrusted", - "on-failure", - "on-request", - "never" - ], - "type": "string" + "oneOf": [ + { + "enum": [ + "untrusted", + "on-failure", + "on-request", + "never" + ], + "type": "string" + }, + { + "additionalProperties": false, + "properties": { + "reject": { + "properties": { + "mcp_elicitations": { + "type": "boolean" + }, + "rules": { + "type": "boolean" + }, + "sandbox_approval": { + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + } + ] }, "ByteRange": { "properties": { diff --git a/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json b/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json index a1363f97d7..bc15473cc0 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json +++ b/codex-rs/app-server-protocol/schema/json/v2/TurnStartParams.json @@ -6,13 +6,46 @@ "type": "string" }, "AskForApproval": { - "enum": [ - "untrusted", - "on-failure", - "on-request", - "never" - ], - "type": "string" + "oneOf": [ + { + "enum": [ + "untrusted", + "on-failure", + "on-request", + "never" + ], + "type": "string" + }, + { + "additionalProperties": false, + "properties": { + "reject": { + "properties": { + "mcp_elicitations": { + "type": "boolean" + }, + "rules": { + "type": "boolean" + }, + "sandbox_approval": { + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + } + }, + "required": [ + "reject" + ], + "title": "RejectAskForApproval", + "type": "object" + } + ] }, "ByteRange": { "properties": { diff --git a/codex-rs/app-server-protocol/schema/typescript/AskForApproval.ts b/codex-rs/app-server-protocol/schema/typescript/AskForApproval.ts index b21e86fd70..227eb44e77 100644 --- a/codex-rs/app-server-protocol/schema/typescript/AskForApproval.ts +++ b/codex-rs/app-server-protocol/schema/typescript/AskForApproval.ts @@ -1,9 +1,10 @@ // GENERATED CODE! DO NOT MODIFY BY HAND! // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. +import type { RejectConfig } from "./RejectConfig"; /** * Determines the conditions under which the user is consulted to approve * running the command proposed by Codex. */ -export type AskForApproval = "untrusted" | "on-failure" | "on-request" | "never"; +export type AskForApproval = "untrusted" | "on-failure" | "on-request" | { "reject": RejectConfig } | "never"; diff --git a/codex-rs/app-server-protocol/schema/typescript/RejectConfig.ts b/codex-rs/app-server-protocol/schema/typescript/RejectConfig.ts new file mode 100644 index 0000000000..320f1096b0 --- /dev/null +++ b/codex-rs/app-server-protocol/schema/typescript/RejectConfig.ts @@ -0,0 +1,17 @@ +// 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. + +export type RejectConfig = { +/** + * Reject approval prompts related to sandbox escalation. + */ +sandbox_approval: boolean, +/** + * Reject prompts triggered by execpolicy `prompt` rules. + */ +rules: boolean, +/** + * Reject MCP elicitation prompts. + */ +mcp_elicitations: boolean, }; diff --git a/codex-rs/app-server-protocol/schema/typescript/index.ts b/codex-rs/app-server-protocol/schema/typescript/index.ts index 2a626d3638..79d8bdd98e 100644 --- a/codex-rs/app-server-protocol/schema/typescript/index.ts +++ b/codex-rs/app-server-protocol/schema/typescript/index.ts @@ -153,6 +153,7 @@ export type { ReasoningItemContent } from "./ReasoningItemContent"; export type { ReasoningItemReasoningSummary } from "./ReasoningItemReasoningSummary"; export type { ReasoningRawContentDeltaEvent } from "./ReasoningRawContentDeltaEvent"; export type { ReasoningSummary } from "./ReasoningSummary"; +export type { RejectConfig } from "./RejectConfig"; export type { RemoteSkillDownloadedEvent } from "./RemoteSkillDownloadedEvent"; export type { RemoteSkillSummary } from "./RemoteSkillSummary"; export type { RemoveConversationListenerParams } from "./RemoveConversationListenerParams"; diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/AskForApproval.ts b/codex-rs/app-server-protocol/schema/typescript/v2/AskForApproval.ts index d3c3e77e39..b5f1bacdfa 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/AskForApproval.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/AskForApproval.ts @@ -2,4 +2,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. -export type AskForApproval = "untrusted" | "on-failure" | "on-request" | "never"; +export type AskForApproval = "untrusted" | "on-failure" | "on-request" | { "reject": { sandbox_approval: boolean, rules: boolean, mcp_elicitations: boolean, } } | "never"; diff --git a/codex-rs/app-server-protocol/src/protocol/thread_history.rs b/codex-rs/app-server-protocol/src/protocol/thread_history.rs index e5005077e5..52850a67e4 100644 --- a/codex-rs/app-server-protocol/src/protocol/thread_history.rs +++ b/codex-rs/app-server-protocol/src/protocol/thread_history.rs @@ -24,9 +24,13 @@ use codex_protocol::protocol::CompactedItem; use codex_protocol::protocol::ContextCompactedEvent; use codex_protocol::protocol::ErrorEvent; use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::ExecCommandBeginEvent; use codex_protocol::protocol::ExecCommandEndEvent; use codex_protocol::protocol::ItemCompletedEvent; +use codex_protocol::protocol::ItemStartedEvent; +use codex_protocol::protocol::McpToolCallBeginEvent; use codex_protocol::protocol::McpToolCallEndEvent; +use codex_protocol::protocol::PatchApplyBeginEvent; use codex_protocol::protocol::PatchApplyEndEvent; use codex_protocol::protocol::ReviewOutputEvent; use codex_protocol::protocol::RolloutItem; @@ -36,8 +40,10 @@ use codex_protocol::protocol::TurnCompleteEvent; use codex_protocol::protocol::TurnStartedEvent; use codex_protocol::protocol::UserMessageEvent; use codex_protocol::protocol::ViewImageToolCallEvent; +use codex_protocol::protocol::WebSearchBeginEvent; use codex_protocol::protocol::WebSearchEndEvent; use std::collections::HashMap; +use tracing::warn; use uuid::Uuid; #[cfg(test)] @@ -57,14 +63,20 @@ pub fn build_turns_from_rollout_items(items: &[RolloutItem]) -> Vec { builder.finish() } -struct ThreadHistoryBuilder { +pub struct ThreadHistoryBuilder { turns: Vec, current_turn: Option, next_item_index: i64, } +impl Default for ThreadHistoryBuilder { + fn default() -> Self { + Self::new() + } +} + impl ThreadHistoryBuilder { - fn new() -> Self { + pub fn new() -> Self { Self { turns: Vec::new(), current_turn: None, @@ -72,14 +84,32 @@ impl ThreadHistoryBuilder { } } - fn finish(mut self) -> Vec { + pub fn reset(&mut self) { + *self = Self::new(); + } + + pub fn finish(mut self) -> Vec { self.finish_current_turn(); self.turns } + pub fn active_turn_snapshot(&self) -> Option { + self.current_turn + .as_ref() + .map(Turn::from) + .or_else(|| self.turns.last().cloned()) + } + + pub fn has_active_turn(&self) -> bool { + self.current_turn.is_some() + } + + /// Shared reducer for persisted rollout replay and in-memory current-turn + /// tracking used by running thread resume/rejoin. + /// /// This function should handle all EventMsg variants that can be persisted in a rollout file. /// See `should_persist_event_msg` in `codex-rs/core/rollout/policy.rs`. - fn handle_event(&mut self, event: &EventMsg) { + pub fn handle_event(&mut self, event: &EventMsg) { match event { EventMsg::UserMessage(payload) => self.handle_user_message(payload), EventMsg::AgentMessage(payload) => { @@ -89,21 +119,35 @@ impl ThreadHistoryBuilder { EventMsg::AgentReasoningRawContent(payload) => { self.handle_agent_reasoning_raw_content(payload) } + EventMsg::WebSearchBegin(payload) => self.handle_web_search_begin(payload), EventMsg::WebSearchEnd(payload) => self.handle_web_search_end(payload), + EventMsg::ExecCommandBegin(payload) => self.handle_exec_command_begin(payload), EventMsg::ExecCommandEnd(payload) => self.handle_exec_command_end(payload), + EventMsg::PatchApplyBegin(payload) => self.handle_patch_apply_begin(payload), EventMsg::PatchApplyEnd(payload) => self.handle_patch_apply_end(payload), + EventMsg::McpToolCallBegin(payload) => self.handle_mcp_tool_call_begin(payload), EventMsg::McpToolCallEnd(payload) => self.handle_mcp_tool_call_end(payload), EventMsg::ViewImageToolCall(payload) => self.handle_view_image_tool_call(payload), + EventMsg::CollabAgentSpawnBegin(payload) => { + self.handle_collab_agent_spawn_begin(payload) + } EventMsg::CollabAgentSpawnEnd(payload) => self.handle_collab_agent_spawn_end(payload), + EventMsg::CollabAgentInteractionBegin(payload) => { + self.handle_collab_agent_interaction_begin(payload) + } EventMsg::CollabAgentInteractionEnd(payload) => { self.handle_collab_agent_interaction_end(payload) } + EventMsg::CollabWaitingBegin(payload) => self.handle_collab_waiting_begin(payload), EventMsg::CollabWaitingEnd(payload) => self.handle_collab_waiting_end(payload), + EventMsg::CollabCloseBegin(payload) => self.handle_collab_close_begin(payload), EventMsg::CollabCloseEnd(payload) => self.handle_collab_close_end(payload), + EventMsg::CollabResumeBegin(payload) => self.handle_collab_resume_begin(payload), EventMsg::CollabResumeEnd(payload) => self.handle_collab_resume_end(payload), EventMsg::ContextCompacted(payload) => self.handle_context_compacted(payload), EventMsg::EnteredReviewMode(payload) => self.handle_entered_review_mode(payload), EventMsg::ExitedReviewMode(payload) => self.handle_exited_review_mode(payload), + EventMsg::ItemStarted(payload) => self.handle_item_started(payload), EventMsg::ItemCompleted(payload) => self.handle_item_completed(payload), EventMsg::Error(payload) => self.handle_error(payload), EventMsg::TokenCount(_) => {} @@ -116,7 +160,7 @@ impl ThreadHistoryBuilder { } } - fn handle_rollout_item(&mut self, item: &RolloutItem) { + pub fn handle_rollout_item(&mut self, item: &RolloutItem) { match item { RolloutItem::EventMsg(event) => self.handle_event(event), RolloutItem::Compacted(payload) => self.handle_compacted(payload), @@ -199,15 +243,51 @@ impl ThreadHistoryBuilder { }); } - fn handle_item_completed(&mut self, payload: &ItemCompletedEvent) { - if let codex_protocol::items::TurnItem::Plan(plan) = &payload.item - && plan.text.is_empty() - { - return; + fn handle_item_started(&mut self, payload: &ItemStartedEvent) { + match &payload.item { + codex_protocol::items::TurnItem::Plan(plan) => { + if plan.text.is_empty() { + return; + } + self.upsert_item_in_turn_id( + &payload.turn_id, + ThreadItem::from(payload.item.clone()), + ); + } + codex_protocol::items::TurnItem::UserMessage(_) + | codex_protocol::items::TurnItem::AgentMessage(_) + | codex_protocol::items::TurnItem::Reasoning(_) + | codex_protocol::items::TurnItem::WebSearch(_) + | codex_protocol::items::TurnItem::ContextCompaction(_) => {} } + } - let item = ThreadItem::from(payload.item.clone()); - self.ensure_turn().items.push(item); + fn handle_item_completed(&mut self, payload: &ItemCompletedEvent) { + match &payload.item { + codex_protocol::items::TurnItem::Plan(plan) => { + if plan.text.is_empty() { + return; + } + self.upsert_item_in_turn_id( + &payload.turn_id, + ThreadItem::from(payload.item.clone()), + ); + } + codex_protocol::items::TurnItem::UserMessage(_) + | codex_protocol::items::TurnItem::AgentMessage(_) + | codex_protocol::items::TurnItem::Reasoning(_) + | codex_protocol::items::TurnItem::WebSearch(_) + | codex_protocol::items::TurnItem::ContextCompaction(_) => {} + } + } + + fn handle_web_search_begin(&mut self, payload: &WebSearchBeginEvent) { + let item = ThreadItem::WebSearch { + id: payload.call_id.clone(), + query: String::new(), + action: None, + }; + self.upsert_item_in_current_turn(item); } fn handle_web_search_end(&mut self, payload: &WebSearchEndEvent) { @@ -216,7 +296,30 @@ impl ThreadHistoryBuilder { query: payload.query.clone(), action: Some(WebSearchAction::from(payload.action.clone())), }; - self.ensure_turn().items.push(item); + self.upsert_item_in_current_turn(item); + } + + fn handle_exec_command_begin(&mut self, payload: &ExecCommandBeginEvent) { + let command = shlex::try_join(payload.command.iter().map(String::as_str)) + .unwrap_or_else(|_| payload.command.join(" ")); + let command_actions = payload + .parsed_cmd + .iter() + .cloned() + .map(CommandAction::from) + .collect(); + let item = ThreadItem::CommandExecution { + id: payload.call_id.clone(), + command, + cwd: payload.cwd.clone(), + process_id: payload.process_id.clone(), + status: CommandExecutionStatus::InProgress, + command_actions, + aggregated_output: None, + exit_code: None, + duration_ms: None, + }; + self.upsert_item_in_turn_id(&payload.turn_id, item); } fn handle_exec_command_end(&mut self, payload: &ExecCommandEndEvent) { @@ -246,33 +349,25 @@ impl ThreadHistoryBuilder { exit_code: Some(payload.exit_code), duration_ms: Some(duration_ms), }; - // Command completions can arrive out of order. Unified exec may return // while a PTY is still running, then emit ExecCommandEnd later from a // background exit watcher when that process finally exits. By then, a // newer user turn may already have started. Route by event turn_id so // replay preserves the original turn association. - if let Some(turn) = self.current_turn.as_mut() - && turn.id == payload.turn_id - { - turn.items.push(item); - return; - } + self.upsert_item_in_turn_id(&payload.turn_id, item); + } - // If the originating turn is already finalized, append there instead - // of attaching to whichever turn is currently active during replay. - if let Some(turn) = self - .turns - .iter_mut() - .find(|turn| turn.id == payload.turn_id) - { - turn.items.push(item); - return; + fn handle_patch_apply_begin(&mut self, payload: &PatchApplyBeginEvent) { + let item = ThreadItem::FileChange { + id: payload.call_id.clone(), + changes: convert_patch_changes(&payload.changes), + status: PatchApplyStatus::InProgress, + }; + if payload.turn_id.is_empty() { + self.upsert_item_in_current_turn(item); + } else { + self.upsert_item_in_turn_id(&payload.turn_id, item); } - - // Backward-compatibility fallback for partial/legacy streams where the - // event turn_id does not match any known replay turn. - self.ensure_turn().items.push(item); } fn handle_patch_apply_end(&mut self, payload: &PatchApplyEndEvent) { @@ -282,7 +377,29 @@ impl ThreadHistoryBuilder { changes: convert_patch_changes(&payload.changes), status, }; - self.ensure_turn().items.push(item); + if payload.turn_id.is_empty() { + self.upsert_item_in_current_turn(item); + } else { + self.upsert_item_in_turn_id(&payload.turn_id, item); + } + } + + fn handle_mcp_tool_call_begin(&mut self, payload: &McpToolCallBeginEvent) { + let item = ThreadItem::McpToolCall { + id: payload.call_id.clone(), + server: payload.invocation.server.clone(), + tool: payload.invocation.tool.clone(), + status: McpToolCallStatus::InProgress, + arguments: payload + .invocation + .arguments + .clone() + .unwrap_or(serde_json::Value::Null), + result: None, + error: None, + duration_ms: None, + }; + self.upsert_item_in_current_turn(item); } fn handle_mcp_tool_call_end(&mut self, payload: &McpToolCallEndEvent) { @@ -321,7 +438,7 @@ impl ThreadHistoryBuilder { error, duration_ms, }; - self.ensure_turn().items.push(item); + self.upsert_item_in_current_turn(item); } fn handle_view_image_tool_call(&mut self, payload: &ViewImageToolCallEvent) { @@ -329,7 +446,23 @@ impl ThreadHistoryBuilder { id: payload.call_id.clone(), path: payload.path.to_string_lossy().into_owned(), }; - self.ensure_turn().items.push(item); + self.upsert_item_in_current_turn(item); + } + + fn handle_collab_agent_spawn_begin( + &mut self, + payload: &codex_protocol::protocol::CollabAgentSpawnBeginEvent, + ) { + let item = ThreadItem::CollabAgentToolCall { + id: payload.call_id.clone(), + tool: CollabAgentTool::SpawnAgent, + status: CollabAgentToolCallStatus::InProgress, + sender_thread_id: payload.sender_thread_id.to_string(), + receiver_thread_ids: Vec::new(), + prompt: Some(payload.prompt.clone()), + agents_states: HashMap::new(), + }; + self.upsert_item_in_current_turn(item); } fn handle_collab_agent_spawn_end( @@ -353,17 +486,31 @@ impl ThreadHistoryBuilder { } None => (Vec::new(), HashMap::new()), }; - self.ensure_turn() - .items - .push(ThreadItem::CollabAgentToolCall { - id: payload.call_id.clone(), - tool: CollabAgentTool::SpawnAgent, - status, - sender_thread_id: payload.sender_thread_id.to_string(), - receiver_thread_ids, - prompt: Some(payload.prompt.clone()), - agents_states, - }); + self.upsert_item_in_current_turn(ThreadItem::CollabAgentToolCall { + id: payload.call_id.clone(), + tool: CollabAgentTool::SpawnAgent, + status, + sender_thread_id: payload.sender_thread_id.to_string(), + receiver_thread_ids, + prompt: Some(payload.prompt.clone()), + agents_states, + }); + } + + fn handle_collab_agent_interaction_begin( + &mut self, + payload: &codex_protocol::protocol::CollabAgentInteractionBeginEvent, + ) { + let item = ThreadItem::CollabAgentToolCall { + id: payload.call_id.clone(), + tool: CollabAgentTool::SendInput, + status: CollabAgentToolCallStatus::InProgress, + sender_thread_id: payload.sender_thread_id.to_string(), + receiver_thread_ids: vec![payload.receiver_thread_id.to_string()], + prompt: Some(payload.prompt.clone()), + agents_states: HashMap::new(), + }; + self.upsert_item_in_current_turn(item); } fn handle_collab_agent_interaction_end( @@ -376,17 +523,35 @@ impl ThreadHistoryBuilder { }; let receiver_id = payload.receiver_thread_id.to_string(); let received_status = CollabAgentState::from(payload.status.clone()); - self.ensure_turn() - .items - .push(ThreadItem::CollabAgentToolCall { - id: payload.call_id.clone(), - tool: CollabAgentTool::SendInput, - status, - sender_thread_id: payload.sender_thread_id.to_string(), - receiver_thread_ids: vec![receiver_id.clone()], - prompt: Some(payload.prompt.clone()), - agents_states: [(receiver_id, received_status)].into_iter().collect(), - }); + self.upsert_item_in_current_turn(ThreadItem::CollabAgentToolCall { + id: payload.call_id.clone(), + tool: CollabAgentTool::SendInput, + status, + sender_thread_id: payload.sender_thread_id.to_string(), + receiver_thread_ids: vec![receiver_id.clone()], + prompt: Some(payload.prompt.clone()), + agents_states: [(receiver_id, received_status)].into_iter().collect(), + }); + } + + fn handle_collab_waiting_begin( + &mut self, + payload: &codex_protocol::protocol::CollabWaitingBeginEvent, + ) { + let item = ThreadItem::CollabAgentToolCall { + id: payload.call_id.clone(), + tool: CollabAgentTool::Wait, + status: CollabAgentToolCallStatus::InProgress, + sender_thread_id: payload.sender_thread_id.to_string(), + receiver_thread_ids: payload + .receiver_thread_ids + .iter() + .map(ToString::to_string) + .collect(), + prompt: None, + agents_states: HashMap::new(), + }; + self.upsert_item_in_current_turn(item); } fn handle_collab_waiting_end( @@ -410,17 +575,31 @@ impl ThreadHistoryBuilder { .iter() .map(|(id, status)| (id.to_string(), CollabAgentState::from(status.clone()))) .collect(); - self.ensure_turn() - .items - .push(ThreadItem::CollabAgentToolCall { - id: payload.call_id.clone(), - tool: CollabAgentTool::Wait, - status, - sender_thread_id: payload.sender_thread_id.to_string(), - receiver_thread_ids, - prompt: None, - agents_states, - }); + self.upsert_item_in_current_turn(ThreadItem::CollabAgentToolCall { + id: payload.call_id.clone(), + tool: CollabAgentTool::Wait, + status, + sender_thread_id: payload.sender_thread_id.to_string(), + receiver_thread_ids, + prompt: None, + agents_states, + }); + } + + fn handle_collab_close_begin( + &mut self, + payload: &codex_protocol::protocol::CollabCloseBeginEvent, + ) { + let item = ThreadItem::CollabAgentToolCall { + id: payload.call_id.clone(), + tool: CollabAgentTool::CloseAgent, + status: CollabAgentToolCallStatus::InProgress, + sender_thread_id: payload.sender_thread_id.to_string(), + receiver_thread_ids: vec![payload.receiver_thread_id.to_string()], + prompt: None, + agents_states: HashMap::new(), + }; + self.upsert_item_in_current_turn(item); } fn handle_collab_close_end(&mut self, payload: &codex_protocol::protocol::CollabCloseEndEvent) { @@ -435,17 +614,31 @@ impl ThreadHistoryBuilder { )] .into_iter() .collect(); - self.ensure_turn() - .items - .push(ThreadItem::CollabAgentToolCall { - id: payload.call_id.clone(), - tool: CollabAgentTool::CloseAgent, - status, - sender_thread_id: payload.sender_thread_id.to_string(), - receiver_thread_ids: vec![receiver_id], - prompt: None, - agents_states, - }); + self.upsert_item_in_current_turn(ThreadItem::CollabAgentToolCall { + id: payload.call_id.clone(), + tool: CollabAgentTool::CloseAgent, + status, + sender_thread_id: payload.sender_thread_id.to_string(), + receiver_thread_ids: vec![receiver_id], + prompt: None, + agents_states, + }); + } + + fn handle_collab_resume_begin( + &mut self, + payload: &codex_protocol::protocol::CollabResumeBeginEvent, + ) { + let item = ThreadItem::CollabAgentToolCall { + id: payload.call_id.clone(), + tool: CollabAgentTool::ResumeAgent, + status: CollabAgentToolCallStatus::InProgress, + sender_thread_id: payload.sender_thread_id.to_string(), + receiver_thread_ids: vec![payload.receiver_thread_id.to_string()], + prompt: None, + agents_states: HashMap::new(), + }; + self.upsert_item_in_current_turn(item); } fn handle_collab_resume_end( @@ -463,17 +656,15 @@ impl ThreadHistoryBuilder { )] .into_iter() .collect(); - self.ensure_turn() - .items - .push(ThreadItem::CollabAgentToolCall { - id: payload.call_id.clone(), - tool: CollabAgentTool::ResumeAgent, - status, - sender_thread_id: payload.sender_thread_id.to_string(), - receiver_thread_ids: vec![receiver_id], - prompt: None, - agents_states, - }); + self.upsert_item_in_current_turn(ThreadItem::CollabAgentToolCall { + id: payload.call_id.clone(), + tool: CollabAgentTool::ResumeAgent, + status, + sender_thread_id: payload.sender_thread_id.to_string(), + receiver_thread_ids: vec![receiver_id], + prompt: None, + agents_states, + }); } fn handle_context_compacted(&mut self, _payload: &ContextCompactedEvent) { @@ -548,6 +739,7 @@ impl ThreadHistoryBuilder { self.finish_current_turn(); self.current_turn = Some( self.new_turn(Some(payload.turn_id.clone())) + .with_status(TurnStatus::InProgress) .opened_explicitly(), ); } @@ -642,6 +834,30 @@ impl ThreadHistoryBuilder { unreachable!("current turn must exist after initialization"); } + fn upsert_item_in_turn_id(&mut self, turn_id: &str, item: ThreadItem) { + if let Some(turn) = self.current_turn.as_mut() + && turn.id == turn_id + { + upsert_turn_item(&mut turn.items, item); + return; + } + + if let Some(turn) = self.turns.iter_mut().find(|turn| turn.id == turn_id) { + upsert_turn_item(&mut turn.items, item); + return; + } + + warn!( + item_id = item.id(), + "dropping turn-scoped item for unknown turn id `{turn_id}`" + ); + } + + fn upsert_item_in_current_turn(&mut self, item: ThreadItem) { + let turn = self.ensure_turn(); + upsert_turn_item(&mut turn.items, item); + } + fn next_item_id(&mut self) -> String { let id = format!("item-{}", self.next_item_index); self.next_item_index += 1; @@ -684,7 +900,7 @@ fn render_review_output_text(output: &ReviewOutputEvent) -> String { } } -fn convert_patch_changes( +pub fn convert_patch_changes( changes: &HashMap, ) -> Vec { let mut converted: Vec = changes @@ -726,6 +942,17 @@ fn format_file_change_diff(change: &codex_protocol::protocol::FileChange) -> Str } } +fn upsert_turn_item(items: &mut Vec, item: ThreadItem) { + if let Some(existing_item) = items + .iter_mut() + .find(|existing_item| existing_item.id() == item.id()) + { + *existing_item = item; + return; + } + items.push(item); +} + struct PendingTurn { id: String, items: Vec, @@ -744,6 +971,11 @@ impl PendingTurn { self.opened_explicitly = true; self } + + fn with_status(mut self, status: TurnStatus) -> Self { + self.status = status; + self + } } impl From for Turn { @@ -757,10 +989,23 @@ impl From for Turn { } } +impl From<&PendingTurn> for Turn { + fn from(value: &PendingTurn) -> Self { + Self { + id: value.id.clone(), + items: value.items.clone(), + error: value.error.clone(), + status: value.status.clone(), + } + } +} + #[cfg(test)] mod tests { use super::*; use codex_protocol::ThreadId; + use codex_protocol::items::TurnItem as CoreTurnItem; + use codex_protocol::items::UserMessageItem as CoreUserMessageItem; use codex_protocol::models::MessagePhase as CoreMessagePhase; use codex_protocol::models::WebSearchAction as CoreWebSearchAction; use codex_protocol::parse_command::ParsedCommand; @@ -771,6 +1016,7 @@ mod tests { use codex_protocol::protocol::CompactedItem; use codex_protocol::protocol::ExecCommandEndEvent; use codex_protocol::protocol::ExecCommandSource; + use codex_protocol::protocol::ItemStartedEvent; use codex_protocol::protocol::McpInvocation; use codex_protocol::protocol::McpToolCallEndEvent; use codex_protocol::protocol::ThreadRolledBackEvent; @@ -816,11 +1062,11 @@ mod tests { }), ]; - let items = events - .into_iter() - .map(RolloutItem::EventMsg) - .collect::>(); - let turns = build_turns_from_rollout_items(&items); + let mut builder = ThreadHistoryBuilder::new(); + for event in &events { + builder.handle_event(event); + } + let turns = builder.finish(); assert_eq!(turns.len(), 2); let first = &turns[0]; @@ -883,6 +1129,55 @@ mod tests { ); } + #[test] + fn ignores_non_plan_item_lifecycle_events() { + let turn_id = "turn-1"; + let thread_id = ThreadId::new(); + let events = vec![ + EventMsg::TurnStarted(TurnStartedEvent { + turn_id: turn_id.to_string(), + model_context_window: None, + collaboration_mode_kind: Default::default(), + }), + EventMsg::UserMessage(UserMessageEvent { + message: "hello".into(), + images: None, + text_elements: Vec::new(), + local_images: Vec::new(), + }), + EventMsg::ItemStarted(ItemStartedEvent { + thread_id, + turn_id: turn_id.to_string(), + item: CoreTurnItem::UserMessage(CoreUserMessageItem { + id: "user-item-id".to_string(), + content: Vec::new(), + }), + }), + EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: turn_id.to_string(), + last_agent_message: None, + }), + ]; + + let items = events + .into_iter() + .map(RolloutItem::EventMsg) + .collect::>(); + let turns = build_turns_from_rollout_items(&items); + assert_eq!(turns.len(), 1); + assert_eq!(turns[0].items.len(), 1); + assert_eq!( + turns[0].items[0], + ThreadItem::UserMessage { + id: "item-1".into(), + content: vec![UserInput::Text { + text: "hello".into(), + text_elements: Vec::new(), + }], + } + ); + } + #[test] fn preserves_agent_message_phase_in_history() { let events = vec![EventMsg::AgentMessage(AgentMessageEvent { @@ -1212,6 +1507,11 @@ mod tests { #[test] fn reconstructs_tool_items_from_persisted_completion_events() { let events = vec![ + EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".into(), + model_context_window: None, + collaboration_mode_kind: Default::default(), + }), EventMsg::UserMessage(UserMessageEvent { message: "run tools".into(), images: None, @@ -1311,6 +1611,11 @@ mod tests { #[test] fn reconstructs_declined_exec_and_patch_items() { let events = vec![ + EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-1".into(), + model_context_window: None, + collaboration_mode_kind: Default::default(), + }), EventMsg::UserMessage(UserMessageEvent { message: "run tools".into(), images: None, @@ -1471,6 +1776,82 @@ mod tests { ); } + #[test] + fn drops_late_turn_scoped_item_for_unknown_turn_id() { + let events = vec![ + EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-a".into(), + model_context_window: None, + collaboration_mode_kind: Default::default(), + }), + EventMsg::UserMessage(UserMessageEvent { + message: "first".into(), + images: None, + text_elements: Vec::new(), + local_images: Vec::new(), + }), + EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-a".into(), + last_agent_message: None, + }), + EventMsg::TurnStarted(TurnStartedEvent { + turn_id: "turn-b".into(), + model_context_window: None, + collaboration_mode_kind: Default::default(), + }), + EventMsg::UserMessage(UserMessageEvent { + message: "second".into(), + images: None, + text_elements: Vec::new(), + local_images: Vec::new(), + }), + EventMsg::ExecCommandEnd(ExecCommandEndEvent { + call_id: "exec-unknown-turn".into(), + process_id: Some("pid-42".into()), + turn_id: "turn-missing".into(), + command: vec!["echo".into(), "done".into()], + cwd: PathBuf::from("/tmp"), + parsed_cmd: vec![ParsedCommand::Unknown { + cmd: "echo done".into(), + }], + source: ExecCommandSource::Agent, + interaction_input: None, + stdout: "done\n".into(), + stderr: String::new(), + aggregated_output: "done\n".into(), + exit_code: 0, + duration: Duration::from_millis(5), + formatted_output: "done\n".into(), + status: CoreExecCommandStatus::Completed, + }), + EventMsg::TurnComplete(TurnCompleteEvent { + turn_id: "turn-b".into(), + last_agent_message: None, + }), + ]; + + let mut builder = ThreadHistoryBuilder::new(); + for event in &events { + builder.handle_event(event); + } + let turns = builder.finish(); + assert_eq!(turns.len(), 2); + assert_eq!(turns[0].id, "turn-a"); + assert_eq!(turns[1].id, "turn-b"); + assert_eq!(turns[0].items.len(), 1); + assert_eq!(turns[1].items.len(), 1); + assert_eq!( + turns[1].items[0], + ThreadItem::UserMessage { + id: "item-2".into(), + content: vec![UserInput::Text { + text: "second".into(), + text_elements: Vec::new(), + }], + } + ); + } + #[test] fn late_turn_complete_does_not_close_active_turn() { let events = vec![ @@ -1572,7 +1953,7 @@ mod tests { assert_eq!(turns.len(), 2); assert_eq!(turns[0].id, "turn-a"); assert_eq!(turns[1].id, "turn-b"); - assert_eq!(turns[1].status, TurnStatus::Completed); + assert_eq!(turns[1].status, TurnStatus::InProgress); assert_eq!(turns[1].items.len(), 2); } diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index ecc3ba259e..6a1959bb25 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -37,6 +37,7 @@ use codex_protocol::protocol::PatchApplyStatus as CorePatchApplyStatus; use codex_protocol::protocol::RateLimitSnapshot as CoreRateLimitSnapshot; use codex_protocol::protocol::RateLimitWindow as CoreRateLimitWindow; use codex_protocol::protocol::ReadOnlyAccess as CoreReadOnlyAccess; +use codex_protocol::protocol::RejectConfig as CoreRejectConfig; use codex_protocol::protocol::SessionSource as CoreSessionSource; use codex_protocol::protocol::SkillDependencies as CoreSkillDependencies; use codex_protocol::protocol::SkillErrorInfo as CoreSkillErrorInfo; @@ -162,6 +163,11 @@ pub enum AskForApproval { UnlessTrusted, OnFailure, OnRequest, + Reject { + sandbox_approval: bool, + rules: bool, + mcp_elicitations: bool, + }, Never, } @@ -171,6 +177,15 @@ impl AskForApproval { AskForApproval::UnlessTrusted => CoreAskForApproval::UnlessTrusted, AskForApproval::OnFailure => CoreAskForApproval::OnFailure, AskForApproval::OnRequest => CoreAskForApproval::OnRequest, + AskForApproval::Reject { + sandbox_approval, + rules, + mcp_elicitations, + } => CoreAskForApproval::Reject(CoreRejectConfig { + sandbox_approval, + rules, + mcp_elicitations, + }), AskForApproval::Never => CoreAskForApproval::Never, } } @@ -182,6 +197,11 @@ impl From for AskForApproval { CoreAskForApproval::UnlessTrusted => AskForApproval::UnlessTrusted, CoreAskForApproval::OnFailure => AskForApproval::OnFailure, CoreAskForApproval::OnRequest => AskForApproval::OnRequest, + CoreAskForApproval::Reject(reject_config) => AskForApproval::Reject { + sandbox_approval: reject_config.sandbox_approval, + rules: reject_config.rules, + mcp_elicitations: reject_config.mcp_elicitations, + }, CoreAskForApproval::Never => AskForApproval::Never, } } @@ -2707,6 +2727,26 @@ pub enum ThreadItem { ContextCompaction { id: String }, } +impl ThreadItem { + pub fn id(&self) -> &str { + match self { + ThreadItem::UserMessage { id, .. } + | ThreadItem::AgentMessage { id, .. } + | ThreadItem::Plan { id, .. } + | ThreadItem::Reasoning { id, .. } + | ThreadItem::CommandExecution { id, .. } + | ThreadItem::FileChange { id, .. } + | ThreadItem::McpToolCall { id, .. } + | ThreadItem::CollabAgentToolCall { id, .. } + | ThreadItem::WebSearch { id, .. } + | ThreadItem::ImageView { id, .. } + | ThreadItem::EnteredReviewMode { id, .. } + | ThreadItem::ExitedReviewMode { id, .. } + | ThreadItem::ContextCompaction { id, .. } => id, + } + } +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] #[serde(tag = "type", rename_all = "camelCase")] #[ts(tag = "type", rename_all = "camelCase")] diff --git a/codex-rs/app-server/src/bespoke_event_handling.rs b/codex-rs/app-server/src/bespoke_event_handling.rs index b206eac355..2e87a16844 100644 --- a/codex-rs/app-server/src/bespoke_event_handling.rs +++ b/codex-rs/app-server/src/bespoke_event_handling.rs @@ -45,7 +45,6 @@ use codex_app_server_protocol::McpToolCallResult; use codex_app_server_protocol::McpToolCallStatus; use codex_app_server_protocol::ModelReroutedNotification; use codex_app_server_protocol::PatchApplyStatus; -use codex_app_server_protocol::PatchChangeKind as V2PatchChangeKind; use codex_app_server_protocol::PlanDeltaNotification; use codex_app_server_protocol::RawResponseItemCompletedNotification; use codex_app_server_protocol::ReasoningSummaryPartAddedNotification; @@ -72,6 +71,7 @@ use codex_app_server_protocol::TurnPlanStep; use codex_app_server_protocol::TurnPlanUpdatedNotification; use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::build_turns_from_rollout_items; +use codex_app_server_protocol::convert_patch_changes; use codex_core::CodexThread; use codex_core::ThreadManager; use codex_core::parse_command::shlex_join; @@ -81,7 +81,6 @@ use codex_core::protocol::Event; use codex_core::protocol::EventMsg; use codex_core::protocol::ExecApprovalRequestEvent; use codex_core::protocol::ExecCommandEndEvent; -use codex_core::protocol::FileChange as CoreFileChange; use codex_core::protocol::McpToolCallBeginEvent; use codex_core::protocol::McpToolCallEndEvent; use codex_core::protocol::Op; @@ -1706,46 +1705,6 @@ fn render_review_output_text(output: &ReviewOutputEvent) -> String { } } -fn convert_patch_changes(changes: &HashMap) -> Vec { - let mut converted: Vec = changes - .iter() - .map(|(path, change)| FileUpdateChange { - path: path.to_string_lossy().into_owned(), - kind: map_patch_change_kind(change), - diff: format_file_change_diff(change), - }) - .collect(); - converted.sort_by(|a, b| a.path.cmp(&b.path)); - converted -} - -fn map_patch_change_kind(change: &CoreFileChange) -> V2PatchChangeKind { - match change { - CoreFileChange::Add { .. } => V2PatchChangeKind::Add, - CoreFileChange::Delete { .. } => V2PatchChangeKind::Delete, - CoreFileChange::Update { move_path, .. } => V2PatchChangeKind::Update { - move_path: move_path.clone(), - }, - } -} - -fn format_file_change_diff(change: &CoreFileChange) -> String { - match change { - CoreFileChange::Add { content } => content.clone(), - CoreFileChange::Delete { content } => content.clone(), - CoreFileChange::Update { - unified_diff, - move_path, - } => { - if let Some(path) = move_path { - format!("{unified_diff}\n\nMoved to: {}", path.display()) - } else { - unified_diff.clone() - } - } - } -} - fn map_file_change_approval_decision( decision: FileChangeApprovalDecision, ) -> (ReviewDecision, Option) { diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index f4cdf6626d..ea2701d635 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -336,6 +336,7 @@ pub(crate) struct CodexMessageProcessor { outgoing: Arc, codex_linux_sandbox_exe: Option, config: Arc, + single_client_mode: bool, cli_overrides: Vec<(String, TomlValue)>, cloud_requirements: Arc>, active_login: Arc>>, @@ -361,6 +362,7 @@ pub(crate) struct CodexMessageProcessorArgs { pub(crate) config: Arc, pub(crate) cli_overrides: Vec<(String, TomlValue)>, pub(crate) cloud_requirements: Arc>, + pub(crate) single_client_mode: bool, pub(crate) feedback: CodexFeedback, } @@ -397,6 +399,7 @@ impl CodexMessageProcessor { config, cli_overrides, cloud_requirements, + single_client_mode, feedback, } = args; Self { @@ -405,6 +408,7 @@ impl CodexMessageProcessor { outgoing: outgoing.clone(), codex_linux_sandbox_exe, config, + single_client_mode, cli_overrides, cloud_requirements, active_login: Arc::new(Mutex::new(None)), @@ -3042,21 +3046,14 @@ impl CodexMessageProcessor { return true; } - if let Err(err) = self - .ensure_conversation_listener( - existing_thread_id, - request_id.connection_id, - false, - ApiVersion::V2, - ) - .await - { - tracing::warn!( - "failed to attach listener for thread {}: {}", - existing_thread_id, - err.message - ); - } + let thread_state = self.thread_state_manager.thread_state(existing_thread_id); + self.ensure_listener_task_running( + existing_thread_id, + existing_thread.clone(), + thread_state.clone(), + ApiVersion::V2, + ) + .await; let config_snapshot = existing_thread.config_snapshot().await; let mismatch_details = collect_resume_override_mismatches(params, &config_snapshot); @@ -3068,41 +3065,39 @@ impl CodexMessageProcessor { ); } - let Some(mut thread) = self - .load_thread_from_rollout_or_send_internal( - request_id.clone(), - existing_thread_id, - rollout_path.as_path(), - config_snapshot.model_provider_id.as_str(), - ) - .await - else { + let listener_command_tx = { + let thread_state = thread_state.lock().await; + thread_state.listener_command_tx() + }; + let Some(listener_command_tx) = listener_command_tx else { + let err = JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: format!( + "failed to enqueue running thread resume for thread {existing_thread_id}: thread listener is not running" + ), + data: None, + }; + self.outgoing.send_error(request_id, err).await; return true; }; - let ThreadConfigSnapshot { - model, - model_provider_id, - approval_policy, - sandbox_policy, - cwd, - reasoning_effort, - .. - } = config_snapshot; - thread.status = self - .thread_watch_manager - .loaded_status_for_thread(&thread.id) - .await; - let response = ThreadResumeResponse { - thread, - model, - model_provider: model_provider_id, - cwd, - approval_policy: approval_policy.into(), - sandbox: sandbox_policy.into(), - reasoning_effort, - }; - self.outgoing.send_response(request_id, response).await; + let command = crate::thread_state::ThreadListenerCommand::SendThreadResumeResponse( + crate::thread_state::PendingThreadResumeRequest { + request_id: request_id.clone(), + rollout_path, + config_snapshot, + }, + ); + if listener_command_tx.send(command).is_err() { + let err = JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message: format!( + "failed to enqueue running thread resume for thread {existing_thread_id}: thread listener command channel is closed" + ), + data: None, + }; + self.outgoing.send_error(request_id, err).await; + } return true; } false @@ -4014,6 +4009,7 @@ impl CodexMessageProcessor { scopes.as_deref().unwrap_or_default(), timeout_secs, config.mcp_oauth_callback_port, + config.mcp_oauth_callback_url.as_deref(), ) .await { @@ -5816,17 +5812,18 @@ impl CodexMessageProcessor { api_version: ApiVersion, ) { let (cancel_tx, mut cancel_rx) = oneshot::channel(); - { + let mut listener_command_rx = { let mut thread_state = thread_state.lock().await; if thread_state.listener_matches(&conversation) { return; } - thread_state.set_listener(cancel_tx, &conversation); - } + thread_state.set_listener(cancel_tx, &conversation) + }; let outgoing_for_task = self.outgoing.clone(); let thread_manager = self.thread_manager.clone(); let thread_watch_manager = self.thread_watch_manager.clone(); let fallback_model_provider = self.config.model_provider_id.clone(); + let single_client_mode = self.single_client_mode; tokio::spawn(async move { loop { tokio::select! { @@ -5868,7 +5865,10 @@ impl CodexMessageProcessor { conversation_id.to_string().into(), ); let (subscribed_connection_ids, raw_events_enabled) = { - let thread_state = thread_state.lock().await; + let mut thread_state = thread_state.lock().await; + if !single_client_mode { + thread_state.track_current_turn_event(&event.msg); + } ( thread_state.subscribed_connection_ids(), thread_state.experimental_raw_events, @@ -5907,6 +5907,25 @@ impl CodexMessageProcessor { ) .await; } + listener_command = listener_command_rx.recv() => { + let Some(listener_command) = listener_command else { + break; + }; + match listener_command { + crate::thread_state::ThreadListenerCommand::SendThreadResumeResponse( + resume_request, + ) => { + handle_pending_thread_resume_request( + conversation_id, + &thread_state, + &thread_watch_manager, + &outgoing_for_task, + resume_request, + ) + .await; + } + } + } } } }); @@ -6205,6 +6224,106 @@ impl CodexMessageProcessor { } } +async fn handle_pending_thread_resume_request( + conversation_id: ThreadId, + thread_state: &Arc>, + thread_watch_manager: &ThreadWatchManager, + outgoing: &Arc, + pending: crate::thread_state::PendingThreadResumeRequest, +) { + let active_turn = { + let state = thread_state.lock().await; + state.active_turn_snapshot() + }; + + let request_id = pending.request_id; + let connection_id = request_id.connection_id; + let mut thread = match load_thread_for_running_resume_response( + conversation_id, + pending.rollout_path.as_path(), + pending.config_snapshot.model_provider_id.as_str(), + active_turn.as_ref(), + ) + .await + { + Ok(thread) => thread, + Err(message) => { + outgoing + .send_error( + request_id, + JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + message, + data: None, + }, + ) + .await; + return; + } + }; + thread.status = thread_watch_manager + .loaded_status_for_thread(&thread.id) + .await; + + let ThreadConfigSnapshot { + model, + model_provider_id, + approval_policy, + sandbox_policy, + cwd, + reasoning_effort, + .. + } = pending.config_snapshot; + let response = ThreadResumeResponse { + thread, + model, + model_provider: model_provider_id, + cwd, + approval_policy: approval_policy.into(), + sandbox: sandbox_policy.into(), + reasoning_effort, + }; + outgoing.send_response(request_id, response).await; + thread_state.lock().await.add_connection(connection_id); +} + +async fn load_thread_for_running_resume_response( + conversation_id: ThreadId, + rollout_path: &Path, + fallback_provider: &str, + active_turn: Option<&Turn>, +) -> std::result::Result { + let mut thread = read_summary_from_rollout(rollout_path, fallback_provider) + .await + .map(summary_to_thread) + .map_err(|err| { + format!( + "failed to load rollout `{}` for thread {conversation_id}: {err}", + rollout_path.display() + ) + })?; + + let mut turns = read_rollout_items_from_rollout(rollout_path) + .await + .map(|items| build_turns_from_rollout_items(&items)) + .map_err(|err| { + format!( + "failed to load rollout `{}` for thread {conversation_id}: {err}", + rollout_path.display() + ) + })?; + if let Some(active_turn) = active_turn { + merge_turn_history_with_active_turn(&mut turns, active_turn.clone()); + } + thread.turns = turns; + Ok(thread) +} + +fn merge_turn_history_with_active_turn(turns: &mut Vec, active_turn: Turn) { + turns.retain(|turn| turn.id != active_turn.id); + turns.push(active_turn); +} + fn collect_resume_override_mismatches( request: &ThreadResumeParams, config_snapshot: &ThreadConfigSnapshot, diff --git a/codex-rs/app-server/src/lib.rs b/codex-rs/app-server/src/lib.rs index 7752ee92c7..8e7e1455c4 100644 --- a/codex-rs/app-server/src/lib.rs +++ b/codex-rs/app-server/src/lib.rs @@ -238,7 +238,8 @@ pub async fn run_main_with_transport( Some(start_websocket_acceptor(bind_address, transport_event_tx.clone()).await?); } } - let shutdown_when_no_connections = matches!(transport, AppServerTransport::Stdio); + let single_client_mode = matches!(transport, AppServerTransport::Stdio); + let shutdown_when_no_connections = single_client_mode; // Parse CLI overrides once and derive the base Config eagerly so later // components do not need to work with raw TOML values. @@ -439,6 +440,7 @@ pub async fn run_main_with_transport( outgoing: outgoing_message_sender, codex_linux_sandbox_exe, config: Arc::new(config), + single_client_mode, cli_overrides, loader_overrides, cloud_requirements: cloud_requirements.clone(), diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index f1c6029a21..e80e1790f8 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -139,6 +139,7 @@ pub(crate) struct MessageProcessorArgs { pub(crate) outgoing: Arc, pub(crate) codex_linux_sandbox_exe: Option, pub(crate) config: Arc, + pub(crate) single_client_mode: bool, pub(crate) cli_overrides: Vec<(String, TomlValue)>, pub(crate) loader_overrides: LoaderOverrides, pub(crate) cloud_requirements: CloudRequirementsLoader, @@ -154,6 +155,7 @@ impl MessageProcessor { outgoing, codex_linux_sandbox_exe, config, + single_client_mode, cli_overrides, loader_overrides, cloud_requirements, @@ -184,6 +186,7 @@ impl MessageProcessor { config: Arc::clone(&config), cli_overrides: cli_overrides.clone(), cloud_requirements: cloud_requirements.clone(), + single_client_mode, feedback, }); let config_api = ConfigApi::new( diff --git a/codex-rs/app-server/src/thread_state.rs b/codex-rs/app-server/src/thread_state.rs index fd0f58abd1..264c9a39d6 100644 --- a/codex-rs/app-server/src/thread_state.rs +++ b/codex-rs/app-server/src/thread_state.rs @@ -1,13 +1,19 @@ use crate::outgoing_message::ConnectionId; use crate::outgoing_message::ConnectionRequestId; +use codex_app_server_protocol::ThreadHistoryBuilder; +use codex_app_server_protocol::Turn; use codex_app_server_protocol::TurnError; use codex_core::CodexThread; +use codex_core::ThreadConfigSnapshot; use codex_protocol::ThreadId; +use codex_protocol::protocol::EventMsg; use std::collections::HashMap; use std::collections::HashSet; +use std::path::PathBuf; use std::sync::Arc; use std::sync::Weak; use tokio::sync::Mutex; +use tokio::sync::mpsc; use tokio::sync::oneshot; use uuid::Uuid; @@ -16,6 +22,16 @@ type PendingInterruptQueue = Vec<( crate::codex_message_processor::ApiVersion, )>; +pub(crate) struct PendingThreadResumeRequest { + pub(crate) request_id: ConnectionRequestId, + pub(crate) rollout_path: PathBuf, + pub(crate) config_snapshot: ThreadConfigSnapshot, +} + +pub(crate) enum ThreadListenerCommand { + SendThreadResumeResponse(PendingThreadResumeRequest), +} + /// Per-conversation accumulation of the latest states e.g. error message while a turn runs. #[derive(Default, Clone)] pub(crate) struct TurnSummary { @@ -31,6 +47,8 @@ pub(crate) struct ThreadState { pub(crate) turn_summary: TurnSummary, pub(crate) cancel_tx: Option>, pub(crate) experimental_raw_events: bool, + listener_command_tx: Option>, + current_turn_history: ThreadHistoryBuilder, listener_thread: Option>, subscribed_connections: HashSet, } @@ -47,17 +65,22 @@ impl ThreadState { &mut self, cancel_tx: oneshot::Sender<()>, conversation: &Arc, - ) { + ) -> mpsc::UnboundedReceiver { if let Some(previous) = self.cancel_tx.replace(cancel_tx) { let _ = previous.send(()); } + let (listener_command_tx, listener_command_rx) = mpsc::unbounded_channel(); + self.listener_command_tx = Some(listener_command_tx); self.listener_thread = Some(Arc::downgrade(conversation)); + listener_command_rx } pub(crate) fn clear_listener(&mut self) { if let Some(cancel_tx) = self.cancel_tx.take() { let _ = cancel_tx.send(()); } + self.listener_command_tx = None; + self.current_turn_history.reset(); self.listener_thread = None; } @@ -76,6 +99,23 @@ impl ThreadState { pub(crate) fn set_experimental_raw_events(&mut self, enabled: bool) { self.experimental_raw_events = enabled; } + + pub(crate) fn listener_command_tx( + &self, + ) -> Option> { + self.listener_command_tx.clone() + } + + pub(crate) fn active_turn_snapshot(&self) -> Option { + self.current_turn_history.active_turn_snapshot() + } + + pub(crate) fn track_current_turn_event(&mut self, event: &EventMsg) { + self.current_turn_history.handle_event(event); + if !self.current_turn_history.has_active_turn() { + self.current_turn_history.reset(); + } + } } #[derive(Clone, Copy)] @@ -200,12 +240,24 @@ impl ThreadStateManager { } pub(crate) async fn remove_connection(&mut self, connection_id: ConnectionId) { - let Some(thread_ids) = self.thread_ids_by_connection.remove(&connection_id) else { - return; - }; + let thread_ids = self + .thread_ids_by_connection + .remove(&connection_id) + .unwrap_or_default(); self.subscription_state_by_id .retain(|_, state| state.connection_id != connection_id); + if thread_ids.is_empty() { + for thread_state in self.thread_states.values() { + let mut thread_state = thread_state.lock().await; + thread_state.remove_connection(connection_id); + if thread_state.subscribed_connection_ids().is_empty() { + thread_state.clear_listener(); + } + } + return; + } + for thread_id in thread_ids { if let Some(thread_state) = self.thread_states.get(&thread_id) { let mut thread_state = thread_state.lock().await; diff --git a/codex-rs/app-server/tests/common/mcp_process.rs b/codex-rs/app-server/tests/common/mcp_process.rs index 2677c53d89..249a280213 100644 --- a/codex-rs/app-server/tests/common/mcp_process.rs +++ b/codex-rs/app-server/tests/common/mcp_process.rs @@ -61,6 +61,7 @@ use codex_app_server_protocol::ThreadResumeParams; use codex_app_server_protocol::ThreadRollbackParams; use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadUnarchiveParams; +use codex_app_server_protocol::TurnCompletedNotification; use codex_app_server_protocol::TurnInterruptParams; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::TurnSteerParams; @@ -75,7 +76,7 @@ pub struct McpProcess { /// not a guarantee. See the `kill_on_drop` documentation for details. #[allow(dead_code)] process: Child, - stdin: ChildStdin, + stdin: Option, stdout: BufReader, pending_messages: VecDeque, } @@ -105,10 +106,6 @@ impl McpProcess { cmd.stderr(Stdio::piped()); cmd.env("CODEX_HOME", codex_home); cmd.env("RUST_LOG", "debug"); - // Bazel/Linux workers can run with smaller default thread stacks, which makes - // tokio-runtime-worker stack overflows more likely in app-server integration tests. - // Pin a larger minimum stack for the spawned test server process. - cmd.env("RUST_MIN_STACK", "4194304"); cmd.env_remove(CODEX_INTERNAL_ORIGINATOR_OVERRIDE_ENV_VAR); for (k, v) in env_overrides { @@ -149,7 +146,7 @@ impl McpProcess { Ok(Self { next_request_id: AtomicI64::new(0), process, - stdin, + stdin: Some(stdin), stdout, pending_messages: VecDeque::new(), }) @@ -576,6 +573,63 @@ impl McpProcess { self.send_request("turn/interrupt", params).await } + /// Deterministically clean up an intentionally in-flight turn. + /// + /// Some tests assert behavior while a turn is still running. Returning from those tests + /// without an explicit interrupt + `codex/event/turn_aborted` wait can leave in-flight work + /// racing teardown and intermittently show up as `LEAK` in nextest. + /// + /// In rare races, the turn can also fail or complete on its own after we send + /// `turn/interrupt` but before the server emits the interrupt response. The helper treats a + /// buffered matching `turn/completed` notification as sufficient terminal cleanup in that + /// case so teardown does not flap on timing. + pub async fn interrupt_turn_and_wait_for_aborted( + &mut self, + thread_id: String, + turn_id: String, + read_timeout: std::time::Duration, + ) -> anyhow::Result<()> { + let interrupt_request_id = self + .send_turn_interrupt_request(TurnInterruptParams { + thread_id: thread_id.clone(), + turn_id: turn_id.clone(), + }) + .await?; + match tokio::time::timeout( + read_timeout, + self.read_stream_until_response_message(RequestId::Integer(interrupt_request_id)), + ) + .await + { + Ok(result) => { + result.with_context(|| "failed while waiting for turn interrupt response")?; + } + Err(err) => { + if self.pending_turn_completed_notification(&thread_id, &turn_id) { + return Ok(()); + } + return Err(err).with_context(|| "timed out waiting for turn interrupt response"); + } + } + match tokio::time::timeout( + read_timeout, + self.read_stream_until_notification_message("codex/event/turn_aborted"), + ) + .await + { + Ok(result) => { + result.with_context(|| "failed while waiting for turn aborted notification")?; + } + Err(err) => { + if self.pending_turn_completed_notification(&thread_id, &turn_id) { + return Ok(()); + } + return Err(err).with_context(|| "timed out waiting for turn aborted notification"); + } + } + Ok(()) + } + /// Send a `turn/steer` JSON-RPC request (v2). pub async fn send_turn_steer_request( &mut self, @@ -815,10 +869,13 @@ impl McpProcess { async fn send_jsonrpc_message(&mut self, message: JSONRPCMessage) -> anyhow::Result<()> { eprintln!("writing message to stdin: {message:?}"); + let Some(stdin) = self.stdin.as_mut() else { + anyhow::bail!("mcp stdin closed"); + }; let payload = serde_json::to_string(&message)?; - self.stdin.write_all(payload.as_bytes()).await?; - self.stdin.write_all(b"\n").await?; - self.stdin.flush().await?; + stdin.write_all(payload.as_bytes()).await?; + stdin.write_all(b"\n").await?; + stdin.flush().await?; Ok(()) } @@ -941,6 +998,25 @@ impl McpProcess { None } + fn pending_turn_completed_notification(&self, thread_id: &str, turn_id: &str) -> bool { + self.pending_messages.iter().any(|message| { + let JSONRPCMessage::Notification(notification) = message else { + return false; + }; + if notification.method != "turn/completed" { + return false; + } + let Some(params) = notification.params.as_ref() else { + return false; + }; + let Ok(payload) = serde_json::from_value::(params.clone()) + else { + return false; + }; + payload.thread_id == thread_id && payload.turn.id == turn_id + }) + } + fn message_request_id(message: &JSONRPCMessage) -> Option<&RequestId> { match message { JSONRPCMessage::Request(request) => Some(&request.id), @@ -965,8 +1041,22 @@ impl Drop for McpProcess { // // Drop can't be async, so we do a bounded synchronous cleanup: // - // 1. Request termination with `start_kill()`. - // 2. Poll `try_wait()` until the OS reports the child exited, with a short timeout. + // 1. Close stdin to request a graceful shutdown via EOF. + // 2. Poll briefly for graceful exit. + // 3. If still alive, request termination with `start_kill()`. + // 4. Poll `try_wait()` until the OS reports the child exited, with a short timeout. + drop(self.stdin.take()); + + let graceful_start = std::time::Instant::now(); + let graceful_timeout = std::time::Duration::from_millis(200); + while graceful_start.elapsed() < graceful_timeout { + match self.process.try_wait() { + Ok(Some(_)) => return, + Ok(None) => std::thread::sleep(std::time::Duration::from_millis(5)), + Err(_) => return, + } + } + let _ = self.process.start_kill(); let start = std::time::Instant::now(); diff --git a/codex-rs/app-server/tests/suite/v2/app_list.rs b/codex-rs/app-server/tests/suite/v2/app_list.rs index 1fd3f5a447..3a2064d4cb 100644 --- a/codex-rs/app-server/tests/suite/v2/app_list.rs +++ b/codex-rs/app-server/tests/suite/v2/app_list.rs @@ -491,39 +491,60 @@ async fn list_apps_returns_connectors_with_accessible_flags() -> Result<()> { }) .await?; + let expected_directory_first = vec![ + AppInfo { + id: "alpha".to_string(), + name: "Alpha".to_string(), + description: Some("Alpha connector".to_string()), + logo_url: Some("https://example.com/alpha.png".to_string()), + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: Some("https://chatgpt.com/apps/alpha/alpha".to_string()), + is_accessible: false, + is_enabled: true, + }, + AppInfo { + id: "beta".to_string(), + name: "beta".to_string(), + description: None, + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: Some("https://chatgpt.com/apps/beta/beta".to_string()), + is_accessible: false, + is_enabled: true, + }, + ]; + let expected_accessible_first = vec![AppInfo { + id: "beta".to_string(), + name: "Beta App".to_string(), + description: None, + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: Some("https://chatgpt.com/apps/beta-app/beta".to_string()), + is_accessible: true, + is_enabled: true, + }]; + let first_update = read_app_list_updated_notification(&mut mcp).await?; - assert_eq!( - first_update.data, - vec![ - AppInfo { - id: "alpha".to_string(), - name: "Alpha".to_string(), - description: Some("Alpha connector".to_string()), - logo_url: Some("https://example.com/alpha.png".to_string()), - logo_url_dark: None, - distribution_channel: None, - branding: None, - app_metadata: None, - labels: None, - install_url: Some("https://chatgpt.com/apps/alpha/alpha".to_string()), - is_accessible: false, - is_enabled: true, - }, - AppInfo { - id: "beta".to_string(), - name: "beta".to_string(), - description: None, - logo_url: None, - logo_url_dark: None, - distribution_channel: None, - branding: None, - app_metadata: None, - labels: None, - install_url: Some("https://chatgpt.com/apps/beta/beta".to_string()), - is_accessible: false, - is_enabled: true, - }, - ] + // app/list emits an update after whichever async load finishes first. Even with + // a tools delay in this test, the accessible-tools path can return first if the + // process-global Codex Apps tools cache is warm from another test. + assert!( + first_update.data == expected_directory_first + || first_update.data == expected_accessible_first, + "unexpected first app/list update: {:#?}", + first_update.data ); let expected = vec![ diff --git a/codex-rs/app-server/tests/suite/v2/thread_resume.rs b/codex-rs/app-server/tests/suite/v2/thread_resume.rs index 576f031565..15bc4012eb 100644 --- a/codex-rs/app-server/tests/suite/v2/thread_resume.rs +++ b/codex-rs/app-server/tests/suite/v2/thread_resume.rs @@ -16,6 +16,7 @@ use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::ThreadStatus; use codex_app_server_protocol::TurnStartParams; +use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::UserInput; use codex_protocol::config_types::Personality; @@ -314,9 +315,14 @@ async fn thread_resume_rejects_history_when_thread_is_running() -> Result<()> { responses::ev_assistant_message("msg-1", "Done"), responses::ev_completed("resp-1"), ]); - let second_body = responses::sse(vec![responses::ev_response_created("resp-2")]); + let second_response = responses::sse_response(responses::sse(vec![ + responses::ev_response_created("resp-2"), + responses::ev_assistant_message("msg-2", "Done"), + responses::ev_completed("resp-2"), + ])) + .set_delay(std::time::Duration::from_millis(500)); let _first_response_mock = responses::mount_sse_once(&server, first_body).await; - let _second_response_mock = responses::mount_sse_once(&server, second_body).await; + let _second_response_mock = responses::mount_response_once(&server, second_response).await; let codex_home = TempDir::new()?; create_config_toml(codex_home.path(), &server.uri())?; @@ -358,9 +364,10 @@ async fn thread_resume_rejects_history_when_thread_is_running() -> Result<()> { .await??; primary.clear_message_buffer(); - let running_turn_id = primary + let thread_id = thread.id.clone(); + let running_turn_request_id = primary .send_turn_start_request(TurnStartParams { - thread_id: thread.id.clone(), + thread_id: thread_id.clone(), input: vec![UserInput::Text { text: "keep running".to_string(), text_elements: Vec::new(), @@ -368,11 +375,13 @@ async fn thread_resume_rejects_history_when_thread_is_running() -> Result<()> { ..Default::default() }) .await?; - timeout( + let running_turn_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, - primary.read_stream_until_response_message(RequestId::Integer(running_turn_id)), + primary.read_stream_until_response_message(RequestId::Integer(running_turn_request_id)), ) .await??; + let TurnStartResponse { turn: running_turn } = + to_response::(running_turn_resp)?; timeout( DEFAULT_READ_TIMEOUT, primary.read_stream_until_notification_message("turn/started"), @@ -381,7 +390,7 @@ async fn thread_resume_rejects_history_when_thread_is_running() -> Result<()> { let resume_id = primary .send_thread_resume_request(ThreadResumeParams { - thread_id: thread.id, + thread_id: thread_id.clone(), history: Some(vec![ResponseItem::Message { id: None, role: "user".to_string(), @@ -407,6 +416,10 @@ async fn thread_resume_rejects_history_when_thread_is_running() -> Result<()> { resume_err.error.message ); + primary + .interrupt_turn_and_wait_for_aborted(thread_id, running_turn.id, DEFAULT_READ_TIMEOUT) + .await?; + Ok(()) } @@ -418,9 +431,14 @@ async fn thread_resume_rejects_mismatched_path_when_thread_is_running() -> Resul responses::ev_assistant_message("msg-1", "Done"), responses::ev_completed("resp-1"), ]); - let second_body = responses::sse(vec![responses::ev_response_created("resp-2")]); + let second_response = responses::sse_response(responses::sse(vec![ + responses::ev_response_created("resp-2"), + responses::ev_assistant_message("msg-2", "Done"), + responses::ev_completed("resp-2"), + ])) + .set_delay(std::time::Duration::from_millis(500)); let _first_response_mock = responses::mount_sse_once(&server, first_body).await; - let _second_response_mock = responses::mount_sse_once(&server, second_body).await; + let _second_response_mock = responses::mount_response_once(&server, second_response).await; let codex_home = TempDir::new()?; create_config_toml(codex_home.path(), &server.uri())?; @@ -462,9 +480,10 @@ async fn thread_resume_rejects_mismatched_path_when_thread_is_running() -> Resul .await??; primary.clear_message_buffer(); - let running_turn_id = primary + let thread_id = thread.id.clone(); + let running_turn_request_id = primary .send_turn_start_request(TurnStartParams { - thread_id: thread.id.clone(), + thread_id: thread_id.clone(), input: vec![UserInput::Text { text: "keep running".to_string(), text_elements: Vec::new(), @@ -472,11 +491,13 @@ async fn thread_resume_rejects_mismatched_path_when_thread_is_running() -> Resul ..Default::default() }) .await?; - timeout( + let running_turn_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, - primary.read_stream_until_response_message(RequestId::Integer(running_turn_id)), + primary.read_stream_until_response_message(RequestId::Integer(running_turn_request_id)), ) .await??; + let TurnStartResponse { turn: running_turn } = + to_response::(running_turn_resp)?; timeout( DEFAULT_READ_TIMEOUT, primary.read_stream_until_notification_message("turn/started"), @@ -485,7 +506,7 @@ async fn thread_resume_rejects_mismatched_path_when_thread_is_running() -> Resul let resume_id = primary .send_thread_resume_request(ThreadResumeParams { - thread_id: thread.id, + thread_id: thread_id.clone(), path: Some(PathBuf::from("/tmp/does-not-match-running-rollout.jsonl")), ..Default::default() }) @@ -501,6 +522,10 @@ async fn thread_resume_rejects_mismatched_path_when_thread_is_running() -> Resul resume_err.error.message ); + primary + .interrupt_turn_and_wait_for_aborted(thread_id, running_turn.id, DEFAULT_READ_TIMEOUT) + .await?; + Ok(()) } diff --git a/codex-rs/app-server/tests/suite/v2/turn_start.rs b/codex-rs/app-server/tests/suite/v2/turn_start.rs index 2eecfc5590..d2d850176d 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start.rs @@ -747,7 +747,12 @@ async fn turn_start_accepts_local_image_input() -> Result<()> { let TurnStartResponse { turn } = to_response::(turn_resp)?; assert!(!turn.id.is_empty()); - // This test only validates that turn/start responds and returns a turn. + timeout( + DEFAULT_READ_TIMEOUT, + mcp.read_stream_until_notification_message("turn/completed"), + ) + .await??; + Ok(()) } diff --git a/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs b/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs index 4233ce6494..4be127dff8 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_start_zsh_fork.rs @@ -29,6 +29,7 @@ use codex_app_server_protocol::ThreadStartParams; use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnCompletedNotification; use codex_app_server_protocol::TurnStartParams; +use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::TurnStatus; use codex_app_server_protocol::UserInput as V2UserInput; use codex_core::features::FEATURES; @@ -100,7 +101,7 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> { let turn_id = mcp .send_turn_start_request(TurnStartParams { - thread_id: thread.id, + thread_id: thread.id.clone(), input: vec![V2UserInput::Text { text: "run echo hi".to_string(), text_elements: Vec::new(), @@ -114,11 +115,12 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> { ..Default::default() }) .await?; - timeout( + let turn_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), ) .await??; + let TurnStartResponse { turn } = to_response::(turn_resp)?; let started_command_execution = timeout(DEFAULT_READ_TIMEOUT, async { loop { @@ -149,6 +151,9 @@ async fn turn_start_shell_zsh_fork_executes_command_v2() -> Result<()> { assert!(command.contains(" -lc 'echo hi'")); assert_eq!(cwd, workspace); + mcp.interrupt_turn_and_wait_for_aborted(thread.id, turn.id, DEFAULT_READ_TIMEOUT) + .await?; + Ok(()) } @@ -504,11 +509,12 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() ..Default::default() }) .await?; - timeout( + let turn_resp: JSONRPCResponse = timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_response_message(RequestId::Integer(turn_id)), ) .await??; + let TurnStartResponse { turn } = to_response::(turn_resp)?; let mut approval_ids = Vec::new(); for decision in [ @@ -577,6 +583,9 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2() assert_eq!(approval_ids.len(), 2); assert_ne!(approval_ids[0], approval_ids[1]); + mcp.interrupt_turn_and_wait_for_aborted(thread.id, turn.id, DEFAULT_READ_TIMEOUT) + .await?; + Ok(()) } diff --git a/codex-rs/app-server/tests/suite/v2/turn_steer.rs b/codex-rs/app-server/tests/suite/v2/turn_steer.rs index 89704326fd..779e77577b 100644 --- a/codex-rs/app-server/tests/suite/v2/turn_steer.rs +++ b/codex-rs/app-server/tests/suite/v2/turn_steer.rs @@ -136,7 +136,7 @@ async fn turn_steer_returns_active_turn_id() -> Result<()> { let steer_req = mcp .send_turn_steer_request(TurnSteerParams { - thread_id: thread.id, + thread_id: thread.id.clone(), input: vec![V2UserInput::Text { text: "steer".to_string(), text_elements: Vec::new(), @@ -152,6 +152,9 @@ async fn turn_steer_returns_active_turn_id() -> Result<()> { let steer: TurnSteerResponse = to_response::(steer_resp)?; assert_eq!(steer.turn_id, turn.id); + mcp.interrupt_turn_and_wait_for_aborted(thread.id, steer.turn_id, DEFAULT_READ_TIMEOUT) + .await?; + Ok(()) } diff --git a/codex-rs/chatgpt/src/connectors.rs b/codex-rs/chatgpt/src/connectors.rs index 6dc2161f0c..27f2f1de94 100644 --- a/codex-rs/chatgpt/src/connectors.rs +++ b/codex-rs/chatgpt/src/connectors.rs @@ -4,8 +4,6 @@ use std::sync::LazyLock; use std::sync::Mutex as StdMutex; use codex_core::config::Config; -use codex_core::default_client::is_first_party_chat_originator; -use codex_core::default_client::originator; use codex_core::features::Feature; use codex_core::token_data::TokenData; use serde::Deserialize; @@ -22,8 +20,10 @@ use codex_core::connectors::AppMetadata; use codex_core::connectors::CONNECTORS_CACHE_TTL; pub use codex_core::connectors::connector_display_label; use codex_core::connectors::connector_install_url; +use codex_core::connectors::filter_disallowed_connectors; pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools; pub use codex_core::connectors::list_accessible_connectors_from_mcp_tools_with_options; +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; pub use codex_core::connectors::with_app_enabled_state; @@ -106,7 +106,7 @@ pub async fn list_cached_all_connectors(config: &Config) -> Option> } let token_data = get_chatgpt_token_data()?; let cache_key = all_connectors_cache_key(config, &token_data); - read_cached_all_connectors(&cache_key) + read_cached_all_connectors(&cache_key).map(filter_disallowed_connectors) } pub async fn list_all_connectors_with_options( @@ -123,7 +123,7 @@ 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(cached_connectors); + return Ok(filter_disallowed_connectors(cached_connectors)); } let mut apps = list_directory_connectors(config).await?; @@ -149,6 +149,7 @@ pub async fn list_all_connectors_with_options( .cmp(&right.name) .then_with(|| left.id.cmp(&right.id)) }); + let connectors = filter_disallowed_connectors(connectors); write_cached_all_connectors(cache_key, &connectors); Ok(connectors) } @@ -453,45 +454,6 @@ fn normalize_connector_value(value: Option<&str>) -> Option { .filter(|value| !value.is_empty()) .map(str::to_string) } - -const DISALLOWED_CONNECTOR_IDS: &[&str] = &[ - "asdk_app_6938a94a61d881918ef32cb999ff937c", - "connector_2b0a9009c9c64bf9933a3dae3f2b1254", - "connector_68de829bf7648191acd70a907364c67c", - "connector_68e004f14af881919eb50893d3d9f523", - "connector_69272cb413a081919685ec3c88d1744e", - "connector_0f9c9d4592e54d0a9a12b3f44a1e2010", -]; -const FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS: &[&str] = - &["connector_0f9c9d4592e54d0a9a12b3f44a1e2010"]; -const DISALLOWED_CONNECTOR_PREFIX: &str = "connector_openai_"; - -fn filter_disallowed_connectors(connectors: Vec) -> Vec { - filter_disallowed_connectors_for_originator(connectors, originator().value.as_str()) -} - -fn filter_disallowed_connectors_for_originator( - connectors: Vec, - originator_value: &str, -) -> Vec { - let disallowed_connector_ids = if is_first_party_chat_originator(originator_value) { - FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS - } else { - DISALLOWED_CONNECTOR_IDS - }; - - connectors - .into_iter() - .filter(|connector| is_connector_allowed(connector, disallowed_connector_ids)) - .collect() -} - -fn is_connector_allowed(connector: &AppInfo, disallowed_connector_ids: &[&str]) -> bool { - let connector_id = connector.id.as_str(); - !connector_id.starts_with(DISALLOWED_CONNECTOR_PREFIX) - && !disallowed_connector_ids.contains(&connector_id) -} - #[cfg(test)] mod tests { use super::*; @@ -554,22 +516,6 @@ mod tests { assert_eq!(filtered, vec![app("delta")]); } - #[test] - fn first_party_chat_originator_filters_target_and_openai_prefixed_connectors() { - let filtered = filter_disallowed_connectors_for_originator( - vec![ - app("connector_openai_foo"), - app("asdk_app_6938a94a61d881918ef32cb999ff937c"), - app("connector_0f9c9d4592e54d0a9a12b3f44a1e2010"), - ], - "codex_atlas", - ); - assert_eq!( - filtered, - vec![app("asdk_app_6938a94a61d881918ef32cb999ff937c"),] - ); - } - fn merged_app(id: &str, is_accessible: bool) -> AppInfo { AppInfo { id: id.to_string(), diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 052af56196..be3678e07e 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -273,6 +273,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re oauth_config.env_http_headers, &Vec::new(), config.mcp_oauth_callback_port, + config.mcp_oauth_callback_url.as_deref(), ) .await?; println!("Successfully logged in."); @@ -356,6 +357,7 @@ async fn run_login(config_overrides: &CliConfigOverrides, login_args: LoginArgs) env_http_headers, &scopes, config.mcp_oauth_callback_port, + config.mcp_oauth_callback_url.as_deref(), ) .await?; println!("Successfully logged in to MCP server '{name}'."); diff --git a/codex-rs/codex-api/src/common.rs b/codex-rs/codex-api/src/common.rs index 3ef96c185b..f27f936b56 100644 --- a/codex-rs/codex-api/src/common.rs +++ b/codex-rs/codex-api/src/common.rs @@ -9,6 +9,7 @@ use futures::Stream; use serde::Deserialize; use serde::Serialize; use serde_json::Value; +use std::collections::HashMap; use std::pin::Pin; use std::task::Context; use std::task::Poll; @@ -175,6 +176,7 @@ impl From<&ResponsesApiRequest> for ResponseCreateWsRequest { include: request.include.clone(), prompt_cache_key: request.prompt_cache_key.clone(), text: request.text.clone(), + client_metadata: None, } } } @@ -197,11 +199,15 @@ pub struct ResponseCreateWsRequest { pub prompt_cache_key: Option, #[serde(skip_serializing_if = "Option::is_none")] pub text: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub client_metadata: Option>, } #[derive(Debug, Serialize)] pub struct ResponseAppendWsRequest { pub input: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub client_metadata: Option>, } #[derive(Debug, Serialize)] #[serde(tag = "type")] diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index 64b7330c3a..fe7c2b5aa9 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -22,7 +22,6 @@ anyhow = { workspace = true } arc-swap = "1.8.2" async-channel = { workspace = true } async-trait = { workspace = true } -askama = { workspace = true } base64 = { workspace = true } bm25 = { workspace = true } chardetng = { workspace = true } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index af2ba45656..5eab764fc3 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -139,6 +139,19 @@ ], "type": "string" }, + { + "additionalProperties": false, + "description": "Fine-grained rejection controls for approval prompts.\n\nWhen a field is `true`, prompts of that category are automatically rejected instead of shown to the user.", + "properties": { + "reject": { + "$ref": "#/definitions/RejectConfig" + } + }, + "required": [ + "reject" + ], + "type": "object" + }, { "description": "Never ask the user to approve commands. Failures are immediately returned to the model, and never escalated to the user for approval.", "enum": [ @@ -612,6 +625,70 @@ ], "type": "object" }, + "NetworkModeSchema": { + "enum": [ + "limited", + "full" + ], + "type": "string" + }, + "NetworkToml": { + "additionalProperties": false, + "properties": { + "admin_url": { + "type": "string" + }, + "allow_local_binding": { + "type": "boolean" + }, + "allow_unix_sockets": { + "items": { + "type": "string" + }, + "type": "array" + }, + "allow_upstream_proxy": { + "type": "boolean" + }, + "allowed_domains": { + "items": { + "type": "string" + }, + "type": "array" + }, + "dangerously_allow_non_loopback_admin": { + "type": "boolean" + }, + "dangerously_allow_non_loopback_proxy": { + "type": "boolean" + }, + "denied_domains": { + "items": { + "type": "string" + }, + "type": "array" + }, + "enable_socks5": { + "type": "boolean" + }, + "enable_socks5_udp": { + "type": "boolean" + }, + "enabled": { + "type": "boolean" + }, + "mode": { + "$ref": "#/definitions/NetworkModeSchema" + }, + "proxy_url": { + "type": "string" + }, + "socks_url": { + "type": "string" + } + }, + "type": "object" + }, "Notice": { "description": "Settings for notices we display to users via the tui and app-server clients (primarily the Codex IDE extension). NOTE: these are different from notifications - notices are warnings, NUX screens, acknowledgements, etc.", "properties": { @@ -853,6 +930,20 @@ }, "type": "object" }, + "PermissionsToml": { + "additionalProperties": false, + "properties": { + "network": { + "allOf": [ + { + "$ref": "#/definitions/NetworkToml" + } + ], + "description": "Network proxy settings from `[permissions.network]`. User config can enable the proxy; managed requirements may still constrain values." + } + }, + "type": "object" + }, "Personality": { "enum": [ "none", @@ -1003,6 +1094,28 @@ } ] }, + "RejectConfig": { + "properties": { + "mcp_elicitations": { + "description": "Reject MCP elicitation prompts.", + "type": "boolean" + }, + "rules": { + "description": "Reject prompts triggered by execpolicy `prompt` rules.", + "type": "boolean" + }, + "sandbox_approval": { + "description": "Reject approval prompts related to sandbox escalation.", + "type": "boolean" + } + }, + "required": [ + "mcp_elicitations", + "rules", + "sandbox_approval" + ], + "type": "object" + }, "SandboxMode": { "enum": [ "read-only", @@ -1566,6 +1679,10 @@ "minimum": 0.0, "type": "integer" }, + "mcp_oauth_callback_url": { + "description": "Optional redirect URI to use during MCP OAuth login. When set, this URI is used in the OAuth authorization request instead of the local listener address. The local callback listener still binds to 127.0.0.1 (using `mcp_oauth_callback_port` when provided).", + "type": "string" + }, "mcp_oauth_credentials_store": { "allOf": [ { @@ -1679,6 +1796,15 @@ ], "description": "OTEL configuration." }, + "permissions": { + "allOf": [ + { + "$ref": "#/definitions/PermissionsToml" + } + ], + "default": null, + "description": "Nested permissions settings." + }, "personality": { "allOf": [ { diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 09ae0eb0d8..658e212fa7 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -26,6 +26,7 @@ //! back. This avoids duplicate handshakes but means a failed prewarm can consume one retry //! budget slot before any turn payload is sent. +use std::collections::HashMap; use std::sync::Arc; use std::sync::OnceLock; use std::sync::atomic::AtomicBool; @@ -623,6 +624,7 @@ impl ModelClientSession { if !responses_websockets_v2_enabled { return ResponsesWsRequest::ResponseAppend(ResponseAppendWsRequest { input: append_items, + client_metadata: payload.client_metadata, }); } } @@ -811,7 +813,10 @@ impl ModelClientSession { effort, summary, )?; - let ws_payload = ResponseCreateWsRequest::from(&request); + let ws_payload = ResponseCreateWsRequest { + client_metadata: build_ws_client_metadata(turn_metadata_header), + ..ResponseCreateWsRequest::from(&request) + }; match self .websocket_connection( @@ -968,6 +973,14 @@ fn parse_turn_metadata_header(turn_metadata_header: Option<&str>) -> Option) -> Option> { + let turn_metadata_header = parse_turn_metadata_header(turn_metadata_header)?; + let turn_metadata = turn_metadata_header.to_str().ok()?.to_string(); + let mut client_metadata = HashMap::new(); + client_metadata.insert(X_CODEX_TURN_METADATA_HEADER.to_string(), turn_metadata); + Some(client_metadata) +} + /// Builds the extra headers attached to Responses API requests. /// /// These headers implement Codex-specific conventions: diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 0ff60c4647..a384ffc00f 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -149,6 +149,7 @@ use crate::mcp::effective_mcp_servers; use crate::mcp::maybe_prompt_and_install_mcp_dependencies; 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::mcp_connection_manager::filter_codex_apps_mcp_tools_only; use crate::mcp_connection_manager::filter_mcp_tools_by_name; use crate::mcp_connection_manager::filter_non_codex_apps_mcp_tools_only; @@ -1281,7 +1282,16 @@ impl Session { .await; let services = SessionServices { - mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())), + // Initialize the MCP connection manager with an uninitialized + // instance. It will be replaced with one created via + // McpConnectionManager::new() once all its constructor args are + // available. This also ensures `SessionConfigured` is emitted + // before any MCP-related events. It is reasonable to consider + // changing this to use Option or OnceCell, though the current + // setup is straightforward enough and performs well. + mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::new_uninitialized( + &config.permissions.approval_policy, + ))), mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()), unified_exec_manager: UnifiedExecProcessManager::new( config.background_terminal_max_timeout, @@ -1385,7 +1395,7 @@ impl Session { // Start the watcher after SessionConfigured so it cannot emit earlier events. sess.start_file_watcher_listener(); - // Construct sandbox_state before initialize() so it can be sent to each + // Construct sandbox_state before MCP startup so it can be sent to each // MCP server immediately after it becomes ready (avoiding blocking). let sandbox_state = SandboxState { sandbox_policy: session_configuration.sandbox_policy.get().clone(), @@ -1399,21 +1409,33 @@ impl Session { .map(|(name, _)| name.clone()) .collect(); required_mcp_servers.sort(); - let cancel_token = sess.mcp_startup_cancellation_token().await; - - sess.services - .mcp_connection_manager - .write() - .await - .initialize( - &mcp_servers, - config.mcp_oauth_credentials_store_mode, - auth_statuses.clone(), - tx_event.clone(), - cancel_token, - sandbox_state, - ) - .await; + { + let mut cancel_guard = sess.services.mcp_startup_cancellation_token.lock().await; + cancel_guard.cancel(); + *cancel_guard = CancellationToken::new(); + } + let (mcp_connection_manager, cancel_token) = McpConnectionManager::new( + &mcp_servers, + config.mcp_oauth_credentials_store_mode, + auth_statuses.clone(), + &session_configuration.approval_policy, + tx_event.clone(), + sandbox_state, + config.codex_home.clone(), + codex_apps_tools_cache_key(auth), + ) + .await; + { + let mut manager_guard = sess.services.mcp_connection_manager.write().await; + *manager_guard = mcp_connection_manager; + } + { + let mut cancel_guard = sess.services.mcp_startup_cancellation_token.lock().await; + if cancel_guard.is_cancelled() { + cancel_token.cancel(); + } + *cancel_guard = cancel_token; + } if !required_mcp_servers.is_empty() { let failures = sess .services @@ -1854,6 +1876,11 @@ impl Session { sandbox_policy_changed: bool, ) -> Arc { let per_turn_config = Self::build_per_turn_config(&session_configuration); + self.services + .mcp_connection_manager + .read() + .await + .set_approval_policy(&session_configuration.approval_policy); if sandbox_policy_changed { let sandbox_state = SandboxState { @@ -3032,19 +3059,29 @@ impl Session { sandbox_cwd: turn_context.cwd.clone(), use_linux_sandbox_bwrap: turn_context.features.enabled(Feature::UseLinuxSandboxBwrap), }; - let cancel_token = self.reset_mcp_startup_cancellation_token().await; - - let mut refreshed_manager = McpConnectionManager::default(); - refreshed_manager - .initialize( - &mcp_servers, - store_mode, - auth_statuses, - self.get_tx_event(), - cancel_token, - sandbox_state, - ) - .await; + { + let mut cancel_guard = self.services.mcp_startup_cancellation_token.lock().await; + cancel_guard.cancel(); + *cancel_guard = CancellationToken::new(); + } + let (refreshed_manager, cancel_token) = McpConnectionManager::new( + &mcp_servers, + store_mode, + auth_statuses, + &turn_context.config.permissions.approval_policy, + self.get_tx_event(), + sandbox_state, + config.codex_home.clone(), + codex_apps_tools_cache_key(auth.as_ref()), + ) + .await; + { + let mut cancel_guard = self.services.mcp_startup_cancellation_token.lock().await; + if cancel_guard.is_cancelled() { + cancel_token.cancel(); + } + *cancel_guard = cancel_token; + } let mut manager = self.services.mcp_connection_manager.write().await; *manager = refreshed_manager; @@ -3093,6 +3130,7 @@ impl Session { .await; } + #[cfg(test)] async fn mcp_startup_cancellation_token(&self) -> CancellationToken { self.services .mcp_startup_cancellation_token @@ -3101,14 +3139,6 @@ impl Session { .clone() } - async fn reset_mcp_startup_cancellation_token(&self) -> CancellationToken { - let mut guard = self.services.mcp_startup_cancellation_token.lock().await; - guard.cancel(); - let cancel_token = CancellationToken::new(); - *guard = cancel_token.clone(); - cancel_token - } - fn show_raw_agent_reasoning(&self) -> bool { self.services.show_raw_agent_reasoning } @@ -7364,7 +7394,11 @@ mod tests { let file_watcher = Arc::new(FileWatcher::noop()); let services = SessionServices { - mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())), + mcp_connection_manager: Arc::new(RwLock::new( + McpConnectionManager::new_mcp_connection_manager_for_tests( + &config.permissions.approval_policy, + ), + )), mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()), unified_exec_manager: UnifiedExecProcessManager::new( config.background_terminal_max_timeout, @@ -7516,7 +7550,11 @@ mod tests { let file_watcher = Arc::new(FileWatcher::noop()); let services = SessionServices { - mcp_connection_manager: Arc::new(RwLock::new(McpConnectionManager::default())), + mcp_connection_manager: Arc::new(RwLock::new( + McpConnectionManager::new_mcp_connection_manager_for_tests( + &config.permissions.approval_policy, + ), + )), mcp_startup_cancellation_token: Mutex::new(CancellationToken::new()), unified_exec_manager: UnifiedExecProcessManager::new( config.background_terminal_max_timeout, diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index f11026ede0..a27508d1a9 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -85,12 +85,14 @@ use tempfile::tempdir; #[cfg(not(target_os = "macos"))] type MacOsSeatbeltProfileExtensions = (); +use crate::config::permissions::network_proxy_config_from_permissions; use crate::config::profile::ConfigProfile; use toml::Value as TomlValue; use toml_edit::DocumentMut; pub mod edit; mod network_proxy_spec; +mod permissions; pub mod profile; pub mod schema; pub mod service; @@ -101,6 +103,8 @@ pub use codex_config::ConstraintResult; pub use network_proxy_spec::NetworkProxySpec; pub use network_proxy_spec::StartedNetworkProxy; +pub use permissions::NetworkToml; +pub use permissions::PermissionsToml; pub use service::ConfigService; pub use service::ConfigServiceError; @@ -295,6 +299,13 @@ pub struct Config { /// When unset, Codex will bind to an ephemeral port chosen by the OS. pub mcp_oauth_callback_port: Option, + /// Optional redirect URI to use during MCP OAuth login. + /// + /// When set, this URI is used in the OAuth authorization request instead + /// of the local listener address. The local callback listener still binds + /// to 127.0.0.1 (using `mcp_oauth_callback_port` when provided). + pub mcp_oauth_callback_url: Option, + /// Combined provider map (defaults merged with user-defined overrides). pub model_providers: HashMap, @@ -947,6 +958,10 @@ pub struct ConfigToml { /// Sandbox configuration to apply if `sandbox` is `WorkspaceWrite`. pub sandbox_workspace_write: Option, + /// Nested permissions settings. + #[serde(default)] + pub permissions: Option, + /// Optional external command to spawn for end-user notifications. #[serde(default)] pub notify: Option>, @@ -1005,6 +1020,12 @@ pub struct ConfigToml { /// When unset, Codex will bind to an ephemeral port chosen by the OS. pub mcp_oauth_callback_port: Option, + /// Optional redirect URI to use during MCP OAuth login. + /// When set, this URI is used in the OAuth authorization request instead + /// of the local listener address. The local callback listener still binds + /// to 127.0.0.1 (using `mcp_oauth_callback_port` when provided). + pub mcp_oauth_callback_url: Option, + /// User-defined provider entries that extend/override the built-in list. #[serde(default)] pub model_providers: HashMap, @@ -1577,6 +1598,8 @@ impl Config { .clone(), None => ConfigProfile::default(), }; + let configured_network_proxy_config = + network_proxy_config_from_permissions(cfg.permissions.as_ref()); let feature_overrides = FeatureOverrides { include_apply_patch_tool: include_apply_patch_tool_override, @@ -1889,18 +1912,29 @@ impl Config { let mcp_servers = constrain_mcp_servers(cfg.mcp_servers.clone(), mcp_servers.as_ref()) .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, format!("{e}")))?; - let network = match network_requirements { - Some(Sourced { value, source }) => { - let network = NetworkProxySpec::from_constraints(&config_layer_stack, value) - .map_err(|err| { - std::io::Error::new( - err.kind(), - format!("failed to build managed network proxy from {source}: {err}"), - ) - })?; - Some(network) + let (network_requirements, network_requirements_source) = match network_requirements { + Some(Sourced { value, source }) => (Some(value), Some(source)), + None => (None, None), + }; + let has_network_requirements = network_requirements.is_some(); + let network = NetworkProxySpec::from_config_and_constraints( + configured_network_proxy_config, + network_requirements, + ) + .map_err(|err| { + if let Some(source) = network_requirements_source.as_ref() { + std::io::Error::new( + err.kind(), + format!("failed to build managed network proxy from {source}: {err}"), + ) + } else { + err } - None => None, + })?; + let network = if has_network_requirements { + Some(network) + } else { + network.enabled().then_some(network) }; let config = Self { @@ -1937,6 +1971,7 @@ impl Config { // is important in code to differentiate the mode from the store implementation. mcp_oauth_credentials_store_mode: cfg.mcp_oauth_credentials_store.unwrap_or_default(), mcp_oauth_callback_port: cfg.mcp_oauth_callback_port, + mcp_oauth_callback_url: cfg.mcp_oauth_callback_url.clone(), model_providers, project_doc_max_bytes: cfg.project_doc_max_bytes.unwrap_or(PROJECT_DOC_MAX_BYTES), project_doc_fallback_filenames: cfg @@ -2339,6 +2374,95 @@ phase_2_model = "gpt-5" ); } + #[test] + fn config_toml_deserializes_permissions_network() { + let toml = r#" +[permissions.network] +enabled = true +proxy_url = "http://127.0.0.1:43128" +enable_socks5 = false +allow_upstream_proxy = false +allowed_domains = ["openai.com"] +"#; + let cfg: ConfigToml = toml::from_str(toml) + .expect("TOML deserialization should succeed for permissions.network"); + + assert_eq!( + cfg.permissions + .and_then(|permissions| permissions.network) + .expect("permissions.network should deserialize"), + NetworkToml { + enabled: Some(true), + proxy_url: Some("http://127.0.0.1:43128".to_string()), + admin_url: None, + enable_socks5: Some(false), + socks_url: None, + enable_socks5_udp: None, + allow_upstream_proxy: Some(false), + dangerously_allow_non_loopback_proxy: None, + dangerously_allow_non_loopback_admin: None, + mode: None, + allowed_domains: Some(vec!["openai.com".to_string()]), + denied_domains: None, + allow_unix_sockets: None, + allow_local_binding: None, + } + ); + } + + #[test] + fn permissions_network_enabled_populates_runtime_network_proxy_spec() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cfg = ConfigToml { + permissions: Some(PermissionsToml { + network: Some(NetworkToml { + enabled: Some(true), + proxy_url: Some("http://127.0.0.1:43128".to_string()), + enable_socks5: Some(false), + ..Default::default() + }), + }), + ..Default::default() + }; + + let config = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + )?; + let network = config + .permissions + .network + .as_ref() + .expect("enabled permissions.network should produce a NetworkProxySpec"); + + assert_eq!(network.proxy_host_and_port(), "127.0.0.1:43128"); + assert!(!network.socks_enabled()); + Ok(()) + } + + #[test] + fn permissions_network_disabled_by_default_does_not_start_proxy() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let cfg = ConfigToml { + permissions: Some(PermissionsToml { + network: Some(NetworkToml { + allowed_domains: Some(vec!["openai.com".to_string()]), + ..Default::default() + }), + }), + ..Default::default() + }; + + let config = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + )?; + assert!(config.permissions.network.is_none()); + Ok(()) + } + #[test] fn tui_config_missing_notifications_field_defaults_to_enabled() { let cfg = r#" @@ -4407,6 +4531,7 @@ model_verbosity = "high" mcp_servers: Constrained::allow_any(HashMap::new()), mcp_oauth_credentials_store_mode: Default::default(), mcp_oauth_callback_port: None, + mcp_oauth_callback_url: None, model_providers: fixture.model_provider_map.clone(), project_doc_max_bytes: PROJECT_DOC_MAX_BYTES, project_doc_fallback_filenames: Vec::new(), @@ -4524,6 +4649,7 @@ model_verbosity = "high" mcp_servers: Constrained::allow_any(HashMap::new()), mcp_oauth_credentials_store_mode: Default::default(), mcp_oauth_callback_port: None, + mcp_oauth_callback_url: None, model_providers: fixture.model_provider_map.clone(), project_doc_max_bytes: PROJECT_DOC_MAX_BYTES, project_doc_fallback_filenames: Vec::new(), @@ -4639,6 +4765,7 @@ model_verbosity = "high" mcp_servers: Constrained::allow_any(HashMap::new()), mcp_oauth_credentials_store_mode: Default::default(), mcp_oauth_callback_port: None, + mcp_oauth_callback_url: None, model_providers: fixture.model_provider_map.clone(), project_doc_max_bytes: PROJECT_DOC_MAX_BYTES, project_doc_fallback_filenames: Vec::new(), @@ -4740,6 +4867,7 @@ model_verbosity = "high" mcp_servers: Constrained::allow_any(HashMap::new()), mcp_oauth_credentials_store_mode: Default::default(), mcp_oauth_callback_port: None, + mcp_oauth_callback_url: None, model_providers: fixture.model_provider_map.clone(), project_doc_max_bytes: PROJECT_DOC_MAX_BYTES, project_doc_fallback_filenames: Vec::new(), @@ -5269,6 +5397,17 @@ trust_level = "untrusted" assert_eq!(cfg.mcp_oauth_callback_port, Some(4321)); } + #[test] + fn config_toml_deserializes_mcp_oauth_callback_url() { + let toml = r#"mcp_oauth_callback_url = "https://example.com/callback""#; + let cfg: ConfigToml = + toml::from_str(toml).expect("TOML deserialization should succeed for callback URL"); + assert_eq!( + cfg.mcp_oauth_callback_url.as_deref(), + Some("https://example.com/callback") + ); + } + #[test] fn config_loads_mcp_oauth_callback_port_from_toml() -> std::io::Result<()> { let codex_home = TempDir::new()?; @@ -5289,6 +5428,29 @@ mcp_oauth_callback_port = 5678 Ok(()) } + #[test] + fn config_loads_mcp_oauth_callback_url_from_toml() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let toml = r#" +model = "gpt-5.1" +mcp_oauth_callback_url = "https://example.com/callback" +"#; + let cfg: ConfigToml = + toml::from_str(toml).expect("TOML deserialization should succeed for callback URL"); + + let config = Config::load_from_base_config_with_overrides( + cfg, + ConfigOverrides::default(), + codex_home.path().to_path_buf(), + )?; + + assert_eq!( + config.mcp_oauth_callback_url.as_deref(), + Some("https://example.com/callback") + ); + Ok(()) + } + #[test] fn test_untrusted_project_gets_unless_trusted_approval_policy() -> anyhow::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/config/network_proxy_spec.rs b/codex-rs/core/src/config/network_proxy_spec.rs index 183eafcb17..1957455d6f 100644 --- a/codex-rs/core/src/config/network_proxy_spec.rs +++ b/codex-rs/core/src/config/network_proxy_spec.rs @@ -1,4 +1,3 @@ -use crate::config; use crate::config_loader::NetworkConstraints; use async_trait::async_trait; use codex_network_proxy::BlockedRequestObserver; @@ -68,6 +67,10 @@ impl ConfigReloader for StaticNetworkProxyReloader { } impl NetworkProxySpec { + pub(crate) fn enabled(&self) -> bool { + self.config.network.enabled + } + pub fn proxy_host_and_port(&self) -> String { host_and_port_from_network_addr(&self.config.network.proxy_url, 3128) } @@ -76,14 +79,15 @@ impl NetworkProxySpec { self.config.network.enable_socks5 } - pub(crate) fn from_constraints( - _config_layer_stack: &config::ConfigLayerStack, - requirements: NetworkConstraints, + pub(crate) fn from_config_and_constraints( + config: NetworkProxyConfig, + requirements: Option, ) -> std::io::Result { - // TODO(mbolin): Use ConfigLayerStack once we are ready to start - // honoring network configuration in config.toml. - let config = NetworkProxyConfig::default(); - let (config, constraints) = Self::apply_requirements(config, &requirements); + let (config, constraints) = if let Some(requirements) = requirements { + Self::apply_requirements(config, &requirements) + } else { + (config, NetworkProxyConstraints::default()) + }; validate_policy_against_constraints(&config, &constraints).map_err(|err| { std::io::Error::new( std::io::ErrorKind::InvalidInput, diff --git a/codex-rs/core/src/config/permissions.rs b/codex-rs/core/src/config/permissions.rs new file mode 100644 index 0000000000..a03b5273da --- /dev/null +++ b/codex-rs/core/src/config/permissions.rs @@ -0,0 +1,110 @@ +use codex_network_proxy::NetworkMode; +use codex_network_proxy::NetworkProxyConfig; +use schemars::JsonSchema; +use serde::Deserialize; +use serde::Serialize; + +#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct PermissionsToml { + /// Network proxy settings from `[permissions.network]`. + /// User config can enable the proxy; managed requirements may still constrain values. + pub network: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct NetworkToml { + pub enabled: Option, + pub proxy_url: Option, + pub admin_url: Option, + pub enable_socks5: Option, + pub socks_url: Option, + pub enable_socks5_udp: Option, + pub allow_upstream_proxy: Option, + pub dangerously_allow_non_loopback_proxy: Option, + pub dangerously_allow_non_loopback_admin: Option, + #[schemars(with = "Option")] + pub mode: Option, + pub allowed_domains: Option>, + pub denied_domains: Option>, + pub allow_unix_sockets: Option>, + pub allow_local_binding: Option, +} + +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] +#[serde(rename_all = "lowercase")] +enum NetworkModeSchema { + Limited, + Full, +} + +impl NetworkToml { + pub(crate) fn apply_to_network_proxy_config(&self, config: &mut NetworkProxyConfig) { + if let Some(enabled) = self.enabled { + config.network.enabled = enabled; + } + if let Some(proxy_url) = self.proxy_url.as_ref() { + config.network.proxy_url = proxy_url.clone(); + } + if let Some(admin_url) = self.admin_url.as_ref() { + config.network.admin_url = admin_url.clone(); + } + if let Some(enable_socks5) = self.enable_socks5 { + config.network.enable_socks5 = enable_socks5; + } + if let Some(socks_url) = self.socks_url.as_ref() { + config.network.socks_url = socks_url.clone(); + } + if let Some(enable_socks5_udp) = self.enable_socks5_udp { + config.network.enable_socks5_udp = enable_socks5_udp; + } + if let Some(allow_upstream_proxy) = self.allow_upstream_proxy { + config.network.allow_upstream_proxy = allow_upstream_proxy; + } + if let Some(dangerously_allow_non_loopback_proxy) = + self.dangerously_allow_non_loopback_proxy + { + config.network.dangerously_allow_non_loopback_proxy = + dangerously_allow_non_loopback_proxy; + } + if let Some(dangerously_allow_non_loopback_admin) = + self.dangerously_allow_non_loopback_admin + { + config.network.dangerously_allow_non_loopback_admin = + dangerously_allow_non_loopback_admin; + } + if let Some(mode) = self.mode { + config.network.mode = mode; + } + if let Some(allowed_domains) = self.allowed_domains.as_ref() { + config.network.allowed_domains = allowed_domains.clone(); + } + if let Some(denied_domains) = self.denied_domains.as_ref() { + config.network.denied_domains = denied_domains.clone(); + } + if let Some(allow_unix_sockets) = self.allow_unix_sockets.as_ref() { + config.network.allow_unix_sockets = allow_unix_sockets.clone(); + } + if let Some(allow_local_binding) = self.allow_local_binding { + config.network.allow_local_binding = allow_local_binding; + } + } + + pub(crate) fn to_network_proxy_config(&self) -> NetworkProxyConfig { + let mut config = NetworkProxyConfig::default(); + self.apply_to_network_proxy_config(&mut config); + config + } +} + +pub(crate) fn network_proxy_config_from_permissions( + permissions: Option<&PermissionsToml>, +) -> NetworkProxyConfig { + permissions + .and_then(|permissions| permissions.network.as_ref()) + .map_or_else( + NetworkProxyConfig::default, + NetworkToml::to_network_proxy_config, + ) +} diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 5a8930231b..a34477850d 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -12,7 +12,6 @@ pub use codex_app_server_protocol::AppInfo; pub use codex_app_server_protocol::AppMetadata; use codex_protocol::protocol::SandboxPolicy; use serde::Deserialize; -use tokio_util::sync::CancellationToken; use tracing::warn; use crate::AuthManager; @@ -20,15 +19,18 @@ use crate::CodexAuth; use crate::SandboxState; use crate::config::Config; use crate::config::types::AppsConfigToml; +use crate::default_client::is_first_party_chat_originator; +use crate::default_client::originator; use crate::features::Feature; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; use crate::mcp::auth::compute_auth_statuses; use crate::mcp::with_codex_apps_mcp; -use crate::mcp_connection_manager::DEFAULT_STARTUP_TIMEOUT; use crate::mcp_connection_manager::McpConnectionManager; +use crate::mcp_connection_manager::codex_apps_tools_cache_key; use crate::token_data::TokenData; pub const CONNECTORS_CACHE_TTL: Duration = Duration::from_secs(3600); +const CONNECTORS_READY_TIMEOUT_ON_EMPTY_TOOLS: Duration = Duration::from_secs(30); #[derive(Clone, PartialEq, Eq)] struct AccessibleConnectorsCacheKey { @@ -48,10 +50,20 @@ struct CachedAccessibleConnectors { static ACCESSIBLE_CONNECTORS_CACHE: LazyLock>> = LazyLock::new(|| StdMutex::new(None)); +#[derive(Debug, Clone)] +pub struct AccessibleConnectorsStatus { + pub connectors: Vec, + pub codex_apps_ready: bool, +} + pub async fn list_accessible_connectors_from_mcp_tools( config: &Config, ) -> anyhow::Result> { - list_accessible_connectors_from_mcp_tools_with_options(config, false).await + Ok( + list_accessible_connectors_from_mcp_tools_with_options_and_status(config, false) + .await? + .connectors, + ) } pub async fn list_cached_accessible_connectors_from_mcp_tools( @@ -64,15 +76,29 @@ pub async fn list_cached_accessible_connectors_from_mcp_tools( let auth_manager = auth_manager_from_config(config); let auth = auth_manager.auth().await; let cache_key = accessible_connectors_cache_key(config, auth.as_ref()); - read_cached_accessible_connectors(&cache_key) + read_cached_accessible_connectors(&cache_key).map(filter_disallowed_connectors) } pub async fn list_accessible_connectors_from_mcp_tools_with_options( config: &Config, force_refetch: bool, ) -> anyhow::Result> { + Ok( + list_accessible_connectors_from_mcp_tools_with_options_and_status(config, force_refetch) + .await? + .connectors, + ) +} + +pub async fn list_accessible_connectors_from_mcp_tools_with_options_and_status( + config: &Config, + force_refetch: bool, +) -> anyhow::Result { if !config.features.enabled(Feature::Apps) { - return Ok(Vec::new()); + return Ok(AccessibleConnectorsStatus { + connectors: Vec::new(), + codex_apps_ready: true, + }); } let auth_manager = auth_manager_from_config(config); @@ -80,21 +106,26 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options( let cache_key = accessible_connectors_cache_key(config, auth.as_ref()); if !force_refetch && let Some(cached_connectors) = read_cached_accessible_connectors(&cache_key) { - return Ok(cached_connectors); + let cached_connectors = filter_disallowed_connectors(cached_connectors); + return Ok(AccessibleConnectorsStatus { + connectors: cached_connectors, + codex_apps_ready: true, + }); } let mcp_servers = with_codex_apps_mcp(HashMap::new(), true, auth.as_ref(), config); if mcp_servers.is_empty() { - return Ok(Vec::new()); + return Ok(AccessibleConnectorsStatus { + connectors: Vec::new(), + codex_apps_ready: true, + }); } let auth_status_entries = compute_auth_statuses(mcp_servers.iter(), config.mcp_oauth_credentials_store_mode).await; - let mut mcp_connection_manager = McpConnectionManager::default(); let (tx_event, rx_event) = unbounded(); drop(rx_event); - let cancel_token = CancellationToken::new(); let sandbox_state = SandboxState { sandbox_policy: SandboxPolicy::new_read_only_policy(), @@ -103,16 +134,17 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options( use_linux_sandbox_bwrap: config.features.enabled(Feature::UseLinuxSandboxBwrap), }; - mcp_connection_manager - .initialize( - &mcp_servers, - config.mcp_oauth_credentials_store_mode, - auth_status_entries, - tx_event, - cancel_token.clone(), - sandbox_state, - ) - .await; + let (mcp_connection_manager, cancel_token) = McpConnectionManager::new( + &mcp_servers, + config.mcp_oauth_credentials_store_mode, + auth_status_entries, + &config.permissions.approval_policy, + tx_event, + sandbox_state, + config.codex_home.clone(), + codex_apps_tools_cache_key(auth.as_ref()), + ) + .await; if force_refetch && let Err(err) = mcp_connection_manager @@ -124,23 +156,45 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_options( ); } + let mut tools = mcp_connection_manager.list_all_tools().await; + let mut should_reload_tools = false; let codex_apps_ready = if let Some(cfg) = mcp_servers.get(CODEX_APPS_MCP_SERVER_NAME) { - let timeout = cfg.startup_timeout_sec.unwrap_or(DEFAULT_STARTUP_TIMEOUT); - mcp_connection_manager - .wait_for_server_ready(CODEX_APPS_MCP_SERVER_NAME, timeout) - .await + let immediate_ready = mcp_connection_manager + .wait_for_server_ready(CODEX_APPS_MCP_SERVER_NAME, Duration::ZERO) + .await; + if immediate_ready { + true + } else if tools.is_empty() { + let timeout = cfg + .startup_timeout_sec + .unwrap_or(CONNECTORS_READY_TIMEOUT_ON_EMPTY_TOOLS); + let ready = mcp_connection_manager + .wait_for_server_ready(CODEX_APPS_MCP_SERVER_NAME, timeout) + .await; + should_reload_tools = ready; + ready + } else { + false + } } else { false }; + if should_reload_tools { + tools = mcp_connection_manager.list_all_tools().await; + } + if codex_apps_ready { + cancel_token.cancel(); + } - let tools = mcp_connection_manager.list_all_tools().await; - cancel_token.cancel(); - - let accessible_connectors = accessible_connectors_from_mcp_tools(&tools); + let accessible_connectors = + filter_disallowed_connectors(accessible_connectors_from_mcp_tools(&tools)); if codex_apps_ready || !accessible_connectors.is_empty() { write_cached_accessible_connectors(cache_key, &accessible_connectors); } - Ok(accessible_connectors) + Ok(AccessibleConnectorsStatus { + connectors: accessible_connectors, + codex_apps_ready, + }) } fn accessible_connectors_cache_key( @@ -292,6 +346,48 @@ pub fn with_app_enabled_state(mut connectors: Vec, config: &Config) -> connectors } +const DISALLOWED_CONNECTOR_IDS: &[&str] = &[ + "asdk_app_6938a94a61d881918ef32cb999ff937c", + "connector_2b0a9009c9c64bf9933a3dae3f2b1254", + "connector_68de829bf7648191acd70a907364c67c", + "connector_68e004f14af881919eb50893d3d9f523", + "connector_69272cb413a081919685ec3c88d1744e", +]; +const FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS: &[&str] = + &["connector_0f9c9d4592e54d0a9a12b3f44a1e2010"]; +const DISALLOWED_CONNECTOR_PREFIX: &str = "connector_openai_"; + +pub fn filter_disallowed_connectors(connectors: Vec) -> Vec { + filter_disallowed_connectors_for_originator(connectors, originator().value.as_str()) +} + +pub(crate) fn is_connector_id_allowed(connector_id: &str) -> bool { + is_connector_id_allowed_for_originator(connector_id, originator().value.as_str()) +} + +fn filter_disallowed_connectors_for_originator( + connectors: Vec, + originator_value: &str, +) -> Vec { + connectors + .into_iter() + .filter(|connector| { + is_connector_id_allowed_for_originator(connector.id.as_str(), originator_value) + }) + .collect() +} + +fn is_connector_id_allowed_for_originator(connector_id: &str, originator_value: &str) -> bool { + let disallowed_connector_ids = if is_first_party_chat_originator(originator_value) { + FIRST_PARTY_CHAT_DISALLOWED_CONNECTOR_IDS + } else { + DISALLOWED_CONNECTOR_IDS + }; + + !connector_id.starts_with(DISALLOWED_CONNECTOR_PREFIX) + && !disallowed_connector_ids.contains(&connector_id) +} + fn read_apps_config(config: &Config) -> Option { let effective_config = config.config_layer_stack.effective_config(); let apps_config = effective_config.as_table()?.get("apps")?.clone(); @@ -372,3 +468,67 @@ pub fn connector_name_slug(name: &str) -> String { fn format_connector_label(name: &str, _id: &str) -> String { name.to_string() } + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + + fn app(id: &str) -> AppInfo { + AppInfo { + id: id.to_string(), + name: id.to_string(), + description: None, + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + install_url: None, + branding: None, + app_metadata: None, + labels: None, + is_accessible: false, + is_enabled: true, + } + } + + #[test] + fn filter_disallowed_connectors_allows_non_disallowed_connectors() { + let filtered = filter_disallowed_connectors(vec![app("asdk_app_hidden"), app("alpha")]); + assert_eq!(filtered, vec![app("asdk_app_hidden"), app("alpha")]); + } + + #[test] + fn filter_disallowed_connectors_filters_openai_prefix() { + let filtered = filter_disallowed_connectors(vec![ + app("connector_openai_foo"), + app("connector_openai_bar"), + app("gamma"), + ]); + assert_eq!(filtered, vec![app("gamma")]); + } + + #[test] + fn filter_disallowed_connectors_filters_disallowed_connector_ids() { + let filtered = filter_disallowed_connectors(vec![ + app("asdk_app_6938a94a61d881918ef32cb999ff937c"), + app("delta"), + ]); + assert_eq!(filtered, vec![app("delta")]); + } + + #[test] + fn first_party_chat_originator_filters_target_and_openai_prefixed_connectors() { + let filtered = filter_disallowed_connectors_for_originator( + vec![ + app("connector_openai_foo"), + app("asdk_app_6938a94a61d881918ef32cb999ff937c"), + app("connector_0f9c9d4592e54d0a9a12b3f44a1e2010"), + ], + "codex_atlas", + ); + assert_eq!( + filtered, + vec![app("asdk_app_6938a94a61d881918ef32cb999ff937c"),] + ); + } +} diff --git a/codex-rs/core/src/exec_policy.rs b/codex-rs/core/src/exec_policy.rs index 62ebdde43d..f2cedb3f94 100644 --- a/codex-rs/core/src/exec_policy.rs +++ b/codex-rs/core/src/exec_policy.rs @@ -32,6 +32,10 @@ use shlex::try_join as shlex_try_join; const PROMPT_CONFLICT_REASON: &str = "approval required by policy, but AskForApproval is set to Never"; +const REJECT_SANDBOX_APPROVAL_REASON: &str = + "approval required by policy, but AskForApproval::Reject.sandbox_approval is set"; +const REJECT_RULES_APPROVAL_REASON: &str = + "approval required by policy rule, but AskForApproval::Reject.rules is set"; const RULES_DIR_NAME: &str = "rules"; const RULE_EXTENSION: &str = "rules"; const DEFAULT_POLICY_FILE: &str = "default.rules"; @@ -91,6 +95,37 @@ fn is_policy_match(rule_match: &RuleMatch) -> bool { } } +/// Returns a rejection reason when `approval_policy` disallows surfacing the +/// current prompt to the user. +/// +/// `prompt_is_rule` distinguishes policy-rule prompts from sandbox/escalation +/// prompts so `Reject.rules` and `Reject.sandbox_approval` are honored +/// independently. When both are present, policy-rule prompts take precedence. +fn prompt_is_rejected_by_policy( + approval_policy: AskForApproval, + prompt_is_rule: bool, +) -> Option<&'static str> { + match approval_policy { + AskForApproval::Never => Some(PROMPT_CONFLICT_REASON), + AskForApproval::OnFailure => None, + AskForApproval::OnRequest => None, + AskForApproval::UnlessTrusted => None, + AskForApproval::Reject(reject_config) => { + if prompt_is_rule { + if reject_config.rejects_rules_approval() { + Some(REJECT_RULES_APPROVAL_REASON) + } else { + None + } + } else if reject_config.rejects_sandbox_approval() { + Some(REJECT_SANDBOX_APPROVAL_REASON) + } else { + None + } + } + } +} + #[derive(Debug, Error)] pub enum ExecPolicyError { #[error("failed to read rules files from {dir}: {source}")] @@ -196,12 +231,14 @@ impl ExecPolicyManager { reason: derive_forbidden_reason(command, &evaluation), }, Decision::Prompt => { - if matches!(approval_policy, AskForApproval::Never) { - ExecApprovalRequirement::Forbidden { - reason: PROMPT_CONFLICT_REASON.to_string(), - } - } else { - ExecApprovalRequirement::NeedsApproval { + let prompt_is_rule = evaluation.matched_rules.iter().any(|rule_match| { + is_policy_match(rule_match) && rule_match.decision() == Decision::Prompt + }); + match prompt_is_rejected_by_policy(approval_policy, prompt_is_rule) { + Some(reason) => ExecApprovalRequirement::Forbidden { + reason: reason.to_string(), + }, + None => ExecApprovalRequirement::NeedsApproval { reason: derive_prompt_reason(command, &evaluation), proposed_execpolicy_amendment: requested_amendment.or_else(|| { if auto_amendment_allowed { @@ -212,7 +249,7 @@ impl ExecPolicyManager { None } }), - } + }, } } Decision::Allow => ExecApprovalRequirement::Skip { @@ -465,6 +502,20 @@ pub fn render_decision_for_unmatched_command( } } } + AskForApproval::Reject(_) => match sandbox_policy { + SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } => { + // Mirror on-request behavior for unmatched commands; prompt-vs-reject is handled + // by `prompt_is_rejected_by_policy`. + Decision::Allow + } + SandboxPolicy::ReadOnly { .. } | SandboxPolicy::WorkspaceWrite { .. } => { + if sandbox_permissions.requires_escalated_permissions() { + Decision::Prompt + } else { + Decision::Allow + } + } + }, } } @@ -690,6 +741,7 @@ mod tests { use crate::config_loader::ConfigRequirementsToml; use codex_app_server_protocol::ConfigLayerSource; use codex_protocol::protocol::AskForApproval; + use codex_protocol::protocol::RejectConfig; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; @@ -1197,6 +1249,122 @@ prefix_rule( ); } + #[test] + fn unmatched_reject_policy_still_prompts_for_restricted_sandbox_escalation() { + let command = vec!["madeup-cmd".to_string()]; + + assert_eq!( + Decision::Prompt, + render_decision_for_unmatched_command( + AskForApproval::Reject(RejectConfig { + sandbox_approval: false, + rules: false, + mcp_elicitations: false, + }), + &SandboxPolicy::new_read_only_policy(), + &command, + SandboxPermissions::RequireEscalated, + false, + ) + ); + } + + #[tokio::test] + async fn exec_approval_requirement_rejects_unmatched_prompt_when_sandbox_rejection_enabled() { + let command = vec!["madeup-cmd".to_string()]; + + let requirement = ExecPolicyManager::default() + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::Reject(RejectConfig { + sandbox_approval: true, + rules: false, + mcp_elicitations: false, + }), + sandbox_policy: &SandboxPolicy::new_read_only_policy(), + sandbox_permissions: SandboxPermissions::RequireEscalated, + prefix_rule: None, + }) + .await; + + assert_eq!( + requirement, + ExecApprovalRequirement::Forbidden { + reason: REJECT_SANDBOX_APPROVAL_REASON.to_string(), + } + ); + } + + #[tokio::test] + async fn mixed_rule_and_sandbox_prompt_prioritizes_rule_for_rejection_decision() { + let policy_src = r#"prefix_rule(pattern=["git"], decision="prompt")"#; + let mut parser = PolicyParser::new(); + parser + .parse("test.rules", policy_src) + .expect("parse policy"); + let manager = ExecPolicyManager::new(Arc::new(parser.build())); + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + "git status && madeup-cmd".to_string(), + ]; + + let requirement = manager + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::Reject(RejectConfig { + sandbox_approval: true, + rules: false, + mcp_elicitations: false, + }), + sandbox_policy: &SandboxPolicy::new_read_only_policy(), + sandbox_permissions: SandboxPermissions::RequireEscalated, + prefix_rule: None, + }) + .await; + + assert!(matches!( + requirement, + ExecApprovalRequirement::NeedsApproval { .. } + )); + } + + #[tokio::test] + async fn mixed_rule_and_sandbox_prompt_rejects_when_rules_rejection_enabled() { + let policy_src = r#"prefix_rule(pattern=["git"], decision="prompt")"#; + let mut parser = PolicyParser::new(); + parser + .parse("test.rules", policy_src) + .expect("parse policy"); + let manager = ExecPolicyManager::new(Arc::new(parser.build())); + let command = vec![ + "bash".to_string(), + "-lc".to_string(), + "git status && madeup-cmd".to_string(), + ]; + + let requirement = manager + .create_exec_approval_requirement_for_command(ExecApprovalRequest { + command: &command, + approval_policy: AskForApproval::Reject(RejectConfig { + sandbox_approval: false, + rules: true, + mcp_elicitations: false, + }), + sandbox_policy: &SandboxPolicy::new_read_only_policy(), + sandbox_permissions: SandboxPermissions::RequireEscalated, + prefix_rule: None, + }) + .await; + + assert_eq!( + requirement, + ExecApprovalRequirement::Forbidden { + reason: REJECT_RULES_APPROVAL_REASON.to_string(), + } + ); + } + #[tokio::test] async fn exec_approval_requirement_falls_back_to_heuristics() { let command = vec!["cargo".to_string(), "build".to_string()]; diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index c79bb00bc9..a4be91046c 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -250,6 +250,9 @@ impl Features { pub fn emit_metrics(&self, otel: &OtelManager) { for feature in FEATURES { + if matches!(feature.stage, Stage::Removed) { + continue; + } if self.enabled(feature.id) != feature.default_enabled { otel.counter( "codex.feature.state", diff --git a/codex-rs/core/src/mcp/mod.rs b/codex-rs/core/src/mcp/mod.rs index 1365b5da8b..f2c2557514 100644 --- a/codex-rs/core/src/mcp/mod.rs +++ b/codex-rs/core/src/mcp/mod.rs @@ -14,7 +14,6 @@ use codex_protocol::mcp::Tool; use codex_protocol::protocol::McpListToolsResponseEvent; use codex_protocol::protocol::SandboxPolicy; use serde_json::Value; -use tokio_util::sync::CancellationToken; use crate::AuthManager; use crate::CodexAuth; @@ -25,6 +24,7 @@ use crate::features::Feature; use crate::mcp::auth::compute_auth_statuses; use crate::mcp_connection_manager::McpConnectionManager; use crate::mcp_connection_manager::SandboxState; +use crate::mcp_connection_manager::codex_apps_tools_cache_key; const MCP_TOOL_NAME_PREFIX: &str = "mcp"; const MCP_TOOL_NAME_DELIMITER: &str = "__"; @@ -191,10 +191,8 @@ pub async fn collect_mcp_snapshot(config: &Config) -> McpListToolsResponseEvent let auth_status_entries = compute_auth_statuses(mcp_servers.iter(), config.mcp_oauth_credentials_store_mode).await; - let mut mcp_connection_manager = McpConnectionManager::default(); let (tx_event, rx_event) = unbounded(); drop(rx_event); - let cancel_token = CancellationToken::new(); // Use ReadOnly sandbox policy for MCP snapshot collection (safest default) let sandbox_state = SandboxState { @@ -204,16 +202,17 @@ pub async fn collect_mcp_snapshot(config: &Config) -> McpListToolsResponseEvent use_linux_sandbox_bwrap: config.features.enabled(Feature::UseLinuxSandboxBwrap), }; - mcp_connection_manager - .initialize( - &mcp_servers, - config.mcp_oauth_credentials_store_mode, - auth_status_entries.clone(), - tx_event, - cancel_token.clone(), - sandbox_state, - ) - .await; + let (mcp_connection_manager, cancel_token) = McpConnectionManager::new( + &mcp_servers, + config.mcp_oauth_credentials_store_mode, + auth_status_entries.clone(), + &config.permissions.approval_policy, + tx_event, + sandbox_state, + config.codex_home.clone(), + codex_apps_tools_cache_key(auth.as_ref()), + ) + .await; let snapshot = collect_mcp_snapshot_from_manager(&mcp_connection_manager, auth_status_entries).await; diff --git a/codex-rs/core/src/mcp/skill_dependencies.rs b/codex-rs/core/src/mcp/skill_dependencies.rs index 0e6d61055e..e8b2d4f7b0 100644 --- a/codex-rs/core/src/mcp/skill_dependencies.rs +++ b/codex-rs/core/src/mcp/skill_dependencies.rs @@ -242,6 +242,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( oauth_config.env_http_headers, &[], config.mcp_oauth_callback_port, + config.mcp_oauth_callback_url.as_deref(), ) .await { diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 62c4dedd47..c76e19809f 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -12,8 +12,9 @@ use std::env; use std::ffi::OsString; use std::path::PathBuf; use std::sync::Arc; -use std::sync::LazyLock; use std::sync::Mutex as StdMutex; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; use std::time::Duration; use std::time::Instant; @@ -25,9 +26,11 @@ use anyhow::anyhow; use async_channel::Sender; use codex_async_utils::CancelErr; use codex_async_utils::OrCancelExt; +use codex_config::Constrained; use codex_protocol::approvals::ElicitationRequestEvent; use codex_protocol::mcp::CallToolResult; use codex_protocol::mcp::RequestId as ProtocolRequestId; +use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::McpStartupCompleteEvent; @@ -44,6 +47,7 @@ use futures::future::FutureExt; use futures::future::Shared; use rmcp::model::ClientCapabilities; use rmcp::model::CreateElicitationRequestParams; +use rmcp::model::ElicitationAction; use rmcp::model::ElicitationCapability; use rmcp::model::FormElicitationCapability; use rmcp::model::Implementation; @@ -73,6 +77,7 @@ use tracing::warn; use crate::codex::INITIAL_SUBMIT_ID; use crate::config::types::McpServerConfig; use crate::config::types::McpServerTransportConfig; +use crate::connectors::is_connector_id_allowed; /// Delimiter used to separate the server name from the tool name in a fully /// qualified tool name. @@ -88,7 +93,8 @@ pub const DEFAULT_STARTUP_TIMEOUT: Duration = Duration::from_secs(10); /// Default timeout for individual tool calls. const DEFAULT_TOOL_TIMEOUT: Duration = Duration::from_secs(60); -const CODEX_APPS_TOOLS_CACHE_TTL: Duration = Duration::from_secs(3600); +const CODEX_APPS_TOOLS_CACHE_SCHEMA_VERSION: u8 = 1; +const CODEX_APPS_TOOLS_CACHE_DIR: &str = "cache/codex_apps_tools"; const MCP_TOOLS_LIST_DURATION_METRIC: &str = "codex.mcp.tools.list.duration_ms"; const MCP_TOOLS_FETCH_UNCACHED_DURATION_METRIC: &str = "codex.mcp.tools.fetch_uncached.duration_ms"; const MCP_TOOLS_CACHE_WRITE_DURATION_METRIC: &str = "codex.mcp.tools.cache_write.duration_ms"; @@ -120,6 +126,27 @@ fn sha1_hex(s: &str) -> String { format!("{sha1:x}") } +pub(crate) fn codex_apps_tools_cache_key( + auth: Option<&crate::CodexAuth>, +) -> CodexAppsToolsCacheKey { + let token_data = auth.and_then(|auth| auth.get_token_data().ok()); + let account_id = token_data + .as_ref() + .and_then(|token_data| token_data.account_id.clone()); + let chatgpt_user_id = token_data + .as_ref() + .and_then(|token_data| token_data.id_token.chatgpt_user_id.clone()); + let is_workspace_account = token_data + .as_ref() + .is_some_and(|token_data| token_data.id_token.is_workspace_account()); + + CodexAppsToolsCacheKey { + account_id, + chatgpt_user_id, + is_workspace_account, + } +} + fn qualify_tools(tools: I) -> HashMap where I: IntoIterator, @@ -162,7 +189,7 @@ where qualified_tools } -#[derive(Clone)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub(crate) struct ToolInfo { pub(crate) server_name: String, pub(crate) tool_name: String, @@ -171,23 +198,67 @@ pub(crate) struct ToolInfo { pub(crate) connector_name: Option, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub(crate) struct CodexAppsToolsCacheKey { + account_id: Option, + chatgpt_user_id: Option, + is_workspace_account: bool, +} + #[derive(Clone)] -struct CachedCodexAppsTools { - expires_at: Instant, +struct CodexAppsToolsCacheContext { + codex_home: PathBuf, + user_key: CodexAppsToolsCacheKey, +} + +impl CodexAppsToolsCacheContext { + fn cache_path(&self) -> PathBuf { + let user_key_json = serde_json::to_string(&self.user_key).unwrap_or_default(); + let user_key_hash = sha1_hex(&user_key_json); + self.codex_home + .join(CODEX_APPS_TOOLS_CACHE_DIR) + .join(format!("{user_key_hash}.json")) + } +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +struct CodexAppsToolsDiskCache { + schema_version: u8, tools: Vec, } -static CODEX_APPS_TOOLS_CACHE: LazyLock>> = - LazyLock::new(|| StdMutex::new(None)); +enum CachedCodexAppsToolsLoad { + Hit(Vec), + Missing, + Invalid, +} type ResponderMap = HashMap<(String, RequestId), oneshot::Sender>; -#[derive(Clone, Default)] +fn elicitation_is_rejected_by_policy(approval_policy: AskForApproval) -> bool { + match approval_policy { + AskForApproval::Never => true, + AskForApproval::OnFailure => false, + AskForApproval::OnRequest => false, + AskForApproval::UnlessTrusted => false, + AskForApproval::Reject(reject_config) => reject_config.rejects_mcp_elicitations(), + } +} + +#[derive(Clone)] struct ElicitationRequestManager { requests: Arc>, + approval_policy: Arc>, } impl ElicitationRequestManager { + fn new(approval_policy: AskForApproval) -> Self { + Self { + requests: Arc::new(Mutex::new(HashMap::new())), + approval_policy: Arc::new(StdMutex::new(approval_policy)), + } + } + async fn resolve( &self, server_name: String, @@ -205,11 +276,23 @@ impl ElicitationRequestManager { fn make_sender(&self, server_name: String, tx_event: Sender) -> SendElicitation { let elicitation_requests = self.requests.clone(); + let approval_policy = self.approval_policy.clone(); Box::new(move |id, elicitation| { let elicitation_requests = elicitation_requests.clone(); let tx_event = tx_event.clone(); let server_name = server_name.clone(); + let approval_policy = approval_policy.clone(); async move { + if approval_policy + .lock() + .is_ok_and(|policy| elicitation_is_rejected_by_policy(*policy)) + { + return Ok(ElicitationResponse { + action: ElicitationAction::Decline, + content: None, + }); + } + let (tx, rx) = oneshot::channel(); { let mut lock = elicitation_requests.lock().await; @@ -256,9 +339,35 @@ struct ManagedClient { tool_filter: ToolFilter, tool_timeout: Option, server_supports_sandbox_state_capability: bool, + codex_apps_tools_cache_context: Option, } impl ManagedClient { + fn listed_tools(&self) -> Vec { + let total_start = Instant::now(); + if let Some(cache_context) = self.codex_apps_tools_cache_context.as_ref() + && let CachedCodexAppsToolsLoad::Hit(tools) = + load_cached_codex_apps_tools(cache_context) + { + emit_duration( + MCP_TOOLS_LIST_DURATION_METRIC, + total_start.elapsed(), + &[("cache", "hit")], + ); + return filter_tools(tools, &self.tool_filter); + } + + if self.codex_apps_tools_cache_context.is_some() { + emit_duration( + MCP_TOOLS_LIST_DURATION_METRIC, + total_start.elapsed(), + &[("cache", "miss")], + ); + } + + self.tools.clone() + } + /// Returns once the server has ack'd the sandbox state update. async fn notify_sandbox_state_change(&self, sandbox_state: &SandboxState) -> Result<()> { if !self.server_supports_sandbox_state_capability { @@ -279,6 +388,8 @@ impl ManagedClient { #[derive(Clone)] struct AsyncManagedClient { client: Shared>>, + startup_snapshot: Option>, + startup_complete: Arc, } impl AsyncManagedClient { @@ -289,33 +400,63 @@ impl AsyncManagedClient { cancel_token: CancellationToken, tx_event: Sender, elicitation_requests: ElicitationRequestManager, + codex_apps_tools_cache_context: Option, ) -> Self { let tool_filter = ToolFilter::from_config(&config); + let startup_snapshot = load_startup_cached_codex_apps_tools_snapshot( + &server_name, + codex_apps_tools_cache_context.as_ref(), + ) + .map(|tools| filter_tools(tools, &tool_filter)); + let startup_tool_filter = tool_filter; + let startup_complete = Arc::new(AtomicBool::new(false)); + let startup_complete_for_fut = Arc::clone(&startup_complete); let fut = async move { - if let Err(error) = validate_mcp_server_name(&server_name) { - return Err(error.into()); - } + let outcome = async { + if let Err(error) = validate_mcp_server_name(&server_name) { + return Err(error.into()); + } - let client = - Arc::new(make_rmcp_client(&server_name, config.transport, store_mode).await?); - match start_server_task( - server_name, - client, - config.startup_timeout_sec.or(Some(DEFAULT_STARTUP_TIMEOUT)), - config.tool_timeout_sec.unwrap_or(DEFAULT_TOOL_TIMEOUT), - tool_filter, - tx_event, - elicitation_requests, - ) - .or_cancel(&cancel_token) - .await - { - Ok(result) => result, - Err(CancelErr::Cancelled) => Err(StartupOutcomeError::Cancelled), + let client = + Arc::new(make_rmcp_client(&server_name, config.transport, store_mode).await?); + match start_server_task( + server_name, + client, + StartServerTaskParams { + startup_timeout: config + .startup_timeout_sec + .or(Some(DEFAULT_STARTUP_TIMEOUT)), + tool_timeout: config.tool_timeout_sec.unwrap_or(DEFAULT_TOOL_TIMEOUT), + tool_filter: startup_tool_filter, + tx_event, + elicitation_requests, + codex_apps_tools_cache_context, + }, + ) + .or_cancel(&cancel_token) + .await + { + Ok(result) => result, + Err(CancelErr::Cancelled) => Err(StartupOutcomeError::Cancelled), + } } + .await; + + startup_complete_for_fut.store(true, Ordering::Release); + outcome }; + let client = fut.boxed().shared(); + if startup_snapshot.is_some() { + let startup_task = client.clone(); + tokio::spawn(async move { + let _ = startup_task.await; + }); + } + Self { - client: fut.boxed().shared(), + client, + startup_snapshot, + startup_complete, } } @@ -323,6 +464,24 @@ impl AsyncManagedClient { self.client.clone().await } + fn startup_snapshot_while_initializing(&self) -> Option> { + if !self.startup_complete.load(Ordering::Acquire) { + return self.startup_snapshot.clone(); + } + None + } + + async fn listed_tools(&self) -> Option> { + if let Some(startup_tools) = self.startup_snapshot_while_initializing() { + return Some(startup_tools); + } + + match self.client().await { + Ok(client) => Some(client.listed_tools()), + Err(_) => self.startup_snapshot.clone(), + } + } + async fn notify_sandbox_state_change(&self, sandbox_state: &SandboxState) -> Result<()> { let managed = self.client().await?; managed.notify_sandbox_state_change(sandbox_state).await @@ -346,32 +505,51 @@ pub struct SandboxState { } /// A thin wrapper around a set of running [`RmcpClient`] instances. -#[derive(Default)] pub(crate) struct McpConnectionManager { clients: HashMap, elicitation_requests: ElicitationRequestManager, } impl McpConnectionManager { + pub(crate) fn new_uninitialized(approval_policy: &Constrained) -> Self { + Self { + clients: HashMap::new(), + elicitation_requests: ElicitationRequestManager::new(approval_policy.value()), + } + } + + #[cfg(test)] + pub(crate) fn new_mcp_connection_manager_for_tests( + approval_policy: &Constrained, + ) -> Self { + Self::new_uninitialized(approval_policy) + } + pub(crate) fn has_servers(&self) -> bool { !self.clients.is_empty() } - pub async fn initialize( - &mut self, + pub fn set_approval_policy(&self, approval_policy: &Constrained) { + if let Ok(mut policy) = self.elicitation_requests.approval_policy.lock() { + *policy = approval_policy.value(); + } + } + + #[allow(clippy::new_ret_no_self, clippy::too_many_arguments)] + pub async fn new( mcp_servers: &HashMap, store_mode: OAuthCredentialsStoreMode, auth_entries: HashMap, + approval_policy: &Constrained, tx_event: Sender, - cancel_token: CancellationToken, initial_sandbox_state: SandboxState, - ) { - if cancel_token.is_cancelled() { - return; - } + codex_home: PathBuf, + codex_apps_tools_cache_key: CodexAppsToolsCacheKey, + ) -> (Self, CancellationToken) { + let cancel_token = CancellationToken::new(); let mut clients = HashMap::new(); let mut join_set = JoinSet::new(); - let elicitation_requests = ElicitationRequestManager::default(); + let elicitation_requests = ElicitationRequestManager::new(approval_policy.value()); let mcp_servers = mcp_servers.clone(); for (server_name, cfg) in mcp_servers.into_iter().filter(|(_, cfg)| cfg.enabled) { let cancel_token = cancel_token.child_token(); @@ -383,6 +561,14 @@ impl McpConnectionManager { }, ) .await; + let codex_apps_tools_cache_context = if server_name == CODEX_APPS_MCP_SERVER_NAME { + Some(CodexAppsToolsCacheContext { + codex_home: codex_home.clone(), + user_key: codex_apps_tools_cache_key.clone(), + }) + } else { + None + }; let async_managed_client = AsyncManagedClient::new( server_name.clone(), cfg, @@ -390,6 +576,7 @@ impl McpConnectionManager { cancel_token.clone(), tx_event.clone(), elicitation_requests.clone(), + codex_apps_tools_cache_context, ); clients.insert(server_name.clone(), async_managed_client.clone()); let tx_event = tx_event.clone(); @@ -435,8 +622,10 @@ impl McpConnectionManager { (server_name, outcome) }); } - self.clients = clients; - self.elicitation_requests = elicitation_requests.clone(); + let manager = Self { + clients, + elicitation_requests: elicitation_requests.clone(), + }; tokio::spawn(async move { let outcomes = join_set.join_all().await; let mut summary = McpStartupCompleteEvent::default(); @@ -459,6 +648,7 @@ impl McpConnectionManager { }) .await; }); + (manager, cancel_token) } async fn client_by_name(&self, name: &str) -> Result { @@ -522,29 +712,11 @@ impl McpConnectionManager { #[instrument(level = "trace", skip_all)] pub async fn list_all_tools(&self) -> HashMap { let mut tools = HashMap::new(); - for (server_name, managed_client) in &self.clients { - let client = managed_client.client().await.ok(); - if let Some(client) = client { - let rmcp_client = client.client; - let tool_timeout = client.tool_timeout; - let tool_filter = client.tool_filter; - let mut server_tools = client.tools; - - if server_name == CODEX_APPS_MCP_SERVER_NAME { - match list_tools_for_client(server_name, &rmcp_client, tool_timeout).await { - Ok(fresh_or_cached_tools) => { - server_tools = fresh_or_cached_tools; - } - Err(err) => { - warn!( - "Failed to refresh tools for MCP server '{server_name}', using startup snapshot: {err:#}" - ); - } - } - } - - tools.extend(qualify_tools(filter_tools(server_tools, tool_filter))); - } + for managed_client in self.clients.values() { + let Some(server_tools) = managed_client.listed_tools().await else { + continue; + }; + tools.extend(qualify_tools(server_tools)); } tools } @@ -562,6 +734,8 @@ impl McpConnectionManager { .await .context("failed to get client")?; + let list_start = Instant::now(); + let fetch_start = Instant::now(); let tools = list_tools_for_client_uncached( CODEX_APPS_MCP_SERVER_NAME, &managed_client.client, @@ -571,8 +745,22 @@ impl McpConnectionManager { .with_context(|| { format!("failed to refresh tools for MCP server '{CODEX_APPS_MCP_SERVER_NAME}'") })?; + emit_duration( + MCP_TOOLS_FETCH_UNCACHED_DURATION_METRIC, + fetch_start.elapsed(), + &[], + ); - write_cached_codex_apps_tools(&tools); + write_cached_codex_apps_tools_if_needed( + CODEX_APPS_MCP_SERVER_NAME, + managed_client.codex_apps_tools_cache_context.as_ref(), + &tools, + ); + emit_duration( + MCP_TOOLS_LIST_DURATION_METRIC, + list_start.elapsed(), + &[("cache", "miss")], + ); Ok(()) } @@ -881,7 +1069,7 @@ impl ToolFilter { } } -fn filter_tools(tools: Vec, filter: ToolFilter) -> Vec { +fn filter_tools(tools: Vec, filter: &ToolFilter) -> Vec { tools .into_iter() .filter(|tool| filter.allows(&tool.tool_name)) @@ -1024,14 +1212,17 @@ fn elicitation_capability_for_server(server_name: &str) -> Option, - startup_timeout: Option, // TODO: cancel_token should handle this. - tool_timeout: Duration, - tool_filter: ToolFilter, - tx_event: Sender, - elicitation_requests: ElicitationRequestManager, + params: StartServerTaskParams, ) -> Result { + let StartServerTaskParams { + startup_timeout, + tool_timeout, + tool_filter, + tx_event, + elicitation_requests, + codex_apps_tools_cache_context, + } = params; let elicitation = elicitation_capability_for_server(&server_name); - let params = InitializeRequestParams { meta: None, capabilities: ClientCapabilities { @@ -1060,9 +1251,29 @@ async fn start_server_task( .await .map_err(StartupOutcomeError::from)?; - let tools = list_tools_for_client(&server_name, &client, startup_timeout) + let list_start = Instant::now(); + let fetch_start = Instant::now(); + let tools = list_tools_for_client_uncached(&server_name, &client, startup_timeout) .await .map_err(StartupOutcomeError::from)?; + emit_duration( + MCP_TOOLS_FETCH_UNCACHED_DURATION_METRIC, + fetch_start.elapsed(), + &[], + ); + write_cached_codex_apps_tools_if_needed( + &server_name, + codex_apps_tools_cache_context.as_ref(), + &tools, + ); + if server_name == CODEX_APPS_MCP_SERVER_NAME { + emit_duration( + MCP_TOOLS_LIST_DURATION_METRIC, + list_start.elapsed(), + &[("cache", "miss")], + ); + } + let tools = filter_tools(tools, &tool_filter); let server_supports_sandbox_state_capability = initialize_result .capabilities @@ -1077,11 +1288,21 @@ async fn start_server_task( tool_timeout: Some(tool_timeout), tool_filter, server_supports_sandbox_state_capability, + codex_apps_tools_cache_context, }; Ok(managed) } +struct StartServerTaskParams { + startup_timeout: Option, // TODO: cancel_token should handle this. + tool_timeout: Duration, + tool_filter: ToolFilter, + tx_event: Sender, + elicitation_requests: ElicitationRequestManager, + codex_apps_tools_cache_context: Option, +} + async fn make_rmcp_client( server_name: &str, transport: McpServerTransportConfig, @@ -1126,80 +1347,99 @@ async fn make_rmcp_client( } } -async fn list_tools_for_client( +fn write_cached_codex_apps_tools_if_needed( server_name: &str, - client: &Arc, - timeout: Option, -) -> Result> { - let total_start = Instant::now(); - if server_name == CODEX_APPS_MCP_SERVER_NAME - && let Some(cached_tools) = read_cached_codex_apps_tools() - { - emit_duration( - MCP_TOOLS_LIST_DURATION_METRIC, - total_start.elapsed(), - &[("cache", "hit")], - ); - return Ok(cached_tools); + cache_context: Option<&CodexAppsToolsCacheContext>, + tools: &[ToolInfo], +) { + if server_name != CODEX_APPS_MCP_SERVER_NAME { + return; } - let fetch_start = Instant::now(); - let tools = list_tools_for_client_uncached(server_name, client, timeout).await?; - emit_duration( - MCP_TOOLS_FETCH_UNCACHED_DURATION_METRIC, - fetch_start.elapsed(), - &[], - ); - - if server_name == CODEX_APPS_MCP_SERVER_NAME { + if let Some(cache_context) = cache_context { let cache_write_start = Instant::now(); - write_cached_codex_apps_tools(&tools); + write_cached_codex_apps_tools(cache_context, tools); emit_duration( MCP_TOOLS_CACHE_WRITE_DURATION_METRIC, cache_write_start.elapsed(), &[], ); } - - if server_name == CODEX_APPS_MCP_SERVER_NAME { - emit_duration( - MCP_TOOLS_LIST_DURATION_METRIC, - total_start.elapsed(), - &[("cache", "miss")], - ); - } - Ok(tools) } -fn read_cached_codex_apps_tools() -> Option> { - let mut cache_guard = CODEX_APPS_TOOLS_CACHE - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - let now = Instant::now(); - - if let Some(cached) = cache_guard.as_ref() - && now < cached.expires_at - { - return Some(cached.tools.clone()); +fn load_startup_cached_codex_apps_tools_snapshot( + server_name: &str, + cache_context: Option<&CodexAppsToolsCacheContext>, +) -> Option> { + if server_name != CODEX_APPS_MCP_SERVER_NAME { + return None; } - if cache_guard - .as_ref() - .is_some_and(|cached| now >= cached.expires_at) - { - *cache_guard = None; + let cache_context = cache_context?; + + match load_cached_codex_apps_tools(cache_context) { + CachedCodexAppsToolsLoad::Hit(tools) => Some(tools), + CachedCodexAppsToolsLoad::Missing | CachedCodexAppsToolsLoad::Invalid => None, } - None } -fn write_cached_codex_apps_tools(tools: &[ToolInfo]) { - let mut cache_guard = CODEX_APPS_TOOLS_CACHE - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - *cache_guard = Some(CachedCodexAppsTools { - expires_at: Instant::now() + CODEX_APPS_TOOLS_CACHE_TTL, - tools: tools.to_vec(), - }); +#[cfg(test)] +fn read_cached_codex_apps_tools( + cache_context: &CodexAppsToolsCacheContext, +) -> Option> { + match load_cached_codex_apps_tools(cache_context) { + CachedCodexAppsToolsLoad::Hit(tools) => Some(tools), + CachedCodexAppsToolsLoad::Missing | CachedCodexAppsToolsLoad::Invalid => None, + } +} + +fn load_cached_codex_apps_tools( + cache_context: &CodexAppsToolsCacheContext, +) -> CachedCodexAppsToolsLoad { + let cache_path = cache_context.cache_path(); + let bytes = match std::fs::read(cache_path) { + Ok(bytes) => bytes, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + return CachedCodexAppsToolsLoad::Missing; + } + Err(_) => return CachedCodexAppsToolsLoad::Invalid, + }; + let cache: CodexAppsToolsDiskCache = match serde_json::from_slice(&bytes) { + Ok(cache) => cache, + Err(_) => return CachedCodexAppsToolsLoad::Invalid, + }; + if cache.schema_version != CODEX_APPS_TOOLS_CACHE_SCHEMA_VERSION { + return CachedCodexAppsToolsLoad::Invalid; + } + CachedCodexAppsToolsLoad::Hit(filter_disallowed_codex_apps_tools(cache.tools)) +} + +fn write_cached_codex_apps_tools(cache_context: &CodexAppsToolsCacheContext, tools: &[ToolInfo]) { + let cache_path = cache_context.cache_path(); + if let Some(parent) = cache_path.parent() + && std::fs::create_dir_all(parent).is_err() + { + return; + } + let tools = filter_disallowed_codex_apps_tools(tools.to_vec()); + let Ok(bytes) = serde_json::to_vec_pretty(&CodexAppsToolsDiskCache { + schema_version: CODEX_APPS_TOOLS_CACHE_SCHEMA_VERSION, + tools, + }) else { + return; + }; + let _ = std::fs::write(cache_path, bytes); +} + +fn filter_disallowed_codex_apps_tools(tools: Vec) -> Vec { + tools + .into_iter() + .filter(|tool| { + tool.connector_id + .as_deref() + .is_none_or(is_connector_id_allowed) + }) + .collect() } fn emit_duration(metric: &str, duration: Duration, tags: &[(&str, &str)]) { @@ -1214,7 +1454,7 @@ async fn list_tools_for_client_uncached( timeout: Option, ) -> Result> { let resp = client.list_tools_with_connector_ids(None, timeout).await?; - Ok(resp + let tools = resp .tools .into_iter() .map(|tool| { @@ -1235,7 +1475,11 @@ async fn list_tools_for_client_uncached( connector_name, } }) - .collect()) + .collect(); + if server_name == CODEX_APPS_MCP_SERVER_NAME { + return Ok(filter_disallowed_codex_apps_tools(tools)); + } + Ok(tools) } fn validate_mcp_server_name(server_name: &str) -> Result<()> { @@ -1319,9 +1563,11 @@ mod mcp_init_error_display_tests {} mod tests { use super::*; use codex_protocol::protocol::McpAuthStatus; + use codex_protocol::protocol::RejectConfig; use rmcp::model::JsonObject; use std::collections::HashSet; use std::sync::Arc; + use tempfile::tempdir; fn create_test_tool(server_name: &str, tool_name: &str) -> ToolInfo { ToolInfo { @@ -1343,19 +1589,63 @@ mod tests { } } - fn with_clean_codex_apps_tools_cache(f: impl FnOnce() -> T) -> T { - let previous_cache = { - let mut cache_guard = CODEX_APPS_TOOLS_CACHE - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - cache_guard.take() - }; - let result = f(); - let mut cache_guard = CODEX_APPS_TOOLS_CACHE - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - *cache_guard = previous_cache; - result + fn create_test_tool_with_connector( + server_name: &str, + tool_name: &str, + connector_id: &str, + connector_name: Option<&str>, + ) -> ToolInfo { + let mut tool = create_test_tool(server_name, tool_name); + tool.connector_id = Some(connector_id.to_string()); + tool.connector_name = connector_name.map(ToOwned::to_owned); + tool + } + + fn create_codex_apps_tools_cache_context( + codex_home: PathBuf, + account_id: Option<&str>, + chatgpt_user_id: Option<&str>, + ) -> CodexAppsToolsCacheContext { + CodexAppsToolsCacheContext { + codex_home, + user_key: CodexAppsToolsCacheKey { + account_id: account_id.map(ToOwned::to_owned), + chatgpt_user_id: chatgpt_user_id.map(ToOwned::to_owned), + is_workspace_account: false, + }, + } + } + + #[test] + fn elicitation_reject_policy_defaults_to_prompting() { + assert!(!elicitation_is_rejected_by_policy( + AskForApproval::OnFailure + )); + assert!(!elicitation_is_rejected_by_policy( + AskForApproval::OnRequest + )); + assert!(!elicitation_is_rejected_by_policy( + AskForApproval::UnlessTrusted + )); + assert!(!elicitation_is_rejected_by_policy(AskForApproval::Reject( + RejectConfig { + sandbox_approval: false, + rules: false, + mcp_elicitations: false, + } + ))); + } + + #[test] + fn elicitation_reject_policy_respects_never_and_reject_config() { + assert!(elicitation_is_rejected_by_policy(AskForApproval::Never)); + assert!(elicitation_is_rejected_by_policy(AskForApproval::Reject( + RejectConfig { + sandbox_approval: false, + rules: false, + mcp_elicitations: true, + } + ))); } #[test] @@ -1500,9 +1790,9 @@ mod tests { disabled: HashSet::from(["tool_a".to_string()]), }; - let filtered: Vec<_> = filter_tools(server1_tools, server1_filter) + let filtered: Vec<_> = filter_tools(server1_tools, &server1_filter) .into_iter() - .chain(filter_tools(server2_tools, server2_filter)) + .chain(filter_tools(server2_tools, &server2_filter)) .collect(); assert_eq!(filtered.len(), 1); @@ -1512,43 +1802,260 @@ mod tests { #[test] fn codex_apps_tools_cache_is_overwritten_by_last_write() { - with_clean_codex_apps_tools_cache(|| { - let tools_gateway_1 = vec![create_test_tool(CODEX_APPS_MCP_SERVER_NAME, "one")]; - let tools_gateway_2 = vec![create_test_tool(CODEX_APPS_MCP_SERVER_NAME, "two")]; + let codex_home = tempdir().expect("tempdir"); + let cache_context = create_codex_apps_tools_cache_context( + codex_home.path().to_path_buf(), + Some("account-one"), + Some("user-one"), + ); + let tools_gateway_1 = vec![create_test_tool(CODEX_APPS_MCP_SERVER_NAME, "one")]; + let tools_gateway_2 = vec![create_test_tool(CODEX_APPS_MCP_SERVER_NAME, "two")]; - write_cached_codex_apps_tools(&tools_gateway_1); - let cached_gateway_1 = - read_cached_codex_apps_tools().expect("cache entry exists for first write"); - assert_eq!(cached_gateway_1[0].tool_name, "one"); + write_cached_codex_apps_tools(&cache_context, &tools_gateway_1); + let cached_gateway_1 = read_cached_codex_apps_tools(&cache_context) + .expect("cache entry exists for first write"); + assert_eq!(cached_gateway_1[0].tool_name, "one"); - write_cached_codex_apps_tools(&tools_gateway_2); - let cached_gateway_2 = - read_cached_codex_apps_tools().expect("cache entry exists for second write"); - assert_eq!(cached_gateway_2[0].tool_name, "two"); - }); + write_cached_codex_apps_tools(&cache_context, &tools_gateway_2); + let cached_gateway_2 = read_cached_codex_apps_tools(&cache_context) + .expect("cache entry exists for second write"); + assert_eq!(cached_gateway_2[0].tool_name, "two"); } #[test] - fn codex_apps_tools_cache_is_cleared_when_expired() { - with_clean_codex_apps_tools_cache(|| { - let tools = vec![create_test_tool(CODEX_APPS_MCP_SERVER_NAME, "stale_tool")]; - write_cached_codex_apps_tools(&tools); + fn codex_apps_tools_cache_is_scoped_per_user() { + let codex_home = tempdir().expect("tempdir"); + let cache_context_user_1 = create_codex_apps_tools_cache_context( + codex_home.path().to_path_buf(), + Some("account-one"), + Some("user-one"), + ); + let cache_context_user_2 = create_codex_apps_tools_cache_context( + codex_home.path().to_path_buf(), + Some("account-two"), + Some("user-two"), + ); + let tools_user_1 = vec![create_test_tool(CODEX_APPS_MCP_SERVER_NAME, "one")]; + let tools_user_2 = vec![create_test_tool(CODEX_APPS_MCP_SERVER_NAME, "two")]; - { - let mut cache_guard = CODEX_APPS_TOOLS_CACHE - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - cache_guard.as_mut().expect("cache exists").expires_at = - Instant::now() - Duration::from_secs(1); - } + write_cached_codex_apps_tools(&cache_context_user_1, &tools_user_1); + write_cached_codex_apps_tools(&cache_context_user_2, &tools_user_2); - assert!(read_cached_codex_apps_tools().is_none()); + let read_user_1 = + read_cached_codex_apps_tools(&cache_context_user_1).expect("cache entry for user one"); + let read_user_2 = + read_cached_codex_apps_tools(&cache_context_user_2).expect("cache entry for user two"); - let cache_guard = CODEX_APPS_TOOLS_CACHE - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - assert!(cache_guard.is_none()); - }); + assert_eq!(read_user_1[0].tool_name, "one"); + assert_eq!(read_user_2[0].tool_name, "two"); + assert_ne!( + cache_context_user_1.cache_path(), + cache_context_user_2.cache_path(), + "each user should get an isolated cache file" + ); + } + + #[test] + fn codex_apps_tools_cache_filters_disallowed_connectors() { + let codex_home = tempdir().expect("tempdir"); + let cache_context = create_codex_apps_tools_cache_context( + codex_home.path().to_path_buf(), + Some("account-one"), + Some("user-one"), + ); + let tools = vec![ + create_test_tool_with_connector( + CODEX_APPS_MCP_SERVER_NAME, + "blocked_tool", + "connector_openai_hidden", + Some("Hidden"), + ), + create_test_tool_with_connector( + CODEX_APPS_MCP_SERVER_NAME, + "allowed_tool", + "calendar", + Some("Calendar"), + ), + ]; + + write_cached_codex_apps_tools(&cache_context, &tools); + let cached = + read_cached_codex_apps_tools(&cache_context).expect("cache entry exists for user"); + + assert_eq!(cached.len(), 1); + assert_eq!(cached[0].tool_name, "allowed_tool"); + assert_eq!(cached[0].connector_id.as_deref(), Some("calendar")); + } + + #[test] + fn codex_apps_tools_cache_is_ignored_when_schema_version_mismatches() { + let codex_home = tempdir().expect("tempdir"); + let cache_context = create_codex_apps_tools_cache_context( + codex_home.path().to_path_buf(), + Some("account-one"), + Some("user-one"), + ); + let cache_path = cache_context.cache_path(); + if let Some(parent) = cache_path.parent() { + std::fs::create_dir_all(parent).expect("create parent"); + } + let bytes = serde_json::to_vec_pretty(&serde_json::json!({ + "schema_version": CODEX_APPS_TOOLS_CACHE_SCHEMA_VERSION + 1, + "tools": [create_test_tool(CODEX_APPS_MCP_SERVER_NAME, "one")], + })) + .expect("serialize"); + std::fs::write(cache_path, bytes).expect("write"); + + assert!(read_cached_codex_apps_tools(&cache_context).is_none()); + } + + #[test] + fn codex_apps_tools_cache_is_ignored_when_json_is_invalid() { + let codex_home = tempdir().expect("tempdir"); + let cache_context = create_codex_apps_tools_cache_context( + codex_home.path().to_path_buf(), + Some("account-one"), + Some("user-one"), + ); + let cache_path = cache_context.cache_path(); + if let Some(parent) = cache_path.parent() { + std::fs::create_dir_all(parent).expect("create parent"); + } + std::fs::write(cache_path, b"{not json").expect("write"); + + assert!(read_cached_codex_apps_tools(&cache_context).is_none()); + } + + #[test] + fn startup_cached_codex_apps_tools_loads_from_disk_cache() { + let codex_home = tempdir().expect("tempdir"); + let cache_context = create_codex_apps_tools_cache_context( + codex_home.path().to_path_buf(), + Some("account-one"), + Some("user-one"), + ); + let cached_tools = vec![create_test_tool( + CODEX_APPS_MCP_SERVER_NAME, + "calendar_search", + )]; + write_cached_codex_apps_tools(&cache_context, &cached_tools); + + let startup_snapshot = load_startup_cached_codex_apps_tools_snapshot( + CODEX_APPS_MCP_SERVER_NAME, + Some(&cache_context), + ); + let startup_tools = startup_snapshot.expect("expected startup snapshot to load from cache"); + + assert_eq!(startup_tools.len(), 1); + assert_eq!(startup_tools[0].server_name, CODEX_APPS_MCP_SERVER_NAME); + assert_eq!(startup_tools[0].tool_name, "calendar_search"); + } + + #[tokio::test] + async fn list_all_tools_uses_startup_snapshot_while_client_is_pending() { + let startup_tools = vec![create_test_tool( + CODEX_APPS_MCP_SERVER_NAME, + "calendar_create_event", + )]; + let pending_client = + futures::future::pending::>() + .boxed() + .shared(); + let approval_policy = Constrained::allow_any(AskForApproval::OnFailure); + let mut manager = McpConnectionManager::new_uninitialized(&approval_policy); + manager.clients.insert( + CODEX_APPS_MCP_SERVER_NAME.to_string(), + AsyncManagedClient { + client: pending_client, + startup_snapshot: Some(startup_tools), + startup_complete: Arc::new(std::sync::atomic::AtomicBool::new(false)), + }, + ); + + let tools = manager.list_all_tools().await; + let tool = tools + .get("mcp__codex_apps__calendar_create_event") + .expect("tool from startup cache"); + assert_eq!(tool.server_name, CODEX_APPS_MCP_SERVER_NAME); + assert_eq!(tool.tool_name, "calendar_create_event"); + } + + #[tokio::test] + async fn list_all_tools_blocks_while_client_is_pending_without_startup_snapshot() { + let pending_client = + futures::future::pending::>() + .boxed() + .shared(); + let approval_policy = Constrained::allow_any(AskForApproval::OnFailure); + let mut manager = McpConnectionManager::new_uninitialized(&approval_policy); + manager.clients.insert( + CODEX_APPS_MCP_SERVER_NAME.to_string(), + AsyncManagedClient { + client: pending_client, + startup_snapshot: None, + startup_complete: Arc::new(std::sync::atomic::AtomicBool::new(false)), + }, + ); + + let timeout_result = + tokio::time::timeout(Duration::from_millis(10), manager.list_all_tools()).await; + assert!(timeout_result.is_err()); + } + + #[tokio::test] + async fn list_all_tools_does_not_block_when_startup_snapshot_cache_hit_is_empty() { + let pending_client = + futures::future::pending::>() + .boxed() + .shared(); + let approval_policy = Constrained::allow_any(AskForApproval::OnFailure); + let mut manager = McpConnectionManager::new_uninitialized(&approval_policy); + manager.clients.insert( + CODEX_APPS_MCP_SERVER_NAME.to_string(), + AsyncManagedClient { + client: pending_client, + startup_snapshot: Some(Vec::new()), + startup_complete: Arc::new(std::sync::atomic::AtomicBool::new(false)), + }, + ); + + let timeout_result = + tokio::time::timeout(Duration::from_millis(10), manager.list_all_tools()).await; + let tools = timeout_result.expect("cache-hit startup snapshot should not block"); + assert!(tools.is_empty()); + } + + #[tokio::test] + async fn list_all_tools_uses_startup_snapshot_when_client_startup_fails() { + let startup_tools = vec![create_test_tool( + CODEX_APPS_MCP_SERVER_NAME, + "calendar_create_event", + )]; + let failed_client = futures::future::ready::>( + Err(StartupOutcomeError::Failed { + error: "startup failed".to_string(), + }), + ) + .boxed() + .shared(); + let approval_policy = Constrained::allow_any(AskForApproval::OnFailure); + let mut manager = McpConnectionManager::new_uninitialized(&approval_policy); + let startup_complete = Arc::new(std::sync::atomic::AtomicBool::new(true)); + manager.clients.insert( + CODEX_APPS_MCP_SERVER_NAME.to_string(), + AsyncManagedClient { + client: failed_client, + startup_snapshot: Some(startup_tools), + startup_complete, + }, + ); + + let tools = manager.list_all_tools().await; + let tool = tools + .get("mcp__codex_apps__calendar_create_event") + .expect("tool from startup cache"); + assert_eq!(tool.server_name, CODEX_APPS_MCP_SERVER_NAME); + assert_eq!(tool.tool_name, "calendar_create_event"); } #[test] diff --git a/codex-rs/core/src/memories/prompts.rs b/codex-rs/core/src/memories/prompts.rs index 9b341c1ffe..9a3ea6a92d 100644 --- a/codex-rs/core/src/memories/prompts.rs +++ b/codex-rs/core/src/memories/prompts.rs @@ -2,43 +2,18 @@ use crate::memories::memory_root; use crate::memories::phase_one; use crate::truncate::TruncationPolicy; use crate::truncate::truncate_text; -use askama::Template; use codex_protocol::openai_models::ModelInfo; use std::path::Path; use tokio::fs; -use tracing::warn; -#[derive(Template)] -#[template(path = "memories/consolidation.md", escape = "none")] -struct ConsolidationPromptTemplate<'a> { - memory_root: &'a str, -} - -#[derive(Template)] -#[template(path = "memories/stage_one_input.md", escape = "none")] -struct StageOneInputTemplate<'a> { - rollout_path: &'a str, - rollout_cwd: &'a str, - rollout_contents: &'a str, -} - -#[derive(Template)] -#[template(path = "memories/read_path.md", escape = "none")] -struct MemoryToolDeveloperInstructionsTemplate<'a> { - base_path: &'a str, - memory_summary: &'a str, -} +const CONSOLIDATION_TEMPLATE: &str = include_str!("../../templates/memories/consolidation.md"); +const STAGE_ONE_INPUT_TEMPLATE: &str = include_str!("../../templates/memories/stage_one_input.md"); +const READ_PATH_TEMPLATE: &str = include_str!("../../templates/memories/read_path.md"); /// Builds the consolidation subagent prompt for a specific memory root. pub(super) fn build_consolidation_prompt(memory_root: &Path) -> String { let memory_root = memory_root.display().to_string(); - let template = ConsolidationPromptTemplate { - memory_root: &memory_root, - }; - template.render().unwrap_or_else(|err| { - warn!("failed to render memories consolidation prompt template: {err}"); - format!("## Memory Phase 2 (Consolidation)\nConsolidate Codex memories in: {memory_root}") - }) + CONSOLIDATION_TEMPLATE.replace("{{ memory_root }}", &memory_root) } /// Builds the stage-1 user message containing rollout metadata and content. @@ -65,12 +40,10 @@ pub(super) fn build_stage_one_input_message( let rollout_path = rollout_path.display().to_string(); let rollout_cwd = rollout_cwd.display().to_string(); - Ok(StageOneInputTemplate { - rollout_path: &rollout_path, - rollout_cwd: &rollout_cwd, - rollout_contents: &truncated_rollout_contents, - } - .render()?) + Ok(STAGE_ONE_INPUT_TEMPLATE + .replace("{{ rollout_path }}", &rollout_path) + .replace("{{ rollout_cwd }}", &rollout_cwd) + .replace("{{ rollout_contents }}", &truncated_rollout_contents)) } /// Build prompt used for read path. This prompt must be added to the developer instructions. In @@ -92,11 +65,11 @@ pub(crate) async fn build_memory_tool_developer_instructions(codex_home: &Path) return None; } let base_path = base_path.display().to_string(); - let template = MemoryToolDeveloperInstructionsTemplate { - base_path: &base_path, - memory_summary: &memory_summary, - }; - template.render().ok() + Some( + READ_PATH_TEMPLATE + .replace("{{ base_path }}", &base_path) + .replace("{{ memory_summary }}", &memory_summary), + ) } #[cfg(test)] diff --git a/codex-rs/core/src/network_proxy_loader.rs b/codex-rs/core/src/network_proxy_loader.rs index 3e06452740..6fcf91f086 100644 --- a/codex-rs/core/src/network_proxy_loader.rs +++ b/codex-rs/core/src/network_proxy_loader.rs @@ -1,4 +1,6 @@ use crate::config::CONFIG_TOML_FILE; +use crate::config::NetworkToml; +use crate::config::PermissionsToml; use crate::config::find_codex_home; use crate::config_loader::CloudRequirementsLoader; use crate::config_loader::ConfigLayerStack; @@ -15,9 +17,9 @@ use codex_network_proxy::NetworkProxyConfig; use codex_network_proxy::NetworkProxyConstraintError; use codex_network_proxy::NetworkProxyConstraints; use codex_network_proxy::NetworkProxyState; -use codex_network_proxy::PartialNetworkProxyConfig; use codex_network_proxy::build_config_state; use codex_network_proxy::validate_policy_against_constraints; +use serde::Deserialize; use std::path::PathBuf; use std::sync::Arc; use tokio::sync::RwLock; @@ -47,10 +49,7 @@ async fn build_config_state_with_mtimes() -> Result<(ConfigState, Vec, + permissions: Option, +} + +fn network_tables_from_toml(value: &toml::Value) -> Result { + value + .clone() + .try_into() + .context("failed to deserialize network tables from config") +} + +fn apply_network_tables(config: &mut NetworkProxyConfig, parsed: NetworkTablesToml) { + if let Some(network) = parsed.network { + network.apply_to_network_proxy_config(config); + } + if let Some(network) = parsed + .permissions + .and_then(|permissions| permissions.network) + { + network.apply_to_network_proxy_config(config); + } +} + +fn config_from_layers(layers: &ConfigLayerStack) -> Result { + let mut config = NetworkProxyConfig::default(); + for layer in layers.get_layers(ConfigLayerStackOrdering::LowestPrecedenceFirst, false) { + let parsed = network_tables_from_toml(&layer.config)?; + apply_network_tables(&mut config, parsed); + } + Ok(config) +} + fn is_user_controlled_layer(layer: &ConfigLayerSource) -> bool { matches!( layer, @@ -215,3 +252,40 @@ impl ConfigReloader for MtimeConfigReloader { Ok(state) } } + +#[cfg(test)] +mod tests { + use super::*; + + use pretty_assertions::assert_eq; + + #[test] + fn higher_precedence_network_table_beats_lower_permissions_network_table() { + let lower_permissions: toml::Value = toml::from_str( + r#" +[permissions.network] +allowed_domains = ["lower.example.com"] +"#, + ) + .expect("lower layer should parse"); + let higher_network: toml::Value = toml::from_str( + r#" +[network] +allowed_domains = ["higher.example.com"] +"#, + ) + .expect("higher layer should parse"); + + let mut config = NetworkProxyConfig::default(); + apply_network_tables( + &mut config, + network_tables_from_toml(&lower_permissions).expect("lower layer should deserialize"), + ); + apply_network_tables( + &mut config, + network_tables_from_toml(&higher_network).expect("higher layer should deserialize"), + ); + + assert_eq!(config.network.allowed_domains, vec!["higher.example.com"]); + } +} diff --git a/codex-rs/core/src/safety.rs b/codex-rs/core/src/safety.rs index b0d359b7e1..350e7dad0f 100644 --- a/codex-rs/core/src/safety.rs +++ b/codex-rs/core/src/safety.rs @@ -38,7 +38,10 @@ pub fn assess_patch_safety( } match policy { - AskForApproval::OnFailure | AskForApproval::Never | AskForApproval::OnRequest => { + AskForApproval::OnFailure + | AskForApproval::Never + | AskForApproval::OnRequest + | AskForApproval::Reject(_) => { // Continue to see if this can be auto-approved. } // TODO(ragona): I'm not sure this is actually correct? I believe in this case @@ -48,11 +51,17 @@ pub fn assess_patch_safety( } } + let rejects_sandbox_approval = matches!(policy, AskForApproval::Never) + || matches!( + policy, + AskForApproval::Reject(reject_config) if reject_config.sandbox_approval + ); + // Even though the patch appears to be constrained to writable paths, it is // possible that paths in the patch are hard links to files outside the // writable roots, so we should still run `apply_patch` in a sandbox in that case. if is_write_patch_constrained_to_writable_paths(action, sandbox_policy, cwd) - || policy == AskForApproval::OnFailure + || matches!(policy, AskForApproval::OnFailure) { if matches!( sandbox_policy, @@ -72,10 +81,20 @@ pub fn assess_patch_safety( sandbox_type, user_explicitly_approved: false, }, - None => SafetyCheck::AskUser, + None => { + if rejects_sandbox_approval { + SafetyCheck::Reject { + reason: + "writing outside of the project; rejected by user approval settings" + .to_string(), + } + } else { + SafetyCheck::AskUser + } + } } } - } else if policy == AskForApproval::Never { + } else if rejects_sandbox_approval { SafetyCheck::Reject { reason: "writing outside of the project; rejected by user approval settings" .to_string(), @@ -174,6 +193,7 @@ fn is_write_patch_constrained_to_writable_paths( #[cfg(test)] mod tests { use super::*; + use codex_protocol::protocol::RejectConfig; use codex_utils_absolute_path::AbsolutePathBuf; use tempfile::TempDir; @@ -253,4 +273,79 @@ mod tests { } ); } + + #[test] + fn reject_with_all_flags_false_matches_on_request_for_out_of_root_patch() { + let tmp = TempDir::new().unwrap(); + let cwd = tmp.path().to_path_buf(); + let parent = cwd.parent().unwrap().to_path_buf(); + let add_outside = + ApplyPatchAction::new_add_for_test(&parent.join("outside.txt"), "".to_string()); + let policy_workspace_only = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + read_only_access: Default::default(), + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + + assert_eq!( + assess_patch_safety( + &add_outside, + AskForApproval::OnRequest, + &policy_workspace_only, + &cwd, + WindowsSandboxLevel::Disabled, + ), + SafetyCheck::AskUser, + ); + assert_eq!( + assess_patch_safety( + &add_outside, + AskForApproval::Reject(RejectConfig { + sandbox_approval: false, + rules: false, + mcp_elicitations: false, + }), + &policy_workspace_only, + &cwd, + WindowsSandboxLevel::Disabled, + ), + SafetyCheck::AskUser, + ); + } + + #[test] + fn reject_sandbox_approval_rejects_out_of_root_patch() { + let tmp = TempDir::new().unwrap(); + let cwd = tmp.path().to_path_buf(); + let parent = cwd.parent().unwrap().to_path_buf(); + let add_outside = + ApplyPatchAction::new_add_for_test(&parent.join("outside.txt"), "".to_string()); + let policy_workspace_only = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + read_only_access: Default::default(), + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + + assert_eq!( + assess_patch_safety( + &add_outside, + AskForApproval::Reject(RejectConfig { + sandbox_approval: true, + rules: false, + mcp_elicitations: false, + }), + &policy_workspace_only, + &cwd, + WindowsSandboxLevel::Disabled, + ), + SafetyCheck::Reject { + reason: "writing outside of the project; rejected by user approval settings" + .to_string(), + }, + ); + } } diff --git a/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/generate_openai_yaml.py b/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/generate_openai_yaml.py index 1a9d784f87..3fd7405345 100644 --- a/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/generate_openai_yaml.py +++ b/codex-rs/core/src/skills/assets/samples/skill-creator/scripts/generate_openai_yaml.py @@ -11,8 +11,6 @@ import re import sys from pathlib import Path -import yaml - ACRONYMS = { "GH", "MCP", @@ -114,6 +112,9 @@ def read_frontmatter_name(skill_dir): print("[ERROR] Invalid SKILL.md frontmatter format.") return None frontmatter_text = match.group(1) + + import yaml + try: frontmatter = yaml.safe_load(frontmatter_text) except yaml.YAMLError as exc: diff --git a/codex-rs/core/src/tools/runtimes/apply_patch.rs b/codex-rs/core/src/tools/runtimes/apply_patch.rs index d16f9b836b..1cdb30b79a 100644 --- a/codex-rs/core/src/tools/runtimes/apply_patch.rs +++ b/codex-rs/core/src/tools/runtimes/apply_patch.rs @@ -127,7 +127,13 @@ impl Approvable for ApplyPatchRuntime { } fn wants_no_sandbox_approval(&self, policy: AskForApproval) -> bool { - !matches!(policy, AskForApproval::Never) + match policy { + AskForApproval::Never => false, + AskForApproval::Reject(reject_config) => !reject_config.rejects_sandbox_approval(), + AskForApproval::OnFailure => true, + AskForApproval::OnRequest => true, + AskForApproval::UnlessTrusted => true, + } } // apply_patch approvals are decided upstream by assess_patch_safety. @@ -159,3 +165,29 @@ impl ToolRuntime for ApplyPatchRuntime { Ok(out) } } + +#[cfg(test)] +mod tests { + use super::*; + use codex_protocol::protocol::RejectConfig; + + #[test] + fn wants_no_sandbox_approval_reject_respects_sandbox_flag() { + let runtime = ApplyPatchRuntime::new(); + assert!(runtime.wants_no_sandbox_approval(AskForApproval::OnRequest)); + assert!( + !runtime.wants_no_sandbox_approval(AskForApproval::Reject(RejectConfig { + sandbox_approval: true, + rules: false, + mcp_elicitations: false, + })) + ); + assert!( + runtime.wants_no_sandbox_approval(AskForApproval::Reject(RejectConfig { + sandbox_approval: false, + rules: false, + mcp_elicitations: false, + })) + ); + } +} diff --git a/codex-rs/core/src/tools/sandboxing.rs b/codex-rs/core/src/tools/sandboxing.rs index 04dc10c3e1..89b4bc01ec 100644 --- a/codex-rs/core/src/tools/sandboxing.rs +++ b/codex-rs/core/src/tools/sandboxing.rs @@ -156,6 +156,8 @@ impl ExecApprovalRequirement { /// - Never, OnFailure: do not ask /// - OnRequest: ask unless sandbox policy is DangerFullAccess +/// - Reject: ask unless sandbox policy is DangerFullAccess, but auto-reject +/// when `sandbox_approval` rejection is enabled. /// - UnlessTrusted: always ask pub(crate) fn default_exec_approval_requirement( policy: AskForApproval, @@ -163,14 +165,23 @@ pub(crate) fn default_exec_approval_requirement( ) -> ExecApprovalRequirement { let needs_approval = match policy { AskForApproval::Never | AskForApproval::OnFailure => false, - AskForApproval::OnRequest => !matches!( + AskForApproval::OnRequest | AskForApproval::Reject(_) => !matches!( sandbox_policy, SandboxPolicy::DangerFullAccess | SandboxPolicy::ExternalSandbox { .. } ), AskForApproval::UnlessTrusted => true, }; - if needs_approval { + if needs_approval + && matches!( + policy, + AskForApproval::Reject(reject_config) if reject_config.rejects_sandbox_approval() + ) + { + ExecApprovalRequirement::Forbidden { + reason: "approval policy rejected sandbox approval prompt".to_string(), + } + } else if needs_approval { ExecApprovalRequirement::NeedsApproval { reason: None, proposed_execpolicy_amendment: None, @@ -224,7 +235,13 @@ pub(crate) trait Approvable { /// Decide we can request an approval for no-sandbox execution. fn wants_no_sandbox_approval(&self, policy: AskForApproval) -> bool { - !matches!(policy, AskForApproval::Never | AskForApproval::OnRequest) + match policy { + AskForApproval::OnFailure => true, + AskForApproval::UnlessTrusted => true, + AskForApproval::Never => false, + AskForApproval::OnRequest => false, + AskForApproval::Reject(reject_config) => !reject_config.sandbox_approval, + } } fn start_approval_async<'a>( @@ -313,6 +330,7 @@ impl<'a> SandboxAttempt<'a> { mod tests { use super::*; use codex_protocol::protocol::NetworkAccess; + use codex_protocol::protocol::RejectConfig; use pretty_assertions::assert_eq; #[test] @@ -344,4 +362,43 @@ mod tests { } ); } + + #[test] + fn default_exec_approval_requirement_rejects_sandbox_prompt_when_configured() { + let policy = AskForApproval::Reject(RejectConfig { + sandbox_approval: true, + rules: false, + mcp_elicitations: false, + }); + + let requirement = + default_exec_approval_requirement(policy, &SandboxPolicy::new_read_only_policy()); + + assert_eq!( + requirement, + ExecApprovalRequirement::Forbidden { + reason: "approval policy rejected sandbox approval prompt".to_string(), + } + ); + } + + #[test] + fn default_exec_approval_requirement_keeps_prompt_when_sandbox_rejection_is_disabled() { + let policy = AskForApproval::Reject(RejectConfig { + sandbox_approval: false, + rules: true, + mcp_elicitations: true, + }); + + let requirement = + default_exec_approval_requirement(policy, &SandboxPolicy::new_read_only_policy()); + + assert_eq!( + requirement, + ExecApprovalRequirement::NeedsApproval { + reason: None, + proposed_execpolicy_amendment: None, + } + ); + } } diff --git a/codex-rs/core/templates/collaboration_mode/plan.md b/codex-rs/core/templates/collaboration_mode/plan.md index adde92745a..5c58310ae4 100644 --- a/codex-rs/core/templates/collaboration_mode/plan.md +++ b/codex-rs/core/templates/collaboration_mode/plan.md @@ -118,3 +118,5 @@ plan content should be human and agent digestible. The final plan must be plan-o Do not ask "should I proceed?" in the final output. The user can easily switch out of Plan mode and request implementation if you have included a `` block in your response. Alternatively, they can decide to stay in Plan mode and continue refining the plan. Only produce at most one `` block per turn, and only when you are presenting a complete spec. + +If the user stays in Plan mode and asks for revisions after a prior ``, any new `` must be a complete replacement. diff --git a/codex-rs/core/tests/suite/client_websockets.rs b/codex-rs/core/tests/suite/client_websockets.rs index 626d0695be..1c7c578cae 100755 --- a/codex-rs/core/tests/suite/client_websockets.rs +++ b/codex-rs/core/tests/suite/client_websockets.rs @@ -714,6 +714,77 @@ async fn responses_websocket_appends_on_prefix() { server.shutdown().await; } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn responses_websocket_forwards_turn_metadata_on_create_and_append() { + skip_if_no_network!(); + + let server = start_websocket_server(vec![vec![ + vec![ + ev_response_created("resp-1"), + ev_assistant_message("msg-1", "assistant output"), + ev_done(), + ], + vec![ev_response_created("resp-2"), ev_completed("resp-2")], + ]]) + .await; + + let harness = websocket_harness(&server).await; + let mut client_session = harness.client.new_session(); + let first_turn_metadata = r#"{"turn_id":"turn-123","sandbox":"workspace-write"}"#; + let enriched_turn_metadata = r#"{"turn_id":"turn-123","sandbox":"workspace-write","workspaces":[{"root_path":"/tmp/repo","latest_git_commit_hash":"abc123","associated_remote_urls":["git@github.com:openai/codex.git"],"has_changes":true}]}"#; + let prompt_one = prompt_with_input(vec![message_item("hello")]); + let prompt_two = prompt_with_input(vec![ + message_item("hello"), + assistant_message_item("msg-1", "assistant output"), + message_item("second"), + ]); + + stream_until_complete_with_turn_metadata( + &mut client_session, + &harness, + &prompt_one, + Some(first_turn_metadata), + ) + .await; + stream_until_complete_with_turn_metadata( + &mut client_session, + &harness, + &prompt_two, + Some(enriched_turn_metadata), + ) + .await; + + let connection = server.single_connection(); + assert_eq!(connection.len(), 2); + let first = connection.first().expect("missing request").body_json(); + let second = connection.get(1).expect("missing request").body_json(); + + assert_eq!(first["type"].as_str(), Some("response.create")); + assert_eq!( + first["client_metadata"]["x-codex-turn-metadata"].as_str(), + Some(first_turn_metadata) + ); + assert_eq!(second["type"].as_str(), Some("response.append")); + assert_eq!( + second["client_metadata"]["x-codex-turn-metadata"].as_str(), + Some(enriched_turn_metadata) + ); + + let first_metadata: serde_json::Value = + serde_json::from_str(first_turn_metadata).expect("first metadata should be valid json"); + let second_metadata: serde_json::Value = serde_json::from_str(enriched_turn_metadata) + .expect("enriched metadata should be valid json"); + + assert_eq!(first_metadata["turn_id"].as_str(), Some("turn-123")); + assert_eq!(second_metadata["turn_id"].as_str(), Some("turn-123")); + assert_eq!( + second_metadata["workspaces"][0]["has_changes"].as_bool(), + Some(true) + ); + + server.shutdown().await; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn responses_websocket_creates_on_prefix_when_previous_completion_cannot_append() { skip_if_no_network!(); @@ -1173,6 +1244,15 @@ async fn stream_until_complete( client_session: &mut ModelClientSession, harness: &WebsocketTestHarness, prompt: &Prompt, +) { + stream_until_complete_with_turn_metadata(client_session, harness, prompt, None).await; +} + +async fn stream_until_complete_with_turn_metadata( + client_session: &mut ModelClientSession, + harness: &WebsocketTestHarness, + prompt: &Prompt, + turn_metadata_header: Option<&str>, ) { let mut stream = client_session .stream( @@ -1181,7 +1261,7 @@ async fn stream_until_complete( &harness.otel_manager, harness.effort, harness.summary, - None, + turn_metadata_header, ) .await .expect("websocket stream failed"); diff --git a/codex-rs/core/tests/suite/rmcp_client.rs b/codex-rs/core/tests/suite/rmcp_client.rs index d1394f7a74..4c8cd563a6 100644 --- a/codex-rs/core/tests/suite/rmcp_client.rs +++ b/codex-rs/core/tests/suite/rmcp_client.rs @@ -840,8 +840,31 @@ async fn streamable_http_tool_call_round_trip() -> anyhow::Result<()> { /// This test writes to a fallback credentials file in CODEX_HOME. /// Ideally, we wouldn't need to serialize the test but it's much more cumbersome to wire CODEX_HOME through the code. #[serial(codex_home)] -#[tokio::test(flavor = "multi_thread", worker_threads = 1)] -async fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> { +#[test] +fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> { + const TEST_STACK_SIZE_BYTES: usize = 8 * 1024 * 1024; + + let handle = std::thread::Builder::new() + .name("streamable_http_with_oauth_round_trip".to_string()) + .stack_size(TEST_STACK_SIZE_BYTES) + .spawn(|| -> anyhow::Result<()> { + let runtime = tokio::runtime::Builder::new_multi_thread() + .worker_threads(1) + .enable_all() + .build()?; + runtime.block_on(streamable_http_with_oauth_round_trip_impl()) + })?; + + match handle.join() { + Ok(result) => result, + Err(_) => Err(anyhow::anyhow!( + "streamable_http_with_oauth_round_trip thread panicked" + )), + } +} + +#[allow(clippy::expect_used)] +async fn streamable_http_with_oauth_round_trip_impl() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); let server = responses::start_mock_server().await; @@ -912,6 +935,11 @@ async fn streamable_http_with_oauth_round_trip() -> anyhow::Result<()> { let fixture = test_codex() .with_config(move |config| { + // This test seeds OAuth tokens in CODEX_HOME/.credentials.json and + // validates file-backed OAuth loading. Force file mode so Linux + // keyring backend quirks do not affect this test. + config.mcp_oauth_credentials_store_mode = serde_json::from_value(json!("file")) + .expect("`file` should deserialize as OAuthCredentialsStoreMode"); let mut servers = config.mcp_servers.get().clone(); servers.insert( server_name.to_string(), diff --git a/codex-rs/core/tests/suite/shell_snapshot.rs b/codex-rs/core/tests/suite/shell_snapshot.rs index b5007fd589..6a0983a6ed 100644 --- a/codex-rs/core/tests/suite/shell_snapshot.rs +++ b/codex-rs/core/tests/suite/shell_snapshot.rs @@ -535,6 +535,10 @@ async fn shell_command_snapshot_still_intercepts_apply_patch() -> Result<()> { }) .await?; + let snapshot_path = wait_for_snapshot(&codex_home).await?; + let snapshot_content = fs::read_to_string(&snapshot_path).await?; + assert_posix_snapshot_sections(&snapshot_content); + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; assert_eq!( @@ -542,10 +546,6 @@ async fn shell_command_snapshot_still_intercepts_apply_patch() -> Result<()> { "hello from snapshot\n" ); - let snapshot_path = wait_for_snapshot(&codex_home).await?; - let snapshot_content = fs::read_to_string(&snapshot_path).await?; - assert_posix_snapshot_sections(&snapshot_content); - Ok(()) } diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 3a98aa5398..65099541c4 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -240,20 +240,35 @@ impl DeveloperInstructions { } pub fn from(approval_policy: AskForApproval, exec_policy: &Policy) -> DeveloperInstructions { + let on_request_instructions = || { + let command_prefixes = format_allow_prefixes(exec_policy.get_allowed_prefixes()); + match command_prefixes { + Some(prefixes) => { + format!( + "{APPROVAL_POLICY_ON_REQUEST_RULE}\n## Approved command prefixes\nThe following prefix rules have already been approved: {prefixes}" + ) + } + None => APPROVAL_POLICY_ON_REQUEST_RULE.to_string(), + } + }; let text = match approval_policy { AskForApproval::Never => APPROVAL_POLICY_NEVER.to_string(), AskForApproval::UnlessTrusted => APPROVAL_POLICY_UNLESS_TRUSTED.to_string(), AskForApproval::OnFailure => APPROVAL_POLICY_ON_FAILURE.to_string(), - AskForApproval::OnRequest => { - let command_prefixes = format_allow_prefixes(exec_policy.get_allowed_prefixes()); - match command_prefixes { - Some(prefixes) => { - format!( - "{APPROVAL_POLICY_ON_REQUEST_RULE}\n## Approved command prefixes\nThe following prefix rules have already been approved: {prefixes}" - ) - } - None => APPROVAL_POLICY_ON_REQUEST_RULE.to_string(), - } + AskForApproval::OnRequest => on_request_instructions(), + AskForApproval::Reject(reject_config) => { + let on_request_instructions = on_request_instructions(); + let sandbox_approval = reject_config.sandbox_approval; + let rules = reject_config.rules; + let mcp_elicitations = reject_config.mcp_elicitations; + format!( + "{on_request_instructions}\n\n\ + Approval policy is `reject`.\n\ + - `sandbox_approval`: {sandbox_approval}\n\ + - `rules`: {rules}\n\ + - `mcp_elicitations`: {mcp_elicitations}\n\ + When a category is `true`, requests in that category are auto-rejected instead of prompting the user." + ) } }; diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index b7e4b547fd..476f902085 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -378,11 +378,41 @@ pub enum AskForApproval { #[default] OnRequest, + /// Fine-grained rejection controls for approval prompts. + /// + /// When a field is `true`, prompts of that category are automatically + /// rejected instead of shown to the user. + Reject(RejectConfig), + /// Never ask the user to approve commands. Failures are immediately returned /// to the model, and never escalated to the user for approval. Never, } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize, JsonSchema, TS)] +pub struct RejectConfig { + /// Reject approval prompts related to sandbox escalation. + pub sandbox_approval: bool, + /// Reject prompts triggered by execpolicy `prompt` rules. + pub rules: bool, + /// Reject MCP elicitation prompts. + pub mcp_elicitations: bool, +} + +impl RejectConfig { + pub const fn rejects_sandbox_approval(self) -> bool { + self.sandbox_approval + } + + pub const fn rejects_rules_approval(self) -> bool { + self.rules + } + + pub const fn rejects_mcp_elicitations(self) -> bool { + self.mcp_elicitations + } +} + /// Represents whether outbound network access is available to the agent. #[derive( Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Display, Default, JsonSchema, TS, @@ -2818,6 +2848,26 @@ mod tests { assert!(enabled.has_full_network_access()); } + #[test] + fn reject_config_mcp_elicitation_flag_is_field_driven() { + assert!( + RejectConfig { + sandbox_approval: false, + rules: false, + mcp_elicitations: true, + } + .rejects_mcp_elicitations() + ); + assert!( + !RejectConfig { + sandbox_approval: false, + rules: false, + mcp_elicitations: false, + } + .rejects_mcp_elicitations() + ); + } + #[test] fn workspace_write_restricted_read_access_includes_effective_writable_roots() { let cwd = if cfg!(windows) { diff --git a/codex-rs/responses-api-proxy/npm/package.json b/codex-rs/responses-api-proxy/npm/package.json index d72b60e218..80a0607544 100644 --- a/codex-rs/responses-api-proxy/npm/package.json +++ b/codex-rs/responses-api-proxy/npm/package.json @@ -18,5 +18,5 @@ "url": "git+https://github.com/openai/codex.git", "directory": "codex-rs/responses-api-proxy/npm" }, - "packageManager": "pnpm@10.28.2+sha512.41872f037ad22f7348e3b1debbaf7e867cfd448f2726d9cf74c08f19507c31d2c8e7a11525b983febc2df640b5438dee6023ebb1f84ed43cc2d654d2bc326264" + "packageManager": "pnpm@10.29.3+sha512.498e1fb4cca5aa06c1dcf2611e6fafc50972ffe7189998c409e90de74566444298ffe43e6cd2acdc775ba1aa7cc5e092a8b7054c811ba8c5770f84693d33d2dc" } diff --git a/codex-rs/rmcp-client/src/perform_oauth_login.rs b/codex-rs/rmcp-client/src/perform_oauth_login.rs index 09b746837e..62a9c3b019 100644 --- a/codex-rs/rmcp-client/src/perform_oauth_login.rs +++ b/codex-rs/rmcp-client/src/perform_oauth_login.rs @@ -8,6 +8,7 @@ use anyhow::Result; use anyhow::anyhow; use anyhow::bail; use reqwest::ClientBuilder; +use reqwest::Url; use rmcp::transport::auth::OAuthState; use tiny_http::Response; use tiny_http::Server; @@ -38,6 +39,7 @@ impl Drop for CallbackServerGuard { } } +#[allow(clippy::too_many_arguments)] pub async fn perform_oauth_login( server_name: &str, server_url: &str, @@ -46,6 +48,7 @@ pub async fn perform_oauth_login( env_http_headers: Option>, scopes: &[String], callback_port: Option, + callback_url: Option<&str>, ) -> Result<()> { let headers = OauthHeaders { http_headers, @@ -59,6 +62,7 @@ pub async fn perform_oauth_login( scopes, true, callback_port, + callback_url, None, ) .await? @@ -76,6 +80,7 @@ pub async fn perform_oauth_login_return_url( scopes: &[String], timeout_secs: Option, callback_port: Option, + callback_url: Option<&str>, ) -> Result { let headers = OauthHeaders { http_headers, @@ -89,6 +94,7 @@ pub async fn perform_oauth_login_return_url( scopes, false, callback_port, + callback_url, timeout_secs, ) .await?; @@ -99,11 +105,15 @@ pub async fn perform_oauth_login_return_url( Ok(OauthLoginHandle::new(authorization_url, completion)) } -fn spawn_callback_server(server: Arc, tx: oneshot::Sender<(String, String)>) { +fn spawn_callback_server( + server: Arc, + tx: oneshot::Sender<(String, String)>, + expected_callback_path: String, +) { tokio::task::spawn_blocking(move || { while let Ok(request) = server.recv() { let path = request.url().to_string(); - match parse_oauth_callback(&path) { + match parse_oauth_callback(&path, &expected_callback_path) { CallbackOutcome::Success(OauthCallbackResult { code, state }) => { let response = Response::from_string( "Authentication complete. You may close this window.", @@ -146,11 +156,11 @@ enum CallbackOutcome { Invalid, } -fn parse_oauth_callback(path: &str) -> CallbackOutcome { +fn parse_oauth_callback(path: &str, expected_callback_path: &str) -> CallbackOutcome { let Some((route, query)) = path.split_once('?') else { return CallbackOutcome::Invalid; }; - if route != "/callback" { + if route != expected_callback_path { return CallbackOutcome::Invalid; } @@ -238,6 +248,53 @@ fn resolve_callback_port(callback_port: Option) -> Result> { Ok(None) } +fn local_redirect_uri(server: &Server) -> Result { + match server.server_addr() { + tiny_http::ListenAddr::IP(std::net::SocketAddr::V4(addr)) => { + let ip = addr.ip(); + let port = addr.port(); + Ok(format!("http://{ip}:{port}/callback")) + } + tiny_http::ListenAddr::IP(std::net::SocketAddr::V6(addr)) => { + let ip = addr.ip(); + let port = addr.port(); + Ok(format!("http://[{ip}]:{port}/callback")) + } + #[cfg(not(target_os = "windows"))] + _ => Err(anyhow!("unable to determine callback address")), + } +} + +fn resolve_redirect_uri(server: &Server, callback_url: Option<&str>) -> Result { + let Some(callback_url) = callback_url else { + return local_redirect_uri(server); + }; + Url::parse(callback_url) + .with_context(|| format!("invalid MCP OAuth callback URL `{callback_url}`"))?; + Ok(callback_url.to_string()) +} + +fn callback_path_from_redirect_uri(redirect_uri: &str) -> Result { + let parsed = Url::parse(redirect_uri) + .with_context(|| format!("invalid redirect URI `{redirect_uri}`"))?; + Ok(parsed.path().to_string()) +} + +fn callback_bind_host(callback_url: Option<&str>) -> &'static str { + let Some(callback_url) = callback_url else { + return "127.0.0.1"; + }; + + let Ok(parsed) = Url::parse(callback_url) else { + return "127.0.0.1"; + }; + + match parsed.host_str() { + Some("localhost" | "127.0.0.1" | "::1") | None => "127.0.0.1", + Some(_) => "0.0.0.0", + } +} + impl OauthLoginFlow { #[allow(clippy::too_many_arguments)] async fn new( @@ -248,14 +305,16 @@ impl OauthLoginFlow { scopes: &[String], launch_browser: bool, callback_port: Option, + callback_url: Option<&str>, timeout_secs: Option, ) -> Result { const DEFAULT_OAUTH_TIMEOUT_SECS: i64 = 300; + let bind_host = callback_bind_host(callback_url); let callback_port = resolve_callback_port(callback_port)?; let bind_addr = match callback_port { - Some(port) => format!("127.0.0.1:{port}"), - None => "127.0.0.1:0".to_string(), + Some(port) => format!("{bind_host}:{port}"), + None => format!("{bind_host}:0"), }; let server = Arc::new(Server::http(&bind_addr).map_err(|err| anyhow!(err))?); @@ -263,23 +322,11 @@ impl OauthLoginFlow { server: Arc::clone(&server), }; - let redirect_uri = match server.server_addr() { - tiny_http::ListenAddr::IP(std::net::SocketAddr::V4(addr)) => { - let ip = addr.ip(); - let port = addr.port(); - format!("http://{ip}:{port}/callback") - } - tiny_http::ListenAddr::IP(std::net::SocketAddr::V6(addr)) => { - let ip = addr.ip(); - let port = addr.port(); - format!("http://[{ip}]:{port}/callback") - } - #[cfg(not(target_os = "windows"))] - _ => return Err(anyhow!("unable to determine callback address")), - }; + let redirect_uri = resolve_redirect_uri(&server, callback_url)?; + let callback_path = callback_path_from_redirect_uri(&redirect_uri)?; let (tx, rx) = oneshot::channel(); - spawn_callback_server(server, tx); + spawn_callback_server(server, tx, callback_path); let OauthHeaders { http_headers, @@ -383,3 +430,35 @@ impl OauthLoginFlow { rx } } + +#[cfg(test)] +mod tests { + use super::CallbackOutcome; + use super::callback_path_from_redirect_uri; + use super::parse_oauth_callback; + + #[test] + fn parse_oauth_callback_accepts_default_path() { + let parsed = parse_oauth_callback("/callback?code=abc&state=xyz", "/callback"); + assert!(matches!(parsed, CallbackOutcome::Success(_))); + } + + #[test] + fn parse_oauth_callback_accepts_custom_path() { + let parsed = parse_oauth_callback("/oauth/callback?code=abc&state=xyz", "/oauth/callback"); + assert!(matches!(parsed, CallbackOutcome::Success(_))); + } + + #[test] + fn parse_oauth_callback_rejects_wrong_path() { + let parsed = parse_oauth_callback("/callback?code=abc&state=xyz", "/oauth/callback"); + assert!(matches!(parsed, CallbackOutcome::Invalid)); + } + + #[test] + fn callback_path_comes_from_redirect_uri() { + let path = callback_path_from_redirect_uri("https://example.com/oauth/callback") + .expect("redirect URI should parse"); + assert_eq!(path, "/oauth/callback"); + } +} diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 9e746b4c57..7dcab8010a 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -539,6 +539,7 @@ pub(crate) struct ChatWidget { mcp_startup_status: Option>, connectors_cache: ConnectorsCacheState, connectors_prefetch_in_flight: bool, + connectors_force_refetch_pending: bool, // Queue of interruptive UI events deferred during an active write cycle interrupts: InterruptManager, // Accumulates the current reasoning block text to extract a header @@ -2650,6 +2651,7 @@ impl ChatWidget { mcp_startup_status: None, connectors_cache: ConnectorsCacheState::default(), connectors_prefetch_in_flight: false, + connectors_force_refetch_pending: false, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), full_reasoning_buffer: String::new(), @@ -2813,6 +2815,7 @@ impl ChatWidget { mcp_startup_status: None, connectors_cache: ConnectorsCacheState::default(), connectors_prefetch_in_flight: false, + connectors_force_refetch_pending: false, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), full_reasoning_buffer: String::new(), @@ -2965,6 +2968,7 @@ impl ChatWidget { mcp_startup_status: None, connectors_cache: ConnectorsCacheState::default(), connectors_prefetch_in_flight: false, + connectors_force_refetch_pending: false, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), full_reasoning_buffer: String::new(), @@ -4601,7 +4605,13 @@ impl ChatWidget { } fn prefetch_connectors_with_options(&mut self, force_refetch: bool) { - if !self.connectors_enabled() || self.connectors_prefetch_in_flight { + if !self.connectors_enabled() { + return; + } + if self.connectors_prefetch_in_flight { + if force_refetch { + self.connectors_force_refetch_pending = true; + } return; } @@ -4613,8 +4623,8 @@ impl ChatWidget { let config = self.config.clone(); let app_event_tx = self.app_event_tx.clone(); tokio::spawn(async move { - let accessible_connectors = - match connectors::list_accessible_connectors_from_mcp_tools_with_options( + let accessible_result = + match connectors::list_accessible_connectors_from_mcp_tools_with_options_and_status( &config, force_refetch, ) @@ -4629,6 +4639,9 @@ impl ChatWidget { return; } }; + let should_schedule_force_refetch = + !force_refetch && !accessible_result.codex_apps_ready; + let accessible_connectors = accessible_result.connectors; app_event_tx.send(AppEvent::ConnectorsLoaded { result: Ok(ConnectorsSnapshot { @@ -4638,7 +4651,8 @@ impl ChatWidget { }); let result: Result = async { - let all_connectors = connectors::list_all_connectors(&config).await?; + let all_connectors = + connectors::list_all_connectors_with_options(&config, force_refetch).await?; let connectors = connectors::merge_connectors_with_accessible( all_connectors, accessible_connectors, @@ -4653,6 +4667,12 @@ impl ChatWidget { result, is_final: true, }); + + if should_schedule_force_refetch { + app_event_tx.send(AppEvent::RefreshConnectors { + force_refetch: true, + }); + } }); } @@ -6821,8 +6841,13 @@ impl ChatWidget { result: Result, is_final: bool, ) { + let mut trigger_pending_force_refetch = false; if is_final { self.connectors_prefetch_in_flight = false; + if self.connectors_force_refetch_pending { + self.connectors_force_refetch_pending = false; + trigger_pending_force_refetch = true; + } } match result { @@ -6857,13 +6882,16 @@ impl ChatWidget { Err(err) => { if matches!(self.connectors_cache, ConnectorsCacheState::Ready(_)) { warn!("failed to refresh apps list; retaining current apps snapshot: {err}"); - return; + } else { + self.connectors_cache = ConnectorsCacheState::Failed(err); + self.bottom_pane.set_connectors_snapshot(None); } - - self.connectors_cache = ConnectorsCacheState::Failed(err); - self.bottom_pane.set_connectors_snapshot(None); } } + + if trigger_pending_force_refetch { + self.prefetch_connectors_with_options(true); + } } pub(crate) fn update_connector_enabled(&mut self, connector_id: &str, enabled: bool) { diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 829c39d0a0..d6d2ab1ad7 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -1627,6 +1627,7 @@ async fn make_chatwidget_manual( mcp_startup_status: None, connectors_cache: ConnectorsCacheState::default(), connectors_prefetch_in_flight: false, + connectors_force_refetch_pending: false, interrupts: InterruptManager::new(), reasoning_buffer: String::new(), full_reasoning_buffer: String::new(), @@ -4582,6 +4583,42 @@ async fn apps_refresh_failure_keeps_existing_full_snapshot() { ); } +#[tokio::test] +async fn apps_refresh_failure_with_cached_snapshot_triggers_pending_force_refetch() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; + chat.config.features.enable(Feature::Apps); + chat.bottom_pane.set_connectors_enabled(true); + chat.connectors_prefetch_in_flight = true; + chat.connectors_force_refetch_pending = true; + + let full_connectors = vec![codex_chatgpt::connectors::AppInfo { + id: "unit_test_apps_refresh_failure_pending_connector".to_string(), + name: "Notion".to_string(), + description: Some("Workspace docs".to_string()), + logo_url: None, + logo_url_dark: None, + distribution_channel: None, + branding: None, + app_metadata: None, + labels: None, + install_url: Some("https://example.test/notion".to_string()), + is_accessible: true, + is_enabled: true, + }]; + chat.connectors_cache = ConnectorsCacheState::Ready(ConnectorsSnapshot { + connectors: full_connectors.clone(), + }); + + chat.on_connectors_loaded(Err("failed to load apps".to_string()), true); + + assert!(chat.connectors_prefetch_in_flight); + assert!(!chat.connectors_force_refetch_pending); + assert_matches!( + &chat.connectors_cache, + ConnectorsCacheState::Ready(snapshot) if snapshot.connectors == full_connectors + ); +} + #[tokio::test] async fn apps_partial_refresh_uses_same_filtering_as_full_refresh() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None).await; diff --git a/defs.bzl b/defs.bzl index b3fe9903ea..6be15d80f9 100644 --- a/defs.bzl +++ b/defs.bzl @@ -141,9 +141,10 @@ def codex_rust_crate( tags = test_tags, ) - maybe_lib = [name] - else: - maybe_lib = [] + if proc_macro: + proc_macro_deps += [name] + else: + deps += [name] sanitized_binaries = [] cargo_env = {} @@ -156,7 +157,7 @@ def codex_rust_crate( name = binary, crate_name = binary.replace("-", "_"), crate_root = main, - deps = maybe_lib + deps, + deps = deps, proc_macro_deps = proc_macro_deps, edition = crate_edition, rustc_flags = rustc_flags_extra, @@ -183,7 +184,7 @@ def codex_rust_crate( srcs = [test], data = native.glob(["tests/**"], allow_empty = True) + sanitized_binaries + test_data_extra, compile_data = native.glob(["tests/**"], allow_empty = True) + integration_compile_data_extra, - deps = maybe_lib + deps + dev_deps + integration_deps_extra, + deps = deps + dev_deps + integration_deps_extra, proc_macro_deps = proc_macro_deps + proc_macro_dev_deps, # Keep `file!()` paths Cargo-like (`core/tests/...`) instead of # Bazel workspace-prefixed (`codex-rs/core/tests/...`) for snapshot parity. diff --git a/package.json b/package.json index 633a6f8331..b4ca5734bb 100644 --- a/package.json +++ b/package.json @@ -19,7 +19,7 @@ }, "engines": { "node": ">=22", - "pnpm": ">=10.28.0" + "pnpm": ">=10.29.3" }, - "packageManager": "pnpm@10.28.2+sha512.41872f037ad22f7348e3b1debbaf7e867cfd448f2726d9cf74c08f19507c31d2c8e7a11525b983febc2df640b5438dee6023ebb1f84ed43cc2d654d2bc326264" + "packageManager": "pnpm@10.29.3+sha512.498e1fb4cca5aa06c1dcf2611e6fafc50972ffe7189998c409e90de74566444298ffe43e6cd2acdc775ba1aa7cc5e092a8b7054c811ba8c5770f84693d33d2dc" } diff --git a/sdk/typescript/package.json b/sdk/typescript/package.json index ab29c3fcec..5959477e93 100644 --- a/sdk/typescript/package.json +++ b/sdk/typescript/package.json @@ -63,5 +63,5 @@ "zod": "^3.24.2", "zod-to-json-schema": "^3.24.6" }, - "packageManager": "pnpm@10.28.2+sha512.41872f037ad22f7348e3b1debbaf7e867cfd448f2726d9cf74c08f19507c31d2c8e7a11525b983febc2df640b5438dee6023ebb1f84ed43cc2d654d2bc326264" + "packageManager": "pnpm@10.29.3+sha512.498e1fb4cca5aa06c1dcf2611e6fafc50972ffe7189998c409e90de74566444298ffe43e6cd2acdc775ba1aa7cc5e092a8b7054c811ba8c5770f84693d33d2dc" } diff --git a/shell-tool-mcp/package.json b/shell-tool-mcp/package.json index 24798b2f6e..ca7dd14656 100644 --- a/shell-tool-mcp/package.json +++ b/shell-tool-mcp/package.json @@ -37,5 +37,5 @@ "tsup": "^8.5.0", "typescript": "^5.9.2" }, - "packageManager": "pnpm@10.28.2+sha512.41872f037ad22f7348e3b1debbaf7e867cfd448f2726d9cf74c08f19507c31d2c8e7a11525b983febc2df640b5438dee6023ebb1f84ed43cc2d654d2bc326264" + "packageManager": "pnpm@10.29.3+sha512.498e1fb4cca5aa06c1dcf2611e6fafc50972ffe7189998c409e90de74566444298ffe43e6cd2acdc775ba1aa7cc5e092a8b7054c811ba8c5770f84693d33d2dc" }