mirror of
https://github.com/openai/codex.git
synced 2026-11-02 19:43:24 +00:00
Merge 1becaea275 into sapling-pr-archive-bolinfest
This commit is contained in:
Generated
+42
@@ -819,6 +819,8 @@ version = "1.2.30"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "deec109607ca693028562ed836a5f1c4b8bd77755c4e132fc5ce11b0b6211ae7"
|
||||
dependencies = [
|
||||
"jobserver",
|
||||
"libc",
|
||||
"shlex",
|
||||
]
|
||||
|
||||
@@ -1197,6 +1199,7 @@ dependencies = [
|
||||
"tracing",
|
||||
"tracing-opentelemetry",
|
||||
"tracing-subscriber",
|
||||
"zstd",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -1348,6 +1351,7 @@ dependencies = [
|
||||
"which",
|
||||
"wildmatch",
|
||||
"wiremock",
|
||||
"zstd",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -3924,6 +3928,16 @@ version = "0.3.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "8eaf4bc02d17cbdd7ff4c7438cafcdf7fb9a4613313ad11b4f8fefe7d3fa0130"
|
||||
|
||||
[[package]]
|
||||
name = "jobserver"
|
||||
version = "0.1.34"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "9afb3de4395d6b3e67a780b6de64b51c978ecf11cb9a462c66be7d4ca9039d33"
|
||||
dependencies = [
|
||||
"getrandom 0.3.3",
|
||||
"libc",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "js-sys"
|
||||
version = "0.3.77"
|
||||
@@ -8809,6 +8823,34 @@ dependencies = [
|
||||
"syn 2.0.104",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "zstd"
|
||||
version = "0.13.3"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "e91ee311a569c327171651566e07972200e76fcfe2242a4fa446149a3881c08a"
|
||||
dependencies = [
|
||||
"zstd-safe",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "zstd-safe"
|
||||
version = "7.2.4"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "8f49c4d5f0abb602a93fb8736af2a4f4dd9512e36f7f570d66e65ff867ed3b9d"
|
||||
dependencies = [
|
||||
"zstd-sys",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "zstd-sys"
|
||||
version = "2.0.16+zstd.1.5.7"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "91e19ebc2adc8f83e43039e79776e3fda8ca919132d68a1fed6a5faca2683748"
|
||||
dependencies = [
|
||||
"cc",
|
||||
"pkg-config",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "zune-core"
|
||||
version = "0.4.12"
|
||||
|
||||
@@ -218,6 +218,7 @@ tracing-subscriber = "0.3.22"
|
||||
tracing-test = "0.2.5"
|
||||
tree-sitter = "0.25.10"
|
||||
tree-sitter-bash = "0.25"
|
||||
zstd = "0.13"
|
||||
tree-sitter-highlight = "0.25.10"
|
||||
ts-rs = "11"
|
||||
tui-scrollbar = "0.2.1"
|
||||
|
||||
@@ -197,6 +197,11 @@ client_request_definitions! {
|
||||
response: v2::ConfigWriteResponse,
|
||||
},
|
||||
|
||||
ConfigRequirementsRead => "configRequirements/read" {
|
||||
params: #[ts(type = "undefined")] #[serde(skip_serializing_if = "Option::is_none")] Option<()>,
|
||||
response: v2::ConfigRequirementsReadResponse,
|
||||
},
|
||||
|
||||
GetAccount => "account/read" {
|
||||
params: v2::GetAccountParams,
|
||||
response: v2::GetAccountResponse,
|
||||
@@ -711,6 +716,22 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn serialize_config_requirements_read() -> Result<()> {
|
||||
let request = ClientRequest::ConfigRequirementsRead {
|
||||
request_id: RequestId::Integer(1),
|
||||
params: None,
|
||||
};
|
||||
assert_eq!(
|
||||
json!({
|
||||
"method": "configRequirements/read",
|
||||
"id": 1,
|
||||
}),
|
||||
serde_json::to_value(&request)?,
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn serialize_account_login_api_key() -> Result<()> {
|
||||
let request = ClientRequest::LoginAccount {
|
||||
|
||||
@@ -453,6 +453,22 @@ pub struct ConfigReadResponse {
|
||||
pub layers: Option<Vec<ConfigLayer>>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct ConfigRequirements {
|
||||
pub allowed_approval_policies: Option<Vec<AskForApproval>>,
|
||||
pub allowed_sandbox_modes: Option<Vec<SandboxMode>>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct ConfigRequirementsReadResponse {
|
||||
/// Null if no requirements are configured (e.g. no requirements.toml/MDM entries).
|
||||
pub requirements: Option<ConfigRequirements>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
@@ -487,14 +503,33 @@ pub struct ConfigEdit {
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub enum ApprovalDecision {
|
||||
pub enum CommandExecutionApprovalDecision {
|
||||
/// User approved the command.
|
||||
Accept,
|
||||
/// Approve and remember the approval for the session.
|
||||
/// User approved the command and future identical commands should run without prompting.
|
||||
AcceptForSession,
|
||||
/// User approved the command, and wants to apply the proposed execpolicy amendment so future
|
||||
/// matching commands can run without prompting.
|
||||
AcceptWithExecpolicyAmendment {
|
||||
execpolicy_amendment: ExecPolicyAmendment,
|
||||
},
|
||||
/// User denied the command. The agent will continue the turn.
|
||||
Decline,
|
||||
/// User denied the command. The turn will also be immediately interrupted.
|
||||
Cancel,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub enum FileChangeApprovalDecision {
|
||||
/// User approved the file changes.
|
||||
Accept,
|
||||
/// User approved the file changes and future changes to the same files should run without prompting.
|
||||
AcceptForSession,
|
||||
/// User denied the file changes. The agent will continue the turn.
|
||||
Decline,
|
||||
/// User denied the file changes. The turn will also be immediately interrupted.
|
||||
Cancel,
|
||||
}
|
||||
|
||||
@@ -1885,7 +1920,7 @@ pub struct CommandExecutionRequestApprovalParams {
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct CommandExecutionRequestApprovalResponse {
|
||||
pub decision: ApprovalDecision,
|
||||
pub decision: CommandExecutionApprovalDecision,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
@@ -1905,7 +1940,7 @@ pub struct FileChangeRequestApprovalParams {
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct FileChangeRequestApprovalResponse {
|
||||
pub decision: ApprovalDecision,
|
||||
pub decision: FileChangeApprovalDecision,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
|
||||
@@ -18,12 +18,13 @@ use clap::Parser;
|
||||
use clap::Subcommand;
|
||||
use codex_app_server_protocol::AddConversationListenerParams;
|
||||
use codex_app_server_protocol::AddConversationSubscriptionResponse;
|
||||
use codex_app_server_protocol::ApprovalDecision;
|
||||
use codex_app_server_protocol::AskForApproval;
|
||||
use codex_app_server_protocol::ClientInfo;
|
||||
use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::CommandExecutionApprovalDecision;
|
||||
use codex_app_server_protocol::CommandExecutionRequestApprovalParams;
|
||||
use codex_app_server_protocol::CommandExecutionRequestApprovalResponse;
|
||||
use codex_app_server_protocol::FileChangeApprovalDecision;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalParams;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalResponse;
|
||||
use codex_app_server_protocol::GetAccountRateLimitsResponse;
|
||||
@@ -845,7 +846,7 @@ impl CodexClient {
|
||||
}
|
||||
|
||||
let response = CommandExecutionRequestApprovalResponse {
|
||||
decision: ApprovalDecision::Accept,
|
||||
decision: CommandExecutionApprovalDecision::Accept,
|
||||
};
|
||||
self.send_server_request_response(request_id, &response)?;
|
||||
println!("< approved commandExecution request for item {item_id}");
|
||||
@@ -876,7 +877,7 @@ impl CodexClient {
|
||||
}
|
||||
|
||||
let response = FileChangeRequestApprovalResponse {
|
||||
decision: ApprovalDecision::Accept,
|
||||
decision: FileChangeApprovalDecision::Accept,
|
||||
};
|
||||
self.send_server_request_response(request_id, &response)?;
|
||||
println!("< approved fileChange request for item {item_id}");
|
||||
|
||||
@@ -11,6 +11,8 @@
|
||||
- [Initialization](#initialization)
|
||||
- [API Overview](#api-overview)
|
||||
- [Events](#events)
|
||||
- [Approvals](#approvals)
|
||||
- [Skills](#skills)
|
||||
- [Auth endpoints](#auth-endpoints)
|
||||
|
||||
## Protocol
|
||||
@@ -86,6 +88,7 @@ Example (from OpenAI's official VSCode extension):
|
||||
- `config/read` — fetch the effective config on disk after resolving config layering.
|
||||
- `config/value/write` — write a single config key/value to the user's config.toml on disk.
|
||||
- `config/batchWrite` — apply multiple config edits atomically to the user's config.toml on disk.
|
||||
- `configRequirements/read` — fetch the loaded requirements allow-lists from `requirements.toml` and/or MDM (or `null` if none are configured).
|
||||
|
||||
### Example: Start or resume a thread
|
||||
|
||||
@@ -196,6 +199,25 @@ You can optionally specify config overrides on the new turn. If specified, these
|
||||
} } }
|
||||
```
|
||||
|
||||
### Example: Start a turn (invoke a skill)
|
||||
|
||||
Invoke a skill by sending a text input that begins with `$<skill-name>`.
|
||||
|
||||
```json
|
||||
{ "method": "turn/start", "id": 33, "params": {
|
||||
"threadId": "thr_123",
|
||||
"input": [
|
||||
{ "type": "text", "text": "$skill-creator Add a new skill for triaging flaky CI and include step-by-step usage." }
|
||||
]
|
||||
} }
|
||||
{ "id": 33, "result": { "turn": {
|
||||
"id": "turn_457",
|
||||
"status": "inProgress",
|
||||
"items": [],
|
||||
"error": null
|
||||
} } }
|
||||
```
|
||||
|
||||
### Example: Interrupt an active turn
|
||||
|
||||
You can cancel a running Turn with `turn/interrupt`.
|
||||
@@ -405,6 +427,30 @@ Order of messages:
|
||||
|
||||
UI guidance for IDEs: surface an approval dialog as soon as the request arrives. The turn will proceed after the server receives a response to the approval request. The terminal `item/completed` notification will be sent with the appropriate status.
|
||||
|
||||
## Skills
|
||||
|
||||
Skills are invoked by sending a text input that starts with `$<skill-name>`. The rest of the text is passed to the skill as its input.
|
||||
|
||||
Example:
|
||||
|
||||
```
|
||||
$skill-creator Add a new skill for triaging flaky CI and include step-by-step usage.
|
||||
```
|
||||
|
||||
Use `skills/list` to fetch the available skills (optionally scoped by `cwd` and/or with `forceReload`).
|
||||
|
||||
```json
|
||||
{ "method": "skills/list", "id": 25, "params": {
|
||||
"cwd": "/Users/me/project",
|
||||
"forceReload": false
|
||||
} }
|
||||
{ "id": 25, "result": {
|
||||
"skills": [
|
||||
{ "name": "skill-creator", "description": "Create or update a Codex skill" }
|
||||
]
|
||||
} }
|
||||
```
|
||||
|
||||
## Auth endpoints
|
||||
|
||||
The JSON-RPC auth/account surface exposes request/response methods plus server-initiated notifications (no `id`). Use these to determine auth state, start or cancel logins, logout, and inspect ChatGPT rate limits.
|
||||
|
||||
@@ -13,9 +13,9 @@ use codex_app_server_protocol::AccountRateLimitsUpdatedNotification;
|
||||
use codex_app_server_protocol::AgentMessageDeltaNotification;
|
||||
use codex_app_server_protocol::ApplyPatchApprovalParams;
|
||||
use codex_app_server_protocol::ApplyPatchApprovalResponse;
|
||||
use codex_app_server_protocol::ApprovalDecision;
|
||||
use codex_app_server_protocol::CodexErrorInfo as V2CodexErrorInfo;
|
||||
use codex_app_server_protocol::CommandAction as V2ParsedCommand;
|
||||
use codex_app_server_protocol::CommandExecutionApprovalDecision;
|
||||
use codex_app_server_protocol::CommandExecutionOutputDeltaNotification;
|
||||
use codex_app_server_protocol::CommandExecutionRequestApprovalParams;
|
||||
use codex_app_server_protocol::CommandExecutionRequestApprovalResponse;
|
||||
@@ -26,6 +26,7 @@ use codex_app_server_protocol::ErrorNotification;
|
||||
use codex_app_server_protocol::ExecCommandApprovalParams;
|
||||
use codex_app_server_protocol::ExecCommandApprovalResponse;
|
||||
use codex_app_server_protocol::ExecPolicyAmendment as V2ExecPolicyAmendment;
|
||||
use codex_app_server_protocol::FileChangeApprovalDecision;
|
||||
use codex_app_server_protocol::FileChangeOutputDeltaNotification;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalParams;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalResponse;
|
||||
@@ -1193,6 +1194,21 @@ fn format_file_change_diff(change: &CoreFileChange) -> String {
|
||||
}
|
||||
}
|
||||
|
||||
fn map_file_change_approval_decision(
|
||||
decision: FileChangeApprovalDecision,
|
||||
) -> (ReviewDecision, Option<PatchApplyStatus>) {
|
||||
match decision {
|
||||
FileChangeApprovalDecision::Accept => (ReviewDecision::Approved, None),
|
||||
FileChangeApprovalDecision::AcceptForSession => (ReviewDecision::ApprovedForSession, None),
|
||||
FileChangeApprovalDecision::Decline => {
|
||||
(ReviewDecision::Denied, Some(PatchApplyStatus::Declined))
|
||||
}
|
||||
FileChangeApprovalDecision::Cancel => {
|
||||
(ReviewDecision::Abort, Some(PatchApplyStatus::Declined))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
async fn on_file_change_request_approval_response(
|
||||
event_turn_id: String,
|
||||
@@ -1211,23 +1227,12 @@ async fn on_file_change_request_approval_response(
|
||||
.unwrap_or_else(|err| {
|
||||
error!("failed to deserialize FileChangeRequestApprovalResponse: {err}");
|
||||
FileChangeRequestApprovalResponse {
|
||||
decision: ApprovalDecision::Decline,
|
||||
decision: FileChangeApprovalDecision::Decline,
|
||||
}
|
||||
});
|
||||
|
||||
let (decision, completion_status) = match response.decision {
|
||||
ApprovalDecision::Accept
|
||||
| ApprovalDecision::AcceptForSession
|
||||
| ApprovalDecision::AcceptWithExecpolicyAmendment { .. } => {
|
||||
(ReviewDecision::Approved, None)
|
||||
}
|
||||
ApprovalDecision::Decline => {
|
||||
(ReviewDecision::Denied, Some(PatchApplyStatus::Declined))
|
||||
}
|
||||
ApprovalDecision::Cancel => {
|
||||
(ReviewDecision::Abort, Some(PatchApplyStatus::Declined))
|
||||
}
|
||||
};
|
||||
let (decision, completion_status) =
|
||||
map_file_change_approval_decision(response.decision);
|
||||
// Allow EventMsg::PatchApplyEnd to emit ItemCompleted for accepted patches.
|
||||
// Only short-circuit on declines/cancels/failures.
|
||||
(decision, completion_status)
|
||||
@@ -1281,16 +1286,18 @@ async fn on_command_execution_request_approval_response(
|
||||
.unwrap_or_else(|err| {
|
||||
error!("failed to deserialize CommandExecutionRequestApprovalResponse: {err}");
|
||||
CommandExecutionRequestApprovalResponse {
|
||||
decision: ApprovalDecision::Decline,
|
||||
decision: CommandExecutionApprovalDecision::Decline,
|
||||
}
|
||||
});
|
||||
|
||||
let decision = response.decision;
|
||||
|
||||
let (decision, completion_status) = match decision {
|
||||
ApprovalDecision::Accept => (ReviewDecision::Approved, None),
|
||||
ApprovalDecision::AcceptForSession => (ReviewDecision::ApprovedForSession, None),
|
||||
ApprovalDecision::AcceptWithExecpolicyAmendment {
|
||||
CommandExecutionApprovalDecision::Accept => (ReviewDecision::Approved, None),
|
||||
CommandExecutionApprovalDecision::AcceptForSession => {
|
||||
(ReviewDecision::ApprovedForSession, None)
|
||||
}
|
||||
CommandExecutionApprovalDecision::AcceptWithExecpolicyAmendment {
|
||||
execpolicy_amendment,
|
||||
} => (
|
||||
ReviewDecision::ApprovedExecpolicyAmendment {
|
||||
@@ -1298,11 +1305,11 @@ async fn on_command_execution_request_approval_response(
|
||||
},
|
||||
None,
|
||||
),
|
||||
ApprovalDecision::Decline => (
|
||||
CommandExecutionApprovalDecision::Decline => (
|
||||
ReviewDecision::Denied,
|
||||
Some(CommandExecutionStatus::Declined),
|
||||
),
|
||||
ApprovalDecision::Cancel => (
|
||||
CommandExecutionApprovalDecision::Cancel => (
|
||||
ReviewDecision::Abort,
|
||||
Some(CommandExecutionStatus::Declined),
|
||||
),
|
||||
@@ -1442,6 +1449,14 @@ mod tests {
|
||||
Arc::new(Mutex::new(HashMap::new()))
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn file_change_accept_for_session_maps_to_approved_for_session() {
|
||||
let (decision, completion_status) =
|
||||
map_file_change_approval_decision(FileChangeApprovalDecision::AcceptForSession);
|
||||
assert_eq!(decision, ReviewDecision::ApprovedForSession);
|
||||
assert_eq!(completion_status, None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_handle_error_records_message() -> Result<()> {
|
||||
let conversation_id = ThreadId::new();
|
||||
|
||||
@@ -510,6 +510,9 @@ impl CodexMessageProcessor {
|
||||
| ClientRequest::ConfigBatchWrite { .. } => {
|
||||
warn!("Config request reached CodexMessageProcessor unexpectedly");
|
||||
}
|
||||
ClientRequest::ConfigRequirementsRead { .. } => {
|
||||
warn!("ConfigRequirementsRead request reached CodexMessageProcessor unexpectedly");
|
||||
}
|
||||
ClientRequest::GetAccountRateLimits {
|
||||
request_id,
|
||||
params: _,
|
||||
|
||||
@@ -3,13 +3,18 @@ use crate::error_code::INVALID_REQUEST_ERROR_CODE;
|
||||
use codex_app_server_protocol::ConfigBatchWriteParams;
|
||||
use codex_app_server_protocol::ConfigReadParams;
|
||||
use codex_app_server_protocol::ConfigReadResponse;
|
||||
use codex_app_server_protocol::ConfigRequirements;
|
||||
use codex_app_server_protocol::ConfigRequirementsReadResponse;
|
||||
use codex_app_server_protocol::ConfigValueWriteParams;
|
||||
use codex_app_server_protocol::ConfigWriteErrorCode;
|
||||
use codex_app_server_protocol::ConfigWriteResponse;
|
||||
use codex_app_server_protocol::JSONRPCErrorError;
|
||||
use codex_app_server_protocol::SandboxMode;
|
||||
use codex_core::config::ConfigService;
|
||||
use codex_core::config::ConfigServiceError;
|
||||
use codex_core::config_loader::ConfigRequirementsToml;
|
||||
use codex_core::config_loader::LoaderOverrides;
|
||||
use codex_core::config_loader::SandboxModeRequirement as CoreSandboxModeRequirement;
|
||||
use serde_json::json;
|
||||
use std::path::PathBuf;
|
||||
use toml::Value as TomlValue;
|
||||
@@ -37,6 +42,19 @@ impl ConfigApi {
|
||||
self.service.read(params).await.map_err(map_error)
|
||||
}
|
||||
|
||||
pub(crate) async fn config_requirements_read(
|
||||
&self,
|
||||
) -> Result<ConfigRequirementsReadResponse, JSONRPCErrorError> {
|
||||
let requirements = self
|
||||
.service
|
||||
.read_requirements()
|
||||
.await
|
||||
.map_err(map_error)?
|
||||
.map(map_requirements_toml_to_api);
|
||||
|
||||
Ok(ConfigRequirementsReadResponse { requirements })
|
||||
}
|
||||
|
||||
pub(crate) async fn write_value(
|
||||
&self,
|
||||
params: ConfigValueWriteParams,
|
||||
@@ -52,6 +70,32 @@ impl ConfigApi {
|
||||
}
|
||||
}
|
||||
|
||||
fn map_requirements_toml_to_api(requirements: ConfigRequirementsToml) -> ConfigRequirements {
|
||||
ConfigRequirements {
|
||||
allowed_approval_policies: requirements.allowed_approval_policies.map(|policies| {
|
||||
policies
|
||||
.into_iter()
|
||||
.map(codex_app_server_protocol::AskForApproval::from)
|
||||
.collect()
|
||||
}),
|
||||
allowed_sandbox_modes: requirements.allowed_sandbox_modes.map(|modes| {
|
||||
modes
|
||||
.into_iter()
|
||||
.filter_map(map_sandbox_mode_requirement_to_api)
|
||||
.collect()
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
fn map_sandbox_mode_requirement_to_api(mode: CoreSandboxModeRequirement) -> Option<SandboxMode> {
|
||||
match mode {
|
||||
CoreSandboxModeRequirement::ReadOnly => Some(SandboxMode::ReadOnly),
|
||||
CoreSandboxModeRequirement::WorkspaceWrite => Some(SandboxMode::WorkspaceWrite),
|
||||
CoreSandboxModeRequirement::DangerFullAccess => Some(SandboxMode::DangerFullAccess),
|
||||
CoreSandboxModeRequirement::ExternalSandbox => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn map_error(err: ConfigServiceError) -> JSONRPCErrorError {
|
||||
if let Some(code) = err.write_error_code() {
|
||||
return config_write_error(code, err.to_string());
|
||||
@@ -73,3 +117,38 @@ fn config_write_error(code: ConfigWriteErrorCode, message: impl Into<String>) ->
|
||||
})),
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_protocol::protocol::AskForApproval as CoreAskForApproval;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn map_requirements_toml_to_api_converts_core_enums() {
|
||||
let requirements = ConfigRequirementsToml {
|
||||
allowed_approval_policies: Some(vec![
|
||||
CoreAskForApproval::Never,
|
||||
CoreAskForApproval::OnRequest,
|
||||
]),
|
||||
allowed_sandbox_modes: Some(vec![
|
||||
CoreSandboxModeRequirement::ReadOnly,
|
||||
CoreSandboxModeRequirement::ExternalSandbox,
|
||||
]),
|
||||
};
|
||||
|
||||
let mapped = map_requirements_toml_to_api(requirements);
|
||||
|
||||
assert_eq!(
|
||||
mapped.allowed_approval_policies,
|
||||
Some(vec![
|
||||
codex_app_server_protocol::AskForApproval::Never,
|
||||
codex_app_server_protocol::AskForApproval::OnRequest,
|
||||
])
|
||||
);
|
||||
assert_eq!(
|
||||
mapped.allowed_sandbox_modes,
|
||||
Some(vec![SandboxMode::ReadOnly]),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -158,6 +158,12 @@ impl MessageProcessor {
|
||||
ClientRequest::ConfigBatchWrite { request_id, params } => {
|
||||
self.handle_config_batch_write(request_id, params).await;
|
||||
}
|
||||
ClientRequest::ConfigRequirementsRead {
|
||||
request_id,
|
||||
params: _,
|
||||
} => {
|
||||
self.handle_config_requirements_read(request_id).await;
|
||||
}
|
||||
other => {
|
||||
self.codex_message_processor.process_request(other).await;
|
||||
}
|
||||
@@ -210,4 +216,11 @@ impl MessageProcessor {
|
||||
Err(error) => self.outgoing.send_error(request_id, error).await,
|
||||
}
|
||||
}
|
||||
|
||||
async fn handle_config_requirements_read(&self, request_id: RequestId) {
|
||||
match self.config_api.config_requirements_read().await {
|
||||
Ok(response) => self.outgoing.send_response(request_id, response).await,
|
||||
Err(error) => self.outgoing.send_error(request_id, error).await,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -40,7 +40,6 @@ fn preset_to_info(preset: &ModelPreset, priority: i32) -> ModelInfo {
|
||||
}
|
||||
}
|
||||
|
||||
// todo(aibrahim): fix the priorities to be the opposite here.
|
||||
/// Write a models_cache.json file to the codex home directory.
|
||||
/// This prevents ModelsManager from making network requests to refresh models.
|
||||
/// The cache will be treated as fresh (within TTL) and used instead of fetching from the network.
|
||||
@@ -51,14 +50,14 @@ pub fn write_models_cache(codex_home: &Path) -> std::io::Result<()> {
|
||||
.iter()
|
||||
.filter(|preset| preset.show_in_picker)
|
||||
.collect();
|
||||
// Convert presets to ModelInfo, assigning priorities (higher = earlier in list)
|
||||
// Priority is used for sorting, so first model gets highest priority
|
||||
// Convert presets to ModelInfo, assigning priorities (lower = earlier in list).
|
||||
// Priority is used for sorting, so the first model gets the lowest priority.
|
||||
let models: Vec<ModelInfo> = presets
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(idx, preset)| {
|
||||
// Higher priority = earlier in list, so reverse the index
|
||||
let priority = (presets.len() - idx) as i32;
|
||||
// Lower priority = earlier in list.
|
||||
let priority = idx as i32;
|
||||
preset_to_info(preset, priority)
|
||||
})
|
||||
.collect();
|
||||
|
||||
@@ -48,29 +48,23 @@ async fn list_models_returns_all_models_with_large_limit() -> Result<()> {
|
||||
|
||||
let expected_models = vec![
|
||||
Model {
|
||||
id: "gpt-5.2".to_string(),
|
||||
model: "gpt-5.2".to_string(),
|
||||
display_name: "gpt-5.2".to_string(),
|
||||
description:
|
||||
"Latest frontier model with improvements across knowledge, reasoning and coding"
|
||||
.to_string(),
|
||||
id: "gpt-5.2-codex".to_string(),
|
||||
model: "gpt-5.2-codex".to_string(),
|
||||
display_name: "gpt-5.2-codex".to_string(),
|
||||
description: "Latest frontier agentic coding model.".to_string(),
|
||||
supported_reasoning_efforts: vec![
|
||||
ReasoningEffortOption {
|
||||
reasoning_effort: ReasoningEffort::Low,
|
||||
description: "Balances speed with some reasoning; useful for straightforward \
|
||||
queries and short explanations"
|
||||
.to_string(),
|
||||
description: "Fast responses with lighter reasoning".to_string(),
|
||||
},
|
||||
ReasoningEffortOption {
|
||||
reasoning_effort: ReasoningEffort::Medium,
|
||||
description: "Provides a solid balance of reasoning depth and latency for \
|
||||
general-purpose tasks"
|
||||
description: "Balances speed and reasoning depth for everyday tasks"
|
||||
.to_string(),
|
||||
},
|
||||
ReasoningEffortOption {
|
||||
reasoning_effort: ReasoningEffort::High,
|
||||
description: "Maximizes reasoning depth for complex or ambiguous problems"
|
||||
.to_string(),
|
||||
description: "Greater reasoning depth for complex problems".to_string(),
|
||||
},
|
||||
ReasoningEffortOption {
|
||||
reasoning_effort: ReasoningEffort::XHigh,
|
||||
@@ -80,25 +74,6 @@ async fn list_models_returns_all_models_with_large_limit() -> Result<()> {
|
||||
default_reasoning_effort: ReasoningEffort::Medium,
|
||||
is_default: true,
|
||||
},
|
||||
Model {
|
||||
id: "gpt-5.1-codex-mini".to_string(),
|
||||
model: "gpt-5.1-codex-mini".to_string(),
|
||||
display_name: "gpt-5.1-codex-mini".to_string(),
|
||||
description: "Optimized for codex. Cheaper, faster, but less capable.".to_string(),
|
||||
supported_reasoning_efforts: vec![
|
||||
ReasoningEffortOption {
|
||||
reasoning_effort: ReasoningEffort::Medium,
|
||||
description: "Dynamically adjusts reasoning based on the task".to_string(),
|
||||
},
|
||||
ReasoningEffortOption {
|
||||
reasoning_effort: ReasoningEffort::High,
|
||||
description: "Maximizes reasoning depth for complex or ambiguous problems"
|
||||
.to_string(),
|
||||
},
|
||||
],
|
||||
default_reasoning_effort: ReasoningEffort::Medium,
|
||||
is_default: false,
|
||||
},
|
||||
Model {
|
||||
id: "gpt-5.1-codex-max".to_string(),
|
||||
model: "gpt-5.1-codex-max".to_string(),
|
||||
@@ -127,23 +102,48 @@ async fn list_models_returns_all_models_with_large_limit() -> Result<()> {
|
||||
is_default: false,
|
||||
},
|
||||
Model {
|
||||
id: "gpt-5.2-codex".to_string(),
|
||||
model: "gpt-5.2-codex".to_string(),
|
||||
display_name: "gpt-5.2-codex".to_string(),
|
||||
description: "Latest frontier agentic coding model.".to_string(),
|
||||
id: "gpt-5.1-codex-mini".to_string(),
|
||||
model: "gpt-5.1-codex-mini".to_string(),
|
||||
display_name: "gpt-5.1-codex-mini".to_string(),
|
||||
description: "Optimized for codex. Cheaper, faster, but less capable.".to_string(),
|
||||
supported_reasoning_efforts: vec![
|
||||
ReasoningEffortOption {
|
||||
reasoning_effort: ReasoningEffort::Medium,
|
||||
description: "Dynamically adjusts reasoning based on the task".to_string(),
|
||||
},
|
||||
ReasoningEffortOption {
|
||||
reasoning_effort: ReasoningEffort::High,
|
||||
description: "Maximizes reasoning depth for complex or ambiguous problems"
|
||||
.to_string(),
|
||||
},
|
||||
],
|
||||
default_reasoning_effort: ReasoningEffort::Medium,
|
||||
is_default: false,
|
||||
},
|
||||
Model {
|
||||
id: "gpt-5.2".to_string(),
|
||||
model: "gpt-5.2".to_string(),
|
||||
display_name: "gpt-5.2".to_string(),
|
||||
description:
|
||||
"Latest frontier model with improvements across knowledge, reasoning and coding"
|
||||
.to_string(),
|
||||
supported_reasoning_efforts: vec![
|
||||
ReasoningEffortOption {
|
||||
reasoning_effort: ReasoningEffort::Low,
|
||||
description: "Fast responses with lighter reasoning".to_string(),
|
||||
description: "Balances speed with some reasoning; useful for straightforward \
|
||||
queries and short explanations"
|
||||
.to_string(),
|
||||
},
|
||||
ReasoningEffortOption {
|
||||
reasoning_effort: ReasoningEffort::Medium,
|
||||
description: "Balances speed and reasoning depth for everyday tasks"
|
||||
description: "Provides a solid balance of reasoning depth and latency for \
|
||||
general-purpose tasks"
|
||||
.to_string(),
|
||||
},
|
||||
ReasoningEffortOption {
|
||||
reasoning_effort: ReasoningEffort::High,
|
||||
description: "Greater reasoning depth for complex problems".to_string(),
|
||||
description: "Maximizes reasoning depth for complex or ambiguous problems"
|
||||
.to_string(),
|
||||
},
|
||||
ReasoningEffortOption {
|
||||
reasoning_effort: ReasoningEffort::XHigh,
|
||||
@@ -187,7 +187,7 @@ async fn list_models_pagination_works() -> Result<()> {
|
||||
} = to_response::<ModelListResponse>(first_response)?;
|
||||
|
||||
assert_eq!(first_items.len(), 1);
|
||||
assert_eq!(first_items[0].id, "gpt-5.2");
|
||||
assert_eq!(first_items[0].id, "gpt-5.2-codex");
|
||||
let next_cursor = first_cursor.ok_or_else(|| anyhow!("cursor for second page"))?;
|
||||
|
||||
let second_request = mcp
|
||||
@@ -209,7 +209,7 @@ async fn list_models_pagination_works() -> Result<()> {
|
||||
} = to_response::<ModelListResponse>(second_response)?;
|
||||
|
||||
assert_eq!(second_items.len(), 1);
|
||||
assert_eq!(second_items[0].id, "gpt-5.1-codex-mini");
|
||||
assert_eq!(second_items[0].id, "gpt-5.1-codex-max");
|
||||
let third_cursor = second_cursor.ok_or_else(|| anyhow!("cursor for third page"))?;
|
||||
|
||||
let third_request = mcp
|
||||
@@ -231,7 +231,7 @@ async fn list_models_pagination_works() -> Result<()> {
|
||||
} = to_response::<ModelListResponse>(third_response)?;
|
||||
|
||||
assert_eq!(third_items.len(), 1);
|
||||
assert_eq!(third_items[0].id, "gpt-5.1-codex-max");
|
||||
assert_eq!(third_items[0].id, "gpt-5.1-codex-mini");
|
||||
let fourth_cursor = third_cursor.ok_or_else(|| anyhow!("cursor for fourth page"))?;
|
||||
|
||||
let fourth_request = mcp
|
||||
@@ -253,7 +253,7 @@ async fn list_models_pagination_works() -> Result<()> {
|
||||
} = to_response::<ModelListResponse>(fourth_response)?;
|
||||
|
||||
assert_eq!(fourth_items.len(), 1);
|
||||
assert_eq!(fourth_items[0].id, "gpt-5.2-codex");
|
||||
assert_eq!(fourth_items[0].id, "gpt-5.2");
|
||||
assert!(fourth_cursor.is_none());
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -8,9 +8,10 @@ use app_test_support::create_mock_chat_completions_server_unchecked;
|
||||
use app_test_support::create_shell_command_sse_response;
|
||||
use app_test_support::format_with_current_shell_display;
|
||||
use app_test_support::to_response;
|
||||
use codex_app_server_protocol::ApprovalDecision;
|
||||
use codex_app_server_protocol::CommandExecutionApprovalDecision;
|
||||
use codex_app_server_protocol::CommandExecutionRequestApprovalResponse;
|
||||
use codex_app_server_protocol::CommandExecutionStatus;
|
||||
use codex_app_server_protocol::FileChangeApprovalDecision;
|
||||
use codex_app_server_protocol::FileChangeOutputDeltaNotification;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalResponse;
|
||||
use codex_app_server_protocol::ItemCompletedNotification;
|
||||
@@ -426,7 +427,7 @@ async fn turn_start_exec_approval_decline_v2() -> Result<()> {
|
||||
mcp.send_response(
|
||||
request_id,
|
||||
serde_json::to_value(CommandExecutionRequestApprovalResponse {
|
||||
decision: ApprovalDecision::Decline,
|
||||
decision: CommandExecutionApprovalDecision::Decline,
|
||||
})?,
|
||||
)
|
||||
.await?;
|
||||
@@ -722,7 +723,7 @@ async fn turn_start_file_change_approval_v2() -> Result<()> {
|
||||
mcp.send_response(
|
||||
request_id,
|
||||
serde_json::to_value(FileChangeRequestApprovalResponse {
|
||||
decision: ApprovalDecision::Accept,
|
||||
decision: FileChangeApprovalDecision::Accept,
|
||||
})?,
|
||||
)
|
||||
.await?;
|
||||
@@ -782,6 +783,190 @@ async fn turn_start_file_change_approval_v2() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_start_file_change_approval_accept_for_session_persists_v2() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let tmp = TempDir::new()?;
|
||||
let codex_home = tmp.path().join("codex_home");
|
||||
std::fs::create_dir(&codex_home)?;
|
||||
let workspace = tmp.path().join("workspace");
|
||||
std::fs::create_dir(&workspace)?;
|
||||
|
||||
let patch_1 = r#"*** Begin Patch
|
||||
*** Add File: README.md
|
||||
+new line
|
||||
*** End Patch
|
||||
"#;
|
||||
let patch_2 = r#"*** Begin Patch
|
||||
*** Update File: README.md
|
||||
@@
|
||||
-new line
|
||||
+updated line
|
||||
*** End Patch
|
||||
"#;
|
||||
|
||||
let responses = vec![
|
||||
create_apply_patch_sse_response(patch_1, "patch-call-1")?,
|
||||
create_final_assistant_message_sse_response("patch 1 applied")?,
|
||||
create_apply_patch_sse_response(patch_2, "patch-call-2")?,
|
||||
create_final_assistant_message_sse_response("patch 2 applied")?,
|
||||
];
|
||||
let server = create_mock_chat_completions_server(responses).await;
|
||||
create_config_toml(&codex_home, &server.uri(), "untrusted")?;
|
||||
|
||||
let mut mcp = McpProcess::new(&codex_home).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let start_req = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
model: Some("mock-model".to_string()),
|
||||
cwd: Some(workspace.to_string_lossy().into_owned()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let start_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(start_req)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(start_resp)?;
|
||||
|
||||
// First turn: expect FileChangeRequestApproval, respond with AcceptForSession, and verify the file exists.
|
||||
let turn_1_req = mcp
|
||||
.send_turn_start_request(TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
input: vec![V2UserInput::Text {
|
||||
text: "apply patch 1".into(),
|
||||
}],
|
||||
cwd: Some(workspace.clone()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let turn_1_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(turn_1_req)),
|
||||
)
|
||||
.await??;
|
||||
let TurnStartResponse { turn: turn_1 } = to_response::<TurnStartResponse>(turn_1_resp)?;
|
||||
|
||||
let started_file_change_1 = timeout(DEFAULT_READ_TIMEOUT, async {
|
||||
loop {
|
||||
let started_notif = mcp
|
||||
.read_stream_until_notification_message("item/started")
|
||||
.await?;
|
||||
let started: ItemStartedNotification =
|
||||
serde_json::from_value(started_notif.params.clone().expect("item/started params"))?;
|
||||
if let ThreadItem::FileChange { .. } = started.item {
|
||||
return Ok::<ThreadItem, anyhow::Error>(started.item);
|
||||
}
|
||||
}
|
||||
})
|
||||
.await??;
|
||||
let ThreadItem::FileChange { id, status, .. } = started_file_change_1 else {
|
||||
unreachable!("loop ensures we break on file change items");
|
||||
};
|
||||
assert_eq!(id, "patch-call-1");
|
||||
assert_eq!(status, PatchApplyStatus::InProgress);
|
||||
|
||||
let server_req = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_request_message(),
|
||||
)
|
||||
.await??;
|
||||
let ServerRequest::FileChangeRequestApproval { request_id, params } = server_req else {
|
||||
panic!("expected FileChangeRequestApproval request")
|
||||
};
|
||||
assert_eq!(params.item_id, "patch-call-1");
|
||||
assert_eq!(params.thread_id, thread.id);
|
||||
assert_eq!(params.turn_id, turn_1.id);
|
||||
|
||||
mcp.send_response(
|
||||
request_id,
|
||||
serde_json::to_value(FileChangeRequestApprovalResponse {
|
||||
decision: FileChangeApprovalDecision::AcceptForSession,
|
||||
})?,
|
||||
)
|
||||
.await?;
|
||||
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("item/fileChange/outputDelta"),
|
||||
)
|
||||
.await??;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("item/completed"),
|
||||
)
|
||||
.await??;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("codex/event/task_complete"),
|
||||
)
|
||||
.await??;
|
||||
|
||||
let readme_path = workspace.join("README.md");
|
||||
assert_eq!(std::fs::read_to_string(&readme_path)?, "new line\n");
|
||||
|
||||
// Second turn: apply a patch to the same file. Approval should be skipped due to AcceptForSession.
|
||||
let turn_2_req = mcp
|
||||
.send_turn_start_request(TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
input: vec![V2UserInput::Text {
|
||||
text: "apply patch 2".into(),
|
||||
}],
|
||||
cwd: Some(workspace.clone()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(turn_2_req)),
|
||||
)
|
||||
.await??;
|
||||
|
||||
let started_file_change_2 = timeout(DEFAULT_READ_TIMEOUT, async {
|
||||
loop {
|
||||
let started_notif = mcp
|
||||
.read_stream_until_notification_message("item/started")
|
||||
.await?;
|
||||
let started: ItemStartedNotification =
|
||||
serde_json::from_value(started_notif.params.clone().expect("item/started params"))?;
|
||||
if let ThreadItem::FileChange { .. } = started.item {
|
||||
return Ok::<ThreadItem, anyhow::Error>(started.item);
|
||||
}
|
||||
}
|
||||
})
|
||||
.await??;
|
||||
let ThreadItem::FileChange { id, status, .. } = started_file_change_2 else {
|
||||
unreachable!("loop ensures we break on file change items");
|
||||
};
|
||||
assert_eq!(id, "patch-call-2");
|
||||
assert_eq!(status, PatchApplyStatus::InProgress);
|
||||
|
||||
// If the server incorrectly emits FileChangeRequestApproval, the helper below will error
|
||||
// (it bails on unexpected JSONRPCMessage::Request), causing the test to fail.
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("item/fileChange/outputDelta"),
|
||||
)
|
||||
.await??;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("item/completed"),
|
||||
)
|
||||
.await??;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("codex/event/task_complete"),
|
||||
)
|
||||
.await??;
|
||||
|
||||
assert_eq!(std::fs::read_to_string(readme_path)?, "updated line\n");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn turn_start_file_change_approval_decline_v2() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -888,7 +1073,7 @@ async fn turn_start_file_change_approval_decline_v2() -> Result<()> {
|
||||
mcp.send_response(
|
||||
request_id,
|
||||
serde_json::to_value(FileChangeRequestApprovalResponse {
|
||||
decision: ApprovalDecision::Decline,
|
||||
decision: FileChangeApprovalDecision::Decline,
|
||||
})?,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -10,6 +10,7 @@ use crate::provider::WireApi;
|
||||
use crate::sse::chat::spawn_chat_stream;
|
||||
use crate::telemetry::SseTelemetry;
|
||||
use codex_client::HttpTransport;
|
||||
use codex_client::RequestCompression;
|
||||
use codex_client::RequestTelemetry;
|
||||
use codex_protocol::models::ContentItem;
|
||||
use codex_protocol::models::ReasoningItemContent;
|
||||
@@ -80,7 +81,13 @@ impl<T: HttpTransport, A: AuthProvider> ChatClient<T, A> {
|
||||
extra_headers: HeaderMap,
|
||||
) -> Result<ResponseStream, ApiError> {
|
||||
self.streaming
|
||||
.stream(self.path(), body, extra_headers, spawn_chat_stream)
|
||||
.stream(
|
||||
self.path(),
|
||||
body,
|
||||
extra_headers,
|
||||
RequestCompression::None,
|
||||
spawn_chat_stream,
|
||||
)
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
@@ -9,9 +9,11 @@ use crate::provider::Provider;
|
||||
use crate::provider::WireApi;
|
||||
use crate::requests::ResponsesRequest;
|
||||
use crate::requests::ResponsesRequestBuilder;
|
||||
use crate::requests::responses::Compression;
|
||||
use crate::sse::spawn_response_stream;
|
||||
use crate::telemetry::SseTelemetry;
|
||||
use codex_client::HttpTransport;
|
||||
use codex_client::RequestCompression;
|
||||
use codex_client::RequestTelemetry;
|
||||
use codex_protocol::protocol::SessionSource;
|
||||
use http::HeaderMap;
|
||||
@@ -33,6 +35,7 @@ pub struct ResponsesOptions {
|
||||
pub conversation_id: Option<String>,
|
||||
pub session_source: Option<SessionSource>,
|
||||
pub extra_headers: HeaderMap,
|
||||
pub compression: Compression,
|
||||
}
|
||||
|
||||
impl<T: HttpTransport, A: AuthProvider> ResponsesClient<T, A> {
|
||||
@@ -56,7 +59,8 @@ impl<T: HttpTransport, A: AuthProvider> ResponsesClient<T, A> {
|
||||
&self,
|
||||
request: ResponsesRequest,
|
||||
) -> Result<ResponseStream, ApiError> {
|
||||
self.stream(request.body, request.headers).await
|
||||
self.stream(request.body, request.headers, request.compression)
|
||||
.await
|
||||
}
|
||||
|
||||
#[instrument(level = "trace", skip_all, err)]
|
||||
@@ -75,6 +79,7 @@ impl<T: HttpTransport, A: AuthProvider> ResponsesClient<T, A> {
|
||||
conversation_id,
|
||||
session_source,
|
||||
extra_headers,
|
||||
compression,
|
||||
} = options;
|
||||
|
||||
let request = ResponsesRequestBuilder::new(model, &prompt.instructions, &prompt.input)
|
||||
@@ -88,6 +93,7 @@ impl<T: HttpTransport, A: AuthProvider> ResponsesClient<T, A> {
|
||||
.session_source(session_source)
|
||||
.store_override(store_override)
|
||||
.extra_headers(extra_headers)
|
||||
.compression(compression)
|
||||
.build(self.streaming.provider())?;
|
||||
|
||||
self.stream_request(request).await
|
||||
@@ -104,9 +110,21 @@ impl<T: HttpTransport, A: AuthProvider> ResponsesClient<T, A> {
|
||||
&self,
|
||||
body: Value,
|
||||
extra_headers: HeaderMap,
|
||||
compression: Compression,
|
||||
) -> Result<ResponseStream, ApiError> {
|
||||
let compression = match compression {
|
||||
Compression::None => RequestCompression::None,
|
||||
Compression::Zstd => RequestCompression::Zstd,
|
||||
};
|
||||
|
||||
self.streaming
|
||||
.stream(self.path(), body, extra_headers, spawn_response_stream)
|
||||
.stream(
|
||||
self.path(),
|
||||
body,
|
||||
extra_headers,
|
||||
compression,
|
||||
spawn_response_stream,
|
||||
)
|
||||
.await
|
||||
}
|
||||
}
|
||||
|
||||
@@ -6,6 +6,7 @@ use crate::provider::Provider;
|
||||
use crate::telemetry::SseTelemetry;
|
||||
use crate::telemetry::run_with_request_telemetry;
|
||||
use codex_client::HttpTransport;
|
||||
use codex_client::RequestCompression;
|
||||
use codex_client::RequestTelemetry;
|
||||
use codex_client::StreamResponse;
|
||||
use http::HeaderMap;
|
||||
@@ -52,6 +53,7 @@ impl<T: HttpTransport, A: AuthProvider> StreamingClient<T, A> {
|
||||
path: &str,
|
||||
body: Value,
|
||||
extra_headers: HeaderMap,
|
||||
compression: RequestCompression,
|
||||
spawner: fn(StreamResponse, Duration, Option<Arc<dyn SseTelemetry>>) -> ResponseStream,
|
||||
) -> Result<ResponseStream, ApiError> {
|
||||
let builder = || {
|
||||
@@ -62,6 +64,7 @@ impl<T: HttpTransport, A: AuthProvider> StreamingClient<T, A> {
|
||||
http::HeaderValue::from_static("text/event-stream"),
|
||||
);
|
||||
req.body = Some(body.clone());
|
||||
req.compression = compression;
|
||||
add_auth_headers(&self.auth, req)
|
||||
};
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use codex_client::Request;
|
||||
use codex_client::RequestCompression;
|
||||
use codex_client::RetryOn;
|
||||
use codex_client::RetryPolicy;
|
||||
use http::Method;
|
||||
@@ -87,6 +88,7 @@ impl Provider {
|
||||
url: self.url_for_path(path),
|
||||
headers: self.headers.clone(),
|
||||
body: None,
|
||||
compression: RequestCompression::None,
|
||||
timeout: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -11,10 +11,18 @@ use codex_protocol::protocol::SessionSource;
|
||||
use http::HeaderMap;
|
||||
use serde_json::Value;
|
||||
|
||||
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
|
||||
pub enum Compression {
|
||||
#[default]
|
||||
None,
|
||||
Zstd,
|
||||
}
|
||||
|
||||
/// Assembled request body plus headers for a Responses stream request.
|
||||
pub struct ResponsesRequest {
|
||||
pub body: Value,
|
||||
pub headers: HeaderMap,
|
||||
pub compression: Compression,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
@@ -32,6 +40,7 @@ pub struct ResponsesRequestBuilder<'a> {
|
||||
session_source: Option<SessionSource>,
|
||||
store_override: Option<bool>,
|
||||
headers: HeaderMap,
|
||||
compression: Compression,
|
||||
}
|
||||
|
||||
impl<'a> ResponsesRequestBuilder<'a> {
|
||||
@@ -94,6 +103,11 @@ impl<'a> ResponsesRequestBuilder<'a> {
|
||||
self
|
||||
}
|
||||
|
||||
pub fn compression(mut self, compression: Compression) -> Self {
|
||||
self.compression = compression;
|
||||
self
|
||||
}
|
||||
|
||||
pub fn build(self, provider: &Provider) -> Result<ResponsesRequest, ApiError> {
|
||||
let model = self
|
||||
.model
|
||||
@@ -138,7 +152,11 @@ impl<'a> ResponsesRequestBuilder<'a> {
|
||||
insert_header(&mut headers, "x-openai-subagent", &subagent);
|
||||
}
|
||||
|
||||
Ok(ResponsesRequest { body, headers })
|
||||
Ok(ResponsesRequest {
|
||||
body,
|
||||
headers,
|
||||
compression: self.compression,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ use codex_api::Provider;
|
||||
use codex_api::ResponsesClient;
|
||||
use codex_api::ResponsesOptions;
|
||||
use codex_api::WireApi;
|
||||
use codex_api::requests::responses::Compression;
|
||||
use codex_client::HttpTransport;
|
||||
use codex_client::Request;
|
||||
use codex_client::Response;
|
||||
@@ -229,7 +230,9 @@ async fn responses_client_uses_responses_path_for_responses_wire() -> Result<()>
|
||||
let client = ResponsesClient::new(transport, provider("openai", WireApi::Responses), NoAuth);
|
||||
|
||||
let body = serde_json::json!({ "echo": true });
|
||||
let _stream = client.stream(body, HeaderMap::new()).await?;
|
||||
let _stream = client
|
||||
.stream(body, HeaderMap::new(), Compression::None)
|
||||
.await?;
|
||||
|
||||
let requests = state.take_stream_requests();
|
||||
assert_path_ends_with(&requests, "/responses");
|
||||
@@ -243,7 +246,9 @@ async fn responses_client_uses_chat_path_for_chat_wire() -> Result<()> {
|
||||
let client = ResponsesClient::new(transport, provider("openai", WireApi::Chat), NoAuth);
|
||||
|
||||
let body = serde_json::json!({ "echo": true });
|
||||
let _stream = client.stream(body, HeaderMap::new()).await?;
|
||||
let _stream = client
|
||||
.stream(body, HeaderMap::new(), Compression::None)
|
||||
.await?;
|
||||
|
||||
let requests = state.take_stream_requests();
|
||||
assert_path_ends_with(&requests, "/chat/completions");
|
||||
@@ -258,7 +263,9 @@ async fn streaming_client_adds_auth_headers() -> Result<()> {
|
||||
let client = ResponsesClient::new(transport, provider("openai", WireApi::Responses), auth);
|
||||
|
||||
let body = serde_json::json!({ "model": "gpt-test" });
|
||||
let _stream = client.stream(body, HeaderMap::new()).await?;
|
||||
let _stream = client
|
||||
.stream(body, HeaderMap::new(), Compression::None)
|
||||
.await?;
|
||||
|
||||
let requests = state.take_stream_requests();
|
||||
assert_eq!(requests.len(), 1);
|
||||
|
||||
@@ -9,6 +9,7 @@ use codex_api::Provider;
|
||||
use codex_api::ResponseEvent;
|
||||
use codex_api::ResponsesClient;
|
||||
use codex_api::WireApi;
|
||||
use codex_api::requests::responses::Compression;
|
||||
use codex_client::HttpTransport;
|
||||
use codex_client::Request;
|
||||
use codex_client::Response;
|
||||
@@ -124,7 +125,11 @@ async fn responses_stream_parses_items_and_completed_end_to_end() -> Result<()>
|
||||
let client = ResponsesClient::new(transport, provider("openai", WireApi::Responses), NoAuth);
|
||||
|
||||
let mut stream = client
|
||||
.stream(serde_json::json!({"echo": true}), HeaderMap::new())
|
||||
.stream(
|
||||
serde_json::json!({"echo": true}),
|
||||
HeaderMap::new(),
|
||||
Compression::None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let mut events = Vec::new();
|
||||
@@ -189,7 +194,11 @@ async fn responses_stream_aggregates_output_text_deltas() -> Result<()> {
|
||||
let client = ResponsesClient::new(transport, provider("openai", WireApi::Responses), NoAuth);
|
||||
|
||||
let stream = client
|
||||
.stream(serde_json::json!({"echo": true}), HeaderMap::new())
|
||||
.stream(
|
||||
serde_json::json!({"echo": true}),
|
||||
HeaderMap::new(),
|
||||
Compression::None,
|
||||
)
|
||||
.await?;
|
||||
|
||||
let mut stream = stream.aggregate();
|
||||
|
||||
@@ -19,6 +19,7 @@ thiserror = { workspace = true }
|
||||
tokio = { workspace = true, features = ["macros", "rt", "time", "sync"] }
|
||||
tracing = { workspace = true }
|
||||
tracing-opentelemetry = { workspace = true }
|
||||
zstd = { workspace = true }
|
||||
|
||||
[lints]
|
||||
workspace = true
|
||||
|
||||
@@ -104,6 +104,13 @@ impl CodexRequestBuilder {
|
||||
self.map(|builder| builder.json(value))
|
||||
}
|
||||
|
||||
pub fn body<B>(self, body: B) -> Self
|
||||
where
|
||||
B: Into<reqwest::Body>,
|
||||
{
|
||||
self.map(|builder| builder.body(body))
|
||||
}
|
||||
|
||||
pub async fn send(self) -> Result<Response, reqwest::Error> {
|
||||
let headers = trace_headers();
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ pub use crate::default_client::CodexRequestBuilder;
|
||||
pub use crate::error::StreamError;
|
||||
pub use crate::error::TransportError;
|
||||
pub use crate::request::Request;
|
||||
pub use crate::request::RequestCompression;
|
||||
pub use crate::request::Response;
|
||||
pub use crate::retry::RetryOn;
|
||||
pub use crate::retry::RetryPolicy;
|
||||
|
||||
@@ -5,12 +5,20 @@ use serde::Serialize;
|
||||
use serde_json::Value;
|
||||
use std::time::Duration;
|
||||
|
||||
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)]
|
||||
pub enum RequestCompression {
|
||||
#[default]
|
||||
None,
|
||||
Zstd,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct Request {
|
||||
pub method: Method,
|
||||
pub url: String,
|
||||
pub headers: HeaderMap,
|
||||
pub body: Option<Value>,
|
||||
pub compression: RequestCompression,
|
||||
pub timeout: Option<Duration>,
|
||||
}
|
||||
|
||||
@@ -21,6 +29,7 @@ impl Request {
|
||||
url,
|
||||
headers: HeaderMap::new(),
|
||||
body: None,
|
||||
compression: RequestCompression::None,
|
||||
timeout: None,
|
||||
}
|
||||
}
|
||||
@@ -29,6 +38,11 @@ impl Request {
|
||||
self.body = serde_json::to_value(body).ok();
|
||||
self
|
||||
}
|
||||
|
||||
pub fn with_compression(mut self, compression: RequestCompression) -> Self {
|
||||
self.compression = compression;
|
||||
self
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
|
||||
@@ -2,6 +2,7 @@ use crate::default_client::CodexHttpClient;
|
||||
use crate::default_client::CodexRequestBuilder;
|
||||
use crate::error::TransportError;
|
||||
use crate::request::Request;
|
||||
use crate::request::RequestCompression;
|
||||
use crate::request::Response;
|
||||
use async_trait::async_trait;
|
||||
use bytes::Bytes;
|
||||
@@ -41,18 +42,70 @@ impl ReqwestTransport {
|
||||
}
|
||||
|
||||
fn build(&self, req: Request) -> Result<CodexRequestBuilder, TransportError> {
|
||||
let mut builder = self
|
||||
.client
|
||||
.request(
|
||||
Method::from_bytes(req.method.as_str().as_bytes()).unwrap_or(Method::GET),
|
||||
&req.url,
|
||||
)
|
||||
.headers(req.headers);
|
||||
if let Some(timeout) = req.timeout {
|
||||
let Request {
|
||||
method,
|
||||
url,
|
||||
mut headers,
|
||||
body,
|
||||
compression,
|
||||
timeout,
|
||||
} = req;
|
||||
|
||||
let mut builder = self.client.request(
|
||||
Method::from_bytes(method.as_str().as_bytes()).unwrap_or(Method::GET),
|
||||
&url,
|
||||
);
|
||||
|
||||
if let Some(timeout) = timeout {
|
||||
builder = builder.timeout(timeout);
|
||||
}
|
||||
if let Some(body) = req.body {
|
||||
builder = builder.json(&body);
|
||||
|
||||
if let Some(body) = body {
|
||||
if compression != RequestCompression::None {
|
||||
if headers.contains_key(http::header::CONTENT_ENCODING) {
|
||||
return Err(TransportError::Build(
|
||||
"request compression was requested but content-encoding is already set"
|
||||
.to_string(),
|
||||
));
|
||||
}
|
||||
|
||||
let json = serde_json::to_vec(&body)
|
||||
.map_err(|err| TransportError::Build(err.to_string()))?;
|
||||
let pre_compression_bytes = json.len();
|
||||
let compression_start = std::time::Instant::now();
|
||||
let (compressed, content_encoding) = match compression {
|
||||
RequestCompression::None => unreachable!("guarded by compression != None"),
|
||||
RequestCompression::Zstd => (
|
||||
zstd::stream::encode_all(std::io::Cursor::new(json), 3)
|
||||
.map_err(|err| TransportError::Build(err.to_string()))?,
|
||||
http::HeaderValue::from_static("zstd"),
|
||||
),
|
||||
};
|
||||
let post_compression_bytes = compressed.len();
|
||||
let compression_duration = compression_start.elapsed();
|
||||
|
||||
// Ensure the server knows to unpack the request body.
|
||||
headers.insert(http::header::CONTENT_ENCODING, content_encoding);
|
||||
if !headers.contains_key(http::header::CONTENT_TYPE) {
|
||||
headers.insert(
|
||||
http::header::CONTENT_TYPE,
|
||||
http::HeaderValue::from_static("application/json"),
|
||||
);
|
||||
}
|
||||
|
||||
tracing::info!(
|
||||
pre_compression_bytes,
|
||||
post_compression_bytes,
|
||||
compression_duration_ms = compression_duration.as_millis(),
|
||||
"Compressed request body with zstd"
|
||||
);
|
||||
|
||||
builder = builder.headers(headers).body(compressed);
|
||||
} else {
|
||||
builder = builder.headers(headers).json(&body);
|
||||
}
|
||||
} else {
|
||||
builder = builder.headers(headers);
|
||||
}
|
||||
Ok(builder)
|
||||
}
|
||||
|
||||
@@ -137,6 +137,7 @@ tracing-subscriber = { workspace = true }
|
||||
tracing-test = { workspace = true, features = ["no-env-filter"] }
|
||||
walkdir = { workspace = true }
|
||||
wiremock = { workspace = true }
|
||||
zstd = { workspace = true }
|
||||
|
||||
[package.metadata.cargo-shear]
|
||||
ignored = ["openssl-sys"]
|
||||
|
||||
@@ -1,10 +1,9 @@
|
||||
use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::protocol::FileChange;
|
||||
use crate::protocol::ReviewDecision;
|
||||
use crate::safety::SafetyCheck;
|
||||
use crate::safety::assess_patch_safety;
|
||||
use crate::tools::sandboxing::ExecApprovalRequirement;
|
||||
use codex_apply_patch::ApplyPatchAction;
|
||||
use codex_apply_patch::ApplyPatchFileChange;
|
||||
use std::collections::HashMap;
|
||||
@@ -30,13 +29,12 @@ pub(crate) enum InternalApplyPatchInvocation {
|
||||
#[derive(Debug)]
|
||||
pub(crate) struct ApplyPatchExec {
|
||||
pub(crate) action: ApplyPatchAction,
|
||||
pub(crate) user_explicitly_approved_this_action: bool,
|
||||
pub(crate) auto_approved: bool,
|
||||
pub(crate) exec_approval_requirement: ExecApprovalRequirement,
|
||||
}
|
||||
|
||||
pub(crate) async fn apply_patch(
|
||||
sess: &Session,
|
||||
turn_context: &TurnContext,
|
||||
call_id: &str,
|
||||
action: ApplyPatchAction,
|
||||
) -> InternalApplyPatchInvocation {
|
||||
match assess_patch_safety(
|
||||
@@ -50,40 +48,24 @@ pub(crate) async fn apply_patch(
|
||||
..
|
||||
} => InternalApplyPatchInvocation::DelegateToExec(ApplyPatchExec {
|
||||
action,
|
||||
user_explicitly_approved_this_action: user_explicitly_approved,
|
||||
auto_approved: !user_explicitly_approved,
|
||||
exec_approval_requirement: ExecApprovalRequirement::Skip {
|
||||
bypass_sandbox: false,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
}),
|
||||
SafetyCheck::AskUser => {
|
||||
// Compute a readable summary of path changes to include in the
|
||||
// approval request so the user can make an informed decision.
|
||||
//
|
||||
// Note that it might be worth expanding this approval request to
|
||||
// give the user the option to expand the set of writable roots so
|
||||
// that similar patches can be auto-approved in the future during
|
||||
// this session.
|
||||
let rx_approve = sess
|
||||
.request_patch_approval(
|
||||
turn_context,
|
||||
call_id.to_owned(),
|
||||
convert_apply_patch_to_protocol(&action),
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
match rx_approve.await.unwrap_or_default() {
|
||||
ReviewDecision::Approved
|
||||
| ReviewDecision::ApprovedExecpolicyAmendment { .. }
|
||||
| ReviewDecision::ApprovedForSession => {
|
||||
InternalApplyPatchInvocation::DelegateToExec(ApplyPatchExec {
|
||||
action,
|
||||
user_explicitly_approved_this_action: true,
|
||||
})
|
||||
}
|
||||
ReviewDecision::Denied | ReviewDecision::Abort => {
|
||||
InternalApplyPatchInvocation::Output(Err(FunctionCallError::RespondToModel(
|
||||
"patch rejected by user".to_string(),
|
||||
)))
|
||||
}
|
||||
}
|
||||
// Delegate the approval prompt (including cached approvals) to the
|
||||
// tool runtime, consistent with how shell/unified_exec approvals
|
||||
// are orchestrator-driven.
|
||||
InternalApplyPatchInvocation::DelegateToExec(ApplyPatchExec {
|
||||
action,
|
||||
auto_approved: false,
|
||||
exec_approval_requirement: ExecApprovalRequirement::NeedsApproval {
|
||||
reason: None,
|
||||
proposed_execpolicy_amendment: None,
|
||||
},
|
||||
})
|
||||
}
|
||||
SafetyCheck::Reject { reason } => InternalApplyPatchInvocation::Output(Err(
|
||||
FunctionCallError::RespondToModel(format!("patch rejected: {reason}")),
|
||||
|
||||
@@ -17,6 +17,7 @@ use codex_api::TransportError;
|
||||
use codex_api::common::Reasoning;
|
||||
use codex_api::create_text_param_for_request;
|
||||
use codex_api::error::ApiError;
|
||||
use codex_api::requests::responses::Compression;
|
||||
use codex_app_server_protocol::AuthMode;
|
||||
use codex_otel::otel_manager::OtelManager;
|
||||
use codex_protocol::ThreadId;
|
||||
@@ -47,6 +48,7 @@ use crate::default_client::build_reqwest_client;
|
||||
use crate::error::CodexErr;
|
||||
use crate::error::Result;
|
||||
use crate::features::FEATURES;
|
||||
use crate::features::Feature;
|
||||
use crate::flags::CODEX_RS_SSE_FIXTURE;
|
||||
use crate::model_provider_info::ModelProviderInfo;
|
||||
use crate::model_provider_info::WireApi;
|
||||
@@ -250,6 +252,20 @@ impl ModelClient {
|
||||
let api_auth = auth_provider_from_auth(auth.clone(), &self.provider).await?;
|
||||
let transport = ReqwestTransport::new(build_reqwest_client());
|
||||
let (request_telemetry, sse_telemetry) = self.build_streaming_telemetry();
|
||||
let compression = if self
|
||||
.config
|
||||
.features
|
||||
.enabled(Feature::EnableRequestCompression)
|
||||
&& auth
|
||||
.as_ref()
|
||||
.is_some_and(|auth| auth.mode == AuthMode::ChatGPT)
|
||||
&& self.provider.is_openai()
|
||||
{
|
||||
Compression::Zstd
|
||||
} else {
|
||||
Compression::None
|
||||
};
|
||||
|
||||
let client = ApiResponsesClient::new(transport, api_provider, api_auth)
|
||||
.with_telemetry(Some(request_telemetry), Some(sse_telemetry));
|
||||
|
||||
@@ -262,6 +278,7 @@ impl ModelClient {
|
||||
conversation_id: Some(conversation_id.clone()),
|
||||
session_source: Some(session_source.clone()),
|
||||
extra_headers: beta_feature_headers(&self.config),
|
||||
compression,
|
||||
};
|
||||
|
||||
let stream_result = client
|
||||
|
||||
+71
-111
@@ -226,29 +226,22 @@ impl Codex {
|
||||
let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY);
|
||||
let (tx_event, rx_event) = async_channel::unbounded();
|
||||
|
||||
let loaded_skills = if config.features.enabled(Feature::Skills) {
|
||||
Some(skills_manager.skills_for_config(&config))
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let loaded_skills = skills_manager.skills_for_config(&config);
|
||||
// let loaded_skills = if config.features.enabled(Feature::Skills) {
|
||||
// Some(skills_manager.skills_for_config(&config))
|
||||
// } else {
|
||||
// None
|
||||
// };
|
||||
|
||||
if let Some(outcome) = &loaded_skills {
|
||||
for err in &outcome.errors {
|
||||
error!(
|
||||
"failed to load skill {}: {}",
|
||||
err.path.display(),
|
||||
err.message
|
||||
);
|
||||
}
|
||||
for err in &loaded_skills.errors {
|
||||
error!(
|
||||
"failed to load skill {}: {}",
|
||||
err.path.display(),
|
||||
err.message
|
||||
);
|
||||
}
|
||||
|
||||
let user_instructions = get_user_instructions(
|
||||
&config,
|
||||
loaded_skills
|
||||
.as_ref()
|
||||
.map(|outcome| outcome.skills.as_slice()),
|
||||
)
|
||||
.await;
|
||||
let user_instructions = get_user_instructions(&config, Some(&loaded_skills.skills)).await;
|
||||
|
||||
let exec_policy = ExecPolicyManager::load(&config.features, &config.config_layer_stack)
|
||||
.await
|
||||
@@ -551,10 +544,7 @@ impl Session {
|
||||
final_output_json_schema: None,
|
||||
codex_linux_sandbox_exe: per_turn_config.codex_linux_sandbox_exe.clone(),
|
||||
tool_call_gate: Arc::new(ReadinessFlag::new()),
|
||||
truncation_policy: TruncationPolicy::new(
|
||||
per_turn_config.as_ref(),
|
||||
model_info.truncation_policy.into(),
|
||||
),
|
||||
truncation_policy: model_info.truncation_policy.into(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1287,10 +1277,6 @@ impl Session {
|
||||
}
|
||||
|
||||
pub(crate) async fn record_model_warning(&self, message: impl Into<String>, ctx: &TurnContext) {
|
||||
if !self.enabled(Feature::ModelWarnings) {
|
||||
return;
|
||||
}
|
||||
|
||||
let item = ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
@@ -1747,7 +1733,7 @@ mod handlers {
|
||||
|
||||
use crate::codex::spawn_review_thread;
|
||||
use crate::config::Config;
|
||||
use crate::features::Feature;
|
||||
|
||||
use crate::mcp::auth::compute_auth_statuses;
|
||||
use crate::mcp::collect_mcp_snapshot_from_manager;
|
||||
use crate::review_prompts::resolve_review_request;
|
||||
@@ -2037,29 +2023,20 @@ mod handlers {
|
||||
} else {
|
||||
cwds
|
||||
};
|
||||
let skills = if sess.enabled(Feature::Skills) {
|
||||
let skills_manager = &sess.services.skills_manager;
|
||||
let mut entries = Vec::new();
|
||||
for cwd in cwds {
|
||||
let outcome = skills_manager.skills_for_cwd(&cwd, force_reload).await;
|
||||
let errors = super::errors_to_info(&outcome.errors);
|
||||
let skills = super::skills_to_info(&outcome.skills);
|
||||
entries.push(SkillsListEntry {
|
||||
cwd,
|
||||
skills,
|
||||
errors,
|
||||
});
|
||||
}
|
||||
entries
|
||||
} else {
|
||||
cwds.into_iter()
|
||||
.map(|cwd| SkillsListEntry {
|
||||
cwd,
|
||||
skills: Vec::new(),
|
||||
errors: Vec::new(),
|
||||
})
|
||||
.collect()
|
||||
};
|
||||
|
||||
let skills_manager = &sess.services.skills_manager;
|
||||
let mut skills = Vec::new();
|
||||
for cwd in cwds {
|
||||
let outcome = skills_manager.skills_for_cwd(&cwd, force_reload).await;
|
||||
let errors = super::errors_to_info(&outcome.errors);
|
||||
let skills_metadata = super::skills_to_info(&outcome.skills);
|
||||
skills.push(SkillsListEntry {
|
||||
cwd,
|
||||
skills: skills_metadata,
|
||||
errors,
|
||||
});
|
||||
}
|
||||
|
||||
let event = Event {
|
||||
id: sub_id,
|
||||
msg: EventMsg::ListSkillsResponse(ListSkillsResponseEvent { skills }),
|
||||
@@ -2212,8 +2189,7 @@ async fn spawn_review_thread(
|
||||
let mut review_features = sess.features.clone();
|
||||
review_features
|
||||
.disable(crate::features::Feature::WebSearchRequest)
|
||||
.disable(crate::features::Feature::WebSearchCached)
|
||||
.disable(crate::features::Feature::ViewImageTool);
|
||||
.disable(crate::features::Feature::WebSearchCached);
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &review_model_info,
|
||||
features: &review_features,
|
||||
@@ -2265,10 +2241,7 @@ async fn spawn_review_thread(
|
||||
final_output_json_schema: None,
|
||||
codex_linux_sandbox_exe: parent_turn_context.codex_linux_sandbox_exe.clone(),
|
||||
tool_call_gate: Arc::new(ReadinessFlag::new()),
|
||||
truncation_policy: TruncationPolicy::new(
|
||||
&per_turn_config,
|
||||
model_info.truncation_policy.into(),
|
||||
),
|
||||
truncation_policy: model_info.truncation_policy.into(),
|
||||
};
|
||||
|
||||
// Seed the child task with the review prompt as the initial user message.
|
||||
@@ -2345,16 +2318,12 @@ pub(crate) async fn run_task(
|
||||
});
|
||||
sess.send_event(&turn_context, event).await;
|
||||
|
||||
let skills_outcome = if sess.enabled(Feature::Skills) {
|
||||
Some(
|
||||
sess.services
|
||||
.skills_manager
|
||||
.skills_for_cwd(&turn_context.cwd, false)
|
||||
.await,
|
||||
)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let skills_outcome = Some(
|
||||
sess.services
|
||||
.skills_manager
|
||||
.skills_for_cwd(&turn_context.cwd, false)
|
||||
.await,
|
||||
);
|
||||
|
||||
let SkillInjections {
|
||||
items: skill_items,
|
||||
@@ -2519,14 +2488,14 @@ async fn run_turn(
|
||||
let prompt = Prompt {
|
||||
input,
|
||||
tools: router.specs(),
|
||||
parallel_tool_calls: model_supports_parallel && sess.enabled(Feature::ParallelToolCalls),
|
||||
parallel_tool_calls: model_supports_parallel,
|
||||
base_instructions_override: turn_context.base_instructions.clone(),
|
||||
output_schema: turn_context.final_output_json_schema.clone(),
|
||||
};
|
||||
|
||||
let mut retries = 0;
|
||||
loop {
|
||||
match try_run_turn(
|
||||
let err = match try_run_turn(
|
||||
Arc::clone(&router),
|
||||
Arc::clone(&sess),
|
||||
Arc::clone(&turn_context),
|
||||
@@ -2536,17 +2505,10 @@ async fn run_turn(
|
||||
)
|
||||
.await
|
||||
{
|
||||
// todo(aibrahim): map special cases and ? on other errors
|
||||
Ok(output) => return Ok(output),
|
||||
Err(CodexErr::TurnAborted) => {
|
||||
return Err(CodexErr::TurnAborted);
|
||||
}
|
||||
Err(CodexErr::Interrupted) => return Err(CodexErr::Interrupted),
|
||||
Err(CodexErr::EnvVar(var)) => return Err(CodexErr::EnvVar(var)),
|
||||
Err(e @ CodexErr::Fatal(_)) => return Err(e),
|
||||
Err(e @ CodexErr::ContextWindowExceeded) => {
|
||||
Err(CodexErr::ContextWindowExceeded) => {
|
||||
sess.set_total_tokens_full(&turn_context).await;
|
||||
return Err(e);
|
||||
return Err(CodexErr::ContextWindowExceeded);
|
||||
}
|
||||
Err(CodexErr::UsageLimitReached(e)) => {
|
||||
let rate_limits = e.rate_limits.clone();
|
||||
@@ -2555,39 +2517,38 @@ async fn run_turn(
|
||||
}
|
||||
return Err(CodexErr::UsageLimitReached(e));
|
||||
}
|
||||
Err(CodexErr::UsageNotIncluded) => return Err(CodexErr::UsageNotIncluded),
|
||||
Err(e @ CodexErr::QuotaExceeded) => return Err(e),
|
||||
Err(e @ CodexErr::InvalidImageRequest()) => return Err(e),
|
||||
Err(e @ CodexErr::InvalidRequest(_)) => return Err(e),
|
||||
Err(e @ CodexErr::RefreshTokenFailed(_)) => return Err(e),
|
||||
Err(e) => {
|
||||
// Use the configured provider-specific stream retry budget.
|
||||
let max_retries = turn_context.client.get_provider().stream_max_retries();
|
||||
if retries < max_retries {
|
||||
retries += 1;
|
||||
let delay = match e {
|
||||
CodexErr::Stream(_, Some(delay)) => delay,
|
||||
_ => backoff(retries),
|
||||
};
|
||||
warn!(
|
||||
"stream disconnected - retrying turn ({retries}/{max_retries} in {delay:?})...",
|
||||
);
|
||||
Err(err) => err,
|
||||
};
|
||||
|
||||
// Surface retry information to any UI/front‑end so the
|
||||
// user understands what is happening instead of staring
|
||||
// at a seemingly frozen screen.
|
||||
sess.notify_stream_error(
|
||||
&turn_context,
|
||||
format!("Reconnecting... {retries}/{max_retries}"),
|
||||
e,
|
||||
)
|
||||
.await;
|
||||
if !err.is_retryable() {
|
||||
return Err(err);
|
||||
}
|
||||
|
||||
tokio::time::sleep(delay).await;
|
||||
} else {
|
||||
return Err(e);
|
||||
// Use the configured provider-specific stream retry budget.
|
||||
let max_retries = turn_context.client.get_provider().stream_max_retries();
|
||||
if retries < max_retries {
|
||||
retries += 1;
|
||||
let delay = match &err {
|
||||
CodexErr::Stream(_, requested_delay) => {
|
||||
requested_delay.unwrap_or_else(|| backoff(retries))
|
||||
}
|
||||
}
|
||||
_ => backoff(retries),
|
||||
};
|
||||
warn!("stream disconnected - retrying turn ({retries}/{max_retries} in {delay:?})...",);
|
||||
|
||||
// Surface retry information to any UI/front‑end so the
|
||||
// user understands what is happening instead of staring
|
||||
// at a seemingly frozen screen.
|
||||
sess.notify_stream_error(
|
||||
&turn_context,
|
||||
format!("Reconnecting... {retries}/{max_retries}"),
|
||||
err,
|
||||
)
|
||||
.await;
|
||||
|
||||
tokio::time::sleep(delay).await;
|
||||
} else {
|
||||
return Err(err);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -3656,8 +3617,7 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn record_model_warning_appends_user_message() {
|
||||
let (mut session, turn_context) = make_session_and_context().await;
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::ModelWarnings);
|
||||
let features = Features::with_defaults();
|
||||
session.features = features;
|
||||
|
||||
session
|
||||
|
||||
@@ -268,7 +268,6 @@ pub struct Config {
|
||||
/// Additional filenames to try when looking for project-level docs.
|
||||
pub project_doc_fallback_filenames: Vec<String>,
|
||||
|
||||
// todo(aibrahim): this should be used in the override model info
|
||||
/// Token budget applied when storing tool/function outputs in the context manager.
|
||||
pub tool_output_token_limit: Option<usize>,
|
||||
|
||||
@@ -1883,7 +1882,7 @@ trust_level = "trusted"
|
||||
profiles.insert(
|
||||
"work".to_string(),
|
||||
ConfigProfile {
|
||||
tools_view_image: Some(false),
|
||||
tools_web_search: Some(false),
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
@@ -1899,7 +1898,7 @@ trust_level = "trusted"
|
||||
codex_home.path().to_path_buf(),
|
||||
)?;
|
||||
|
||||
assert!(!config.features.enabled(Feature::ViewImageTool));
|
||||
assert!(!config.features.enabled(Feature::WebSearchRequest));
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@ use crate::config::edit::ConfigEdit;
|
||||
use crate::config::edit::ConfigEditsBuilder;
|
||||
use crate::config_loader::ConfigLayerEntry;
|
||||
use crate::config_loader::ConfigLayerStack;
|
||||
use crate::config_loader::ConfigRequirementsToml;
|
||||
use crate::config_loader::LoaderOverrides;
|
||||
use crate::config_loader::load_config_layers_state;
|
||||
use crate::config_loader::merge_toml_values;
|
||||
@@ -157,6 +158,22 @@ impl ConfigService {
|
||||
})
|
||||
}
|
||||
|
||||
pub async fn read_requirements(
|
||||
&self,
|
||||
) -> Result<Option<ConfigRequirementsToml>, ConfigServiceError> {
|
||||
let layers = self
|
||||
.load_thread_agnostic_config()
|
||||
.await
|
||||
.map_err(|err| ConfigServiceError::io("failed to read configuration layers", err))?;
|
||||
|
||||
let requirements = layers.requirements_toml().clone();
|
||||
if requirements.is_empty() {
|
||||
Ok(None)
|
||||
} else {
|
||||
Ok(Some(requirements))
|
||||
}
|
||||
}
|
||||
|
||||
pub async fn write_value(
|
||||
&self,
|
||||
params: ConfigValueWriteParams,
|
||||
|
||||
@@ -58,6 +58,10 @@ impl From<SandboxMode> for SandboxModeRequirement {
|
||||
}
|
||||
|
||||
impl ConfigRequirementsToml {
|
||||
pub fn is_empty(&self) -> bool {
|
||||
self.allowed_approval_policies.is_none() && self.allowed_sandbox_modes.is_none()
|
||||
}
|
||||
|
||||
/// For every field in `other` that is `Some`, if the corresponding field in
|
||||
/// `self` is `None`, copy the value from `other` into `self`.
|
||||
pub fn merge_unset_fields(&mut self, mut other: ConfigRequirementsToml) {
|
||||
|
||||
@@ -12,7 +12,6 @@ mod tests;
|
||||
|
||||
use crate::config::CONFIG_TOML_FILE;
|
||||
use crate::config::ConfigToml;
|
||||
use crate::config_loader::config_requirements::ConfigRequirementsToml;
|
||||
use crate::config_loader::layer_io::LoadedConfigLayers;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_protocol::config_types::SandboxMode;
|
||||
@@ -25,6 +24,8 @@ use std::path::Path;
|
||||
use toml::Value as TomlValue;
|
||||
|
||||
pub use config_requirements::ConfigRequirements;
|
||||
pub use config_requirements::ConfigRequirementsToml;
|
||||
pub use config_requirements::SandboxModeRequirement;
|
||||
pub use merge::merge_toml_values;
|
||||
pub use state::ConfigLayerEntry;
|
||||
pub use state::ConfigLayerStack;
|
||||
@@ -201,7 +202,9 @@ pub async fn load_config_layers_state(
|
||||
));
|
||||
}
|
||||
|
||||
ConfigLayerStack::new(layers, config_requirements_toml.try_into()?)
|
||||
let requirements_toml = config_requirements_toml.clone();
|
||||
let requirements = config_requirements_toml.try_into()?;
|
||||
ConfigLayerStack::new(layers, requirements, requirements_toml)
|
||||
}
|
||||
|
||||
/// Attempts to load a config.toml file from `config_toml`.
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use crate::config_loader::ConfigRequirements;
|
||||
use crate::config_loader::ConfigRequirementsToml;
|
||||
|
||||
use super::fingerprint::record_origins;
|
||||
use super::fingerprint::version_for_toml;
|
||||
@@ -86,18 +87,25 @@ pub struct ConfigLayerStack {
|
||||
/// Constraints that must be enforced when deriving a [Config] from the
|
||||
/// layers.
|
||||
requirements: ConfigRequirements,
|
||||
|
||||
/// Raw requirements data as loaded from requirements.toml/MDM/legacy
|
||||
/// sources. This preserves the original allow-lists so they can be
|
||||
/// surfaced via APIs.
|
||||
requirements_toml: ConfigRequirementsToml,
|
||||
}
|
||||
|
||||
impl ConfigLayerStack {
|
||||
pub fn new(
|
||||
layers: Vec<ConfigLayerEntry>,
|
||||
requirements: ConfigRequirements,
|
||||
requirements_toml: ConfigRequirementsToml,
|
||||
) -> std::io::Result<Self> {
|
||||
let user_layer_index = verify_layer_ordering(&layers)?;
|
||||
Ok(Self {
|
||||
layers,
|
||||
user_layer_index,
|
||||
requirements,
|
||||
requirements_toml,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -111,6 +119,10 @@ impl ConfigLayerStack {
|
||||
&self.requirements
|
||||
}
|
||||
|
||||
pub fn requirements_toml(&self) -> &ConfigRequirementsToml {
|
||||
&self.requirements_toml
|
||||
}
|
||||
|
||||
/// Creates a new [ConfigLayerStack] using the specified values to inject a
|
||||
/// "user layer" into the stack. If such a layer already exists, it is
|
||||
/// replaced; otherwise, it is inserted into the stack at the appropriate
|
||||
@@ -131,6 +143,7 @@ impl ConfigLayerStack {
|
||||
layers,
|
||||
user_layer_index: self.user_layer_index,
|
||||
requirements: self.requirements.clone(),
|
||||
requirements_toml: self.requirements_toml.clone(),
|
||||
}
|
||||
}
|
||||
None => {
|
||||
@@ -151,6 +164,7 @@ impl ConfigLayerStack {
|
||||
layers,
|
||||
user_layer_index: Some(user_layer_index),
|
||||
requirements: self.requirements.clone(),
|
||||
requirements_toml: self.requirements_toml.clone(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -181,6 +181,43 @@ impl From<CancelErr> for CodexErr {
|
||||
}
|
||||
}
|
||||
|
||||
impl CodexErr {
|
||||
pub fn is_retryable(&self) -> bool {
|
||||
match self {
|
||||
CodexErr::TurnAborted
|
||||
| CodexErr::Interrupted
|
||||
| CodexErr::EnvVar(_)
|
||||
| CodexErr::Fatal(_)
|
||||
| CodexErr::UsageNotIncluded
|
||||
| CodexErr::QuotaExceeded
|
||||
| CodexErr::InvalidImageRequest()
|
||||
| CodexErr::InvalidRequest(_)
|
||||
| CodexErr::RefreshTokenFailed(_)
|
||||
| CodexErr::UnsupportedOperation(_)
|
||||
| CodexErr::Sandbox(_)
|
||||
| CodexErr::LandlockSandboxExecutableNotProvided
|
||||
| CodexErr::RetryLimit(_)
|
||||
| CodexErr::ContextWindowExceeded
|
||||
| CodexErr::ThreadNotFound(_)
|
||||
| CodexErr::Spawn
|
||||
| CodexErr::SessionConfiguredNotFirstEvent
|
||||
| CodexErr::UsageLimitReached(_) => false,
|
||||
CodexErr::Stream(..)
|
||||
| CodexErr::Timeout
|
||||
| CodexErr::UnexpectedStatus(_)
|
||||
| CodexErr::ResponseStreamFailed(_)
|
||||
| CodexErr::ConnectionFailed(_)
|
||||
| CodexErr::InternalServerError
|
||||
| CodexErr::InternalAgentDied
|
||||
| CodexErr::Io(_)
|
||||
| CodexErr::Json(_)
|
||||
| CodexErr::TokioJoin(_) => true,
|
||||
#[cfg(target_os = "linux")]
|
||||
CodexErr::LandlockRuleset(_) | CodexErr::LandlockPathFd(_) => false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct ConnectionFailedError {
|
||||
pub source: reqwest::Error,
|
||||
|
||||
@@ -421,6 +421,7 @@ mod tests {
|
||||
use crate::config_loader::ConfigLayerEntry;
|
||||
use crate::config_loader::ConfigLayerStack;
|
||||
use crate::config_loader::ConfigRequirements;
|
||||
use crate::config_loader::ConfigRequirementsToml;
|
||||
use crate::features::Feature;
|
||||
use crate::features::Features;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
@@ -441,7 +442,12 @@ mod tests {
|
||||
ConfigLayerSource::Project { dot_codex_folder },
|
||||
TomlValue::Table(Default::default()),
|
||||
);
|
||||
ConfigLayerStack::new(vec![layer], ConfigRequirements::default()).expect("ConfigLayerStack")
|
||||
ConfigLayerStack::new(
|
||||
vec![layer],
|
||||
ConfigRequirements::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)
|
||||
.expect("ConfigLayerStack")
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -573,7 +579,11 @@ mod tests {
|
||||
TomlValue::Table(Default::default()),
|
||||
),
|
||||
];
|
||||
let config_stack = ConfigLayerStack::new(layers, ConfigRequirements::default())?;
|
||||
let config_stack = ConfigLayerStack::new(
|
||||
layers,
|
||||
ConfigRequirements::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)?;
|
||||
|
||||
let policy = load_exec_policy(&config_stack).await?;
|
||||
|
||||
|
||||
@@ -60,10 +60,6 @@ pub enum Feature {
|
||||
// Stable.
|
||||
/// Create a ghost commit at each turn.
|
||||
GhostCommit,
|
||||
/// Include the view_image tool.
|
||||
ViewImageTool,
|
||||
/// Send warnings to the model to correct it on the tool usage.
|
||||
ModelWarnings,
|
||||
/// Enable the default shell tool.
|
||||
ShellTool,
|
||||
|
||||
@@ -87,16 +83,14 @@ pub enum Feature {
|
||||
RemoteCompaction,
|
||||
/// Refresh remote models and emit AppReady once the list is available.
|
||||
RemoteModels,
|
||||
/// Allow model to call multiple tools in parallel (only for models supporting it).
|
||||
ParallelToolCalls,
|
||||
/// Experimental shell snapshotting.
|
||||
ShellSnapshot,
|
||||
/// Experimental TUI v2 (viewport) implementation.
|
||||
Tui2,
|
||||
/// Enable discovery and injection of skills.
|
||||
Skills,
|
||||
/// Enforce UTF8 output in Powershell.
|
||||
PowershellUtf8,
|
||||
/// Compress request bodies (zstd) when sending streaming requests to codex-backend.
|
||||
EnableRequestCompression,
|
||||
}
|
||||
|
||||
impl Feature {
|
||||
@@ -231,7 +225,6 @@ impl Features {
|
||||
experimental_use_freeform_apply_patch: cfg.experimental_use_freeform_apply_patch,
|
||||
experimental_use_unified_exec_tool: cfg.experimental_use_unified_exec_tool,
|
||||
tools_web_search: cfg.tools.as_ref().and_then(|t| t.web_search),
|
||||
tools_view_image: cfg.tools.as_ref().and_then(|t| t.view_image),
|
||||
..Default::default()
|
||||
};
|
||||
base_legacy.apply(&mut features);
|
||||
@@ -247,7 +240,6 @@ impl Features {
|
||||
|
||||
experimental_use_unified_exec_tool: config_profile.experimental_use_unified_exec_tool,
|
||||
tools_web_search: config_profile.tools_web_search,
|
||||
tools_view_image: config_profile.tools_view_image,
|
||||
};
|
||||
profile_legacy.apply(&mut features);
|
||||
if let Some(profile_features) = config_profile.features.as_ref() {
|
||||
@@ -303,30 +295,12 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
stage: Stage::Stable,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ParallelToolCalls,
|
||||
key: "parallel",
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ViewImageTool,
|
||||
key: "view_image_tool",
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ShellTool,
|
||||
key: "shell_tool",
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ModelWarnings,
|
||||
key: "warnings",
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::WebSearchRequest,
|
||||
key: "web_search_request",
|
||||
@@ -396,18 +370,18 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
stage: Stage::Experimental,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::Skills,
|
||||
key: "skills",
|
||||
stage: Stage::Experimental,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::PowershellUtf8,
|
||||
key: "powershell_utf8",
|
||||
stage: Stage::Experimental,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::EnableRequestCompression,
|
||||
key: "enable_request_compression",
|
||||
stage: Stage::Experimental,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::Tui2,
|
||||
key: "tui2",
|
||||
|
||||
@@ -47,7 +47,6 @@ pub struct LegacyFeatureToggles {
|
||||
pub experimental_use_freeform_apply_patch: Option<bool>,
|
||||
pub experimental_use_unified_exec_tool: Option<bool>,
|
||||
pub tools_web_search: Option<bool>,
|
||||
pub tools_view_image: Option<bool>,
|
||||
}
|
||||
|
||||
impl LegacyFeatureToggles {
|
||||
@@ -76,12 +75,6 @@ impl LegacyFeatureToggles {
|
||||
self.tools_web_search,
|
||||
"tools.web_search",
|
||||
);
|
||||
set_if_some(
|
||||
features,
|
||||
Feature::ViewImageTool,
|
||||
self.tools_view_image,
|
||||
"tools.view_image",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -5,9 +5,11 @@ use codex_protocol::openai_models::ModelInfo;
|
||||
use codex_protocol::openai_models::ModelVisibility;
|
||||
use codex_protocol::openai_models::ReasoningEffort;
|
||||
use codex_protocol::openai_models::ReasoningEffortPreset;
|
||||
use codex_protocol::openai_models::TruncationMode;
|
||||
use codex_protocol::openai_models::TruncationPolicyConfig;
|
||||
|
||||
use crate::config::Config;
|
||||
use crate::truncate::approx_bytes_for_tokens;
|
||||
use tracing::warn;
|
||||
|
||||
const BASE_INSTRUCTIONS: &str = include_str!("../../prompt.md");
|
||||
@@ -71,6 +73,19 @@ pub(crate) fn with_config_overrides(mut model: ModelInfo, config: &Config) -> Mo
|
||||
if let Some(auto_compact_token_limit) = config.model_auto_compact_token_limit {
|
||||
model.auto_compact_token_limit = Some(auto_compact_token_limit);
|
||||
}
|
||||
if let Some(token_limit) = config.tool_output_token_limit {
|
||||
model.truncation_policy = match model.truncation_policy.mode {
|
||||
TruncationMode::Bytes => {
|
||||
let byte_limit =
|
||||
i64::try_from(approx_bytes_for_tokens(token_limit)).unwrap_or(i64::MAX);
|
||||
TruncationPolicyConfig::bytes(byte_limit)
|
||||
}
|
||||
TruncationMode::Tokens => {
|
||||
let limit = i64::try_from(token_limit).unwrap_or(i64::MAX);
|
||||
TruncationPolicyConfig::tokens(limit)
|
||||
}
|
||||
};
|
||||
}
|
||||
model
|
||||
}
|
||||
|
||||
|
||||
@@ -277,7 +277,7 @@ mod tests {
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
assess_patch_safety(&add_inside, AskForApproval::OnRequest, &policy, &cwd,),
|
||||
assess_patch_safety(&add_inside, AskForApproval::OnRequest, &policy, &cwd),
|
||||
SafetyCheck::AutoApprove {
|
||||
sandbox_type: SandboxType::None,
|
||||
user_explicitly_approved: false,
|
||||
|
||||
@@ -328,6 +328,8 @@ Write the YAML frontmatter with `name` and `description`:
|
||||
- Include all "when to use" information here - Not in the body. The body is only loaded after triggering, so "When to Use This Skill" sections in the body are not helpful to Codex.
|
||||
- Example description for a `docx` skill: "Comprehensive document creation, editing, and analysis with support for tracked changes, comments, formatting preservation, and text extraction. Use when Codex needs to work with professional documents (.docx files) for: (1) Creating new documents, (2) Modifying or editing content, (3) Working with tracked changes, (4) Adding comments, or any other document tasks"
|
||||
|
||||
Ensure the frontmatter is valid YAML. Keep `name` and `description` as single-line scalars. If either could be interpreted as YAML syntax, wrap it in quotes.
|
||||
|
||||
Do not include any other fields in YAML frontmatter.
|
||||
|
||||
##### Body
|
||||
|
||||
@@ -6,7 +6,7 @@ use crate::skills::model::SkillMetadata;
|
||||
use crate::skills::system::system_cache_root_dir;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use dunce::canonicalize as normalize_path;
|
||||
use dunce::canonicalize as canonicalize_path;
|
||||
use serde::Deserialize;
|
||||
use std::collections::HashSet;
|
||||
use std::collections::VecDeque;
|
||||
@@ -36,6 +36,9 @@ const SKILLS_DIR_NAME: &str = "skills";
|
||||
const MAX_NAME_LEN: usize = 64;
|
||||
const MAX_DESCRIPTION_LEN: usize = 1024;
|
||||
const MAX_SHORT_DESCRIPTION_LEN: usize = MAX_DESCRIPTION_LEN;
|
||||
// Traversal depth from the skills root.
|
||||
const MAX_SCAN_DEPTH: usize = 6;
|
||||
const MAX_SKILLS_DIRS_PER_ROOT: usize = 2000;
|
||||
|
||||
#[derive(Debug)]
|
||||
enum SkillParseError {
|
||||
@@ -165,7 +168,7 @@ pub(crate) fn skill_roots_from_layer_stack(
|
||||
}
|
||||
|
||||
fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut SkillLoadOutcome) {
|
||||
let Ok(root) = normalize_path(root) else {
|
||||
let Ok(root) = canonicalize_path(root) else {
|
||||
return;
|
||||
};
|
||||
|
||||
@@ -173,8 +176,38 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil
|
||||
return;
|
||||
}
|
||||
|
||||
let mut queue: VecDeque<PathBuf> = VecDeque::from([root]);
|
||||
while let Some(dir) = queue.pop_front() {
|
||||
fn enqueue_dir(
|
||||
queue: &mut VecDeque<(PathBuf, usize)>,
|
||||
visited_dirs: &mut HashSet<PathBuf>,
|
||||
truncated_by_dir_limit: &mut bool,
|
||||
path: PathBuf,
|
||||
depth: usize,
|
||||
) {
|
||||
if depth > MAX_SCAN_DEPTH {
|
||||
return;
|
||||
}
|
||||
if visited_dirs.len() >= MAX_SKILLS_DIRS_PER_ROOT {
|
||||
*truncated_by_dir_limit = true;
|
||||
return;
|
||||
}
|
||||
if visited_dirs.insert(path.clone()) {
|
||||
queue.push_back((path, depth));
|
||||
}
|
||||
}
|
||||
|
||||
// Follow symlinks for user, admin, and repo skills. System skills are written by Codex itself.
|
||||
let follow_symlinks = matches!(
|
||||
scope,
|
||||
SkillScope::Repo | SkillScope::User | SkillScope::Admin
|
||||
);
|
||||
|
||||
let mut visited_dirs: HashSet<PathBuf> = HashSet::new();
|
||||
visited_dirs.insert(root.clone());
|
||||
|
||||
let mut queue: VecDeque<(PathBuf, usize)> = VecDeque::from([(root.clone(), 0)]);
|
||||
let mut truncated_by_dir_limit = false;
|
||||
|
||||
while let Some((dir, depth)) = queue.pop_front() {
|
||||
let entries = match fs::read_dir(&dir) {
|
||||
Ok(entries) => entries,
|
||||
Err(e) => {
|
||||
@@ -199,11 +232,64 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil
|
||||
};
|
||||
|
||||
if file_type.is_symlink() {
|
||||
if !follow_symlinks {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Follow the symlink to determine what it points to.
|
||||
let metadata = match fs::metadata(&path) {
|
||||
Ok(metadata) => metadata,
|
||||
Err(e) => {
|
||||
error!(
|
||||
"failed to stat skills entry {} (symlink): {e:#}",
|
||||
path.display()
|
||||
);
|
||||
continue;
|
||||
}
|
||||
};
|
||||
|
||||
if metadata.is_dir() {
|
||||
let Ok(resolved_dir) = canonicalize_path(&path) else {
|
||||
continue;
|
||||
};
|
||||
enqueue_dir(
|
||||
&mut queue,
|
||||
&mut visited_dirs,
|
||||
&mut truncated_by_dir_limit,
|
||||
resolved_dir,
|
||||
depth + 1,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
if metadata.is_file() && file_name == SKILLS_FILENAME {
|
||||
match parse_skill_file(&path, scope) {
|
||||
Ok(skill) => outcome.skills.push(skill),
|
||||
Err(err) => {
|
||||
if scope != SkillScope::System {
|
||||
outcome.errors.push(SkillError {
|
||||
path,
|
||||
message: err.to_string(),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
continue;
|
||||
}
|
||||
|
||||
if file_type.is_dir() {
|
||||
queue.push_back(path);
|
||||
let Ok(resolved_dir) = canonicalize_path(&path) else {
|
||||
continue;
|
||||
};
|
||||
enqueue_dir(
|
||||
&mut queue,
|
||||
&mut visited_dirs,
|
||||
&mut truncated_by_dir_limit,
|
||||
resolved_dir,
|
||||
depth + 1,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -224,6 +310,14 @@ fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut Skil
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if truncated_by_dir_limit {
|
||||
tracing::warn!(
|
||||
"skills scan truncated after {} directories (root: {})",
|
||||
MAX_SKILLS_DIRS_PER_ROOT,
|
||||
root.display()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_skill_file(path: &Path, scope: SkillScope) -> Result<SkillMetadata, SkillParseError> {
|
||||
@@ -253,7 +347,7 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result<SkillMetadata, Ski
|
||||
)?;
|
||||
}
|
||||
|
||||
let resolved_path = normalize_path(path).unwrap_or_else(|_| path.to_path_buf());
|
||||
let resolved_path = canonicalize_path(path).unwrap_or_else(|_| path.to_path_buf());
|
||||
|
||||
Ok(SkillMetadata {
|
||||
name,
|
||||
@@ -316,6 +410,7 @@ mod tests {
|
||||
use crate::config_loader::ConfigLayerEntry;
|
||||
use crate::config_loader::ConfigLayerStack;
|
||||
use crate::config_loader::ConfigRequirements;
|
||||
use crate::config_loader::ConfigRequirementsToml;
|
||||
use codex_protocol::protocol::SkillScope;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
@@ -350,7 +445,7 @@ mod tests {
|
||||
}
|
||||
|
||||
fn normalized(path: &Path) -> PathBuf {
|
||||
normalize_path(path).unwrap_or_else(|_| path.to_path_buf())
|
||||
canonicalize_path(path).unwrap_or_else(|_| path.to_path_buf())
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -377,7 +472,11 @@ mod tests {
|
||||
TomlValue::Table(toml::map::Map::new()),
|
||||
),
|
||||
];
|
||||
let stack = ConfigLayerStack::new(layers, ConfigRequirements::default())?;
|
||||
let stack = ConfigLayerStack::new(
|
||||
layers,
|
||||
ConfigRequirements::default(),
|
||||
ConfigRequirementsToml::default(),
|
||||
)?;
|
||||
|
||||
let got = skill_roots_from_layer_stack(&stack)
|
||||
.into_iter()
|
||||
@@ -429,6 +528,243 @@ mod tests {
|
||||
path
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn symlink_dir(target: &Path, link: &Path) {
|
||||
std::os::unix::fs::symlink(target, link).unwrap();
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
fn symlink_file(target: &Path, link: &Path) {
|
||||
std::os::unix::fs::symlink(target, link).unwrap();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[cfg(unix)]
|
||||
async fn loads_skills_via_symlinked_subdir_for_user_scope() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let shared = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
let shared_skill_path = write_skill_at(shared.path(), "demo", "linked-skill", "from link");
|
||||
|
||||
fs::create_dir_all(codex_home.path().join("skills")).unwrap();
|
||||
symlink_dir(shared.path(), &codex_home.path().join("skills/shared"));
|
||||
|
||||
let cfg = make_config(&codex_home).await;
|
||||
let outcome = load_skills(&cfg);
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "linked-skill".to_string(),
|
||||
description: "from link".to_string(),
|
||||
short_description: None,
|
||||
path: normalized(&shared_skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[cfg(unix)]
|
||||
async fn loads_skills_via_symlinked_skill_file_for_user_scope() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let shared = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
let shared_skill_path =
|
||||
write_skill_at(shared.path(), "demo", "linked-file-skill", "from link");
|
||||
|
||||
let skill_dir = codex_home.path().join("skills/demo");
|
||||
fs::create_dir_all(&skill_dir).unwrap();
|
||||
symlink_file(&shared_skill_path, &skill_dir.join(SKILLS_FILENAME));
|
||||
|
||||
let cfg = make_config(&codex_home).await;
|
||||
let outcome = load_skills(&cfg);
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "linked-file-skill".to_string(),
|
||||
description: "from link".to_string(),
|
||||
short_description: None,
|
||||
path: normalized(&shared_skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[cfg(unix)]
|
||||
async fn does_not_loop_on_symlink_cycle_for_user_scope() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
// Create a cycle:
|
||||
// $CODEX_HOME/skills/cycle/loop -> $CODEX_HOME/skills/cycle
|
||||
let cycle_dir = codex_home.path().join("skills/cycle");
|
||||
fs::create_dir_all(&cycle_dir).unwrap();
|
||||
symlink_dir(&cycle_dir, &cycle_dir.join("loop"));
|
||||
|
||||
let skill_path = write_skill_at(&cycle_dir, "demo", "cycle-skill", "still loads");
|
||||
|
||||
let cfg = make_config(&codex_home).await;
|
||||
let outcome = load_skills(&cfg);
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "cycle-skill".to_string(),
|
||||
description: "still loads".to_string(),
|
||||
short_description: None,
|
||||
path: normalized(&skill_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[cfg(unix)]
|
||||
fn loads_skills_via_symlinked_subdir_for_admin_scope() {
|
||||
let admin_root = tempfile::tempdir().expect("tempdir");
|
||||
let shared = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
let shared_skill_path =
|
||||
write_skill_at(shared.path(), "demo", "admin-linked-skill", "from link");
|
||||
fs::create_dir_all(admin_root.path()).unwrap();
|
||||
symlink_dir(shared.path(), &admin_root.path().join("shared"));
|
||||
|
||||
let outcome = load_skills_from_roots([SkillRoot {
|
||||
path: admin_root.path().to_path_buf(),
|
||||
scope: SkillScope::Admin,
|
||||
}]);
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "admin-linked-skill".to_string(),
|
||||
description: "from link".to_string(),
|
||||
short_description: None,
|
||||
path: normalized(&shared_skill_path),
|
||||
scope: SkillScope::Admin,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[cfg(unix)]
|
||||
async fn loads_skills_via_symlinked_subdir_for_repo_scope() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let repo_dir = tempfile::tempdir().expect("tempdir");
|
||||
mark_as_git_repo(repo_dir.path());
|
||||
let shared = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
let linked_skill_path =
|
||||
write_skill_at(shared.path(), "demo", "repo-linked-skill", "from link");
|
||||
let repo_skills_root = repo_dir
|
||||
.path()
|
||||
.join(REPO_ROOT_CONFIG_DIR_NAME)
|
||||
.join(SKILLS_DIR_NAME);
|
||||
fs::create_dir_all(&repo_skills_root).unwrap();
|
||||
symlink_dir(shared.path(), &repo_skills_root.join("shared"));
|
||||
|
||||
let cfg = make_config_for_cwd(&codex_home, repo_dir.path().to_path_buf()).await;
|
||||
let outcome = load_skills(&cfg);
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "repo-linked-skill".to_string(),
|
||||
description: "from link".to_string(),
|
||||
short_description: None,
|
||||
path: normalized(&linked_skill_path),
|
||||
scope: SkillScope::Repo,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
#[cfg(unix)]
|
||||
async fn system_scope_ignores_symlinked_subdir() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let shared = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
write_skill_at(shared.path(), "demo", "system-linked-skill", "from link");
|
||||
|
||||
let system_root = codex_home.path().join("skills/.system");
|
||||
fs::create_dir_all(&system_root).unwrap();
|
||||
symlink_dir(shared.path(), &system_root.join("shared"));
|
||||
|
||||
let cfg = make_config(&codex_home).await;
|
||||
let outcome = load_skills(&cfg);
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(outcome.skills.len(), 0);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn respects_max_scan_depth_for_user_scope() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
|
||||
let within_depth_path = write_skill(
|
||||
&codex_home,
|
||||
"d0/d1/d2/d3/d4/d5",
|
||||
"within-depth-skill",
|
||||
"loads",
|
||||
);
|
||||
let _too_deep_path = write_skill(
|
||||
&codex_home,
|
||||
"d0/d1/d2/d3/d4/d5/d6",
|
||||
"too-deep-skill",
|
||||
"should not load",
|
||||
);
|
||||
|
||||
let cfg = make_config(&codex_home).await;
|
||||
let outcome = load_skills(&cfg);
|
||||
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(
|
||||
outcome.skills,
|
||||
vec![SkillMetadata {
|
||||
name: "within-depth-skill".to_string(),
|
||||
description: "loads".to_string(),
|
||||
short_description: None,
|
||||
path: normalized(&within_depth_path),
|
||||
scope: SkillScope::User,
|
||||
}]
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn loads_valid_skill() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
@@ -1029,7 +1365,7 @@ mod tests {
|
||||
outcome.errors
|
||||
);
|
||||
let expected_path =
|
||||
normalize_path(&nested_skill_path).unwrap_or_else(|_| nested_skill_path.clone());
|
||||
canonicalize_path(&nested_skill_path).unwrap_or_else(|_| nested_skill_path.clone());
|
||||
assert_eq!(
|
||||
vec![SkillMetadata {
|
||||
name: "dupe-skill".to_string(),
|
||||
|
||||
@@ -85,8 +85,7 @@ async fn start_review_conversation(
|
||||
// re-enable blocked tools (web search, view image).
|
||||
sub_agent_config
|
||||
.features
|
||||
.disable(crate::features::Feature::WebSearchRequest)
|
||||
.disable(crate::features::Feature::ViewImageTool);
|
||||
.disable(crate::features::Feature::WebSearchRequest);
|
||||
|
||||
// Set explicit review rubric for the sub-agent
|
||||
sub_agent_config.base_instructions = Some(crate::REVIEW_PROMPT.to_string());
|
||||
|
||||
@@ -169,53 +169,6 @@ impl ThreadManager {
|
||||
.await
|
||||
}
|
||||
|
||||
#[deprecated(note = "use get_thread")]
|
||||
pub async fn get_conversation(&self, thread_id: ThreadId) -> CodexResult<Arc<CodexThread>> {
|
||||
self.get_thread(thread_id).await
|
||||
}
|
||||
|
||||
#[deprecated(note = "use start_thread")]
|
||||
pub async fn new_conversation(&self, config: Config) -> CodexResult<NewThread> {
|
||||
self.start_thread(config).await
|
||||
}
|
||||
|
||||
#[deprecated(note = "use resume_thread_from_rollout")]
|
||||
pub async fn resume_conversation_from_rollout(
|
||||
&self,
|
||||
config: Config,
|
||||
rollout_path: PathBuf,
|
||||
auth_manager: Arc<AuthManager>,
|
||||
) -> CodexResult<NewThread> {
|
||||
self.resume_thread_from_rollout(config, rollout_path, auth_manager)
|
||||
.await
|
||||
}
|
||||
|
||||
#[deprecated(note = "use resume_thread_with_history")]
|
||||
pub async fn resume_conversation_with_history(
|
||||
&self,
|
||||
config: Config,
|
||||
initial_history: InitialHistory,
|
||||
auth_manager: Arc<AuthManager>,
|
||||
) -> CodexResult<NewThread> {
|
||||
self.resume_thread_with_history(config, initial_history, auth_manager)
|
||||
.await
|
||||
}
|
||||
|
||||
#[deprecated(note = "use remove_thread")]
|
||||
pub async fn remove_conversation(&self, thread_id: &ThreadId) -> Option<Arc<CodexThread>> {
|
||||
self.remove_thread(thread_id).await
|
||||
}
|
||||
|
||||
#[deprecated(note = "use fork_thread")]
|
||||
pub async fn fork_conversation(
|
||||
&self,
|
||||
nth_user_message: usize,
|
||||
config: Config,
|
||||
path: PathBuf,
|
||||
) -> CodexResult<NewThread> {
|
||||
self.fork_thread(nth_user_message, config, path).await
|
||||
}
|
||||
|
||||
/// Removes the thread from the manager's internal map, though the thread is stored
|
||||
/// as `Arc<CodexThread>`, it is possible that other references to it exist elsewhere.
|
||||
/// Returns the thread if the thread was found and removed.
|
||||
@@ -322,7 +275,6 @@ impl ThreadManagerState {
|
||||
));
|
||||
self.threads.write().await.insert(thread_id, thread.clone());
|
||||
|
||||
#[allow(deprecated)]
|
||||
Ok(NewThread {
|
||||
thread_id,
|
||||
thread,
|
||||
|
||||
@@ -305,7 +305,12 @@ impl ToolEmitter {
|
||||
// Normalize common rejection messages for exec tools so tests and
|
||||
// users see a clear, consistent phrase.
|
||||
let normalized = if msg == "rejected by user" {
|
||||
"exec command rejected by user".to_string()
|
||||
match self {
|
||||
Self::Shell { .. } | Self::UnifiedExec { .. } => {
|
||||
"exec command rejected by user".to_string()
|
||||
}
|
||||
Self::ApplyPatch { .. } => "patch rejected by user".to_string(),
|
||||
}
|
||||
} else {
|
||||
msg
|
||||
};
|
||||
|
||||
@@ -26,11 +26,38 @@ use crate::tools::sandboxing::ToolCtx;
|
||||
use crate::tools::spec::ApplyPatchToolArgs;
|
||||
use crate::tools::spec::JsonSchema;
|
||||
use async_trait::async_trait;
|
||||
use codex_apply_patch::ApplyPatchAction;
|
||||
use codex_apply_patch::ApplyPatchFileChange;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
|
||||
pub struct ApplyPatchHandler;
|
||||
|
||||
const APPLY_PATCH_LARK_GRAMMAR: &str = include_str!("tool_apply_patch.lark");
|
||||
|
||||
fn file_paths_for_action(action: &ApplyPatchAction) -> Vec<AbsolutePathBuf> {
|
||||
let mut keys = Vec::new();
|
||||
let cwd = action.cwd.as_path();
|
||||
|
||||
for (path, change) in action.changes() {
|
||||
if let Some(key) = to_abs_path(cwd, path) {
|
||||
keys.push(key);
|
||||
}
|
||||
|
||||
if let ApplyPatchFileChange::Update { move_path, .. } = change
|
||||
&& let Some(dest) = move_path
|
||||
&& let Some(key) = to_abs_path(cwd, dest)
|
||||
{
|
||||
keys.push(key);
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
}
|
||||
|
||||
fn to_abs_path(cwd: &Path, path: &Path) -> Option<AbsolutePathBuf> {
|
||||
AbsolutePathBuf::resolve_path_against_base(path, cwd).ok()
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl ToolHandler for ApplyPatchHandler {
|
||||
fn kind(&self) -> ToolKind {
|
||||
@@ -81,9 +108,7 @@ impl ToolHandler for ApplyPatchHandler {
|
||||
let command = vec!["apply_patch".to_string(), patch_input.clone()];
|
||||
match codex_apply_patch::maybe_parse_apply_patch_verified(&command, &cwd) {
|
||||
codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => {
|
||||
match apply_patch::apply_patch(session.as_ref(), turn.as_ref(), &call_id, changes)
|
||||
.await
|
||||
{
|
||||
match apply_patch::apply_patch(turn.as_ref(), changes).await {
|
||||
InternalApplyPatchInvocation::Output(item) => {
|
||||
let content = item?;
|
||||
Ok(ToolOutput::Function {
|
||||
@@ -93,10 +118,10 @@ impl ToolHandler for ApplyPatchHandler {
|
||||
})
|
||||
}
|
||||
InternalApplyPatchInvocation::DelegateToExec(apply) => {
|
||||
let emitter = ToolEmitter::apply_patch(
|
||||
convert_apply_patch_to_protocol(&apply.action),
|
||||
!apply.user_explicitly_approved_this_action,
|
||||
);
|
||||
let changes = convert_apply_patch_to_protocol(&apply.action);
|
||||
let file_paths = file_paths_for_action(&apply.action);
|
||||
let emitter =
|
||||
ToolEmitter::apply_patch(changes.clone(), apply.auto_approved);
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
session.as_ref(),
|
||||
turn.as_ref(),
|
||||
@@ -106,10 +131,11 @@ impl ToolHandler for ApplyPatchHandler {
|
||||
emitter.begin(event_ctx).await;
|
||||
|
||||
let req = ApplyPatchRequest {
|
||||
patch: apply.action.patch.clone(),
|
||||
cwd: apply.action.cwd.clone(),
|
||||
action: apply.action,
|
||||
file_paths,
|
||||
changes,
|
||||
exec_approval_requirement: apply.exec_approval_requirement,
|
||||
timeout_ms: None,
|
||||
user_explicitly_approved: apply.user_explicitly_approved_this_action,
|
||||
codex_exe: turn.codex_linux_sandbox_exe.clone(),
|
||||
};
|
||||
|
||||
@@ -178,7 +204,7 @@ pub(crate) async fn intercept_apply_patch(
|
||||
turn,
|
||||
)
|
||||
.await;
|
||||
match apply_patch::apply_patch(session, turn, call_id, changes).await {
|
||||
match apply_patch::apply_patch(turn, changes).await {
|
||||
InternalApplyPatchInvocation::Output(item) => {
|
||||
let content = item?;
|
||||
Ok(Some(ToolOutput::Function {
|
||||
@@ -188,19 +214,19 @@ pub(crate) async fn intercept_apply_patch(
|
||||
}))
|
||||
}
|
||||
InternalApplyPatchInvocation::DelegateToExec(apply) => {
|
||||
let emitter = ToolEmitter::apply_patch(
|
||||
convert_apply_patch_to_protocol(&apply.action),
|
||||
!apply.user_explicitly_approved_this_action,
|
||||
);
|
||||
let changes = convert_apply_patch_to_protocol(&apply.action);
|
||||
let approval_keys = file_paths_for_action(&apply.action);
|
||||
let emitter = ToolEmitter::apply_patch(changes.clone(), apply.auto_approved);
|
||||
let event_ctx =
|
||||
ToolEventCtx::new(session, turn, call_id, tracker.as_ref().copied());
|
||||
emitter.begin(event_ctx).await;
|
||||
|
||||
let req = ApplyPatchRequest {
|
||||
patch: apply.action.patch.clone(),
|
||||
cwd: apply.action.cwd.clone(),
|
||||
action: apply.action,
|
||||
file_paths: approval_keys,
|
||||
changes,
|
||||
exec_approval_requirement: apply.exec_approval_requirement,
|
||||
timeout_ms,
|
||||
user_explicitly_approved: apply.user_explicitly_approved_this_action,
|
||||
codex_exe: turn.codex_linux_sandbox_exe.clone(),
|
||||
};
|
||||
|
||||
@@ -342,3 +368,35 @@ It is important to remember:
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use codex_apply_patch::MaybeApplyPatchVerified;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[test]
|
||||
fn approval_keys_include_move_destination() {
|
||||
let tmp = TempDir::new().expect("tmp");
|
||||
let cwd = tmp.path();
|
||||
std::fs::create_dir_all(cwd.join("old")).expect("create old dir");
|
||||
std::fs::create_dir_all(cwd.join("renamed/dir")).expect("create dest dir");
|
||||
std::fs::write(cwd.join("old/name.txt"), "old content\n").expect("write old file");
|
||||
let patch = r#"*** Begin Patch
|
||||
*** Update File: old/name.txt
|
||||
*** Move to: renamed/dir/name.txt
|
||||
@@
|
||||
-old content
|
||||
+new content
|
||||
*** End Patch"#;
|
||||
let argv = vec!["apply_patch".to_string(), patch.to_string()];
|
||||
let action = match codex_apply_patch::maybe_parse_apply_patch_verified(&argv, cwd) {
|
||||
MaybeApplyPatchVerified::Body(action) => action,
|
||||
other => panic!("expected patch body, got: {other:?}"),
|
||||
};
|
||||
|
||||
let keys = file_paths_for_action(&action);
|
||||
assert_eq!(keys.len(), 2);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -11,6 +11,7 @@ use crate::sandboxing::SandboxPermissions;
|
||||
use crate::sandboxing::execute_env;
|
||||
use crate::tools::sandboxing::Approvable;
|
||||
use crate::tools::sandboxing::ApprovalCtx;
|
||||
use crate::tools::sandboxing::ExecApprovalRequirement;
|
||||
use crate::tools::sandboxing::SandboxAttempt;
|
||||
use crate::tools::sandboxing::Sandboxable;
|
||||
use crate::tools::sandboxing::SandboxablePreference;
|
||||
@@ -18,30 +19,28 @@ use crate::tools::sandboxing::ToolCtx;
|
||||
use crate::tools::sandboxing::ToolError;
|
||||
use crate::tools::sandboxing::ToolRuntime;
|
||||
use crate::tools::sandboxing::with_cached_approval;
|
||||
use codex_apply_patch::ApplyPatchAction;
|
||||
use codex_protocol::protocol::AskForApproval;
|
||||
use codex_protocol::protocol::FileChange;
|
||||
use codex_protocol::protocol::ReviewDecision;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use futures::future::BoxFuture;
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
#[derive(Debug)]
|
||||
pub struct ApplyPatchRequest {
|
||||
pub patch: String,
|
||||
pub cwd: PathBuf,
|
||||
pub action: ApplyPatchAction,
|
||||
pub file_paths: Vec<AbsolutePathBuf>,
|
||||
pub changes: std::collections::HashMap<PathBuf, FileChange>,
|
||||
pub exec_approval_requirement: ExecApprovalRequirement,
|
||||
pub timeout_ms: Option<u64>,
|
||||
pub user_explicitly_approved: bool,
|
||||
pub codex_exe: Option<PathBuf>,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
pub struct ApplyPatchRuntime;
|
||||
|
||||
#[derive(serde::Serialize, Clone, Debug, Eq, PartialEq, Hash)]
|
||||
pub(crate) struct ApprovalKey {
|
||||
patch: String,
|
||||
cwd: PathBuf,
|
||||
}
|
||||
|
||||
impl ApplyPatchRuntime {
|
||||
pub fn new() -> Self {
|
||||
Self
|
||||
@@ -58,8 +57,8 @@ impl ApplyPatchRuntime {
|
||||
let program = exe.to_string_lossy().to_string();
|
||||
Ok(CommandSpec {
|
||||
program,
|
||||
args: vec![CODEX_APPLY_PATCH_ARG1.to_string(), req.patch.clone()],
|
||||
cwd: req.cwd.clone(),
|
||||
args: vec![CODEX_APPLY_PATCH_ARG1.to_string(), req.action.patch.clone()],
|
||||
cwd: req.action.cwd.clone(),
|
||||
expiration: req.timeout_ms.into(),
|
||||
// Run apply_patch with a minimal environment for determinism and to avoid leaks.
|
||||
env: HashMap::new(),
|
||||
@@ -87,13 +86,10 @@ impl Sandboxable for ApplyPatchRuntime {
|
||||
}
|
||||
|
||||
impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
|
||||
type ApprovalKey = ApprovalKey;
|
||||
type ApprovalKey = AbsolutePathBuf;
|
||||
|
||||
fn approval_key(&self, req: &ApplyPatchRequest) -> Self::ApprovalKey {
|
||||
ApprovalKey {
|
||||
patch: req.patch.clone(),
|
||||
cwd: req.cwd.clone(),
|
||||
}
|
||||
fn approval_keys(&self, req: &ApplyPatchRequest) -> Vec<Self::ApprovalKey> {
|
||||
req.file_paths.clone()
|
||||
}
|
||||
|
||||
fn start_approval_async<'a>(
|
||||
@@ -101,31 +97,25 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
|
||||
req: &'a ApplyPatchRequest,
|
||||
ctx: ApprovalCtx<'a>,
|
||||
) -> BoxFuture<'a, ReviewDecision> {
|
||||
let key = self.approval_key(req);
|
||||
let session = ctx.session;
|
||||
let turn = ctx.turn;
|
||||
let call_id = ctx.call_id.to_string();
|
||||
let cwd = req.cwd.clone();
|
||||
let retry_reason = ctx.retry_reason.clone();
|
||||
let user_explicitly_approved = req.user_explicitly_approved;
|
||||
let approval_keys = self.approval_keys(req);
|
||||
let changes = req.changes.clone();
|
||||
Box::pin(async move {
|
||||
with_cached_approval(&session.services, key, move || async move {
|
||||
if let Some(reason) = retry_reason {
|
||||
session
|
||||
.request_command_approval(
|
||||
turn,
|
||||
call_id,
|
||||
vec!["apply_patch".to_string()],
|
||||
cwd,
|
||||
Some(reason),
|
||||
None,
|
||||
)
|
||||
.await
|
||||
} else if user_explicitly_approved {
|
||||
ReviewDecision::ApprovedForSession
|
||||
} else {
|
||||
ReviewDecision::Approved
|
||||
}
|
||||
if let Some(reason) = retry_reason {
|
||||
let rx_approve = session
|
||||
.request_patch_approval(turn, call_id, changes.clone(), Some(reason), None)
|
||||
.await;
|
||||
return rx_approve.await.unwrap_or_default();
|
||||
}
|
||||
|
||||
with_cached_approval(&session.services, approval_keys, || async move {
|
||||
let rx_approve = session
|
||||
.request_patch_approval(turn, call_id, changes, None, None)
|
||||
.await;
|
||||
rx_approve.await.unwrap_or_default()
|
||||
})
|
||||
.await
|
||||
})
|
||||
@@ -134,6 +124,17 @@ impl Approvable<ApplyPatchRequest> for ApplyPatchRuntime {
|
||||
fn wants_no_sandbox_approval(&self, policy: AskForApproval) -> bool {
|
||||
!matches!(policy, AskForApproval::Never)
|
||||
}
|
||||
|
||||
// apply_patch approvals are decided upstream by assess_patch_safety.
|
||||
//
|
||||
// This override ensures the orchestrator runs the patch approval flow when required instead
|
||||
// of falling back to the global exec approval policy.
|
||||
fn exec_approval_requirement(
|
||||
&self,
|
||||
req: &ApplyPatchRequest,
|
||||
) -> Option<ExecApprovalRequirement> {
|
||||
Some(req.exec_approval_requirement.clone())
|
||||
}
|
||||
}
|
||||
|
||||
impl ToolRuntime<ApplyPatchRequest, ExecToolCallOutput> for ApplyPatchRuntime {
|
||||
|
||||
@@ -74,12 +74,12 @@ impl Sandboxable for ShellRuntime {
|
||||
impl Approvable<ShellRequest> for ShellRuntime {
|
||||
type ApprovalKey = ApprovalKey;
|
||||
|
||||
fn approval_key(&self, req: &ShellRequest) -> Self::ApprovalKey {
|
||||
ApprovalKey {
|
||||
fn approval_keys(&self, req: &ShellRequest) -> Vec<Self::ApprovalKey> {
|
||||
vec![ApprovalKey {
|
||||
command: req.command.clone(),
|
||||
cwd: req.cwd.clone(),
|
||||
sandbox_permissions: req.sandbox_permissions,
|
||||
}
|
||||
}]
|
||||
}
|
||||
|
||||
fn start_approval_async<'a>(
|
||||
@@ -87,7 +87,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
req: &'a ShellRequest,
|
||||
ctx: ApprovalCtx<'a>,
|
||||
) -> BoxFuture<'a, ReviewDecision> {
|
||||
let key = self.approval_key(req);
|
||||
let keys = self.approval_keys(req);
|
||||
let command = req.command.clone();
|
||||
let cwd = req.cwd.clone();
|
||||
let reason = ctx
|
||||
@@ -98,7 +98,7 @@ impl Approvable<ShellRequest> for ShellRuntime {
|
||||
let turn = ctx.turn;
|
||||
let call_id = ctx.call_id.to_string();
|
||||
Box::pin(async move {
|
||||
with_cached_approval(&session.services, key, move || async move {
|
||||
with_cached_approval(&session.services, keys, move || async move {
|
||||
session
|
||||
.request_command_approval(
|
||||
turn,
|
||||
|
||||
@@ -92,12 +92,12 @@ impl Sandboxable for UnifiedExecRuntime<'_> {
|
||||
impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
type ApprovalKey = UnifiedExecApprovalKey;
|
||||
|
||||
fn approval_key(&self, req: &UnifiedExecRequest) -> Self::ApprovalKey {
|
||||
UnifiedExecApprovalKey {
|
||||
fn approval_keys(&self, req: &UnifiedExecRequest) -> Vec<Self::ApprovalKey> {
|
||||
vec![UnifiedExecApprovalKey {
|
||||
command: req.command.clone(),
|
||||
cwd: req.cwd.clone(),
|
||||
sandbox_permissions: req.sandbox_permissions,
|
||||
}
|
||||
}]
|
||||
}
|
||||
|
||||
fn start_approval_async<'b>(
|
||||
@@ -105,7 +105,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
req: &'b UnifiedExecRequest,
|
||||
ctx: ApprovalCtx<'b>,
|
||||
) -> BoxFuture<'b, ReviewDecision> {
|
||||
let key = self.approval_key(req);
|
||||
let keys = self.approval_keys(req);
|
||||
let session = ctx.session;
|
||||
let turn = ctx.turn;
|
||||
let call_id = ctx.call_id.to_string();
|
||||
@@ -116,7 +116,7 @@ impl Approvable<UnifiedExecRequest> for UnifiedExecRuntime<'_> {
|
||||
.clone()
|
||||
.or_else(|| req.justification.clone());
|
||||
Box::pin(async move {
|
||||
with_cached_approval(&session.services, key, || async move {
|
||||
with_cached_approval(&session.services, keys, || async move {
|
||||
session
|
||||
.request_command_approval(
|
||||
turn,
|
||||
|
||||
@@ -49,28 +49,44 @@ impl ApprovalStore {
|
||||
}
|
||||
}
|
||||
|
||||
/// Takes a vector of approval keys and returns a ReviewDecision.
|
||||
/// There will be one key in most cases, but apply_patch can modify multiple files at once.
|
||||
///
|
||||
/// - If all keys are already approved for session, we skip prompting.
|
||||
/// - If the user approves for session, we store the decision for each key individually
|
||||
/// so future requests touching any subset can also skip prompting.
|
||||
pub(crate) async fn with_cached_approval<K, F, Fut>(
|
||||
services: &SessionServices,
|
||||
key: K,
|
||||
keys: Vec<K>,
|
||||
fetch: F,
|
||||
) -> ReviewDecision
|
||||
where
|
||||
K: Serialize + Clone,
|
||||
K: Serialize,
|
||||
F: FnOnce() -> Fut,
|
||||
Fut: Future<Output = ReviewDecision>,
|
||||
{
|
||||
{
|
||||
// To be defensive here, don't bother with checking the cache if keys are empty.
|
||||
if keys.is_empty() {
|
||||
return fetch().await;
|
||||
}
|
||||
|
||||
let already_approved = {
|
||||
let store = services.tool_approvals.lock().await;
|
||||
if let Some(decision) = store.get(&key) {
|
||||
return decision;
|
||||
}
|
||||
keys.iter()
|
||||
.all(|key| matches!(store.get(key), Some(ReviewDecision::ApprovedForSession)))
|
||||
};
|
||||
|
||||
if already_approved {
|
||||
return ReviewDecision::ApprovedForSession;
|
||||
}
|
||||
|
||||
let decision = fetch().await;
|
||||
|
||||
if matches!(decision, ReviewDecision::ApprovedForSession) {
|
||||
let mut store = services.tool_approvals.lock().await;
|
||||
store.put(key, ReviewDecision::ApprovedForSession);
|
||||
for key in keys {
|
||||
store.put(key, ReviewDecision::ApprovedForSession);
|
||||
}
|
||||
}
|
||||
|
||||
decision
|
||||
@@ -161,7 +177,14 @@ pub(crate) enum SandboxOverride {
|
||||
pub(crate) trait Approvable<Req> {
|
||||
type ApprovalKey: Hash + Eq + Clone + Debug + Serialize;
|
||||
|
||||
fn approval_key(&self, req: &Req) -> Self::ApprovalKey;
|
||||
// In most cases (shell, unified_exec), a request will have a single approval key.
|
||||
//
|
||||
// However, apply_patch needs session "approve once, don't ask again" semantics that
|
||||
// apply to multiple atomic targets (e.g., apply_patch approves per file path). Returning
|
||||
// a list of keys lets the runtime treat the request as approved-for-session only if
|
||||
// *all* keys are already approved, while still caching approvals per-key so future
|
||||
// requests touching a subset can be auto-approved.
|
||||
fn approval_keys(&self, req: &Req) -> Vec<Self::ApprovalKey>;
|
||||
|
||||
/// Some tools may request to skip the sandbox on the first attempt
|
||||
/// (e.g., when the request explicitly asks for escalated permissions).
|
||||
|
||||
@@ -22,7 +22,6 @@ pub(crate) struct ToolsConfig {
|
||||
pub apply_patch_tool_type: Option<ApplyPatchToolType>,
|
||||
pub web_search_request: bool,
|
||||
pub web_search_cached: bool,
|
||||
pub include_view_image_tool: bool,
|
||||
pub experimental_supported_tools: Vec<String>,
|
||||
}
|
||||
|
||||
@@ -40,7 +39,6 @@ impl ToolsConfig {
|
||||
let include_apply_patch_tool = features.enabled(Feature::ApplyPatchFreeform);
|
||||
let include_web_search_request = features.enabled(Feature::WebSearchRequest);
|
||||
let include_web_search_cached = features.enabled(Feature::WebSearchCached);
|
||||
let include_view_image_tool = features.enabled(Feature::ViewImageTool);
|
||||
|
||||
let shell_type = if !features.enabled(Feature::ShellTool) {
|
||||
ConfigShellToolType::Disabled
|
||||
@@ -72,7 +70,6 @@ impl ToolsConfig {
|
||||
apply_patch_tool_type,
|
||||
web_search_request: include_web_search_request,
|
||||
web_search_cached: include_web_search_cached,
|
||||
include_view_image_tool,
|
||||
experimental_supported_tools: model_info.experimental_supported_tools.clone(),
|
||||
}
|
||||
}
|
||||
@@ -1107,10 +1104,8 @@ pub(crate) fn build_specs(
|
||||
});
|
||||
}
|
||||
|
||||
if config.include_view_image_tool {
|
||||
builder.push_spec_with_parallel_support(create_view_image_tool(), true);
|
||||
builder.register_handler("view_image", view_image_handler);
|
||||
}
|
||||
builder.push_spec_with_parallel_support(create_view_image_tool(), true);
|
||||
builder.register_handler("view_image", view_image_handler);
|
||||
|
||||
if let Some(mcp_tools) = mcp_tools {
|
||||
let mut entries: Vec<(String, mcp_types::Tool)> = mcp_tools.into_iter().collect();
|
||||
@@ -1236,7 +1231,6 @@ mod tests {
|
||||
let mut features = Features::with_defaults();
|
||||
features.enable(Feature::UnifiedExec);
|
||||
features.enable(Feature::WebSearchRequest);
|
||||
features.enable(Feature::ViewImageTool);
|
||||
let config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
@@ -1556,7 +1550,6 @@ mod tests {
|
||||
let config = test_config();
|
||||
let model_info = ModelsManager::construct_model_info_offline("gpt-5-codex", &config);
|
||||
let mut features = Features::with_defaults();
|
||||
features.disable(Feature::ViewImageTool);
|
||||
features.enable(Feature::UnifiedExec);
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
@@ -1575,8 +1568,7 @@ mod tests {
|
||||
fn test_test_model_info_includes_sync_tool() {
|
||||
let config = test_config();
|
||||
let model_info = ModelsManager::construct_model_info_offline("test-gpt-5-codex", &config);
|
||||
let mut features = Features::with_defaults();
|
||||
features.disable(Feature::ViewImageTool);
|
||||
let features = Features::with_defaults();
|
||||
let tools_config = ToolsConfig::new(&ToolsConfigParams {
|
||||
model_info: &model_info,
|
||||
features: &features,
|
||||
|
||||
@@ -2,7 +2,6 @@
|
||||
//! and suffix on UTF-8 boundaries, and helpers for line/token‑based truncation
|
||||
//! used across the core crate.
|
||||
|
||||
use crate::config::Config;
|
||||
use codex_protocol::models::FunctionCallOutputContentItem;
|
||||
use codex_protocol::openai_models::TruncationMode;
|
||||
use codex_protocol::openai_models::TruncationPolicyConfig;
|
||||
@@ -47,27 +46,6 @@ impl TruncationPolicy {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn new(config: &Config, truncation_policy: TruncationPolicy) -> Self {
|
||||
let config_token_limit = config.tool_output_token_limit;
|
||||
|
||||
match truncation_policy {
|
||||
TruncationPolicy::Bytes(family_bytes) => {
|
||||
if let Some(token_limit) = config_token_limit {
|
||||
Self::Bytes(approx_bytes_for_tokens(token_limit))
|
||||
} else {
|
||||
Self::Bytes(family_bytes)
|
||||
}
|
||||
}
|
||||
TruncationPolicy::Tokens(family_tokens) => {
|
||||
if let Some(token_limit) = config_token_limit {
|
||||
Self::Tokens(token_limit)
|
||||
} else {
|
||||
Self::Tokens(family_tokens)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns a token budget derived from this policy.
|
||||
///
|
||||
/// - For `Tokens`, this is the explicit token limit.
|
||||
@@ -313,7 +291,7 @@ pub(crate) fn approx_token_count(text: &str) -> usize {
|
||||
len.saturating_add(APPROX_BYTES_PER_TOKEN.saturating_sub(1)) / APPROX_BYTES_PER_TOKEN
|
||||
}
|
||||
|
||||
fn approx_bytes_for_tokens(tokens: usize) -> usize {
|
||||
pub(crate) fn approx_bytes_for_tokens(tokens: usize) -> usize {
|
||||
tokens.saturating_mul(APPROX_BYTES_PER_TOKEN)
|
||||
}
|
||||
|
||||
|
||||
@@ -1561,6 +1561,123 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[cfg(unix)]
|
||||
async fn approving_apply_patch_for_session_skips_future_prompts_for_same_file() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let approval_policy = AskForApproval::OnRequest;
|
||||
let sandbox_policy = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
};
|
||||
let sandbox_policy_for_config = sandbox_policy.clone();
|
||||
|
||||
let mut builder = test_codex()
|
||||
.with_model("gpt-5.1-codex")
|
||||
.with_config(move |config| {
|
||||
config.approval_policy = Constrained::allow_any(approval_policy);
|
||||
config.sandbox_policy = Constrained::allow_any(sandbox_policy_for_config);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let target = TargetPath::OutsideWorkspace("apply_patch_allow_session.txt");
|
||||
let (path, patch_path) = target.resolve_for_patch(&test);
|
||||
let _ = fs::remove_file(&path);
|
||||
|
||||
let patch_add = build_add_file_patch(&patch_path, "before");
|
||||
let patch_update = format!(
|
||||
"*** Begin Patch\n*** Update File: {patch_path}\n@@\n-before\n+after\n*** End Patch\n"
|
||||
);
|
||||
|
||||
let call_id_1 = "apply_patch_allow_session_1";
|
||||
let call_id_2 = "apply_patch_allow_session_2";
|
||||
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_apply_patch_function_call(call_id_1, &patch_add),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_turn(
|
||||
&test,
|
||||
"apply_patch allow session",
|
||||
approval_policy,
|
||||
sandbox_policy.clone(),
|
||||
)
|
||||
.await?;
|
||||
let _ = expect_patch_approval(&test, call_id_1).await;
|
||||
test.codex
|
||||
.submit(Op::PatchApproval {
|
||||
id: "0".into(),
|
||||
decision: ReviewDecision::ApprovedForSession,
|
||||
})
|
||||
.await?;
|
||||
wait_for_completion(&test).await;
|
||||
assert!(fs::read_to_string(&path)?.contains("before"));
|
||||
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_response_created("resp-3"),
|
||||
ev_apply_patch_function_call(call_id_2, &patch_update),
|
||||
ev_completed("resp-3"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
let _ = mount_sse_once(
|
||||
&server,
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-2", "done"),
|
||||
ev_completed("resp-4"),
|
||||
]),
|
||||
)
|
||||
.await;
|
||||
|
||||
submit_turn(
|
||||
&test,
|
||||
"apply_patch allow session followup",
|
||||
approval_policy,
|
||||
sandbox_policy.clone(),
|
||||
)
|
||||
.await?;
|
||||
|
||||
let event = wait_for_event(&test.codex, |event| {
|
||||
matches!(
|
||||
event,
|
||||
EventMsg::ApplyPatchApprovalRequest(_) | EventMsg::TaskComplete(_)
|
||||
)
|
||||
})
|
||||
.await;
|
||||
match event {
|
||||
EventMsg::TaskComplete(_) => {}
|
||||
EventMsg::ApplyPatchApprovalRequest(event) => {
|
||||
panic!("unexpected patch approval request: {:?}", event.call_id)
|
||||
}
|
||||
other => panic!("unexpected event: {other:?}"),
|
||||
}
|
||||
|
||||
assert!(fs::read_to_string(&path)?.contains("after"));
|
||||
let _ = fs::remove_file(path);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[cfg(unix)]
|
||||
async fn approving_execpolicy_amendment_persists_policy_and_skips_future_prompts() -> Result<()> {
|
||||
|
||||
@@ -15,7 +15,6 @@ use codex_core::WireApi;
|
||||
use codex_core::auth::AuthCredentialsStoreMode;
|
||||
use codex_core::built_in_model_providers;
|
||||
use codex_core::error::CodexErr;
|
||||
use codex_core::features::Feature;
|
||||
use codex_core::models_manager::manager::ModelsManager;
|
||||
use codex_core::protocol::EventMsg;
|
||||
use codex_core::protocol::Op;
|
||||
@@ -683,7 +682,6 @@ async fn skills_append_to_instructions() {
|
||||
let mut config = load_default_config_for_test(&codex_home).await;
|
||||
config.model_provider = model_provider;
|
||||
config.cwd = codex_home.path().to_path_buf();
|
||||
config.features.enable(Feature::Skills);
|
||||
|
||||
let thread_manager = ThreadManager::with_models_provider_and_home(
|
||||
CodexAuth::from_api_key("Test API Key"),
|
||||
|
||||
@@ -35,6 +35,7 @@ mod json_result;
|
||||
mod list_dir;
|
||||
mod list_models;
|
||||
mod live_cli;
|
||||
mod model_info_overrides;
|
||||
mod model_overrides;
|
||||
mod model_tools;
|
||||
mod models_etag_responses;
|
||||
@@ -43,6 +44,7 @@ mod prompt_caching;
|
||||
mod quota_exceeded;
|
||||
mod read_file;
|
||||
mod remote_models;
|
||||
mod request_compression;
|
||||
mod resume;
|
||||
mod resume_warning;
|
||||
mod review;
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
use codex_core::models_manager::manager::ModelsManager;
|
||||
use codex_protocol::openai_models::TruncationPolicyConfig;
|
||||
use core_test_support::load_default_config_for_test;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn offline_model_info_without_tool_output_override() {
|
||||
let codex_home = TempDir::new().expect("create temp dir");
|
||||
let config = load_default_config_for_test(&codex_home).await;
|
||||
|
||||
let model_info = ModelsManager::construct_model_info_offline("gpt-5.1", &config);
|
||||
|
||||
assert_eq!(
|
||||
model_info.truncation_policy,
|
||||
TruncationPolicyConfig::bytes(10_000)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn offline_model_info_with_tool_output_override() {
|
||||
let codex_home = TempDir::new().expect("create temp dir");
|
||||
let mut config = load_default_config_for_test(&codex_home).await;
|
||||
config.tool_output_token_limit = Some(123);
|
||||
|
||||
let model_info = ModelsManager::construct_model_info_offline("gpt-5.1-codex", &config);
|
||||
|
||||
assert_eq!(
|
||||
model_info.truncation_policy,
|
||||
TruncationPolicyConfig::tokens(123)
|
||||
);
|
||||
}
|
||||
@@ -186,6 +186,95 @@ async fn remote_models_remote_model_uses_unified_exec() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn remote_models_truncation_policy_without_override_preserves_remote() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
|
||||
let server = MockServer::builder()
|
||||
.body_print_limit(BodyPrintLimit::Limited(80_000))
|
||||
.start()
|
||||
.await;
|
||||
|
||||
let slug = "codex-test-truncation-policy";
|
||||
let remote_model = test_remote_model_with_policy(
|
||||
slug,
|
||||
ModelVisibility::List,
|
||||
1,
|
||||
TruncationPolicyConfig::bytes(12_000),
|
||||
);
|
||||
mount_models_once(
|
||||
&server,
|
||||
ModelsResponse {
|
||||
models: vec![remote_model],
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let harness = build_remote_models_harness(&server, |config| {
|
||||
config.model = Some("gpt-5.1".to_string());
|
||||
})
|
||||
.await?;
|
||||
|
||||
let models_manager = harness.thread_manager.get_models_manager();
|
||||
wait_for_model_available(&models_manager, slug, &harness.config).await;
|
||||
|
||||
let model_info = models_manager
|
||||
.construct_model_info(slug, &harness.config)
|
||||
.await;
|
||||
assert_eq!(
|
||||
model_info.truncation_policy,
|
||||
TruncationPolicyConfig::bytes(12_000)
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn remote_models_truncation_policy_with_tool_output_override() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
|
||||
let server = MockServer::builder()
|
||||
.body_print_limit(BodyPrintLimit::Limited(80_000))
|
||||
.start()
|
||||
.await;
|
||||
|
||||
let slug = "codex-test-truncation-override";
|
||||
let remote_model = test_remote_model_with_policy(
|
||||
slug,
|
||||
ModelVisibility::List,
|
||||
1,
|
||||
TruncationPolicyConfig::bytes(10_000),
|
||||
);
|
||||
mount_models_once(
|
||||
&server,
|
||||
ModelsResponse {
|
||||
models: vec![remote_model],
|
||||
},
|
||||
)
|
||||
.await;
|
||||
|
||||
let harness = build_remote_models_harness(&server, |config| {
|
||||
config.model = Some("gpt-5.1".to_string());
|
||||
config.tool_output_token_limit = Some(50);
|
||||
})
|
||||
.await?;
|
||||
|
||||
let models_manager = harness.thread_manager.get_models_manager();
|
||||
wait_for_model_available(&models_manager, slug, &harness.config).await;
|
||||
|
||||
let model_info = models_manager
|
||||
.construct_model_info(slug, &harness.config)
|
||||
.await;
|
||||
assert_eq!(
|
||||
model_info.truncation_policy,
|
||||
TruncationPolicyConfig::bytes(200)
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn remote_models_apply_remote_base_instructions() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -461,6 +550,20 @@ where
|
||||
}
|
||||
|
||||
fn test_remote_model(slug: &str, visibility: ModelVisibility, priority: i32) -> ModelInfo {
|
||||
test_remote_model_with_policy(
|
||||
slug,
|
||||
visibility,
|
||||
priority,
|
||||
TruncationPolicyConfig::bytes(10_000),
|
||||
)
|
||||
}
|
||||
|
||||
fn test_remote_model_with_policy(
|
||||
slug: &str,
|
||||
visibility: ModelVisibility,
|
||||
priority: i32,
|
||||
truncation_policy: TruncationPolicyConfig,
|
||||
) -> ModelInfo {
|
||||
ModelInfo {
|
||||
slug: slug.to_string(),
|
||||
display_name: format!("{slug} display"),
|
||||
@@ -480,7 +583,7 @@ fn test_remote_model(slug: &str, visibility: ModelVisibility, priority: i32) ->
|
||||
support_verbosity: false,
|
||||
default_verbosity: None,
|
||||
apply_patch_tool_type: None,
|
||||
truncation_policy: TruncationPolicyConfig::bytes(10_000),
|
||||
truncation_policy,
|
||||
supports_parallel_tool_calls: false,
|
||||
context_window: Some(272_000),
|
||||
auto_compact_token_limit: None,
|
||||
|
||||
@@ -0,0 +1,116 @@
|
||||
#![cfg(not(target_os = "windows"))]
|
||||
|
||||
use codex_core::CodexAuth;
|
||||
use codex_core::features::Feature;
|
||||
use codex_core::protocol::EventMsg;
|
||||
use codex_core::protocol::Op;
|
||||
use codex_protocol::user_input::UserInput;
|
||||
use core_test_support::responses::ev_completed;
|
||||
use core_test_support::responses::ev_response_created;
|
||||
use core_test_support::responses::get_responses_requests;
|
||||
use core_test_support::responses::mount_sse_once;
|
||||
use core_test_support::responses::sse;
|
||||
use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn request_body_is_zstd_compressed_for_codex_backend_when_enabled() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
mount_sse_once(
|
||||
&server,
|
||||
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let base_url = format!("{}/backend-api/codex/v1", server.uri());
|
||||
let mut builder = test_codex()
|
||||
.with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing())
|
||||
.with_config(move |config| {
|
||||
config.features.enable(Feature::EnableRequestCompression);
|
||||
config.model_provider.base_url = Some(base_url);
|
||||
});
|
||||
let codex = builder.build(&server).await?.codex;
|
||||
|
||||
codex
|
||||
.submit(Op::UserInput {
|
||||
items: vec![UserInput::Text {
|
||||
text: "compress me".into(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
// Wait until the task completes so the request definitely hit the server.
|
||||
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
|
||||
|
||||
let requests = get_responses_requests(&server).await;
|
||||
assert_eq!(requests.len(), 1);
|
||||
|
||||
let request = &requests[0];
|
||||
let content_encoding = request
|
||||
.headers
|
||||
.get("content-encoding")
|
||||
.and_then(|v| v.to_str().ok());
|
||||
assert_eq!(content_encoding, Some("zstd"));
|
||||
|
||||
let decompressed = zstd::stream::decode_all(std::io::Cursor::new(request.body.clone()))?;
|
||||
let json: serde_json::Value = serde_json::from_slice(&decompressed)?;
|
||||
assert!(
|
||||
json.get("input").is_some(),
|
||||
"expected request body to decode as Responses API JSON"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn request_body_is_not_compressed_for_api_key_auth_even_when_enabled() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
mount_sse_once(
|
||||
&server,
|
||||
sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]),
|
||||
)
|
||||
.await;
|
||||
|
||||
let base_url = format!("{}/backend-api/codex/v1", server.uri());
|
||||
let mut builder = test_codex().with_config(move |config| {
|
||||
config.features.enable(Feature::EnableRequestCompression);
|
||||
config.model_provider.base_url = Some(base_url);
|
||||
});
|
||||
let codex = builder.build(&server).await?.codex;
|
||||
|
||||
codex
|
||||
.submit(Op::UserInput {
|
||||
items: vec![UserInput::Text {
|
||||
text: "do not compress".into(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
})
|
||||
.await?;
|
||||
|
||||
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
|
||||
|
||||
let requests = get_responses_requests(&server).await;
|
||||
assert_eq!(requests.len(), 1);
|
||||
|
||||
let request = &requests[0];
|
||||
assert!(
|
||||
request.headers.get("content-encoding").is_none(),
|
||||
"did not expect request compression for API-key auth"
|
||||
);
|
||||
|
||||
let json: serde_json::Value = serde_json::from_slice(&request.body)?;
|
||||
assert!(
|
||||
json.get("input").is_some(),
|
||||
"expected request body to be plain Responses API JSON"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
@@ -2,7 +2,6 @@
|
||||
#![allow(clippy::unwrap_used, clippy::expect_used)]
|
||||
|
||||
use anyhow::Result;
|
||||
use codex_core::features::Feature;
|
||||
use codex_core::protocol::AskForApproval;
|
||||
use codex_core::protocol::Op;
|
||||
use codex_core::protocol::SandboxPolicy;
|
||||
@@ -41,13 +40,9 @@ async fn user_turn_includes_skill_instructions() -> Result<()> {
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let skill_body = "skill body";
|
||||
let mut builder = test_codex()
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::Skills);
|
||||
})
|
||||
.with_pre_build_hook(|home| {
|
||||
write_skill(home, "demo", "demo skill", skill_body);
|
||||
});
|
||||
let mut builder = test_codex().with_pre_build_hook(|home| {
|
||||
write_skill(home, "demo", "demo skill", skill_body);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let skill_path = test.codex_home_path().join("skills/demo/SKILL.md");
|
||||
@@ -111,15 +106,11 @@ async fn skill_load_errors_surface_in_session_configured() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex()
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::Skills);
|
||||
})
|
||||
.with_pre_build_hook(|home| {
|
||||
let skill_dir = home.join("skills").join("broken");
|
||||
fs::create_dir_all(&skill_dir).unwrap();
|
||||
fs::write(skill_dir.join("SKILL.md"), "not yaml").unwrap();
|
||||
});
|
||||
let mut builder = test_codex().with_pre_build_hook(|home| {
|
||||
let skill_dir = home.join("skills").join("broken");
|
||||
fs::create_dir_all(&skill_dir).unwrap();
|
||||
fs::write(skill_dir.join("SKILL.md"), "not yaml").unwrap();
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
test.codex
|
||||
@@ -169,17 +160,13 @@ async fn list_skills_includes_system_cache_entries() -> Result<()> {
|
||||
const SYSTEM_SKILL_NAME: &str = "skill-creator";
|
||||
|
||||
let server = start_mock_server().await;
|
||||
let mut builder = test_codex()
|
||||
.with_config(|config| {
|
||||
config.features.enable(Feature::Skills);
|
||||
})
|
||||
.with_pre_build_hook(|home| {
|
||||
let system_skill_path = system_skill_md_path(home, SYSTEM_SKILL_NAME);
|
||||
assert!(
|
||||
!system_skill_path.exists(),
|
||||
"expected embedded system skills not yet installed, but {system_skill_path:?} exists"
|
||||
);
|
||||
});
|
||||
let mut builder = test_codex().with_pre_build_hook(|home| {
|
||||
let system_skill_path = system_skill_md_path(home, SYSTEM_SKILL_NAME);
|
||||
assert!(
|
||||
!system_skill_path.exists(),
|
||||
"expected embedded system skills not yet installed, but {system_skill_path:?} exists"
|
||||
);
|
||||
});
|
||||
let test = builder.build(&server).await?;
|
||||
|
||||
let system_skill_path = system_skill_md_path(test.codex_home_path(), SYSTEM_SKILL_NAME);
|
||||
|
||||
@@ -498,6 +498,12 @@ fn patch_options() -> Vec<ApprovalOption> {
|
||||
display_shortcut: None,
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))],
|
||||
},
|
||||
ApprovalOption {
|
||||
label: "Yes, and don't ask again for these files".to_string(),
|
||||
decision: ApprovalDecision::Review(ReviewDecision::ApprovedForSession),
|
||||
display_shortcut: None,
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))],
|
||||
},
|
||||
ApprovalOption {
|
||||
label: "No, and tell Codex what to do differently".to_string(),
|
||||
decision: ApprovalDecision::Review(ReviewDecision::Abort),
|
||||
|
||||
@@ -147,6 +147,7 @@ use strum::IntoEnumIterator;
|
||||
|
||||
const USER_SHELL_COMMAND_HELP_TITLE: &str = "Prefix a command with ! to run it locally";
|
||||
const USER_SHELL_COMMAND_HELP_HINT: &str = "Example: !ls";
|
||||
const DEFAULT_OPENAI_BASE_URL: &str = "https://api.openai.com/v1";
|
||||
// Track information about an in-flight exec command.
|
||||
struct RunningCommand {
|
||||
command: Vec<String>,
|
||||
@@ -2453,6 +2454,45 @@ impl ChatWidget {
|
||||
self.open_model_popup_with_presets(presets);
|
||||
}
|
||||
|
||||
fn model_menu_header(&self, title: &str, subtitle: &str) -> Box<dyn Renderable> {
|
||||
let title = title.to_string();
|
||||
let subtitle = subtitle.to_string();
|
||||
let mut header = ColumnRenderable::new();
|
||||
header.push(Line::from(title.bold()));
|
||||
header.push(Line::from(subtitle.dim()));
|
||||
if let Some(warning) = self.model_menu_warning_line() {
|
||||
header.push(warning);
|
||||
}
|
||||
Box::new(header)
|
||||
}
|
||||
|
||||
fn model_menu_warning_line(&self) -> Option<Line<'static>> {
|
||||
let base_url = self.custom_openai_base_url()?;
|
||||
let warning = format!(
|
||||
"Warning: OPENAI_BASE_URL is set to {base_url}. Selecting models may not be supported or work properly."
|
||||
);
|
||||
Some(Line::from(warning.red()))
|
||||
}
|
||||
|
||||
fn custom_openai_base_url(&self) -> Option<String> {
|
||||
if !self.config.model_provider.is_openai() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let base_url = self.config.model_provider.base_url.as_ref()?;
|
||||
let trimmed = base_url.trim();
|
||||
if trimmed.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let normalized = trimmed.trim_end_matches('/');
|
||||
if normalized == DEFAULT_OPENAI_BASE_URL {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(trimmed.to_string())
|
||||
}
|
||||
|
||||
pub(crate) fn open_model_popup_with_presets(&mut self, presets: Vec<ModelPreset>) {
|
||||
let presets: Vec<ModelPreset> = presets
|
||||
.into_iter()
|
||||
@@ -2521,11 +2561,14 @@ impl ChatWidget {
|
||||
});
|
||||
}
|
||||
|
||||
let header = self.model_menu_header(
|
||||
"Select Model",
|
||||
"Pick a quick auto mode or browse all models.",
|
||||
);
|
||||
self.bottom_pane.show_selection_view(SelectionViewParams {
|
||||
title: Some("Select Model".to_string()),
|
||||
subtitle: Some("Pick a quick auto mode or browse all models.".to_string()),
|
||||
footer_hint: Some(standard_popup_hint_line()),
|
||||
items,
|
||||
header,
|
||||
..Default::default()
|
||||
});
|
||||
}
|
||||
@@ -2576,14 +2619,14 @@ impl ChatWidget {
|
||||
});
|
||||
}
|
||||
|
||||
let header = self.model_menu_header(
|
||||
"Select Model and Effort",
|
||||
"Access legacy models by running codex -m <model_name> or in your config.toml",
|
||||
);
|
||||
self.bottom_pane.show_selection_view(SelectionViewParams {
|
||||
title: Some("Select Model and Effort".to_string()),
|
||||
subtitle: Some(
|
||||
"Access legacy models by running codex -m <model_name> or in your config.toml"
|
||||
.to_string(),
|
||||
),
|
||||
footer_hint: Some("Press enter to select reasoning effort, or esc to dismiss.".into()),
|
||||
items,
|
||||
header,
|
||||
..Default::default()
|
||||
});
|
||||
}
|
||||
|
||||
+4
-1
@@ -2,6 +2,8 @@
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
expression: terminal.backend().vt100().screen().contents()
|
||||
---
|
||||
|
||||
|
||||
Would you like to make the following edits?
|
||||
|
||||
Reason: The model wants to apply changes
|
||||
@@ -12,6 +14,7 @@ expression: terminal.backend().vt100().screen().contents()
|
||||
2 +world
|
||||
|
||||
› 1. Yes, proceed (y)
|
||||
2. No, and tell Codex what to do differently (esc)
|
||||
2. Yes, and don't ask again for these files (a)
|
||||
3. No, and tell Codex what to do differently (esc)
|
||||
|
||||
Press enter to confirm or esc to cancel
|
||||
|
||||
@@ -498,6 +498,12 @@ fn patch_options() -> Vec<ApprovalOption> {
|
||||
display_shortcut: None,
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('y'))],
|
||||
},
|
||||
ApprovalOption {
|
||||
label: "Yes, and don't ask again for these files".to_string(),
|
||||
decision: ApprovalDecision::Review(ReviewDecision::ApprovedForSession),
|
||||
display_shortcut: None,
|
||||
additional_shortcuts: vec![key_hint::plain(KeyCode::Char('a'))],
|
||||
},
|
||||
ApprovalOption {
|
||||
label: "No, and tell Codex what to do differently".to_string(),
|
||||
decision: ApprovalDecision::Review(ReviewDecision::Abort),
|
||||
|
||||
+4
-1
@@ -2,6 +2,8 @@
|
||||
source: tui2/src/chatwidget/tests.rs
|
||||
expression: terminal.backend().vt100().screen().contents()
|
||||
---
|
||||
|
||||
|
||||
Would you like to make the following edits?
|
||||
|
||||
Reason: The model wants to apply changes
|
||||
@@ -12,6 +14,7 @@ expression: terminal.backend().vt100().screen().contents()
|
||||
2 +world
|
||||
|
||||
› 1. Yes, proceed (y)
|
||||
2. No, and tell Codex what to do differently (esc)
|
||||
2. Yes, and don't ask again for these files (a)
|
||||
3. No, and tell Codex what to do differently (esc)
|
||||
|
||||
Press enter to confirm or esc to cancel
|
||||
|
||||
+4
-1
@@ -2,6 +2,8 @@
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
expression: terminal.backend().vt100().screen().contents()
|
||||
---
|
||||
|
||||
|
||||
Would you like to make the following edits?
|
||||
|
||||
Reason: The model wants to apply changes
|
||||
@@ -12,6 +14,7 @@ expression: terminal.backend().vt100().screen().contents()
|
||||
2 +world
|
||||
|
||||
› 1. Yes, proceed (y)
|
||||
2. No, and tell Codex what to do differently (esc)
|
||||
2. Yes, and don't ask again for these files (a)
|
||||
3. No, and tell Codex what to do differently (esc)
|
||||
|
||||
Press enter to confirm or esc to cancel
|
||||
|
||||
@@ -17,7 +17,7 @@ use ts_rs::TS;
|
||||
/// using [AbsolutePathBufGuard::new]. If no base path is set, the
|
||||
/// deserialization will fail unless the path being deserialized is already
|
||||
/// absolute.
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, JsonSchema, TS)]
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, JsonSchema, TS)]
|
||||
pub struct AbsolutePathBuf(PathBuf);
|
||||
|
||||
impl AbsolutePathBuf {
|
||||
|
||||
Reference in New Issue
Block a user