mirror of
https://github.com/openai/codex.git
synced 2026-05-12 07:12:37 +00:00
Compare commits
28 Commits
dev/zhao/e
...
owen/test_
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f3135b8c54 | ||
|
|
77c457121e | ||
|
|
5ebdc9af1b | ||
|
|
f6a7da4ac3 | ||
|
|
1d09ac89a1 | ||
|
|
127e307f89 | ||
|
|
21ad1c1c90 | ||
|
|
349734e38d | ||
|
|
2222cab9ea | ||
|
|
c2f8c4e9f4 | ||
|
|
72b95db12f | ||
|
|
37ee6bf2c3 | ||
|
|
8b1e397211 | ||
|
|
85e687c74a | ||
|
|
9ee855ec57 | ||
|
|
4b78e2ab09 | ||
|
|
85e2fabc9f | ||
|
|
a8d5ad37b8 | ||
|
|
32e4a3a4d7 | ||
|
|
f443555728 | ||
|
|
ff4ca9959c | ||
|
|
5b25915d7e | ||
|
|
c0564edebe | ||
|
|
c936c68c84 | ||
|
|
41760f8a09 | ||
|
|
440c7acd8f | ||
|
|
0cc3b50228 | ||
|
|
8532876ad8 |
42
codex-rs/Cargo.lock
generated
42
codex-rs/Cargo.lock
generated
@@ -1068,6 +1068,7 @@ dependencies = [
|
||||
"serde_json",
|
||||
"thiserror 2.0.17",
|
||||
"tokio",
|
||||
"tracing",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
@@ -1186,6 +1187,7 @@ dependencies = [
|
||||
"seccompiler",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"serde_yaml",
|
||||
"serial_test",
|
||||
"sha1",
|
||||
"sha2",
|
||||
@@ -1254,7 +1256,6 @@ dependencies = [
|
||||
"async-trait",
|
||||
"clap",
|
||||
"codex-core",
|
||||
"codex-execpolicy",
|
||||
"libc",
|
||||
"path-absolutize",
|
||||
"pretty_assertions",
|
||||
@@ -1264,7 +1265,6 @@ dependencies = [
|
||||
"shlex",
|
||||
"socket2 0.6.0",
|
||||
"tempfile",
|
||||
"thiserror 2.0.17",
|
||||
"tokio",
|
||||
"tokio-util",
|
||||
"tracing",
|
||||
@@ -1283,6 +1283,7 @@ dependencies = [
|
||||
"serde_json",
|
||||
"shlex",
|
||||
"starlark",
|
||||
"tempfile",
|
||||
"thiserror 2.0.17",
|
||||
]
|
||||
|
||||
@@ -4466,6 +4467,12 @@ version = "1.0.15"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a"
|
||||
|
||||
[[package]]
|
||||
name = "pastey"
|
||||
version = "0.2.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "57d6c094ee800037dff99e02cab0eaf3142826586742a270ab3d7a62656bd27a"
|
||||
|
||||
[[package]]
|
||||
name = "path-absolutize"
|
||||
version = "3.1.1"
|
||||
@@ -5144,8 +5151,9 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "rmcp"
|
||||
version = "0.9.0"
|
||||
source = "git+https://github.com/bolinfest/rust-sdk?branch=pr556#4d9cc16f4c76c84486344f542ed9a3e9364019ba"
|
||||
version = "0.10.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "38b18323edc657390a6ed4d7a9110b0dec2dc3ed128eb2a123edfbafabdbddc5"
|
||||
dependencies = [
|
||||
"async-trait",
|
||||
"base64",
|
||||
@@ -5156,7 +5164,7 @@ dependencies = [
|
||||
"http-body",
|
||||
"http-body-util",
|
||||
"oauth2",
|
||||
"paste",
|
||||
"pastey",
|
||||
"pin-project-lite",
|
||||
"process-wrap",
|
||||
"rand 0.9.2",
|
||||
@@ -5178,8 +5186,9 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "rmcp-macros"
|
||||
version = "0.9.0"
|
||||
source = "git+https://github.com/bolinfest/rust-sdk?branch=pr556#4d9cc16f4c76c84486344f542ed9a3e9364019ba"
|
||||
version = "0.10.0"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "c75d0a62676bf8c8003c4e3c348e2ceb6a7b3e48323681aaf177fdccdac2ce50"
|
||||
dependencies = [
|
||||
"darling 0.21.3",
|
||||
"proc-macro2",
|
||||
@@ -5767,6 +5776,19 @@ dependencies = [
|
||||
"syn 2.0.104",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "serde_yaml"
|
||||
version = "0.9.34+deprecated"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "6a8b1a1a2ebf674015cc02edccce75287f1a0130d394307b36743c2f5d504b47"
|
||||
dependencies = [
|
||||
"indexmap 2.12.0",
|
||||
"itoa",
|
||||
"ryu",
|
||||
"serde",
|
||||
"unsafe-libyaml",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "serial2"
|
||||
version = "0.2.31"
|
||||
@@ -6981,6 +7003,12 @@ version = "0.2.6"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "ebc1c04c71510c7f702b52b7c350734c9ff1295c464a03335b00bb84fc54f853"
|
||||
|
||||
[[package]]
|
||||
name = "unsafe-libyaml"
|
||||
version = "0.2.11"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861"
|
||||
|
||||
[[package]]
|
||||
name = "untrusted"
|
||||
version = "0.9.0"
|
||||
|
||||
@@ -59,15 +59,15 @@ license = "Apache-2.0"
|
||||
# Internal
|
||||
app_test_support = { path = "app-server/tests/common" }
|
||||
codex-ansi-escape = { path = "ansi-escape" }
|
||||
codex-api = { path = "codex-api" }
|
||||
codex-app-server = { path = "app-server" }
|
||||
codex-app-server-protocol = { path = "app-server-protocol" }
|
||||
codex-apply-patch = { path = "apply-patch" }
|
||||
codex-arg0 = { path = "arg0" }
|
||||
codex-async-utils = { path = "async-utils" }
|
||||
codex-backend-client = { path = "backend-client" }
|
||||
codex-api = { path = "codex-api" }
|
||||
codex-client = { path = "codex-client" }
|
||||
codex-chatgpt = { path = "chatgpt" }
|
||||
codex-client = { path = "codex-client" }
|
||||
codex-common = { path = "common" }
|
||||
codex-core = { path = "core" }
|
||||
codex-exec = { path = "exec" }
|
||||
@@ -169,15 +169,16 @@ pulldown-cmark = "0.10"
|
||||
rand = "0.9"
|
||||
ratatui = "0.29.0"
|
||||
ratatui-macros = "0.6.0"
|
||||
regex-lite = "0.1.7"
|
||||
regex = "1.12.2"
|
||||
regex-lite = "0.1.7"
|
||||
reqwest = "0.12"
|
||||
rmcp = { version = "0.9.0", default-features = false }
|
||||
rmcp = { version = "0.10.0", default-features = false }
|
||||
schemars = "0.8.22"
|
||||
seccompiler = "0.5.0"
|
||||
sentry = "0.34.0"
|
||||
serde = "1"
|
||||
serde_json = "1"
|
||||
serde_yaml = "0.9"
|
||||
serde_with = "3.16"
|
||||
serial_test = "3.2.0"
|
||||
sha1 = "0.10.6"
|
||||
@@ -288,7 +289,6 @@ opt-level = 0
|
||||
# ratatui = { path = "../../ratatui" }
|
||||
crossterm = { git = "https://github.com/nornagon/crossterm", branch = "nornagon/color-query" }
|
||||
ratatui = { git = "https://github.com/nornagon/ratatui", branch = "nornagon-v0.29.0-patch" }
|
||||
rmcp = { git = "https://github.com/bolinfest/rust-sdk", branch = "pr556" }
|
||||
|
||||
# Uncomment to debug local changes.
|
||||
# rmcp = { path = "../../rust-sdk/crates/rmcp" }
|
||||
|
||||
@@ -164,6 +164,12 @@ client_request_definitions! {
|
||||
response: v2::FeedbackUploadResponse,
|
||||
},
|
||||
|
||||
/// Execute a command (argv vector) under the server's sandbox.
|
||||
OneOffCommandExec => "command/exec" {
|
||||
params: v2::CommandExecParams,
|
||||
response: v2::CommandExecResponse,
|
||||
},
|
||||
|
||||
ConfigRead => "config/read" {
|
||||
params: v2::ConfigReadParams,
|
||||
response: v2::ConfigReadResponse,
|
||||
@@ -511,6 +517,7 @@ server_notification_definitions! {
|
||||
ItemCompleted => "item/completed" (v2::ItemCompletedNotification),
|
||||
AgentMessageDelta => "item/agentMessage/delta" (v2::AgentMessageDeltaNotification),
|
||||
CommandExecutionOutputDelta => "item/commandExecution/outputDelta" (v2::CommandExecutionOutputDeltaNotification),
|
||||
FileChangeOutputDelta => "item/fileChange/outputDelta" (v2::FileChangeOutputDeltaNotification),
|
||||
McpToolCallProgress => "item/mcpToolCall/progress" (v2::McpToolCallProgressNotification),
|
||||
AccountUpdated => "account/updated" (v2::AccountUpdatedNotification),
|
||||
AccountRateLimitsUpdated => "account/rateLimits/updated" (v2::AccountRateLimitsUpdatedNotification),
|
||||
|
||||
15
codex-rs/app-server-protocol/src/protocol/mappers.rs
Normal file
15
codex-rs/app-server-protocol/src/protocol/mappers.rs
Normal file
@@ -0,0 +1,15 @@
|
||||
use crate::protocol::v1;
|
||||
use crate::protocol::v2;
|
||||
|
||||
impl From<v1::ExecOneOffCommandParams> for v2::CommandExecParams {
|
||||
fn from(value: v1::ExecOneOffCommandParams) -> Self {
|
||||
Self {
|
||||
command: value.command,
|
||||
timeout_ms: value
|
||||
.timeout_ms
|
||||
.map(|timeout| i64::try_from(timeout).unwrap_or(60_000)),
|
||||
cwd: value.cwd,
|
||||
sandbox_policy: value.sandbox_policy.map(std::convert::Into::into),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -2,6 +2,7 @@
|
||||
// Exposes protocol pieces used by `lib.rs` via `pub use protocol::common::*;`.
|
||||
|
||||
pub mod common;
|
||||
mod mappers;
|
||||
pub mod thread_history;
|
||||
pub mod v1;
|
||||
pub mod v2;
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use crate::protocol::v2::ThreadItem;
|
||||
use crate::protocol::v2::Turn;
|
||||
use crate::protocol::v2::TurnError;
|
||||
use crate::protocol::v2::TurnStatus;
|
||||
use crate::protocol::v2::UserInput;
|
||||
use codex_protocol::protocol::AgentReasoningEvent;
|
||||
@@ -142,6 +143,7 @@ impl ThreadHistoryBuilder {
|
||||
PendingTurn {
|
||||
id: self.next_turn_id(),
|
||||
items: Vec::new(),
|
||||
error: None,
|
||||
status: TurnStatus::Completed,
|
||||
}
|
||||
}
|
||||
@@ -190,6 +192,7 @@ impl ThreadHistoryBuilder {
|
||||
struct PendingTurn {
|
||||
id: String,
|
||||
items: Vec<ThreadItem>,
|
||||
error: Option<TurnError>,
|
||||
status: TurnStatus,
|
||||
}
|
||||
|
||||
@@ -198,6 +201,7 @@ impl From<PendingTurn> for Turn {
|
||||
Self {
|
||||
id: value.id,
|
||||
items: value.items,
|
||||
error: value.error,
|
||||
status: value.status,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -632,6 +632,26 @@ pub struct FeedbackUploadResponse {
|
||||
pub thread_id: String,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct CommandExecParams {
|
||||
pub command: Vec<String>,
|
||||
#[ts(type = "number | null")]
|
||||
pub timeout_ms: Option<i64>,
|
||||
pub cwd: Option<PathBuf>,
|
||||
pub sandbox_policy: Option<SandboxPolicy>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct CommandExecResponse {
|
||||
pub exit_code: i32,
|
||||
pub stdout: String,
|
||||
pub stderr: String,
|
||||
}
|
||||
|
||||
// === Threads, Turns, and Items ===
|
||||
// Thread APIs
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema, TS)]
|
||||
@@ -766,6 +786,7 @@ pub struct Thread {
|
||||
/// Model provider used for this thread (for example, 'openai').
|
||||
pub model_provider: String,
|
||||
/// Unix timestamp (in seconds) when the thread was created.
|
||||
#[ts(type = "number")]
|
||||
pub created_at: i64,
|
||||
/// [UNSTABLE] Path to the thread on disk.
|
||||
pub path: PathBuf,
|
||||
@@ -856,8 +877,9 @@ pub struct Turn {
|
||||
/// For all other responses and notifications returning a Turn,
|
||||
/// the items field will be an empty list.
|
||||
pub items: Vec<ThreadItem>,
|
||||
#[serde(flatten)]
|
||||
pub status: TurnStatus,
|
||||
/// Only populated when the Turn's status is failed.
|
||||
pub error: Option<TurnError>,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS, Error)]
|
||||
@@ -879,12 +901,12 @@ pub struct ErrorNotification {
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(tag = "status", rename_all = "camelCase")]
|
||||
#[ts(tag = "status", export_to = "v2/")]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub enum TurnStatus {
|
||||
Completed,
|
||||
Interrupted,
|
||||
Failed { error: TurnError },
|
||||
Failed,
|
||||
InProgress,
|
||||
}
|
||||
|
||||
@@ -1053,6 +1075,7 @@ pub enum ThreadItem {
|
||||
/// The command's exit code.
|
||||
exit_code: Option<i32>,
|
||||
/// The duration of the command execution in milliseconds.
|
||||
#[ts(type = "number | null")]
|
||||
duration_ms: Option<i64>,
|
||||
},
|
||||
#[serde(rename_all = "camelCase")]
|
||||
@@ -1319,6 +1342,7 @@ pub struct ReasoningSummaryTextDeltaNotification {
|
||||
pub turn_id: String,
|
||||
pub item_id: String,
|
||||
pub delta: String,
|
||||
#[ts(type = "number")]
|
||||
pub summary_index: i64,
|
||||
}
|
||||
|
||||
@@ -1329,6 +1353,7 @@ pub struct ReasoningSummaryPartAddedNotification {
|
||||
pub thread_id: String,
|
||||
pub turn_id: String,
|
||||
pub item_id: String,
|
||||
#[ts(type = "number")]
|
||||
pub summary_index: i64,
|
||||
}
|
||||
|
||||
@@ -1340,6 +1365,7 @@ pub struct ReasoningTextDeltaNotification {
|
||||
pub turn_id: String,
|
||||
pub item_id: String,
|
||||
pub delta: String,
|
||||
#[ts(type = "number")]
|
||||
pub content_index: i64,
|
||||
}
|
||||
|
||||
@@ -1353,6 +1379,16 @@ pub struct CommandExecutionOutputDeltaNotification {
|
||||
pub delta: String,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct FileChangeOutputDeltaNotification {
|
||||
pub thread_id: String,
|
||||
pub turn_id: String,
|
||||
pub item_id: String,
|
||||
pub delta: String,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
@@ -1464,7 +1500,9 @@ impl From<CoreRateLimitSnapshot> for RateLimitSnapshot {
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct RateLimitWindow {
|
||||
pub used_percent: i32,
|
||||
#[ts(type = "number | null")]
|
||||
pub window_duration_mins: Option<i64>,
|
||||
#[ts(type = "number | null")]
|
||||
pub resets_at: Option<i64>,
|
||||
}
|
||||
|
||||
|
||||
@@ -98,6 +98,9 @@ enum CliCommand {
|
||||
#[arg()]
|
||||
user_message: Option<String>,
|
||||
},
|
||||
/// Start two V2 turns that exercise apply_patch without requiring approval.
|
||||
#[command(name = "trigger-patch-update")]
|
||||
TriggerPatchUpdate,
|
||||
/// Start a V2 turn that should not elicit an ExecCommand approval.
|
||||
#[command(name = "no-trigger-cmd-approval")]
|
||||
NoTriggerCmdApproval,
|
||||
@@ -128,6 +131,7 @@ fn main() -> Result<()> {
|
||||
CliCommand::TriggerPatchApproval { user_message } => {
|
||||
trigger_patch_approval(codex_bin, user_message)
|
||||
}
|
||||
CliCommand::TriggerPatchUpdate => trigger_patch_update(codex_bin),
|
||||
CliCommand::NoTriggerCmdApproval => no_trigger_cmd_approval(codex_bin),
|
||||
CliCommand::SendFollowUpV2 {
|
||||
first_message,
|
||||
@@ -188,6 +192,26 @@ fn trigger_patch_approval(codex_bin: String, user_message: Option<String>) -> Re
|
||||
)
|
||||
}
|
||||
|
||||
fn trigger_patch_update(codex_bin: String) -> Result<()> {
|
||||
let first_message =
|
||||
"Create a file called apply_patch_demo.txt containing 'new line' using apply_patch";
|
||||
let follow_up_message =
|
||||
"Update apply_patch_demo.txt to contain just 'updated line' using apply_patch";
|
||||
let workspace_write = SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: Vec::new(),
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
};
|
||||
send_two_turns_with_policies(
|
||||
codex_bin,
|
||||
first_message.to_string(),
|
||||
follow_up_message.to_string(),
|
||||
Some(AskForApproval::Never),
|
||||
Some(workspace_write),
|
||||
)
|
||||
}
|
||||
|
||||
fn no_trigger_cmd_approval(codex_bin: String) -> Result<()> {
|
||||
let prompt = "Run `touch should_not_trigger_approval.txt`";
|
||||
send_message_v2_with_policies(codex_bin, prompt.to_string(), None, None)
|
||||
@@ -226,6 +250,16 @@ fn send_follow_up_v2(
|
||||
codex_bin: String,
|
||||
first_message: String,
|
||||
follow_up_message: String,
|
||||
) -> Result<()> {
|
||||
send_two_turns_with_policies(codex_bin, first_message, follow_up_message, None, None)
|
||||
}
|
||||
|
||||
fn send_two_turns_with_policies(
|
||||
codex_bin: String,
|
||||
first_message: String,
|
||||
follow_up_message: String,
|
||||
approval_policy: Option<AskForApproval>,
|
||||
sandbox_policy: Option<SandboxPolicy>,
|
||||
) -> Result<()> {
|
||||
let mut client = CodexClient::spawn(codex_bin)?;
|
||||
|
||||
@@ -240,6 +274,8 @@ fn send_follow_up_v2(
|
||||
input: vec![V2UserInput::Text {
|
||||
text: first_message,
|
||||
}],
|
||||
approval_policy: approval_policy.clone(),
|
||||
sandbox_policy: sandbox_policy.clone(),
|
||||
..Default::default()
|
||||
};
|
||||
let first_turn_response = client.turn_start(first_turn_params)?;
|
||||
@@ -251,6 +287,8 @@ fn send_follow_up_v2(
|
||||
input: vec![V2UserInput::Text {
|
||||
text: follow_up_message,
|
||||
}],
|
||||
approval_policy,
|
||||
sandbox_policy,
|
||||
..Default::default()
|
||||
};
|
||||
let follow_up_response = client.turn_start(follow_up_params)?;
|
||||
@@ -563,7 +601,9 @@ impl CodexClient {
|
||||
ServerNotification::TurnCompleted(payload) => {
|
||||
if payload.turn.id == turn_id {
|
||||
println!("\n< turn/completed notification: {:?}", payload.turn.status);
|
||||
if let TurnStatus::Failed { error } = &payload.turn.status {
|
||||
if payload.turn.status == TurnStatus::Failed
|
||||
&& let Some(error) = payload.turn.error
|
||||
{
|
||||
println!("[turn error] {}", error.message);
|
||||
}
|
||||
break;
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# codex-app-server
|
||||
|
||||
`codex app-server` is the interface Codex uses to power rich interfaces such as the [Codex VS Code extension](https://marketplace.visualstudio.com/items?itemName=openai.chatgpt). The message schema is currently unstable, but those who wish to build experimental UIs on top of Codex may find it valuable.
|
||||
`codex app-server` is the interface Codex uses to power rich interfaces such as the [Codex VS Code extension](https://marketplace.visualstudio.com/items?itemName=openai.chatgpt).
|
||||
|
||||
## Table of Contents
|
||||
- [Protocol](#protocol)
|
||||
@@ -66,6 +66,7 @@ The JSON-RPC API exposes dedicated methods for managing Codex conversations. Thr
|
||||
- `turn/start` — add user input to a thread and begin Codex generation; responds with the initial `turn` object and streams `turn/started`, `item/*`, and `turn/completed` notifications.
|
||||
- `turn/interrupt` — request cancellation of an in-flight turn by `(thread_id, turn_id)`; success is an empty `{}` response and the turn finishes with `status: "interrupted"`.
|
||||
- `review/start` — kick off Codex’s automated reviewer for a thread; responds like `turn/start` and emits `item/started`/`item/completed` notifications with `enteredReviewMode` and `exitedReviewMode` items, plus a final assistant `agentMessage` containing the review.
|
||||
- `command/exec` — run a single command under the server sandbox without starting a thread/turn (handy for utilities and validation).
|
||||
|
||||
### 1) Start or resume a thread
|
||||
|
||||
@@ -241,6 +242,25 @@ containing an `exitedReviewMode` item with the final review text:
|
||||
|
||||
The `review` string is plain text that already bundles the overall explanation plus a bullet list for each structured finding (matching `ThreadItem::ExitedReviewMode` in the generated schema). Use this notification to render the reviewer output in your client.
|
||||
|
||||
### 7) One-off command execution
|
||||
|
||||
Run a standalone command (argv vector) in the server’s sandbox without creating a thread or turn:
|
||||
|
||||
```json
|
||||
{ "method": "command/exec", "id": 32, "params": {
|
||||
"command": ["ls", "-la"],
|
||||
"cwd": "/Users/me/project", // optional; defaults to server cwd
|
||||
"sandboxPolicy": { "type": "workspaceWrite" }, // optional; defaults to user config
|
||||
"timeoutMs": 10000 // optional; ms timeout; defaults to server timeout
|
||||
} }
|
||||
{ "id": 32, "result": { "exitCode": 0, "stdout": "...", "stderr": "" } }
|
||||
```
|
||||
|
||||
Notes:
|
||||
- Empty `command` arrays are rejected.
|
||||
- `sandboxPolicy` accepts the same shape used by `turn/start` (e.g., `dangerFullAccess`, `readOnly`, `workspaceWrite` with flags).
|
||||
- When omitted, `timeoutMs` falls back to the server default.
|
||||
|
||||
## Events (work-in-progress)
|
||||
|
||||
Event notifications are the server-initiated event stream for thread lifecycles, turn lifecycles, and the items within them. After you start or resume a thread, keep reading stdout for `thread/started`, `turn/*`, and `item/*` notifications.
|
||||
|
||||
@@ -18,6 +18,7 @@ use codex_app_server_protocol::ContextCompactedNotification;
|
||||
use codex_app_server_protocol::ErrorNotification;
|
||||
use codex_app_server_protocol::ExecCommandApprovalParams;
|
||||
use codex_app_server_protocol::ExecCommandApprovalResponse;
|
||||
use codex_app_server_protocol::FileChangeOutputDeltaNotification;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalParams;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalResponse;
|
||||
use codex_app_server_protocol::FileUpdateChange;
|
||||
@@ -61,6 +62,7 @@ use codex_core::protocol::ReviewDecision;
|
||||
use codex_core::protocol::TokenCountEvent;
|
||||
use codex_core::protocol::TurnDiffEvent;
|
||||
use codex_core::review_format::format_review_findings_block;
|
||||
use codex_core::review_prompts;
|
||||
use codex_protocol::ConversationId;
|
||||
use codex_protocol::plan_tool::UpdatePlanArgs;
|
||||
use codex_protocol::protocol::ReviewOutputEvent;
|
||||
@@ -350,8 +352,32 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
}))
|
||||
.await;
|
||||
}
|
||||
EventMsg::ViewImageToolCall(view_image_event) => {
|
||||
let item = ThreadItem::ImageView {
|
||||
id: view_image_event.call_id.clone(),
|
||||
path: view_image_event.path.to_string_lossy().into_owned(),
|
||||
};
|
||||
let started = ItemStartedNotification {
|
||||
thread_id: conversation_id.to_string(),
|
||||
turn_id: event_turn_id.clone(),
|
||||
item: item.clone(),
|
||||
};
|
||||
outgoing
|
||||
.send_server_notification(ServerNotification::ItemStarted(started))
|
||||
.await;
|
||||
let completed = ItemCompletedNotification {
|
||||
thread_id: conversation_id.to_string(),
|
||||
turn_id: event_turn_id.clone(),
|
||||
item,
|
||||
};
|
||||
outgoing
|
||||
.send_server_notification(ServerNotification::ItemCompleted(completed))
|
||||
.await;
|
||||
}
|
||||
EventMsg::EnteredReviewMode(review_request) => {
|
||||
let review = review_request.user_facing_hint;
|
||||
let review = review_request
|
||||
.user_facing_hint
|
||||
.unwrap_or_else(|| review_prompts::user_facing_hint(&review_request.target));
|
||||
let item = ThreadItem::EnteredReviewMode {
|
||||
id: event_turn_id.clone(),
|
||||
review,
|
||||
@@ -501,17 +527,44 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
.await;
|
||||
}
|
||||
EventMsg::ExecCommandOutputDelta(exec_command_output_delta_event) => {
|
||||
let notification = CommandExecutionOutputDeltaNotification {
|
||||
thread_id: conversation_id.to_string(),
|
||||
turn_id: event_turn_id.clone(),
|
||||
item_id: exec_command_output_delta_event.call_id.clone(),
|
||||
delta: String::from_utf8_lossy(&exec_command_output_delta_event.chunk).to_string(),
|
||||
let item_id = exec_command_output_delta_event.call_id.clone();
|
||||
let delta = String::from_utf8_lossy(&exec_command_output_delta_event.chunk).to_string();
|
||||
// The underlying EventMsg::ExecCommandOutputDelta is used for shell, unified_exec,
|
||||
// and apply_patch tool calls. We represent apply_patch with the FileChange item, and
|
||||
// everything else with the CommandExecution item.
|
||||
//
|
||||
// We need to detect which item type it is so we can emit the right notification.
|
||||
// We already have state tracking FileChange items on item/started, so let's use that.
|
||||
let is_file_change = {
|
||||
let map = turn_summary_store.lock().await;
|
||||
map.get(&conversation_id)
|
||||
.is_some_and(|summary| summary.file_change_started.contains(&item_id))
|
||||
};
|
||||
outgoing
|
||||
.send_server_notification(ServerNotification::CommandExecutionOutputDelta(
|
||||
notification,
|
||||
))
|
||||
.await;
|
||||
if is_file_change {
|
||||
let notification = FileChangeOutputDeltaNotification {
|
||||
thread_id: conversation_id.to_string(),
|
||||
turn_id: event_turn_id.clone(),
|
||||
item_id,
|
||||
delta,
|
||||
};
|
||||
outgoing
|
||||
.send_server_notification(ServerNotification::FileChangeOutputDelta(
|
||||
notification,
|
||||
))
|
||||
.await;
|
||||
} else {
|
||||
let notification = CommandExecutionOutputDeltaNotification {
|
||||
thread_id: conversation_id.to_string(),
|
||||
turn_id: event_turn_id.clone(),
|
||||
item_id,
|
||||
delta,
|
||||
};
|
||||
outgoing
|
||||
.send_server_notification(ServerNotification::CommandExecutionOutputDelta(
|
||||
notification,
|
||||
))
|
||||
.await;
|
||||
}
|
||||
}
|
||||
EventMsg::ExecCommandEnd(exec_command_end_event) => {
|
||||
let ExecCommandEndEvent {
|
||||
@@ -665,6 +718,7 @@ async fn emit_turn_completed_with_status(
|
||||
conversation_id: ConversationId,
|
||||
event_turn_id: String,
|
||||
status: TurnStatus,
|
||||
error: Option<TurnError>,
|
||||
outgoing: &OutgoingMessageSender,
|
||||
) {
|
||||
let notification = TurnCompletedNotification {
|
||||
@@ -672,6 +726,7 @@ async fn emit_turn_completed_with_status(
|
||||
turn: Turn {
|
||||
id: event_turn_id,
|
||||
items: vec![],
|
||||
error,
|
||||
status,
|
||||
},
|
||||
};
|
||||
@@ -760,13 +815,12 @@ async fn handle_turn_complete(
|
||||
) {
|
||||
let turn_summary = find_and_remove_turn_summary(conversation_id, turn_summary_store).await;
|
||||
|
||||
let status = if let Some(error) = turn_summary.last_error {
|
||||
TurnStatus::Failed { error }
|
||||
} else {
|
||||
TurnStatus::Completed
|
||||
let (status, error) = match turn_summary.last_error {
|
||||
Some(error) => (TurnStatus::Failed, Some(error)),
|
||||
None => (TurnStatus::Completed, None),
|
||||
};
|
||||
|
||||
emit_turn_completed_with_status(conversation_id, event_turn_id, status, outgoing).await;
|
||||
emit_turn_completed_with_status(conversation_id, event_turn_id, status, error, outgoing).await;
|
||||
}
|
||||
|
||||
async fn handle_turn_interrupted(
|
||||
@@ -781,6 +835,7 @@ async fn handle_turn_interrupted(
|
||||
conversation_id,
|
||||
event_turn_id,
|
||||
TurnStatus::Interrupted,
|
||||
None,
|
||||
outgoing,
|
||||
)
|
||||
.await;
|
||||
@@ -1253,6 +1308,7 @@ mod tests {
|
||||
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
|
||||
assert_eq!(n.turn.id, event_turn_id);
|
||||
assert_eq!(n.turn.status, TurnStatus::Completed);
|
||||
assert_eq!(n.turn.error, None);
|
||||
}
|
||||
other => bail!("unexpected message: {other:?}"),
|
||||
}
|
||||
@@ -1293,6 +1349,7 @@ mod tests {
|
||||
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
|
||||
assert_eq!(n.turn.id, event_turn_id);
|
||||
assert_eq!(n.turn.status, TurnStatus::Interrupted);
|
||||
assert_eq!(n.turn.error, None);
|
||||
}
|
||||
other => bail!("unexpected message: {other:?}"),
|
||||
}
|
||||
@@ -1332,14 +1389,13 @@ mod tests {
|
||||
match msg {
|
||||
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
|
||||
assert_eq!(n.turn.id, event_turn_id);
|
||||
assert_eq!(n.turn.status, TurnStatus::Failed);
|
||||
assert_eq!(
|
||||
n.turn.status,
|
||||
TurnStatus::Failed {
|
||||
error: TurnError {
|
||||
message: "bad".to_string(),
|
||||
codex_error_info: Some(V2CodexErrorInfo::Other),
|
||||
}
|
||||
}
|
||||
n.turn.error,
|
||||
Some(TurnError {
|
||||
message: "bad".to_string(),
|
||||
codex_error_info: Some(V2CodexErrorInfo::Other),
|
||||
})
|
||||
);
|
||||
}
|
||||
other => bail!("unexpected message: {other:?}"),
|
||||
@@ -1600,14 +1656,13 @@ mod tests {
|
||||
match msg {
|
||||
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
|
||||
assert_eq!(n.turn.id, a_turn1);
|
||||
assert_eq!(n.turn.status, TurnStatus::Failed);
|
||||
assert_eq!(
|
||||
n.turn.status,
|
||||
TurnStatus::Failed {
|
||||
error: TurnError {
|
||||
message: "a1".to_string(),
|
||||
codex_error_info: Some(V2CodexErrorInfo::BadRequest),
|
||||
}
|
||||
}
|
||||
n.turn.error,
|
||||
Some(TurnError {
|
||||
message: "a1".to_string(),
|
||||
codex_error_info: Some(V2CodexErrorInfo::BadRequest),
|
||||
})
|
||||
);
|
||||
}
|
||||
other => bail!("unexpected message: {other:?}"),
|
||||
@@ -1621,14 +1676,13 @@ mod tests {
|
||||
match msg {
|
||||
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
|
||||
assert_eq!(n.turn.id, b_turn1);
|
||||
assert_eq!(n.turn.status, TurnStatus::Failed);
|
||||
assert_eq!(
|
||||
n.turn.status,
|
||||
TurnStatus::Failed {
|
||||
error: TurnError {
|
||||
message: "b1".to_string(),
|
||||
codex_error_info: None,
|
||||
}
|
||||
}
|
||||
n.turn.error,
|
||||
Some(TurnError {
|
||||
message: "b1".to_string(),
|
||||
codex_error_info: None,
|
||||
})
|
||||
);
|
||||
}
|
||||
other => bail!("unexpected message: {other:?}"),
|
||||
@@ -1643,6 +1697,7 @@ mod tests {
|
||||
OutgoingMessage::AppServerNotification(ServerNotification::TurnCompleted(n)) => {
|
||||
assert_eq!(n.turn.id, a_turn2);
|
||||
assert_eq!(n.turn.status, TurnStatus::Completed);
|
||||
assert_eq!(n.turn.error, None);
|
||||
}
|
||||
other => bail!("unexpected message: {other:?}"),
|
||||
}
|
||||
|
||||
@@ -21,9 +21,9 @@ use codex_app_server_protocol::CancelLoginAccountParams;
|
||||
use codex_app_server_protocol::CancelLoginAccountResponse;
|
||||
use codex_app_server_protocol::CancelLoginChatGptResponse;
|
||||
use codex_app_server_protocol::ClientRequest;
|
||||
use codex_app_server_protocol::CommandExecParams;
|
||||
use codex_app_server_protocol::ConversationGitInfo;
|
||||
use codex_app_server_protocol::ConversationSummary;
|
||||
use codex_app_server_protocol::ExecOneOffCommandParams;
|
||||
use codex_app_server_protocol::ExecOneOffCommandResponse;
|
||||
use codex_app_server_protocol::FeedbackUploadParams;
|
||||
use codex_app_server_protocol::FeedbackUploadResponse;
|
||||
@@ -64,7 +64,7 @@ use codex_app_server_protocol::ResumeConversationResponse;
|
||||
use codex_app_server_protocol::ReviewDelivery as ApiReviewDelivery;
|
||||
use codex_app_server_protocol::ReviewStartParams;
|
||||
use codex_app_server_protocol::ReviewStartResponse;
|
||||
use codex_app_server_protocol::ReviewTarget;
|
||||
use codex_app_server_protocol::ReviewTarget as ApiReviewTarget;
|
||||
use codex_app_server_protocol::SandboxMode;
|
||||
use codex_app_server_protocol::SendUserMessageParams;
|
||||
use codex_app_server_protocol::SendUserMessageResponse;
|
||||
@@ -124,6 +124,7 @@ use codex_core::protocol::EventMsg;
|
||||
use codex_core::protocol::Op;
|
||||
use codex_core::protocol::ReviewDelivery as CoreReviewDelivery;
|
||||
use codex_core::protocol::ReviewRequest;
|
||||
use codex_core::protocol::ReviewTarget as CoreReviewTarget;
|
||||
use codex_core::protocol::SessionConfiguredEvent;
|
||||
use codex_core::read_head_for_summary;
|
||||
use codex_feedback::CodexFeedback;
|
||||
@@ -255,7 +256,7 @@ impl CodexMessageProcessor {
|
||||
}
|
||||
|
||||
fn review_request_from_target(
|
||||
target: ReviewTarget,
|
||||
target: ApiReviewTarget,
|
||||
) -> Result<(ReviewRequest, String), JSONRPCErrorError> {
|
||||
fn invalid_request(message: String) -> JSONRPCErrorError {
|
||||
JSONRPCErrorError {
|
||||
@@ -265,73 +266,52 @@ impl CodexMessageProcessor {
|
||||
}
|
||||
}
|
||||
|
||||
match target {
|
||||
// TODO(jif) those messages will be extracted in a follow-up PR.
|
||||
ReviewTarget::UncommittedChanges => Ok((
|
||||
ReviewRequest {
|
||||
prompt: "Review the current code changes (staged, unstaged, and untracked files) and provide prioritized findings.".to_string(),
|
||||
user_facing_hint: "current changes".to_string(),
|
||||
},
|
||||
"Review uncommitted changes".to_string(),
|
||||
)),
|
||||
ReviewTarget::BaseBranch { branch } => {
|
||||
let cleaned_target = match target {
|
||||
ApiReviewTarget::UncommittedChanges => ApiReviewTarget::UncommittedChanges,
|
||||
ApiReviewTarget::BaseBranch { branch } => {
|
||||
let branch = branch.trim().to_string();
|
||||
if branch.is_empty() {
|
||||
return Err(invalid_request("branch must not be empty".to_string()));
|
||||
}
|
||||
let prompt = format!("Review the code changes against the base branch '{branch}'. Start by finding the merge diff between the current branch and {branch}'s upstream e.g. (`git merge-base HEAD \"$(git rev-parse --abbrev-ref \"{branch}@{{upstream}}\")\"`), then run `git diff` against that SHA to see what changes we would merge into the {branch} branch. Provide prioritized, actionable findings.");
|
||||
let hint = format!("changes against '{branch}'");
|
||||
let display = format!("Review changes against base branch '{branch}'");
|
||||
Ok((
|
||||
ReviewRequest {
|
||||
prompt,
|
||||
user_facing_hint: hint,
|
||||
},
|
||||
display,
|
||||
))
|
||||
ApiReviewTarget::BaseBranch { branch }
|
||||
}
|
||||
ReviewTarget::Commit { sha, title } => {
|
||||
ApiReviewTarget::Commit { sha, title } => {
|
||||
let sha = sha.trim().to_string();
|
||||
if sha.is_empty() {
|
||||
return Err(invalid_request("sha must not be empty".to_string()));
|
||||
}
|
||||
let brief_title = title
|
||||
let title = title
|
||||
.map(|t| t.trim().to_string())
|
||||
.filter(|t| !t.is_empty());
|
||||
let prompt = if let Some(title) = brief_title.clone() {
|
||||
format!("Review the code changes introduced by commit {sha} (\"{title}\"). Provide prioritized, actionable findings.")
|
||||
} else {
|
||||
format!("Review the code changes introduced by commit {sha}. Provide prioritized, actionable findings.")
|
||||
};
|
||||
let short_sha = sha.chars().take(7).collect::<String>();
|
||||
let hint = format!("commit {short_sha}");
|
||||
let display = if let Some(title) = brief_title {
|
||||
format!("Review commit {short_sha}: {title}")
|
||||
} else {
|
||||
format!("Review commit {short_sha}")
|
||||
};
|
||||
Ok((
|
||||
ReviewRequest {
|
||||
prompt,
|
||||
user_facing_hint: hint,
|
||||
},
|
||||
display,
|
||||
))
|
||||
ApiReviewTarget::Commit { sha, title }
|
||||
}
|
||||
ReviewTarget::Custom { instructions } => {
|
||||
ApiReviewTarget::Custom { instructions } => {
|
||||
let trimmed = instructions.trim().to_string();
|
||||
if trimmed.is_empty() {
|
||||
return Err(invalid_request("instructions must not be empty".to_string()));
|
||||
return Err(invalid_request(
|
||||
"instructions must not be empty".to_string(),
|
||||
));
|
||||
}
|
||||
ApiReviewTarget::Custom {
|
||||
instructions: trimmed,
|
||||
}
|
||||
Ok((
|
||||
ReviewRequest {
|
||||
prompt: trimmed.clone(),
|
||||
user_facing_hint: trimmed.clone(),
|
||||
},
|
||||
trimmed,
|
||||
))
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
let core_target = match cleaned_target {
|
||||
ApiReviewTarget::UncommittedChanges => CoreReviewTarget::UncommittedChanges,
|
||||
ApiReviewTarget::BaseBranch { branch } => CoreReviewTarget::BaseBranch { branch },
|
||||
ApiReviewTarget::Commit { sha, title } => CoreReviewTarget::Commit { sha, title },
|
||||
ApiReviewTarget::Custom { instructions } => CoreReviewTarget::Custom { instructions },
|
||||
};
|
||||
|
||||
let hint = codex_core::review_prompts::user_facing_hint(&core_target);
|
||||
let review_request = ReviewRequest {
|
||||
target: core_target,
|
||||
user_facing_hint: Some(hint.clone()),
|
||||
};
|
||||
|
||||
Ok((review_request, hint))
|
||||
}
|
||||
|
||||
pub async fn process_request(&mut self, request: ClientRequest) {
|
||||
@@ -467,9 +447,12 @@ impl CodexMessageProcessor {
|
||||
ClientRequest::FuzzyFileSearch { request_id, params } => {
|
||||
self.fuzzy_file_search(request_id, params).await;
|
||||
}
|
||||
ClientRequest::ExecOneOffCommand { request_id, params } => {
|
||||
ClientRequest::OneOffCommandExec { request_id, params } => {
|
||||
self.exec_one_off_command(request_id, params).await;
|
||||
}
|
||||
ClientRequest::ExecOneOffCommand { request_id, params } => {
|
||||
self.exec_one_off_command(request_id, params.into()).await;
|
||||
}
|
||||
ClientRequest::ConfigRead { .. }
|
||||
| ClientRequest::ConfigValueWrite { .. }
|
||||
| ClientRequest::ConfigBatchWrite { .. } => {
|
||||
@@ -1154,7 +1137,7 @@ impl CodexMessageProcessor {
|
||||
}
|
||||
}
|
||||
|
||||
async fn exec_one_off_command(&self, request_id: RequestId, params: ExecOneOffCommandParams) {
|
||||
async fn exec_one_off_command(&self, request_id: RequestId, params: CommandExecParams) {
|
||||
tracing::debug!("ExecOneOffCommand params: {params:?}");
|
||||
|
||||
if params.command.is_empty() {
|
||||
@@ -1169,7 +1152,9 @@ impl CodexMessageProcessor {
|
||||
|
||||
let cwd = params.cwd.unwrap_or_else(|| self.config.cwd.clone());
|
||||
let env = create_env(&self.config.shell_environment_policy);
|
||||
let timeout_ms = params.timeout_ms;
|
||||
let timeout_ms = params
|
||||
.timeout_ms
|
||||
.and_then(|timeout_ms| u64::try_from(timeout_ms).ok());
|
||||
let exec_params = ExecParams {
|
||||
command: params.command,
|
||||
cwd,
|
||||
@@ -1182,6 +1167,7 @@ impl CodexMessageProcessor {
|
||||
|
||||
let effective_policy = params
|
||||
.sandbox_policy
|
||||
.map(|policy| policy.to_core())
|
||||
.unwrap_or_else(|| self.config.sandbox_policy.clone());
|
||||
|
||||
let codex_linux_sandbox_exe = self.config.codex_linux_sandbox_exe.clone();
|
||||
@@ -2469,6 +2455,7 @@ impl CodexMessageProcessor {
|
||||
let turn = Turn {
|
||||
id: turn_id.clone(),
|
||||
items: vec![],
|
||||
error: None,
|
||||
status: TurnStatus::InProgress,
|
||||
};
|
||||
|
||||
@@ -2510,6 +2497,7 @@ impl CodexMessageProcessor {
|
||||
Turn {
|
||||
id: turn_id,
|
||||
items,
|
||||
error: None,
|
||||
status: TurnStatus::InProgress,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -93,7 +93,7 @@ async fn review_start_runs_review_turn_and_emits_code_review_item() -> Result<()
|
||||
match started.item {
|
||||
ThreadItem::EnteredReviewMode { id, review } => {
|
||||
assert_eq!(id, turn_id);
|
||||
assert_eq!(review, "commit 1234567");
|
||||
assert_eq!(review, "commit 1234567: Tidy UI colors");
|
||||
saw_entered_review_mode = true;
|
||||
break;
|
||||
}
|
||||
|
||||
@@ -11,6 +11,7 @@ use app_test_support::to_response;
|
||||
use codex_app_server_protocol::ApprovalDecision;
|
||||
use codex_app_server_protocol::CommandExecutionRequestApprovalResponse;
|
||||
use codex_app_server_protocol::CommandExecutionStatus;
|
||||
use codex_app_server_protocol::FileChangeOutputDeltaNotification;
|
||||
use codex_app_server_protocol::FileChangeRequestApprovalResponse;
|
||||
use codex_app_server_protocol::ItemCompletedNotification;
|
||||
use codex_app_server_protocol::ItemStartedNotification;
|
||||
@@ -725,6 +726,26 @@ async fn turn_start_file_change_approval_v2() -> Result<()> {
|
||||
)
|
||||
.await?;
|
||||
|
||||
let output_delta_notif = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("item/fileChange/outputDelta"),
|
||||
)
|
||||
.await??;
|
||||
let output_delta: FileChangeOutputDeltaNotification = serde_json::from_value(
|
||||
output_delta_notif
|
||||
.params
|
||||
.clone()
|
||||
.expect("item/fileChange/outputDelta params"),
|
||||
)?;
|
||||
assert_eq!(output_delta.thread_id, thread.id);
|
||||
assert_eq!(output_delta.turn_id, turn.id);
|
||||
assert_eq!(output_delta.item_id, "patch-call");
|
||||
assert!(
|
||||
!output_delta.delta.is_empty(),
|
||||
"expected delta to be non-empty, got: {}",
|
||||
output_delta.delta
|
||||
);
|
||||
|
||||
let completed_file_change = timeout(DEFAULT_READ_TIMEOUT, async {
|
||||
loop {
|
||||
let completed_notif = mcp
|
||||
|
||||
@@ -18,6 +18,8 @@ use codex_cli::login::run_logout;
|
||||
use codex_cloud_tasks::Cli as CloudTasksCli;
|
||||
use codex_common::CliConfigOverrides;
|
||||
use codex_exec::Cli as ExecCli;
|
||||
use codex_exec::Command as ExecCommand;
|
||||
use codex_exec::ReviewArgs;
|
||||
use codex_execpolicy::ExecPolicyCheckCommand;
|
||||
use codex_responses_api_proxy::Args as ResponsesApiProxyArgs;
|
||||
use codex_tui::AppExitInfo;
|
||||
@@ -72,6 +74,9 @@ enum Subcommand {
|
||||
#[clap(visible_alias = "e")]
|
||||
Exec(ExecCli),
|
||||
|
||||
/// Run a code review non-interactively.
|
||||
Review(ReviewArgs),
|
||||
|
||||
/// Manage login.
|
||||
Login(LoginCommand),
|
||||
|
||||
@@ -449,6 +454,15 @@ async fn cli_main(codex_linux_sandbox_exe: Option<PathBuf>) -> anyhow::Result<()
|
||||
);
|
||||
codex_exec::run_main(exec_cli, codex_linux_sandbox_exe).await?;
|
||||
}
|
||||
Some(Subcommand::Review(review_args)) => {
|
||||
let mut exec_cli = ExecCli::try_parse_from(["codex", "exec"])?;
|
||||
exec_cli.command = Some(ExecCommand::Review(review_args));
|
||||
prepend_config_flags(
|
||||
&mut exec_cli.config_overrides,
|
||||
root_config_overrides.clone(),
|
||||
);
|
||||
codex_exec::run_main(exec_cli, codex_linux_sandbox_exe).await?;
|
||||
}
|
||||
Some(Subcommand::McpServer) => {
|
||||
codex_mcp_server::run_main(codex_linux_sandbox_exe, root_config_overrides).await?;
|
||||
}
|
||||
|
||||
@@ -1,21 +1,22 @@
|
||||
[package]
|
||||
name = "codex-client"
|
||||
version.workspace = true
|
||||
edition.workspace = true
|
||||
license.workspace = true
|
||||
name = "codex-client"
|
||||
version.workspace = true
|
||||
|
||||
[dependencies]
|
||||
async-trait = { workspace = true }
|
||||
bytes = { workspace = true }
|
||||
eventsource-stream = { workspace = true }
|
||||
futures = { workspace = true }
|
||||
http = { workspace = true }
|
||||
rand = { workspace = true }
|
||||
reqwest = { workspace = true, features = ["json", "stream"] }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
thiserror = { workspace = true }
|
||||
tokio = { workspace = true, features = ["macros", "rt", "time", "sync"] }
|
||||
rand = { workspace = true }
|
||||
eventsource-stream = { workspace = true }
|
||||
tracing = { workspace = true }
|
||||
|
||||
[lints]
|
||||
workspace = true
|
||||
|
||||
@@ -8,6 +8,9 @@ use futures::stream::BoxStream;
|
||||
use http::HeaderMap;
|
||||
use http::Method;
|
||||
use http::StatusCode;
|
||||
use tracing::Level;
|
||||
use tracing::enabled;
|
||||
use tracing::trace;
|
||||
|
||||
pub type ByteStream = BoxStream<'static, Result<Bytes, TransportError>>;
|
||||
|
||||
@@ -83,6 +86,15 @@ impl HttpTransport for ReqwestTransport {
|
||||
}
|
||||
|
||||
async fn stream(&self, req: Request) -> Result<StreamResponse, TransportError> {
|
||||
if enabled!(Level::TRACE) {
|
||||
trace!(
|
||||
"{} to {}: {}",
|
||||
req.method,
|
||||
req.url,
|
||||
req.body.as_ref().unwrap_or_default()
|
||||
);
|
||||
}
|
||||
|
||||
let builder = self.build(req)?;
|
||||
let resp = builder.send().await.map_err(Self::map_error)?;
|
||||
let status = resp.status();
|
||||
|
||||
@@ -52,6 +52,7 @@ regex-lite = { workspace = true }
|
||||
reqwest = { workspace = true, features = ["json", "stream"] }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
serde_json = { workspace = true }
|
||||
serde_yaml = { workspace = true }
|
||||
sha1 = { workspace = true }
|
||||
sha2 = { workspace = true }
|
||||
shlex = { workspace = true }
|
||||
|
||||
@@ -976,6 +976,7 @@ impl Session {
|
||||
.read()
|
||||
.await
|
||||
.resolve_elicitation(server_name, id, response)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Records input items: always append to conversation history and
|
||||
@@ -1035,6 +1036,22 @@ impl Session {
|
||||
state.record_items(items.iter(), turn_context.truncation_policy);
|
||||
}
|
||||
|
||||
pub(crate) async fn record_model_warning(&self, message: impl Into<String>, ctx: &TurnContext) {
|
||||
if !self.enabled(Feature::ModelWarnings).await {
|
||||
return;
|
||||
}
|
||||
|
||||
let item = ResponseItem::Message {
|
||||
id: None,
|
||||
role: "user".to_string(),
|
||||
content: vec![ContentItem::InputText {
|
||||
text: format!("Warning: {}", message.into()),
|
||||
}],
|
||||
};
|
||||
|
||||
self.record_conversation_items(ctx, &[item]).await;
|
||||
}
|
||||
|
||||
pub(crate) async fn replace_history(&self, items: Vec<ResponseItem>) {
|
||||
let mut state = self.state.lock().await;
|
||||
state.replace_history(items);
|
||||
@@ -1476,6 +1493,7 @@ mod handlers {
|
||||
use crate::codex::spawn_review_thread;
|
||||
use crate::config::Config;
|
||||
use crate::mcp::auth::compute_auth_statuses;
|
||||
use crate::review_prompts::resolve_review_request;
|
||||
use crate::tasks::CompactTask;
|
||||
use crate::tasks::RegularTask;
|
||||
use crate::tasks::UndoTask;
|
||||
@@ -1784,14 +1802,28 @@ mod handlers {
|
||||
let turn_context = sess
|
||||
.new_turn_with_sub_id(sub_id.clone(), SessionSettingsUpdate::default())
|
||||
.await;
|
||||
spawn_review_thread(
|
||||
Arc::clone(sess),
|
||||
Arc::clone(config),
|
||||
turn_context.clone(),
|
||||
sub_id,
|
||||
review_request,
|
||||
)
|
||||
.await;
|
||||
match resolve_review_request(review_request, config.cwd.as_path()) {
|
||||
Ok(resolved) => {
|
||||
spawn_review_thread(
|
||||
Arc::clone(sess),
|
||||
Arc::clone(config),
|
||||
turn_context.clone(),
|
||||
sub_id,
|
||||
resolved,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
Err(err) => {
|
||||
let event = Event {
|
||||
id: sub_id,
|
||||
msg: EventMsg::Error(ErrorEvent {
|
||||
message: err.to_string(),
|
||||
codex_error_info: Some(CodexErrorInfo::Other),
|
||||
}),
|
||||
};
|
||||
sess.send_event(&turn_context, event.msg).await;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1801,7 +1833,7 @@ async fn spawn_review_thread(
|
||||
config: Arc<Config>,
|
||||
parent_turn_context: Arc<TurnContext>,
|
||||
sub_id: String,
|
||||
review_request: ReviewRequest,
|
||||
resolved: crate::review_prompts::ResolvedReviewRequest,
|
||||
) {
|
||||
let model = config.review_model.clone();
|
||||
let review_model_family = find_family_for_model(&model)
|
||||
@@ -1817,7 +1849,7 @@ async fn spawn_review_thread(
|
||||
});
|
||||
|
||||
let base_instructions = REVIEW_PROMPT.to_string();
|
||||
let review_prompt = review_request.prompt.clone();
|
||||
let review_prompt = resolved.prompt.clone();
|
||||
let provider = parent_turn_context.client.get_provider();
|
||||
let auth_manager = parent_turn_context.client.get_auth_manager();
|
||||
let model_family = review_model_family.clone();
|
||||
@@ -1879,6 +1911,10 @@ async fn spawn_review_thread(
|
||||
sess.spawn_task(tc.clone(), input, ReviewTask::new()).await;
|
||||
|
||||
// Announce entering review mode so UIs can switch modes.
|
||||
let review_request = ReviewRequest {
|
||||
target: resolved.target,
|
||||
user_facing_hint: Some(resolved.user_facing_hint),
|
||||
};
|
||||
sess.send_event(&tc, EventMsg::EnteredReviewMode(review_request))
|
||||
.await;
|
||||
}
|
||||
@@ -2445,7 +2481,10 @@ mod tests {
|
||||
use crate::tools::format_exec_output_str;
|
||||
|
||||
use crate::protocol::CompactedItem;
|
||||
use crate::protocol::CreditsSnapshot;
|
||||
use crate::protocol::InitialHistory;
|
||||
use crate::protocol::RateLimitSnapshot;
|
||||
use crate::protocol::RateLimitWindow;
|
||||
use crate::protocol::ResumedHistory;
|
||||
use crate::state::TaskKind;
|
||||
use crate::tasks::SessionTask;
|
||||
@@ -2515,6 +2554,75 @@ mod tests {
|
||||
assert_eq!(expected, actual);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn set_rate_limits_retains_previous_credits() {
|
||||
let codex_home = tempfile::tempdir().expect("create temp dir");
|
||||
let config = Config::load_from_base_config_with_overrides(
|
||||
ConfigToml::default(),
|
||||
ConfigOverrides::default(),
|
||||
codex_home.path().to_path_buf(),
|
||||
)
|
||||
.expect("load default test config");
|
||||
let config = Arc::new(config);
|
||||
let session_configuration = SessionConfiguration {
|
||||
provider: config.model_provider.clone(),
|
||||
model: config.model.clone(),
|
||||
model_reasoning_effort: config.model_reasoning_effort,
|
||||
model_reasoning_summary: config.model_reasoning_summary,
|
||||
developer_instructions: config.developer_instructions.clone(),
|
||||
user_instructions: config.user_instructions.clone(),
|
||||
base_instructions: config.base_instructions.clone(),
|
||||
compact_prompt: config.compact_prompt.clone(),
|
||||
approval_policy: config.approval_policy,
|
||||
sandbox_policy: config.sandbox_policy.clone(),
|
||||
cwd: config.cwd.clone(),
|
||||
original_config_do_not_use: Arc::clone(&config),
|
||||
features: Features::default(),
|
||||
exec_policy: Arc::new(ExecPolicy::empty()),
|
||||
session_source: SessionSource::Exec,
|
||||
};
|
||||
|
||||
let mut state = SessionState::new(session_configuration);
|
||||
let initial = RateLimitSnapshot {
|
||||
primary: Some(RateLimitWindow {
|
||||
used_percent: 10.0,
|
||||
window_minutes: Some(15),
|
||||
resets_at: Some(1_700),
|
||||
}),
|
||||
secondary: None,
|
||||
credits: Some(CreditsSnapshot {
|
||||
has_credits: true,
|
||||
unlimited: false,
|
||||
balance: Some("10.00".to_string()),
|
||||
}),
|
||||
};
|
||||
state.set_rate_limits(initial.clone());
|
||||
|
||||
let update = RateLimitSnapshot {
|
||||
primary: Some(RateLimitWindow {
|
||||
used_percent: 40.0,
|
||||
window_minutes: Some(30),
|
||||
resets_at: Some(1_800),
|
||||
}),
|
||||
secondary: Some(RateLimitWindow {
|
||||
used_percent: 5.0,
|
||||
window_minutes: Some(60),
|
||||
resets_at: Some(1_900),
|
||||
}),
|
||||
credits: None,
|
||||
};
|
||||
state.set_rate_limits(update.clone());
|
||||
|
||||
assert_eq!(
|
||||
state.latest_rate_limits,
|
||||
Some(RateLimitSnapshot {
|
||||
primary: update.primary.clone(),
|
||||
secondary: update.secondary,
|
||||
credits: initial.credits,
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prefers_structured_content_when_present() {
|
||||
let ctr = CallToolResult {
|
||||
@@ -2787,6 +2895,40 @@ mod tests {
|
||||
(session, turn_context, rx_event)
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn record_model_warning_appends_user_message() {
|
||||
let (session, turn_context) = make_session_and_context();
|
||||
|
||||
session
|
||||
.state
|
||||
.lock()
|
||||
.await
|
||||
.session_configuration
|
||||
.features
|
||||
.enable(Feature::ModelWarnings);
|
||||
|
||||
session
|
||||
.record_model_warning("too many unified exec sessions", &turn_context)
|
||||
.await;
|
||||
|
||||
let mut history = session.clone_history().await;
|
||||
let history_items = history.get_history();
|
||||
let last = history_items.last().expect("warning recorded");
|
||||
|
||||
match last {
|
||||
ResponseItem::Message { role, content, .. } => {
|
||||
assert_eq!(role, "user");
|
||||
assert_eq!(
|
||||
content,
|
||||
&vec![ContentItem::InputText {
|
||||
text: "Warning: too many unified exec sessions".to_string(),
|
||||
}]
|
||||
);
|
||||
}
|
||||
other => panic!("expected user message, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
struct NeverEndingTask {
|
||||
kind: TaskKind,
|
||||
|
||||
@@ -34,6 +34,7 @@ use crate::project_doc::DEFAULT_PROJECT_DOC_FILENAME;
|
||||
use crate::project_doc::LOCAL_PROJECT_DOC_FILENAME;
|
||||
use crate::protocol::AskForApproval;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
use crate::util::resolve_path;
|
||||
use codex_app_server_protocol::Tools;
|
||||
use codex_app_server_protocol::UserSavedConfig;
|
||||
use codex_protocol::config_types::ForcedLoginMethod;
|
||||
@@ -1016,15 +1017,8 @@ impl Config {
|
||||
let additional_writable_roots: Vec<PathBuf> = additional_writable_roots
|
||||
.into_iter()
|
||||
.map(|path| {
|
||||
let absolute = if path.is_absolute() {
|
||||
path
|
||||
} else {
|
||||
resolved_cwd.join(path)
|
||||
};
|
||||
match canonicalize(&absolute) {
|
||||
Ok(canonical) => canonical,
|
||||
Err(_) => absolute,
|
||||
}
|
||||
let absolute = resolve_path(&resolved_cwd, &path);
|
||||
canonicalize(&absolute).unwrap_or(absolute)
|
||||
})
|
||||
.collect();
|
||||
let active_project = cfg
|
||||
@@ -1299,11 +1293,7 @@ impl Config {
|
||||
return Ok(None);
|
||||
};
|
||||
|
||||
let full_path = if p.is_relative() {
|
||||
cwd.join(p)
|
||||
} else {
|
||||
p.to_path_buf()
|
||||
};
|
||||
let full_path = resolve_path(cwd, p);
|
||||
|
||||
let contents = std::fs::read_to_string(&full_path).map_err(|e| {
|
||||
std::io::Error::new(
|
||||
|
||||
@@ -51,6 +51,10 @@ pub enum Feature {
|
||||
ShellTool,
|
||||
/// Allow model to call multiple tools in parallel (only for models supporting it).
|
||||
ParallelToolCalls,
|
||||
/// Experimental skills injection (CLI flag-driven).
|
||||
Skills,
|
||||
/// Send warnings to the model to correct it on the tool usage.
|
||||
ModelWarnings,
|
||||
}
|
||||
|
||||
impl Feature {
|
||||
@@ -265,6 +269,12 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ShellTool,
|
||||
key: "shell_tool",
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
},
|
||||
// Unstable features.
|
||||
FeatureSpec {
|
||||
id: Feature::UnifiedExec,
|
||||
@@ -321,9 +331,15 @@ pub const FEATURES: &[FeatureSpec] = &[
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::ShellTool,
|
||||
key: "shell_tool",
|
||||
stage: Stage::Stable,
|
||||
default_enabled: true,
|
||||
id: Feature::ModelWarnings,
|
||||
key: "warnings",
|
||||
stage: Stage::Experimental,
|
||||
default_enabled: false,
|
||||
},
|
||||
FeatureSpec {
|
||||
id: Feature::Skills,
|
||||
key: "skills",
|
||||
stage: Stage::Experimental,
|
||||
default_enabled: false,
|
||||
},
|
||||
];
|
||||
|
||||
@@ -2,6 +2,7 @@ use std::collections::HashSet;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use crate::util::resolve_path;
|
||||
use codex_app_server_protocol::GitSha;
|
||||
use codex_protocol::protocol::GitInfo;
|
||||
use futures::future::join_all;
|
||||
@@ -131,11 +132,15 @@ pub async fn recent_commits(cwd: &Path, limit: usize) -> Vec<CommitLogEntry> {
|
||||
}
|
||||
|
||||
let fmt = "%H%x1f%ct%x1f%s"; // <sha> <US> <commit_time> <US> <subject>
|
||||
let n = limit.max(1).to_string();
|
||||
let Some(log_out) =
|
||||
run_git_command_with_timeout(&["log", "-n", &n, &format!("--pretty=format:{fmt}")], cwd)
|
||||
.await
|
||||
else {
|
||||
let limit_arg = (limit > 0).then(|| limit.to_string());
|
||||
let mut args: Vec<String> = vec!["log".to_string()];
|
||||
if let Some(n) = &limit_arg {
|
||||
args.push("-n".to_string());
|
||||
args.push(n.clone());
|
||||
}
|
||||
args.push(format!("--pretty=format:{fmt}"));
|
||||
let arg_refs: Vec<&str> = args.iter().map(String::as_str).collect();
|
||||
let Some(log_out) = run_git_command_with_timeout(&arg_refs, cwd).await else {
|
||||
return Vec::new();
|
||||
};
|
||||
if !log_out.status.success() {
|
||||
@@ -544,11 +549,7 @@ pub fn resolve_root_git_project_for_trust(cwd: &Path) -> Option<PathBuf> {
|
||||
.trim()
|
||||
.to_string();
|
||||
|
||||
let git_dir_path_raw = if Path::new(&git_dir_s).is_absolute() {
|
||||
PathBuf::from(&git_dir_s)
|
||||
} else {
|
||||
base.join(&git_dir_s)
|
||||
};
|
||||
let git_dir_path_raw = resolve_path(base, &PathBuf::from(&git_dir_s));
|
||||
|
||||
// Normalize to handle macOS /var vs /private/var and resolve ".." segments.
|
||||
let git_dir_path = std::fs::canonicalize(&git_dir_path_raw).unwrap_or(git_dir_path_raw);
|
||||
|
||||
@@ -16,7 +16,7 @@ mod codex_conversation;
|
||||
mod compact_remote;
|
||||
pub use codex_conversation::CodexConversation;
|
||||
mod codex_delegate;
|
||||
pub mod command_safety;
|
||||
mod command_safety;
|
||||
pub mod config;
|
||||
pub mod config_loader;
|
||||
mod context_manager;
|
||||
@@ -58,6 +58,7 @@ pub use model_provider_info::create_oss_provider_with_base_url;
|
||||
mod conversation_manager;
|
||||
mod event_mapping;
|
||||
pub mod review_format;
|
||||
pub mod review_prompts;
|
||||
pub use codex_protocol::protocol::InitialHistory;
|
||||
pub use conversation_manager::ConversationManager;
|
||||
pub use conversation_manager::NewConversation;
|
||||
@@ -72,6 +73,7 @@ mod rollout;
|
||||
pub(crate) mod safety;
|
||||
pub mod seatbelt;
|
||||
pub mod shell;
|
||||
pub mod skills;
|
||||
pub mod spawn;
|
||||
pub mod terminal;
|
||||
mod tools;
|
||||
|
||||
@@ -12,7 +12,6 @@ use std::env;
|
||||
use std::ffi::OsString;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
use std::sync::Mutex;
|
||||
use std::time::Duration;
|
||||
|
||||
use crate::mcp::auth::McpAuthStatusEntry;
|
||||
@@ -55,6 +54,7 @@ use serde::Serialize;
|
||||
use serde_json::json;
|
||||
use sha1::Digest;
|
||||
use sha1::Sha1;
|
||||
use tokio::sync::Mutex;
|
||||
use tokio::sync::oneshot;
|
||||
use tokio::task::JoinSet;
|
||||
use tokio_util::sync::CancellationToken;
|
||||
@@ -128,7 +128,7 @@ struct ElicitationRequestManager {
|
||||
}
|
||||
|
||||
impl ElicitationRequestManager {
|
||||
fn resolve(
|
||||
async fn resolve(
|
||||
&self,
|
||||
server_name: String,
|
||||
id: RequestId,
|
||||
@@ -136,7 +136,7 @@ impl ElicitationRequestManager {
|
||||
) -> Result<()> {
|
||||
self.requests
|
||||
.lock()
|
||||
.map_err(|e| anyhow!("failed to lock elicitation requests: {e:?}"))?
|
||||
.await
|
||||
.remove(&(server_name, id))
|
||||
.ok_or_else(|| anyhow!("elicitation request not found"))?
|
||||
.send(response)
|
||||
@@ -151,7 +151,8 @@ impl ElicitationRequestManager {
|
||||
let server_name = server_name.clone();
|
||||
async move {
|
||||
let (tx, rx) = oneshot::channel();
|
||||
if let Ok(mut lock) = elicitation_requests.lock() {
|
||||
{
|
||||
let mut lock = elicitation_requests.lock().await;
|
||||
lock.insert((server_name.clone(), id.clone()), tx);
|
||||
}
|
||||
let _ = tx_event
|
||||
@@ -365,13 +366,15 @@ impl McpConnectionManager {
|
||||
.context("failed to get client")
|
||||
}
|
||||
|
||||
pub fn resolve_elicitation(
|
||||
pub async fn resolve_elicitation(
|
||||
&self,
|
||||
server_name: String,
|
||||
id: RequestId,
|
||||
response: ElicitationResponse,
|
||||
) -> Result<()> {
|
||||
self.elicitation_requests.resolve(server_name, id, response)
|
||||
self.elicitation_requests
|
||||
.resolve(server_name, id, response)
|
||||
.await
|
||||
}
|
||||
|
||||
/// Returns a single map that contains all tools. Each key is the
|
||||
|
||||
@@ -18,6 +18,7 @@ use std::fs::File;
|
||||
use std::fs::OpenOptions;
|
||||
use std::io::Result;
|
||||
use std::io::Write;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use serde::Deserialize;
|
||||
@@ -42,7 +43,7 @@ const HISTORY_FILENAME: &str = "history.jsonl";
|
||||
const MAX_RETRIES: usize = 10;
|
||||
const RETRY_SLEEP: Duration = Duration::from_millis(100);
|
||||
|
||||
#[derive(Serialize, Deserialize, Debug, Clone)]
|
||||
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
|
||||
pub struct HistoryEntry {
|
||||
pub session_id: String,
|
||||
pub ts: u64,
|
||||
@@ -142,23 +143,54 @@ pub(crate) async fn append_entry(
|
||||
/// the current number of entries by counting newline characters.
|
||||
pub(crate) async fn history_metadata(config: &Config) -> (u64, usize) {
|
||||
let path = history_filepath(config);
|
||||
history_metadata_for_file(&path).await
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
let log_id = {
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
// Obtain metadata (async) to get the identifier.
|
||||
let meta = match fs::metadata(&path).await {
|
||||
Ok(m) => m,
|
||||
Err(e) if e.kind() == std::io::ErrorKind::NotFound => return (0, 0),
|
||||
Err(_) => return (0, 0),
|
||||
};
|
||||
meta.ino()
|
||||
/// Given a `log_id` (on Unix this is the file's inode number,
|
||||
/// on Windows this is the file's creation time) and a zero-based
|
||||
/// `offset`, return the corresponding `HistoryEntry` if the identifier matches
|
||||
/// the current history file **and** the requested offset exists. Any I/O or
|
||||
/// parsing errors are logged and result in `None`.
|
||||
///
|
||||
/// Note this function is not async because it uses a sync advisory file
|
||||
/// locking API.
|
||||
#[cfg(any(unix, windows))]
|
||||
pub(crate) fn lookup(log_id: u64, offset: usize, config: &Config) -> Option<HistoryEntry> {
|
||||
let path = history_filepath(config);
|
||||
lookup_history_entry(&path, log_id, offset)
|
||||
}
|
||||
|
||||
/// On Unix systems, ensure the file permissions are `0o600` (rw-------). If the
|
||||
/// permissions cannot be changed the error is propagated to the caller.
|
||||
#[cfg(unix)]
|
||||
async fn ensure_owner_only_permissions(file: &File) -> Result<()> {
|
||||
let metadata = file.metadata()?;
|
||||
let current_mode = metadata.permissions().mode() & 0o777;
|
||||
if current_mode != 0o600 {
|
||||
let mut perms = metadata.permissions();
|
||||
perms.set_mode(0o600);
|
||||
let perms_clone = perms.clone();
|
||||
let file_clone = file.try_clone()?;
|
||||
tokio::task::spawn_blocking(move || file_clone.set_permissions(perms_clone)).await??;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
// On Windows, simply succeed.
|
||||
async fn ensure_owner_only_permissions(_file: &File) -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn history_metadata_for_file(path: &Path) -> (u64, usize) {
|
||||
let log_id = match fs::metadata(path).await {
|
||||
Ok(metadata) => history_log_id(&metadata).unwrap_or(0),
|
||||
Err(e) if e.kind() == std::io::ErrorKind::NotFound => return (0, 0),
|
||||
Err(_) => return (0, 0),
|
||||
};
|
||||
#[cfg(not(unix))]
|
||||
let log_id = 0u64;
|
||||
|
||||
// Open the file.
|
||||
let mut file = match fs::File::open(&path).await {
|
||||
let mut file = match fs::File::open(path).await {
|
||||
Ok(f) => f,
|
||||
Err(_) => return (log_id, 0),
|
||||
};
|
||||
@@ -179,21 +211,12 @@ pub(crate) async fn history_metadata(config: &Config) -> (u64, usize) {
|
||||
(log_id, count)
|
||||
}
|
||||
|
||||
/// Given a `log_id` (on Unix this is the file's inode number) and a zero-based
|
||||
/// `offset`, return the corresponding `HistoryEntry` if the identifier matches
|
||||
/// the current history file **and** the requested offset exists. Any I/O or
|
||||
/// parsing errors are logged and result in `None`.
|
||||
///
|
||||
/// Note this function is not async because it uses a sync advisory file
|
||||
/// locking API.
|
||||
#[cfg(unix)]
|
||||
pub(crate) fn lookup(log_id: u64, offset: usize, config: &Config) -> Option<HistoryEntry> {
|
||||
#[cfg(any(unix, windows))]
|
||||
fn lookup_history_entry(path: &Path, log_id: u64, offset: usize) -> Option<HistoryEntry> {
|
||||
use std::io::BufRead;
|
||||
use std::io::BufReader;
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
|
||||
let path = history_filepath(config);
|
||||
let file: File = match OpenOptions::new().read(true).open(&path) {
|
||||
let file: File = match OpenOptions::new().read(true).open(path) {
|
||||
Ok(f) => f,
|
||||
Err(e) => {
|
||||
tracing::warn!(error = %e, "failed to open history file");
|
||||
@@ -209,7 +232,9 @@ pub(crate) fn lookup(log_id: u64, offset: usize, config: &Config) -> Option<Hist
|
||||
}
|
||||
};
|
||||
|
||||
if metadata.ino() != log_id {
|
||||
let current_log_id = history_log_id(&metadata)?;
|
||||
|
||||
if log_id != 0 && current_log_id != log_id {
|
||||
return None;
|
||||
}
|
||||
|
||||
@@ -256,31 +281,104 @@ pub(crate) fn lookup(log_id: u64, offset: usize, config: &Config) -> Option<Hist
|
||||
None
|
||||
}
|
||||
|
||||
/// Fallback stub for non-Unix systems: currently always returns `None`.
|
||||
#[cfg(not(unix))]
|
||||
pub(crate) fn lookup(log_id: u64, offset: usize, config: &Config) -> Option<HistoryEntry> {
|
||||
let _ = (log_id, offset, config);
|
||||
None
|
||||
}
|
||||
|
||||
/// On Unix systems ensure the file permissions are `0o600` (rw-------). If the
|
||||
/// permissions cannot be changed the error is propagated to the caller.
|
||||
#[cfg(unix)]
|
||||
async fn ensure_owner_only_permissions(file: &File) -> Result<()> {
|
||||
let metadata = file.metadata()?;
|
||||
let current_mode = metadata.permissions().mode() & 0o777;
|
||||
if current_mode != 0o600 {
|
||||
let mut perms = metadata.permissions();
|
||||
perms.set_mode(0o600);
|
||||
let perms_clone = perms.clone();
|
||||
let file_clone = file.try_clone()?;
|
||||
tokio::task::spawn_blocking(move || file_clone.set_permissions(perms_clone)).await??;
|
||||
fn history_log_id(metadata: &std::fs::Metadata) -> Option<u64> {
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::MetadataExt;
|
||||
Some(metadata.ino())
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
{
|
||||
use std::os::windows::fs::MetadataExt;
|
||||
Some(metadata.creation_time())
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(not(unix))]
|
||||
async fn ensure_owner_only_permissions(_file: &File) -> Result<()> {
|
||||
// For now, on non-Unix, simply succeed.
|
||||
Ok(())
|
||||
#[cfg(all(test, any(unix, windows)))]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs::File;
|
||||
use std::io::Write;
|
||||
use tempfile::TempDir;
|
||||
|
||||
#[tokio::test]
|
||||
async fn lookup_reads_history_entries() {
|
||||
let temp_dir = TempDir::new().expect("create temp dir");
|
||||
let history_path = temp_dir.path().join(HISTORY_FILENAME);
|
||||
|
||||
let entries = vec![
|
||||
HistoryEntry {
|
||||
session_id: "first-session".to_string(),
|
||||
ts: 1,
|
||||
text: "first".to_string(),
|
||||
},
|
||||
HistoryEntry {
|
||||
session_id: "second-session".to_string(),
|
||||
ts: 2,
|
||||
text: "second".to_string(),
|
||||
},
|
||||
];
|
||||
|
||||
let mut file = File::create(&history_path).expect("create history file");
|
||||
for entry in &entries {
|
||||
writeln!(
|
||||
file,
|
||||
"{}",
|
||||
serde_json::to_string(entry).expect("serialize history entry")
|
||||
)
|
||||
.expect("write history entry");
|
||||
}
|
||||
|
||||
let (log_id, count) = history_metadata_for_file(&history_path).await;
|
||||
assert_eq!(count, entries.len());
|
||||
|
||||
let second_entry =
|
||||
lookup_history_entry(&history_path, log_id, 1).expect("fetch second history entry");
|
||||
assert_eq!(second_entry, entries[1]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn lookup_uses_stable_log_id_after_appends() {
|
||||
let temp_dir = TempDir::new().expect("create temp dir");
|
||||
let history_path = temp_dir.path().join(HISTORY_FILENAME);
|
||||
|
||||
let initial = HistoryEntry {
|
||||
session_id: "first-session".to_string(),
|
||||
ts: 1,
|
||||
text: "first".to_string(),
|
||||
};
|
||||
let appended = HistoryEntry {
|
||||
session_id: "second-session".to_string(),
|
||||
ts: 2,
|
||||
text: "second".to_string(),
|
||||
};
|
||||
|
||||
let mut file = File::create(&history_path).expect("create history file");
|
||||
writeln!(
|
||||
file,
|
||||
"{}",
|
||||
serde_json::to_string(&initial).expect("serialize initial entry")
|
||||
)
|
||||
.expect("write initial entry");
|
||||
|
||||
let (log_id, count) = history_metadata_for_file(&history_path).await;
|
||||
assert_eq!(count, 1);
|
||||
|
||||
let mut append = std::fs::OpenOptions::new()
|
||||
.append(true)
|
||||
.open(&history_path)
|
||||
.expect("open history file for append");
|
||||
writeln!(
|
||||
append,
|
||||
"{}",
|
||||
serde_json::to_string(&appended).expect("serialize appended entry")
|
||||
)
|
||||
.expect("append history entry");
|
||||
|
||||
let fetched =
|
||||
lookup_history_entry(&history_path, log_id, 1).expect("lookup appended history entry");
|
||||
assert_eq!(fetched, appended);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -76,6 +76,8 @@ pub(crate) fn get_model_info(model_family: &ModelFamily) -> Option<ModelInfo> {
|
||||
|
||||
_ if slug.starts_with("codex-") => Some(ModelInfo::new(CONTEXT_WINDOW_272K)),
|
||||
|
||||
_ if slug.starts_with("exp-") => Some(ModelInfo::new(CONTEXT_WINDOW_272K)),
|
||||
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,6 +14,9 @@
|
||||
//! 3. We do **not** walk past the Git root.
|
||||
|
||||
use crate::config::Config;
|
||||
use crate::features::Feature;
|
||||
use crate::skills::load_skills;
|
||||
use crate::skills::render_skills_section;
|
||||
use dunce::canonicalize as normalize_path;
|
||||
use std::path::PathBuf;
|
||||
use tokio::io::AsyncReadExt;
|
||||
@@ -31,18 +34,47 @@ const PROJECT_DOC_SEPARATOR: &str = "\n\n--- project-doc ---\n\n";
|
||||
/// Combines `Config::instructions` and `AGENTS.md` (if present) into a single
|
||||
/// string of instructions.
|
||||
pub(crate) async fn get_user_instructions(config: &Config) -> Option<String> {
|
||||
match read_project_docs(config).await {
|
||||
Ok(Some(project_doc)) => match &config.user_instructions {
|
||||
Some(original_instructions) => Some(format!(
|
||||
"{original_instructions}{PROJECT_DOC_SEPARATOR}{project_doc}"
|
||||
)),
|
||||
None => Some(project_doc),
|
||||
},
|
||||
Ok(None) => config.user_instructions.clone(),
|
||||
let skills_section = if config.features.enabled(Feature::Skills) {
|
||||
let skills_outcome = load_skills(config);
|
||||
for err in &skills_outcome.errors {
|
||||
error!(
|
||||
"failed to load skill {}: {}",
|
||||
err.path.display(),
|
||||
err.message
|
||||
);
|
||||
}
|
||||
render_skills_section(&skills_outcome.skills)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
let project_docs = match read_project_docs(config).await {
|
||||
Ok(docs) => docs,
|
||||
Err(e) => {
|
||||
error!("error trying to find project doc: {e:#}");
|
||||
config.user_instructions.clone()
|
||||
return config.user_instructions.clone();
|
||||
}
|
||||
};
|
||||
|
||||
let combined_project_docs = merge_project_docs_with_skills(project_docs, skills_section);
|
||||
|
||||
let mut parts: Vec<String> = Vec::new();
|
||||
|
||||
if let Some(instructions) = config.user_instructions.clone() {
|
||||
parts.push(instructions);
|
||||
}
|
||||
|
||||
if let Some(project_doc) = combined_project_docs {
|
||||
if !parts.is_empty() {
|
||||
parts.push(PROJECT_DOC_SEPARATOR.to_string());
|
||||
}
|
||||
parts.push(project_doc);
|
||||
}
|
||||
|
||||
if parts.is_empty() {
|
||||
None
|
||||
} else {
|
||||
Some(parts.concat())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -195,12 +227,25 @@ fn candidate_filenames<'a>(config: &'a Config) -> Vec<&'a str> {
|
||||
names
|
||||
}
|
||||
|
||||
fn merge_project_docs_with_skills(
|
||||
project_doc: Option<String>,
|
||||
skills_section: Option<String>,
|
||||
) -> Option<String> {
|
||||
match (project_doc, skills_section) {
|
||||
(Some(doc), Some(skills)) => Some(format!("{doc}\n\n{skills}")),
|
||||
(Some(doc), None) => Some(doc),
|
||||
(None, Some(skills)) => Some(skills),
|
||||
(None, None) => None,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::config::ConfigOverrides;
|
||||
use crate::config::ConfigToml;
|
||||
use std::fs;
|
||||
use std::path::PathBuf;
|
||||
use tempfile::TempDir;
|
||||
|
||||
/// Helper that returns a `Config` pointing at `root` and using `limit` as
|
||||
@@ -219,6 +264,7 @@ mod tests {
|
||||
|
||||
config.cwd = root.path().to_path_buf();
|
||||
config.project_doc_max_bytes = limit;
|
||||
config.features.enable(Feature::Skills);
|
||||
|
||||
config.user_instructions = instructions.map(ToOwned::to_owned);
|
||||
config
|
||||
@@ -447,4 +493,58 @@ mod tests {
|
||||
.eq(DEFAULT_PROJECT_DOC_FILENAME)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skills_are_appended_to_project_doc() {
|
||||
let tmp = tempfile::tempdir().expect("tempdir");
|
||||
fs::write(tmp.path().join("AGENTS.md"), "base doc").unwrap();
|
||||
|
||||
let cfg = make_config(&tmp, 4096, None);
|
||||
create_skill(
|
||||
cfg.codex_home.clone(),
|
||||
"pdf-processing",
|
||||
"extract from pdfs",
|
||||
);
|
||||
|
||||
let res = get_user_instructions(&cfg)
|
||||
.await
|
||||
.expect("instructions expected");
|
||||
let expected_path = dunce::canonicalize(
|
||||
cfg.codex_home
|
||||
.join("skills/pdf-processing/SKILL.md")
|
||||
.as_path(),
|
||||
)
|
||||
.unwrap_or_else(|_| cfg.codex_home.join("skills/pdf-processing/SKILL.md"));
|
||||
let expected_path_str = expected_path.to_string_lossy().replace('\\', "/");
|
||||
let expected = format!(
|
||||
"base doc\n\n## Skills\nThese skills are discovered at startup from ~/.codex/skills; each entry shows name, description, and file path so you can open the source for full instructions. Content is not inlined to keep context lean.\n- pdf-processing: extract from pdfs (file: {expected_path_str})"
|
||||
);
|
||||
assert_eq!(res, expected);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn skills_render_without_project_doc() {
|
||||
let tmp = tempfile::tempdir().expect("tempdir");
|
||||
let cfg = make_config(&tmp, 4096, None);
|
||||
create_skill(cfg.codex_home.clone(), "linting", "run clippy");
|
||||
|
||||
let res = get_user_instructions(&cfg)
|
||||
.await
|
||||
.expect("instructions expected");
|
||||
let expected_path =
|
||||
dunce::canonicalize(cfg.codex_home.join("skills/linting/SKILL.md").as_path())
|
||||
.unwrap_or_else(|_| cfg.codex_home.join("skills/linting/SKILL.md"));
|
||||
let expected_path_str = expected_path.to_string_lossy().replace('\\', "/");
|
||||
let expected = format!(
|
||||
"## Skills\nThese skills are discovered at startup from ~/.codex/skills; each entry shows name, description, and file path so you can open the source for full instructions. Content is not inlined to keep context lean.\n- linting: run clippy (file: {expected_path_str})"
|
||||
);
|
||||
assert_eq!(res, expected);
|
||||
}
|
||||
|
||||
fn create_skill(codex_home: PathBuf, name: &str, description: &str) {
|
||||
let skill_dir = codex_home.join(format!("skills/{name}"));
|
||||
fs::create_dir_all(&skill_dir).unwrap();
|
||||
let content = format!("---\nname: {name}\ndescription: {description}\n---\n\n# Body\n");
|
||||
fs::write(skill_dir.join("SKILL.md"), content).unwrap();
|
||||
}
|
||||
}
|
||||
|
||||
93
codex-rs/core/src/review_prompts.rs
Normal file
93
codex-rs/core/src/review_prompts.rs
Normal file
@@ -0,0 +1,93 @@
|
||||
use codex_git::merge_base_with_head;
|
||||
use codex_protocol::protocol::ReviewRequest;
|
||||
use codex_protocol::protocol::ReviewTarget;
|
||||
use std::path::Path;
|
||||
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
pub struct ResolvedReviewRequest {
|
||||
pub target: ReviewTarget,
|
||||
pub prompt: String,
|
||||
pub user_facing_hint: String,
|
||||
}
|
||||
|
||||
const UNCOMMITTED_PROMPT: &str = "Review the current code changes (staged, unstaged, and untracked files) and provide prioritized findings.";
|
||||
|
||||
const BASE_BRANCH_PROMPT_BACKUP: &str = "Review the code changes against the base branch '{branch}'. Start by finding the merge diff between the current branch and {branch}'s upstream e.g. (`git merge-base HEAD \"$(git rev-parse --abbrev-ref \"{branch}@{upstream}\")\"`), then run `git diff` against that SHA to see what changes we would merge into the {branch} branch. Provide prioritized, actionable findings.";
|
||||
const BASE_BRANCH_PROMPT: &str = "Review the code changes against the base branch '{baseBranch}'. The merge base commit for this comparison is {mergeBaseSha}. Run `git diff {mergeBaseSha}` to inspect the changes relative to {baseBranch}. Provide prioritized, actionable findings.";
|
||||
|
||||
const COMMIT_PROMPT_WITH_TITLE: &str = "Review the code changes introduced by commit {sha} (\"{title}\"). Provide prioritized, actionable findings.";
|
||||
const COMMIT_PROMPT: &str =
|
||||
"Review the code changes introduced by commit {sha}. Provide prioritized, actionable findings.";
|
||||
|
||||
pub fn resolve_review_request(
|
||||
request: ReviewRequest,
|
||||
cwd: &Path,
|
||||
) -> anyhow::Result<ResolvedReviewRequest> {
|
||||
let target = request.target;
|
||||
let prompt = review_prompt(&target, cwd)?;
|
||||
let user_facing_hint = request
|
||||
.user_facing_hint
|
||||
.unwrap_or_else(|| user_facing_hint(&target));
|
||||
|
||||
Ok(ResolvedReviewRequest {
|
||||
target,
|
||||
prompt,
|
||||
user_facing_hint,
|
||||
})
|
||||
}
|
||||
|
||||
pub fn review_prompt(target: &ReviewTarget, cwd: &Path) -> anyhow::Result<String> {
|
||||
match target {
|
||||
ReviewTarget::UncommittedChanges => Ok(UNCOMMITTED_PROMPT.to_string()),
|
||||
ReviewTarget::BaseBranch { branch } => {
|
||||
if let Some(commit) = merge_base_with_head(cwd, branch)? {
|
||||
Ok(BASE_BRANCH_PROMPT
|
||||
.replace("{baseBranch}", branch)
|
||||
.replace("{mergeBaseSha}", &commit))
|
||||
} else {
|
||||
Ok(BASE_BRANCH_PROMPT_BACKUP.replace("{branch}", branch))
|
||||
}
|
||||
}
|
||||
ReviewTarget::Commit { sha, title } => {
|
||||
if let Some(title) = title {
|
||||
Ok(COMMIT_PROMPT_WITH_TITLE
|
||||
.replace("{sha}", sha)
|
||||
.replace("{title}", title))
|
||||
} else {
|
||||
Ok(COMMIT_PROMPT.replace("{sha}", sha))
|
||||
}
|
||||
}
|
||||
ReviewTarget::Custom { instructions } => {
|
||||
let prompt = instructions.trim();
|
||||
if prompt.is_empty() {
|
||||
anyhow::bail!("Review prompt cannot be empty");
|
||||
}
|
||||
Ok(prompt.to_string())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn user_facing_hint(target: &ReviewTarget) -> String {
|
||||
match target {
|
||||
ReviewTarget::UncommittedChanges => "current changes".to_string(),
|
||||
ReviewTarget::BaseBranch { branch } => format!("changes against '{branch}'"),
|
||||
ReviewTarget::Commit { sha, title } => {
|
||||
let short_sha: String = sha.chars().take(7).collect();
|
||||
if let Some(title) = title {
|
||||
format!("commit {short_sha}: {title}")
|
||||
} else {
|
||||
format!("commit {short_sha}")
|
||||
}
|
||||
}
|
||||
ReviewTarget::Custom { instructions } => instructions.trim().to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
impl From<ResolvedReviewRequest> for ReviewRequest {
|
||||
fn from(resolved: ResolvedReviewRequest) -> Self {
|
||||
ReviewRequest {
|
||||
target: resolved.target,
|
||||
user_facing_hint: Some(resolved.user_facing_hint),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -6,6 +6,7 @@ use codex_apply_patch::ApplyPatchAction;
|
||||
use codex_apply_patch::ApplyPatchFileChange;
|
||||
|
||||
use crate::exec::SandboxType;
|
||||
use crate::util::resolve_path;
|
||||
|
||||
use crate::protocol::AskForApproval;
|
||||
use crate::protocol::SandboxPolicy;
|
||||
@@ -150,11 +151,7 @@ fn is_write_patch_constrained_to_writable_paths(
|
||||
// and roots are converted to absolute, normalized forms before the
|
||||
// prefix check.
|
||||
let is_path_writable = |p: &PathBuf| {
|
||||
let abs = if p.is_absolute() {
|
||||
p.clone()
|
||||
} else {
|
||||
cwd.join(p)
|
||||
};
|
||||
let abs = resolve_path(cwd, p);
|
||||
let abs = match normalize(&abs) {
|
||||
Some(v) => v,
|
||||
None => return false,
|
||||
|
||||
291
codex-rs/core/src/skills/loader.rs
Normal file
291
codex-rs/core/src/skills/loader.rs
Normal file
@@ -0,0 +1,291 @@
|
||||
use crate::config::Config;
|
||||
use crate::skills::model::SkillError;
|
||||
use crate::skills::model::SkillLoadOutcome;
|
||||
use crate::skills::model::SkillMetadata;
|
||||
use dunce::canonicalize as normalize_path;
|
||||
use serde::Deserialize;
|
||||
use std::collections::VecDeque;
|
||||
use std::error::Error;
|
||||
use std::fmt;
|
||||
use std::fs;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use tracing::error;
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
struct SkillFrontmatter {
|
||||
name: String,
|
||||
description: String,
|
||||
}
|
||||
|
||||
const SKILLS_FILENAME: &str = "SKILL.md";
|
||||
const SKILLS_DIR_NAME: &str = "skills";
|
||||
const MAX_NAME_LEN: usize = 100;
|
||||
const MAX_DESCRIPTION_LEN: usize = 500;
|
||||
|
||||
#[derive(Debug)]
|
||||
enum SkillParseError {
|
||||
Read(std::io::Error),
|
||||
MissingFrontmatter,
|
||||
InvalidYaml(serde_yaml::Error),
|
||||
MissingField(&'static str),
|
||||
InvalidField { field: &'static str, reason: String },
|
||||
}
|
||||
|
||||
impl fmt::Display for SkillParseError {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
match self {
|
||||
SkillParseError::Read(e) => write!(f, "failed to read file: {e}"),
|
||||
SkillParseError::MissingFrontmatter => {
|
||||
write!(f, "missing YAML frontmatter delimited by ---")
|
||||
}
|
||||
SkillParseError::InvalidYaml(e) => write!(f, "invalid YAML: {e}"),
|
||||
SkillParseError::MissingField(field) => write!(f, "missing field `{field}`"),
|
||||
SkillParseError::InvalidField { field, reason } => {
|
||||
write!(f, "invalid {field}: {reason}")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Error for SkillParseError {}
|
||||
|
||||
pub fn load_skills(config: &Config) -> SkillLoadOutcome {
|
||||
let mut outcome = SkillLoadOutcome::default();
|
||||
let roots = skill_roots(config);
|
||||
for root in roots {
|
||||
discover_skills_under_root(&root, &mut outcome);
|
||||
}
|
||||
|
||||
outcome
|
||||
.skills
|
||||
.sort_by(|a, b| a.name.cmp(&b.name).then_with(|| a.path.cmp(&b.path)));
|
||||
|
||||
outcome
|
||||
}
|
||||
|
||||
fn skill_roots(config: &Config) -> Vec<PathBuf> {
|
||||
vec![config.codex_home.join(SKILLS_DIR_NAME)]
|
||||
}
|
||||
|
||||
fn discover_skills_under_root(root: &Path, outcome: &mut SkillLoadOutcome) {
|
||||
let Ok(root) = normalize_path(root) else {
|
||||
return;
|
||||
};
|
||||
|
||||
if !root.is_dir() {
|
||||
return;
|
||||
}
|
||||
|
||||
let mut queue: VecDeque<PathBuf> = VecDeque::from([root]);
|
||||
while let Some(dir) = queue.pop_front() {
|
||||
let entries = match fs::read_dir(&dir) {
|
||||
Ok(entries) => entries,
|
||||
Err(e) => {
|
||||
error!("failed to read skills dir {}: {e:#}", dir.display());
|
||||
continue;
|
||||
}
|
||||
};
|
||||
|
||||
for entry in entries.flatten() {
|
||||
let path = entry.path();
|
||||
let file_name = match path.file_name().and_then(|f| f.to_str()) {
|
||||
Some(name) => name,
|
||||
None => continue,
|
||||
};
|
||||
|
||||
if file_name.starts_with('.') {
|
||||
continue;
|
||||
}
|
||||
|
||||
let Ok(file_type) = entry.file_type() else {
|
||||
continue;
|
||||
};
|
||||
|
||||
if file_type.is_symlink() {
|
||||
continue;
|
||||
}
|
||||
|
||||
if file_type.is_dir() {
|
||||
queue.push_back(path);
|
||||
continue;
|
||||
}
|
||||
|
||||
if file_type.is_file() && file_name == SKILLS_FILENAME {
|
||||
match parse_skill_file(&path) {
|
||||
Ok(skill) => outcome.skills.push(skill),
|
||||
Err(err) => outcome.errors.push(SkillError {
|
||||
path,
|
||||
message: err.to_string(),
|
||||
}),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_skill_file(path: &Path) -> Result<SkillMetadata, SkillParseError> {
|
||||
let contents = fs::read_to_string(path).map_err(SkillParseError::Read)?;
|
||||
|
||||
let frontmatter = extract_frontmatter(&contents).ok_or(SkillParseError::MissingFrontmatter)?;
|
||||
|
||||
let parsed: SkillFrontmatter =
|
||||
serde_yaml::from_str(&frontmatter).map_err(SkillParseError::InvalidYaml)?;
|
||||
|
||||
let name = sanitize_single_line(&parsed.name);
|
||||
let description = sanitize_single_line(&parsed.description);
|
||||
|
||||
validate_field(&name, MAX_NAME_LEN, "name")?;
|
||||
validate_field(&description, MAX_DESCRIPTION_LEN, "description")?;
|
||||
|
||||
let resolved_path = normalize_path(path).unwrap_or_else(|_| path.to_path_buf());
|
||||
|
||||
Ok(SkillMetadata {
|
||||
name,
|
||||
description,
|
||||
path: resolved_path,
|
||||
})
|
||||
}
|
||||
|
||||
fn sanitize_single_line(raw: &str) -> String {
|
||||
raw.split_whitespace().collect::<Vec<_>>().join(" ")
|
||||
}
|
||||
|
||||
fn validate_field(
|
||||
value: &str,
|
||||
max_len: usize,
|
||||
field_name: &'static str,
|
||||
) -> Result<(), SkillParseError> {
|
||||
if value.is_empty() {
|
||||
return Err(SkillParseError::MissingField(field_name));
|
||||
}
|
||||
if value.len() > max_len {
|
||||
return Err(SkillParseError::InvalidField {
|
||||
field: field_name,
|
||||
reason: format!("exceeds maximum length of {max_len} characters"),
|
||||
});
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn extract_frontmatter(contents: &str) -> Option<String> {
|
||||
let mut lines = contents.lines();
|
||||
if !matches!(lines.next(), Some(line) if line.trim() == "---") {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut frontmatter_lines: Vec<&str> = Vec::new();
|
||||
let mut found_closing = false;
|
||||
for line in lines.by_ref() {
|
||||
if line.trim() == "---" {
|
||||
found_closing = true;
|
||||
break;
|
||||
}
|
||||
frontmatter_lines.push(line);
|
||||
}
|
||||
|
||||
if frontmatter_lines.is_empty() || !found_closing {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(frontmatter_lines.join("\n"))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use crate::config::ConfigOverrides;
|
||||
use crate::config::ConfigToml;
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn make_config(codex_home: &TempDir) -> Config {
|
||||
let mut config = Config::load_from_base_config_with_overrides(
|
||||
ConfigToml::default(),
|
||||
ConfigOverrides::default(),
|
||||
codex_home.path().to_path_buf(),
|
||||
)
|
||||
.expect("defaults for test should always succeed");
|
||||
|
||||
config.cwd = codex_home.path().to_path_buf();
|
||||
config
|
||||
}
|
||||
|
||||
fn write_skill(codex_home: &TempDir, dir: &str, name: &str, description: &str) -> PathBuf {
|
||||
let skill_dir = codex_home.path().join(format!("skills/{dir}"));
|
||||
fs::create_dir_all(&skill_dir).unwrap();
|
||||
let indented_description = description.replace('\n', "\n ");
|
||||
let content = format!(
|
||||
"---\nname: {name}\ndescription: |-\n {indented_description}\n---\n\n# Body\n"
|
||||
);
|
||||
let path = skill_dir.join(SKILLS_FILENAME);
|
||||
fs::write(&path, content).unwrap();
|
||||
path
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn loads_valid_skill() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
write_skill(&codex_home, "demo", "demo-skill", "does things\ncarefully");
|
||||
let cfg = make_config(&codex_home);
|
||||
|
||||
let outcome = load_skills(&cfg);
|
||||
assert!(
|
||||
outcome.errors.is_empty(),
|
||||
"unexpected errors: {:?}",
|
||||
outcome.errors
|
||||
);
|
||||
assert_eq!(outcome.skills.len(), 1);
|
||||
let skill = &outcome.skills[0];
|
||||
assert_eq!(skill.name, "demo-skill");
|
||||
assert_eq!(skill.description, "does things carefully");
|
||||
let path_str = skill.path.to_string_lossy().replace('\\', "/");
|
||||
assert!(
|
||||
path_str.ends_with("skills/demo/SKILL.md"),
|
||||
"unexpected path {path_str}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn skips_hidden_and_invalid() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let hidden_dir = codex_home.path().join("skills/.hidden");
|
||||
fs::create_dir_all(&hidden_dir).unwrap();
|
||||
fs::write(
|
||||
hidden_dir.join(SKILLS_FILENAME),
|
||||
"---\nname: hidden\ndescription: hidden\n---\n",
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
// Invalid because missing closing frontmatter.
|
||||
let invalid_dir = codex_home.path().join("skills/invalid");
|
||||
fs::create_dir_all(&invalid_dir).unwrap();
|
||||
fs::write(invalid_dir.join(SKILLS_FILENAME), "---\nname: bad").unwrap();
|
||||
|
||||
let cfg = make_config(&codex_home);
|
||||
let outcome = load_skills(&cfg);
|
||||
assert_eq!(outcome.skills.len(), 0);
|
||||
assert_eq!(outcome.errors.len(), 1);
|
||||
assert!(
|
||||
outcome.errors[0]
|
||||
.message
|
||||
.contains("missing YAML frontmatter"),
|
||||
"expected frontmatter error"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn enforces_length_limits() {
|
||||
let codex_home = tempfile::tempdir().expect("tempdir");
|
||||
let long_desc = "a".repeat(MAX_DESCRIPTION_LEN + 1);
|
||||
write_skill(&codex_home, "too-long", "toolong", &long_desc);
|
||||
let cfg = make_config(&codex_home);
|
||||
|
||||
let outcome = load_skills(&cfg);
|
||||
assert_eq!(outcome.skills.len(), 0);
|
||||
assert_eq!(outcome.errors.len(), 1);
|
||||
assert!(
|
||||
outcome.errors[0].message.contains("invalid description"),
|
||||
"expected length error"
|
||||
);
|
||||
}
|
||||
}
|
||||
9
codex-rs/core/src/skills/mod.rs
Normal file
9
codex-rs/core/src/skills/mod.rs
Normal file
@@ -0,0 +1,9 @@
|
||||
pub mod loader;
|
||||
pub mod model;
|
||||
pub mod render;
|
||||
|
||||
pub use loader::load_skills;
|
||||
pub use model::SkillError;
|
||||
pub use model::SkillLoadOutcome;
|
||||
pub use model::SkillMetadata;
|
||||
pub use render::render_skills_section;
|
||||
20
codex-rs/core/src/skills/model.rs
Normal file
20
codex-rs/core/src/skills/model.rs
Normal file
@@ -0,0 +1,20 @@
|
||||
use std::path::PathBuf;
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct SkillMetadata {
|
||||
pub name: String,
|
||||
pub description: String,
|
||||
pub path: PathBuf,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct SkillError {
|
||||
pub path: PathBuf,
|
||||
pub message: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Default)]
|
||||
pub struct SkillLoadOutcome {
|
||||
pub skills: Vec<SkillMetadata>,
|
||||
pub errors: Vec<SkillError>,
|
||||
}
|
||||
21
codex-rs/core/src/skills/render.rs
Normal file
21
codex-rs/core/src/skills/render.rs
Normal file
@@ -0,0 +1,21 @@
|
||||
use crate::skills::model::SkillMetadata;
|
||||
|
||||
pub fn render_skills_section(skills: &[SkillMetadata]) -> Option<String> {
|
||||
if skills.is_empty() {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut lines: Vec<String> = Vec::new();
|
||||
lines.push("## Skills".to_string());
|
||||
lines.push("These skills are discovered at startup from ~/.codex/skills; each entry shows name, description, and file path so you can open the source for full instructions. Content is not inlined to keep context lean.".to_string());
|
||||
|
||||
for skill in skills {
|
||||
let path_str = skill.path.to_string_lossy().replace('\\', "/");
|
||||
lines.push(format!(
|
||||
"- {}: {} (file: {})",
|
||||
skill.name, skill.description, path_str
|
||||
));
|
||||
}
|
||||
|
||||
Some(lines.join("\n"))
|
||||
}
|
||||
@@ -62,7 +62,10 @@ impl SessionState {
|
||||
}
|
||||
|
||||
pub(crate) fn set_rate_limits(&mut self, snapshot: RateLimitSnapshot) {
|
||||
self.latest_rate_limits = Some(snapshot);
|
||||
self.latest_rate_limits = Some(merge_rate_limit_credits(
|
||||
self.latest_rate_limits.as_ref(),
|
||||
snapshot,
|
||||
));
|
||||
}
|
||||
|
||||
pub(crate) fn token_info_and_rate_limits(
|
||||
@@ -79,3 +82,14 @@ impl SessionState {
|
||||
self.history.get_total_token_usage()
|
||||
}
|
||||
}
|
||||
|
||||
// Sometimes new snapshots don't include credits
|
||||
fn merge_rate_limit_credits(
|
||||
previous: Option<&RateLimitSnapshot>,
|
||||
mut snapshot: RateLimitSnapshot,
|
||||
) -> RateLimitSnapshot {
|
||||
if snapshot.credits.is_none() {
|
||||
snapshot.credits = previous.and_then(|prior| prior.credits.clone());
|
||||
}
|
||||
snapshot
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
use std::collections::BTreeMap;
|
||||
use std::path::Path;
|
||||
|
||||
use crate::apply_patch;
|
||||
use crate::apply_patch::InternalApplyPatchInvocation;
|
||||
@@ -7,7 +8,10 @@ use crate::client_common::tools::FreeformTool;
|
||||
use crate::client_common::tools::FreeformToolFormat;
|
||||
use crate::client_common::tools::ResponsesApiTool;
|
||||
use crate::client_common::tools::ToolSpec;
|
||||
use crate::codex::Session;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::function_tool::FunctionCallError;
|
||||
use crate::tools::context::SharedTurnDiffTracker;
|
||||
use crate::tools::context::ToolInvocation;
|
||||
use crate::tools::context::ToolOutput;
|
||||
use crate::tools::context::ToolPayload;
|
||||
@@ -164,6 +168,80 @@ pub enum ApplyPatchToolType {
|
||||
Function,
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) async fn intercept_apply_patch(
|
||||
command: &[String],
|
||||
cwd: &Path,
|
||||
timeout_ms: Option<u64>,
|
||||
session: &Session,
|
||||
turn: &TurnContext,
|
||||
tracker: Option<&SharedTurnDiffTracker>,
|
||||
call_id: &str,
|
||||
tool_name: &str,
|
||||
) -> Result<Option<ToolOutput>, FunctionCallError> {
|
||||
match codex_apply_patch::maybe_parse_apply_patch_verified(command, cwd) {
|
||||
codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => {
|
||||
match apply_patch::apply_patch(session, turn, call_id, changes).await {
|
||||
InternalApplyPatchInvocation::Output(item) => {
|
||||
let content = item?;
|
||||
Ok(Some(ToolOutput::Function {
|
||||
content,
|
||||
content_items: None,
|
||||
success: Some(true),
|
||||
}))
|
||||
}
|
||||
InternalApplyPatchInvocation::DelegateToExec(apply) => {
|
||||
let emitter = ToolEmitter::apply_patch(
|
||||
convert_apply_patch_to_protocol(&apply.action),
|
||||
!apply.user_explicitly_approved_this_action,
|
||||
);
|
||||
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(),
|
||||
timeout_ms,
|
||||
user_explicitly_approved: apply.user_explicitly_approved_this_action,
|
||||
codex_exe: turn.codex_linux_sandbox_exe.clone(),
|
||||
};
|
||||
|
||||
let mut orchestrator = ToolOrchestrator::new();
|
||||
let mut runtime = ApplyPatchRuntime::new();
|
||||
let tool_ctx = ToolCtx {
|
||||
session,
|
||||
turn,
|
||||
call_id: call_id.to_string(),
|
||||
tool_name: tool_name.to_string(),
|
||||
};
|
||||
let out = orchestrator
|
||||
.run(&mut runtime, &req, &tool_ctx, turn, turn.approval_policy)
|
||||
.await;
|
||||
let event_ctx =
|
||||
ToolEventCtx::new(session, turn, call_id, tracker.as_ref().copied());
|
||||
let content = emitter.finish(event_ctx, out).await?;
|
||||
Ok(Some(ToolOutput::Function {
|
||||
content,
|
||||
content_items: None,
|
||||
success: Some(true),
|
||||
}))
|
||||
}
|
||||
}
|
||||
}
|
||||
codex_apply_patch::MaybeApplyPatchVerified::CorrectnessError(parse_error) => {
|
||||
Err(FunctionCallError::RespondToModel(format!(
|
||||
"apply_patch verification failed: {parse_error}"
|
||||
)))
|
||||
}
|
||||
codex_apply_patch::MaybeApplyPatchVerified::ShellParseError(error) => {
|
||||
tracing::trace!("Failed to parse apply_patch input, {error:?}");
|
||||
Ok(None)
|
||||
}
|
||||
codex_apply_patch::MaybeApplyPatchVerified::NotApplyPatch => Ok(None),
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns a custom tool that can be used to edit files. Well-suited for GPT-5 models
|
||||
/// https://platform.openai.com/docs/guides/function-calling#custom-tools
|
||||
pub(crate) fn create_apply_patch_freeform_tool() -> ToolSpec {
|
||||
|
||||
@@ -3,9 +3,6 @@ use codex_protocol::models::ShellCommandToolCallParams;
|
||||
use codex_protocol::models::ShellToolCallParams;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::apply_patch;
|
||||
use crate::apply_patch::InternalApplyPatchInvocation;
|
||||
use crate::apply_patch::convert_apply_patch_to_protocol;
|
||||
use crate::codex::TurnContext;
|
||||
use crate::exec::ExecParams;
|
||||
use crate::exec_env::create_env;
|
||||
@@ -19,11 +16,10 @@ use crate::tools::context::ToolOutput;
|
||||
use crate::tools::context::ToolPayload;
|
||||
use crate::tools::events::ToolEmitter;
|
||||
use crate::tools::events::ToolEventCtx;
|
||||
use crate::tools::handlers::apply_patch::intercept_apply_patch;
|
||||
use crate::tools::orchestrator::ToolOrchestrator;
|
||||
use crate::tools::registry::ToolHandler;
|
||||
use crate::tools::registry::ToolKind;
|
||||
use crate::tools::runtimes::apply_patch::ApplyPatchRequest;
|
||||
use crate::tools::runtimes::apply_patch::ApplyPatchRuntime;
|
||||
use crate::tools::runtimes::shell::ShellRequest;
|
||||
use crate::tools::runtimes::shell::ShellRuntime;
|
||||
use crate::tools::sandboxing::ToolCtx;
|
||||
@@ -210,81 +206,19 @@ impl ShellHandler {
|
||||
}
|
||||
|
||||
// Intercept apply_patch if present.
|
||||
match codex_apply_patch::maybe_parse_apply_patch_verified(
|
||||
if let Some(output) = intercept_apply_patch(
|
||||
&exec_params.command,
|
||||
&exec_params.cwd,
|
||||
) {
|
||||
codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => {
|
||||
match apply_patch::apply_patch(session.as_ref(), turn.as_ref(), &call_id, changes)
|
||||
.await
|
||||
{
|
||||
InternalApplyPatchInvocation::Output(item) => {
|
||||
// Programmatic apply_patch path; return its result.
|
||||
let content = item?;
|
||||
return Ok(ToolOutput::Function {
|
||||
content,
|
||||
content_items: None,
|
||||
success: Some(true),
|
||||
});
|
||||
}
|
||||
InternalApplyPatchInvocation::DelegateToExec(apply) => {
|
||||
let emitter = ToolEmitter::apply_patch(
|
||||
convert_apply_patch_to_protocol(&apply.action),
|
||||
!apply.user_explicitly_approved_this_action,
|
||||
);
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
session.as_ref(),
|
||||
turn.as_ref(),
|
||||
&call_id,
|
||||
Some(&tracker),
|
||||
);
|
||||
emitter.begin(event_ctx).await;
|
||||
|
||||
let req = ApplyPatchRequest {
|
||||
patch: apply.action.patch.clone(),
|
||||
cwd: apply.action.cwd.clone(),
|
||||
timeout_ms: exec_params.expiration.timeout_ms(),
|
||||
user_explicitly_approved: apply.user_explicitly_approved_this_action,
|
||||
codex_exe: turn.codex_linux_sandbox_exe.clone(),
|
||||
};
|
||||
let mut orchestrator = ToolOrchestrator::new();
|
||||
let mut runtime = ApplyPatchRuntime::new();
|
||||
let tool_ctx = ToolCtx {
|
||||
session: session.as_ref(),
|
||||
turn: turn.as_ref(),
|
||||
call_id: call_id.clone(),
|
||||
tool_name: tool_name.to_string(),
|
||||
};
|
||||
let out = orchestrator
|
||||
.run(&mut runtime, &req, &tool_ctx, &turn, turn.approval_policy)
|
||||
.await;
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
session.as_ref(),
|
||||
turn.as_ref(),
|
||||
&call_id,
|
||||
Some(&tracker),
|
||||
);
|
||||
let content = emitter.finish(event_ctx, out).await?;
|
||||
return Ok(ToolOutput::Function {
|
||||
content,
|
||||
content_items: None,
|
||||
success: Some(true),
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
codex_apply_patch::MaybeApplyPatchVerified::CorrectnessError(parse_error) => {
|
||||
return Err(FunctionCallError::RespondToModel(format!(
|
||||
"apply_patch verification failed: {parse_error}"
|
||||
)));
|
||||
}
|
||||
codex_apply_patch::MaybeApplyPatchVerified::ShellParseError(error) => {
|
||||
tracing::trace!("Failed to parse shell command, {error:?}");
|
||||
// Fall through to regular shell execution.
|
||||
}
|
||||
codex_apply_patch::MaybeApplyPatchVerified::NotApplyPatch => {
|
||||
// Fall through to regular shell execution.
|
||||
}
|
||||
exec_params.expiration.timeout_ms(),
|
||||
session.as_ref(),
|
||||
turn.as_ref(),
|
||||
Some(&tracker),
|
||||
&call_id,
|
||||
tool_name,
|
||||
)
|
||||
.await?
|
||||
{
|
||||
return Ok(output);
|
||||
}
|
||||
|
||||
let source = ExecCommandSource::Agent;
|
||||
|
||||
@@ -13,6 +13,7 @@ use crate::tools::context::ToolPayload;
|
||||
use crate::tools::events::ToolEmitter;
|
||||
use crate::tools::events::ToolEventCtx;
|
||||
use crate::tools::events::ToolEventStage;
|
||||
use crate::tools::handlers::apply_patch::intercept_apply_patch;
|
||||
use crate::tools::registry::ToolHandler;
|
||||
use crate::tools::registry::ToolKind;
|
||||
use crate::unified_exec::ExecCommandRequest;
|
||||
@@ -103,6 +104,7 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
let ToolInvocation {
|
||||
session,
|
||||
turn,
|
||||
tracker,
|
||||
call_id,
|
||||
tool_name,
|
||||
payload,
|
||||
@@ -153,12 +155,26 @@ impl ToolHandler for UnifiedExecHandler {
|
||||
)));
|
||||
}
|
||||
|
||||
let workdir = workdir
|
||||
.as_deref()
|
||||
.filter(|value| !value.is_empty())
|
||||
.map(PathBuf::from);
|
||||
let workdir = workdir.filter(|value| !value.is_empty());
|
||||
|
||||
let workdir = workdir.map(|dir| context.turn.resolve_path(Some(dir)));
|
||||
let cwd = workdir.clone().unwrap_or_else(|| context.turn.cwd.clone());
|
||||
|
||||
if let Some(output) = intercept_apply_patch(
|
||||
&command,
|
||||
&cwd,
|
||||
Some(yield_time_ms),
|
||||
context.session.as_ref(),
|
||||
context.turn.as_ref(),
|
||||
Some(&tracker),
|
||||
&context.call_id,
|
||||
tool_name.as_str(),
|
||||
)
|
||||
.await?
|
||||
{
|
||||
return Ok(output);
|
||||
}
|
||||
|
||||
let event_ctx = ToolEventCtx::new(
|
||||
context.session.as_ref(),
|
||||
context.turn.as_ref(),
|
||||
|
||||
@@ -48,6 +48,9 @@ pub(crate) const UNIFIED_EXEC_OUTPUT_MAX_BYTES: usize = 1024 * 1024; // 1 MiB
|
||||
pub(crate) const UNIFIED_EXEC_OUTPUT_MAX_TOKENS: usize = UNIFIED_EXEC_OUTPUT_MAX_BYTES / 4;
|
||||
pub(crate) const MAX_UNIFIED_EXEC_SESSIONS: usize = 64;
|
||||
|
||||
// Send a warning message to the models when it reaches this number of sessions.
|
||||
pub(crate) const WARNING_UNIFIED_EXEC_SESSIONS: usize = 60;
|
||||
|
||||
pub(crate) struct UnifiedExecContext {
|
||||
pub session: Arc<Session>,
|
||||
pub turn: Arc<TurnContext>,
|
||||
|
||||
@@ -41,6 +41,7 @@ use super::UnifiedExecContext;
|
||||
use super::UnifiedExecError;
|
||||
use super::UnifiedExecResponse;
|
||||
use super::UnifiedExecSessionManager;
|
||||
use super::WARNING_UNIFIED_EXEC_SESSIONS;
|
||||
use super::WriteStdinRequest;
|
||||
use super::clamp_yield_time;
|
||||
use super::generate_chunk_id;
|
||||
@@ -421,9 +422,22 @@ impl UnifiedExecSessionManager {
|
||||
started_at,
|
||||
last_used: started_at,
|
||||
};
|
||||
let mut store = self.session_store.lock().await;
|
||||
Self::prune_sessions_if_needed(&mut store);
|
||||
store.sessions.insert(process_id, entry);
|
||||
let number_sessions = {
|
||||
let mut store = self.session_store.lock().await;
|
||||
Self::prune_sessions_if_needed(&mut store);
|
||||
store.sessions.insert(process_id, entry);
|
||||
store.sessions.len()
|
||||
};
|
||||
|
||||
if number_sessions >= WARNING_UNIFIED_EXEC_SESSIONS {
|
||||
context
|
||||
.session
|
||||
.record_model_warning(
|
||||
format!("The maximum number of unified exec sessions you can keep open is {WARNING_UNIFIED_EXEC_SESSIONS} and you currently have {number_sessions} sessions open. Reuse older sessions or close them to prevent automatic pruning of old session"),
|
||||
&context.turn
|
||||
)
|
||||
.await;
|
||||
};
|
||||
}
|
||||
|
||||
async fn emit_exec_end_from_entry(
|
||||
@@ -633,9 +647,9 @@ impl UnifiedExecSessionManager {
|
||||
collected
|
||||
}
|
||||
|
||||
fn prune_sessions_if_needed(store: &mut SessionStore) {
|
||||
fn prune_sessions_if_needed(store: &mut SessionStore) -> bool {
|
||||
if store.sessions.len() < MAX_UNIFIED_EXEC_SESSIONS {
|
||||
return;
|
||||
return false;
|
||||
}
|
||||
|
||||
let meta: Vec<(String, Instant, bool)> = store
|
||||
@@ -646,7 +660,10 @@ impl UnifiedExecSessionManager {
|
||||
|
||||
if let Some(session_id) = Self::session_id_to_prune_from_meta(&meta) {
|
||||
store.remove(&session_id);
|
||||
return true;
|
||||
}
|
||||
|
||||
false
|
||||
}
|
||||
|
||||
// Centralized pruning policy so we can easily swap strategies later.
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
|
||||
use rand::Rng;
|
||||
@@ -37,6 +39,14 @@ pub(crate) fn try_parse_error_message(text: &str) -> String {
|
||||
text.to_string()
|
||||
}
|
||||
|
||||
pub fn resolve_path(base: &Path, path: &PathBuf) -> PathBuf {
|
||||
if path.is_absolute() {
|
||||
path.clone()
|
||||
} else {
|
||||
base.join(path)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
@@ -431,6 +431,9 @@ pub fn ev_apply_patch_call(
|
||||
ApplyPatchModelOutput::ShellViaHeredoc => {
|
||||
ev_apply_patch_shell_call_via_heredoc(call_id, patch)
|
||||
}
|
||||
ApplyPatchModelOutput::ShellCommandViaHeredoc => {
|
||||
ev_apply_patch_shell_command_call_via_heredoc(call_id, patch)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -492,6 +495,13 @@ pub fn ev_apply_patch_shell_call_via_heredoc(call_id: &str, patch: &str) -> Valu
|
||||
ev_function_call(call_id, "shell", &arguments)
|
||||
}
|
||||
|
||||
pub fn ev_apply_patch_shell_command_call_via_heredoc(call_id: &str, patch: &str) -> Value {
|
||||
let args = serde_json::json!({ "command": format!("apply_patch <<'EOF'\n{patch}\nEOF\n") });
|
||||
let arguments = serde_json::to_string(&args).expect("serialize apply_patch arguments");
|
||||
|
||||
ev_function_call(call_id, "shell_command", &arguments)
|
||||
}
|
||||
|
||||
pub fn sse_failed(id: &str, code: &str, message: &str) -> String {
|
||||
sse(vec![serde_json::json!({
|
||||
"type": "response.failed",
|
||||
|
||||
@@ -36,6 +36,7 @@ pub enum ApplyPatchModelOutput {
|
||||
Function,
|
||||
Shell,
|
||||
ShellViaHeredoc,
|
||||
ShellCommandViaHeredoc,
|
||||
}
|
||||
|
||||
/// A collection of different ways the model can output an apply_patch call
|
||||
@@ -312,7 +313,10 @@ impl TestCodexHarness {
|
||||
ApplyPatchModelOutput::Freeform => self.custom_tool_call_output(call_id).await,
|
||||
ApplyPatchModelOutput::Function
|
||||
| ApplyPatchModelOutput::Shell
|
||||
| ApplyPatchModelOutput::ShellViaHeredoc => self.function_call_stdout(call_id).await,
|
||||
| ApplyPatchModelOutput::ShellViaHeredoc
|
||||
| ApplyPatchModelOutput::ShellCommandViaHeredoc => {
|
||||
self.function_call_stdout(call_id).await
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
use anyhow::Result;
|
||||
use core_test_support::responses::ev_apply_patch_call;
|
||||
use core_test_support::responses::ev_shell_command_call;
|
||||
use core_test_support::test_codex::ApplyPatchModelOutput;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::fs;
|
||||
@@ -127,6 +128,7 @@ D delete.txt
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_multiple_chunks(model_output: ApplyPatchModelOutput) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -153,6 +155,7 @@ async fn apply_patch_cli_multiple_chunks(model_output: ApplyPatchModelOutput) ->
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_moves_file_to_new_directory(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -181,6 +184,7 @@ async fn apply_patch_cli_moves_file_to_new_directory(
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_updates_file_appends_trailing_newline(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -208,6 +212,7 @@ async fn apply_patch_cli_updates_file_appends_trailing_newline(
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_insert_only_hunk_modifies_file(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -233,6 +238,7 @@ async fn apply_patch_cli_insert_only_hunk_modifies_file(
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_move_overwrites_existing_destination(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -263,6 +269,7 @@ async fn apply_patch_cli_move_overwrites_existing_destination(
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_move_without_content_change_has_no_turn_diff(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -320,6 +327,7 @@ async fn apply_patch_cli_move_without_content_change_has_no_turn_diff(
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_add_overwrites_existing_file(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -345,6 +353,7 @@ async fn apply_patch_cli_add_overwrites_existing_file(
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_rejects_invalid_hunk_header(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -376,6 +385,7 @@ async fn apply_patch_cli_rejects_invalid_hunk_header(
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_reports_missing_context(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -409,6 +419,7 @@ async fn apply_patch_cli_reports_missing_context(
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_reports_missing_target_file(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -444,6 +455,7 @@ async fn apply_patch_cli_reports_missing_target_file(
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_delete_missing_file_reports_error(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -480,6 +492,7 @@ async fn apply_patch_cli_delete_missing_file_reports_error(
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_rejects_empty_patch(model_output: ApplyPatchModelOutput) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -504,6 +517,7 @@ async fn apply_patch_cli_rejects_empty_patch(model_output: ApplyPatchModelOutput
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_delete_directory_reports_verification_error(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -530,6 +544,7 @@ async fn apply_patch_cli_delete_directory_reports_verification_error(
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_rejects_path_traversal_outside_workspace(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -582,6 +597,7 @@ async fn apply_patch_cli_rejects_path_traversal_outside_workspace(
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -635,6 +651,7 @@ async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace(
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_verification_failure_has_no_side_effects(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -677,11 +694,10 @@ async fn apply_patch_shell_command_heredoc_with_cd_updates_relative_workdir() ->
|
||||
|
||||
let script = "cd sub && apply_patch <<'EOF'\n*** Begin Patch\n*** Update File: in_sub.txt\n@@\n-before\n+after\n*** End Patch\nEOF\n";
|
||||
let call_id = "shell-heredoc-cd";
|
||||
let args = json!({ "command": script, "timeout_ms": 5_000 });
|
||||
let bodies = vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?),
|
||||
ev_shell_command_call(call_id, script),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
@@ -702,6 +718,86 @@ async fn apply_patch_shell_command_heredoc_with_cd_updates_relative_workdir() ->
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn apply_patch_shell_command_heredoc_with_cd_emits_turn_diff() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let harness = apply_patch_harness_with(|builder| builder.with_model("gpt-5.1")).await?;
|
||||
let test = harness.test();
|
||||
let codex = test.codex.clone();
|
||||
let cwd = test.cwd.clone();
|
||||
|
||||
// Prepare a file inside a subdir; update it via cd && apply_patch heredoc form.
|
||||
let sub = test.workspace_path("sub");
|
||||
fs::create_dir_all(&sub)?;
|
||||
let target = sub.join("in_sub.txt");
|
||||
fs::write(&target, "before\n")?;
|
||||
|
||||
let script = "cd sub && apply_patch <<'EOF'\n*** Begin Patch\n*** Update File: in_sub.txt\n@@\n-before\n+after\n*** End Patch\nEOF\n";
|
||||
let call_id = "shell-heredoc-cd";
|
||||
let args = json!({ "command": script, "timeout_ms": 5_000 });
|
||||
let bodies = vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_function_call(call_id, "shell_command", &serde_json::to_string(&args)?),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_assistant_message("msg-1", "ok"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
];
|
||||
mount_sse_sequence(harness.server(), bodies).await;
|
||||
|
||||
let model = test.session_configured.model.clone();
|
||||
codex
|
||||
.submit(Op::UserTurn {
|
||||
items: vec![UserInput::Text {
|
||||
text: "apply via shell heredoc with cd".into(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: cwd.path().to_path_buf(),
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
model,
|
||||
effort: None,
|
||||
summary: ReasoningSummary::Auto,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let mut saw_turn_diff = None;
|
||||
let mut saw_patch_begin = false;
|
||||
let mut patch_end_success = None;
|
||||
wait_for_event(&codex, |event| match event {
|
||||
EventMsg::PatchApplyBegin(begin) => {
|
||||
saw_patch_begin = true;
|
||||
assert_eq!(begin.call_id, call_id);
|
||||
false
|
||||
}
|
||||
EventMsg::PatchApplyEnd(end) => {
|
||||
assert_eq!(end.call_id, call_id);
|
||||
patch_end_success = Some(end.success);
|
||||
false
|
||||
}
|
||||
EventMsg::TurnDiff(ev) => {
|
||||
saw_turn_diff = Some(ev.unified_diff.clone());
|
||||
false
|
||||
}
|
||||
EventMsg::TaskComplete(_) => true,
|
||||
_ => false,
|
||||
})
|
||||
.await;
|
||||
|
||||
assert!(saw_patch_begin, "expected PatchApplyBegin event");
|
||||
let patch_end_success =
|
||||
patch_end_success.expect("expected PatchApplyEnd event to capture success flag");
|
||||
assert!(patch_end_success);
|
||||
|
||||
let diff = saw_turn_diff.expect("expected TurnDiff event");
|
||||
assert!(diff.contains("diff --git"), "diff header missing: {diff:?}");
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn apply_patch_shell_command_failure_propagates_error_and_skips_diff() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -776,7 +872,11 @@ async fn apply_patch_shell_command_failure_propagates_error_and_skips_diff() ->
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn apply_patch_function_accepts_lenient_heredoc_wrapped_patch() -> Result<()> {
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_function_accepts_lenient_heredoc_wrapped_patch(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
let harness = apply_patch_harness().await?;
|
||||
@@ -784,16 +884,8 @@ async fn apply_patch_function_accepts_lenient_heredoc_wrapped_patch() -> Result<
|
||||
let file_name = "lenient.txt";
|
||||
let patch_inner =
|
||||
format!("*** Begin Patch\n*** Add File: {file_name}\n+lenient\n*** End Patch\n");
|
||||
let wrapped = format!("<<'EOF'\n{patch_inner}EOF\n");
|
||||
let call_id = "apply-lenient";
|
||||
mount_apply_patch(
|
||||
&harness,
|
||||
call_id,
|
||||
wrapped.as_str(),
|
||||
"ok",
|
||||
ApplyPatchModelOutput::Function,
|
||||
)
|
||||
.await;
|
||||
mount_apply_patch(&harness, call_id, patch_inner.as_str(), "ok", model_output).await;
|
||||
|
||||
harness.submit("apply lenient heredoc patch").await?;
|
||||
|
||||
@@ -807,6 +899,7 @@ async fn apply_patch_function_accepts_lenient_heredoc_wrapped_patch() -> Result<
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_end_of_file_anchor(model_output: ApplyPatchModelOutput) -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -829,6 +922,7 @@ async fn apply_patch_cli_end_of_file_anchor(model_output: ApplyPatchModelOutput)
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_cli_missing_second_chunk_context_rejected(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -863,6 +957,7 @@ async fn apply_patch_cli_missing_second_chunk_context_rejected(
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_emits_turn_diff_event_with_unified_diff(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -918,6 +1013,7 @@ async fn apply_patch_emits_turn_diff_event_with_unified_diff(
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_turn_diff_for_rename_with_content_change(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
@@ -1132,6 +1228,7 @@ async fn apply_patch_aggregates_diff_preserves_success_after_failure() -> Result
|
||||
#[test_case(ApplyPatchModelOutput::Function)]
|
||||
#[test_case(ApplyPatchModelOutput::Shell)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellViaHeredoc)]
|
||||
#[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc)]
|
||||
async fn apply_patch_change_context_disambiguates_target(
|
||||
model_output: ApplyPatchModelOutput,
|
||||
) -> Result<()> {
|
||||
|
||||
@@ -15,6 +15,7 @@ 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::model_family::find_family_for_model;
|
||||
use codex_core::protocol::EventMsg;
|
||||
use codex_core::protocol::Op;
|
||||
@@ -34,6 +35,7 @@ use core_test_support::skip_if_no_network;
|
||||
use core_test_support::test_codex::TestCodex;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use dunce::canonicalize as normalize_path;
|
||||
use futures::StreamExt;
|
||||
use serde_json::json;
|
||||
use std::io::Write;
|
||||
@@ -620,6 +622,74 @@ async fn includes_user_instructions_message_in_request() {
|
||||
assert_message_ends_with(&request_body["input"][1], "</environment_context>");
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn skills_append_to_instructions_when_feature_enabled() {
|
||||
skip_if_no_network!();
|
||||
let server = MockServer::start().await;
|
||||
|
||||
let resp_mock = responses::mount_sse_once(&server, sse_completed("resp1")).await;
|
||||
|
||||
let model_provider = ModelProviderInfo {
|
||||
base_url: Some(format!("{}/v1", server.uri())),
|
||||
..built_in_model_providers()["openai"].clone()
|
||||
};
|
||||
|
||||
let codex_home = TempDir::new().unwrap();
|
||||
let skill_dir = codex_home.path().join("skills/demo");
|
||||
std::fs::create_dir_all(&skill_dir).expect("create skill dir");
|
||||
std::fs::write(
|
||||
skill_dir.join("SKILL.md"),
|
||||
"---\nname: demo\ndescription: build charts\n---\n\n# body\n",
|
||||
)
|
||||
.expect("write skill");
|
||||
|
||||
let mut config = load_default_config_for_test(&codex_home);
|
||||
config.model_provider = model_provider;
|
||||
config.features.enable(Feature::Skills);
|
||||
config.cwd = codex_home.path().to_path_buf();
|
||||
|
||||
let conversation_manager =
|
||||
ConversationManager::with_auth(CodexAuth::from_api_key("Test API Key"));
|
||||
let codex = conversation_manager
|
||||
.new_conversation(config)
|
||||
.await
|
||||
.expect("create new conversation")
|
||||
.conversation;
|
||||
|
||||
codex
|
||||
.submit(Op::UserInput {
|
||||
items: vec![UserInput::Text {
|
||||
text: "hello".into(),
|
||||
}],
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await;
|
||||
|
||||
let request = resp_mock.single_request();
|
||||
let request_body = request.body_json();
|
||||
|
||||
assert_message_role(&request_body["input"][0], "user");
|
||||
let instructions_text = request_body["input"][0]["content"][0]["text"]
|
||||
.as_str()
|
||||
.expect("instructions text");
|
||||
assert!(
|
||||
instructions_text.contains("## Skills"),
|
||||
"expected skills section present"
|
||||
);
|
||||
assert!(
|
||||
instructions_text.contains("demo: build charts"),
|
||||
"expected skill summary"
|
||||
);
|
||||
let expected_path = normalize_path(skill_dir.join("SKILL.md")).unwrap();
|
||||
let expected_path_str = expected_path.to_string_lossy().replace('\\', "/");
|
||||
assert!(
|
||||
instructions_text.contains(&expected_path_str),
|
||||
"expected path {expected_path_str} in instructions"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn includes_configured_effort_in_request() -> anyhow::Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
|
||||
@@ -3,6 +3,7 @@ use codex_core::protocol::EventMsg;
|
||||
use codex_core::protocol::Op;
|
||||
use codex_core::protocol::ReviewDecision;
|
||||
use codex_core::protocol::ReviewRequest;
|
||||
use codex_core::protocol::ReviewTarget;
|
||||
use codex_core::protocol::SandboxPolicy;
|
||||
use core_test_support::responses::ev_apply_patch_function_call;
|
||||
use core_test_support::responses::ev_assistant_message;
|
||||
@@ -68,8 +69,10 @@ async fn codex_delegate_forwards_exec_approval_and_proceeds_on_approval() {
|
||||
test.codex
|
||||
.submit(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
prompt: "Please review".to_string(),
|
||||
user_facing_hint: "review".to_string(),
|
||||
target: ReviewTarget::Custom {
|
||||
instructions: "Please review".to_string(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
},
|
||||
})
|
||||
.await
|
||||
@@ -143,8 +146,10 @@ async fn codex_delegate_forwards_patch_approval_and_proceeds_on_decision() {
|
||||
test.codex
|
||||
.submit(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
prompt: "Please review".to_string(),
|
||||
user_facing_hint: "review".to_string(),
|
||||
target: ReviewTarget::Custom {
|
||||
instructions: "Please review".to_string(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
},
|
||||
})
|
||||
.await
|
||||
@@ -197,8 +202,10 @@ async fn codex_delegate_ignores_legacy_deltas() {
|
||||
test.codex
|
||||
.submit(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
prompt: "Please review".to_string(),
|
||||
user_facing_hint: "review".to_string(),
|
||||
target: ReviewTarget::Custom {
|
||||
instructions: "Please review".to_string(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
},
|
||||
})
|
||||
.await
|
||||
|
||||
@@ -16,6 +16,7 @@ use codex_core::protocol::ReviewFinding;
|
||||
use codex_core::protocol::ReviewLineRange;
|
||||
use codex_core::protocol::ReviewOutputEvent;
|
||||
use codex_core::protocol::ReviewRequest;
|
||||
use codex_core::protocol::ReviewTarget;
|
||||
use codex_core::protocol::RolloutItem;
|
||||
use codex_core::protocol::RolloutLine;
|
||||
use codex_core::review_format::render_review_output_text;
|
||||
@@ -81,8 +82,10 @@ async fn review_op_emits_lifecycle_and_review_output() {
|
||||
codex
|
||||
.submit(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
prompt: "Please review my changes".to_string(),
|
||||
user_facing_hint: "my changes".to_string(),
|
||||
target: ReviewTarget::Custom {
|
||||
instructions: "Please review my changes".to_string(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
},
|
||||
})
|
||||
.await
|
||||
@@ -199,8 +202,10 @@ async fn review_op_with_plain_text_emits_review_fallback() {
|
||||
codex
|
||||
.submit(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
prompt: "Plain text review".to_string(),
|
||||
user_facing_hint: "plain text review".to_string(),
|
||||
target: ReviewTarget::Custom {
|
||||
instructions: "Plain text review".to_string(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
},
|
||||
})
|
||||
.await
|
||||
@@ -257,8 +262,10 @@ async fn review_filters_agent_message_related_events() {
|
||||
codex
|
||||
.submit(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
prompt: "Filter streaming events".to_string(),
|
||||
user_facing_hint: "Filter streaming events".to_string(),
|
||||
target: ReviewTarget::Custom {
|
||||
instructions: "Filter streaming events".to_string(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
},
|
||||
})
|
||||
.await
|
||||
@@ -336,8 +343,10 @@ async fn review_does_not_emit_agent_message_on_structured_output() {
|
||||
codex
|
||||
.submit(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
prompt: "check structured".to_string(),
|
||||
user_facing_hint: "check structured".to_string(),
|
||||
target: ReviewTarget::Custom {
|
||||
instructions: "check structured".to_string(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
},
|
||||
})
|
||||
.await
|
||||
@@ -393,8 +402,10 @@ async fn review_uses_custom_review_model_from_config() {
|
||||
codex
|
||||
.submit(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
prompt: "use custom model".to_string(),
|
||||
user_facing_hint: "use custom model".to_string(),
|
||||
target: ReviewTarget::Custom {
|
||||
instructions: "use custom model".to_string(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
},
|
||||
})
|
||||
.await
|
||||
@@ -510,8 +521,10 @@ async fn review_input_isolated_from_parent_history() {
|
||||
codex
|
||||
.submit(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
prompt: review_prompt.clone(),
|
||||
user_facing_hint: review_prompt.clone(),
|
||||
target: ReviewTarget::Custom {
|
||||
instructions: review_prompt.clone(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
},
|
||||
})
|
||||
.await
|
||||
@@ -621,8 +634,10 @@ async fn review_history_surfaces_in_parent_session() {
|
||||
codex
|
||||
.submit(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
prompt: "Start a review".to_string(),
|
||||
user_facing_hint: "Start a review".to_string(),
|
||||
target: ReviewTarget::Custom {
|
||||
instructions: "Start a review".to_string(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
},
|
||||
})
|
||||
.await
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
#![cfg(not(target_os = "windows"))]
|
||||
use std::collections::HashMap;
|
||||
use std::ffi::OsStr;
|
||||
use std::fs;
|
||||
use std::sync::OnceLock;
|
||||
|
||||
use anyhow::Context;
|
||||
@@ -23,6 +25,7 @@ use core_test_support::responses::start_mock_server;
|
||||
use core_test_support::skip_if_no_network;
|
||||
use core_test_support::skip_if_sandbox;
|
||||
use core_test_support::test_codex::TestCodex;
|
||||
use core_test_support::test_codex::TestCodexHarness;
|
||||
use core_test_support::test_codex::test_codex;
|
||||
use core_test_support::wait_for_event;
|
||||
use core_test_support::wait_for_event_match;
|
||||
@@ -148,6 +151,130 @@ fn collect_tool_outputs(bodies: &[Value]) -> Result<HashMap<String, ParsedUnifie
|
||||
Ok(outputs)
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn unified_exec_intercepts_apply_patch_exec_command() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
|
||||
let builder = test_codex().with_config(|config| {
|
||||
config.include_apply_patch_tool = true;
|
||||
config.use_experimental_unified_exec_tool = true;
|
||||
config.features.enable(Feature::UnifiedExec);
|
||||
});
|
||||
let harness = TestCodexHarness::with_builder(builder).await?;
|
||||
|
||||
let patch =
|
||||
"*** Begin Patch\n*** Add File: uexec_apply.txt\n+hello from unified exec\n*** End Patch";
|
||||
let command = format!("apply_patch <<'EOF'\n{patch}\nEOF\n");
|
||||
let call_id = "uexec-apply-patch";
|
||||
let args = json!({
|
||||
"cmd": command,
|
||||
"yield_time_ms": 250,
|
||||
});
|
||||
|
||||
let responses = vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_assistant_message("msg-1", "done"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
];
|
||||
mount_sse_sequence(harness.server(), responses).await;
|
||||
|
||||
let test = harness.test();
|
||||
let codex = test.codex.clone();
|
||||
let cwd = test.cwd_path().to_path_buf();
|
||||
let session_model = test.session_configured.model.clone();
|
||||
|
||||
codex
|
||||
.submit(Op::UserTurn {
|
||||
items: vec![UserInput::Text {
|
||||
text: "apply patch via unified exec".into(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd,
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
model: session_model,
|
||||
effort: None,
|
||||
summary: ReasoningSummary::Auto,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let mut saw_patch_begin = false;
|
||||
let mut patch_end = None;
|
||||
let mut saw_exec_begin = false;
|
||||
let mut saw_exec_end = false;
|
||||
wait_for_event(&codex, |event| match event {
|
||||
EventMsg::PatchApplyBegin(begin) if begin.call_id == call_id => {
|
||||
saw_patch_begin = true;
|
||||
assert!(
|
||||
begin
|
||||
.changes
|
||||
.keys()
|
||||
.any(|path| path.file_name() == Some(OsStr::new("uexec_apply.txt"))),
|
||||
"expected apply_patch changes to target uexec_apply.txt",
|
||||
);
|
||||
false
|
||||
}
|
||||
EventMsg::PatchApplyEnd(end) if end.call_id == call_id => {
|
||||
patch_end = Some(end.clone());
|
||||
false
|
||||
}
|
||||
EventMsg::ExecCommandBegin(event) if event.call_id == call_id => {
|
||||
saw_exec_begin = true;
|
||||
false
|
||||
}
|
||||
EventMsg::ExecCommandEnd(event) if event.call_id == call_id => {
|
||||
saw_exec_end = true;
|
||||
false
|
||||
}
|
||||
EventMsg::TaskComplete(_) => true,
|
||||
_ => false,
|
||||
})
|
||||
.await;
|
||||
|
||||
assert!(
|
||||
saw_patch_begin,
|
||||
"expected apply_patch to emit PatchApplyBegin"
|
||||
);
|
||||
let patch_end = patch_end.expect("expected apply_patch to emit PatchApplyEnd");
|
||||
assert!(
|
||||
patch_end.success,
|
||||
"expected apply_patch to finish successfully: stdout={:?} stderr={:?}",
|
||||
patch_end.stdout, patch_end.stderr,
|
||||
);
|
||||
assert!(
|
||||
!saw_exec_begin,
|
||||
"apply_patch should be intercepted before exec_command begin"
|
||||
);
|
||||
assert!(
|
||||
!saw_exec_end,
|
||||
"apply_patch should not emit exec_command end events"
|
||||
);
|
||||
|
||||
let output = harness.function_call_stdout(call_id).await;
|
||||
assert!(
|
||||
output.contains("Success. Updated the following files:"),
|
||||
"expected apply_patch output, got: {output:?}"
|
||||
);
|
||||
assert!(
|
||||
output.contains("A uexec_apply.txt"),
|
||||
"expected apply_patch file summary, got: {output:?}"
|
||||
);
|
||||
assert_eq!(
|
||||
fs::read_to_string(harness.path("uexec_apply.txt"))?,
|
||||
"hello from unified exec\n"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn unified_exec_emits_exec_command_begin_event() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
@@ -224,6 +351,82 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn unified_exec_resolves_relative_workdir() -> Result<()> {
|
||||
skip_if_no_network!(Ok(()));
|
||||
skip_if_sandbox!(Ok(()));
|
||||
|
||||
let server = start_mock_server().await;
|
||||
|
||||
let mut builder = test_codex().with_model("gpt-5").with_config(|config| {
|
||||
config.use_experimental_unified_exec_tool = true;
|
||||
config.features.enable(Feature::UnifiedExec);
|
||||
});
|
||||
let TestCodex {
|
||||
codex,
|
||||
cwd,
|
||||
session_configured,
|
||||
..
|
||||
} = builder.build(&server).await?;
|
||||
|
||||
let workdir_rel = std::path::PathBuf::from("uexec_relative_workdir");
|
||||
std::fs::create_dir_all(cwd.path().join(&workdir_rel))?;
|
||||
|
||||
let call_id = "uexec-workdir-relative";
|
||||
let args = json!({
|
||||
"cmd": "pwd",
|
||||
"yield_time_ms": 250,
|
||||
"workdir": workdir_rel.to_string_lossy().to_string(),
|
||||
});
|
||||
|
||||
let responses = vec![
|
||||
sse(vec![
|
||||
ev_response_created("resp-1"),
|
||||
ev_function_call(call_id, "exec_command", &serde_json::to_string(&args)?),
|
||||
ev_completed("resp-1"),
|
||||
]),
|
||||
sse(vec![
|
||||
ev_response_created("resp-2"),
|
||||
ev_assistant_message("msg-1", "finished"),
|
||||
ev_completed("resp-2"),
|
||||
]),
|
||||
];
|
||||
mount_sse_sequence(&server, responses).await;
|
||||
|
||||
let session_model = session_configured.model.clone();
|
||||
|
||||
codex
|
||||
.submit(Op::UserTurn {
|
||||
items: vec![UserInput::Text {
|
||||
text: "run relative workdir test".into(),
|
||||
}],
|
||||
final_output_json_schema: None,
|
||||
cwd: cwd.path().to_path_buf(),
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::DangerFullAccess,
|
||||
model: session_model,
|
||||
effort: None,
|
||||
summary: ReasoningSummary::Auto,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let begin_event = wait_for_event_match(&codex, |msg| match msg {
|
||||
EventMsg::ExecCommandBegin(event) if event.call_id == call_id => Some(event.clone()),
|
||||
_ => None,
|
||||
})
|
||||
.await;
|
||||
|
||||
assert_eq!(
|
||||
begin_event.cwd,
|
||||
cwd.path().join(workdir_rel),
|
||||
"exec_command cwd should resolve relative workdir against turn cwd",
|
||||
);
|
||||
|
||||
wait_for_event(&codex, |event| matches!(event, EventMsg::TaskComplete(_))).await;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
#[ignore = "flaky"]
|
||||
async fn unified_exec_respects_workdir_override() -> Result<()> {
|
||||
|
||||
@@ -22,6 +22,7 @@ rustPlatform.buildRustPackage (_: {
|
||||
cargoLock.outputHashes = {
|
||||
"ratatui-0.29.0" = "sha256-HBvT5c8GsiCxMffNjJGLmHnvG77A6cqEL+1ARurBXho=";
|
||||
"crossterm-0.28.1" = "sha256-6qCtfSMuXACKFb9ATID39XyFDIEMFDmbx6SSmNe+728=";
|
||||
"rmcp-0.9.0" = "sha256-0iPrpf0Ha/facO3p5e0hUKHBqGp/iS+C+OdS+pRKMOU=";
|
||||
};
|
||||
|
||||
meta = with lib; {
|
||||
|
||||
@@ -24,7 +24,6 @@ anyhow = { workspace = true }
|
||||
async-trait = { workspace = true }
|
||||
clap = { workspace = true, features = ["derive"] }
|
||||
codex-core = { workspace = true }
|
||||
codex-execpolicy = { workspace = true }
|
||||
libc = { workspace = true }
|
||||
path-absolutize = { workspace = true }
|
||||
rmcp = { workspace = true, default-features = false, features = [
|
||||
@@ -52,7 +51,6 @@ tokio = { workspace = true, features = [
|
||||
"signal",
|
||||
] }
|
||||
tokio-util = { workspace = true }
|
||||
thiserror = { workspace = true }
|
||||
tracing = { workspace = true }
|
||||
tracing-subscriber = { workspace = true, features = ["env-filter", "fmt"] }
|
||||
|
||||
|
||||
@@ -55,21 +55,19 @@
|
||||
//! | |
|
||||
//! o<-----x
|
||||
//!
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use clap::Parser;
|
||||
use tracing_subscriber::EnvFilter;
|
||||
use tracing_subscriber::{self};
|
||||
|
||||
use crate::posix::exec_policy::ExecPolicyEvaluator;
|
||||
use crate::posix::exec_policy::load_policy_from_codex_home;
|
||||
use crate::posix::mcp_escalation_policy::SharedExecPolicy;
|
||||
use crate::posix::mcp_escalation_policy::ExecPolicyOutcome;
|
||||
|
||||
mod escalate_client;
|
||||
mod escalate_protocol;
|
||||
mod escalate_server;
|
||||
mod escalation_policy;
|
||||
mod exec_policy;
|
||||
mod mcp;
|
||||
mod mcp_escalation_policy;
|
||||
mod socket;
|
||||
@@ -116,10 +114,7 @@ pub async fn main_mcp_server() -> anyhow::Result<()> {
|
||||
};
|
||||
|
||||
tracing::info!("Starting MCP server");
|
||||
let policy = load_policy_from_codex_home().await?;
|
||||
let policy: SharedExecPolicy = std::sync::Arc::new(ExecPolicyEvaluator::new(policy));
|
||||
|
||||
let service = mcp::serve(bash_path, execve_wrapper, policy)
|
||||
let service = mcp::serve(bash_path, execve_wrapper, dummy_exec_policy)
|
||||
.await
|
||||
.inspect_err(|e| {
|
||||
tracing::error!("serving error: {:?}", e);
|
||||
@@ -149,3 +144,27 @@ pub async fn main_execve_wrapper() -> anyhow::Result<()> {
|
||||
let exit_code = escalate_client::run(file, argv).await?;
|
||||
std::process::exit(exit_code);
|
||||
}
|
||||
|
||||
// TODO: replace with execpolicy
|
||||
|
||||
fn dummy_exec_policy(file: &Path, argv: &[String], _workdir: &Path) -> ExecPolicyOutcome {
|
||||
if file.ends_with("rm") {
|
||||
ExecPolicyOutcome::Forbidden
|
||||
} else if file.ends_with("git") {
|
||||
ExecPolicyOutcome::Prompt {
|
||||
run_with_escalated_permissions: false,
|
||||
}
|
||||
} else if file == Path::new("/opt/homebrew/bin/gh")
|
||||
&& let [_, arg1, arg2, ..] = argv
|
||||
&& arg1 == "issue"
|
||||
&& arg2 == "list"
|
||||
{
|
||||
ExecPolicyOutcome::Allow {
|
||||
run_with_escalated_permissions: true,
|
||||
}
|
||||
} else {
|
||||
ExecPolicyOutcome::Allow {
|
||||
run_with_escalated_permissions: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,266 +0,0 @@
|
||||
use std::io::ErrorKind;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
|
||||
use codex_core::command_safety::is_dangerous_command::command_might_be_dangerous;
|
||||
use codex_execpolicy::Decision;
|
||||
use codex_execpolicy::Evaluation;
|
||||
use codex_execpolicy::Policy;
|
||||
use codex_execpolicy::PolicyParser;
|
||||
use thiserror::Error;
|
||||
use tokio::fs;
|
||||
|
||||
use crate::posix::mcp_escalation_policy::ExecPolicy;
|
||||
use crate::posix::mcp_escalation_policy::ExecPolicyOutcome;
|
||||
|
||||
const POLICY_DIR_NAME: &str = "policy";
|
||||
const POLICY_EXTENSION: &str = "codexpolicy";
|
||||
|
||||
#[derive(Debug, Error)]
|
||||
pub(crate) enum ExecPolicyError {
|
||||
#[error("failed to resolve CODEX_HOME: {source}")]
|
||||
CodexHome { source: std::io::Error },
|
||||
|
||||
#[error("failed to read execpolicy files from {dir}: {source}")]
|
||||
ReadDir {
|
||||
dir: PathBuf,
|
||||
source: std::io::Error,
|
||||
},
|
||||
|
||||
#[error("failed to read execpolicy file {path}: {source}")]
|
||||
ReadFile {
|
||||
path: PathBuf,
|
||||
source: std::io::Error,
|
||||
},
|
||||
|
||||
#[error("failed to parse execpolicy file {path}: {source}")]
|
||||
ParsePolicy {
|
||||
path: String,
|
||||
source: codex_execpolicy::Error,
|
||||
},
|
||||
}
|
||||
|
||||
pub(crate) async fn load_policy_from_codex_home() -> Result<Arc<Policy>, ExecPolicyError> {
|
||||
let codex_home = codex_core::config::find_codex_home()
|
||||
.map_err(|source| ExecPolicyError::CodexHome { source })?;
|
||||
|
||||
let policy_dir = codex_home.join(POLICY_DIR_NAME);
|
||||
let policy_paths = collect_policy_files(&policy_dir).await?;
|
||||
|
||||
let mut parser = PolicyParser::new();
|
||||
for policy_path in &policy_paths {
|
||||
let contents =
|
||||
fs::read_to_string(policy_path)
|
||||
.await
|
||||
.map_err(|source| ExecPolicyError::ReadFile {
|
||||
path: policy_path.clone(),
|
||||
source,
|
||||
})?;
|
||||
let identifier = policy_path.to_string_lossy().to_string();
|
||||
parser
|
||||
.parse(&identifier, &contents)
|
||||
.map_err(|source| ExecPolicyError::ParsePolicy {
|
||||
path: identifier,
|
||||
source,
|
||||
})?;
|
||||
}
|
||||
|
||||
let policy = Arc::new(parser.build());
|
||||
tracing::debug!(
|
||||
"loaded execpolicy from {} files in {}",
|
||||
policy_paths.len(),
|
||||
policy_dir.display()
|
||||
);
|
||||
Ok(policy)
|
||||
}
|
||||
|
||||
pub(crate) struct ExecPolicyEvaluator {
|
||||
policy: Arc<Policy>,
|
||||
}
|
||||
|
||||
impl ExecPolicyEvaluator {
|
||||
pub(crate) fn new(policy: Arc<Policy>) -> Self {
|
||||
Self { policy }
|
||||
}
|
||||
|
||||
fn command_for(file: &Path, argv: &[String]) -> Vec<String> {
|
||||
let cmd0 = argv
|
||||
.first()
|
||||
.and_then(|s| Path::new(s).file_name())
|
||||
.and_then(|s| s.to_str())
|
||||
.filter(|s| !s.is_empty())
|
||||
.map(std::string::ToString::to_string)
|
||||
.or_else(|| {
|
||||
file.file_name()
|
||||
.and_then(|s| s.to_str())
|
||||
.filter(|s| !s.is_empty())
|
||||
.map(std::string::ToString::to_string)
|
||||
})
|
||||
.unwrap_or_else(|| file.display().to_string());
|
||||
|
||||
let mut command = Vec::with_capacity(argv.len().max(1));
|
||||
command.push(cmd0);
|
||||
if argv.len() > 1 {
|
||||
command.extend(argv.iter().skip(1).cloned());
|
||||
}
|
||||
command
|
||||
}
|
||||
}
|
||||
|
||||
impl ExecPolicy for ExecPolicyEvaluator {
|
||||
fn evaluate(&self, file: &Path, argv: &[String], _workdir: &Path) -> ExecPolicyOutcome {
|
||||
let command = Self::command_for(file, argv);
|
||||
|
||||
match self.policy.check_multiple(std::iter::once(&command)) {
|
||||
Evaluation::Match { decision, .. } => match decision {
|
||||
Decision::Forbidden => ExecPolicyOutcome::Forbidden,
|
||||
Decision::Prompt => ExecPolicyOutcome::Prompt {
|
||||
run_with_escalated_permissions: true,
|
||||
},
|
||||
Decision::Allow => ExecPolicyOutcome::Allow {
|
||||
run_with_escalated_permissions: true,
|
||||
},
|
||||
},
|
||||
Evaluation::NoMatch { .. } => {
|
||||
if command_might_be_dangerous(&command) {
|
||||
ExecPolicyOutcome::Prompt {
|
||||
run_with_escalated_permissions: false,
|
||||
}
|
||||
} else {
|
||||
ExecPolicyOutcome::Allow {
|
||||
run_with_escalated_permissions: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn collect_policy_files(dir: &Path) -> Result<Vec<PathBuf>, ExecPolicyError> {
|
||||
let mut read_dir = match fs::read_dir(dir).await {
|
||||
Ok(read_dir) => read_dir,
|
||||
Err(err) if err.kind() == ErrorKind::NotFound => return Ok(Vec::new()),
|
||||
Err(source) => {
|
||||
return Err(ExecPolicyError::ReadDir {
|
||||
dir: dir.to_path_buf(),
|
||||
source,
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
let mut policy_paths = Vec::new();
|
||||
while let Some(entry) =
|
||||
read_dir
|
||||
.next_entry()
|
||||
.await
|
||||
.map_err(|source| ExecPolicyError::ReadDir {
|
||||
dir: dir.to_path_buf(),
|
||||
source,
|
||||
})?
|
||||
{
|
||||
let path = entry.path();
|
||||
let file_type = entry
|
||||
.file_type()
|
||||
.await
|
||||
.map_err(|source| ExecPolicyError::ReadDir {
|
||||
dir: dir.to_path_buf(),
|
||||
source,
|
||||
})?;
|
||||
|
||||
if path
|
||||
.extension()
|
||||
.and_then(|ext| ext.to_str())
|
||||
.is_some_and(|ext| ext == POLICY_EXTENSION)
|
||||
&& file_type.is_file()
|
||||
{
|
||||
policy_paths.push(path);
|
||||
}
|
||||
}
|
||||
|
||||
policy_paths.sort();
|
||||
Ok(policy_paths)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use tempfile::tempdir;
|
||||
|
||||
struct EnvVarGuard {
|
||||
key: &'static str,
|
||||
original: Option<std::ffi::OsString>,
|
||||
}
|
||||
|
||||
impl EnvVarGuard {
|
||||
fn set(key: &'static str, value: &std::ffi::OsStr) -> Self {
|
||||
let original = std::env::var_os(key);
|
||||
unsafe {
|
||||
std::env::set_var(key, value);
|
||||
}
|
||||
Self { key, original }
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for EnvVarGuard {
|
||||
fn drop(&mut self) {
|
||||
unsafe {
|
||||
match &self.original {
|
||||
Some(value) => std::env::set_var(self.key, value),
|
||||
None => std::env::remove_var(self.key),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn allow_policy_bypasses_sandbox() {
|
||||
let mut parser = PolicyParser::new();
|
||||
parser
|
||||
.parse(
|
||||
"test.codexpolicy",
|
||||
r#"prefix_rule(pattern=["echo"], decision="allow")"#,
|
||||
)
|
||||
.expect("parse policy");
|
||||
let evaluator = ExecPolicyEvaluator::new(Arc::new(parser.build()));
|
||||
|
||||
let outcome = evaluator.evaluate(
|
||||
Path::new("/bin/echo"),
|
||||
&["echo".to_string()],
|
||||
Path::new("/"),
|
||||
);
|
||||
assert!(matches!(
|
||||
outcome,
|
||||
ExecPolicyOutcome::Allow {
|
||||
run_with_escalated_permissions: true
|
||||
}
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_match_dangerous_command_prompts() {
|
||||
let evaluator = ExecPolicyEvaluator::new(Arc::new(Policy::empty()));
|
||||
let outcome = evaluator.evaluate(
|
||||
Path::new("/bin/rm"),
|
||||
&["rm".to_string(), "-rf".to_string(), "/".to_string()],
|
||||
Path::new("/"),
|
||||
);
|
||||
assert!(matches!(
|
||||
outcome,
|
||||
ExecPolicyOutcome::Prompt {
|
||||
run_with_escalated_permissions: false
|
||||
}
|
||||
));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn missing_policy_dir_loads_empty() {
|
||||
let dir = tempdir().expect("tempdir");
|
||||
let _guard = EnvVarGuard::set("CODEX_HOME", dir.path().as_os_str());
|
||||
let policy = load_policy_from_codex_home().await.expect("load policy");
|
||||
assert!(matches!(
|
||||
policy.check_multiple(std::iter::once(&vec!["rm".to_string()])),
|
||||
Evaluation::NoMatch { .. }
|
||||
));
|
||||
}
|
||||
}
|
||||
@@ -27,8 +27,8 @@ use tracing::debug;
|
||||
|
||||
use crate::posix::escalate_server::EscalateServer;
|
||||
use crate::posix::escalate_server::{self};
|
||||
use crate::posix::mcp_escalation_policy::ExecPolicy;
|
||||
use crate::posix::mcp_escalation_policy::McpEscalationPolicy;
|
||||
use crate::posix::mcp_escalation_policy::SharedExecPolicy;
|
||||
use crate::posix::stopwatch::Stopwatch;
|
||||
|
||||
/// Path to our patched bash.
|
||||
@@ -78,13 +78,13 @@ pub struct ExecTool {
|
||||
tool_router: ToolRouter<ExecTool>,
|
||||
bash_path: PathBuf,
|
||||
execve_wrapper: PathBuf,
|
||||
policy: SharedExecPolicy,
|
||||
policy: ExecPolicy,
|
||||
sandbox_state: Arc<RwLock<Option<SandboxState>>>,
|
||||
}
|
||||
|
||||
#[tool_router]
|
||||
impl ExecTool {
|
||||
pub fn new(bash_path: PathBuf, execve_wrapper: PathBuf, policy: SharedExecPolicy) -> Self {
|
||||
pub fn new(bash_path: PathBuf, execve_wrapper: PathBuf, policy: ExecPolicy) -> Self {
|
||||
Self {
|
||||
tool_router: Self::tool_router(),
|
||||
bash_path,
|
||||
@@ -121,7 +121,7 @@ impl ExecTool {
|
||||
let escalate_server = EscalateServer::new(
|
||||
self.bash_path.clone(),
|
||||
self.execve_wrapper.clone(),
|
||||
McpEscalationPolicy::new(self.policy.clone(), context, stopwatch.clone()),
|
||||
McpEscalationPolicy::new(self.policy, context, stopwatch.clone()),
|
||||
);
|
||||
|
||||
let result = escalate_server
|
||||
@@ -198,7 +198,7 @@ impl ServerHandler for ExecTool {
|
||||
pub(crate) async fn serve(
|
||||
bash_path: PathBuf,
|
||||
execve_wrapper: PathBuf,
|
||||
policy: SharedExecPolicy,
|
||||
policy: ExecPolicy,
|
||||
) -> Result<RunningService<RoleServer, ExecTool>, rmcp::service::ServerInitializeError> {
|
||||
let tool = ExecTool::new(bash_path, execve_wrapper, policy);
|
||||
tool.serve(stdio()).await
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
|
||||
use rmcp::ErrorData as McpError;
|
||||
use rmcp::RoleServer;
|
||||
@@ -19,11 +18,7 @@ use crate::posix::stopwatch::Stopwatch;
|
||||
/// `argv` is the argv, including the program name (`argv[0]`).
|
||||
/// `workdir` is the absolute, canonical path to the working directory in which to execute the
|
||||
/// command.
|
||||
pub(crate) trait ExecPolicy: Send + Sync {
|
||||
fn evaluate(&self, file: &Path, argv: &[String], workdir: &Path) -> ExecPolicyOutcome;
|
||||
}
|
||||
|
||||
pub(crate) type SharedExecPolicy = Arc<dyn ExecPolicy>;
|
||||
pub(crate) type ExecPolicy = fn(file: &Path, argv: &[String], workdir: &Path) -> ExecPolicyOutcome;
|
||||
|
||||
pub(crate) enum ExecPolicyOutcome {
|
||||
Allow {
|
||||
@@ -38,14 +33,14 @@ pub(crate) enum ExecPolicyOutcome {
|
||||
/// ExecPolicy with access to the MCP RequestContext so that it can leverage
|
||||
/// elicitations.
|
||||
pub(crate) struct McpEscalationPolicy {
|
||||
policy: SharedExecPolicy,
|
||||
policy: ExecPolicy,
|
||||
context: RequestContext<RoleServer>,
|
||||
stopwatch: Stopwatch,
|
||||
}
|
||||
|
||||
impl McpEscalationPolicy {
|
||||
pub(crate) fn new(
|
||||
policy: SharedExecPolicy,
|
||||
policy: ExecPolicy,
|
||||
context: RequestContext<RoleServer>,
|
||||
stopwatch: Stopwatch,
|
||||
) -> Self {
|
||||
@@ -108,7 +103,7 @@ impl EscalationPolicy for McpEscalationPolicy {
|
||||
argv: &[String],
|
||||
workdir: &Path,
|
||||
) -> Result<EscalateAction, rmcp::ErrorData> {
|
||||
let outcome = self.policy.evaluate(file, argv, workdir);
|
||||
let outcome = (self.policy)(file, argv, workdir);
|
||||
let action = match outcome {
|
||||
ExecPolicyOutcome::Allow {
|
||||
run_with_escalated_permissions,
|
||||
|
||||
@@ -91,6 +91,9 @@ pub struct Cli {
|
||||
pub enum Command {
|
||||
/// Resume a previous session by id or pick the most recent with --last.
|
||||
Resume(ResumeArgs),
|
||||
|
||||
/// Run a code review against the current repository.
|
||||
Review(ReviewArgs),
|
||||
}
|
||||
|
||||
#[derive(Parser, Debug)]
|
||||
@@ -109,6 +112,41 @@ pub struct ResumeArgs {
|
||||
pub prompt: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Parser, Debug)]
|
||||
pub struct ReviewArgs {
|
||||
/// Review staged, unstaged, and untracked changes.
|
||||
#[arg(
|
||||
long = "uncommitted",
|
||||
default_value_t = false,
|
||||
conflicts_with_all = ["base", "commit", "prompt"]
|
||||
)]
|
||||
pub uncommitted: bool,
|
||||
|
||||
/// Review changes against the given base branch.
|
||||
#[arg(
|
||||
long = "base",
|
||||
value_name = "BRANCH",
|
||||
conflicts_with_all = ["uncommitted", "commit", "prompt"]
|
||||
)]
|
||||
pub base: Option<String>,
|
||||
|
||||
/// Review the changes introduced by a commit.
|
||||
#[arg(
|
||||
long = "commit",
|
||||
value_name = "SHA",
|
||||
conflicts_with_all = ["uncommitted", "base", "prompt"]
|
||||
)]
|
||||
pub commit: Option<String>,
|
||||
|
||||
/// Optional commit title to display in the review summary.
|
||||
#[arg(long = "title", value_name = "TITLE", requires = "commit")]
|
||||
pub commit_title: Option<String>,
|
||||
|
||||
/// Custom review instructions. If `-` is used, read from stdin.
|
||||
#[arg(value_name = "PROMPT", value_hint = clap::ValueHint::Other)]
|
||||
pub prompt: Option<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, ValueEnum)]
|
||||
#[value(rename_all = "kebab-case")]
|
||||
pub enum Color {
|
||||
|
||||
@@ -11,6 +11,8 @@ pub mod event_processor_with_jsonl_output;
|
||||
pub mod exec_events;
|
||||
|
||||
pub use cli::Cli;
|
||||
pub use cli::Command;
|
||||
pub use cli::ReviewArgs;
|
||||
use codex_common::oss::ensure_oss_provider_ready;
|
||||
use codex_common::oss::get_default_model_for_oss_provider;
|
||||
use codex_core::AuthManager;
|
||||
@@ -29,6 +31,8 @@ use codex_core::protocol::AskForApproval;
|
||||
use codex_core::protocol::Event;
|
||||
use codex_core::protocol::EventMsg;
|
||||
use codex_core::protocol::Op;
|
||||
use codex_core::protocol::ReviewRequest;
|
||||
use codex_core::protocol::ReviewTarget;
|
||||
use codex_core::protocol::SessionSource;
|
||||
use codex_protocol::approvals::ElicitationAction;
|
||||
use codex_protocol::config_types::SandboxMode;
|
||||
@@ -53,6 +57,16 @@ use crate::event_processor::EventProcessor;
|
||||
use codex_core::default_client::set_default_originator;
|
||||
use codex_core::find_conversation_path_by_id_str;
|
||||
|
||||
enum InitialOperation {
|
||||
UserTurn {
|
||||
items: Vec<UserInput>,
|
||||
output_schema: Option<Value>,
|
||||
},
|
||||
Review {
|
||||
review_request: ReviewRequest,
|
||||
},
|
||||
}
|
||||
|
||||
pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option<PathBuf>) -> anyhow::Result<()> {
|
||||
if let Err(err) = set_default_originator("codex_exec".to_string()) {
|
||||
tracing::warn!(?err, "Failed to set codex exec originator override {err:?}");
|
||||
@@ -79,64 +93,6 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option<PathBuf>) -> any
|
||||
config_overrides,
|
||||
} = cli;
|
||||
|
||||
// Determine the prompt source (parent or subcommand) and read from stdin if needed.
|
||||
let prompt_arg = match &command {
|
||||
// Allow prompt before the subcommand by falling back to the parent-level prompt
|
||||
// when the Resume subcommand did not provide its own prompt.
|
||||
Some(ExecCommand::Resume(args)) => {
|
||||
let resume_prompt = args
|
||||
.prompt
|
||||
.clone()
|
||||
// When using `resume --last <PROMPT>`, clap still parses the first positional
|
||||
// as `session_id`. Reinterpret it as the prompt so the flag works with JSON mode.
|
||||
.or_else(|| {
|
||||
if args.last {
|
||||
args.session_id.clone()
|
||||
} else {
|
||||
None
|
||||
}
|
||||
});
|
||||
resume_prompt.or(prompt)
|
||||
}
|
||||
None => prompt,
|
||||
};
|
||||
|
||||
let prompt = match prompt_arg {
|
||||
Some(p) if p != "-" => p,
|
||||
// Either `-` was passed or no positional arg.
|
||||
maybe_dash => {
|
||||
// When no arg (None) **and** stdin is a TTY, bail out early – unless the
|
||||
// user explicitly forced reading via `-`.
|
||||
let force_stdin = matches!(maybe_dash.as_deref(), Some("-"));
|
||||
|
||||
if std::io::stdin().is_terminal() && !force_stdin {
|
||||
eprintln!(
|
||||
"No prompt provided. Either specify one as an argument or pipe the prompt into stdin."
|
||||
);
|
||||
std::process::exit(1);
|
||||
}
|
||||
|
||||
// Ensure the user knows we are waiting on stdin, as they may
|
||||
// have gotten into this state by mistake. If so, and they are not
|
||||
// writing to stdin, Codex will hang indefinitely, so this should
|
||||
// help them debug in that case.
|
||||
if !force_stdin {
|
||||
eprintln!("Reading prompt from stdin...");
|
||||
}
|
||||
let mut buffer = String::new();
|
||||
if let Err(e) = std::io::stdin().read_to_string(&mut buffer) {
|
||||
eprintln!("Failed to read prompt from stdin: {e}");
|
||||
std::process::exit(1);
|
||||
} else if buffer.trim().is_empty() {
|
||||
eprintln!("No prompt provided via stdin.");
|
||||
std::process::exit(1);
|
||||
}
|
||||
buffer
|
||||
}
|
||||
};
|
||||
|
||||
let output_schema = load_output_schema(output_schema_path);
|
||||
|
||||
let (stdout_with_ansi, stderr_with_ansi) = match color {
|
||||
cli::Color::Always => (true, true),
|
||||
cli::Color::Never => (false, false),
|
||||
@@ -329,8 +285,8 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option<PathBuf>) -> any
|
||||
conversation_id: _,
|
||||
conversation,
|
||||
session_configured,
|
||||
} = if let Some(ExecCommand::Resume(args)) = command {
|
||||
let resume_path = resolve_resume_path(&config, &args).await?;
|
||||
} = if let Some(ExecCommand::Resume(args)) = command.as_ref() {
|
||||
let resume_path = resolve_resume_path(&config, args).await?;
|
||||
|
||||
if let Some(path) = resume_path {
|
||||
conversation_manager
|
||||
@@ -346,9 +302,64 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option<PathBuf>) -> any
|
||||
.new_conversation(config.clone())
|
||||
.await?
|
||||
};
|
||||
// Print the effective configuration and prompt so users can see what Codex
|
||||
let (initial_operation, prompt_summary) = match (command, prompt, images) {
|
||||
(Some(ExecCommand::Review(review_cli)), _, _) => {
|
||||
let review_request = build_review_request(review_cli)?;
|
||||
let summary = codex_core::review_prompts::user_facing_hint(&review_request.target);
|
||||
(InitialOperation::Review { review_request }, summary)
|
||||
}
|
||||
(Some(ExecCommand::Resume(args)), root_prompt, imgs) => {
|
||||
let prompt_arg = args
|
||||
.prompt
|
||||
.clone()
|
||||
.or_else(|| {
|
||||
if args.last {
|
||||
args.session_id.clone()
|
||||
} else {
|
||||
None
|
||||
}
|
||||
})
|
||||
.or(root_prompt);
|
||||
let prompt_text = resolve_prompt(prompt_arg);
|
||||
let mut items: Vec<UserInput> = imgs
|
||||
.into_iter()
|
||||
.map(|path| UserInput::LocalImage { path })
|
||||
.collect();
|
||||
items.push(UserInput::Text {
|
||||
text: prompt_text.clone(),
|
||||
});
|
||||
let output_schema = load_output_schema(output_schema_path.clone());
|
||||
(
|
||||
InitialOperation::UserTurn {
|
||||
items,
|
||||
output_schema,
|
||||
},
|
||||
prompt_text,
|
||||
)
|
||||
}
|
||||
(None, root_prompt, imgs) => {
|
||||
let prompt_text = resolve_prompt(root_prompt);
|
||||
let mut items: Vec<UserInput> = imgs
|
||||
.into_iter()
|
||||
.map(|path| UserInput::LocalImage { path })
|
||||
.collect();
|
||||
items.push(UserInput::Text {
|
||||
text: prompt_text.clone(),
|
||||
});
|
||||
let output_schema = load_output_schema(output_schema_path);
|
||||
(
|
||||
InitialOperation::UserTurn {
|
||||
items,
|
||||
output_schema,
|
||||
},
|
||||
prompt_text,
|
||||
)
|
||||
}
|
||||
};
|
||||
|
||||
// Print the effective configuration and initial request so users can see what Codex
|
||||
// is using.
|
||||
event_processor.print_config_summary(&config, &prompt, &session_configured);
|
||||
event_processor.print_config_summary(&config, &prompt_summary, &session_configured);
|
||||
|
||||
info!("Codex initialized with event: {session_configured:?}");
|
||||
|
||||
@@ -391,25 +402,32 @@ pub async fn run_main(cli: Cli, codex_linux_sandbox_exe: Option<PathBuf>) -> any
|
||||
});
|
||||
}
|
||||
|
||||
// Package images and prompt into a single user input turn.
|
||||
let mut items: Vec<UserInput> = images
|
||||
.into_iter()
|
||||
.map(|path| UserInput::LocalImage { path })
|
||||
.collect();
|
||||
items.push(UserInput::Text { text: prompt });
|
||||
let initial_prompt_task_id = conversation
|
||||
.submit(Op::UserTurn {
|
||||
match initial_operation {
|
||||
InitialOperation::UserTurn {
|
||||
items,
|
||||
cwd: default_cwd,
|
||||
approval_policy: default_approval_policy,
|
||||
sandbox_policy: default_sandbox_policy,
|
||||
model: default_model,
|
||||
effort: default_effort,
|
||||
summary: default_summary,
|
||||
final_output_json_schema: output_schema,
|
||||
})
|
||||
.await?;
|
||||
info!("Sent prompt with event ID: {initial_prompt_task_id}");
|
||||
output_schema,
|
||||
} => {
|
||||
let task_id = conversation
|
||||
.submit(Op::UserTurn {
|
||||
items,
|
||||
cwd: default_cwd,
|
||||
approval_policy: default_approval_policy,
|
||||
sandbox_policy: default_sandbox_policy,
|
||||
model: default_model,
|
||||
effort: default_effort,
|
||||
summary: default_summary,
|
||||
final_output_json_schema: output_schema,
|
||||
})
|
||||
.await?;
|
||||
info!("Sent prompt with event ID: {task_id}");
|
||||
task_id
|
||||
}
|
||||
InitialOperation::Review { review_request } => {
|
||||
let task_id = conversation.submit(Op::Review { review_request }).await?;
|
||||
info!("Sent review request with event ID: {task_id}");
|
||||
task_id
|
||||
}
|
||||
};
|
||||
|
||||
// Run the loop until the task is complete.
|
||||
// Track whether a fatal error was reported by the server so we can
|
||||
@@ -503,3 +521,130 @@ fn load_output_schema(path: Option<PathBuf>) -> Option<Value> {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn resolve_prompt(prompt_arg: Option<String>) -> String {
|
||||
match prompt_arg {
|
||||
Some(p) if p != "-" => p,
|
||||
maybe_dash => {
|
||||
let force_stdin = matches!(maybe_dash.as_deref(), Some("-"));
|
||||
|
||||
if std::io::stdin().is_terminal() && !force_stdin {
|
||||
eprintln!(
|
||||
"No prompt provided. Either specify one as an argument or pipe the prompt into stdin."
|
||||
);
|
||||
std::process::exit(1);
|
||||
}
|
||||
|
||||
if !force_stdin {
|
||||
eprintln!("Reading prompt from stdin...");
|
||||
}
|
||||
let mut buffer = String::new();
|
||||
if let Err(e) = std::io::stdin().read_to_string(&mut buffer) {
|
||||
eprintln!("Failed to read prompt from stdin: {e}");
|
||||
std::process::exit(1);
|
||||
} else if buffer.trim().is_empty() {
|
||||
eprintln!("No prompt provided via stdin.");
|
||||
std::process::exit(1);
|
||||
}
|
||||
buffer
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn build_review_request(args: ReviewArgs) -> anyhow::Result<ReviewRequest> {
|
||||
let target = if args.uncommitted {
|
||||
ReviewTarget::UncommittedChanges
|
||||
} else if let Some(branch) = args.base {
|
||||
ReviewTarget::BaseBranch { branch }
|
||||
} else if let Some(sha) = args.commit {
|
||||
ReviewTarget::Commit {
|
||||
sha,
|
||||
title: args.commit_title,
|
||||
}
|
||||
} else if let Some(prompt_arg) = args.prompt {
|
||||
let prompt = resolve_prompt(Some(prompt_arg)).trim().to_string();
|
||||
if prompt.is_empty() {
|
||||
anyhow::bail!("Review prompt cannot be empty");
|
||||
}
|
||||
ReviewTarget::Custom {
|
||||
instructions: prompt,
|
||||
}
|
||||
} else {
|
||||
anyhow::bail!(
|
||||
"Specify --uncommitted, --base, --commit, or provide custom review instructions"
|
||||
);
|
||||
};
|
||||
|
||||
Ok(ReviewRequest {
|
||||
target,
|
||||
user_facing_hint: None,
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
#[test]
|
||||
fn builds_uncommitted_review_request() {
|
||||
let request = build_review_request(ReviewArgs {
|
||||
uncommitted: true,
|
||||
base: None,
|
||||
commit: None,
|
||||
commit_title: None,
|
||||
prompt: None,
|
||||
})
|
||||
.expect("builds uncommitted review request");
|
||||
|
||||
let expected = ReviewRequest {
|
||||
target: ReviewTarget::UncommittedChanges,
|
||||
user_facing_hint: None,
|
||||
};
|
||||
|
||||
assert_eq!(request, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn builds_commit_review_request_with_title() {
|
||||
let request = build_review_request(ReviewArgs {
|
||||
uncommitted: false,
|
||||
base: None,
|
||||
commit: Some("123456789".to_string()),
|
||||
commit_title: Some("Add review command".to_string()),
|
||||
prompt: None,
|
||||
})
|
||||
.expect("builds commit review request");
|
||||
|
||||
let expected = ReviewRequest {
|
||||
target: ReviewTarget::Commit {
|
||||
sha: "123456789".to_string(),
|
||||
title: Some("Add review command".to_string()),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
};
|
||||
|
||||
assert_eq!(request, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn builds_custom_review_request_trims_prompt() {
|
||||
let request = build_review_request(ReviewArgs {
|
||||
uncommitted: false,
|
||||
base: None,
|
||||
commit: None,
|
||||
commit_title: None,
|
||||
prompt: Some(" custom review instructions ".to_string()),
|
||||
})
|
||||
.expect("builds custom review request");
|
||||
|
||||
let expected = ReviewRequest {
|
||||
target: ReviewTarget::Custom {
|
||||
instructions: "custom review instructions".to_string(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
};
|
||||
|
||||
assert_eq!(request, expected);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -28,3 +28,4 @@ thiserror = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
pretty_assertions = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
|
||||
226
codex-rs/execpolicy/src/amend.rs
Normal file
226
codex-rs/execpolicy/src/amend.rs
Normal file
@@ -0,0 +1,226 @@
|
||||
use std::fs::OpenOptions;
|
||||
use std::io::Read;
|
||||
use std::io::Seek;
|
||||
use std::io::SeekFrom;
|
||||
use std::io::Write;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use serde_json;
|
||||
use thiserror::Error;
|
||||
|
||||
#[derive(Debug, Error)]
|
||||
pub enum AmendError {
|
||||
#[error("prefix rule requires at least one token")]
|
||||
EmptyPrefix,
|
||||
#[error("policy path has no parent: {path}")]
|
||||
MissingParent { path: PathBuf },
|
||||
#[error("failed to create policy directory {dir}: {source}")]
|
||||
CreatePolicyDir {
|
||||
dir: PathBuf,
|
||||
source: std::io::Error,
|
||||
},
|
||||
#[error("failed to format prefix tokens: {source}")]
|
||||
SerializePrefix { source: serde_json::Error },
|
||||
#[error("failed to open policy file {path}: {source}")]
|
||||
OpenPolicyFile {
|
||||
path: PathBuf,
|
||||
source: std::io::Error,
|
||||
},
|
||||
#[error("failed to write to policy file {path}: {source}")]
|
||||
WritePolicyFile {
|
||||
path: PathBuf,
|
||||
source: std::io::Error,
|
||||
},
|
||||
#[error("failed to lock policy file {path}: {source}")]
|
||||
LockPolicyFile {
|
||||
path: PathBuf,
|
||||
source: std::io::Error,
|
||||
},
|
||||
#[error("failed to seek policy file {path}: {source}")]
|
||||
SeekPolicyFile {
|
||||
path: PathBuf,
|
||||
source: std::io::Error,
|
||||
},
|
||||
#[error("failed to read policy file {path}: {source}")]
|
||||
ReadPolicyFile {
|
||||
path: PathBuf,
|
||||
source: std::io::Error,
|
||||
},
|
||||
#[error("failed to read metadata for policy file {path}: {source}")]
|
||||
PolicyMetadata {
|
||||
path: PathBuf,
|
||||
source: std::io::Error,
|
||||
},
|
||||
}
|
||||
|
||||
/// Note this thread uses advisory file locking and performs blocking I/O, so it should be used with
|
||||
/// [`tokio::task::spawn_blocking`] when called from an async context.
|
||||
pub fn blocking_append_allow_prefix_rule(
|
||||
policy_path: &Path,
|
||||
prefix: &[String],
|
||||
) -> Result<(), AmendError> {
|
||||
if prefix.is_empty() {
|
||||
return Err(AmendError::EmptyPrefix);
|
||||
}
|
||||
|
||||
let tokens = prefix
|
||||
.iter()
|
||||
.map(serde_json::to_string)
|
||||
.collect::<Result<Vec<_>, _>>()
|
||||
.map_err(|source| AmendError::SerializePrefix { source })?;
|
||||
let pattern = format!("[{}]", tokens.join(", "));
|
||||
let rule = format!(r#"prefix_rule(pattern={pattern}, decision="allow")"#);
|
||||
|
||||
let dir = policy_path
|
||||
.parent()
|
||||
.ok_or_else(|| AmendError::MissingParent {
|
||||
path: policy_path.to_path_buf(),
|
||||
})?;
|
||||
match std::fs::create_dir(dir) {
|
||||
Ok(()) => {}
|
||||
Err(ref source) if source.kind() == std::io::ErrorKind::AlreadyExists => {}
|
||||
Err(source) => {
|
||||
return Err(AmendError::CreatePolicyDir {
|
||||
dir: dir.to_path_buf(),
|
||||
source,
|
||||
});
|
||||
}
|
||||
}
|
||||
append_locked_line(policy_path, &rule)
|
||||
}
|
||||
|
||||
fn append_locked_line(policy_path: &Path, line: &str) -> Result<(), AmendError> {
|
||||
let mut file = OpenOptions::new()
|
||||
.create(true)
|
||||
.read(true)
|
||||
.append(true)
|
||||
.open(policy_path)
|
||||
.map_err(|source| AmendError::OpenPolicyFile {
|
||||
path: policy_path.to_path_buf(),
|
||||
source,
|
||||
})?;
|
||||
file.lock().map_err(|source| AmendError::LockPolicyFile {
|
||||
path: policy_path.to_path_buf(),
|
||||
source,
|
||||
})?;
|
||||
|
||||
let len = file
|
||||
.metadata()
|
||||
.map_err(|source| AmendError::PolicyMetadata {
|
||||
path: policy_path.to_path_buf(),
|
||||
source,
|
||||
})?
|
||||
.len();
|
||||
|
||||
// Ensure file ends in a newline before appending.
|
||||
if len > 0 {
|
||||
file.seek(SeekFrom::End(-1))
|
||||
.map_err(|source| AmendError::SeekPolicyFile {
|
||||
path: policy_path.to_path_buf(),
|
||||
source,
|
||||
})?;
|
||||
let mut last = [0; 1];
|
||||
file.read_exact(&mut last)
|
||||
.map_err(|source| AmendError::ReadPolicyFile {
|
||||
path: policy_path.to_path_buf(),
|
||||
source,
|
||||
})?;
|
||||
|
||||
if last[0] != b'\n' {
|
||||
file.write_all(b"\n")
|
||||
.map_err(|source| AmendError::WritePolicyFile {
|
||||
path: policy_path.to_path_buf(),
|
||||
source,
|
||||
})?;
|
||||
}
|
||||
}
|
||||
|
||||
file.write_all(format!("{line}\n").as_bytes())
|
||||
.map_err(|source| AmendError::WritePolicyFile {
|
||||
path: policy_path.to_path_buf(),
|
||||
source,
|
||||
})?;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[test]
|
||||
fn appends_rule_and_creates_directories() {
|
||||
let tmp = tempdir().expect("create temp dir");
|
||||
let policy_path = tmp.path().join("policy").join("default.codexpolicy");
|
||||
|
||||
blocking_append_allow_prefix_rule(
|
||||
&policy_path,
|
||||
&[String::from("echo"), String::from("Hello, world!")],
|
||||
)
|
||||
.expect("append rule");
|
||||
|
||||
let contents =
|
||||
std::fs::read_to_string(&policy_path).expect("default.codexpolicy should exist");
|
||||
assert_eq!(
|
||||
contents,
|
||||
r#"prefix_rule(pattern=["echo", "Hello, world!"], decision="allow")
|
||||
"#
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn appends_rule_without_duplicate_newline() {
|
||||
let tmp = tempdir().expect("create temp dir");
|
||||
let policy_path = tmp.path().join("policy").join("default.codexpolicy");
|
||||
std::fs::create_dir_all(policy_path.parent().unwrap()).expect("create policy dir");
|
||||
std::fs::write(
|
||||
&policy_path,
|
||||
r#"prefix_rule(pattern=["ls"], decision="allow")
|
||||
"#,
|
||||
)
|
||||
.expect("write seed rule");
|
||||
|
||||
blocking_append_allow_prefix_rule(
|
||||
&policy_path,
|
||||
&[String::from("echo"), String::from("Hello, world!")],
|
||||
)
|
||||
.expect("append rule");
|
||||
|
||||
let contents = std::fs::read_to_string(&policy_path).expect("read policy");
|
||||
assert_eq!(
|
||||
contents,
|
||||
r#"prefix_rule(pattern=["ls"], decision="allow")
|
||||
prefix_rule(pattern=["echo", "Hello, world!"], decision="allow")
|
||||
"#
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn inserts_newline_when_missing_before_append() {
|
||||
let tmp = tempdir().expect("create temp dir");
|
||||
let policy_path = tmp.path().join("policy").join("default.codexpolicy");
|
||||
std::fs::create_dir_all(policy_path.parent().unwrap()).expect("create policy dir");
|
||||
std::fs::write(
|
||||
&policy_path,
|
||||
r#"prefix_rule(pattern=["ls"], decision="allow")"#,
|
||||
)
|
||||
.expect("write seed rule without newline");
|
||||
|
||||
blocking_append_allow_prefix_rule(
|
||||
&policy_path,
|
||||
&[String::from("echo"), String::from("Hello, world!")],
|
||||
)
|
||||
.expect("append rule");
|
||||
|
||||
let contents = std::fs::read_to_string(&policy_path).expect("read policy");
|
||||
assert_eq!(
|
||||
contents,
|
||||
r#"prefix_rule(pattern=["ls"], decision="allow")
|
||||
prefix_rule(pattern=["echo", "Hello, world!"], decision="allow")
|
||||
"#
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -1,3 +1,4 @@
|
||||
pub mod amend;
|
||||
pub mod decision;
|
||||
pub mod error;
|
||||
pub mod execpolicycheck;
|
||||
@@ -5,6 +6,8 @@ pub mod parser;
|
||||
pub mod policy;
|
||||
pub mod rule;
|
||||
|
||||
pub use amend::AmendError;
|
||||
pub use amend::blocking_append_allow_prefix_rule;
|
||||
pub use decision::Decision;
|
||||
pub use error::Error;
|
||||
pub use error::Result;
|
||||
|
||||
@@ -1,9 +1,15 @@
|
||||
use crate::decision::Decision;
|
||||
use crate::error::Error;
|
||||
use crate::error::Result;
|
||||
use crate::rule::PatternToken;
|
||||
use crate::rule::PrefixPattern;
|
||||
use crate::rule::PrefixRule;
|
||||
use crate::rule::RuleMatch;
|
||||
use crate::rule::RuleRef;
|
||||
use multimap::MultiMap;
|
||||
use serde::Deserialize;
|
||||
use serde::Serialize;
|
||||
use std::sync::Arc;
|
||||
|
||||
#[derive(Clone, Debug)]
|
||||
pub struct Policy {
|
||||
@@ -23,6 +29,27 @@ impl Policy {
|
||||
&self.rules_by_program
|
||||
}
|
||||
|
||||
pub fn add_prefix_rule(&mut self, prefix: &[String], decision: Decision) -> Result<()> {
|
||||
let (first_token, rest) = prefix
|
||||
.split_first()
|
||||
.ok_or_else(|| Error::InvalidPattern("prefix cannot be empty".to_string()))?;
|
||||
|
||||
let rule: RuleRef = Arc::new(PrefixRule {
|
||||
pattern: PrefixPattern {
|
||||
first: Arc::from(first_token.as_str()),
|
||||
rest: rest
|
||||
.iter()
|
||||
.map(|token| PatternToken::Single(token.clone()))
|
||||
.collect::<Vec<_>>()
|
||||
.into(),
|
||||
},
|
||||
decision,
|
||||
});
|
||||
|
||||
self.rules_by_program.insert(first_token.clone(), rule);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub fn check(&self, cmd: &[String]) -> Evaluation {
|
||||
let rules = match cmd.first() {
|
||||
Some(first) => match self.rules_by_program.get_vec(first) {
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
use std::any::Any;
|
||||
use std::sync::Arc;
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use codex_execpolicy::Decision;
|
||||
use codex_execpolicy::Error;
|
||||
use codex_execpolicy::Evaluation;
|
||||
use codex_execpolicy::Policy;
|
||||
use codex_execpolicy::PolicyParser;
|
||||
use codex_execpolicy::RuleMatch;
|
||||
use codex_execpolicy::RuleRef;
|
||||
@@ -35,16 +39,14 @@ fn rule_snapshots(rules: &[RuleRef]) -> Vec<RuleSnapshot> {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn basic_match() {
|
||||
fn basic_match() -> Result<()> {
|
||||
let policy_src = r#"
|
||||
prefix_rule(
|
||||
pattern = ["git", "status"],
|
||||
)
|
||||
"#;
|
||||
let mut parser = PolicyParser::new();
|
||||
parser
|
||||
.parse("test.codexpolicy", policy_src)
|
||||
.expect("parse policy");
|
||||
parser.parse("test.codexpolicy", policy_src)?;
|
||||
let policy = parser.build();
|
||||
let cmd = tokens(&["git", "status"]);
|
||||
let evaluation = policy.check(&cmd);
|
||||
@@ -58,10 +60,54 @@ prefix_rule(
|
||||
},
|
||||
evaluation
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_multiple_policy_files() {
|
||||
fn add_prefix_rule_extends_policy() -> Result<()> {
|
||||
let mut policy = Policy::empty();
|
||||
policy.add_prefix_rule(&tokens(&["ls", "-l"]), Decision::Prompt)?;
|
||||
|
||||
let rules = rule_snapshots(policy.rules().get_vec("ls").context("missing ls rules")?);
|
||||
assert_eq!(
|
||||
vec![RuleSnapshot::Prefix(PrefixRule {
|
||||
pattern: PrefixPattern {
|
||||
first: Arc::from("ls"),
|
||||
rest: vec![PatternToken::Single(String::from("-l"))].into(),
|
||||
},
|
||||
decision: Decision::Prompt,
|
||||
})],
|
||||
rules
|
||||
);
|
||||
|
||||
let evaluation = policy.check(&tokens(&["ls", "-l", "/tmp"]));
|
||||
assert_eq!(
|
||||
Evaluation::Match {
|
||||
decision: Decision::Prompt,
|
||||
matched_rules: vec![RuleMatch::PrefixRuleMatch {
|
||||
matched_prefix: tokens(&["ls", "-l"]),
|
||||
decision: Decision::Prompt,
|
||||
}],
|
||||
},
|
||||
evaluation
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn add_prefix_rule_rejects_empty_prefix() -> Result<()> {
|
||||
let mut policy = Policy::empty();
|
||||
let result = policy.add_prefix_rule(&[], Decision::Allow);
|
||||
|
||||
match result.unwrap_err() {
|
||||
Error::InvalidPattern(message) => assert_eq!(message, "prefix cannot be empty"),
|
||||
other => panic!("expected InvalidPattern(..), got {other:?}"),
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_multiple_policy_files() -> Result<()> {
|
||||
let first_policy = r#"
|
||||
prefix_rule(
|
||||
pattern = ["git"],
|
||||
@@ -75,15 +121,11 @@ prefix_rule(
|
||||
)
|
||||
"#;
|
||||
let mut parser = PolicyParser::new();
|
||||
parser
|
||||
.parse("first.codexpolicy", first_policy)
|
||||
.expect("parse policy");
|
||||
parser
|
||||
.parse("second.codexpolicy", second_policy)
|
||||
.expect("parse policy");
|
||||
parser.parse("first.codexpolicy", first_policy)?;
|
||||
parser.parse("second.codexpolicy", second_policy)?;
|
||||
let policy = parser.build();
|
||||
|
||||
let git_rules = rule_snapshots(policy.rules().get_vec("git").expect("git rules"));
|
||||
let git_rules = rule_snapshots(policy.rules().get_vec("git").context("missing git rules")?);
|
||||
assert_eq!(
|
||||
vec![
|
||||
RuleSnapshot::Prefix(PrefixRule {
|
||||
@@ -133,23 +175,27 @@ prefix_rule(
|
||||
},
|
||||
commit_eval
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn only_first_token_alias_expands_to_multiple_rules() {
|
||||
fn only_first_token_alias_expands_to_multiple_rules() -> Result<()> {
|
||||
let policy_src = r#"
|
||||
prefix_rule(
|
||||
pattern = [["bash", "sh"], ["-c", "-l"]],
|
||||
)
|
||||
"#;
|
||||
let mut parser = PolicyParser::new();
|
||||
parser
|
||||
.parse("test.codexpolicy", policy_src)
|
||||
.expect("parse policy");
|
||||
parser.parse("test.codexpolicy", policy_src)?;
|
||||
let policy = parser.build();
|
||||
|
||||
let bash_rules = rule_snapshots(policy.rules().get_vec("bash").expect("bash rules"));
|
||||
let sh_rules = rule_snapshots(policy.rules().get_vec("sh").expect("sh rules"));
|
||||
let bash_rules = rule_snapshots(
|
||||
policy
|
||||
.rules()
|
||||
.get_vec("bash")
|
||||
.context("missing bash rules")?,
|
||||
);
|
||||
let sh_rules = rule_snapshots(policy.rules().get_vec("sh").context("missing sh rules")?);
|
||||
assert_eq!(
|
||||
vec![RuleSnapshot::Prefix(PrefixRule {
|
||||
pattern: PrefixPattern {
|
||||
@@ -194,22 +240,21 @@ prefix_rule(
|
||||
},
|
||||
sh_eval
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tail_aliases_are_not_cartesian_expanded() {
|
||||
fn tail_aliases_are_not_cartesian_expanded() -> Result<()> {
|
||||
let policy_src = r#"
|
||||
prefix_rule(
|
||||
pattern = ["npm", ["i", "install"], ["--legacy-peer-deps", "--no-save"]],
|
||||
)
|
||||
"#;
|
||||
let mut parser = PolicyParser::new();
|
||||
parser
|
||||
.parse("test.codexpolicy", policy_src)
|
||||
.expect("parse policy");
|
||||
parser.parse("test.codexpolicy", policy_src)?;
|
||||
let policy = parser.build();
|
||||
|
||||
let rules = rule_snapshots(policy.rules().get_vec("npm").expect("npm rules"));
|
||||
let rules = rule_snapshots(policy.rules().get_vec("npm").context("missing npm rules")?);
|
||||
assert_eq!(
|
||||
vec![RuleSnapshot::Prefix(PrefixRule {
|
||||
pattern: PrefixPattern {
|
||||
@@ -251,10 +296,11 @@ prefix_rule(
|
||||
},
|
||||
npm_install
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn match_and_not_match_examples_are_enforced() {
|
||||
fn match_and_not_match_examples_are_enforced() -> Result<()> {
|
||||
let policy_src = r#"
|
||||
prefix_rule(
|
||||
pattern = ["git", "status"],
|
||||
@@ -266,9 +312,7 @@ prefix_rule(
|
||||
)
|
||||
"#;
|
||||
let mut parser = PolicyParser::new();
|
||||
parser
|
||||
.parse("test.codexpolicy", policy_src)
|
||||
.expect("parse policy");
|
||||
parser.parse("test.codexpolicy", policy_src)?;
|
||||
let policy = parser.build();
|
||||
let match_eval = policy.check(&tokens(&["git", "status"]));
|
||||
assert_eq!(
|
||||
@@ -289,10 +333,11 @@ prefix_rule(
|
||||
"status",
|
||||
]));
|
||||
assert_eq!(Evaluation::NoMatch {}, no_match_eval);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strictest_decision_wins_across_matches() {
|
||||
fn strictest_decision_wins_across_matches() -> Result<()> {
|
||||
let policy_src = r#"
|
||||
prefix_rule(
|
||||
pattern = ["git"],
|
||||
@@ -304,9 +349,7 @@ prefix_rule(
|
||||
)
|
||||
"#;
|
||||
let mut parser = PolicyParser::new();
|
||||
parser
|
||||
.parse("test.codexpolicy", policy_src)
|
||||
.expect("parse policy");
|
||||
parser.parse("test.codexpolicy", policy_src)?;
|
||||
let policy = parser.build();
|
||||
|
||||
let commit = policy.check(&tokens(&["git", "commit", "-m", "hi"]));
|
||||
@@ -326,10 +369,11 @@ prefix_rule(
|
||||
},
|
||||
commit
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strictest_decision_across_multiple_commands() {
|
||||
fn strictest_decision_across_multiple_commands() -> Result<()> {
|
||||
let policy_src = r#"
|
||||
prefix_rule(
|
||||
pattern = ["git"],
|
||||
@@ -341,9 +385,7 @@ prefix_rule(
|
||||
)
|
||||
"#;
|
||||
let mut parser = PolicyParser::new();
|
||||
parser
|
||||
.parse("test.codexpolicy", policy_src)
|
||||
.expect("parse policy");
|
||||
parser.parse("test.codexpolicy", policy_src)?;
|
||||
let policy = parser.build();
|
||||
|
||||
let commands = vec![
|
||||
@@ -372,4 +414,5 @@ prefix_rule(
|
||||
},
|
||||
evaluation
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -38,6 +38,13 @@ SERVER_NOTIFICATION_TYPE_NAMES: list[str] = []
|
||||
# order to compile without warnings.
|
||||
LARGE_ENUMS = {"ServerResult"}
|
||||
|
||||
# some types need setting a default value for `r#type`
|
||||
# ref: [#7417](https://github.com/openai/codex/pull/7417)
|
||||
default_type_values: dict[str, str] = {
|
||||
"ToolInputSchema": "object",
|
||||
"ToolOutputSchema": "object",
|
||||
}
|
||||
|
||||
|
||||
def main() -> int:
|
||||
parser = argparse.ArgumentParser(
|
||||
@@ -351,6 +358,14 @@ class StructField:
|
||||
out.append(f" pub {self.name}: {self.type_name},\n")
|
||||
|
||||
|
||||
def append_serde_attr(existing: str | None, fragment: str) -> str:
|
||||
if existing is None:
|
||||
return f"#[serde({fragment})]"
|
||||
assert existing.startswith("#[serde(") and existing.endswith(")]"), existing
|
||||
body = existing[len("#[serde(") : -2]
|
||||
return f"#[serde({body}, {fragment})]"
|
||||
|
||||
|
||||
def define_struct(
|
||||
name: str,
|
||||
properties: dict[str, Any],
|
||||
@@ -359,6 +374,14 @@ def define_struct(
|
||||
) -> list[str]:
|
||||
out: list[str] = []
|
||||
|
||||
type_default_fn: str | None = None
|
||||
if name in default_type_values:
|
||||
snake_name = to_snake_case(name) or name
|
||||
type_default_fn = f"{snake_name}_type_default_str"
|
||||
out.append(f"fn {type_default_fn}() -> String {{\n")
|
||||
out.append(f' "{default_type_values[name]}".to_string()\n')
|
||||
out.append("}\n\n")
|
||||
|
||||
fields: list[StructField] = []
|
||||
for prop_name, prop in properties.items():
|
||||
if prop_name == "_meta":
|
||||
@@ -380,6 +403,10 @@ def define_struct(
|
||||
if is_optional:
|
||||
prop_type = f"Option<{prop_type}>"
|
||||
rs_prop = rust_prop_name(prop_name, is_optional)
|
||||
|
||||
if prop_name == "type" and type_default_fn:
|
||||
rs_prop.serde = append_serde_attr(rs_prop.serde, f'default = "{type_default_fn}"')
|
||||
|
||||
if prop_type.startswith("&'static str"):
|
||||
fields.append(StructField("const", rs_prop.name, prop_type, rs_prop.serde, rs_prop.ts))
|
||||
else:
|
||||
|
||||
@@ -1474,6 +1474,10 @@ pub struct Tool {
|
||||
pub title: Option<String>,
|
||||
}
|
||||
|
||||
fn tool_output_schema_type_default_str() -> String {
|
||||
"object".to_string()
|
||||
}
|
||||
|
||||
/// An optional JSON Schema object defining the structure of the tool's output returned in
|
||||
/// the structuredContent field of a CallToolResult.
|
||||
#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, TS)]
|
||||
@@ -1484,9 +1488,14 @@ pub struct ToolOutputSchema {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub required: Option<Vec<String>>,
|
||||
#[serde(default = "tool_output_schema_type_default_str")]
|
||||
pub r#type: String, // &'static str = "object"
|
||||
}
|
||||
|
||||
fn tool_input_schema_type_default_str() -> String {
|
||||
"object".to_string()
|
||||
}
|
||||
|
||||
/// A JSON Schema object defining the expected parameters for the tool.
|
||||
#[derive(Debug, Clone, PartialEq, Deserialize, Serialize, JsonSchema, TS)]
|
||||
pub struct ToolInputSchema {
|
||||
@@ -1496,6 +1505,7 @@ pub struct ToolInputSchema {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub required: Option<Vec<String>>,
|
||||
#[serde(default = "tool_input_schema_type_default_str")]
|
||||
pub r#type: String, // &'static str = "object"
|
||||
}
|
||||
|
||||
|
||||
@@ -132,6 +132,7 @@ pub enum ResponseItem {
|
||||
GhostSnapshot {
|
||||
ghost_commit: GhostCommit,
|
||||
},
|
||||
#[serde(alias = "compaction")]
|
||||
CompactionSummary {
|
||||
encrypted_content: String,
|
||||
},
|
||||
@@ -537,6 +538,7 @@ mod tests {
|
||||
use anyhow::Result;
|
||||
use mcp_types::ImageContent;
|
||||
use mcp_types::TextContent;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
|
||||
#[test]
|
||||
@@ -650,6 +652,21 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn deserializes_compaction_alias() -> Result<()> {
|
||||
let json = r#"{"type":"compaction","encrypted_content":"abc"}"#;
|
||||
|
||||
let item: ResponseItem = serde_json::from_str(json)?;
|
||||
|
||||
assert_eq!(
|
||||
item,
|
||||
ResponseItem::CompactionSummary {
|
||||
encrypted_content: "abc".into(),
|
||||
}
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn roundtrips_web_search_call_actions() -> Result<()> {
|
||||
let cases = vec![
|
||||
|
||||
@@ -1263,11 +1263,40 @@ pub enum ReviewDelivery {
|
||||
Detached,
|
||||
}
|
||||
|
||||
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema, TS)]
|
||||
#[serde(tag = "type", rename_all = "camelCase")]
|
||||
#[ts(tag = "type")]
|
||||
pub enum ReviewTarget {
|
||||
/// Review the working tree: staged, unstaged, and untracked files.
|
||||
UncommittedChanges,
|
||||
|
||||
/// Review changes between the current branch and the given base branch.
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(rename_all = "camelCase")]
|
||||
BaseBranch { branch: String },
|
||||
|
||||
/// Review the changes introduced by a specific commit.
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(rename_all = "camelCase")]
|
||||
Commit {
|
||||
sha: String,
|
||||
/// Optional human-readable label (e.g., commit subject) for UIs.
|
||||
title: Option<String>,
|
||||
},
|
||||
|
||||
/// Arbitrary instructions provided by the user.
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(rename_all = "camelCase")]
|
||||
Custom { instructions: String },
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, JsonSchema, TS)]
|
||||
/// Review request sent to the review session.
|
||||
pub struct ReviewRequest {
|
||||
pub prompt: String,
|
||||
pub user_facing_hint: String,
|
||||
pub target: ReviewTarget,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
#[ts(optional)]
|
||||
pub user_facing_hint: Option<String>,
|
||||
}
|
||||
|
||||
/// Structured review result produced by a child review session.
|
||||
|
||||
@@ -14,6 +14,8 @@ use crate::pager_overlay::Overlay;
|
||||
use crate::render::highlight::highlight_bash_to_lines;
|
||||
use crate::render::renderable::Renderable;
|
||||
use crate::resume_picker::ResumeSelection;
|
||||
use crate::skill_error_prompt::SkillErrorPromptOutcome;
|
||||
use crate::skill_error_prompt::run_skill_error_prompt;
|
||||
use crate::tui;
|
||||
use crate::tui::TuiEvent;
|
||||
use crate::update_action::UpdateAction;
|
||||
@@ -36,6 +38,7 @@ use codex_core::protocol::Op;
|
||||
use codex_core::protocol::SessionSource;
|
||||
use codex_core::protocol::TokenUsage;
|
||||
use codex_core::protocol_config_types::ReasoningEffort as ReasoningEffortConfig;
|
||||
use codex_core::skills::load_skills;
|
||||
use codex_protocol::ConversationId;
|
||||
use color_eyre::eyre::Result;
|
||||
use color_eyre::eyre::WrapErr;
|
||||
@@ -267,6 +270,20 @@ impl App {
|
||||
SessionSource::Cli,
|
||||
));
|
||||
|
||||
let skills_outcome = load_skills(&config);
|
||||
if !skills_outcome.errors.is_empty() {
|
||||
match run_skill_error_prompt(tui, &skills_outcome.errors).await {
|
||||
SkillErrorPromptOutcome::Exit => {
|
||||
return Ok(AppExitInfo {
|
||||
token_usage: TokenUsage::default(),
|
||||
conversation_id: None,
|
||||
update_action: None,
|
||||
});
|
||||
}
|
||||
SkillErrorPromptOutcome::Continue => {}
|
||||
}
|
||||
}
|
||||
|
||||
let enhanced_keys_supported = tui.enhanced_keys_supported();
|
||||
|
||||
let mut chat_widget = match resume_selection {
|
||||
|
||||
@@ -113,6 +113,7 @@ pub(crate) struct ChatComposer {
|
||||
footer_mode: FooterMode,
|
||||
footer_hint_override: Option<Vec<(String, String)>>,
|
||||
context_window_percent: Option<i64>,
|
||||
context_window_used_tokens: Option<i64>,
|
||||
}
|
||||
|
||||
/// Popup state – at most one can be visible at any time.
|
||||
@@ -156,6 +157,7 @@ impl ChatComposer {
|
||||
footer_mode: FooterMode::ShortcutSummary,
|
||||
footer_hint_override: None,
|
||||
context_window_percent: None,
|
||||
context_window_used_tokens: None,
|
||||
};
|
||||
// Apply configuration via the setter to keep side-effects centralized.
|
||||
this.set_disable_paste_burst(disable_paste_burst);
|
||||
@@ -1387,6 +1389,7 @@ impl ChatComposer {
|
||||
use_shift_enter_hint: self.use_shift_enter_hint,
|
||||
is_task_running: self.is_task_running,
|
||||
context_window_percent: self.context_window_percent,
|
||||
context_window_used_tokens: self.context_window_used_tokens,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1517,10 +1520,13 @@ impl ChatComposer {
|
||||
self.is_task_running = running;
|
||||
}
|
||||
|
||||
pub(crate) fn set_context_window_percent(&mut self, percent: Option<i64>) {
|
||||
if self.context_window_percent != percent {
|
||||
self.context_window_percent = percent;
|
||||
pub(crate) fn set_context_window(&mut self, percent: Option<i64>, used_tokens: Option<i64>) {
|
||||
if self.context_window_percent == percent && self.context_window_used_tokens == used_tokens
|
||||
{
|
||||
return;
|
||||
}
|
||||
self.context_window_percent = percent;
|
||||
self.context_window_used_tokens = used_tokens;
|
||||
}
|
||||
|
||||
pub(crate) fn set_esc_backtrack_hint(&mut self, show: bool) {
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use crate::key_hint;
|
||||
use crate::key_hint::KeyBinding;
|
||||
use crate::render::line_utils::prefix_lines;
|
||||
use crate::status::format_tokens_compact;
|
||||
use crate::ui_consts::FOOTER_INDENT_COLS;
|
||||
use crossterm::event::KeyCode;
|
||||
use ratatui::buffer::Buffer;
|
||||
@@ -18,6 +19,7 @@ pub(crate) struct FooterProps {
|
||||
pub(crate) use_shift_enter_hint: bool,
|
||||
pub(crate) is_task_running: bool,
|
||||
pub(crate) context_window_percent: Option<i64>,
|
||||
pub(crate) context_window_used_tokens: Option<i64>,
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
@@ -81,7 +83,10 @@ fn footer_lines(props: FooterProps) -> Vec<Line<'static>> {
|
||||
is_task_running: props.is_task_running,
|
||||
})],
|
||||
FooterMode::ShortcutSummary => {
|
||||
let mut line = context_window_line(props.context_window_percent);
|
||||
let mut line = context_window_line(
|
||||
props.context_window_percent,
|
||||
props.context_window_used_tokens,
|
||||
);
|
||||
line.push_span(" · ".dim());
|
||||
line.extend(vec![
|
||||
key_hint::plain(KeyCode::Char('?')).into(),
|
||||
@@ -94,7 +99,10 @@ fn footer_lines(props: FooterProps) -> Vec<Line<'static>> {
|
||||
esc_backtrack_hint: props.esc_backtrack_hint,
|
||||
}),
|
||||
FooterMode::EscHint => vec![esc_hint_line(props.esc_backtrack_hint)],
|
||||
FooterMode::ContextOnly => vec![context_window_line(props.context_window_percent)],
|
||||
FooterMode::ContextOnly => vec![context_window_line(
|
||||
props.context_window_percent,
|
||||
props.context_window_used_tokens,
|
||||
)],
|
||||
}
|
||||
}
|
||||
|
||||
@@ -221,9 +229,18 @@ fn build_columns(entries: Vec<Line<'static>>) -> Vec<Line<'static>> {
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn context_window_line(percent: Option<i64>) -> Line<'static> {
|
||||
let percent = percent.unwrap_or(100).clamp(0, 100);
|
||||
Line::from(vec![Span::from(format!("{percent}% context left")).dim()])
|
||||
fn context_window_line(percent: Option<i64>, used_tokens: Option<i64>) -> Line<'static> {
|
||||
if let Some(percent) = percent {
|
||||
let percent = percent.clamp(0, 100);
|
||||
return Line::from(vec![Span::from(format!("{percent}% context left")).dim()]);
|
||||
}
|
||||
|
||||
if let Some(tokens) = used_tokens {
|
||||
let used_fmt = format_tokens_compact(tokens);
|
||||
return Line::from(vec![Span::from(format!("{used_fmt} used")).dim()]);
|
||||
}
|
||||
|
||||
Line::from(vec![Span::from("100% context left").dim()])
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
|
||||
@@ -400,6 +417,7 @@ mod tests {
|
||||
use_shift_enter_hint: false,
|
||||
is_task_running: false,
|
||||
context_window_percent: None,
|
||||
context_window_used_tokens: None,
|
||||
},
|
||||
);
|
||||
|
||||
@@ -411,6 +429,7 @@ mod tests {
|
||||
use_shift_enter_hint: true,
|
||||
is_task_running: false,
|
||||
context_window_percent: None,
|
||||
context_window_used_tokens: None,
|
||||
},
|
||||
);
|
||||
|
||||
@@ -422,6 +441,7 @@ mod tests {
|
||||
use_shift_enter_hint: false,
|
||||
is_task_running: false,
|
||||
context_window_percent: None,
|
||||
context_window_used_tokens: None,
|
||||
},
|
||||
);
|
||||
|
||||
@@ -433,6 +453,7 @@ mod tests {
|
||||
use_shift_enter_hint: false,
|
||||
is_task_running: true,
|
||||
context_window_percent: None,
|
||||
context_window_used_tokens: None,
|
||||
},
|
||||
);
|
||||
|
||||
@@ -444,6 +465,7 @@ mod tests {
|
||||
use_shift_enter_hint: false,
|
||||
is_task_running: false,
|
||||
context_window_percent: None,
|
||||
context_window_used_tokens: None,
|
||||
},
|
||||
);
|
||||
|
||||
@@ -455,6 +477,7 @@ mod tests {
|
||||
use_shift_enter_hint: false,
|
||||
is_task_running: false,
|
||||
context_window_percent: None,
|
||||
context_window_used_tokens: None,
|
||||
},
|
||||
);
|
||||
|
||||
@@ -466,6 +489,19 @@ mod tests {
|
||||
use_shift_enter_hint: false,
|
||||
is_task_running: true,
|
||||
context_window_percent: Some(72),
|
||||
context_window_used_tokens: None,
|
||||
},
|
||||
);
|
||||
|
||||
snapshot_footer(
|
||||
"footer_context_tokens_used",
|
||||
FooterProps {
|
||||
mode: FooterMode::ShortcutSummary,
|
||||
esc_backtrack_hint: false,
|
||||
use_shift_enter_hint: false,
|
||||
is_task_running: false,
|
||||
context_window_percent: None,
|
||||
context_window_used_tokens: Some(123_456),
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
@@ -76,6 +76,7 @@ pub(crate) struct BottomPane {
|
||||
/// Queued user messages to show above the composer while a turn is running.
|
||||
queued_user_messages: QueuedUserMessages,
|
||||
context_window_percent: Option<i64>,
|
||||
context_window_used_tokens: Option<i64>,
|
||||
}
|
||||
|
||||
pub(crate) struct BottomPaneParams {
|
||||
@@ -118,6 +119,7 @@ impl BottomPane {
|
||||
esc_backtrack_hint: false,
|
||||
animations_enabled,
|
||||
context_window_percent: None,
|
||||
context_window_used_tokens: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -130,6 +132,11 @@ impl BottomPane {
|
||||
self.context_window_percent
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn context_window_used_tokens(&self) -> Option<i64> {
|
||||
self.context_window_used_tokens
|
||||
}
|
||||
|
||||
fn active_view(&self) -> Option<&dyn BottomPaneView> {
|
||||
self.view_stack.last().map(std::convert::AsRef::as_ref)
|
||||
}
|
||||
@@ -344,13 +351,16 @@ impl BottomPane {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn set_context_window_percent(&mut self, percent: Option<i64>) {
|
||||
if self.context_window_percent == percent {
|
||||
pub(crate) fn set_context_window(&mut self, percent: Option<i64>, used_tokens: Option<i64>) {
|
||||
if self.context_window_percent == percent && self.context_window_used_tokens == used_tokens
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
self.context_window_percent = percent;
|
||||
self.composer.set_context_window_percent(percent);
|
||||
self.context_window_used_tokens = used_tokens;
|
||||
self.composer
|
||||
.set_context_window(percent, self.context_window_used_tokens);
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
---
|
||||
source: tui/src/bottom_pane/footer.rs
|
||||
expression: terminal.backend()
|
||||
---
|
||||
" 123K used · ? for shortcuts "
|
||||
@@ -20,6 +20,7 @@ use codex_core::protocol::AgentReasoningRawContentDeltaEvent;
|
||||
use codex_core::protocol::AgentReasoningRawContentEvent;
|
||||
use codex_core::protocol::ApplyPatchApprovalRequestEvent;
|
||||
use codex_core::protocol::BackgroundEventEvent;
|
||||
use codex_core::protocol::CreditsSnapshot;
|
||||
use codex_core::protocol::DeprecationNoticeEvent;
|
||||
use codex_core::protocol::ErrorEvent;
|
||||
use codex_core::protocol::Event;
|
||||
@@ -40,6 +41,7 @@ use codex_core::protocol::Op;
|
||||
use codex_core::protocol::PatchApplyBeginEvent;
|
||||
use codex_core::protocol::RateLimitSnapshot;
|
||||
use codex_core::protocol::ReviewRequest;
|
||||
use codex_core::protocol::ReviewTarget;
|
||||
use codex_core::protocol::StreamErrorEvent;
|
||||
use codex_core::protocol::TaskCompleteEvent;
|
||||
use codex_core::protocol::TokenUsage;
|
||||
@@ -514,7 +516,7 @@ impl ChatWidget {
|
||||
match info {
|
||||
Some(info) => self.apply_token_info(info),
|
||||
None => {
|
||||
self.bottom_pane.set_context_window_percent(None);
|
||||
self.bottom_pane.set_context_window(None, None);
|
||||
self.token_info = None;
|
||||
}
|
||||
}
|
||||
@@ -522,7 +524,8 @@ impl ChatWidget {
|
||||
|
||||
fn apply_token_info(&mut self, info: TokenUsageInfo) {
|
||||
let percent = self.context_remaining_percent(&info);
|
||||
self.bottom_pane.set_context_window_percent(percent);
|
||||
let used_tokens = self.context_used_tokens(&info, percent.is_some());
|
||||
self.bottom_pane.set_context_window(percent, used_tokens);
|
||||
self.token_info = Some(info);
|
||||
}
|
||||
|
||||
@@ -535,12 +538,20 @@ impl ChatWidget {
|
||||
})
|
||||
}
|
||||
|
||||
fn context_used_tokens(&self, info: &TokenUsageInfo, percent_known: bool) -> Option<i64> {
|
||||
if percent_known {
|
||||
return None;
|
||||
}
|
||||
|
||||
Some(info.total_token_usage.tokens_in_context_window())
|
||||
}
|
||||
|
||||
fn restore_pre_review_token_info(&mut self) {
|
||||
if let Some(saved) = self.pre_review_token_info.take() {
|
||||
match saved {
|
||||
Some(info) => self.apply_token_info(info),
|
||||
None => {
|
||||
self.bottom_pane.set_context_window_percent(None);
|
||||
self.bottom_pane.set_context_window(None, None);
|
||||
self.token_info = None;
|
||||
}
|
||||
}
|
||||
@@ -548,7 +559,19 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
pub(crate) fn on_rate_limit_snapshot(&mut self, snapshot: Option<RateLimitSnapshot>) {
|
||||
if let Some(snapshot) = snapshot {
|
||||
if let Some(mut snapshot) = snapshot {
|
||||
if snapshot.credits.is_none() {
|
||||
snapshot.credits = self
|
||||
.rate_limit_snapshot
|
||||
.as_ref()
|
||||
.and_then(|display| display.credits.as_ref())
|
||||
.map(|credits| CreditsSnapshot {
|
||||
has_credits: credits.has_credits,
|
||||
unlimited: credits.unlimited,
|
||||
balance: credits.balance.clone(),
|
||||
});
|
||||
}
|
||||
|
||||
let warnings = self.rate_limit_warnings.take_warnings(
|
||||
snapshot
|
||||
.secondary
|
||||
@@ -1812,7 +1835,10 @@ impl ChatWidget {
|
||||
self.pre_review_token_info = Some(self.token_info.clone());
|
||||
}
|
||||
self.is_review_mode = true;
|
||||
let banner = format!(">> Code review started: {} <<", review.user_facing_hint);
|
||||
let hint = review
|
||||
.user_facing_hint
|
||||
.unwrap_or_else(|| codex_core::review_prompts::user_facing_hint(&review.target));
|
||||
let banner = format!(">> Code review started: {hint} <<");
|
||||
self.add_to_history(history_cell::new_review_status_line(banner));
|
||||
self.request_redraw();
|
||||
}
|
||||
@@ -2889,16 +2915,14 @@ impl ChatWidget {
|
||||
|
||||
items.push(SelectionItem {
|
||||
name: "Review uncommitted changes".to_string(),
|
||||
actions: vec![Box::new(
|
||||
move |tx: &AppEventSender| {
|
||||
tx.send(AppEvent::CodexOp(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
prompt: "Review the current code changes (staged, unstaged, and untracked files) and provide prioritized findings.".to_string(),
|
||||
user_facing_hint: "current changes".to_string(),
|
||||
},
|
||||
}));
|
||||
},
|
||||
)],
|
||||
actions: vec![Box::new(move |tx: &AppEventSender| {
|
||||
tx.send(AppEvent::CodexOp(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
target: ReviewTarget::UncommittedChanges,
|
||||
user_facing_hint: None,
|
||||
},
|
||||
}));
|
||||
})],
|
||||
dismiss_on_select: true,
|
||||
..Default::default()
|
||||
});
|
||||
@@ -2947,10 +2971,10 @@ impl ChatWidget {
|
||||
actions: vec![Box::new(move |tx3: &AppEventSender| {
|
||||
tx3.send(AppEvent::CodexOp(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
prompt: format!(
|
||||
"Review the code changes against the base branch '{branch}'. Start by finding the merge diff between the current branch and {branch}'s upstream e.g. (`git merge-base HEAD \"$(git rev-parse --abbrev-ref \"{branch}@{{upstream}}\")\"`), then run `git diff` against that SHA to see what changes we would merge into the {branch} branch. Provide prioritized, actionable findings."
|
||||
),
|
||||
user_facing_hint: format!("changes against '{branch}'"),
|
||||
target: ReviewTarget::BaseBranch {
|
||||
branch: branch.clone(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
},
|
||||
}));
|
||||
})],
|
||||
@@ -2977,20 +3001,18 @@ impl ChatWidget {
|
||||
for entry in commits {
|
||||
let subject = entry.subject.clone();
|
||||
let sha = entry.sha.clone();
|
||||
let short = sha.chars().take(7).collect::<String>();
|
||||
let search_val = format!("{subject} {sha}");
|
||||
|
||||
items.push(SelectionItem {
|
||||
name: subject.clone(),
|
||||
actions: vec![Box::new(move |tx3: &AppEventSender| {
|
||||
let hint = format!("commit {short}");
|
||||
let prompt = format!(
|
||||
"Review the code changes introduced by commit {sha} (\"{subject}\"). Provide prioritized, actionable findings."
|
||||
);
|
||||
tx3.send(AppEvent::CodexOp(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
prompt,
|
||||
user_facing_hint: hint,
|
||||
target: ReviewTarget::Commit {
|
||||
sha: sha.clone(),
|
||||
title: Some(subject.clone()),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
},
|
||||
}));
|
||||
})],
|
||||
@@ -3023,8 +3045,10 @@ impl ChatWidget {
|
||||
}
|
||||
tx.send(AppEvent::CodexOp(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
prompt: trimmed.clone(),
|
||||
user_facing_hint: trimmed,
|
||||
target: ReviewTarget::Custom {
|
||||
instructions: trimmed,
|
||||
},
|
||||
user_facing_hint: None,
|
||||
},
|
||||
}));
|
||||
}),
|
||||
@@ -3226,20 +3250,18 @@ pub(crate) fn show_review_commit_picker_with_entries(
|
||||
for entry in entries {
|
||||
let subject = entry.subject.clone();
|
||||
let sha = entry.sha.clone();
|
||||
let short = sha.chars().take(7).collect::<String>();
|
||||
let search_val = format!("{subject} {sha}");
|
||||
|
||||
items.push(SelectionItem {
|
||||
name: subject.clone(),
|
||||
actions: vec![Box::new(move |tx3: &AppEventSender| {
|
||||
let hint = format!("commit {short}");
|
||||
let prompt = format!(
|
||||
"Review the code changes introduced by commit {sha} (\"{subject}\"). Provide prioritized, actionable findings."
|
||||
);
|
||||
tx3.send(AppEvent::CodexOp(Op::Review {
|
||||
review_request: ReviewRequest {
|
||||
prompt,
|
||||
user_facing_hint: hint,
|
||||
target: ReviewTarget::Commit {
|
||||
sha: sha.clone(),
|
||||
title: Some(subject.clone()),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
},
|
||||
}));
|
||||
})],
|
||||
|
||||
@@ -18,6 +18,7 @@ use codex_core::protocol::AgentReasoningDeltaEvent;
|
||||
use codex_core::protocol::AgentReasoningEvent;
|
||||
use codex_core::protocol::ApplyPatchApprovalRequestEvent;
|
||||
use codex_core::protocol::BackgroundEventEvent;
|
||||
use codex_core::protocol::CreditsSnapshot;
|
||||
use codex_core::protocol::Event;
|
||||
use codex_core::protocol::EventMsg;
|
||||
use codex_core::protocol::ExecApprovalRequestEvent;
|
||||
@@ -35,6 +36,7 @@ use codex_core::protocol::ReviewFinding;
|
||||
use codex_core::protocol::ReviewLineRange;
|
||||
use codex_core::protocol::ReviewOutputEvent;
|
||||
use codex_core::protocol::ReviewRequest;
|
||||
use codex_core::protocol::ReviewTarget;
|
||||
use codex_core::protocol::StreamErrorEvent;
|
||||
use codex_core::protocol::TaskCompleteEvent;
|
||||
use codex_core::protocol::TaskStartedEvent;
|
||||
@@ -153,8 +155,10 @@ fn entered_review_mode_uses_request_hint() {
|
||||
chat.handle_codex_event(Event {
|
||||
id: "review-start".into(),
|
||||
msg: EventMsg::EnteredReviewMode(ReviewRequest {
|
||||
prompt: "Review the latest changes".to_string(),
|
||||
user_facing_hint: "feature branch".to_string(),
|
||||
target: ReviewTarget::BaseBranch {
|
||||
branch: "feature".to_string(),
|
||||
},
|
||||
user_facing_hint: Some("feature branch".to_string()),
|
||||
}),
|
||||
});
|
||||
|
||||
@@ -172,8 +176,8 @@ fn entered_review_mode_defaults_to_current_changes_banner() {
|
||||
chat.handle_codex_event(Event {
|
||||
id: "review-start".into(),
|
||||
msg: EventMsg::EnteredReviewMode(ReviewRequest {
|
||||
prompt: "Review the current changes".to_string(),
|
||||
user_facing_hint: "current changes".to_string(),
|
||||
target: ReviewTarget::UncommittedChanges,
|
||||
user_facing_hint: None,
|
||||
}),
|
||||
});
|
||||
|
||||
@@ -239,8 +243,10 @@ fn review_restores_context_window_indicator() {
|
||||
chat.handle_codex_event(Event {
|
||||
id: "review-start".into(),
|
||||
msg: EventMsg::EnteredReviewMode(ReviewRequest {
|
||||
prompt: "Review the latest changes".to_string(),
|
||||
user_facing_hint: "feature branch".to_string(),
|
||||
target: ReviewTarget::BaseBranch {
|
||||
branch: "feature".to_string(),
|
||||
},
|
||||
user_facing_hint: Some("feature branch".to_string()),
|
||||
}),
|
||||
});
|
||||
|
||||
@@ -292,6 +298,41 @@ fn token_count_none_resets_context_indicator() {
|
||||
assert_eq!(chat.bottom_pane.context_window_percent(), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn context_indicator_shows_used_tokens_when_window_unknown() {
|
||||
let (mut chat, _rx, _ops) = make_chatwidget_manual();
|
||||
|
||||
chat.config.model_context_window = None;
|
||||
let auto_compact_limit = 200_000;
|
||||
chat.config.model_auto_compact_token_limit = Some(auto_compact_limit);
|
||||
|
||||
// No model window, so the indicator should fall back to showing tokens used.
|
||||
let total_tokens = 106_000;
|
||||
let token_usage = TokenUsage {
|
||||
total_tokens,
|
||||
..TokenUsage::default()
|
||||
};
|
||||
let token_info = TokenUsageInfo {
|
||||
total_token_usage: token_usage.clone(),
|
||||
last_token_usage: token_usage,
|
||||
model_context_window: None,
|
||||
};
|
||||
|
||||
chat.handle_codex_event(Event {
|
||||
id: "token-usage".into(),
|
||||
msg: EventMsg::TokenCount(TokenCountEvent {
|
||||
info: Some(token_info),
|
||||
rate_limits: None,
|
||||
}),
|
||||
});
|
||||
|
||||
assert_eq!(chat.bottom_pane.context_window_percent(), None);
|
||||
assert_eq!(
|
||||
chat.bottom_pane.context_window_used_tokens(),
|
||||
Some(total_tokens)
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg_attr(
|
||||
target_os = "macos",
|
||||
ignore = "system configuration APIs are blocked under macOS seatbelt"
|
||||
@@ -481,6 +522,53 @@ fn test_rate_limit_warnings_monthly() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rate_limit_snapshot_keeps_prior_credits_when_missing_from_headers() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual();
|
||||
|
||||
chat.on_rate_limit_snapshot(Some(RateLimitSnapshot {
|
||||
primary: None,
|
||||
secondary: None,
|
||||
credits: Some(CreditsSnapshot {
|
||||
has_credits: true,
|
||||
unlimited: false,
|
||||
balance: Some("17.5".to_string()),
|
||||
}),
|
||||
}));
|
||||
let initial_balance = chat
|
||||
.rate_limit_snapshot
|
||||
.as_ref()
|
||||
.and_then(|snapshot| snapshot.credits.as_ref())
|
||||
.and_then(|credits| credits.balance.as_deref());
|
||||
assert_eq!(initial_balance, Some("17.5"));
|
||||
|
||||
chat.on_rate_limit_snapshot(Some(RateLimitSnapshot {
|
||||
primary: Some(RateLimitWindow {
|
||||
used_percent: 80.0,
|
||||
window_minutes: Some(60),
|
||||
resets_at: Some(123),
|
||||
}),
|
||||
secondary: None,
|
||||
credits: None,
|
||||
}));
|
||||
|
||||
let display = chat
|
||||
.rate_limit_snapshot
|
||||
.as_ref()
|
||||
.expect("rate limits should be cached");
|
||||
let credits = display
|
||||
.credits
|
||||
.as_ref()
|
||||
.expect("credits should persist when headers omit them");
|
||||
|
||||
assert_eq!(credits.balance.as_deref(), Some("17.5"));
|
||||
assert!(!credits.unlimited);
|
||||
assert_eq!(
|
||||
display.primary.as_ref().map(|window| window.used_percent),
|
||||
Some(80.0)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rate_limit_switch_prompt_skips_when_on_lower_cost_model() {
|
||||
let (mut chat, _, _) = make_chatwidget_manual();
|
||||
@@ -1312,12 +1400,13 @@ fn custom_prompt_submit_sends_review_op() {
|
||||
match evt {
|
||||
AppEvent::CodexOp(Op::Review { review_request }) => {
|
||||
assert_eq!(
|
||||
review_request.prompt,
|
||||
"please audit dependencies".to_string()
|
||||
);
|
||||
assert_eq!(
|
||||
review_request.user_facing_hint,
|
||||
"please audit dependencies".to_string()
|
||||
review_request,
|
||||
ReviewRequest {
|
||||
target: ReviewTarget::Custom {
|
||||
instructions: "please audit dependencies".to_string(),
|
||||
},
|
||||
user_facing_hint: None,
|
||||
}
|
||||
);
|
||||
}
|
||||
other => panic!("unexpected app event: {other:?}"),
|
||||
|
||||
@@ -181,6 +181,14 @@ pub fn normalize_pasted_path(pasted: &str) -> Option<PathBuf> {
|
||||
drive || unc
|
||||
};
|
||||
if looks_like_windows_path {
|
||||
#[cfg(target_os = "linux")]
|
||||
{
|
||||
if is_probably_wsl()
|
||||
&& let Some(converted) = convert_windows_path_to_wsl(pasted)
|
||||
{
|
||||
return Some(converted);
|
||||
}
|
||||
}
|
||||
return Some(PathBuf::from(pasted));
|
||||
}
|
||||
|
||||
@@ -193,6 +201,41 @@ pub fn normalize_pasted_path(pasted: &str) -> Option<PathBuf> {
|
||||
None
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
fn is_probably_wsl() -> bool {
|
||||
std::env::var_os("WSL_DISTRO_NAME").is_some()
|
||||
|| std::env::var_os("WSL_INTEROP").is_some()
|
||||
|| std::env::var_os("WSLENV").is_some()
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
fn convert_windows_path_to_wsl(input: &str) -> Option<PathBuf> {
|
||||
if input.starts_with("\\\\") {
|
||||
return None;
|
||||
}
|
||||
|
||||
let drive_letter = input.chars().next()?.to_ascii_lowercase();
|
||||
if !drive_letter.is_ascii_lowercase() {
|
||||
return None;
|
||||
}
|
||||
|
||||
if input.get(1..2) != Some(":") {
|
||||
return None;
|
||||
}
|
||||
|
||||
let mut result = PathBuf::from(format!("/mnt/{drive_letter}"));
|
||||
for component in input
|
||||
.get(2..)?
|
||||
.trim_start_matches(['\\', '/'])
|
||||
.split(['\\', '/'])
|
||||
.filter(|component| !component.is_empty())
|
||||
{
|
||||
result.push(component);
|
||||
}
|
||||
|
||||
Some(result)
|
||||
}
|
||||
|
||||
/// Infer an image format for the provided path based on its extension.
|
||||
pub fn pasted_image_format(path: &Path) -> EncodedImageFormat {
|
||||
match path
|
||||
@@ -210,6 +253,40 @@ pub fn pasted_image_format(path: &Path) -> EncodedImageFormat {
|
||||
#[cfg(test)]
|
||||
mod pasted_paths_tests {
|
||||
use super::*;
|
||||
#[cfg(target_os = "linux")]
|
||||
use std::ffi::OsString;
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
struct EnvVarGuard {
|
||||
key: &'static str,
|
||||
original: Option<OsString>,
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
impl EnvVarGuard {
|
||||
fn set(key: &'static str, value: &str) -> Self {
|
||||
let original = std::env::var_os(key);
|
||||
unsafe {
|
||||
std::env::set_var(key, value);
|
||||
}
|
||||
Self { key, original }
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
impl Drop for EnvVarGuard {
|
||||
fn drop(&mut self) {
|
||||
if let Some(original) = &self.original {
|
||||
unsafe {
|
||||
std::env::set_var(self.key, original);
|
||||
}
|
||||
} else {
|
||||
unsafe {
|
||||
std::env::remove_var(self.key);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
#[test]
|
||||
@@ -223,7 +300,17 @@ mod pasted_paths_tests {
|
||||
fn normalize_file_url_windows() {
|
||||
let input = r"C:\Temp\example.png";
|
||||
let result = normalize_pasted_path(input).expect("should parse file URL");
|
||||
assert_eq!(result, PathBuf::from(r"C:\Temp\example.png"));
|
||||
#[cfg(target_os = "linux")]
|
||||
let expected = if is_probably_wsl()
|
||||
&& let Some(converted) = convert_windows_path_to_wsl(input)
|
||||
{
|
||||
converted
|
||||
} else {
|
||||
PathBuf::from(r"C:\Temp\example.png")
|
||||
};
|
||||
#[cfg(not(target_os = "linux"))]
|
||||
let expected = PathBuf::from(r"C:\Temp\example.png");
|
||||
assert_eq!(result, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -291,10 +378,17 @@ mod pasted_paths_tests {
|
||||
fn normalize_unquoted_windows_path_with_spaces() {
|
||||
let input = r"C:\\Users\\Alice\\My Pictures\\example image.png";
|
||||
let result = normalize_pasted_path(input).expect("should accept unquoted windows path");
|
||||
assert_eq!(
|
||||
result,
|
||||
#[cfg(target_os = "linux")]
|
||||
let expected = if is_probably_wsl()
|
||||
&& let Some(converted) = convert_windows_path_to_wsl(input)
|
||||
{
|
||||
converted
|
||||
} else {
|
||||
PathBuf::from(r"C:\\Users\\Alice\\My Pictures\\example image.png")
|
||||
);
|
||||
};
|
||||
#[cfg(not(target_os = "linux"))]
|
||||
let expected = PathBuf::from(r"C:\\Users\\Alice\\My Pictures\\example image.png");
|
||||
assert_eq!(result, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -322,4 +416,16 @@ mod pasted_paths_tests {
|
||||
EncodedImageFormat::Other
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
#[test]
|
||||
fn normalize_windows_path_in_wsl() {
|
||||
let _guard = EnvVarGuard::set("WSL_DISTRO_NAME", "Ubuntu-24.04");
|
||||
let input = r"C:\\Users\\Alice\\Pictures\\example image.png";
|
||||
let result = normalize_pasted_path(input).expect("should convert windows path on wsl");
|
||||
assert_eq!(
|
||||
result,
|
||||
PathBuf::from("/mnt/c/Users/Alice/Pictures/example image.png")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -67,6 +67,7 @@ mod resume_picker;
|
||||
mod selection_list;
|
||||
mod session_log;
|
||||
mod shimmer;
|
||||
mod skill_error_prompt;
|
||||
mod slash_command;
|
||||
mod status;
|
||||
mod status_indicator_widget;
|
||||
|
||||
164
codex-rs/tui/src/skill_error_prompt.rs
Normal file
164
codex-rs/tui/src/skill_error_prompt.rs
Normal file
@@ -0,0 +1,164 @@
|
||||
use crate::tui::FrameRequester;
|
||||
use crate::tui::Tui;
|
||||
use crate::tui::TuiEvent;
|
||||
use codex_core::skills::SkillError;
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
use crossterm::event::KeyEventKind;
|
||||
use crossterm::event::KeyModifiers;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Rect;
|
||||
use ratatui::prelude::Stylize as _;
|
||||
use ratatui::text::Line;
|
||||
use ratatui::widgets::Block;
|
||||
use ratatui::widgets::Borders;
|
||||
use ratatui::widgets::Clear;
|
||||
use ratatui::widgets::Paragraph;
|
||||
use ratatui::widgets::Widget;
|
||||
use ratatui::widgets::WidgetRef;
|
||||
use ratatui::widgets::Wrap;
|
||||
use tokio_stream::StreamExt;
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub(crate) enum SkillErrorPromptOutcome {
|
||||
Continue,
|
||||
Exit,
|
||||
}
|
||||
|
||||
pub(crate) async fn run_skill_error_prompt(
|
||||
tui: &mut Tui,
|
||||
errors: &[SkillError],
|
||||
) -> SkillErrorPromptOutcome {
|
||||
struct AltScreenGuard<'a> {
|
||||
tui: &'a mut Tui,
|
||||
}
|
||||
impl<'a> AltScreenGuard<'a> {
|
||||
fn enter(tui: &'a mut Tui) -> Self {
|
||||
let _ = tui.enter_alt_screen();
|
||||
Self { tui }
|
||||
}
|
||||
}
|
||||
impl Drop for AltScreenGuard<'_> {
|
||||
fn drop(&mut self) {
|
||||
let _ = self.tui.leave_alt_screen();
|
||||
}
|
||||
}
|
||||
|
||||
let alt = AltScreenGuard::enter(tui);
|
||||
let mut screen = SkillErrorScreen::new(alt.tui.frame_requester(), errors);
|
||||
|
||||
let _ = alt.tui.draw(u16::MAX, |frame| {
|
||||
frame.render_widget_ref(&screen, frame.area());
|
||||
});
|
||||
|
||||
let events = alt.tui.event_stream();
|
||||
tokio::pin!(events);
|
||||
|
||||
while !screen.is_done() {
|
||||
if let Some(event) = events.next().await {
|
||||
match event {
|
||||
TuiEvent::Key(key_event) => screen.handle_key(key_event),
|
||||
TuiEvent::Paste(_) => {}
|
||||
TuiEvent::Draw => {
|
||||
let _ = alt.tui.draw(u16::MAX, |frame| {
|
||||
frame.render_widget_ref(&screen, frame.area());
|
||||
});
|
||||
}
|
||||
}
|
||||
} else {
|
||||
screen.confirm_continue();
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
screen.outcome()
|
||||
}
|
||||
|
||||
struct SkillErrorScreen {
|
||||
request_frame: FrameRequester,
|
||||
lines: Vec<Line<'static>>,
|
||||
done: bool,
|
||||
exit: bool,
|
||||
}
|
||||
|
||||
impl SkillErrorScreen {
|
||||
fn new(request_frame: FrameRequester, errors: &[SkillError]) -> Self {
|
||||
let mut lines: Vec<Line<'static>> = Vec::new();
|
||||
lines.push(Line::from("Skill validation errors detected".bold()));
|
||||
lines.push(Line::from(
|
||||
"Fix these SKILL.md files and restart. Invalid skills are ignored until resolved. Press enter or esc to continue, Ctrl+C or Ctrl+D to exit.",
|
||||
));
|
||||
lines.push(Line::from(""));
|
||||
|
||||
for error in errors {
|
||||
let message = format!("- {}: {}", error.path.display(), error.message);
|
||||
lines.push(Line::from(message));
|
||||
}
|
||||
|
||||
Self {
|
||||
request_frame,
|
||||
lines,
|
||||
done: false,
|
||||
exit: false,
|
||||
}
|
||||
}
|
||||
|
||||
fn is_done(&self) -> bool {
|
||||
self.done
|
||||
}
|
||||
|
||||
fn confirm_continue(&mut self) {
|
||||
self.done = true;
|
||||
self.exit = false;
|
||||
self.request_frame.schedule_frame();
|
||||
}
|
||||
|
||||
fn confirm_exit(&mut self) {
|
||||
self.done = true;
|
||||
self.exit = true;
|
||||
self.request_frame.schedule_frame();
|
||||
}
|
||||
|
||||
fn outcome(&self) -> SkillErrorPromptOutcome {
|
||||
if self.exit {
|
||||
SkillErrorPromptOutcome::Exit
|
||||
} else {
|
||||
SkillErrorPromptOutcome::Continue
|
||||
}
|
||||
}
|
||||
|
||||
fn handle_key(&mut self, key_event: KeyEvent) {
|
||||
if key_event.kind == KeyEventKind::Release {
|
||||
return;
|
||||
}
|
||||
|
||||
if key_event
|
||||
.modifiers
|
||||
.intersects(KeyModifiers::CONTROL | KeyModifiers::META)
|
||||
&& matches!(key_event.code, KeyCode::Char('c') | KeyCode::Char('d'))
|
||||
{
|
||||
self.confirm_exit();
|
||||
return;
|
||||
}
|
||||
|
||||
match key_event.code {
|
||||
KeyCode::Enter | KeyCode::Esc | KeyCode::Char(' ') | KeyCode::Char('q') => {
|
||||
self.confirm_continue();
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl WidgetRef for &SkillErrorScreen {
|
||||
fn render_ref(&self, area: Rect, buf: &mut Buffer) {
|
||||
Clear.render(area, buf);
|
||||
let block = Block::default()
|
||||
.title("Skill errors".bold())
|
||||
.borders(Borders::ALL);
|
||||
Paragraph::new(self.lines.clone())
|
||||
.block(block)
|
||||
.wrap(Wrap { trim: true })
|
||||
.render(area, buf);
|
||||
}
|
||||
}
|
||||
@@ -5,6 +5,7 @@ mod helpers;
|
||||
mod rate_limits;
|
||||
|
||||
pub(crate) use card::new_status_output;
|
||||
pub(crate) use helpers::format_tokens_compact;
|
||||
pub(crate) use rate_limits::RateLimitSnapshotDisplay;
|
||||
pub(crate) use rate_limits::rate_limit_snapshot_display;
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ use std::collections::HashSet;
|
||||
use std::ffi::OsString;
|
||||
use std::fs;
|
||||
use std::io;
|
||||
use std::path::Component;
|
||||
use std::path::Path;
|
||||
use std::path::PathBuf;
|
||||
|
||||
@@ -24,6 +25,24 @@ use crate::operations::run_git_for_stdout_all;
|
||||
const DEFAULT_COMMIT_MESSAGE: &str = "codex snapshot";
|
||||
/// Default threshold that triggers a warning about large untracked directories.
|
||||
const LARGE_UNTRACKED_WARNING_THRESHOLD: usize = 200;
|
||||
/// Directories that should always be ignored when capturing ghost snapshots,
|
||||
/// even if they are not listed in .gitignore.
|
||||
///
|
||||
/// These are typically large dependency or build trees that are not useful
|
||||
/// for undo and can cause snapshots to grow without bound.
|
||||
const DEFAULT_IGNORED_DIR_NAMES: &[&str] = &[
|
||||
"node_modules",
|
||||
".venv",
|
||||
"venv",
|
||||
"env",
|
||||
".env",
|
||||
"dist",
|
||||
"build",
|
||||
".pytest_cache",
|
||||
".mypy_cache",
|
||||
".cache",
|
||||
".tox",
|
||||
];
|
||||
|
||||
/// Options to control ghost commit creation.
|
||||
pub struct CreateGhostCommitOptions<'a> {
|
||||
@@ -334,12 +353,11 @@ fn capture_existing_untracked(
|
||||
repo_prefix: Option<&Path>,
|
||||
) -> Result<UntrackedSnapshot, GitToolingError> {
|
||||
// Ask git for the zero-delimited porcelain status so we can enumerate
|
||||
// every untracked or ignored path (including ones filtered by prefix).
|
||||
// every untracked path (including ones filtered by prefix).
|
||||
let mut args = vec![
|
||||
OsString::from("status"),
|
||||
OsString::from("--porcelain=2"),
|
||||
OsString::from("-z"),
|
||||
OsString::from("--ignored=matching"),
|
||||
OsString::from("--untracked-files=all"),
|
||||
];
|
||||
if let Some(prefix) = repo_prefix {
|
||||
@@ -373,6 +391,9 @@ fn capture_existing_untracked(
|
||||
}
|
||||
|
||||
let normalized = normalize_relative_path(Path::new(path_part))?;
|
||||
if should_ignore_for_snapshot(&normalized) {
|
||||
continue;
|
||||
}
|
||||
let absolute = repo_root.join(&normalized);
|
||||
let is_dir = absolute.is_dir();
|
||||
if is_dir {
|
||||
@@ -385,6 +406,19 @@ fn capture_existing_untracked(
|
||||
Ok(snapshot)
|
||||
}
|
||||
|
||||
fn should_ignore_for_snapshot(path: &Path) -> bool {
|
||||
path.components().any(|component| {
|
||||
if let Component::Normal(name) = component
|
||||
&& let Some(name_str) = name.to_str()
|
||||
{
|
||||
return DEFAULT_IGNORED_DIR_NAMES
|
||||
.iter()
|
||||
.any(|ignored| ignored == &name_str);
|
||||
}
|
||||
false
|
||||
})
|
||||
}
|
||||
|
||||
/// Removes untracked files and directories that were not present when the snapshot was captured.
|
||||
fn remove_new_untracked(
|
||||
repo_root: &Path,
|
||||
@@ -480,6 +514,7 @@ mod tests {
|
||||
use assert_matches::assert_matches;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::process::Command;
|
||||
use walkdir::WalkDir;
|
||||
|
||||
/// Runs a git command in the test repository and asserts success.
|
||||
fn run_git_in(repo_path: &Path, args: &[&str]) {
|
||||
@@ -621,6 +656,168 @@ mod tests {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn snapshot_ignores_default_ignored_directories() -> Result<(), GitToolingError> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
let repo = temp.path();
|
||||
init_test_repo(repo);
|
||||
|
||||
std::fs::write(repo.join("tracked.txt"), "contents\n")?;
|
||||
run_git_in(repo, &["add", "tracked.txt"]);
|
||||
run_git_in(
|
||||
repo,
|
||||
&[
|
||||
"-c",
|
||||
"user.name=Tester",
|
||||
"-c",
|
||||
"user.email=test@example.com",
|
||||
"commit",
|
||||
"-m",
|
||||
"initial",
|
||||
],
|
||||
);
|
||||
|
||||
let node_modules = repo.join("node_modules");
|
||||
std::fs::create_dir_all(node_modules.join("@scope/package/src"))?;
|
||||
for idx in 0..50 {
|
||||
let file = node_modules.join(format!("file-{idx}.js"));
|
||||
std::fs::write(file, "console.log('ignored');\n")?;
|
||||
}
|
||||
std::fs::write(
|
||||
node_modules.join("@scope/package/src/index.js"),
|
||||
"console.log('nested ignored');\n",
|
||||
)?;
|
||||
|
||||
let venv = repo.join(".venv");
|
||||
std::fs::create_dir_all(venv.join("lib/python/site-packages"))?;
|
||||
std::fs::write(
|
||||
venv.join("lib/python/site-packages/pkg.py"),
|
||||
"print('ignored')\n",
|
||||
)?;
|
||||
|
||||
let (ghost, report) =
|
||||
create_ghost_commit_with_report(&CreateGhostCommitOptions::new(repo))?;
|
||||
assert!(ghost.parent().is_some());
|
||||
|
||||
for file in ghost.preexisting_untracked_files() {
|
||||
let components = file.components().collect::<Vec<_>>();
|
||||
let mut has_default_ignored_component = false;
|
||||
for component in components {
|
||||
if let Component::Normal(name) = component
|
||||
&& let Some(name_str) = name.to_str()
|
||||
&& DEFAULT_IGNORED_DIR_NAMES
|
||||
.iter()
|
||||
.any(|ignored| ignored == &name_str)
|
||||
{
|
||||
has_default_ignored_component = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
!has_default_ignored_component,
|
||||
"unexpected default-ignored file captured: {file:?}"
|
||||
);
|
||||
}
|
||||
|
||||
for dir in ghost.preexisting_untracked_dirs() {
|
||||
let components = dir.components().collect::<Vec<_>>();
|
||||
let mut has_default_ignored_component = false;
|
||||
for component in components {
|
||||
if let Component::Normal(name) = component
|
||||
&& let Some(name_str) = name.to_str()
|
||||
&& DEFAULT_IGNORED_DIR_NAMES
|
||||
.iter()
|
||||
.any(|ignored| ignored == &name_str)
|
||||
{
|
||||
has_default_ignored_component = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
!has_default_ignored_component,
|
||||
"unexpected default-ignored dir captured: {dir:?}"
|
||||
);
|
||||
}
|
||||
|
||||
for entry in &report.large_untracked_dirs {
|
||||
let components = entry.path.components().collect::<Vec<_>>();
|
||||
let mut has_default_ignored_component = false;
|
||||
for component in components {
|
||||
if let Component::Normal(name) = component
|
||||
&& let Some(name_str) = name.to_str()
|
||||
&& DEFAULT_IGNORED_DIR_NAMES
|
||||
.iter()
|
||||
.any(|ignored| ignored == &name_str)
|
||||
{
|
||||
has_default_ignored_component = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
!has_default_ignored_component,
|
||||
"unexpected default-ignored dir in large_untracked_dirs: {:?}",
|
||||
entry.path
|
||||
);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn restore_preserves_default_ignored_directories() -> Result<(), GitToolingError> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
let repo = temp.path();
|
||||
init_test_repo(repo);
|
||||
|
||||
std::fs::write(repo.join("tracked.txt"), "snapshot version\n")?;
|
||||
run_git_in(repo, &["add", "tracked.txt"]);
|
||||
run_git_in(
|
||||
repo,
|
||||
&[
|
||||
"-c",
|
||||
"user.name=Tester",
|
||||
"-c",
|
||||
"user.email=test@example.com",
|
||||
"commit",
|
||||
"-m",
|
||||
"initial",
|
||||
],
|
||||
);
|
||||
|
||||
let node_modules = repo.join("node_modules");
|
||||
std::fs::create_dir_all(node_modules.join("pkg"))?;
|
||||
std::fs::write(
|
||||
node_modules.join("pkg/index.js"),
|
||||
"console.log('before');\n",
|
||||
)?;
|
||||
|
||||
let ghost = create_ghost_commit(&CreateGhostCommitOptions::new(repo))?;
|
||||
|
||||
std::fs::write(repo.join("tracked.txt"), "snapshot delta\n")?;
|
||||
std::fs::write(node_modules.join("pkg/index.js"), "console.log('after');\n")?;
|
||||
std::fs::write(node_modules.join("pkg/extra.js"), "console.log('extra');\n")?;
|
||||
std::fs::write(repo.join("temp.txt"), "new file\n")?;
|
||||
|
||||
restore_ghost_commit(repo, &ghost)?;
|
||||
|
||||
let tracked_after = std::fs::read_to_string(repo.join("tracked.txt"))?;
|
||||
assert_eq!(tracked_after, "snapshot version\n");
|
||||
|
||||
let node_modules_exists = node_modules.exists();
|
||||
assert!(node_modules_exists);
|
||||
|
||||
let files_under_node_modules: Vec<_> = WalkDir::new(&node_modules)
|
||||
.into_iter()
|
||||
.filter_map(Result::ok)
|
||||
.filter(|entry| entry.file_type().is_file())
|
||||
.collect();
|
||||
assert!(!files_under_node_modules.is_empty());
|
||||
|
||||
assert!(!repo.join("temp.txt").exists());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn create_snapshot_reports_nested_large_untracked_dirs_under_tracked_parent()
|
||||
-> Result<(), GitToolingError> {
|
||||
@@ -880,8 +1077,8 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
/// Restoring removes ignored directories created after the snapshot.
|
||||
fn restore_removes_new_ignored_directory() -> Result<(), GitToolingError> {
|
||||
/// Restoring leaves ignored directories created after the snapshot untouched.
|
||||
fn restore_preserves_new_ignored_directory() -> Result<(), GitToolingError> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
let repo = temp.path();
|
||||
init_test_repo(repo);
|
||||
@@ -910,7 +1107,124 @@ mod tests {
|
||||
|
||||
restore_ghost_commit(repo, &ghost)?;
|
||||
|
||||
assert!(!vscode.exists());
|
||||
assert!(vscode.exists());
|
||||
let settings_after = std::fs::read_to_string(vscode.join("settings.json"))?;
|
||||
assert_eq!(settings_after, "{\n \"after\": true\n}\n");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
/// Restoring leaves ignored files created after the snapshot untouched.
|
||||
fn restore_preserves_new_ignored_file() -> Result<(), GitToolingError> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
let repo = temp.path();
|
||||
init_test_repo(repo);
|
||||
|
||||
std::fs::write(repo.join(".gitignore"), "ignored.txt\n")?;
|
||||
std::fs::write(repo.join("tracked.txt"), "snapshot version\n")?;
|
||||
run_git_in(repo, &["add", ".gitignore", "tracked.txt"]);
|
||||
run_git_in(
|
||||
repo,
|
||||
&[
|
||||
"-c",
|
||||
"user.name=Tester",
|
||||
"-c",
|
||||
"user.email=test@example.com",
|
||||
"commit",
|
||||
"-m",
|
||||
"initial",
|
||||
],
|
||||
);
|
||||
|
||||
let ghost = create_ghost_commit(&CreateGhostCommitOptions::new(repo))?;
|
||||
|
||||
let ignored = repo.join("ignored.txt");
|
||||
std::fs::write(&ignored, "created later\n")?;
|
||||
|
||||
restore_ghost_commit(repo, &ghost)?;
|
||||
|
||||
assert!(ignored.exists());
|
||||
let contents = std::fs::read_to_string(&ignored)?;
|
||||
assert_eq!(contents, "created later\n");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
/// Restoring keeps deleted ignored files deleted when they were absent before the snapshot.
|
||||
fn restore_respects_removed_ignored_file() -> Result<(), GitToolingError> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
let repo = temp.path();
|
||||
init_test_repo(repo);
|
||||
|
||||
std::fs::write(repo.join(".gitignore"), "ignored.txt\n")?;
|
||||
std::fs::write(repo.join("tracked.txt"), "snapshot version\n")?;
|
||||
let ignored = repo.join("ignored.txt");
|
||||
std::fs::write(&ignored, "initial state\n")?;
|
||||
run_git_in(repo, &["add", ".gitignore", "tracked.txt"]);
|
||||
run_git_in(
|
||||
repo,
|
||||
&[
|
||||
"-c",
|
||||
"user.name=Tester",
|
||||
"-c",
|
||||
"user.email=test@example.com",
|
||||
"commit",
|
||||
"-m",
|
||||
"initial",
|
||||
],
|
||||
);
|
||||
|
||||
let ghost = create_ghost_commit(&CreateGhostCommitOptions::new(repo))?;
|
||||
|
||||
std::fs::remove_file(&ignored)?;
|
||||
|
||||
restore_ghost_commit(repo, &ghost)?;
|
||||
|
||||
assert!(!ignored.exists());
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
/// Restoring leaves files matched by glob ignores intact.
|
||||
fn restore_preserves_ignored_glob_matches() -> Result<(), GitToolingError> {
|
||||
let temp = tempfile::tempdir()?;
|
||||
let repo = temp.path();
|
||||
init_test_repo(repo);
|
||||
|
||||
std::fs::write(repo.join(".gitignore"), "dummy-dir/*.txt\n")?;
|
||||
std::fs::write(repo.join("tracked.txt"), "snapshot version\n")?;
|
||||
run_git_in(repo, &["add", ".gitignore", "tracked.txt"]);
|
||||
run_git_in(
|
||||
repo,
|
||||
&[
|
||||
"-c",
|
||||
"user.name=Tester",
|
||||
"-c",
|
||||
"user.email=test@example.com",
|
||||
"commit",
|
||||
"-m",
|
||||
"initial",
|
||||
],
|
||||
);
|
||||
|
||||
let ghost = create_ghost_commit(&CreateGhostCommitOptions::new(repo))?;
|
||||
|
||||
let dummy_dir = repo.join("dummy-dir");
|
||||
std::fs::create_dir_all(&dummy_dir)?;
|
||||
let file1 = dummy_dir.join("file1.txt");
|
||||
let file2 = dummy_dir.join("file2.txt");
|
||||
std::fs::write(&file1, "first\n")?;
|
||||
std::fs::write(&file2, "second\n")?;
|
||||
|
||||
restore_ghost_commit(repo, &ghost)?;
|
||||
|
||||
assert!(file1.exists());
|
||||
assert!(file2.exists());
|
||||
assert_eq!(std::fs::read_to_string(file1)?, "first\n");
|
||||
assert_eq!(std::fs::read_to_string(file2)?, "second\n");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -195,6 +195,7 @@ If the selected model is known to support reasoning (for example: `o3`, `o4-mini
|
||||
- `"low"`
|
||||
- `"medium"` (default)
|
||||
- `"high"`
|
||||
- `"xhigh"` (available only on `gpt-5.1-codex-max`)
|
||||
|
||||
Note: to minimize reasoning, choose `"minimal"`.
|
||||
|
||||
|
||||
@@ -37,7 +37,7 @@ model_provider = "openai"
|
||||
# Reasoning & Verbosity (Responses API capable models)
|
||||
################################################################################
|
||||
|
||||
# Reasoning effort: minimal | low | medium | high (default: medium)
|
||||
# Reasoning effort: minimal | low | medium | high | xhigh (default: medium; xhigh only on gpt-5.1-codex-max)
|
||||
model_reasoning_effort = "medium"
|
||||
|
||||
# Reasoning summary: auto | concise | detailed | none (default: auto)
|
||||
|
||||
@@ -8,7 +8,7 @@ In 2021, OpenAI released Codex, an AI system designed to generate code from natu
|
||||
|
||||
### Which models are supported?
|
||||
|
||||
We recommend using Codex with GPT-5.1 Codex, our best coding model. The default reasoning level is medium, and you can upgrade to high for complex tasks with the `/model` command.
|
||||
We recommend using Codex with GPT-5.1 Codex Max, our best coding model. The default reasoning level is medium, and you can upgrade to high or xhigh (Codex Max only) for complex tasks with the `/model` command.
|
||||
|
||||
You can also use older models by using API-based auth and launching codex with the `--model` flag.
|
||||
|
||||
|
||||
62
docs/skills.md
Normal file
62
docs/skills.md
Normal file
@@ -0,0 +1,62 @@
|
||||
# Skills (experimental)
|
||||
|
||||
> **Warning:** This is an experimental and non-stable feature. If you depend on it, please expect breaking changes over the coming weeks and understand that there is currently no guarantee that this works well. Use at your own risk!
|
||||
|
||||
Codex can automatically discover reusable "skills" you keep on disk. A skill is a small bundle with a name, a short description (what it does and when to use it), and an optional body of instructions you can open when needed. Codex injects only the name, description, and file path into the runtime context; the body stays on disk.
|
||||
|
||||
## Where skills live
|
||||
|
||||
- Location (v1): `~/.codex/skills/**/SKILL.md` (recursive). Hidden entries and symlinks are skipped. Only files named exactly `SKILL.md` count.
|
||||
- Sorting: rendered by name, then path for stability.
|
||||
|
||||
## File format
|
||||
|
||||
- YAML frontmatter + body.
|
||||
- Required:
|
||||
- `name` (non-empty, ≤100 chars, sanitized to one line)
|
||||
- `description` (non-empty, ≤500 chars, sanitized to one line)
|
||||
- Extra keys are ignored. The body can contain any Markdown; it is not injected into context.
|
||||
|
||||
## Loading and rendering
|
||||
|
||||
- Loaded once at startup.
|
||||
- If valid skills exist, Codex appends a runtime-only `## Skills` section after `AGENTS.md`, one bullet per skill: `- <name>: <description> (file: /absolute/path/to/SKILL.md)`.
|
||||
- If no valid skills exist, the section is omitted. On-disk files are never modified.
|
||||
|
||||
## Validation and errors
|
||||
|
||||
- Invalid skills (missing/invalid YAML, empty/over-length fields) trigger a blocking, dismissible startup modal in the TUI that lists each path and error. Errors are also logged. You can dismiss to continue (invalid skills are ignored) or exit. Fix SKILL.md files and restart to clear the modal.
|
||||
|
||||
## Create a skill
|
||||
|
||||
1. Create `~/.codex/skills/<skill-name>/`.
|
||||
2. Add `SKILL.md`:
|
||||
|
||||
```
|
||||
---
|
||||
name: your-skill-name
|
||||
description: what it does and when to use it (<=500 chars)
|
||||
---
|
||||
|
||||
# Optional body
|
||||
Add instructions, references, examples, or scripts (kept on disk).
|
||||
```
|
||||
|
||||
3. Keep `name`/`description` within the limits; avoid newlines in those fields.
|
||||
4. Restart Codex to load the new skill.
|
||||
|
||||
## Example
|
||||
|
||||
```
|
||||
mkdir -p ~/.codex/skills/pdf-processing
|
||||
cat <<'SKILL_EXAMPLE' > ~/.codex/skills/pdf-processing/SKILL.md
|
||||
---
|
||||
name: pdf-processing
|
||||
description: Extract text and tables from PDFs; use when PDFs, forms, or document extraction are mentioned.
|
||||
---
|
||||
|
||||
# PDF Processing
|
||||
- Use pdfplumber to extract text.
|
||||
- For form filling, see FORMS.md.
|
||||
SKILL_EXAMPLE
|
||||
```
|
||||
Reference in New Issue
Block a user