Merge remote-tracking branch 'origin/main' into jchu/codex-cli-owner-nudge-analytics-2026.04.22

This commit is contained in:
James Chu
2026-04-27 18:32:37 -04:00
75 changed files with 3614 additions and 2516 deletions
@@ -1028,21 +1028,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -392,21 +392,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -177,21 +177,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -177,21 +177,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -1199,21 +1199,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -731,21 +731,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -8597,21 +8597,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -5116,21 +5116,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -146,21 +146,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -184,21 +184,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -177,21 +177,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -183,21 +183,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -569,21 +569,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -257,21 +257,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -569,21 +569,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -209,21 +209,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -569,21 +569,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -218,21 +218,6 @@
"title": "MinimalFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
"enum": [
"current_working_directory"
],
"type": "string"
}
},
"required": [
"kind"
],
"title": "CurrentWorkingDirectoryFileSystemSpecialPath",
"type": "object"
},
{
"properties": {
"kind": {
@@ -2,4 +2,4 @@
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
export type FileSystemSpecialPath = { "kind": "root" } | { "kind": "minimal" } | { "kind": "current_working_directory" } | { "kind": "project_roots", subpath: string | null, } | { "kind": "tmpdir" } | { "kind": "slash_tmp" } | { "kind": "unknown", path: string, subpath: string | null, };
export type FileSystemSpecialPath = { "kind": "root" } | { "kind": "minimal" } | { "kind": "project_roots", subpath: string | null, } | { "kind": "tmpdir" } | { "kind": "slash_tmp" } | { "kind": "unknown", path: string, subpath: string | null, };
@@ -1434,7 +1434,7 @@ v2_enum_from_core!(
pub enum FileSystemSpecialPath {
Root,
Minimal,
CurrentWorkingDirectory,
#[serde(alias = "current_working_directory")]
ProjectRoots {
subpath: Option<PathBuf>,
},
@@ -1451,7 +1451,6 @@ impl From<CoreFileSystemSpecialPath> for FileSystemSpecialPath {
match value {
CoreFileSystemSpecialPath::Root => Self::Root,
CoreFileSystemSpecialPath::Minimal => Self::Minimal,
CoreFileSystemSpecialPath::CurrentWorkingDirectory => Self::CurrentWorkingDirectory,
CoreFileSystemSpecialPath::ProjectRoots { subpath } => Self::ProjectRoots { subpath },
CoreFileSystemSpecialPath::Tmpdir => Self::Tmpdir,
CoreFileSystemSpecialPath::SlashTmp => Self::SlashTmp,
@@ -1465,7 +1464,6 @@ impl From<FileSystemSpecialPath> for CoreFileSystemSpecialPath {
match value {
FileSystemSpecialPath::Root => Self::Root,
FileSystemSpecialPath::Minimal => Self::Minimal,
FileSystemSpecialPath::CurrentWorkingDirectory => Self::CurrentWorkingDirectory,
FileSystemSpecialPath::ProjectRoots { subpath } => Self::ProjectRoots { subpath },
FileSystemSpecialPath::Tmpdir => Self::Tmpdir,
FileSystemSpecialPath::SlashTmp => Self::SlashTmp,
@@ -8151,6 +8149,26 @@ mod tests {
.expect_err("zero glob scan depth should fail deserialization");
}
#[test]
fn legacy_current_working_directory_special_path_deserializes_as_project_roots() {
let special_path = serde_json::from_value::<FileSystemSpecialPath>(json!({
"kind": "current_working_directory",
}))
.expect("legacy cwd special path should deserialize");
assert_eq!(
special_path,
FileSystemSpecialPath::ProjectRoots { subpath: None }
);
assert_eq!(
serde_json::to_value(&special_path).expect("serialize special path"),
json!({
"kind": "project_roots",
"subpath": null,
})
);
}
#[test]
fn permissions_request_approval_response_uses_granted_permission_profile_without_macos() {
let read_only_path = if cfg!(windows) {
+2 -2
View File
@@ -909,7 +909,7 @@ Run a standalone command (argv vector) in the server’s sandbox without creatin
"type": "managed",
"fileSystem": { "type": "restricted", "entries": [
{ "path": { "type": "special", "value": { "kind": "root" } }, "access": "read" },
{ "path": { "type": "special", "value": { "kind": "current_working_directory" } }, "access": "write" }
{ "path": { "type": "special", "value": { "kind": "project_roots", "subpath": null } }, "access": "write" }
] },
"network": { "enabled": false }
},
@@ -1255,7 +1255,7 @@ the client can offer session-scoped and/or persistent approval choices.
### Permission requests
The built-in `request_permissions` tool sends an `item/permissions/requestApproval` JSON-RPC request to the client with the requested permission profile. This v2 payload mirrors the command-execution `additionalPermissions` shape: it can request network access and additional filesystem access. The `cwd` field identifies the directory used to resolve cwd-relative permissions such as `:cwd`, `:project_roots`, and relative deny globs.
The built-in `request_permissions` tool sends an `item/permissions/requestApproval` JSON-RPC request to the client with the requested permission profile. This v2 payload mirrors the command-execution `additionalPermissions` shape: it can request network access and additional filesystem access. The `cwd` field identifies the directory used to resolve project-root permissions and relative deny globs.
```json
{
@@ -2,8 +2,8 @@ use crate::codex_message_processor::ApiVersion;
use crate::codex_message_processor::read_rollout_items_from_rollout;
use crate::codex_message_processor::read_summary_from_rollout;
use crate::codex_message_processor::summary_to_thread;
use crate::error_code::INTERNAL_ERROR_CODE;
use crate::error_code::INVALID_REQUEST_ERROR_CODE;
use crate::error_code::internal_error;
use crate::error_code::invalid_request;
use crate::outgoing_message::ClientRequestResult;
use crate::outgoing_message::ThreadScopedOutgoingMessageSender;
use crate::server_request_error::is_turn_transition_server_request_error;
@@ -51,7 +51,6 @@ use codex_app_server_protocol::HookStartedNotification;
use codex_app_server_protocol::InterruptConversationResponse;
use codex_app_server_protocol::ItemCompletedNotification;
use codex_app_server_protocol::ItemStartedNotification;
use codex_app_server_protocol::JSONRPCErrorError;
use codex_app_server_protocol::McpServerElicitationAction;
use codex_app_server_protocol::McpServerElicitationRequestParams;
use codex_app_server_protocol::McpServerElicitationRequestResponse;
@@ -1876,12 +1875,12 @@ pub(crate) async fn apply_bespoke_event_handling(
if let Some(request_id) = pending {
let Some(rollout_path) = conversation.rollout_path() else {
let error = JSONRPCErrorError {
code: INVALID_REQUEST_ERROR_CODE,
message: "thread has no persisted rollout".to_string(),
data: None,
};
outgoing.send_error(request_id, error).await;
outgoing
.send_error(
request_id,
invalid_request("thread has no persisted rollout"),
)
.await;
return;
};
let response = match read_summary_from_rollout(
@@ -1912,29 +1911,29 @@ pub(crate) async fn apply_bespoke_event_handling(
ThreadRollbackResponse { thread }
}
Err(err) => {
let error = JSONRPCErrorError {
code: INTERNAL_ERROR_CODE,
message: format!(
"failed to load rollout `{}`: {err}",
rollout_path.display()
),
data: None,
};
outgoing.send_error(request_id.clone(), error).await;
outgoing
.send_error(
request_id.clone(),
internal_error(format!(
"failed to load rollout `{}`: {err}",
rollout_path.display()
)),
)
.await;
return;
}
}
}
Err(err) => {
let error = JSONRPCErrorError {
code: INTERNAL_ERROR_CODE,
message: format!(
"failed to load rollout `{}`: {err}",
rollout_path.display()
),
data: None,
};
outgoing.send_error(request_id.clone(), error).await;
outgoing
.send_error(
request_id.clone(),
internal_error(format!(
"failed to load rollout `{}`: {err}",
rollout_path.display()
)),
)
.await;
return;
}
};
@@ -2332,14 +2331,7 @@ async fn handle_thread_rollback_failed(
if let Some(request_id) = pending_rollback {
outgoing
.send_error(
request_id,
JSONRPCErrorError {
code: INVALID_REQUEST_ERROR_CODE,
message: message.clone(),
data: None,
},
)
.send_error(request_id, invalid_request(message))
.await;
}
}
@@ -4008,7 +4000,7 @@ mod tests {
file_system: Some(CoreFileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
}],
@@ -4054,7 +4046,7 @@ mod tests {
file_system: Some(CoreFileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
}],
@@ -4104,7 +4096,8 @@ mod tests {
"path": {
"type": "special",
"value": {
"kind": "current_working_directory"
"kind": "project_roots",
"subpath": null
}
},
"access": "write"
File diff suppressed because it is too large Load Diff
@@ -246,7 +246,7 @@ async fn command_exec_accepts_permission_profile() -> Result<()> {
#[cfg(unix)]
#[tokio::test]
async fn command_exec_permission_profile_cwd_uses_command_cwd() -> Result<()> {
async fn command_exec_permission_profile_project_roots_use_command_cwd() -> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
let codex_home = TempDir::new()?;
let command_dir = codex_home.path().join("command-cwd");
@@ -264,7 +264,7 @@ async fn command_exec_permission_profile_cwd_uses_command_cwd() -> Result<()> {
};
entries.push(FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::ProjectRoots { subpath: None },
},
access: FileSystemAccessMode::Write,
});
@@ -298,7 +298,7 @@ async fn command_exec_permission_profile_cwd_uses_command_cwd() -> Result<()> {
let response: CommandExecResponse = to_response(response)?;
assert_eq!(
response.exit_code, 0,
"parent cwd write should fail under command-cwd-scoped profile: {response:?}"
"parent cwd write should fail under command project-root profile: {response:?}"
);
assert_eq!(
std::fs::read_to_string(command_dir.join("child.txt"))?,
@@ -306,7 +306,7 @@ async fn command_exec_permission_profile_cwd_uses_command_cwd() -> Result<()> {
);
assert!(
!codex_home.path().join("parent.txt").exists(),
"permissionProfile :cwd write should not grant the server cwd when command cwd differs"
"permissionProfile :project_roots write should not grant the server cwd when command cwd differs"
);
Ok(())
+37 -7
View File
@@ -98,6 +98,7 @@ use tokio::sync::oneshot;
use tokio::sync::oneshot::error::TryRecvError;
use tokio_tungstenite::tungstenite::Error;
use tokio_tungstenite::tungstenite::Message;
use tokio_util::sync::CancellationToken;
use tracing::instrument;
use tracing::trace;
use tracing::warn;
@@ -1232,7 +1233,8 @@ impl ModelClientSession {
Err(ApiError::Transport(
unauthorized_transport @ TransportError::Http { status, .. },
)) if status == StatusCode::UNAUTHORIZED => {
inference_trace_attempt.record_failed(&unauthorized_transport);
inference_trace_attempt
.record_failed(&unauthorized_transport, /*output_items*/ &[]);
pending_retry = PendingUnauthorizedRetry::from_recovery(
handle_unauthorized(
unauthorized_transport,
@@ -1245,7 +1247,7 @@ impl ModelClientSession {
}
Err(err) => {
let err = map_api_error(err);
inference_trace_attempt.record_failed(&err);
inference_trace_attempt.record_failed(&err, /*output_items*/ &[]);
return Err(err);
}
}
@@ -1372,7 +1374,7 @@ impl ModelClientSession {
.await
.map_err(|err| {
let err = map_api_error(err);
inference_trace_attempt.record_failed(&err);
inference_trace_attempt.record_failed(&err, /*output_items*/ &[]);
err
})?;
let (stream, last_request_rx) = map_response_stream(
@@ -1621,6 +1623,9 @@ fn parent_thread_id_header_value(session_source: &SessionSource) -> Option<Strin
}
}
const RESPONSE_STREAM_CHANNEL_CAPACITY: usize = 1600;
const STREAM_DROPPED_REASON: &str = "response stream dropped before provider terminal event";
fn map_response_stream<S>(
api_stream: S,
session_telemetry: SessionTelemetry,
@@ -1632,15 +1637,28 @@ where
+ Send
+ 'static,
{
let (tx_event, rx_event) = mpsc::channel::<Result<ResponseEvent>>(1600);
let (tx_event, rx_event) =
mpsc::channel::<Result<ResponseEvent>>(RESPONSE_STREAM_CHANNEL_CAPACITY);
let (tx_last_response, rx_last_response) = oneshot::channel::<LastResponse>();
let consumer_dropped = CancellationToken::new();
let consumer_dropped_for_stream = consumer_dropped.clone();
tokio::spawn(async move {
let mut logged_error = false;
let mut tx_last_response = Some(tx_last_response);
let mut items_added: Vec<ResponseItem> = Vec::new();
let mut api_stream = api_stream;
while let Some(event) = api_stream.next().await {
loop {
let event = tokio::select! {
_ = consumer_dropped.cancelled() => {
inference_trace_attempt.record_cancelled(STREAM_DROPPED_REASON, &items_added);
return;
}
event = api_stream.next() => event,
};
let Some(event) = event else {
break;
};
match event {
Ok(ResponseEvent::OutputItemDone(item)) => {
items_added.push(item.clone());
@@ -1649,6 +1667,8 @@ where
.await
.is_err()
{
inference_trace_attempt
.record_cancelled(STREAM_DROPPED_REASON, &items_added);
return;
}
}
@@ -1691,12 +1711,14 @@ where
}
Ok(event) => {
if tx_event.send(Ok(event)).await.is_err() {
inference_trace_attempt
.record_cancelled(STREAM_DROPPED_REASON, &items_added);
return;
}
}
Err(err) => {
let mapped = map_api_error(err);
inference_trace_attempt.record_failed(&mapped);
inference_trace_attempt.record_failed(&mapped, &items_added);
if !logged_error {
session_telemetry.see_event_completed_failed(&mapped);
logged_error = true;
@@ -1707,9 +1729,17 @@ where
}
}
}
inference_trace_attempt
.record_failed("stream closed before response.completed", &items_added);
});
(ResponseStream { rx_event }, rx_last_response)
(
ResponseStream {
rx_event,
consumer_dropped: consumer_dropped_for_stream,
},
rx_last_response,
)
}
/// Handles a 401 response by optionally refreshing ChatGPT tokens once.
+10
View File
@@ -13,6 +13,7 @@ use std::pin::Pin;
use std::task::Context;
use std::task::Poll;
use tokio::sync::mpsc;
use tokio_util::sync::CancellationToken;
/// Review thread system prompt. Edit `core/src/review_prompt.md` to customize.
pub const REVIEW_PROMPT: &str = include_str!("../review_prompt.md");
@@ -175,6 +176,9 @@ fn strip_total_output_header(output: &str) -> Option<(&str, u32)> {
pub struct ResponseStream {
pub(crate) rx_event: mpsc::Receiver<Result<ResponseEvent>>,
/// Signals the mapper task that the consumer stopped polling before the
/// provider stream reached its own terminal event.
pub(crate) consumer_dropped: CancellationToken,
}
impl Stream for ResponseStream {
@@ -185,6 +189,12 @@ impl Stream for ResponseStream {
}
}
impl Drop for ResponseStream {
fn drop(&mut self) {
self.consumer_dropped.cancel();
}
}
#[cfg(test)]
#[path = "client_common_tests.rs"]
mod tests;
+191
View File
@@ -7,17 +7,37 @@ use super::X_CODEX_PARENT_THREAD_ID_HEADER;
use super::X_CODEX_TURN_METADATA_HEADER;
use super::X_CODEX_WINDOW_ID_HEADER;
use super::X_OPENAI_SUBAGENT_HEADER;
use codex_api::ApiError;
use codex_api::ResponseEvent;
use codex_app_server_protocol::AuthMode;
use codex_model_provider::BearerAuthProvider;
use codex_model_provider_info::WireApi;
use codex_model_provider_info::create_oss_provider_with_base_url;
use codex_otel::SessionTelemetry;
use codex_protocol::ThreadId;
use codex_protocol::models::ContentItem;
use codex_protocol::models::ResponseItem;
use codex_protocol::openai_models::ModelInfo;
use codex_protocol::protocol::SessionSource;
use codex_protocol::protocol::SubAgentSource;
use codex_rollout_trace::ExecutionStatus;
use codex_rollout_trace::InferenceTraceAttempt;
use codex_rollout_trace::InferenceTraceContext;
use codex_rollout_trace::RawTraceEventPayload;
use codex_rollout_trace::RolloutTrace;
use codex_rollout_trace::TraceWriter;
use codex_rollout_trace::replay_bundle;
use futures::StreamExt;
use pretty_assertions::assert_eq;
use serde_json::json;
use std::collections::VecDeque;
use std::pin::Pin;
use std::sync::Arc;
use std::task::Context;
use std::task::Poll;
use std::time::Duration;
use tempfile::TempDir;
use tokio::sync::Notify;
fn test_model_client(session_source: SessionSource) -> ModelClient {
let provider = create_oss_provider_with_base_url("https://example.com/v1", WireApi::Responses);
@@ -79,6 +99,92 @@ fn test_session_telemetry() -> SessionTelemetry {
)
}
fn started_inference_attempt(temp: &TempDir) -> anyhow::Result<InferenceTraceAttempt> {
let writer = Arc::new(TraceWriter::create(
temp.path(),
"trace-1".to_string(),
"rollout-1".to_string(),
"thread-root".to_string(),
)?);
writer.append(RawTraceEventPayload::ThreadStarted {
thread_id: "thread-root".to_string(),
agent_path: "/root".to_string(),
metadata_payload: None,
})?;
writer.append(RawTraceEventPayload::CodexTurnStarted {
codex_turn_id: "turn-1".to_string(),
thread_id: "thread-root".to_string(),
})?;
let inference_trace = InferenceTraceContext::enabled(
writer,
"thread-root".to_string(),
"turn-1".to_string(),
"gpt-test".to_string(),
"test-provider".to_string(),
);
let attempt = inference_trace.start_attempt();
attempt.record_started(&json!({
"model": "gpt-test",
"input": [{
"type": "message",
"role": "user",
"content": [{"type": "input_text", "text": "hello"}]
}],
}));
Ok(attempt)
}
fn output_message(id: &str, text: &str) -> ResponseItem {
ResponseItem::Message {
id: Some(id.to_string()),
role: "assistant".to_string(),
content: vec![ContentItem::OutputText {
text: text.to_string(),
}],
phase: None,
}
}
async fn replay_until_cancelled(temp: &TempDir) -> anyhow::Result<RolloutTrace> {
let mut rollout = replay_bundle(temp.path())?;
for _ in 0..50 {
let inference = rollout
.inference_calls
.values()
.next()
.expect("inference should be reduced");
if inference.execution.status == ExecutionStatus::Cancelled {
return Ok(rollout);
}
tokio::time::sleep(Duration::from_millis(10)).await;
rollout = replay_bundle(temp.path())?;
}
Ok(rollout)
}
struct NotifyAfterEventStream {
events: VecDeque<ResponseEvent>,
yielded: usize,
notify_after: usize,
notify: Arc<Notify>,
}
impl futures::Stream for NotifyAfterEventStream {
type Item = std::result::Result<ResponseEvent, ApiError>;
fn poll_next(mut self: Pin<&mut Self>, _cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
let Some(event) = self.events.pop_front() else {
return Poll::Pending;
};
self.yielded += 1;
if self.yielded == self.notify_after {
self.notify.notify_one();
}
Poll::Ready(Some(Ok(event)))
}
}
#[test]
fn build_subagent_headers_sets_other_subagent_label() {
let client = test_model_client(SessionSource::SubAgent(SubAgentSource::Other(
@@ -151,6 +257,91 @@ async fn summarize_memories_returns_empty_for_empty_input() {
assert_eq!(output.len(), 0);
}
#[tokio::test]
async fn dropped_response_stream_traces_cancelled_partial_output() -> anyhow::Result<()> {
let temp = TempDir::new()?;
let attempt = started_inference_attempt(&temp)?;
// The provider has produced one complete output item, but no terminal
// response.completed event. The harness has enough information to keep this
// item in history, so the trace should preserve it when the stream is
// abandoned.
let item = output_message("msg-1", "partial answer");
let api_stream = futures::stream::iter([Ok(ResponseEvent::OutputItemDone(item))])
.chain(futures::stream::pending());
let (mut stream, _) = super::map_response_stream(api_stream, test_session_telemetry(), attempt);
let observed = stream
.next()
.await
.expect("mapped stream should yield output item")?;
assert!(matches!(observed, ResponseEvent::OutputItemDone(_)));
// Dropping the consumer is how turn interruption/preemption stops polling
// the provider stream. The mapper task observes that drop asynchronously
// and records cancellation using the output items it has already seen.
drop(stream);
// Cancellation is recorded by the mapper task after Drop wakes it, so the
// replay may need a short wait before the terminal event appears on disk.
let rollout = replay_until_cancelled(&temp).await?;
let inference = rollout
.inference_calls
.values()
.next()
.expect("inference should be reduced");
assert_eq!(inference.execution.status, ExecutionStatus::Cancelled);
assert_eq!(inference.response_item_ids.len(), 1);
assert_eq!(rollout.raw_payloads.len(), 2);
Ok(())
}
#[tokio::test]
async fn dropped_backpressured_response_stream_traces_cancelled_partial_output()
-> anyhow::Result<()> {
let temp = TempDir::new()?;
let attempt = started_inference_attempt(&temp)?;
let backpressured_item_yielded = Arc::new(Notify::new());
let mut events = VecDeque::new();
for _ in 0..super::RESPONSE_STREAM_CHANNEL_CAPACITY {
events.push_back(ResponseEvent::Created);
}
events.push_back(ResponseEvent::OutputItemDone(output_message(
"msg-1",
"partial answer",
)));
let api_stream = NotifyAfterEventStream {
events,
yielded: 0,
notify_after: super::RESPONSE_STREAM_CHANNEL_CAPACITY + 1,
notify: Arc::clone(&backpressured_item_yielded),
};
let (stream, _) = super::map_response_stream(api_stream, test_session_telemetry(), attempt);
// Fill the mapper channel with non-terminal events, then yield one output
// item. The mapper has observed that item and is blocked trying to send it
// downstream, so dropping the consumer covers the send-failure path rather
// than the `consumer_dropped` select branch.
backpressured_item_yielded.notified().await;
drop(stream);
let rollout = replay_until_cancelled(&temp).await?;
let inference = rollout
.inference_calls
.values()
.next()
.expect("inference should be reduced");
assert_eq!(inference.execution.status, ExecutionStatus::Cancelled);
assert_eq!(inference.response_item_ids.len(), 1);
assert_eq!(rollout.raw_payloads.len(), 2);
Ok(())
}
#[test]
fn auth_request_telemetry_context_tracks_attached_auth_and_retry_phase() {
let auth_context = AuthRequestTelemetryContext::new(
+1 -1
View File
@@ -947,7 +947,7 @@ async fn permission_profile_override_applies_runtime_roots_to_legacy_projection(
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
+1 -1
View File
@@ -1004,7 +1004,7 @@ fn managed_cwd_write_profile_is_not_read_only() {
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
+18 -6
View File
@@ -535,7 +535,9 @@ fn windows_restricted_token_rejects_split_only_filesystem_policies() {
let file_system_policy = FileSystemSandboxPolicy::restricted(vec![
codex_protocol::permissions::FileSystemSandboxEntry {
path: codex_protocol::permissions::FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::CurrentWorkingDirectory,
value: codex_protocol::permissions::FileSystemSpecialPath::project_roots(
/*subpath*/ None,
),
},
access: codex_protocol::permissions::FileSystemAccessMode::Write,
},
@@ -630,7 +632,9 @@ fn windows_restricted_token_supports_full_read_split_write_read_carveouts() {
},
codex_protocol::permissions::FileSystemSandboxEntry {
path: codex_protocol::permissions::FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::CurrentWorkingDirectory,
value: codex_protocol::permissions::FileSystemSpecialPath::project_roots(
/*subpath*/ None,
),
},
access: codex_protocol::permissions::FileSystemAccessMode::Write,
},
@@ -720,7 +724,9 @@ fn windows_elevated_supports_split_write_read_carveouts() {
},
codex_protocol::permissions::FileSystemSandboxEntry {
path: codex_protocol::permissions::FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::CurrentWorkingDirectory,
value: codex_protocol::permissions::FileSystemSpecialPath::project_roots(
/*subpath*/ None,
),
},
access: codex_protocol::permissions::FileSystemAccessMode::Write,
},
@@ -774,7 +780,9 @@ fn windows_elevated_rejects_unreadable_split_carveouts() {
},
codex_protocol::permissions::FileSystemSandboxEntry {
path: codex_protocol::permissions::FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::CurrentWorkingDirectory,
value: codex_protocol::permissions::FileSystemSpecialPath::project_roots(
/*subpath*/ None,
),
},
access: codex_protocol::permissions::FileSystemAccessMode::Write,
},
@@ -821,7 +829,9 @@ fn windows_elevated_rejects_unreadable_globs() {
},
codex_protocol::permissions::FileSystemSandboxEntry {
path: codex_protocol::permissions::FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::CurrentWorkingDirectory,
value: codex_protocol::permissions::FileSystemSpecialPath::project_roots(
/*subpath*/ None,
),
},
access: codex_protocol::permissions::FileSystemAccessMode::Write,
},
@@ -870,7 +880,9 @@ fn windows_elevated_rejects_reopened_writable_descendants() {
},
codex_protocol::permissions::FileSystemSandboxEntry {
path: codex_protocol::permissions::FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::CurrentWorkingDirectory,
value: codex_protocol::permissions::FileSystemSpecialPath::project_roots(
/*subpath*/ None,
),
},
access: codex_protocol::permissions::FileSystemAccessMode::Write,
},
+1 -1
View File
@@ -258,7 +258,7 @@ fn explicit_read_only_subpaths_prevent_auto_approval_for_external_sandbox() {
let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
+14 -14
View File
@@ -149,19 +149,19 @@ impl SessionConfiguration {
);
let file_system_policy_matches_legacy = current_file_system_sandbox_policy
.is_semantically_equivalent_to(&legacy_file_system_projection, &self.cwd);
let file_system_policy_has_rebindable_cwd_write = current_file_system_sandbox_policy
.entries
.iter()
.any(|entry| {
entry.access.can_write()
&& matches!(
&entry.path,
FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory
| FileSystemSpecialPath::ProjectRoots { subpath: None },
}
)
});
let file_system_policy_has_rebindable_project_root_write =
current_file_system_sandbox_policy
.entries
.iter()
.any(|entry| {
entry.access.can_write()
&& matches!(
&entry.path,
FileSystemPath::Special {
value: FileSystemSpecialPath::ProjectRoots { subpath: None },
}
)
});
if let Some(collaboration_mode) = updates.collaboration_mode.clone() {
next_configuration.collaboration_mode = collaboration_mode;
}
@@ -228,7 +228,7 @@ impl SessionConfiguration {
)?;
} else if cwd_changed
&& file_system_policy_matches_legacy
&& file_system_policy_has_rebindable_cwd_write
&& file_system_policy_has_rebindable_project_root_write
{
// Preserve richer split policies across cwd-only updates; only
// rederive when the session is already using a structurally
+3 -3
View File
@@ -2874,7 +2874,7 @@ async fn session_configuration_apply_preserves_profile_file_system_policy_on_cwd
let file_system_sandbox_policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
@@ -3187,7 +3187,7 @@ async fn session_configuration_apply_preserves_absolute_cwd_write_root_on_cwd_up
!updated
.file_system_sandbox_policy()
.can_write_path_with_cwd(next_cwd.as_path(), updated.cwd.as_path()),
"cwd-only update must not reinterpret an absolute old-cwd grant as :cwd"
"cwd-only update must not reinterpret an absolute old-cwd grant as :project_roots"
);
}
@@ -3850,7 +3850,7 @@ async fn request_permissions_response_materializes_session_cwd_grants_before_rec
file_system: Some(FileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
}],
+1 -1
View File
@@ -354,7 +354,7 @@ mod tests {
entries: vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
+1 -3
View File
@@ -117,9 +117,7 @@ pub(crate) fn file_system_policy_has_cwd_dependent_entries(
.any(|entry| match &entry.path {
FileSystemPath::GlobPattern { pattern } => !Path::new(pattern).is_absolute(),
FileSystemPath::Special {
value:
FileSystemSpecialPath::CurrentWorkingDirectory
| FileSystemSpecialPath::ProjectRoots { .. },
value: FileSystemSpecialPath::ProjectRoots { .. },
} => true,
FileSystemPath::Path { .. } | FileSystemPath::Special { .. } => false,
})
+4 -4
View File
@@ -118,7 +118,7 @@ fn sandbox_cwd(sandbox: &FileSystemSandboxContext) -> Result<AbsolutePathBuf, JS
let file_system_policy = sandbox.permissions.file_system_sandbox_policy();
if file_system_policy_has_cwd_dependent_entries(&file_system_policy) {
return Err(invalid_request(
"file system sandbox context with cwd-relative permissions requires cwd".to_string(),
"file system sandbox context with dynamic permissions requires cwd".to_string(),
));
}
@@ -467,7 +467,7 @@ mod tests {
let cwd = AbsolutePathBuf::from_absolute_path(std::env::temp_dir().as_path())
.expect("absolute cwd");
let policy = restricted_policy(vec![special_entry(
FileSystemSpecialPath::CurrentWorkingDirectory,
FileSystemSpecialPath::project_roots(/*subpath*/ None),
FileSystemAccessMode::Write,
)]);
let sandbox_context = sandbox_context_with_cwd(&policy, cwd.clone());
@@ -479,7 +479,7 @@ mod tests {
fn sandbox_cwd_rejects_cwd_dependent_profile_without_context_cwd() {
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
}]);
@@ -491,7 +491,7 @@ mod tests {
assert_eq!(
err.message,
"file system sandbox context with cwd-relative permissions requires cwd"
"file system sandbox context with dynamic permissions requires cwd"
);
}
@@ -252,7 +252,7 @@ mod tests {
fn remote_sandbox_context_preserves_required_cwd() {
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
}]);
+3
View File
@@ -3,5 +3,8 @@ load("//:defs.bzl", "codex_rust_crate")
codex_rust_crate(
name = "exec",
crate_name = "codex_exec",
test_shard_counts = {
"exec-all-test": 8,
},
test_tags = ["no-sandbox"],
)
+1 -1
View File
@@ -1143,7 +1143,7 @@ mod tests {
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
@@ -200,7 +200,9 @@ fn split_only_filesystem_policy_requires_direct_runtime_enforcement() {
let policy = FileSystemSandboxPolicy::restricted(vec![
codex_protocol::permissions::FileSystemSandboxEntry {
path: codex_protocol::permissions::FileSystemPath::Special {
value: codex_protocol::permissions::FileSystemSpecialPath::CurrentWorkingDirectory,
value: codex_protocol::permissions::FileSystemSpecialPath::project_roots(
/*subpath*/ None,
),
},
access: codex_protocol::permissions::FileSystemAccessMode::Write,
},
+6 -2
View File
@@ -402,7 +402,11 @@ impl PermissionProfile {
}
}
/// Managed workspace-write filesystem access with restricted network access.
/// Managed workspace-write filesystem access with restricted network
/// access.
///
/// The returned profile contains symbolic `:project_roots` entries that
/// must be resolved against the active permission root before enforcement.
pub fn workspace_write() -> Self {
Self::Managed {
file_system: ManagedFileSystemPermissions::Restricted {
@@ -415,7 +419,7 @@ impl PermissionProfile {
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
+61 -39
View File
@@ -80,7 +80,7 @@ impl FileSystemAccessMode {
pub enum FileSystemSpecialPath {
Root,
Minimal,
CurrentWorkingDirectory,
#[serde(alias = "current_working_directory")]
ProjectRoots {
#[serde(default, skip_serializing_if = "Option::is_none")]
#[ts(optional)]
@@ -414,9 +414,9 @@ impl FileSystemSandboxPolicy {
/// Converts a legacy sandbox policy into a cwd-independent filesystem policy.
///
/// `WorkspaceWrite` uses symbolic entries for cwd-scoped access so callers
/// can preserve the active cwd binding until the policy is actually
/// resolved for a turn or command.
/// `WorkspaceWrite` uses symbolic project-root entries so callers can keep
/// the profile independent of the concrete root until it is resolved for a
/// turn or command.
pub fn from_legacy_sandbox_policy(sandbox_policy: &SandboxPolicy) -> Self {
let mut file_system_policy = Self::from(sandbox_policy);
let SandboxPolicy::WorkspaceWrite {
@@ -571,6 +571,28 @@ impl FileSystemSandboxPolicy {
self.resolve_access_with_cwd(path, cwd).can_write()
}
/// Replaces symbolic `:project_roots` entries with absolute paths resolved
/// against `cwd`.
///
/// Use this when a durable permission profile must survive a cwd-only
/// update without rebinding its project-root authority to the new cwd.
pub fn materialize_project_roots_with_cwd(mut self, cwd: &Path) -> Self {
let cwd = AbsolutePathBuf::from_absolute_path(cwd).ok();
for entry in &mut self.entries {
let FileSystemPath::Special {
value: FileSystemSpecialPath::ProjectRoots { .. },
} = &entry.path
else {
continue;
};
if let Some(path) = resolve_file_system_path(&entry.path, cwd.as_ref()) {
entry.path = FileSystemPath::Path { path };
}
}
self
}
pub fn with_additional_readable_roots(
mut self,
cwd: &Path,
@@ -867,11 +889,6 @@ impl FileSystemSandboxPolicy {
}
},
FileSystemSpecialPath::Minimal => {}
FileSystemSpecialPath::CurrentWorkingDirectory => {
if entry.access.can_write() {
workspace_root_writable = true;
}
}
FileSystemSpecialPath::ProjectRoots { subpath } => {
if subpath.is_none() && entry.access.can_write() {
workspace_root_writable = true;
@@ -1001,7 +1018,7 @@ impl From<&SandboxPolicy> for FileSystemSandboxPolicy {
entries.push(FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
});
@@ -1099,20 +1116,8 @@ fn special_paths_share_target(left: &FileSystemSpecialPath, right: &FileSystemSp
match (left, right) {
(FileSystemSpecialPath::Root, FileSystemSpecialPath::Root)
| (FileSystemSpecialPath::Minimal, FileSystemSpecialPath::Minimal)
| (
FileSystemSpecialPath::CurrentWorkingDirectory,
FileSystemSpecialPath::CurrentWorkingDirectory,
)
| (FileSystemSpecialPath::Tmpdir, FileSystemSpecialPath::Tmpdir)
| (FileSystemSpecialPath::SlashTmp, FileSystemSpecialPath::SlashTmp) => true,
(
FileSystemSpecialPath::CurrentWorkingDirectory,
FileSystemSpecialPath::ProjectRoots { subpath: None },
)
| (
FileSystemSpecialPath::ProjectRoots { subpath: None },
FileSystemSpecialPath::CurrentWorkingDirectory,
) => true,
(
FileSystemSpecialPath::ProjectRoots { subpath: left },
FileSystemSpecialPath::ProjectRoots { subpath: right },
@@ -1210,10 +1215,6 @@ fn resolve_file_system_special_path(
FileSystemSpecialPath::Root
| FileSystemSpecialPath::Minimal
| FileSystemSpecialPath::Unknown { .. } => None,
FileSystemSpecialPath::CurrentWorkingDirectory => {
let cwd = cwd?;
Some(cwd.clone())
}
FileSystemSpecialPath::ProjectRoots { subpath } => {
let cwd = cwd?;
match subpath.as_ref() {
@@ -1550,7 +1551,7 @@ mod tests {
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
}]);
@@ -1566,7 +1567,7 @@ mod tests {
}
#[test]
fn legacy_workspace_write_projection_preserves_symbolic_cwd() {
fn legacy_workspace_write_projection_preserves_symbolic_project_root() {
let policy = SandboxPolicy::WorkspaceWrite {
writable_roots: Vec::new(),
network_access: false,
@@ -1585,7 +1586,7 @@ mod tests {
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
@@ -1611,6 +1612,27 @@ mod tests {
);
}
#[test]
fn legacy_current_working_directory_special_path_deserializes_as_project_roots()
-> serde_json::Result<()> {
let value = serde_json::json!({
"kind": "current_working_directory",
});
let special_path = serde_json::from_value::<FileSystemSpecialPath>(value)?;
assert_eq!(
special_path,
FileSystemSpecialPath::project_roots(/*subpath*/ None)
);
assert_eq!(
serde_json::to_value(&special_path)?,
serde_json::json!({
"kind": "project_roots",
})
);
Ok(())
}
#[cfg(unix)]
#[test]
fn writable_roots_skip_default_dot_codex_when_explicit_user_rule_exists() {
@@ -1624,7 +1646,7 @@ mod tests {
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
@@ -1703,7 +1725,7 @@ mod tests {
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
@@ -1774,7 +1796,7 @@ mod tests {
#[cfg(unix)]
#[test]
fn current_working_directory_special_path_preserves_symlinked_cwd() {
fn project_roots_special_path_preserves_symlinked_root() {
let cwd = TempDir::new().expect("tempdir");
let real_root = cwd.path().join("real");
let link_root = cwd.path().join("link");
@@ -1804,7 +1826,7 @@ mod tests {
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
@@ -2094,7 +2116,7 @@ mod tests {
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
@@ -2141,7 +2163,7 @@ mod tests {
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
@@ -2318,7 +2340,7 @@ mod tests {
let cwd_root = AbsolutePathBuf::from_absolute_path(cwd.path()).expect("absolute cwd");
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Read,
}]);
@@ -2336,7 +2358,7 @@ mod tests {
let cwd_root = AbsolutePathBuf::from_absolute_path(cwd.path()).expect("absolute cwd");
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
}]);
@@ -2356,7 +2378,7 @@ mod tests {
.expect("resolve extra root");
let policy = FileSystemSandboxPolicy::restricted(vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
}]);
@@ -2368,7 +2390,7 @@ mod tests {
FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
+2 -2
View File
@@ -4397,7 +4397,7 @@ mod tests {
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
@@ -4459,7 +4459,7 @@ mod tests {
let policy = FileSystemSandboxPolicy::restricted(vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
+93 -26
View File
@@ -6,6 +6,7 @@
use std::fmt::Display;
use std::sync::Arc;
use std::sync::atomic::AtomicBool;
use std::sync::atomic::AtomicU64;
use std::sync::atomic::Ordering;
@@ -54,35 +55,36 @@ struct EnabledInferenceTraceContext {
///
/// A Codex turn can create multiple attempts when auth recovery retries the
/// HTTP request or WebSocket setup falls back to HTTP. Completion is often
/// observed after the client returns the response stream, so attempts are
/// cloneable and self-contained.
#[derive(Clone, Debug)]
/// observed after the client returns the response stream, so the attempt owns
/// the terminal guard that prevents duplicate lifecycle events.
#[derive(Debug)]
pub struct InferenceTraceAttempt {
state: InferenceTraceAttemptState,
}
#[derive(Clone, Debug)]
#[derive(Debug)]
enum InferenceTraceAttemptState {
Disabled,
Enabled(EnabledInferenceTraceAttempt),
}
#[derive(Clone, Debug)]
#[derive(Debug)]
struct EnabledInferenceTraceAttempt {
context: EnabledInferenceTraceContext,
inference_call_id: InferenceCallId,
terminal_recorded: AtomicBool,
}
/// Non-delta response payload saved when a traced inference stream completes.
/// Non-delta response payload saved for completed or interrupted inference streams.
///
/// We intentionally record completed output items instead of every stream delta
/// here. The raw stream can be added later as a separate payload class; this
/// response summary gives the reducer stable response identity, usage, and
/// model-visible output without duplicating high-volume text deltas.
/// response summary gives the reducer stable response identity when available
/// plus model-visible output without duplicating high-volume text deltas.
#[derive(Serialize)]
struct TracedResponseStreamCompleted<'a> {
response_id: &'a str,
token_usage: &'a Option<TokenUsage>,
struct TracedResponseStreamOutput<'a> {
response_id: Option<&'a str>,
token_usage: Option<&'a TokenUsage>,
output_items: Vec<JsonValue>,
}
@@ -123,6 +125,7 @@ impl InferenceTraceContext {
state: InferenceTraceAttemptState::Enabled(EnabledInferenceTraceAttempt {
context: context.clone(),
inference_call_id: next_inference_call_id(),
terminal_recorded: AtomicBool::new(false),
}),
}
}
@@ -162,9 +165,9 @@ impl InferenceTraceAttempt {
);
}
/// Records a bounded, non-streaming summary of the completed response stream.
/// Records successful provider completion and serializes the observed output items.
///
/// The caller passes protocol-native response items so this crate owns the
/// Callers pass protocol-native response items so this crate owns the
/// trace-specific serialization rules. That keeps codex-core focused on
/// transport behavior while preserving trace evidence that normal request
/// serialization intentionally omits.
@@ -174,18 +177,14 @@ impl InferenceTraceAttempt {
token_usage: &Option<TokenUsage>,
output_items: &[ResponseItem],
) {
let InferenceTraceAttemptState::Enabled(attempt) = &self.state else {
let Some(attempt) = self.take_terminal_attempt() else {
return;
};
let response_payload = TracedResponseStreamCompleted {
response_id,
token_usage,
output_items: output_items.iter().map(trace_response_item_json).collect(),
};
let Some(response_payload) = write_json_payload_best_effort(
&attempt.context.writer,
RawPayloadKind::InferenceResponse,
&response_payload,
let Some(response_payload) = write_response_payload_best_effort(
attempt,
Some(response_id),
token_usage.as_ref(),
output_items,
) else {
return;
};
@@ -201,19 +200,69 @@ impl InferenceTraceAttempt {
}
/// Records pre-response and mid-stream failures.
pub fn record_failed(&self, error: impl Display) {
let InferenceTraceAttemptState::Enabled(attempt) = &self.state else {
pub fn record_failed(&self, error: impl Display, output_items: &[ResponseItem]) {
let Some(attempt) = self.take_terminal_attempt() else {
return;
};
let partial_response_payload = if output_items.is_empty() {
None
} else {
write_response_payload_best_effort(
attempt,
/*response_id*/ None,
/*token_usage*/ None,
output_items,
)
};
append_with_context_best_effort(
&attempt.context,
RawTraceEventPayload::InferenceFailed {
inference_call_id: attempt.inference_call_id.clone(),
error: error.to_string(),
partial_response_payload: None,
partial_response_payload,
},
);
}
/// Records a provider stream that Codex intentionally stopped consuming.
///
/// This happens when the turn is interrupted or when mailbox delivery
/// preempts the current sampling request. Complete output items observed
/// before that point are retained as partial response evidence.
pub fn record_cancelled(&self, reason: impl Display, output_items: &[ResponseItem]) {
let Some(attempt) = self.take_terminal_attempt() else {
return;
};
let partial_response_payload = if output_items.is_empty() {
None
} else {
write_response_payload_best_effort(
attempt,
/*response_id*/ None,
/*token_usage*/ None,
output_items,
)
};
append_with_context_best_effort(
&attempt.context,
RawTraceEventPayload::InferenceCancelled {
inference_call_id: attempt.inference_call_id.clone(),
reason: reason.to_string(),
partial_response_payload,
},
);
}
fn take_terminal_attempt(&self) -> Option<&EnabledInferenceTraceAttempt> {
let attempt = match &self.state {
InferenceTraceAttemptState::Disabled => return None,
InferenceTraceAttemptState::Enabled(attempt) => attempt,
};
if attempt.terminal_recorded.swap(true, Ordering::AcqRel) {
return None;
}
Some(attempt)
}
}
/// Serializes a response item for trace evidence rather than future request construction.
@@ -260,6 +309,24 @@ fn write_json_payload_best_effort(
writer.write_json_payload(kind, payload).ok()
}
fn write_response_payload_best_effort(
attempt: &EnabledInferenceTraceAttempt,
response_id: Option<&str>,
token_usage: Option<&TokenUsage>,
output_items: &[ResponseItem],
) -> Option<crate::RawPayloadRef> {
let response_payload = TracedResponseStreamOutput {
response_id,
token_usage,
output_items: output_items.iter().map(trace_response_item_json).collect(),
};
write_json_payload_best_effort(
&attempt.context.writer,
RawPayloadKind::InferenceResponse,
&response_payload,
)
}
fn append_with_context_best_effort(
context: &EnabledInferenceTraceContext,
payload: RawTraceEventPayload,
+11
View File
@@ -110,6 +110,13 @@ pub enum RawTraceEventPayload {
/// Partial response payload, when stream events arrived before failure.
partial_response_payload: Option<RawPayloadRef>,
},
InferenceCancelled {
inference_call_id: InferenceCallId,
/// Why Codex stopped consuming the provider stream before a terminal response event.
reason: String,
/// Completed output items observed before cancellation, if any.
partial_response_payload: Option<RawPayloadRef>,
},
ToolCallStarted {
tool_call_id: ToolCallId,
/// Protocol/model call ID when this runtime call came from model output.
@@ -253,6 +260,10 @@ impl RawTraceEventPayload {
partial_response_payload,
..
}
| RawTraceEventPayload::InferenceCancelled {
partial_response_payload,
..
}
| RawTraceEventPayload::ToolCallStarted {
invocation_payload: partial_response_payload,
..
@@ -103,6 +103,35 @@ impl TraceReducer {
Ok(())
}
/// Closes any inference streams that are still live when the owning turn ends.
///
/// Normal completion events close the active inference before the turn ends.
/// If a call is still `Running`, Codex stopped observing that provider stream
/// earlier and the reduced graph should not present it as live.
pub(super) fn close_running_inference_calls_for_turn_end(
&mut self,
seq: RawEventSeq,
wall_time_unix_ms: i64,
codex_turn_id: &str,
turn_status: &ExecutionStatus,
) {
let inference_status = match turn_status {
ExecutionStatus::Running => return,
ExecutionStatus::Completed | ExecutionStatus::Cancelled => ExecutionStatus::Cancelled,
ExecutionStatus::Failed => ExecutionStatus::Failed,
ExecutionStatus::Aborted => ExecutionStatus::Aborted,
};
for inference in self.rollout.inference_calls.values_mut() {
if inference.codex_turn_id == codex_turn_id
&& inference.execution.status == ExecutionStatus::Running
{
inference.execution.ended_at_unix_ms = Some(wall_time_unix_ms);
inference.execution.ended_seq = Some(seq);
inference.execution.status = inference_status.clone();
}
}
}
/// Completes an inference call and, when present, reduces response output items.
pub(super) fn complete_inference_call(
&mut self,
@@ -130,14 +159,25 @@ impl TraceReducer {
let Some(inference) = self.rollout.inference_calls.get_mut(&inference_call_id) else {
bail!("inference call {inference_call_id} disappeared during response reduction");
};
inference.execution.ended_at_unix_ms = Some(wall_time_unix_ms);
inference.execution.ended_seq = Some(seq);
inference.execution.status = status;
inference.upstream_request_id = response_id;
inference.raw_response_payload_id = response_payload.map(|payload| payload.raw_payload_id);
// Turn-end cleanup can close a stream before the async mapper observes
// cancellation. Preserve that terminal status while still retaining any
// late partial response evidence from the mapper.
if inference.execution.status == ExecutionStatus::Running {
inference.execution.ended_at_unix_ms = Some(wall_time_unix_ms);
inference.execution.ended_seq = Some(seq);
inference.execution.status = status;
inference.upstream_request_id = response_id;
}
if let Some(response_payload) = response_payload {
inference.raw_response_payload_id = Some(response_payload.raw_payload_id);
}
if let Some(response_item_ids) = response_item_ids {
inference.response_item_ids = response_item_ids;
}
Ok(())
}
}
#[cfg(test)]
#[path = "inference_tests.rs"]
mod tests;
@@ -0,0 +1,148 @@
use pretty_assertions::assert_eq;
use serde_json::json;
use tempfile::TempDir;
use crate::model::ConversationItemKind;
use crate::model::ExecutionStatus;
use crate::payload::RawPayloadKind;
use crate::raw_event::RawTraceEventPayload;
use crate::reducer::test_support::append_inference_start;
use crate::reducer::test_support::create_started_writer;
use crate::reducer::test_support::message;
use crate::reducer::test_support::start_turn;
use crate::replay_bundle;
#[test]
fn cancelled_inference_reduces_partial_response_items() -> anyhow::Result<()> {
let temp = TempDir::new()?;
let writer = create_started_writer(&temp)?;
start_turn(&writer, "turn-1")?;
let request = writer.write_json_payload(
RawPayloadKind::InferenceRequest,
&json!({
"input": [message("user", "draft")]
}),
)?;
append_inference_start(&writer, "inference-1", "turn-1", request)?;
let partial_response = writer.write_json_payload(
RawPayloadKind::InferenceResponse,
&json!({
"response_id": null,
"token_usage": null,
"output_items": [{
"type": "message",
"role": "assistant",
"content": [{"type": "output_text", "text": "partial"}]
}]
}),
)?;
writer.append(RawTraceEventPayload::InferenceCancelled {
inference_call_id: "inference-1".to_string(),
reason: "test interruption".to_string(),
partial_response_payload: Some(partial_response),
})?;
let rollout = replay_bundle(temp.path())?;
let inference = &rollout.inference_calls["inference-1"];
let response_item_id = &inference.response_item_ids[0];
assert_eq!(inference.execution.status, ExecutionStatus::Cancelled);
assert_eq!(inference.response_item_ids.len(), 1);
assert_eq!(
rollout.conversation_items[response_item_id].kind,
ConversationItemKind::Message,
);
assert_eq!(
rollout.conversation_items[response_item_id].produced_by,
vec![crate::model::ProducerRef::Inference {
inference_call_id: "inference-1".to_string(),
}],
);
Ok(())
}
#[test]
fn cancelled_turn_closes_running_inference_call() -> anyhow::Result<()> {
let temp = TempDir::new()?;
let writer = create_started_writer(&temp)?;
start_turn(&writer, "turn-1")?;
let request = writer.write_json_payload(
RawPayloadKind::InferenceRequest,
&json!({
"input": [message("user", "wait")]
}),
)?;
append_inference_start(&writer, "inference-1", "turn-1", request)?;
let turn_end = writer.append(RawTraceEventPayload::CodexTurnEnded {
codex_turn_id: "turn-1".to_string(),
status: ExecutionStatus::Cancelled,
})?;
let rollout = replay_bundle(temp.path())?;
let inference = &rollout.inference_calls["inference-1"];
assert_eq!(inference.execution.status, ExecutionStatus::Cancelled);
assert_eq!(inference.execution.ended_seq, Some(turn_end.seq));
Ok(())
}
#[test]
fn late_cancelled_inference_preserves_turn_end_status() -> anyhow::Result<()> {
let temp = TempDir::new()?;
let writer = create_started_writer(&temp)?;
start_turn(&writer, "turn-1")?;
let request = writer.write_json_payload(
RawPayloadKind::InferenceRequest,
&json!({
"input": [message("user", "interrupt")]
}),
)?;
append_inference_start(&writer, "inference-1", "turn-1", request)?;
let turn_end = writer.append(RawTraceEventPayload::CodexTurnEnded {
codex_turn_id: "turn-1".to_string(),
status: ExecutionStatus::Failed,
})?;
let partial_response = writer.write_json_payload(
RawPayloadKind::InferenceResponse,
&json!({
"response_id": null,
"token_usage": null,
"output_items": [{
"type": "message",
"role": "assistant",
"content": [{"type": "output_text", "text": "late partial"}]
}]
}),
)?;
writer.append(RawTraceEventPayload::InferenceCancelled {
inference_call_id: "inference-1".to_string(),
reason: "stream mapper noticed cancellation after turn end".to_string(),
partial_response_payload: Some(partial_response.clone()),
})?;
let rollout = replay_bundle(temp.path())?;
let inference = &rollout.inference_calls["inference-1"];
assert_eq!(inference.execution.status, ExecutionStatus::Failed);
assert_eq!(inference.execution.ended_seq, Some(turn_end.seq));
assert_eq!(
inference.raw_response_payload_id,
Some(partial_response.raw_payload_id),
);
assert_eq!(inference.response_item_ids.len(), 1);
let response_item_id = &inference.response_item_ids[0];
assert_eq!(
rollout.conversation_items[response_item_id].body.parts,
vec![crate::model::ConversationPart::Text {
text: "late partial".to_string(),
}],
);
Ok(())
}
+14
View File
@@ -254,6 +254,20 @@ impl TraceReducer {
partial_response_payload,
)?;
}
RawTraceEventPayload::InferenceCancelled {
inference_call_id,
partial_response_payload,
..
} => {
self.complete_inference_call(
event.seq,
event.wall_time_unix_ms,
inference_call_id,
ExecutionStatus::Cancelled,
/*response_id*/ None,
partial_response_payload,
)?;
}
RawTraceEventPayload::ProtocolEventObserved { .. } => {
// Protocol wrappers are raw debug breadcrumbs. Typed hooks own
// the reduced graph, so these payload refs are retained without
@@ -187,6 +187,12 @@ impl TraceReducer {
&codex_turn_id,
&status,
)?;
self.close_running_inference_calls_for_turn_end(
seq,
wall_time_unix_ms,
&codex_turn_id,
&status,
);
Ok(())
}
+1 -1
View File
@@ -134,7 +134,7 @@ fn disabled_thread_context_accepts_trace_calls_without_writing() -> anyhow::Resu
inference_attempt.record_started(&serde_json::json!({ "kind": "inference" }));
let token_usage: Option<codex_protocol::protocol::TokenUsage> = None;
inference_attempt.record_completed("response-1", &token_usage, &[]);
inference_attempt.record_failed("inference failed");
inference_attempt.record_failed("inference failed", &[]);
let compaction_trace = thread_trace.compaction_trace_context(
"turn-1",
+21 -5
View File
@@ -444,9 +444,19 @@ impl RolloutRecorder {
));
}
// Warm the DB by repairing every filesystem hit before querying SQLite.
// For metadata-filtered listings the filesystem page is the page we return. Track those
// IDs so the later DB page only triggers full reconciliation for DB-only hits.
let fs_page_thread_ids = fs_page
.items
.iter()
.filter_map(|item| item.thread_id)
.collect::<HashSet<_>>();
// Warm the DB by repairing every filesystem hit before querying SQLite. Source/provider/cwd
// filters are already validated from rollout head metadata, so lightweight read-repair is
// enough there. Search can depend on full title metadata, so keep full reconciliation.
for item in &fs_page.items {
if listing_has_metadata_filters {
if search_term.is_some() {
state_db::reconcile_rollout(
state_db_ctx.as_deref(),
item.path.as_path(),
@@ -517,6 +527,12 @@ impl RolloutRecorder {
}
if listing_has_metadata_filters {
for item in &db_page.items {
// Rows that also appeared in the filesystem page were just validated from the
// rollout head. Rows only found by SQLite may be stale filter matches, so fully
// reconcile those before returning the filesystem-backed page.
if fs_page_thread_ids.contains(&item.id) {
continue;
}
state_db::reconcile_rollout(
state_db_ctx.as_deref(),
item.rollout_path.as_path(),
@@ -1030,13 +1046,13 @@ fn fill_missing_thread_item_metadata(item: &mut ThreadItem, state_item: ThreadIt
if item.cwd.is_none() {
item.cwd = cwd;
}
if item.git_branch.is_none() {
if git_branch.is_some() {
item.git_branch = git_branch;
}
if item.git_sha.is_none() {
if git_sha.is_some() {
item.git_sha = git_sha;
}
if item.git_origin_url.is_none() {
if git_origin_url.is_some() {
item.git_origin_url = git_origin_url;
}
if item.source.is_none() {
+4 -4
View File
@@ -779,7 +779,7 @@ async fn list_threads_metadata_filter_overlays_state_db_list_metadata() -> std::
}
#[test]
fn fill_missing_thread_item_metadata_preserves_filesystem_identity() {
fn fill_missing_thread_item_metadata_preserves_identity_and_prefers_state_git_fields() {
let filesystem_thread_id = ThreadId::new();
let state_thread_id = ThreadId::new();
let filesystem_path = PathBuf::from("/tmp/filesystem-rollout.jsonl");
@@ -789,9 +789,9 @@ fn fill_missing_thread_item_metadata_preserves_filesystem_identity() {
thread_id: Some(filesystem_thread_id),
first_user_message: Some("filesystem message".to_string()),
cwd: None,
git_branch: None,
git_sha: None,
git_origin_url: None,
git_branch: Some("filesystem-branch".to_string()),
git_sha: Some("filesystem-sha".to_string()),
git_origin_url: Some("https://example.com/filesystem.git".to_string()),
source: None,
agent_nickname: None,
agent_role: None,
+1 -6
View File
@@ -350,9 +350,7 @@ fn materialize_cwd_dependent_entry(
) -> FileSystemSandboxEntry {
match &entry.path {
FileSystemPath::Special {
value:
FileSystemSpecialPath::CurrentWorkingDirectory
| FileSystemSpecialPath::ProjectRoots { .. },
value: FileSystemSpecialPath::ProjectRoots { .. },
} => resolve_permission_path(&entry.path, cwd)
.map(|path| FileSystemSandboxEntry {
path: FileSystemPath::Path { path },
@@ -380,9 +378,6 @@ fn resolve_permission_path(path: &FileSystemPath, cwd: &Path) -> Option<Absolute
let root = cwd.ancestors().last()?;
AbsolutePathBuf::from_absolute_path(root).ok()
}
FileSystemSpecialPath::CurrentWorkingDirectory => {
AbsolutePathBuf::from_absolute_path(cwd).ok()
}
FileSystemSpecialPath::ProjectRoots { subpath } => {
let cwd = AbsolutePathBuf::from_absolute_path(cwd).ok()?;
Some(match subpath {
@@ -299,7 +299,7 @@ fn intersect_permission_profiles_accepts_child_path_granted_for_requested_cwd()
file_system: Some(FileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
}],
@@ -332,7 +332,7 @@ fn intersect_permission_profiles_materializes_cwd_grant_for_reuse() {
file_system: Some(FileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
}],
@@ -383,7 +383,7 @@ fn intersect_permission_profiles_deduplicates_materialized_grants() {
entries: vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
@@ -425,7 +425,7 @@ fn intersect_permission_profiles_materializes_cwd_deny_entries() {
},
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::None,
},
@@ -474,7 +474,7 @@ fn intersect_permission_profiles_drops_deny_entries_without_filesystem_grants()
entries: vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
@@ -512,7 +512,7 @@ fn intersect_permission_profiles_rejects_concrete_grants_matched_by_requested_de
entries: vec![
FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
},
@@ -550,7 +550,7 @@ fn intersect_permission_profiles_materializes_relative_deny_globs_for_reuse() {
.expect("absolute later cwd");
let cwd_write = FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
};
@@ -629,7 +629,7 @@ fn intersect_permission_profiles_drops_broader_cwd_grant_for_requested_child_pat
file_system: Some(FileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Special {
value: FileSystemSpecialPath::CurrentWorkingDirectory,
value: FileSystemSpecialPath::project_roots(/*subpath*/ None),
},
access: FileSystemAccessMode::Write,
}],
@@ -102,7 +102,7 @@ impl ApprovalRequest {
}
}
fn matches_resolved_request(&self, request: &ResolvedAppServerRequest) -> bool {
pub(super) fn matches_resolved_request(&self, request: &ResolvedAppServerRequest) -> bool {
match (self, request) {
(
ApprovalRequest::Exec { id, .. },
@@ -875,7 +875,6 @@ fn special_path_label(value: &FileSystemSpecialPath) -> String {
match value {
FileSystemSpecialPath::Root => ":root".to_string(),
FileSystemSpecialPath::Minimal => ":minimal".to_string(),
FileSystemSpecialPath::CurrentWorkingDirectory => ":cwd".to_string(),
FileSystemSpecialPath::ProjectRoots { subpath } => path_label(":project_roots", subpath),
FileSystemSpecialPath::Tmpdir => ":tmpdir".to_string(),
FileSystemSpecialPath::SlashTmp => "/tmp".to_string(),
+244 -7
View File
@@ -13,6 +13,7 @@
//!
//! Some UI is time-based rather than input-based, such as the transient "press again to quit"
//! hint. The pane schedules redraws so those hints can expire even when the UI is otherwise idle.
use std::collections::VecDeque;
use std::path::PathBuf;
use crate::app::app_server_requests::ResolvedAppServerRequest;
@@ -42,6 +43,7 @@ use ratatui::buffer::Buffer;
use ratatui::layout::Rect;
use ratatui::text::Line;
use std::time::Duration;
use std::time::Instant;
mod app_link_view;
mod approval_overlay;
@@ -139,6 +141,8 @@ pub(crate) use selection_tabs::SelectionTab;
/// Keeping a single value ensures Ctrl+C and Ctrl+D behave identically.
pub(crate) const QUIT_SHORTCUT_TIMEOUT: Duration = Duration::from_secs(1);
const APPROVAL_PROMPT_TYPING_IDLE_DELAY: Duration = Duration::from_secs(1);
/// Whether Ctrl+C/Ctrl+D require a second press to quit.
///
/// This UX experiment was enabled by default, but requiring a double press to quit feels janky in
@@ -170,6 +174,11 @@ pub(crate) use experimental_features_view::ExperimentalFeaturesView;
pub(crate) use list_selection_view::SelectionAction;
pub(crate) use list_selection_view::SelectionItem;
struct DelayedApprovalRequest {
request: ApprovalRequest,
features: Features,
}
/// Pane displayed in the lower half of the chat UI.
///
/// This is the owning container for the prompt input (`ChatComposer`) and the view stack
@@ -182,6 +191,8 @@ pub(crate) struct BottomPane {
/// Stack of views displayed instead of the composer (e.g. popups/modals).
view_stack: Vec<Box<dyn BottomPaneView>>,
delayed_approval_requests: VecDeque<DelayedApprovalRequest>,
last_composer_activity_at: Option<Instant>,
app_event_tx: AppEventSender,
frame_requester: FrameRequester,
@@ -243,6 +254,8 @@ impl BottomPane {
Self {
composer,
view_stack: Vec::new(),
delayed_approval_requests: VecDeque::new(),
last_composer_activity_at: None,
app_event_tx,
frame_requester,
has_input_focus,
@@ -442,6 +455,48 @@ impl BottomPane {
}
}
fn approval_prompt_delay_remaining(&self, now: Instant) -> Option<Duration> {
self.last_composer_activity_at.and_then(|last_activity_at| {
last_activity_at
.checked_add(APPROVAL_PROMPT_TYPING_IDLE_DELAY)
.and_then(|show_at| show_at.checked_duration_since(now))
.filter(|delay| !delay.is_zero())
})
}
fn record_composer_activity_at(&mut self, now: Instant) {
self.last_composer_activity_at = Some(now);
if !self.delayed_approval_requests.is_empty()
&& let Some(delay) = self.approval_prompt_delay_remaining(now)
{
self.request_redraw_in(delay);
}
}
fn maybe_show_delayed_approval_requests_at(&mut self, now: Instant) {
if self.delayed_approval_requests.is_empty() || !self.view_stack.is_empty() {
return;
}
if let Some(delay) = self.approval_prompt_delay_remaining(now) {
self.request_redraw_in(delay);
return;
}
// Promote the oldest delayed approval once typing has been idle long enough.
// `ApprovalOverlay` advances its internal queue with `pop()`, so drain the
// remaining delayed approvals from the back to preserve FIFO display order.
let Some(first) = self.delayed_approval_requests.pop_front() else {
return;
};
let mut modal =
ApprovalOverlay::new(first.request, self.app_event_tx.clone(), first.features);
while let Some(delayed) = self.delayed_approval_requests.pop_back() {
modal.enqueue_request(delayed.request);
}
self.pause_status_timer_for_modal();
self.push_view(Box::new(modal));
}
/// Forward a key event to the active view or the composer.
pub fn handle_key_event(&mut self, key_event: KeyEvent) -> InputResult {
// If a modal/view is active, handle it here; otherwise forward to composer.
@@ -512,7 +567,21 @@ impl BottomPane {
self.request_redraw();
return InputResult::None;
}
let records_composer_activity =
matches!(key_event.kind, KeyEventKind::Press | KeyEventKind::Repeat)
&& !key_hint::has_ctrl_or_alt(key_event.modifiers)
&& matches!(
key_event.code,
KeyCode::Char(_)
| KeyCode::Backspace
| KeyCode::Delete
| KeyCode::Enter
| KeyCode::Tab
);
let (input_result, needs_redraw) = self.composer.handle_key_event(key_event);
if records_composer_activity {
self.record_composer_activity_at(Instant::now());
}
if needs_redraw {
self.request_redraw();
}
@@ -560,6 +629,7 @@ impl BottomPane {
}
pub fn handle_paste(&mut self, pasted: String) {
let has_pasted_text = !pasted.is_empty();
if let Some(view) = self.view_stack.last_mut() {
let needs_redraw = view.handle_paste(pasted);
let view_complete = view.is_complete();
@@ -572,6 +642,9 @@ impl BottomPane {
}
} else {
let needs_redraw = self.composer.handle_paste(pasted);
if has_pasted_text {
self.record_composer_activity_at(Instant::now());
}
self.composer.sync_popups();
if needs_redraw {
self.request_redraw();
@@ -586,7 +659,12 @@ impl BottomPane {
}
pub(crate) fn pre_draw_tick(&mut self) {
self.pre_draw_tick_at(Instant::now());
}
fn pre_draw_tick_at(&mut self, now: Instant) {
self.composer.sync_popups();
self.maybe_show_delayed_approval_requests_at(now);
}
/// Replace the composer text with `text`.
@@ -989,10 +1067,22 @@ impl BottomPane {
request
};
// Otherwise create a new approval modal overlay.
let modal = ApprovalOverlay::new(request, self.app_event_tx.clone(), features.clone());
self.pause_status_timer_for_modal();
self.push_view(Box::new(modal));
let now = Instant::now();
if !self.delayed_approval_requests.is_empty()
|| self.approval_prompt_delay_remaining(now).is_some()
{
self.delayed_approval_requests
.push_back(DelayedApprovalRequest {
request,
features: features.clone(),
});
self.maybe_show_delayed_approval_requests_at(now);
} else {
// No recent composer activity, so show the approval modal immediately.
let modal = ApprovalOverlay::new(request, self.app_event_tx.clone(), features.clone());
self.pause_status_timer_for_modal();
self.push_view(Box::new(modal));
}
}
/// Called when the agent requests user input.
@@ -1107,11 +1197,19 @@ impl BottomPane {
&mut self,
request: &ResolvedAppServerRequest,
) -> bool {
let delayed_len = self.delayed_approval_requests.len();
self.delayed_approval_requests
.retain(|delayed| !delayed.request.matches_resolved_request(request));
let delayed_changed = self.delayed_approval_requests.len() != delayed_len;
if self.view_stack.is_empty() {
return false;
if delayed_changed {
self.request_redraw();
}
return delayed_changed;
}
let mut changed = false;
let mut changed = delayed_changed;
let mut completed_indices = Vec::new();
for index in (0..self.view_stack.len()).rev() {
let view = &mut self.view_stack[index];
@@ -1356,6 +1454,7 @@ mod tests {
use crate::test_support::PathBufExt;
use crate::test_support::test_path_buf;
use codex_protocol::protocol::Op;
use codex_protocol::protocol::ReviewDecision;
use codex_protocol::protocol::SkillScope;
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
@@ -1367,6 +1466,7 @@ mod tests {
use ratatui::layout::Rect;
use std::cell::Cell;
use std::rc::Rc;
use std::time::Instant;
use tokio::sync::mpsc::unbounded_channel;
fn snapshot_buffer(buf: &Buffer) -> String {
@@ -1388,13 +1488,20 @@ mod tests {
}
fn test_pane(app_event_tx: AppEventSender) -> BottomPane {
test_pane_with_disable_paste_burst(app_event_tx, /*disable_paste_burst*/ false)
}
fn test_pane_with_disable_paste_burst(
app_event_tx: AppEventSender,
disable_paste_burst: bool,
) -> BottomPane {
BottomPane::new(BottomPaneParams {
app_event_tx,
frame_requester: FrameRequester::test_dummy(),
has_input_focus: true,
enhanced_keys_supported: false,
placeholder_text: "Ask Codex to do anything".to_string(),
disable_paste_burst: false,
disable_paste_burst,
animations_enabled: true,
skills: Some(Vec::new()),
})
@@ -1565,6 +1672,136 @@ mod tests {
);
}
#[test]
fn approval_request_shows_immediately_without_recent_typing() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let features = Features::with_defaults();
let mut pane = test_pane(tx);
pane.push_approval_request(exec_request(), &features);
assert_eq!(pane.view_stack.len(), 1);
assert!(pane.delayed_approval_requests.is_empty());
}
#[test]
fn approval_request_is_delayed_after_recent_typing() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let features = Features::with_defaults();
let mut pane = test_pane(tx);
let now = Instant::now();
pane.last_composer_activity_at = Some(now);
pane.push_approval_request(exec_request(), &features);
assert!(pane.view_stack.is_empty());
assert_eq!(pane.delayed_approval_requests.len(), 1);
pane.pre_draw_tick_at(
now + APPROVAL_PROMPT_TYPING_IDLE_DELAY - Duration::from_millis(/*millis*/ 1),
);
assert!(pane.view_stack.is_empty());
assert_eq!(pane.delayed_approval_requests.len(), 1);
pane.pre_draw_tick_at(now + APPROVAL_PROMPT_TYPING_IDLE_DELAY);
assert_eq!(pane.view_stack.len(), 1);
assert!(pane.delayed_approval_requests.is_empty());
}
#[test]
fn continued_typing_resets_delayed_approval_idle_deadline() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let features = Features::with_defaults();
let mut pane = test_pane(tx);
let first_activity = Instant::now();
pane.last_composer_activity_at = Some(first_activity);
pane.push_approval_request(exec_request(), &features);
let continued_activity = first_activity + Duration::from_millis(/*millis*/ 750);
pane.record_composer_activity_at(continued_activity);
pane.pre_draw_tick_at(first_activity + APPROVAL_PROMPT_TYPING_IDLE_DELAY);
assert!(pane.view_stack.is_empty());
assert_eq!(pane.delayed_approval_requests.len(), 1);
pane.pre_draw_tick_at(continued_activity + APPROVAL_PROMPT_TYPING_IDLE_DELAY);
assert_eq!(pane.view_stack.len(), 1);
assert!(pane.delayed_approval_requests.is_empty());
}
#[test]
fn typed_approval_shortcuts_during_delay_stay_in_composer() {
let (tx_raw, mut rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let features = Features::with_defaults();
let mut pane = test_pane_with_disable_paste_burst(tx, /*disable_paste_burst*/ true);
pane.last_composer_activity_at = Some(Instant::now());
pane.push_approval_request(exec_request(), &features);
pane.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE));
pane.handle_key_event(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE));
assert_eq!(pane.composer_text(), "ya");
assert!(pane.view_stack.is_empty());
assert_eq!(pane.delayed_approval_requests.len(), 1);
while let Ok(event) = rx.try_recv() {
assert!(
!matches!(event, AppEvent::SubmitThreadOp { .. }),
"delayed approval shortcut should not submit an approval: {event:?}"
);
}
}
#[test]
fn delayed_approval_shortcut_works_after_idle_deadline() {
let (tx_raw, mut rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let features = Features::with_defaults();
let mut pane = test_pane(tx);
let now = Instant::now();
pane.last_composer_activity_at = Some(now);
pane.push_approval_request(exec_request(), &features);
pane.pre_draw_tick_at(now + APPROVAL_PROMPT_TYPING_IDLE_DELAY);
pane.handle_key_event(KeyEvent::new(KeyCode::Char('y'), KeyModifiers::NONE));
let mut approval_decision = None;
while let Ok(event) = rx.try_recv() {
if let AppEvent::SubmitThreadOp {
op: Op::ExecApproval { decision, .. },
..
} = event
{
approval_decision = Some(decision);
}
}
assert_eq!(approval_decision, Some(ReviewDecision::Approved));
}
#[test]
fn dismiss_app_server_request_prunes_delayed_approval() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let features = Features::with_defaults();
let mut pane = test_pane(tx);
let now = Instant::now();
pane.last_composer_activity_at = Some(now);
pane.push_approval_request(exec_request(), &features);
assert!(
pane.dismiss_app_server_request(&ResolvedAppServerRequest::ExecApproval {
id: "1".to_string(),
})
);
assert!(pane.delayed_approval_requests.is_empty());
pane.pre_draw_tick_at(now + APPROVAL_PROMPT_TYPING_IDLE_DELAY);
assert!(pane.view_stack.is_empty());
}
#[test]
fn dismiss_app_server_request_removes_matching_buried_view() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
+23
View File
@@ -154,6 +154,8 @@ where
inline_styles: Vec<Style>,
indent_stack: Vec<IndentContext>,
list_indices: Vec<Option<u64>>,
list_needs_blank_before_next_item: Vec<bool>,
list_item_contains_code_block: Vec<bool>,
link: Option<LinkState>,
needs_newline: bool,
pending_marker_line: bool,
@@ -184,6 +186,8 @@ where
inline_styles: Vec::new(),
indent_stack: Vec::new(),
list_indices: Vec::new(),
list_needs_blank_before_next_item: Vec::new(),
list_item_contains_code_block: Vec::new(),
link: None,
needs_newline: false,
pending_marker_line: false,
@@ -292,6 +296,11 @@ where
TagEnd::CodeBlock => self.end_codeblock(),
TagEnd::List(_) => self.end_list(),
TagEnd::Item => {
if self.list_item_contains_code_block.pop().unwrap_or(false)
&& let Some(needs_blank) = self.list_needs_blank_before_next_item.last_mut()
{
*needs_blank = true;
}
self.indent_stack.pop();
self.pending_marker_line = false;
}
@@ -476,15 +485,26 @@ where
self.push_line(Line::default());
}
self.list_indices.push(index);
self.list_needs_blank_before_next_item.push(false);
}
fn end_list(&mut self) {
self.list_indices.pop();
self.list_needs_blank_before_next_item.pop();
self.needs_newline = true;
}
fn start_item(&mut self) {
if self
.list_needs_blank_before_next_item
.last_mut()
.map(std::mem::take)
.unwrap_or(false)
{
self.push_blank_line();
}
self.pending_marker_line = true;
self.list_item_contains_code_block.push(false);
let depth = self.list_indices.len();
let is_ordered = self
.list_indices
@@ -524,6 +544,9 @@ where
}
fn start_codeblock(&mut self, lang: Option<String>, indent: Option<Span<'static>>) {
for item_contains_code_block in &mut self.list_item_contains_code_block {
*item_contains_code_block = true;
}
self.flush_current_line();
if !self.text.lines.is_empty() {
self.push_blank_line();
+52
View File
@@ -15,6 +15,18 @@ fn render_markdown_text_for_cwd(input: &str, cwd: &Path) -> Text<'static> {
render_markdown_text_with_width_and_cwd(input, /*width*/ None, Some(cwd))
}
fn plain_lines(text: &Text<'_>) -> Vec<String> {
text.lines
.iter()
.map(|line| {
line.spans
.iter()
.map(|span| span.content.clone())
.collect::<String>()
})
.collect()
}
#[test]
fn empty() {
assert_eq!(render_markdown_text(""), Text::default());
@@ -1128,6 +1140,46 @@ fn code_block_inside_unordered_list_item_multiple_lines() {
assert_eq!(lines, vec!["- Item", "", " first", " second"]);
}
#[test]
fn list_item_after_code_block_keeps_blank_separator() {
let md = "1. First:\n\n ```rust\n fn first() {}\n ```\n\n2. Second:\n";
let text = render_markdown_text(md);
let lines = plain_lines(&text);
assert_eq!(
lines,
vec!["1. First:", "", " fn first() {}", "", "2. Second:"]
);
assert_snapshot!(
"list_item_after_code_block_keeps_blank_separator",
lines.join("\n")
);
}
#[test]
fn outer_list_item_after_nested_code_block_keeps_blank_separator() {
let md = "1. First:\n - Nested:\n\n ```rust\n fn first() {}\n ```\n\n2. Second:\n";
let text = render_markdown_text(md);
let lines = plain_lines(&text);
assert_eq!(
lines,
vec![
"1. First:",
" - Nested:",
"",
" fn first() {}",
"",
"2. Second:",
]
);
}
#[test]
fn list_item_after_simple_item_stays_compact() {
let md = "1. First\n\n2. Second\n";
let text = render_markdown_text(md);
assert_eq!(plain_lines(&text), vec!["1. First", "2. Second"]);
}
#[test]
fn markdown_render_complex_snapshot() {
let md = r#"# H1: Markdown Streaming Test
@@ -0,0 +1,9 @@
---
source: tui/src/markdown_render_tests.rs
expression: "lines.join(\"\\n\")"
---
1. First:
fn first() {}
2. Second: