Merge branch 'main' into jif/prompt-cleaning

This commit is contained in:
jif-oai
2025-12-03 09:54:31 +00:00
committed by GitHub
85 changed files with 4324 additions and 1159 deletions
+37 -5
View File
@@ -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",
@@ -1281,6 +1283,7 @@ dependencies = [
"serde_json",
"shlex",
"starlark",
"tempfile",
"thiserror 2.0.17",
]
@@ -1612,6 +1615,7 @@ dependencies = [
"textwrap 0.16.2",
"tokio",
"tokio-stream",
"tokio-util",
"toml",
"tracing",
"tracing-appender",
@@ -4464,6 +4468,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"
@@ -5142,8 +5152,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",
@@ -5154,7 +5165,7 @@ dependencies = [
"http-body",
"http-body-util",
"oauth2",
"paste",
"pastey",
"pin-project-lite",
"process-wrap",
"rand 0.9.2",
@@ -5176,8 +5187,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",
@@ -5765,6 +5777,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"
@@ -6576,6 +6601,7 @@ dependencies = [
"futures-sink",
"futures-util",
"pin-project-lite",
"slab",
"tokio",
]
@@ -6979,6 +7005,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"
+5 -5
View File
@@ -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),
@@ -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")]
@@ -1232,6 +1255,7 @@ pub struct TurnCompletedNotification {
/// Notification that the turn-level unified diff has changed.
/// Contains the latest aggregated diff across all file changes in the turn.
pub struct TurnDiffUpdatedNotification {
pub thread_id: String,
pub turn_id: String,
pub diff: String,
}
@@ -1304,6 +1328,8 @@ pub struct ItemCompletedNotification {
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct AgentMessageDeltaNotification {
pub thread_id: String,
pub turn_id: String,
pub item_id: String,
pub delta: String,
}
@@ -1312,8 +1338,11 @@ pub struct AgentMessageDeltaNotification {
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct ReasoningSummaryTextDeltaNotification {
pub thread_id: String,
pub turn_id: String,
pub item_id: String,
pub delta: String,
#[ts(type = "number")]
pub summary_index: i64,
}
@@ -1321,7 +1350,10 @@ pub struct ReasoningSummaryTextDeltaNotification {
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct ReasoningSummaryPartAddedNotification {
pub thread_id: String,
pub turn_id: String,
pub item_id: String,
#[ts(type = "number")]
pub summary_index: i64,
}
@@ -1329,8 +1361,11 @@ pub struct ReasoningSummaryPartAddedNotification {
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct ReasoningTextDeltaNotification {
pub thread_id: String,
pub turn_id: String,
pub item_id: String,
pub delta: String,
#[ts(type = "number")]
pub content_index: i64,
}
@@ -1338,6 +1373,18 @@ pub struct ReasoningTextDeltaNotification {
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct CommandExecutionOutputDeltaNotification {
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/")]
pub struct FileChangeOutputDeltaNotification {
pub thread_id: String,
pub turn_id: String,
pub item_id: String,
pub delta: String,
}
@@ -1346,6 +1393,8 @@ pub struct CommandExecutionOutputDeltaNotification {
#[serde(rename_all = "camelCase")]
#[ts(export_to = "v2/")]
pub struct McpToolCallProgressNotification {
pub thread_id: String,
pub turn_id: String,
pub item_id: String,
pub message: String,
}
@@ -1451,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>,
}
+3 -1
View File
@@ -563,7 +563,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;
+21 -1
View File
@@ -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.
+108 -35
View File
@@ -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;
@@ -260,6 +262,8 @@ pub(crate) async fn apply_bespoke_event_handling(
}
EventMsg::AgentMessageContentDelta(event) => {
let notification = AgentMessageDeltaNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item_id: event.item_id,
delta: event.delta,
};
@@ -278,6 +282,8 @@ pub(crate) async fn apply_bespoke_event_handling(
}
EventMsg::ReasoningContentDelta(event) => {
let notification = ReasoningSummaryTextDeltaNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item_id: event.item_id,
delta: event.delta,
summary_index: event.summary_index,
@@ -290,6 +296,8 @@ pub(crate) async fn apply_bespoke_event_handling(
}
EventMsg::ReasoningRawContentDelta(event) => {
let notification = ReasoningTextDeltaNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item_id: event.item_id,
delta: event.delta,
content_index: event.content_index,
@@ -300,6 +308,8 @@ pub(crate) async fn apply_bespoke_event_handling(
}
EventMsg::AgentReasoningSectionBreak(event) => {
let notification = ReasoningSummaryPartAddedNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.clone(),
item_id: event.item_id,
summary_index: event.summary_index,
};
@@ -342,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,
@@ -493,15 +527,44 @@ pub(crate) async fn apply_bespoke_event_handling(
.await;
}
EventMsg::ExecCommandOutputDelta(exec_command_output_delta_event) => {
let notification = CommandExecutionOutputDeltaNotification {
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 {
@@ -588,6 +651,7 @@ pub(crate) async fn apply_bespoke_event_handling(
}
EventMsg::TurnDiff(turn_diff_event) => {
handle_turn_diff(
conversation_id,
&event_turn_id,
turn_diff_event,
api_version,
@@ -610,6 +674,7 @@ pub(crate) async fn apply_bespoke_event_handling(
}
async fn handle_turn_diff(
conversation_id: ConversationId,
event_turn_id: &str,
turn_diff_event: TurnDiffEvent,
api_version: ApiVersion,
@@ -617,6 +682,7 @@ async fn handle_turn_diff(
) {
if let ApiVersion::V2 = api_version {
let notification = TurnDiffUpdatedNotification {
thread_id: conversation_id.to_string(),
turn_id: event_turn_id.to_string(),
diff: turn_diff_event.unified_diff,
};
@@ -652,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 {
@@ -659,6 +726,7 @@ async fn emit_turn_completed_with_status(
turn: Turn {
id: event_turn_id,
items: vec![],
error,
status,
},
};
@@ -747,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(
@@ -768,6 +835,7 @@ async fn handle_turn_interrupted(
conversation_id,
event_turn_id,
TurnStatus::Interrupted,
None,
outgoing,
)
.await;
@@ -1240,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:?}"),
}
@@ -1280,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:?}"),
}
@@ -1319,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:?}"),
@@ -1587,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:?}"),
@@ -1608,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:?}"),
@@ -1630,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:?}"),
}
@@ -1774,8 +1842,10 @@ mod tests {
let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY);
let outgoing = OutgoingMessageSender::new(tx);
let unified_diff = "--- a\n+++ b\n".to_string();
let conversation_id = ConversationId::new();
handle_turn_diff(
conversation_id,
"turn-1",
TurnDiffEvent {
unified_diff: unified_diff.clone(),
@@ -1793,6 +1863,7 @@ mod tests {
OutgoingMessage::AppServerNotification(ServerNotification::TurnDiffUpdated(
notification,
)) => {
assert_eq!(notification.thread_id, conversation_id.to_string());
assert_eq!(notification.turn_id, "turn-1");
assert_eq!(notification.diff, unified_diff);
}
@@ -1806,8 +1877,10 @@ mod tests {
async fn test_handle_turn_diff_is_noop_for_v1() -> Result<()> {
let (tx, mut rx) = mpsc::channel(CHANNEL_CAPACITY);
let outgoing = OutgoingMessageSender::new(tx);
let conversation_id = ConversationId::new();
handle_turn_diff(
conversation_id,
"turn-1",
TurnDiffEvent {
unified_diff: "diff".to_string(),
@@ -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,
}
}
+1 -1
View File
@@ -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
+14
View File
@@ -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?;
}
+5 -4
View File
@@ -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
+12
View File
@@ -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();
+1
View File
@@ -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 }
+152 -10
View File
@@ -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,
+4 -14
View File
@@ -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(
+2 -2
View File
@@ -256,8 +256,8 @@ pub struct History {
/// If true, history entries will not be written to disk.
pub persistence: HistoryPersistence,
/// If set, the maximum size of the history file in bytes.
/// TODO(mbolin): Not currently honored.
/// If set, the maximum size of the history file in bytes. The oldest entries
/// are dropped once the file exceeds this limit.
pub max_bytes: Option<usize>,
}
+13 -10
View File
@@ -115,7 +115,7 @@ fn evaluate_with_policy(
}
}
pub(crate) fn create_approval_requirement_for_command(
pub(crate) async fn create_approval_requirement_for_command(
policy: &Policy,
command: &[String],
approval_policy: AskForApproval,
@@ -296,8 +296,8 @@ prefix_rule(pattern=["rm"], decision="forbidden")
);
}
#[test]
fn approval_requirement_prefers_execpolicy_match() {
#[tokio::test]
async fn approval_requirement_prefers_execpolicy_match() {
let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#;
let mut parser = PolicyParser::new();
parser
@@ -312,7 +312,8 @@ prefix_rule(pattern=["rm"], decision="forbidden")
AskForApproval::OnRequest,
&SandboxPolicy::DangerFullAccess,
SandboxPermissions::UseDefault,
);
)
.await;
assert_eq!(
requirement,
@@ -322,8 +323,8 @@ prefix_rule(pattern=["rm"], decision="forbidden")
);
}
#[test]
fn approval_requirement_respects_approval_policy() {
#[tokio::test]
async fn approval_requirement_respects_approval_policy() {
let policy_src = r#"prefix_rule(pattern=["rm"], decision="prompt")"#;
let mut parser = PolicyParser::new();
parser
@@ -338,7 +339,8 @@ prefix_rule(pattern=["rm"], decision="forbidden")
AskForApproval::Never,
&SandboxPolicy::DangerFullAccess,
SandboxPermissions::UseDefault,
);
)
.await;
assert_eq!(
requirement,
@@ -348,8 +350,8 @@ prefix_rule(pattern=["rm"], decision="forbidden")
);
}
#[test]
fn approval_requirement_falls_back_to_heuristics() {
#[tokio::test]
async fn approval_requirement_falls_back_to_heuristics() {
let command = vec!["python".to_string()];
let empty_policy = Policy::empty();
@@ -359,7 +361,8 @@ prefix_rule(pattern=["rm"], decision="forbidden")
AskForApproval::UnlessTrusted,
&SandboxPolicy::ReadOnly,
SandboxPermissions::UseDefault,
);
)
.await;
assert_eq!(
requirement,
+20 -4
View File
@@ -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,
},
];
+11 -10
View File
@@ -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);
+2
View File
@@ -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;
+9 -6
View File
@@ -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
+384 -52
View File
@@ -16,8 +16,14 @@
use std::fs::File;
use std::fs::OpenOptions;
use std::io::BufRead;
use std::io::BufReader;
use std::io::Read;
use std::io::Result;
use std::io::Seek;
use std::io::SeekFrom;
use std::io::Write;
use std::path::Path;
use std::path::PathBuf;
use serde::Deserialize;
@@ -39,10 +45,13 @@ use std::os::unix::fs::PermissionsExt;
/// Filename that stores the message history inside `~/.codex`.
const HISTORY_FILENAME: &str = "history.jsonl";
/// When history exceeds the hard cap, trim it down to this fraction of `max_bytes`.
const HISTORY_SOFT_CAP_RATIO: f64 = 0.8;
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,
@@ -97,11 +106,12 @@ pub(crate) async fn append_entry(
.map_err(|e| std::io::Error::other(format!("failed to serialise history entry: {e}")))?;
line.push('\n');
// Open in append-only mode.
// Open the history file for read/write access (append-only on Unix).
let mut options = OpenOptions::new();
options.append(true).read(true).create(true);
options.read(true).write(true).create(true);
#[cfg(unix)]
{
options.append(true);
options.mode(0o600);
}
@@ -110,6 +120,8 @@ pub(crate) async fn append_entry(
// Ensure permissions.
ensure_owner_only_permissions(&history_file).await?;
let history_max_bytes = config.history.max_bytes;
// Perform a blocking write under an advisory write lock using std::fs.
tokio::task::spawn_blocking(move || -> Result<()> {
// Retry a few times to avoid indefinite blocking when contended.
@@ -117,8 +129,12 @@ pub(crate) async fn append_entry(
match history_file.try_lock() {
Ok(()) => {
// While holding the exclusive lock, write the full line.
// We do not open the file with `append(true)` on Windows, so ensure the
// cursor is positioned at the end before writing.
history_file.seek(SeekFrom::End(0))?;
history_file.write_all(line.as_bytes())?;
history_file.flush()?;
enforce_history_limit(&mut history_file, history_max_bytes)?;
return Ok(());
}
Err(std::fs::TryLockError::WouldBlock) => {
@@ -138,27 +154,144 @@ pub(crate) async fn append_entry(
Ok(())
}
/// Trim the history file to honor `max_bytes`, dropping the oldest lines while holding
/// the write lock so the newest entry is always retained. When the file exceeds the
/// hard cap, it rewrites the remaining tail to a soft cap to avoid trimming again
/// immediately on the next write.
fn enforce_history_limit(file: &mut File, max_bytes: Option<usize>) -> Result<()> {
let Some(max_bytes) = max_bytes else {
return Ok(());
};
if max_bytes == 0 {
return Ok(());
}
let max_bytes = match u64::try_from(max_bytes) {
Ok(value) => value,
Err(_) => return Ok(()),
};
let mut current_len = file.metadata()?.len();
if current_len <= max_bytes {
return Ok(());
}
let mut reader_file = file.try_clone()?;
reader_file.seek(SeekFrom::Start(0))?;
let mut buf_reader = BufReader::new(reader_file);
let mut line_lengths = Vec::new();
let mut line_buf = String::new();
loop {
line_buf.clear();
let bytes = buf_reader.read_line(&mut line_buf)?;
if bytes == 0 {
break;
}
line_lengths.push(bytes as u64);
}
if line_lengths.is_empty() {
return Ok(());
}
let last_index = line_lengths.len() - 1;
let trim_target = trim_target_bytes(max_bytes, line_lengths[last_index]);
let mut drop_bytes = 0u64;
let mut idx = 0usize;
while current_len > trim_target && idx < last_index {
current_len = current_len.saturating_sub(line_lengths[idx]);
drop_bytes += line_lengths[idx];
idx += 1;
}
if drop_bytes == 0 {
return Ok(());
}
let mut reader = buf_reader.into_inner();
reader.seek(SeekFrom::Start(drop_bytes))?;
let capacity = usize::try_from(current_len).unwrap_or(0);
let mut tail = Vec::with_capacity(capacity);
reader.read_to_end(&mut tail)?;
file.set_len(0)?;
file.seek(SeekFrom::Start(0))?;
file.write_all(&tail)?;
file.flush()?;
Ok(())
}
fn trim_target_bytes(max_bytes: u64, newest_entry_len: u64) -> u64 {
let soft_cap_bytes = ((max_bytes as f64) * HISTORY_SOFT_CAP_RATIO)
.floor()
.clamp(1.0, max_bytes as f64) as u64;
soft_cap_bytes.max(newest_entry_len)
}
/// Asynchronously fetch the history file's *identifier* (inode on Unix) and
/// 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.
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 +312,11 @@ 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> {
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 +332,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 +381,238 @@ 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);
#[cfg(unix)]
fn history_log_id(metadata: &std::fs::Metadata) -> Option<u64> {
use std::os::unix::fs::MetadataExt;
Some(metadata.ino())
}
#[cfg(windows)]
fn history_log_id(metadata: &std::fs::Metadata) -> Option<u64> {
use std::os::windows::fs::MetadataExt;
Some(metadata.creation_time())
}
#[cfg(not(any(unix, windows)))]
fn history_log_id(_metadata: &std::fs::Metadata) -> Option<u64> {
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??;
}
Ok(())
}
#[cfg(test)]
mod tests {
use super::*;
use crate::config::Config;
use crate::config::ConfigOverrides;
use crate::config::ConfigToml;
use codex_protocol::ConversationId;
use pretty_assertions::assert_eq;
use std::fs::File;
use std::io::Write;
use tempfile::TempDir;
#[cfg(not(unix))]
async fn ensure_owner_only_permissions(_file: &File) -> Result<()> {
// For now, on non-Unix, simply succeed.
Ok(())
#[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);
}
#[tokio::test]
async fn append_entry_trims_history_when_beyond_max_bytes() {
let codex_home = TempDir::new().expect("create temp dir");
let mut config = Config::load_from_base_config_with_overrides(
ConfigToml::default(),
ConfigOverrides::default(),
codex_home.path().to_path_buf(),
)
.expect("load config");
let conversation_id = ConversationId::new();
let entry_one = "a".repeat(200);
let entry_two = "b".repeat(200);
let history_path = codex_home.path().join("history.jsonl");
append_entry(&entry_one, &conversation_id, &config)
.await
.expect("write first entry");
let first_len = std::fs::metadata(&history_path).expect("metadata").len();
let limit_bytes = first_len + 10;
config.history.max_bytes =
Some(usize::try_from(limit_bytes).expect("limit should fit into usize"));
append_entry(&entry_two, &conversation_id, &config)
.await
.expect("write second entry");
let contents = std::fs::read_to_string(&history_path).expect("read history");
let entries = contents
.lines()
.map(|line| serde_json::from_str::<HistoryEntry>(line).expect("parse entry"))
.collect::<Vec<HistoryEntry>>();
assert_eq!(
entries.len(),
1,
"only one entry left because entry_one should be evicted"
);
assert_eq!(entries[0].text, entry_two);
assert!(std::fs::metadata(&history_path).expect("metadata").len() <= limit_bytes);
}
#[tokio::test]
async fn append_entry_trims_history_to_soft_cap() {
let codex_home = TempDir::new().expect("create temp dir");
let mut config = Config::load_from_base_config_with_overrides(
ConfigToml::default(),
ConfigOverrides::default(),
codex_home.path().to_path_buf(),
)
.expect("load config");
let conversation_id = ConversationId::new();
let short_entry = "a".repeat(200);
let long_entry = "b".repeat(400);
let history_path = codex_home.path().join("history.jsonl");
append_entry(&short_entry, &conversation_id, &config)
.await
.expect("write first entry");
let short_entry_len = std::fs::metadata(&history_path).expect("metadata").len();
append_entry(&long_entry, &conversation_id, &config)
.await
.expect("write second entry");
let two_entry_len = std::fs::metadata(&history_path).expect("metadata").len();
let long_entry_len = two_entry_len
.checked_sub(short_entry_len)
.expect("second entry length should be larger than first entry length");
config.history.max_bytes = Some(
usize::try_from((2 * long_entry_len) + (short_entry_len / 2))
.expect("max bytes should fit into usize"),
);
append_entry(&long_entry, &conversation_id, &config)
.await
.expect("write third entry");
let contents = std::fs::read_to_string(&history_path).expect("read history");
let entries = contents
.lines()
.map(|line| serde_json::from_str::<HistoryEntry>(line).expect("parse entry"))
.collect::<Vec<HistoryEntry>>();
assert_eq!(entries.len(), 1);
assert_eq!(entries[0].text, long_entry);
let pruned_len = std::fs::metadata(&history_path).expect("metadata").len() as u64;
let max_bytes = config
.history
.max_bytes
.expect("max bytes should be configured") as u64;
assert!(pruned_len <= max_bytes);
let soft_cap_bytes = ((max_bytes as f64) * HISTORY_SOFT_CAP_RATIO)
.floor()
.clamp(1.0, max_bytes as f64) as u64;
let len_without_first = 2 * long_entry_len;
assert!(
len_without_first <= max_bytes,
"dropping only the first entry would satisfy the hard cap"
);
assert!(
len_without_first > soft_cap_bytes,
"soft cap should require more aggressive trimming than the hard cap"
);
assert_eq!(pruned_len, long_entry_len);
assert!(pruned_len <= soft_cap_bytes.max(long_entry_len));
}
}
+2
View File
@@ -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,
}
}
+109 -9
View File
@@ -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
View 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),
}
}
}
+24 -113
View File
@@ -9,6 +9,7 @@ use std::sync::atomic::AtomicBool;
use time::OffsetDateTime;
use time::PrimitiveDateTime;
use time::format_description::FormatItem;
use time::format_description::well_known::Rfc3339;
use time::macros::format_description;
use uuid::Uuid;
@@ -39,18 +40,15 @@ pub struct ConversationItem {
pub path: PathBuf,
/// First up to `HEAD_RECORD_LIMIT` JSONL records parsed as JSON (includes meta line).
pub head: Vec<serde_json::Value>,
/// Last up to `TAIL_RECORD_LIMIT` JSONL response records parsed as JSON.
pub tail: Vec<serde_json::Value>,
/// RFC3339 timestamp string for when the session was created, if available.
pub created_at: Option<String>,
/// RFC3339 timestamp string for the most recent response in the tail, if available.
/// RFC3339 timestamp string for the most recent update (from file mtime).
pub updated_at: Option<String>,
}
#[derive(Default)]
struct HeadTailSummary {
head: Vec<serde_json::Value>,
tail: Vec<serde_json::Value>,
saw_session_meta: bool,
saw_user_event: bool,
source: Option<SessionSource>,
@@ -62,7 +60,6 @@ struct HeadTailSummary {
/// Hard cap to bound worst‑case work per request.
const MAX_SCAN_FILES: usize = 10000;
const HEAD_RECORD_LIMIT: usize = 10;
const TAIL_RECORD_LIMIT: usize = 10;
/// Pagination cursor identifying a file by timestamp and UUID.
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -141,13 +138,6 @@ pub(crate) async fn get_conversations(
Ok(result)
}
/// Load the full contents of a single conversation session file at `path`.
/// Returns the entire file contents as a String.
#[allow(dead_code)]
pub(crate) async fn get_conversation(path: &Path) -> io::Result<String> {
tokio::fs::read_to_string(path).await
}
/// Load conversation file paths from disk using directory traversal.
///
/// Directory layout: `~/.codex/sessions/YYYY/MM/DD/rollout-YYYY-MM-DDThh-mm-ss-<uuid>.jsonl`
@@ -212,9 +202,8 @@ async fn traverse_directories_for_paths(
more_matches_available = true;
break 'outer;
}
// Read head and simultaneously detect message events within the same
// first N JSONL records to avoid a second file read.
let summary = read_head_and_tail(&path, HEAD_RECORD_LIMIT, TAIL_RECORD_LIMIT)
// Read head and detect message events; stop once meta + user are found.
let summary = read_head_summary(&path, HEAD_RECORD_LIMIT)
.await
.unwrap_or_default();
if !allowed_sources.is_empty()
@@ -233,16 +222,19 @@ async fn traverse_directories_for_paths(
if summary.saw_session_meta && summary.saw_user_event {
let HeadTailSummary {
head,
tail,
created_at,
mut updated_at,
..
} = summary;
updated_at = updated_at.or_else(|| created_at.clone());
if updated_at.is_none() {
updated_at = file_modified_rfc3339(&path)
.await
.unwrap_or(None)
.or_else(|| created_at.clone());
}
items.push(ConversationItem {
path,
head,
tail,
created_at,
updated_at,
});
@@ -384,11 +376,7 @@ impl<'a> ProviderMatcher<'a> {
}
}
async fn read_head_and_tail(
path: &Path,
head_limit: usize,
tail_limit: usize,
) -> io::Result<HeadTailSummary> {
async fn read_head_summary(path: &Path, head_limit: usize) -> io::Result<HeadTailSummary> {
use tokio::io::AsyncBufReadExt;
let file = tokio::fs::File::open(path).await?;
@@ -441,107 +429,30 @@ async fn read_head_and_tail(
}
}
}
if summary.saw_session_meta && summary.saw_user_event {
break;
}
}
if tail_limit != 0 {
let (tail, updated_at) = read_tail_records(path, tail_limit).await?;
summary.tail = tail;
summary.updated_at = updated_at;
}
Ok(summary)
}
/// Read up to `HEAD_RECORD_LIMIT` records from the start of the rollout file at `path`.
/// This should be enough to produce a summary including the session meta line.
pub async fn read_head_for_summary(path: &Path) -> io::Result<Vec<serde_json::Value>> {
let summary = read_head_and_tail(path, HEAD_RECORD_LIMIT, 0).await?;
let summary = read_head_summary(path, HEAD_RECORD_LIMIT).await?;
Ok(summary.head)
}
async fn read_tail_records(
path: &Path,
max_records: usize,
) -> io::Result<(Vec<serde_json::Value>, Option<String>)> {
use std::io::SeekFrom;
use tokio::io::AsyncReadExt;
use tokio::io::AsyncSeekExt;
if max_records == 0 {
return Ok((Vec::new(), None));
}
const CHUNK_SIZE: usize = 8192;
let mut file = tokio::fs::File::open(path).await?;
let mut pos = file.seek(SeekFrom::End(0)).await?;
if pos == 0 {
return Ok((Vec::new(), None));
}
let mut buffer: Vec<u8> = Vec::new();
let mut latest_timestamp: Option<String> = None;
loop {
let slice_start = match (pos > 0, buffer.iter().position(|&b| b == b'\n')) {
(true, Some(idx)) => idx + 1,
_ => 0,
};
let (tail, newest_ts) = collect_last_response_values(&buffer[slice_start..], max_records);
if latest_timestamp.is_none() {
latest_timestamp = newest_ts.clone();
}
if tail.len() >= max_records || pos == 0 {
return Ok((tail, latest_timestamp.or(newest_ts)));
}
let read_size = CHUNK_SIZE.min(pos as usize);
if read_size == 0 {
return Ok((tail, latest_timestamp.or(newest_ts)));
}
pos -= read_size as u64;
file.seek(SeekFrom::Start(pos)).await?;
let mut chunk = vec![0; read_size];
file.read_exact(&mut chunk).await?;
chunk.extend_from_slice(&buffer);
buffer = chunk;
}
}
fn collect_last_response_values(
buffer: &[u8],
max_records: usize,
) -> (Vec<serde_json::Value>, Option<String>) {
use std::borrow::Cow;
if buffer.is_empty() || max_records == 0 {
return (Vec::new(), None);
}
let text: Cow<'_, str> = String::from_utf8_lossy(buffer);
let mut collected_rev: Vec<serde_json::Value> = Vec::new();
let mut latest_timestamp: Option<String> = None;
for line in text.lines().rev() {
let trimmed = line.trim();
if trimmed.is_empty() {
continue;
}
let parsed: serde_json::Result<RolloutLine> = serde_json::from_str(trimmed);
let Ok(rollout_line) = parsed else { continue };
let RolloutLine { timestamp, item } = rollout_line;
if let RolloutItem::ResponseItem(item) = item
&& let Ok(val) = serde_json::to_value(&item)
{
if latest_timestamp.is_none() {
latest_timestamp = Some(timestamp.clone());
}
collected_rev.push(val);
if collected_rev.len() == max_records {
break;
}
}
}
collected_rev.reverse();
(collected_rev, latest_timestamp)
async fn file_modified_rfc3339(path: &Path) -> io::Result<Option<String>> {
let meta = tokio::fs::metadata(path).await?;
let modified = meta.modified().ok();
let Some(modified) = modified else {
return Ok(None);
};
let dt = OffsetDateTime::from(modified);
Ok(dt.format(&Rfc3339).ok())
}
/// Locate a recorded conversation rollout file by its UUID string using the existing
+36 -250
View File
@@ -16,13 +16,11 @@ use crate::rollout::INTERACTIVE_SESSION_SOURCES;
use crate::rollout::list::ConversationItem;
use crate::rollout::list::ConversationsPage;
use crate::rollout::list::Cursor;
use crate::rollout::list::get_conversation;
use crate::rollout::list::get_conversations;
use anyhow::Result;
use codex_protocol::ConversationId;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::protocol::CompactedItem;
use codex_protocol::protocol::EventMsg;
use codex_protocol::protocol::RolloutItem;
use codex_protocol::protocol::RolloutLine;
@@ -226,28 +224,28 @@ async fn test_list_conversations_latest_first() {
"model_provider": "test-provider",
})];
let updated_times: Vec<Option<String>> =
page.items.iter().map(|i| i.updated_at.clone()).collect();
let expected = ConversationsPage {
items: vec![
ConversationItem {
path: p1,
head: head_3,
tail: Vec::new(),
created_at: Some("2025-01-03T12-00-00".into()),
updated_at: Some("2025-01-03T12-00-00".into()),
updated_at: updated_times.first().cloned().flatten(),
},
ConversationItem {
path: p2,
head: head_2,
tail: Vec::new(),
created_at: Some("2025-01-02T12-00-00".into()),
updated_at: Some("2025-01-02T12-00-00".into()),
updated_at: updated_times.get(1).cloned().flatten(),
},
ConversationItem {
path: p3,
head: head_1,
tail: Vec::new(),
created_at: Some("2025-01-01T12-00-00".into()),
updated_at: Some("2025-01-01T12-00-00".into()),
updated_at: updated_times.get(2).cloned().flatten(),
},
],
next_cursor: None,
@@ -355,6 +353,8 @@ async fn test_pagination_cursor() {
"source": "vscode",
"model_provider": "test-provider",
})];
let updated_page1: Vec<Option<String>> =
page1.items.iter().map(|i| i.updated_at.clone()).collect();
let expected_cursor1: Cursor =
serde_json::from_str(&format!("\"2025-03-04T09-00-00|{u4}\"")).unwrap();
let expected_page1 = ConversationsPage {
@@ -362,16 +362,14 @@ async fn test_pagination_cursor() {
ConversationItem {
path: p5,
head: head_5,
tail: Vec::new(),
created_at: Some("2025-03-05T09-00-00".into()),
updated_at: Some("2025-03-05T09-00-00".into()),
updated_at: updated_page1.first().cloned().flatten(),
},
ConversationItem {
path: p4,
head: head_4,
tail: Vec::new(),
created_at: Some("2025-03-04T09-00-00".into()),
updated_at: Some("2025-03-04T09-00-00".into()),
updated_at: updated_page1.get(1).cloned().flatten(),
},
],
next_cursor: Some(expected_cursor1.clone()),
@@ -422,6 +420,8 @@ async fn test_pagination_cursor() {
"source": "vscode",
"model_provider": "test-provider",
})];
let updated_page2: Vec<Option<String>> =
page2.items.iter().map(|i| i.updated_at.clone()).collect();
let expected_cursor2: Cursor =
serde_json::from_str(&format!("\"2025-03-02T09-00-00|{u2}\"")).unwrap();
let expected_page2 = ConversationsPage {
@@ -429,16 +429,14 @@ async fn test_pagination_cursor() {
ConversationItem {
path: p3,
head: head_3,
tail: Vec::new(),
created_at: Some("2025-03-03T09-00-00".into()),
updated_at: Some("2025-03-03T09-00-00".into()),
updated_at: updated_page2.first().cloned().flatten(),
},
ConversationItem {
path: p2,
head: head_2,
tail: Vec::new(),
created_at: Some("2025-03-02T09-00-00".into()),
updated_at: Some("2025-03-02T09-00-00".into()),
updated_at: updated_page2.get(1).cloned().flatten(),
},
],
next_cursor: Some(expected_cursor2.clone()),
@@ -473,13 +471,14 @@ async fn test_pagination_cursor() {
"source": "vscode",
"model_provider": "test-provider",
})];
let updated_page3: Vec<Option<String>> =
page3.items.iter().map(|i| i.updated_at.clone()).collect();
let expected_page3 = ConversationsPage {
items: vec![ConversationItem {
path: p1,
head: head_1,
tail: Vec::new(),
created_at: Some("2025-03-01T09-00-00".into()),
updated_at: Some("2025-03-01T09-00-00".into()),
updated_at: updated_page3.first().cloned().flatten(),
}],
next_cursor: None,
num_scanned_files: 5, // scanned 05, 04 (anchor), 03, 02 (anchor), 01
@@ -510,7 +509,7 @@ async fn test_get_conversation_contents() {
.unwrap();
let path = &page.items[0].path;
let content = get_conversation(path).await.unwrap();
let content = tokio::fs::read_to_string(path).await.unwrap();
// Page equality (single item)
let expected_path = home
@@ -533,9 +532,8 @@ async fn test_get_conversation_contents() {
items: vec![ConversationItem {
path: expected_path,
head: expected_head,
tail: Vec::new(),
created_at: Some(ts.into()),
updated_at: Some(ts.into()),
updated_at: page.items[0].updated_at.clone(),
}],
next_cursor: None,
num_scanned_files: 1,
@@ -570,7 +568,7 @@ async fn test_get_conversation_contents() {
}
#[tokio::test]
async fn test_tail_includes_last_response_items() -> Result<()> {
async fn test_updated_at_uses_file_mtime() -> Result<()> {
let temp = TempDir::new().unwrap();
let home = temp.path();
@@ -636,229 +634,16 @@ async fn test_tail_includes_last_response_items() -> Result<()> {
)
.await?;
let item = page.items.first().expect("conversation item");
let tail_len = item.tail.len();
assert_eq!(tail_len, 10usize.min(total_messages));
let expected: Vec<serde_json::Value> = (total_messages - tail_len..total_messages)
.map(|idx| {
serde_json::json!({
"type": "message",
"role": "assistant",
"content": [
{
"type": "output_text",
"text": format!("reply-{idx}"),
}
],
})
})
.collect();
assert_eq!(item.tail, expected);
assert_eq!(item.created_at.as_deref(), Some(ts));
let expected_updated = format!("{ts}-{last:02}", last = total_messages - 1);
assert_eq!(item.updated_at.as_deref(), Some(expected_updated.as_str()));
Ok(())
}
#[tokio::test]
async fn test_tail_handles_short_sessions() -> Result<()> {
let temp = TempDir::new().unwrap();
let home = temp.path();
let ts = "2025-06-02T08-30-00";
let uuid = Uuid::from_u128(7);
let day_dir = home.join("sessions").join("2025").join("06").join("02");
fs::create_dir_all(&day_dir)?;
let file_path = day_dir.join(format!("rollout-{ts}-{uuid}.jsonl"));
let mut file = File::create(&file_path)?;
let conversation_id = ConversationId::from_string(&uuid.to_string())?;
let meta_line = RolloutLine {
timestamp: ts.to_string(),
item: RolloutItem::SessionMeta(SessionMetaLine {
meta: SessionMeta {
id: conversation_id,
timestamp: ts.to_string(),
instructions: None,
cwd: ".".into(),
originator: "test_originator".into(),
cli_version: "test_version".into(),
source: SessionSource::VSCode,
model_provider: Some("test-provider".into()),
},
git: None,
}),
};
writeln!(file, "{}", serde_json::to_string(&meta_line)?)?;
let user_event_line = RolloutLine {
timestamp: ts.to_string(),
item: RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent {
message: "hi".into(),
images: None,
})),
};
writeln!(file, "{}", serde_json::to_string(&user_event_line)?)?;
for idx in 0..3 {
let response_line = RolloutLine {
timestamp: format!("{ts}-{idx:02}"),
item: RolloutItem::ResponseItem(ResponseItem::Message {
id: None,
role: "assistant".into(),
content: vec![ContentItem::OutputText {
text: format!("short-{idx}"),
}],
}),
};
writeln!(file, "{}", serde_json::to_string(&response_line)?)?;
}
drop(file);
let provider_filter = provider_vec(&[TEST_PROVIDER]);
let page = get_conversations(
home,
1,
None,
INTERACTIVE_SESSION_SOURCES,
Some(provider_filter.as_slice()),
TEST_PROVIDER,
)
.await?;
let tail = &page.items.first().expect("conversation item").tail;
assert_eq!(tail.len(), 3);
let expected: Vec<serde_json::Value> = (0..3)
.map(|idx| {
serde_json::json!({
"type": "message",
"role": "assistant",
"content": [
{
"type": "output_text",
"text": format!("short-{idx}"),
}
],
})
})
.collect();
assert_eq!(tail, &expected);
let expected_updated = format!("{ts}-{last:02}", last = 2);
assert_eq!(
page.items[0].updated_at.as_deref(),
Some(expected_updated.as_str())
);
Ok(())
}
#[tokio::test]
async fn test_tail_skips_trailing_non_responses() -> Result<()> {
let temp = TempDir::new().unwrap();
let home = temp.path();
let ts = "2025-06-03T10-00-00";
let uuid = Uuid::from_u128(11);
let day_dir = home.join("sessions").join("2025").join("06").join("03");
fs::create_dir_all(&day_dir)?;
let file_path = day_dir.join(format!("rollout-{ts}-{uuid}.jsonl"));
let mut file = File::create(&file_path)?;
let conversation_id = ConversationId::from_string(&uuid.to_string())?;
let meta_line = RolloutLine {
timestamp: ts.to_string(),
item: RolloutItem::SessionMeta(SessionMetaLine {
meta: SessionMeta {
id: conversation_id,
timestamp: ts.to_string(),
instructions: None,
cwd: ".".into(),
originator: "test_originator".into(),
cli_version: "test_version".into(),
source: SessionSource::VSCode,
model_provider: Some("test-provider".into()),
},
git: None,
}),
};
writeln!(file, "{}", serde_json::to_string(&meta_line)?)?;
let user_event_line = RolloutLine {
timestamp: ts.to_string(),
item: RolloutItem::EventMsg(EventMsg::UserMessage(UserMessageEvent {
message: "hello".into(),
images: None,
})),
};
writeln!(file, "{}", serde_json::to_string(&user_event_line)?)?;
for idx in 0..4 {
let response_line = RolloutLine {
timestamp: format!("{ts}-{idx:02}"),
item: RolloutItem::ResponseItem(ResponseItem::Message {
id: None,
role: "assistant".into(),
content: vec![ContentItem::OutputText {
text: format!("response-{idx}"),
}],
}),
};
writeln!(file, "{}", serde_json::to_string(&response_line)?)?;
}
let compacted_line = RolloutLine {
timestamp: format!("{ts}-compacted"),
item: RolloutItem::Compacted(CompactedItem {
message: "compacted".into(),
replacement_history: None,
}),
};
writeln!(file, "{}", serde_json::to_string(&compacted_line)?)?;
let shutdown_event = RolloutLine {
timestamp: format!("{ts}-shutdown"),
item: RolloutItem::EventMsg(EventMsg::ShutdownComplete),
};
writeln!(file, "{}", serde_json::to_string(&shutdown_event)?)?;
drop(file);
let provider_filter = provider_vec(&[TEST_PROVIDER]);
let page = get_conversations(
home,
1,
None,
INTERACTIVE_SESSION_SOURCES,
Some(provider_filter.as_slice()),
TEST_PROVIDER,
)
.await?;
let tail = &page.items.first().expect("conversation item").tail;
let expected: Vec<serde_json::Value> = (0..4)
.map(|idx| {
serde_json::json!({
"type": "message",
"role": "assistant",
"content": [
{
"type": "output_text",
"text": format!("response-{idx}"),
}
],
})
})
.collect();
assert_eq!(tail, &expected);
let expected_updated = format!("{ts}-{last:02}", last = 3);
assert_eq!(
page.items[0].updated_at.as_deref(),
Some(expected_updated.as_str())
);
let updated = item
.updated_at
.as_deref()
.and_then(|s| chrono::DateTime::parse_from_rfc3339(s).ok())
.map(|dt| dt.with_timezone(&chrono::Utc))
.expect("updated_at set from file mtime");
let now = chrono::Utc::now();
let age = now - updated;
assert!(age.num_seconds().abs() < 30);
Ok(())
}
@@ -913,22 +698,22 @@ async fn test_stable_ordering_same_second_pagination() {
"model_provider": "test-provider",
})]
};
let updated_page1: Vec<Option<String>> =
page1.items.iter().map(|i| i.updated_at.clone()).collect();
let expected_cursor1: Cursor = serde_json::from_str(&format!("\"{ts}|{u2}\"")).unwrap();
let expected_page1 = ConversationsPage {
items: vec![
ConversationItem {
path: p3,
head: head(u3),
tail: Vec::new(),
created_at: Some(ts.to_string()),
updated_at: Some(ts.to_string()),
updated_at: updated_page1.first().cloned().flatten(),
},
ConversationItem {
path: p2,
head: head(u2),
tail: Vec::new(),
created_at: Some(ts.to_string()),
updated_at: Some(ts.to_string()),
updated_at: updated_page1.get(1).cloned().flatten(),
},
],
next_cursor: Some(expected_cursor1.clone()),
@@ -953,13 +738,14 @@ async fn test_stable_ordering_same_second_pagination() {
.join("07")
.join("01")
.join(format!("rollout-2025-07-01T00-00-00-{u1}.jsonl"));
let updated_page2: Vec<Option<String>> =
page2.items.iter().map(|i| i.updated_at.clone()).collect();
let expected_page2 = ConversationsPage {
items: vec![ConversationItem {
path: p1,
head: head(u1),
tail: Vec::new(),
created_at: Some(ts.to_string()),
updated_at: Some(ts.to_string()),
updated_at: updated_page2.first().cloned().flatten(),
}],
next_cursor: None,
num_scanned_files: 3, // scanned u3, u2 (anchor), u1
+2 -5
View File
@@ -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
View 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
View 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
View 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
View 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"))
}
+15 -1
View File
@@ -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,86 @@ 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) => {
session
.record_model_warning(
format!("apply_patch was requested via {tool_name}. Use the apply_patch tool instead of exec_command."),
turn,
)
.await;
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 {
+22 -85
View File
@@ -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;
@@ -297,6 +231,15 @@ impl ShellHandler {
let event_ctx = ToolEventCtx::new(session.as_ref(), turn.as_ref(), &call_id, None);
emitter.begin(event_ctx).await;
let approval_requirement = create_approval_requirement_for_command(
&turn.exec_policy,
&exec_params.command,
turn.approval_policy,
&turn.sandbox_policy,
SandboxPermissions::from(exec_params.with_escalated_permissions.unwrap_or(false)),
)
.await;
let req = ShellRequest {
command: exec_params.command.clone(),
cwd: exec_params.cwd.clone(),
@@ -304,13 +247,7 @@ impl ShellHandler {
env: exec_params.env.clone(),
with_escalated_permissions: exec_params.with_escalated_permissions,
justification: exec_params.justification.clone(),
approval_requirement: create_approval_requirement_for_command(
&turn.exec_policy,
&exec_params.command,
turn.approval_policy,
&turn.sandbox_policy,
SandboxPermissions::from(exec_params.with_escalated_permissions.unwrap_or(false)),
),
approval_requirement,
};
let mut orchestrator = ToolOrchestrator::new();
let mut runtime = ShellRuntime::new();
@@ -6,6 +6,7 @@ use crate::protocol::EventMsg;
use crate::protocol::ExecCommandOutputDeltaEvent;
use crate::protocol::ExecCommandSource;
use crate::protocol::ExecOutputStream;
use crate::shell::default_user_shell;
use crate::shell::get_shell_by_model_provided_path;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
@@ -13,6 +14,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;
@@ -30,8 +32,8 @@ struct ExecCommandArgs {
cmd: String,
#[serde(default)]
workdir: Option<String>,
#[serde(default = "default_shell")]
shell: String,
#[serde(default)]
shell: Option<String>,
#[serde(default = "default_login")]
login: bool,
#[serde(default = "default_exec_yield_time_ms")]
@@ -64,10 +66,6 @@ fn default_write_stdin_yield_time_ms() -> u64 {
250
}
fn default_shell() -> String {
"/bin/bash".to_string()
}
fn default_login() -> bool {
true
}
@@ -103,6 +101,7 @@ impl ToolHandler for UnifiedExecHandler {
let ToolInvocation {
session,
turn,
tracker,
call_id,
tool_name,
payload,
@@ -153,12 +152,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(),
@@ -241,7 +254,12 @@ impl ToolHandler for UnifiedExecHandler {
}
fn get_command(args: &ExecCommandArgs) -> Vec<String> {
let shell = get_shell_by_model_provided_path(&PathBuf::from(args.shell.clone()));
let shell = if let Some(shell_str) = &args.shell {
get_shell_by_model_provided_path(&PathBuf::from(shell_str))
} else {
default_user_shell()
};
shell.derive_exec_args(&args.cmd, args.login)
}
@@ -273,3 +291,65 @@ fn format_response(response: &UnifiedExecResponse) -> String {
sections.join("\n")
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_get_command_uses_default_shell_when_unspecified() {
let json = r#"{"cmd": "echo hello"}"#;
let args: ExecCommandArgs =
serde_json::from_str(json).expect("deserialize ExecCommandArgs");
assert!(args.shell.is_none());
let command = get_command(&args);
assert_eq!(command.len(), 3);
assert_eq!(command[2], "echo hello");
}
#[test]
fn test_get_command_respects_explicit_bash_shell() {
let json = r#"{"cmd": "echo hello", "shell": "/bin/bash"}"#;
let args: ExecCommandArgs =
serde_json::from_str(json).expect("deserialize ExecCommandArgs");
assert_eq!(args.shell.as_deref(), Some("/bin/bash"));
let command = get_command(&args);
assert_eq!(command[2], "echo hello");
}
#[test]
fn test_get_command_respects_explicit_powershell_shell() {
let json = r#"{"cmd": "echo hello", "shell": "powershell"}"#;
let args: ExecCommandArgs =
serde_json::from_str(json).expect("deserialize ExecCommandArgs");
assert_eq!(args.shell.as_deref(), Some("powershell"));
let command = get_command(&args);
assert_eq!(command[2], "echo hello");
}
#[test]
fn test_get_command_respects_explicit_cmd_shell() {
let json = r#"{"cmd": "echo hello", "shell": "cmd"}"#;
let args: ExecCommandArgs =
serde_json::from_str(json).expect("deserialize ExecCommandArgs");
assert_eq!(args.shell.as_deref(), Some("cmd"));
let command = get_command(&args);
assert_eq!(command[2], "echo hello");
}
}
+3
View File
@@ -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(
@@ -540,19 +554,21 @@ impl UnifiedExecSessionManager {
let env = apply_unified_exec_env(create_env(&context.turn.shell_environment_policy));
let mut orchestrator = ToolOrchestrator::new();
let mut runtime = UnifiedExecRuntime::new(self);
let approval_requirement = create_approval_requirement_for_command(
&context.turn.exec_policy,
command,
context.turn.approval_policy,
&context.turn.sandbox_policy,
SandboxPermissions::from(with_escalated_permissions.unwrap_or(false)),
)
.await;
let req = UnifiedExecToolRequest::new(
command.to_vec(),
cwd,
env,
with_escalated_permissions,
justification,
create_approval_requirement_for_command(
&context.turn.exec_policy,
command,
context.turn.approval_policy,
&context.turn.sandbox_policy,
SandboxPermissions::from(with_escalated_permissions.unwrap_or(false)),
),
approval_requirement,
);
let tool_ctx = ToolCtx {
session: context.session.as_ref(),
@@ -633,9 +649,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 +662,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.
+10
View File
@@ -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::*;
+10
View File
@@ -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",
+5 -1
View File
@@ -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
}
}
}
}
+109 -12
View File
@@ -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<()> {
+70
View File
@@ -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(()));
+13 -6
View File
@@ -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
+29 -14
View File
@@ -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
+229 -32
View File
@@ -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(()));
@@ -168,6 +295,7 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> {
let call_id = "uexec-begin-event";
let args = json!({
"shell": "bash".to_string(),
"cmd": "/bin/echo hello unified exec".to_string(),
"yield_time_ms": 250,
});
@@ -209,14 +337,8 @@ async fn unified_exec_emits_exec_command_begin_event() -> Result<()> {
})
.await;
assert_eq!(
begin_event.command,
vec![
"/bin/bash".to_string(),
"-lc".to_string(),
"/bin/echo hello unified exec".to_string()
]
);
assert_command(&begin_event.command, "-lc", "/bin/echo hello unified exec");
assert_eq!(begin_event.cwd, cwd.path());
wait_for_event(&codex, |event| matches!(event, EventMsg::TaskComplete(_))).await;
@@ -224,6 +346,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<()> {
@@ -579,6 +777,7 @@ async fn unified_exec_emits_begin_for_write_stdin() -> Result<()> {
let open_call_id = "uexec-open-for-begin";
let open_args = json!({
"shell": "bash".to_string(),
"cmd": "bash -i".to_string(),
"yield_time_ms": 200,
});
@@ -640,14 +839,7 @@ async fn unified_exec_emits_begin_for_write_stdin() -> Result<()> {
})
.await;
assert_eq!(
begin_event.command,
vec![
"/bin/bash".to_string(),
"-lc".to_string(),
"bash -i".to_string()
]
);
assert_command(&begin_event.command, "-lc", "bash -i");
assert_eq!(
begin_event.interaction_input,
Some("echo hello".to_string())
@@ -681,6 +873,7 @@ async fn unified_exec_emits_begin_event_for_write_stdin_requests() -> Result<()>
let open_call_id = "uexec-open-session";
let open_args = json!({
"shell": "bash".to_string(),
"cmd": "bash -i".to_string(),
"yield_time_ms": 250,
});
@@ -756,14 +949,9 @@ async fn unified_exec_emits_begin_event_for_write_stdin_requests() -> Result<()>
.iter()
.find(|ev| ev.call_id == open_call_id)
.expect("missing exec_command begin");
assert_eq!(
open_event.command,
vec![
"/bin/bash".to_string(),
"-lc".to_string(),
"bash -i".to_string()
]
);
assert_command(&open_event.command, "-lc", "bash -i");
assert!(
open_event.interaction_input.is_none(),
"startup begin events should not include interaction input"
@@ -774,14 +962,9 @@ async fn unified_exec_emits_begin_event_for_write_stdin_requests() -> Result<()>
.iter()
.find(|ev| ev.call_id == poll_call_id)
.expect("missing write_stdin begin");
assert_eq!(
poll_event.command,
vec![
"/bin/bash".to_string(),
"-lc".to_string(),
"bash -i".to_string()
]
);
assert_command(&poll_event.command, "-lc", "bash -i");
assert!(
poll_event.interaction_input.is_none(),
"poll begin events should omit interaction input"
@@ -1918,3 +2101,17 @@ async fn unified_exec_prunes_exited_sessions_first() -> Result<()> {
Ok(())
}
fn assert_command(command: &[String], expected_args: &str, expected_cmd: &str) {
assert_eq!(command.len(), 3);
let shell_path = &command[0];
assert!(
shell_path == "/bin/bash"
|| shell_path == "/usr/bin/bash"
|| shell_path == "/usr/local/bin/bash"
|| shell_path.ends_with("/bash"),
"unexpected bash path: {shell_path}"
);
assert_eq!(command[1], expected_args);
assert_eq!(command[2], expected_cmd);
}
+1
View File
@@ -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; {
+1
View File
@@ -78,6 +78,7 @@ mod stopwatch;
const CODEX_EXECVE_WRAPPER_EXE_NAME: &str = "codex-execve-wrapper";
#[derive(Parser)]
#[clap(version)]
struct McpServerCli {
/// Executable to delegate execve(2) calls to in Bash.
#[arg(long = "execve")]
+38
View File
@@ -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 {
+225 -80
View File
@@ -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);
}
}
+1
View File
@@ -28,3 +28,4 @@ thiserror = { workspace = true }
[dev-dependencies]
pretty_assertions = { workspace = true }
tempfile = { workspace = true }
+226
View 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")
"#
);
}
}
+3
View File
@@ -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;
+27
View File
@@ -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) {
+78 -35
View File
@@ -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(())
}
+13 -16
View File
@@ -8,11 +8,10 @@ use std::time::Instant;
use crate::pkce::PkceCodes;
use crate::server::ServerOptions;
use std::io::Write;
use std::io::{self};
use std::io;
const ANSI_YELLOW: &str = "\x1b[93m";
const ANSI_BOLD: &str = "\x1b[1m";
const ANSI_BLUE: &str = "\x1b[94m";
const ANSI_GRAY: &str = "\x1b[90m";
const ANSI_RESET: &str = "\x1b[0m";
#[derive(Deserialize)]
@@ -138,14 +137,16 @@ async fn poll_for_token(
}
}
fn print_colored_warning_device_code() {
let mut stdout = io::stdout().lock();
let _ = write!(
stdout,
"{ANSI_YELLOW}{ANSI_BOLD}Only use device code authentication when browser login is not available.{ANSI_RESET}{ANSI_YELLOW}\n\
{ANSI_BOLD}Keep the code secret; do not share it.{ANSI_RESET}{ANSI_RESET}\n\n"
fn print_device_code_prompt(code: &str) {
println!(
"\nWelcome to Codex [v{ANSI_GRAY}{version}{ANSI_RESET}]\n{ANSI_GRAY}OpenAI's command-line coding agent{ANSI_RESET}\n\
\nFollow these steps to sign in with ChatGPT using device code authorization:\n\
\n1. Open this link in your browser\n {ANSI_BLUE}https://auth.openai.com/codex/device{ANSI_RESET}\n\
\n2. Enter this one-time code {ANSI_GRAY}(expires in 15 minutes){ANSI_RESET}\n {ANSI_BLUE}{code}{ANSI_RESET}\n\
\n{ANSI_GRAY}Device codes are a common phishing target. Never share this code.{ANSI_RESET}\n",
version = env!("CARGO_PKG_VERSION"),
code = code
);
let _ = stdout.flush();
}
/// Full device code login flow.
@@ -153,13 +154,9 @@ pub async fn run_device_code_login(opts: ServerOptions) -> std::io::Result<()> {
let client = reqwest::Client::new();
let base_url = opts.issuer.trim_end_matches('/');
let api_base_url = format!("{}/api/accounts", opts.issuer.trim_end_matches('/'));
print_colored_warning_device_code();
let uc = request_user_code(&client, &api_base_url, &opts.client_id).await?;
println!(
"To authenticate:\n 1. Open in your browser: {ANSI_BOLD}https://auth.openai.com/codex/device{ANSI_RESET}\n 2. Enter the one-time code below within 15 minutes:\n\n {ANSI_BOLD}{}{ANSI_RESET}\n",
uc.user_code
);
print_device_code_prompt(&uc.user_code);
let code_resp = poll_for_token(
&client,
+27
View File
@@ -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:
+10
View File
@@ -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"
}
+17
View File
@@ -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![
+31 -2
View File
@@ -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.
+1
View File
@@ -91,6 +91,7 @@ unicode-width = { workspace = true }
url = { workspace = true }
codex-windows-sandbox = { workspace = true }
tokio-util = { workspace = true, features = ["time"] }
[target.'cfg(unix)'.dependencies]
libc = { workspace = true }
+17
View File
@@ -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 {
+100 -4
View File
@@ -101,6 +101,7 @@ pub(crate) struct ChatComposer {
dismissed_file_popup_token: Option<String>,
current_file_query: Option<String>,
pending_pastes: Vec<(String, String)>,
large_paste_counters: HashMap<usize, usize>,
has_focus: bool,
attached_images: Vec<AttachedImage>,
placeholder_text: String,
@@ -113,6 +114,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.
@@ -146,6 +148,7 @@ impl ChatComposer {
dismissed_file_popup_token: None,
current_file_query: None,
pending_pastes: Vec::new(),
large_paste_counters: HashMap::new(),
has_focus: has_input_focus,
attached_images: Vec::new(),
placeholder_text,
@@ -156,6 +159,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);
@@ -220,7 +224,7 @@ impl ChatComposer {
pub fn handle_paste(&mut self, pasted: String) -> bool {
let char_count = pasted.chars().count();
if char_count > LARGE_PASTE_CHAR_THRESHOLD {
let placeholder = format!("[Pasted Content {char_count} chars]");
let placeholder = self.next_large_paste_placeholder(char_count);
self.textarea.insert_element(&placeholder);
self.pending_pastes.push((placeholder, pasted));
} else if char_count > 1 && self.handle_paste_image_path(pasted.clone()) {
@@ -360,6 +364,17 @@ impl ChatComposer {
self.set_has_focus(has_focus);
}
fn next_large_paste_placeholder(&mut self, char_count: usize) -> String {
let base = format!("[Pasted Content {char_count} chars]");
let next_suffix = self.large_paste_counters.entry(char_count).or_insert(0);
*next_suffix += 1;
if *next_suffix == 1 {
base
} else {
format!("{base} #{next_suffix}")
}
}
pub(crate) fn insert_str(&mut self, text: &str) {
self.textarea.insert_str(text);
self.sync_command_popup();
@@ -1387,6 +1402,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 +1533,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) {
@@ -2665,6 +2684,83 @@ mod tests {
);
}
#[test]
fn deleting_duplicate_length_pastes_removes_only_target() {
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyModifiers;
let (tx, _rx) = unbounded_channel::<AppEvent>();
let sender = AppEventSender::new(tx);
let mut composer = ChatComposer::new(
true,
sender,
false,
"Ask Codex to do anything".to_string(),
false,
);
let paste = "x".repeat(LARGE_PASTE_CHAR_THRESHOLD + 4);
let placeholder_base = format!("[Pasted Content {} chars]", paste.chars().count());
let placeholder_second = format!("{placeholder_base} #2");
composer.handle_paste(paste.clone());
composer.handle_paste(paste.clone());
assert_eq!(
composer.textarea.text(),
format!("{placeholder_base}{placeholder_second}")
);
assert_eq!(composer.pending_pastes.len(), 2);
composer.textarea.set_cursor(composer.textarea.text().len());
composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE));
assert_eq!(composer.textarea.text(), placeholder_base);
assert_eq!(composer.pending_pastes.len(), 1);
assert_eq!(composer.pending_pastes[0].0, placeholder_base);
assert_eq!(composer.pending_pastes[0].1, paste);
}
#[test]
fn large_paste_numbering_does_not_reuse_after_deletion() {
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyModifiers;
let (tx, _rx) = unbounded_channel::<AppEvent>();
let sender = AppEventSender::new(tx);
let mut composer = ChatComposer::new(
true,
sender,
false,
"Ask Codex to do anything".to_string(),
false,
);
let paste = "x".repeat(LARGE_PASTE_CHAR_THRESHOLD + 4);
let base = format!("[Pasted Content {} chars]", paste.chars().count());
let second = format!("{base} #2");
let third = format!("{base} #3");
composer.handle_paste(paste.clone());
composer.handle_paste(paste.clone());
assert_eq!(composer.textarea.text(), format!("{base}{second}"));
composer.textarea.set_cursor(base.len());
composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE));
assert_eq!(composer.textarea.text(), second);
assert_eq!(composer.pending_pastes.len(), 1);
assert_eq!(composer.pending_pastes[0].0, second);
composer.textarea.set_cursor(composer.textarea.text().len());
composer.handle_paste(paste);
assert_eq!(composer.textarea.text(), format!("{second}{third}"));
assert_eq!(composer.pending_pastes.len(), 2);
assert_eq!(composer.pending_pastes[0].0, second);
assert_eq!(composer.pending_pastes[1].0, third);
}
#[test]
fn test_partial_placeholder_deletion() {
use crossterm::event::KeyCode;
+41 -5
View File
@@ -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),
},
);
}
+13 -3
View File
@@ -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 "
+60 -36
View File
@@ -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
@@ -1360,7 +1383,9 @@ impl ChatWidget {
modifiers,
kind: KeyEventKind::Press,
..
} if modifiers.contains(KeyModifiers::CONTROL) && c.eq_ignore_ascii_case(&'v') => {
} if modifiers.intersects(KeyModifiers::CONTROL | KeyModifiers::ALT)
&& c.eq_ignore_ascii_case(&'v') =>
{
match paste_image_to_temp_png() {
Ok((path, info)) => {
self.attach_image(
@@ -1812,7 +1837,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 +2917,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 +2973,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 +3003,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 +3047,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 +3252,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,
},
}));
})],
+101 -12
View File
@@ -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:?}"),
+110 -4
View File
@@ -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")
);
}
}
+1
View File
@@ -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;
-15
View File
@@ -1075,7 +1075,6 @@ mod tests {
ConversationItem {
path: PathBuf::from(path),
head: head_with_ts_and_user_text(ts, &[preview]),
tail: Vec::new(),
created_at: Some(ts.to_string()),
updated_at: Some(ts.to_string()),
}
@@ -1150,14 +1149,12 @@ mod tests {
let a = ConversationItem {
path: PathBuf::from("/tmp/a.jsonl"),
head: head_with_ts_and_user_text("2025-01-01T00:00:00Z", &["A"]),
tail: Vec::new(),
created_at: Some("2025-01-01T00:00:00Z".into()),
updated_at: Some("2025-01-01T00:00:00Z".into()),
};
let b = ConversationItem {
path: PathBuf::from("/tmp/b.jsonl"),
head: head_with_ts_and_user_text("2025-01-02T00:00:00Z", &["B"]),
tail: Vec::new(),
created_at: Some("2025-01-02T00:00:00Z".into()),
updated_at: Some("2025-01-02T00:00:00Z".into()),
};
@@ -1171,21 +1168,9 @@ mod tests {
#[test]
fn row_uses_tail_timestamp_for_updated_at() {
let head = head_with_ts_and_user_text("2025-01-01T00:00:00Z", &["Hello"]);
let tail = vec![json!({
"timestamp": "2025-01-01T01:00:00Z",
"type": "message",
"role": "assistant",
"content": [
{
"type": "output_text",
"text": "hi",
}
],
})];
let item = ConversationItem {
path: PathBuf::from("/tmp/a.jsonl"),
head,
tail,
created_at: Some("2025-01-01T00:00:00Z".into()),
updated_at: Some("2025-01-01T01:00:00Z".into()),
};
+164
View 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);
}
}
+1
View File
@@ -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;
+31 -100
View File
@@ -9,8 +9,6 @@ use std::pin::Pin;
use std::sync::Arc;
use std::sync::atomic::AtomicBool;
use std::sync::atomic::Ordering;
use std::time::Duration;
use std::time::Instant;
use crossterm::Command;
use crossterm::SynchronizedUpdate;
@@ -32,10 +30,13 @@ use ratatui::crossterm::execute;
use ratatui::crossterm::terminal::disable_raw_mode;
use ratatui::crossterm::terminal::enable_raw_mode;
use ratatui::layout::Offset;
use ratatui::layout::Rect;
use ratatui::text::Line;
use tokio::select;
use tokio::sync::broadcast;
use tokio_stream::Stream;
pub use self::frame_requester::FrameRequester;
use crate::custom_terminal;
use crate::custom_terminal::Terminal as CustomTerminal;
#[cfg(unix)]
@@ -43,6 +44,7 @@ use crate::tui::job_control::SUSPEND_KEY;
#[cfg(unix)]
use crate::tui::job_control::SuspendContext;
mod frame_requester;
#[cfg(unix)]
mod job_control;
@@ -159,8 +161,8 @@ pub enum TuiEvent {
}
pub struct Tui {
frame_schedule_tx: tokio::sync::mpsc::UnboundedSender<Instant>,
draw_tx: tokio::sync::broadcast::Sender<()>,
frame_requester: FrameRequester,
draw_tx: broadcast::Sender<()>,
pub(crate) terminal: Terminal,
pending_history_lines: Vec<Line<'static>>,
alt_saved_viewport: Option<ratatui::layout::Rect>,
@@ -173,36 +175,10 @@ pub struct Tui {
enhanced_keys_supported: bool,
}
#[derive(Clone, Debug)]
pub struct FrameRequester {
frame_schedule_tx: tokio::sync::mpsc::UnboundedSender<Instant>,
}
impl FrameRequester {
pub fn schedule_frame(&self) {
let _ = self.frame_schedule_tx.send(Instant::now());
}
pub fn schedule_frame_in(&self, dur: Duration) {
let _ = self.frame_schedule_tx.send(Instant::now() + dur);
}
}
#[cfg(test)]
impl FrameRequester {
/// Create a no-op frame requester for tests.
pub(crate) fn test_dummy() -> Self {
let (tx, _rx) = tokio::sync::mpsc::unbounded_channel();
FrameRequester {
frame_schedule_tx: tx,
}
}
}
impl Tui {
pub fn new(terminal: Terminal) -> Self {
let (frame_schedule_tx, frame_schedule_rx) = tokio::sync::mpsc::unbounded_channel();
let (draw_tx, _) = tokio::sync::broadcast::channel(1);
spawn_frame_scheduler(frame_schedule_rx, draw_tx.clone());
let (draw_tx, _) = broadcast::channel(1);
let frame_requester = FrameRequester::new(draw_tx.clone());
// Detect keyboard enhancement support before any EventStream is created so the
// crossterm poller can acquire its lock without contention.
@@ -212,7 +188,7 @@ impl Tui {
let _ = crate::terminal_palette::default_colors();
Self {
frame_schedule_tx,
frame_requester,
draw_tx,
terminal,
pending_history_lines: vec![],
@@ -226,9 +202,7 @@ impl Tui {
}
pub fn frame_requester(&self) -> FrameRequester {
FrameRequester {
frame_schedule_tx: self.frame_schedule_tx.clone(),
}
self.frame_requester.clone()
}
pub fn enhanced_keys_supported(&self) -> bool {
@@ -362,34 +336,14 @@ impl Tui {
// Precompute any viewport updates that need a cursor-position query before entering
// the synchronized update, to avoid racing with the event reader.
let mut pending_viewport_area: Option<ratatui::layout::Rect> = None;
{
let terminal = &mut self.terminal;
let screen_size = terminal.size()?;
let last_known_screen_size = terminal.last_known_screen_size;
if screen_size != last_known_screen_size
&& let Ok(cursor_pos) = terminal.get_cursor_position()
{
let last_known_cursor_pos = terminal.last_known_cursor_pos;
// If we resized AND the cursor moved, we adjust the viewport area to keep the
// cursor in the same position. This is a heuristic that seems to work well
// at least in iTerm2.
if cursor_pos.y != last_known_cursor_pos.y {
let cursor_delta = cursor_pos.y as i32 - last_known_cursor_pos.y as i32;
let new_viewport_area = terminal.viewport_area.offset(Offset {
x: 0,
y: cursor_delta,
});
pending_viewport_area = Some(new_viewport_area);
}
}
}
let mut pending_viewport_area = self.pending_viewport_area()?;
stdout().sync_update(|_| {
#[cfg(unix)]
if let Some(prepared) = prepared_resume.take() {
prepared.apply(&mut self.terminal)?;
}
let terminal = &mut self.terminal;
if let Some(new_area) = pending_viewport_area.take() {
terminal.set_viewport_area(new_area);
@@ -440,51 +394,28 @@ impl Tui {
})
})?
}
}
/// Spawn background scheduler to coalesce frame requests and emit draws at deadlines.
fn spawn_frame_scheduler(
frame_schedule_rx: tokio::sync::mpsc::UnboundedReceiver<Instant>,
draw_tx: tokio::sync::broadcast::Sender<()>,
) {
tokio::spawn(async move {
use tokio::select;
use tokio::time::Instant as TokioInstant;
use tokio::time::sleep_until;
let mut rx = frame_schedule_rx;
let mut next_deadline: Option<Instant> = None;
loop {
let target = next_deadline
.unwrap_or_else(|| Instant::now() + Duration::from_secs(60 * 60 * 24 * 365));
let sleep_fut = sleep_until(TokioInstant::from_std(target));
tokio::pin!(sleep_fut);
select! {
recv = rx.recv() => {
match recv {
Some(at) => {
if next_deadline.is_none_or(|cur| at < cur) {
next_deadline = Some(at);
}
// Do not send a draw immediately here. By continuing the loop,
// we recompute the sleep target so the draw fires once via the
// sleep branch, coalescing multiple requests into a single draw.
continue;
}
None => break,
}
}
_ = &mut sleep_fut => {
if next_deadline.is_some() {
next_deadline = None;
let _ = draw_tx.send(());
}
}
fn pending_viewport_area(&mut self) -> Result<Option<Rect>> {
let terminal = &mut self.terminal;
let screen_size = terminal.size()?;
let last_known_screen_size = terminal.last_known_screen_size;
if screen_size != last_known_screen_size
&& let Ok(cursor_pos) = terminal.get_cursor_position()
{
let last_known_cursor_pos = terminal.last_known_cursor_pos;
// If we resized AND the cursor moved, we adjust the viewport area to keep the
// cursor in the same position. This is a heuristic that seems to work well
// at least in iTerm2.
if cursor_pos.y != last_known_cursor_pos.y {
let offset = Offset {
x: 0,
y: cursor_pos.y as i32 - last_known_cursor_pos.y as i32,
};
return Ok(Some(terminal.viewport_area.offset(offset)));
}
}
});
Ok(None)
}
}
/// Command that emits an OSC 9 desktop notification with a message.
+249
View File
@@ -0,0 +1,249 @@
//! Frame draw scheduling utilities for the TUI.
//!
//! This module exposes [`FrameRequester`], a lightweight handle that widgets and
//! background tasks can clone to request future redraws of the TUI.
//!
//! Internally it spawns a [`FrameScheduler`] task that coalesces many requests
//! into a single notification on a broadcast channel used by the main TUI event
//! loop. This keeps animations and status updates smooth without redrawing more
//! often than necessary.
//!
//! This follows the actor-style design from
//! [“Actors with Tokio”](https://ryhl.io/blog/actors-with-tokio/), with a
//! dedicated scheduler task and lightweight request handles.
use std::time::Duration;
use std::time::Instant;
use tokio::sync::broadcast;
use tokio::sync::mpsc;
/// A requester for scheduling future frame draws on the TUI event loop.
///
/// This is the handler side of an actor/handler pair with `FrameScheduler`, which coalesces
/// multiple frame requests into a single draw operation.
///
/// Clones of this type can be freely shared across tasks to make it possible to trigger frame draws
/// from anywhere in the TUI code.
#[derive(Clone, Debug)]
pub struct FrameRequester {
frame_schedule_tx: mpsc::UnboundedSender<Instant>,
}
impl FrameRequester {
/// Create a new FrameRequester and spawn its associated FrameScheduler task.
///
/// The provided `draw_tx` is used to notify the TUI event loop of scheduled draws.
pub fn new(draw_tx: broadcast::Sender<()>) -> Self {
let (tx, rx) = mpsc::unbounded_channel();
let scheduler = FrameScheduler::new(rx, draw_tx);
tokio::spawn(scheduler.run());
Self {
frame_schedule_tx: tx,
}
}
/// Schedule a frame draw as soon as possible.
pub fn schedule_frame(&self) {
let _ = self.frame_schedule_tx.send(Instant::now());
}
/// Schedule a frame draw to occur after the specified duration.
pub fn schedule_frame_in(&self, dur: Duration) {
let _ = self.frame_schedule_tx.send(Instant::now() + dur);
}
}
#[cfg(test)]
impl FrameRequester {
/// Create a no-op frame requester for tests.
pub(crate) fn test_dummy() -> Self {
let (tx, _rx) = mpsc::unbounded_channel();
FrameRequester {
frame_schedule_tx: tx,
}
}
}
/// A scheduler for coalescing frame draw requests and notifying the TUI event loop.
///
/// This type is internal to `FrameRequester` and is spawned as a task to handle scheduling logic.
struct FrameScheduler {
receiver: mpsc::UnboundedReceiver<Instant>,
draw_tx: broadcast::Sender<()>,
}
impl FrameScheduler {
/// Create a new FrameScheduler with the provided receiver and draw notification sender.
fn new(receiver: mpsc::UnboundedReceiver<Instant>, draw_tx: broadcast::Sender<()>) -> Self {
Self { receiver, draw_tx }
}
/// Run the scheduling loop, coalescing frame requests and notifying the TUI event loop.
///
/// This method runs indefinitely until all senders are dropped. A single draw notification
/// is sent for multiple requests scheduled before the next draw deadline.
async fn run(mut self) {
const ONE_YEAR: Duration = Duration::from_secs(60 * 60 * 24 * 365);
let mut next_deadline: Option<Instant> = None;
loop {
let target = next_deadline.unwrap_or_else(|| Instant::now() + ONE_YEAR);
let deadline = tokio::time::sleep_until(target.into());
tokio::pin!(deadline);
tokio::select! {
draw_at = self.receiver.recv() => {
let Some(draw_at) = draw_at else {
// All senders dropped; exit the scheduler.
break
};
next_deadline = Some(next_deadline.map_or(draw_at, |cur| cur.min(draw_at)));
// Do not send a draw immediately here. By continuing the loop,
// we recompute the sleep target so the draw fires once via the
// sleep branch, coalescing multiple requests into a single draw.
continue;
}
_ = &mut deadline => {
if next_deadline.is_some() {
next_deadline = None;
let _ = self.draw_tx.send(());
}
}
}
}
}
}
#[cfg(test)]
mod tests {
use super::*;
use tokio::time;
use tokio_util::time::FutureExt;
#[tokio::test(flavor = "current_thread", start_paused = true)]
async fn test_schedule_frame_immediate_triggers_once() {
let (draw_tx, mut draw_rx) = broadcast::channel(16);
let requester = FrameRequester::new(draw_tx);
requester.schedule_frame();
// Advance time minimally to let the scheduler process and hit the deadline == now.
time::advance(Duration::from_millis(1)).await;
// First draw should arrive.
let first = draw_rx
.recv()
.timeout(Duration::from_millis(50))
.await
.expect("timed out waiting for first draw");
assert!(first.is_ok(), "broadcast closed unexpectedly");
// No second draw should arrive.
let second = draw_rx.recv().timeout(Duration::from_millis(20)).await;
assert!(second.is_err(), "unexpected extra draw received");
}
#[tokio::test(flavor = "current_thread", start_paused = true)]
async fn test_schedule_frame_in_triggers_at_delay() {
let (draw_tx, mut draw_rx) = broadcast::channel(16);
let requester = FrameRequester::new(draw_tx);
requester.schedule_frame_in(Duration::from_millis(50));
// Advance less than the delay: no draw yet.
time::advance(Duration::from_millis(30)).await;
let early = draw_rx.recv().timeout(Duration::from_millis(10)).await;
assert!(early.is_err(), "draw fired too early");
// Advance past the deadline: one draw should fire.
time::advance(Duration::from_millis(25)).await;
let first = draw_rx
.recv()
.timeout(Duration::from_millis(50))
.await
.expect("timed out waiting for scheduled draw");
assert!(first.is_ok(), "broadcast closed unexpectedly");
// No second draw should arrive.
let second = draw_rx.recv().timeout(Duration::from_millis(20)).await;
assert!(second.is_err(), "unexpected extra draw received");
}
#[tokio::test(flavor = "current_thread", start_paused = true)]
async fn test_coalesces_multiple_requests_into_single_draw() {
let (draw_tx, mut draw_rx) = broadcast::channel(16);
let requester = FrameRequester::new(draw_tx);
// Schedule multiple immediate requests close together.
requester.schedule_frame();
requester.schedule_frame();
requester.schedule_frame();
// Allow the scheduler to process and hit the coalesced deadline.
time::advance(Duration::from_millis(1)).await;
// Expect only a single draw notification despite three requests.
let first = draw_rx
.recv()
.timeout(Duration::from_millis(50))
.await
.expect("timed out waiting for coalesced draw");
assert!(first.is_ok(), "broadcast closed unexpectedly");
// No additional draw should be sent for the same coalesced batch.
let second = draw_rx.recv().timeout(Duration::from_millis(20)).await;
assert!(second.is_err(), "unexpected extra draw received");
}
#[tokio::test(flavor = "current_thread", start_paused = true)]
async fn test_coalesces_mixed_immediate_and_delayed_requests() {
let (draw_tx, mut draw_rx) = broadcast::channel(16);
let requester = FrameRequester::new(draw_tx);
// Schedule a delayed draw and then an immediate one; should coalesce and fire at the earliest (immediate).
requester.schedule_frame_in(Duration::from_millis(100));
requester.schedule_frame();
time::advance(Duration::from_millis(1)).await;
let first = draw_rx
.recv()
.timeout(Duration::from_millis(50))
.await
.expect("timed out waiting for coalesced immediate draw");
assert!(first.is_ok(), "broadcast closed unexpectedly");
// The later delayed request should have been coalesced into the earlier one; no second draw.
let second = draw_rx.recv().timeout(Duration::from_millis(120)).await;
assert!(second.is_err(), "unexpected extra draw received");
}
#[tokio::test(flavor = "current_thread", start_paused = true)]
async fn test_multiple_delayed_requests_coalesce_to_earliest() {
let (draw_tx, mut draw_rx) = broadcast::channel(16);
let requester = FrameRequester::new(draw_tx);
// Schedule multiple delayed draws; they should coalesce to the earliest (10ms).
requester.schedule_frame_in(Duration::from_millis(100));
requester.schedule_frame_in(Duration::from_millis(20));
requester.schedule_frame_in(Duration::from_millis(120));
// Advance to just before the earliest deadline: no draw yet.
time::advance(Duration::from_millis(10)).await;
let early = draw_rx.recv().timeout(Duration::from_millis(10)).await;
assert!(early.is_err(), "draw fired too early");
// Advance past the earliest deadline: one draw should fire.
time::advance(Duration::from_millis(20)).await;
let first = draw_rx
.recv()
.timeout(Duration::from_millis(50))
.await
.expect("timed out waiting for earliest coalesced draw");
assert!(first.is_ok(), "broadcast closed unexpectedly");
// No additional draw should fire for the later delayed requests.
let second = draw_rx.recv().timeout(Duration::from_millis(120)).await;
assert!(second.is_err(), "unexpected extra draw received");
}
}
+319 -5
View File
@@ -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(())
}