Merge a45c74f569 into sapling-pr-archive-bolinfest

This commit is contained in:
Michael Bolin
2026-06-02 12:30:35 -07:00
committed by GitHub
55 changed files with 1267 additions and 286 deletions
+14 -3
View File
@@ -134,11 +134,22 @@ jobs:
- name: Stage Windows binaries
shell: bash
run: |
output_dir="target/${{ matrix.target }}/release/staged-${{ matrix.bundle }}"
release_dir="target/${{ matrix.target }}/release"
output_dir="$release_dir/staged-${{ matrix.bundle }}"
mkdir -p "$output_dir"
for binary in ${{ matrix.binaries }}; do
cp "target/${{ matrix.target }}/release/${binary}.exe" "$output_dir/${binary}.exe"
cp "target/${{ matrix.target }}/release/${binary}.pdb" "$output_dir/${binary}.pdb"
pdb_name="${binary//-/_}"
pdb_path="$release_dir/${pdb_name}.pdb"
if [[ ! -f "$pdb_path" ]]; then
pdb_path="$release_dir/${binary}.pdb"
fi
if [[ ! -f "$pdb_path" ]]; then
echo "PDB for $binary not found at $release_dir/${pdb_name}.pdb or $release_dir/${binary}.pdb" >&2
exit 1
fi
cp "$release_dir/${binary}.exe" "$output_dir/${binary}.exe"
cp "$pdb_path" "$output_dir/${binary}.pdb"
done
- name: Upload Windows binaries
@@ -62,6 +62,7 @@ use crate::facts::SkillInvokedInput;
use crate::facts::SubAgentThreadStartedInput;
use crate::facts::ThreadInitializationMode;
use crate::facts::TrackEventsContext;
use crate::facts::TurnCodexErrorFact;
use crate::facts::TurnResolvedConfigFact;
use crate::facts::TurnStatus;
use crate::facts::TurnSteerRequestError;
@@ -132,6 +133,7 @@ use codex_plugin::PluginTelemetryMetadata;
use codex_protocol::approvals::NetworkApprovalProtocol;
use codex_protocol::config_types::ApprovalsReviewer;
use codex_protocol::config_types::ModeKind;
use codex_protocol::error::CodexErr;
use codex_protocol::models::NetworkPermissions as CoreNetworkPermissions;
use codex_protocol::models::PermissionProfile as CorePermissionProfile;
use codex_protocol::protocol::AskForApproval;
@@ -833,6 +835,7 @@ fn sample_permissions_approval_request(request_id: i64) -> ServerRequest {
thread_id: "thread-1".to_string(),
turn_id: "turn-1".to_string(),
item_id: "permissions-1".to_string(),
environment_id: None,
started_at_ms: 1_000,
cwd: test_path_buf("/tmp").abs(),
reason: Some("need network".to_string()),
@@ -3256,6 +3259,9 @@ fn turn_event_serializes_expected_shape() {
is_first_turn: true,
status: Some(TurnStatus::Completed),
turn_error: None,
codex_error_kind: None,
codex_error_subreason: None,
codex_error_http_status_code: None,
steer_count: Some(0),
total_tool_call_count: None,
shell_command_count: None,
@@ -3318,6 +3324,9 @@ fn turn_event_serializes_expected_shape() {
"is_first_turn": true,
"status": "completed",
"turn_error": null,
"codex_error_kind": null,
"codex_error_subreason": null,
"codex_error_http_status_code": null,
"steer_count": 0,
"total_tool_call_count": null,
"shell_command_count": null,
@@ -3983,6 +3992,18 @@ async fn turn_lifecycle_emits_failed_turn_event() {
/*include_token_usage*/ false,
)
.await;
reducer
.ingest(
AnalyticsFact::Custom(CustomAnalyticsFact::TurnCodexError(Box::new(
TurnCodexErrorFact::from_codex_err(
"thread-2".to_string(),
"turn-2".to_string(),
&CodexErr::InvalidRequest("unknown turn environment id `env-2`".to_string()),
),
))),
&mut out,
)
.await;
reducer
.ingest(
AnalyticsFact::Notification(Box::new(sample_turn_completed_notification(
@@ -3999,6 +4020,18 @@ async fn turn_lifecycle_emits_failed_turn_event() {
let payload = serde_json::to_value(&out[0]).expect("serialize turn event");
assert_eq!(payload["event_params"]["status"], json!("failed"));
assert_eq!(payload["event_params"]["turn_error"], json!("badRequest"));
assert_eq!(
payload["event_params"]["codex_error_kind"],
json!("invalid_request")
);
assert_eq!(
payload["event_params"]["codex_error_subreason"],
json!("unknown turn environment id `env-2`")
);
assert_eq!(
payload["event_params"]["codex_error_http_status_code"],
json!(null)
);
}
#[tokio::test]
@@ -4031,6 +4064,7 @@ async fn turn_lifecycle_emits_interrupted_turn_event_without_error() {
let payload = serde_json::to_value(&out[0]).expect("serialize turn event");
assert_eq!(payload["event_params"]["status"], json!("interrupted"));
assert_eq!(payload["event_params"]["turn_error"], json!(null));
assert_eq!(payload["event_params"]["codex_error_kind"], json!(null));
}
#[tokio::test]
+7
View File
@@ -18,6 +18,7 @@ use crate::facts::SkillInvocation;
use crate::facts::SkillInvokedInput;
use crate::facts::SubAgentThreadStartedInput;
use crate::facts::TrackEventsContext;
use crate::facts::TurnCodexErrorFact;
use crate::facts::TurnResolvedConfigFact;
use crate::facts::TurnTokenUsageFact;
use crate::reducer::AnalyticsReducer;
@@ -256,6 +257,12 @@ impl AnalyticsEventsClient {
)));
}
pub fn track_turn_codex_error(&self, fact: TurnCodexErrorFact) {
self.record_fact(AnalyticsFact::Custom(CustomAnalyticsFact::TurnCodexError(
Box::new(fact),
)));
}
pub fn track_plugin_installed(&self, plugin: PluginTelemetryMetadata) {
self.record_fact(AnalyticsFact::Custom(
CustomAnalyticsFact::PluginStateChanged(PluginStateChangedInput {
+4
View File
@@ -3,6 +3,7 @@ use std::time::Instant;
use crate::facts::AcceptedLineFingerprint;
use crate::facts::AppInvocation;
use crate::facts::CodexCompactionEvent;
use crate::facts::CodexErrKind;
use crate::facts::CompactionImplementation;
use crate::facts::CompactionPhase;
use crate::facts::CompactionReason;
@@ -797,6 +798,9 @@ pub(crate) struct CodexTurnEventParams {
pub(crate) is_first_turn: bool,
pub(crate) status: Option<TurnStatus>,
pub(crate) turn_error: Option<CodexErrorInfo>,
pub(crate) codex_error_kind: Option<CodexErrKind>,
pub(crate) codex_error_subreason: Option<String>,
pub(crate) codex_error_http_status_code: Option<u16>,
pub(crate) steer_count: Option<usize>,
pub(crate) total_tool_call_count: Option<usize>,
pub(crate) shell_command_count: Option<usize>,
+146
View File
@@ -15,6 +15,7 @@ use codex_protocol::config_types::ModeKind;
use codex_protocol::config_types::Personality;
use codex_protocol::config_types::ReasoningSummary;
use codex_protocol::config_types::ServiceTier;
use codex_protocol::error::CodexErr;
use codex_protocol::models::PermissionProfile;
use codex_protocol::openai_models::ReasoningEffort;
use codex_protocol::protocol::AskForApproval;
@@ -29,6 +30,9 @@ use codex_protocol::request_permissions::RequestPermissionsResponse;
use serde::Serialize;
use std::path::PathBuf;
const INVALID_REQUEST_SUBREASON_MAX_BYTES: usize = 512;
const INVALID_REQUEST_SUBREASON_TRUNCATION_SUFFIX: &str = "...";
#[derive(Clone, Debug, PartialEq, Eq, Serialize)]
pub struct AcceptedLineFingerprint {
pub path_hash: String,
@@ -99,6 +103,147 @@ pub struct TurnTokenUsageFact {
pub token_usage: TokenUsage,
}
#[derive(Clone)]
pub struct TurnCodexErrorFact {
pub(crate) turn_id: String,
pub(crate) thread_id: String,
pub(crate) error: TurnCodexError,
}
impl TurnCodexErrorFact {
pub fn from_codex_err(thread_id: String, turn_id: String, error: &CodexErr) -> Self {
Self {
turn_id,
thread_id,
error: TurnCodexError::from_codex_err(error),
}
}
}
#[derive(Clone, Copy, Debug, Serialize)]
#[serde(rename_all = "snake_case")]
pub(crate) enum CodexErrKind {
TurnAborted,
Stream,
ContextWindowExceeded,
ThreadNotFound,
AgentLimitReached,
SessionConfiguredNotFirstEvent,
Timeout,
RequestTimeout,
Spawn,
Interrupted,
UnexpectedStatus,
InvalidRequest,
InvalidImageRequest,
UsageLimitReached,
ServerOverloaded,
CyberPolicy,
ResponseStreamFailed,
ConnectionFailed,
QuotaExceeded,
UsageNotIncluded,
InternalServerError,
RetryLimit,
InternalAgentDied,
Sandbox,
LandlockSandboxExecutableNotProvided,
UnsupportedOperation,
RefreshTokenFailed,
Fatal,
Io,
Json,
#[cfg(target_os = "linux")]
LandlockRuleset,
#[cfg(target_os = "linux")]
LandlockPathFd,
TokioJoin,
EnvVar,
}
#[derive(Clone)]
pub(crate) struct TurnCodexError {
pub(crate) kind: CodexErrKind,
pub(crate) subreason: Option<String>,
pub(crate) http_status_code: Option<u16>,
}
impl TurnCodexError {
fn from_codex_err(error: &CodexErr) -> Self {
Self {
kind: error.into(),
subreason: match error {
CodexErr::InvalidRequest(message) => {
// InvalidRequest can contain raw provider response bodies, so bound the
// analytics copy without changing the source CodexErr.
let subreason = if message.len() <= INVALID_REQUEST_SUBREASON_MAX_BYTES {
message.clone()
} else {
let truncated_len = message.floor_char_boundary(
INVALID_REQUEST_SUBREASON_MAX_BYTES
.saturating_sub(INVALID_REQUEST_SUBREASON_TRUNCATION_SUFFIX.len()),
);
format!(
"{}{INVALID_REQUEST_SUBREASON_TRUNCATION_SUFFIX}",
&message[..truncated_len]
)
};
Some(subreason)
}
_ => None,
},
http_status_code: error.http_status_code_value(),
}
}
}
impl From<&CodexErr> for CodexErrKind {
fn from(error: &CodexErr) -> Self {
match error {
CodexErr::TurnAborted => CodexErrKind::TurnAborted,
CodexErr::Stream(..) => CodexErrKind::Stream,
CodexErr::ContextWindowExceeded => CodexErrKind::ContextWindowExceeded,
CodexErr::ThreadNotFound(_) => CodexErrKind::ThreadNotFound,
CodexErr::AgentLimitReached { .. } => CodexErrKind::AgentLimitReached,
CodexErr::SessionConfiguredNotFirstEvent => {
CodexErrKind::SessionConfiguredNotFirstEvent
}
CodexErr::Timeout => CodexErrKind::Timeout,
CodexErr::RequestTimeout => CodexErrKind::RequestTimeout,
CodexErr::Spawn => CodexErrKind::Spawn,
CodexErr::Interrupted => CodexErrKind::Interrupted,
CodexErr::UnexpectedStatus(_) => CodexErrKind::UnexpectedStatus,
CodexErr::InvalidRequest(_) => CodexErrKind::InvalidRequest,
CodexErr::InvalidImageRequest() => CodexErrKind::InvalidImageRequest,
CodexErr::UsageLimitReached(_) => CodexErrKind::UsageLimitReached,
CodexErr::ServerOverloaded => CodexErrKind::ServerOverloaded,
CodexErr::CyberPolicy { .. } => CodexErrKind::CyberPolicy,
CodexErr::ResponseStreamFailed(_) => CodexErrKind::ResponseStreamFailed,
CodexErr::ConnectionFailed(_) => CodexErrKind::ConnectionFailed,
CodexErr::QuotaExceeded => CodexErrKind::QuotaExceeded,
CodexErr::UsageNotIncluded => CodexErrKind::UsageNotIncluded,
CodexErr::InternalServerError => CodexErrKind::InternalServerError,
CodexErr::RetryLimit(_) => CodexErrKind::RetryLimit,
CodexErr::InternalAgentDied => CodexErrKind::InternalAgentDied,
CodexErr::Sandbox(_) => CodexErrKind::Sandbox,
CodexErr::LandlockSandboxExecutableNotProvided => {
CodexErrKind::LandlockSandboxExecutableNotProvided
}
CodexErr::UnsupportedOperation(_) => CodexErrKind::UnsupportedOperation,
CodexErr::RefreshTokenFailed(_) => CodexErrKind::RefreshTokenFailed,
CodexErr::Fatal(_) => CodexErrKind::Fatal,
CodexErr::Io(_) => CodexErrKind::Io,
CodexErr::Json(_) => CodexErrKind::Json,
#[cfg(target_os = "linux")]
CodexErr::LandlockRuleset(_) => CodexErrKind::LandlockRuleset,
#[cfg(target_os = "linux")]
CodexErr::LandlockPathFd(_) => CodexErrKind::LandlockPathFd,
CodexErr::TokioJoin(_) => CodexErrKind::TokioJoin,
CodexErr::EnvVar(_) => CodexErrKind::EnvVar,
}
}
}
#[derive(Clone, Copy, Debug, Serialize)]
#[serde(rename_all = "snake_case")]
pub enum TurnStatus {
@@ -329,6 +474,7 @@ pub(crate) enum CustomAnalyticsFact {
GuardianReview(Box<GuardianReviewEventParams>),
TurnResolvedConfig(Box<TurnResolvedConfigFact>),
TurnTokenUsage(Box<TurnTokenUsageFact>),
TurnCodexError(Box<TurnCodexErrorFact>),
SkillInvoked(SkillInvokedInput),
AppMentioned(AppMentionedInput),
AppUsed(AppUsedInput),
+1
View File
@@ -38,6 +38,7 @@ pub use facts::SkillInvocation;
pub use facts::SubAgentThreadStartedInput;
pub use facts::ThreadInitializationMode;
pub use facts::TrackEventsContext;
pub use facts::TurnCodexErrorFact;
pub use facts::TurnResolvedConfigFact;
pub use facts::TurnStatus;
pub use facts::TurnSteerRejectionReason;
+39
View File
@@ -70,6 +70,8 @@ use crate::facts::PluginUsedInput;
use crate::facts::SkillInvokedInput;
use crate::facts::SubAgentThreadStartedInput;
use crate::facts::ThreadInitializationMode;
use crate::facts::TurnCodexError;
use crate::facts::TurnCodexErrorFact;
use crate::facts::TurnResolvedConfigFact;
use crate::facts::TurnStatus;
use crate::facts::TurnSteerRejectionReason;
@@ -322,6 +324,7 @@ struct TurnState {
started_at: Option<u64>,
token_usage: Option<TokenUsage>,
completed: Option<CompletedTurnState>,
codex_error: Option<TurnCodexError>,
latest_diff: Option<String>,
steer_count: usize,
tool_counts: TurnToolCounts,
@@ -461,6 +464,9 @@ impl AnalyticsReducer {
CustomAnalyticsFact::TurnTokenUsage(input) => {
self.ingest_turn_token_usage(*input, out).await;
}
CustomAnalyticsFact::TurnCodexError(input) => {
self.ingest_turn_codex_error(*input);
}
CustomAnalyticsFact::SkillInvoked(input) => {
self.ingest_skill_invoked(input, out).await;
}
@@ -606,6 +612,7 @@ impl AnalyticsReducer {
started_at: None,
token_usage: None,
completed: None,
codex_error: None,
latest_diff: None,
steer_count: 0,
tool_counts: TurnToolCounts::default(),
@@ -630,6 +637,7 @@ impl AnalyticsReducer {
started_at: None,
token_usage: None,
completed: None,
codex_error: None,
latest_diff: None,
steer_count: 0,
tool_counts: TurnToolCounts::default(),
@@ -639,6 +647,29 @@ impl AnalyticsReducer {
self.maybe_emit_turn_event(&turn_id, out).await;
}
fn ingest_turn_codex_error(&mut self, input: TurnCodexErrorFact) {
let TurnCodexErrorFact {
turn_id,
thread_id,
error,
} = input;
let turn_state = self.turns.entry(turn_id).or_insert(TurnState {
connection_id: None,
thread_id: None,
num_input_images: None,
resolved_config: None,
started_at: None,
token_usage: None,
completed: None,
codex_error: None,
latest_diff: None,
steer_count: 0,
tool_counts: TurnToolCounts::default(),
});
turn_state.thread_id.get_or_insert(thread_id);
turn_state.codex_error = Some(error);
}
async fn ingest_skill_invoked(
&mut self,
input: SkillInvokedInput,
@@ -795,6 +826,7 @@ impl AnalyticsReducer {
started_at: None,
token_usage: None,
completed: None,
codex_error: None,
latest_diff: None,
steer_count: 0,
tool_counts: TurnToolCounts::default(),
@@ -1154,6 +1186,7 @@ impl AnalyticsReducer {
started_at: None,
token_usage: None,
completed: None,
codex_error: None,
latest_diff: None,
steer_count: 0,
tool_counts: TurnToolCounts::default(),
@@ -1175,6 +1208,7 @@ impl AnalyticsReducer {
started_at: None,
token_usage: None,
completed: None,
codex_error: None,
latest_diff: None,
steer_count: 0,
tool_counts: TurnToolCounts::default(),
@@ -1194,6 +1228,7 @@ impl AnalyticsReducer {
started_at: None,
token_usage: None,
completed: None,
codex_error: None,
latest_diff: None,
steer_count: 0,
tool_counts: TurnToolCounts::default(),
@@ -2451,6 +2486,7 @@ fn codex_turn_event_params(
is_first_turn,
} = resolved_config;
let token_usage = turn_state.token_usage.clone();
let codex_error = turn_state.codex_error.as_ref();
CodexTurnEventParams {
thread_id,
session_id: thread_metadata.session_id.clone(),
@@ -2483,6 +2519,9 @@ fn codex_turn_event_params(
is_first_turn,
status: completed.status,
turn_error: completed.turn_error,
codex_error_kind: codex_error.map(|error| error.kind),
codex_error_subreason: codex_error.and_then(|error| error.subreason.clone()),
codex_error_http_status_code: codex_error.and_then(|error| error.http_status_code),
steer_count: Some(turn_state.steer_count),
total_tool_call_count: Some(turn_state.tool_counts.total),
shell_command_count: Some(turn_state.tool_counts.shell_command),
@@ -285,6 +285,13 @@
"cwd": {
"$ref": "#/definitions/AbsolutePathBuf"
},
"environmentId": {
"default": null,
"type": [
"null",
"string"
]
},
"itemId": {
"type": "string"
},
@@ -319,4 +326,4 @@
],
"title": "PermissionsRequestApprovalParams",
"type": "object"
}
}
+8 -1
View File
@@ -1590,6 +1590,13 @@
"cwd": {
"$ref": "#/definitions/AbsolutePathBuf"
},
"environmentId": {
"default": null,
"type": [
"null",
"string"
]
},
"itemId": {
"type": "string"
},
@@ -1998,4 +2005,4 @@
}
],
"title": "ServerRequest"
}
}
@@ -3783,6 +3783,13 @@
"cwd": {
"$ref": "#/definitions/v2/AbsolutePathBuf"
},
"environmentId": {
"default": null,
"type": [
"null",
"string"
]
},
"itemId": {
"type": "string"
},
@@ -19232,4 +19239,4 @@
},
"title": "CodexAppServerProtocol",
"type": "object"
}
}
@@ -4,7 +4,7 @@
import type { AbsolutePathBuf } from "../AbsolutePathBuf";
import type { RequestPermissionProfile } from "./RequestPermissionProfile";
export type PermissionsRequestApprovalParams = { threadId: string, turnId: string, itemId: string,
export type PermissionsRequestApprovalParams = { threadId: string, turnId: string, itemId: string, environmentId: string | null,
/**
* Unix timestamp (in milliseconds) when this approval request started.
*/
@@ -663,6 +663,8 @@ pub struct PermissionsRequestApprovalParams {
pub thread_id: String,
pub turn_id: String,
pub item_id: String,
#[serde(default)]
pub environment_id: Option<String>,
/// Unix timestamp (in milliseconds) when this approval request started.
#[ts(type = "number")]
pub started_at_ms: i64,
@@ -400,6 +400,7 @@ fn permissions_request_approval_uses_request_permission_profile() {
"threadId": "thr_123",
"turnId": "turn_123",
"itemId": "call_123",
"environmentId": "remote",
"startedAtMs": 1,
"cwd": absolute_path_string("repo"),
"reason": "Select a workspace root",
@@ -416,6 +417,7 @@ fn permissions_request_approval_uses_request_permission_profile() {
.expect("permissions request should deserialize");
assert_eq!(params.cwd, absolute_path("repo"));
assert_eq!(params.environment_id.as_deref(), Some("remote"));
assert_eq!(
params.permissions,
RequestPermissionProfile {
+2 -1
View File
@@ -1379,7 +1379,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 project-root permissions 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 `environmentId` and `cwd` fields identify the environment and directory used to resolve project-root permissions and relative deny globs.
```json
{
@@ -1389,6 +1389,7 @@ The built-in `request_permissions` tool sends an `item/permissions/requestApprov
"threadId": "thr_123",
"turnId": "turn_123",
"itemId": "call_123",
"environmentId": "local",
"cwd": "/Users/me/project",
"reason": "Select a workspace root",
"permissions": {
@@ -761,6 +761,7 @@ pub(crate) async fn apply_bespoke_event_handling(
thread_id: conversation_id.to_string(),
turn_id: request.turn_id.clone(),
item_id: request.call_id.clone(),
environment_id: request.environment_id.clone(),
started_at_ms: request.started_at_ms,
cwd: request_cwd.clone(),
reason: request.reason,
@@ -0,0 +1,214 @@
use std::path::Path;
use std::time::Duration;
use anyhow::Context;
use anyhow::Result;
use app_test_support::ChatGptAuthFixture;
use app_test_support::McpProcess;
use app_test_support::to_response;
use app_test_support::write_chatgpt_auth;
use codex_app_server_protocol::ItemCompletedNotification;
use codex_app_server_protocol::JSONRPCResponse;
use codex_app_server_protocol::RequestId;
use codex_app_server_protocol::ThreadItem;
use codex_app_server_protocol::ThreadStartParams;
use codex_app_server_protocol::ThreadStartResponse;
use codex_app_server_protocol::TurnStartParams;
use codex_app_server_protocol::TurnStartResponse;
use codex_app_server_protocol::UserInput as V2UserInput;
use codex_config::types::AuthCredentialsStoreMode;
use core_test_support::responses;
use pretty_assertions::assert_eq;
use serde_json::json;
use tempfile::TempDir;
use tokio::time::timeout;
use wiremock::Mock;
use wiremock::MockServer;
use wiremock::ResponseTemplate;
use wiremock::matchers::method;
use wiremock::matchers::path;
const RESULT: &str = "cG5n";
// macOS and Windows Bazel CI can spend tens of seconds starting app-server
// subprocesses or processing test RPCs under load.
#[cfg(any(target_os = "macos", windows))]
const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(60);
#[cfg(not(any(target_os = "macos", windows)))]
const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(10);
#[tokio::test]
async fn standalone_image_generation_persists_image_and_returns_it_to_model() -> Result<()> {
let call_id = "image-run-1";
let server = responses::start_mock_server().await;
mount_image_response(&server).await;
let response_mock = responses::mount_sse_sequence(
&server,
vec![
responses::sse(vec![
responses::ev_response_created("resp-1"),
responses::ev_function_call_with_namespace(
call_id,
"image_gen",
"imagegen",
&json!({
"action": "generate",
"prompt": "paint a blue whale",
})
.to_string(),
),
responses::ev_completed("resp-1"),
]),
responses::sse(vec![
responses::ev_assistant_message("msg-1", "Done"),
responses::ev_completed("resp-2"),
]),
],
)
.await;
let codex_home = TempDir::new()?;
create_config_toml(codex_home.path(), &server.uri())?;
write_chatgpt_auth(
codex_home.path(),
ChatGptAuthFixture::new("access-chatgpt"),
AuthCredentialsStoreMode::File,
)?;
let mut mcp = McpProcess::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]).await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
let thread_req = mcp
.send_thread_start_request(ThreadStartParams::default())
.await?;
let thread_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(thread_req)),
)
.await??;
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(thread_resp)?;
let turn_req = mcp
.send_turn_start_request(TurnStartParams {
thread_id: thread.id,
client_user_message_id: None,
input: vec![V2UserInput::Text {
text: "Generate an image".to_string(),
text_elements: Vec::new(),
}],
..Default::default()
})
.await?;
let turn_resp: JSONRPCResponse = timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_response_message(RequestId::Integer(turn_req)),
)
.await??;
let _turn: TurnStartResponse = to_response::<TurnStartResponse>(turn_resp)?;
let completed = timeout(
DEFAULT_READ_TIMEOUT,
wait_for_image_generation_completed(&mut mcp),
)
.await??;
timeout(
DEFAULT_READ_TIMEOUT,
mcp.read_stream_until_notification_message("turn/completed"),
)
.await??;
let ThreadItem::ImageGeneration {
status,
revised_prompt,
result,
saved_path: Some(saved_path),
..
} = completed.item
else {
panic!("expected completed image generation item with saved path");
};
assert_eq!(status, "completed");
assert_eq!(revised_prompt.as_deref(), Some("paint a blue whale"));
assert_eq!(result, RESULT);
assert_eq!(std::fs::read(&saved_path)?, b"png");
let requests = response_mock.requests();
assert_eq!(requests.len(), 2);
let output = requests[1].function_call_output(call_id);
assert_eq!(
output["output"][0],
json!({
"type": "input_image",
"image_url": format!("data:image/png;base64,{RESULT}"),
"detail": "high",
})
);
assert_eq!(output["output"].as_array().map(Vec::len), Some(1));
assert!(
!requests[1]
.message_input_texts("developer")
.iter()
.any(|text| text.contains("Generated images are saved to")),
"standalone image generation should not emit the legacy developer-message hint"
);
Ok(())
}
async fn wait_for_image_generation_completed(
mcp: &mut McpProcess,
) -> Result<ItemCompletedNotification> {
loop {
let notification = mcp
.read_stream_until_notification_message("item/completed")
.await?;
let completed: ItemCompletedNotification = serde_json::from_value(
notification
.params
.context("item/completed notification should include params")?,
)?;
if matches!(&completed.item, ThreadItem::ImageGeneration { .. }) {
return Ok(completed);
}
}
}
async fn mount_image_response(server: &MockServer) {
Mock::given(method("POST"))
.and(path("/api/codex/images/generations"))
.respond_with(ResponseTemplate::new(200).set_body_json(json!({
"created": 1,
"data": [{"b64_json": RESULT}],
})))
.expect(1)
.mount(server)
.await;
}
fn create_config_toml(codex_home: &Path, server_uri: &str) -> std::io::Result<()> {
std::fs::write(
codex_home.join("config.toml"),
format!(
r#"
model = "mock-model"
approval_policy = "never"
sandbox_mode = "read-only"
model_provider = "openai-custom"
chatgpt_base_url = "{server_uri}"
[features]
imagegenext = true
[model_providers.openai-custom]
name = "OpenAI"
base_url = "{server_uri}/api/codex"
wire_api = "responses"
request_max_retries = 0
stream_max_retries = 0
supports_websockets = false
requires_openai_auth = true
"#
),
)
}
@@ -17,6 +17,7 @@ mod experimental_feature_list;
mod external_agent_config;
mod fs;
mod hooks_list;
mod image_generation;
mod initialize;
mod marketplace_add;
mod marketplace_remove;
+3 -3
View File
@@ -738,11 +738,11 @@ impl ConfigToml {
.or(if sandbox_mode_was_explicit {
None
} else {
// If no sandbox_mode is set but this directory has a trust decision,
// default to workspace-write except on unsandboxed Windows where we
// If no sandbox_mode is set for a trusted directory, default
// to workspace-write except on unsandboxed Windows where we
// default to read-only.
active_project.and_then(|p| {
if p.is_trusted() || p.is_untrusted() {
if p.is_trusted() {
if cfg!(target_os = "windows")
&& windows_sandbox_level == WindowsSandboxLevel::Disabled
{
+1
View File
@@ -755,6 +755,7 @@ async fn handle_request_permissions(
) {
let call_id = event.call_id;
let args = RequestPermissionsArgs {
environment_id: event.environment_id,
reason: event.reason,
permissions: event.permissions,
};
+5 -1
View File
@@ -183,9 +183,11 @@ async fn run_codex_thread_interactive_respects_pre_cancelled_spawn() {
#[tokio::test]
async fn handle_request_permissions_uses_tool_call_id_for_round_trip() {
let (parent_session, parent_ctx, rx_events) =
let (parent_session, mut parent_ctx, rx_events) =
crate::session::tests::make_session_and_context_with_rx().await;
*parent_session.active_turn.lock().await = Some(crate::state::ActiveTurn::default());
let parent_ctx_mut = Arc::get_mut(&mut parent_ctx).expect("single turn context ref");
parent_ctx_mut.environments.turn_environments[0].environment_id = "remote".to_string();
let (tx_sub, rx_sub) = bounded(SUBMISSION_CHANNEL_CAPACITY);
let (_tx_events, rx_events_child) = bounded(SUBMISSION_CHANNEL_CAPACITY);
@@ -228,6 +230,7 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() {
RequestPermissionsEvent {
call_id: request_call_id,
turn_id: "child-turn-1".to_string(),
environment_id: Some("remote".to_string()),
started_at_ms: 0,
reason: Some("need access".to_string()),
permissions: RequestPermissionProfile {
@@ -252,6 +255,7 @@ async fn handle_request_permissions_uses_tool_call_id_for_round_trip() {
panic!("expected RequestPermissions event");
};
assert_eq!(request.call_id, call_id.clone());
assert_eq!(request.environment_id.as_deref(), Some("remote"));
assert_eq!(request.cwd, Some(delegated_cwd));
parent_session
+2
View File
@@ -240,6 +240,7 @@ async fn run_compact_task_inner_impl(
continue;
}
sess.set_total_tokens_full(turn_context.as_ref()).await;
sess.track_turn_codex_error(turn_context.as_ref(), &e);
let event = EventMsg::Error(e.to_error_event(/*message_prefix*/ None));
sess.send_event(&turn_context, event).await;
return Err(e);
@@ -257,6 +258,7 @@ async fn run_compact_task_inner_impl(
tokio::time::sleep(delay).await;
continue;
} else {
sess.track_turn_codex_error(turn_context.as_ref(), &e);
let event = EventMsg::Error(e.to_error_event(/*message_prefix*/ None));
sess.send_event(&turn_context, event).await;
return Err(e);
+1
View File
@@ -138,6 +138,7 @@ async fn run_remote_compact_task_inner(
}
attempt.track(sess.as_ref(), status, error.clone()).await;
if let Err(err) = result {
sess.track_turn_codex_error(turn_context, &err);
let event = EventMsg::Error(
err.to_error_event(Some("Error running remote compact task".to_string())),
);
+1
View File
@@ -155,6 +155,7 @@ async fn run_remote_compact_task_inner(
}
attempt.track(sess.as_ref(), status, error.clone()).await;
if let Err(err) = result {
sess.track_turn_codex_error(turn_context, &err);
let event = EventMsg::Error(
err.to_error_event(Some("Error running remote compact task".to_string())),
);
@@ -1486,11 +1486,7 @@ async fn system_allowed_permissions_keep_builtin_permission_fallbacks() -> anyho
),
(
Some(TrustLevel::Untrusted),
if cfg!(target_os = "windows") {
BUILT_IN_PERMISSION_PROFILE_READ_ONLY
} else {
BUILT_IN_PERMISSION_PROFILE_WORKSPACE
},
BUILT_IN_PERMISSION_PROFILE_READ_ONLY,
),
(None, BUILT_IN_PERMISSION_PROFILE_READ_ONLY),
] {
+56 -31
View File
@@ -2551,6 +2551,50 @@ async fn empty_config_defaults_to_builtin_profile_for_trusted_project() -> std::
Ok(())
}
#[tokio::test]
async fn empty_config_defaults_to_builtin_read_only_for_untrusted_project() -> std::io::Result<()> {
let codex_home = TempDir::new()?;
let cwd = TempDir::new()?;
let project_key = cwd.path().to_string_lossy().to_string();
let config = Config::load_from_base_config_with_overrides(
ConfigToml {
projects: Some(HashMap::from([(
project_key,
ProjectConfig {
trust_level: Some(TrustLevel::Untrusted),
},
)])),
..Default::default()
},
ConfigOverrides {
cwd: Some(cwd.path().to_path_buf()),
..Default::default()
},
codex_home.abs(),
)
.await?;
let policy = config.permissions.file_system_sandbox_policy();
assert_eq!(
config
.permissions
.active_permission_profile()
.as_ref()
.map(|active| active.id.as_str()),
Some(BUILT_IN_PERMISSION_PROFILE_READ_ONLY)
);
assert!(
policy.can_read_path_with_cwd(cwd.path(), cwd.path()),
"expected untrusted project fallback to allow reads, policy: {policy:?}"
);
assert!(
!policy.can_write_path_with_cwd(cwd.path(), cwd.path()),
"expected untrusted project fallback to use :read-only, policy: {policy:?}"
);
Ok(())
}
#[tokio::test]
async fn implicit_builtin_workspace_profile_preserves_sandbox_workspace_write_settings()
-> std::io::Result<()> {
@@ -8579,7 +8623,7 @@ async fn test_load_config_rejects_legacy_ollama_chat_provider_with_helpful_error
}
#[tokio::test]
async fn test_untrusted_project_gets_workspace_write_sandbox() -> anyhow::Result<()> {
async fn test_untrusted_project_gets_read_only_sandbox() -> anyhow::Result<()> {
let config_with_untrusted = r#"
[projects."/tmp/test"]
trust_level = "untrusted"
@@ -8601,18 +8645,10 @@ trust_level = "untrusted"
)
.await;
// Verify that untrusted projects get WorkspaceWrite (or ReadOnly on Windows due to downgrade)
if cfg!(target_os = "windows") {
assert!(
matches!(resolution, SandboxPolicy::ReadOnly { .. }),
"Expected ReadOnly on Windows, got {resolution:?}"
);
} else {
assert!(
matches!(resolution, SandboxPolicy::WorkspaceWrite { .. }),
"Expected WorkspaceWrite for untrusted project, got {resolution:?}"
);
}
assert!(
matches!(resolution, SandboxPolicy::ReadOnly { .. }),
"Expected ReadOnly for untrusted project, got {resolution:?}"
);
Ok(())
}
@@ -8971,24 +9007,13 @@ async fn test_untrusted_project_gets_unless_trusted_approval_policy() -> anyhow:
"Expected UnlessTrusted approval policy for untrusted project"
);
// Verify that untrusted projects still get WorkspaceWrite sandbox (or ReadOnly on Windows)
if cfg!(target_os = "windows") {
assert!(
matches!(
&config.legacy_sandbox_policy(),
SandboxPolicy::ReadOnly { .. }
),
"Expected ReadOnly on Windows"
);
} else {
assert!(
matches!(
&config.legacy_sandbox_policy(),
SandboxPolicy::WorkspaceWrite { .. }
),
"Expected WorkspaceWrite sandbox for untrusted project"
);
}
assert!(
matches!(
&config.legacy_sandbox_policy(),
SandboxPolicy::ReadOnly { .. }
),
"Expected ReadOnly sandbox for untrusted project"
);
Ok(())
}
+1 -1
View File
@@ -49,7 +49,7 @@ pub(crate) fn default_builtin_permission_profile_name(
active_project: &ProjectConfig,
windows_sandbox_level: WindowsSandboxLevel,
) -> &'static str {
if (active_project.is_trusted() || active_project.is_untrusted())
if active_project.is_trusted()
&& !(cfg!(target_os = "windows") && windows_sandbox_level == WindowsSandboxLevel::Disabled)
{
BUILT_IN_WORKSPACE_PROFILE
+72 -30
View File
@@ -45,6 +45,7 @@ use chrono::Local;
use chrono::Utc;
use codex_analytics::AnalyticsEventsClient;
use codex_analytics::SubAgentThreadStartedInput;
use codex_analytics::TurnCodexErrorFact;
use codex_app_server_protocol::McpServerElicitationRequest;
use codex_app_server_protocol::McpServerElicitationRequestParams;
use codex_config::types::OAuthCredentialsStoreMode;
@@ -1634,6 +1635,17 @@ impl Session {
)
}
/// Record a terminal CodexErr before the app-server completion notification is reduced.
pub(crate) fn track_turn_codex_error(&self, turn_context: &TurnContext, error: &CodexErr) {
self.services
.analytics_events_client
.track_turn_codex_error(TurnCodexErrorFact::from_codex_err(
self.conversation_id.to_string(),
turn_context.sub_id.clone(),
error,
));
}
/// Persist the event to rollout and send it to clients.
pub(crate) async fn send_event(&self, turn_context: &TurnContext, msg: EventMsg) {
let legacy_source = msg.clone();
@@ -2100,34 +2112,16 @@ impl Session {
rx_approve
}
pub async fn request_permissions(
self: &Arc<Self>,
turn_context: &Arc<TurnContext>,
call_id: String,
args: RequestPermissionsArgs,
cancellation_token: CancellationToken,
) -> Option<RequestPermissionsResponse> {
self.request_permissions_for_cwd(
turn_context,
call_id,
args,
#[allow(deprecated)]
turn_context.cwd.clone(),
cancellation_token,
)
.await
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
)]
pub(crate) async fn request_permissions_for_cwd(
pub(crate) async fn request_permissions_for_environment(
self: &Arc<Self>,
turn_context: &Arc<TurnContext>,
call_id: String,
args: RequestPermissionsArgs,
cwd: AbsolutePathBuf,
environment: TurnEnvironmentSelection,
cancellation_token: CancellationToken,
) -> Option<RequestPermissionsResponse> {
match turn_context.as_ref().approval_policy.value() {
@@ -2221,10 +2215,11 @@ impl Session {
let response = Self::normalize_request_permissions_response(
requested_permissions,
response,
cwd.as_path(),
environment.cwd.as_path(),
);
self.record_granted_request_permissions_for_turn(
&response,
&environment.environment_id,
originating_turn_state.as_ref(),
)
.await;
@@ -2242,7 +2237,7 @@ impl Session {
PendingRequestPermissions {
tx_response,
requested_permissions: requested_permissions.clone(),
cwd: cwd.clone(),
environment: environment.clone(),
},
)
}
@@ -2256,10 +2251,11 @@ impl Session {
let event = EventMsg::RequestPermissions(RequestPermissionsEvent {
call_id: call_id.clone(),
turn_id: turn_context.sub_id.clone(),
environment_id: Some(environment.environment_id.clone()),
started_at_ms: now_unix_timestamp_ms(),
reason: args.reason,
permissions: requested_permissions,
cwd: Some(cwd),
cwd: Some(environment.cwd),
});
self.send_event(turn_context.as_ref(), event).await;
tokio::select! {
@@ -2276,6 +2272,41 @@ impl Session {
}
}
pub(crate) async fn request_permissions_for_cwd(
self: &Arc<Self>,
turn_context: &Arc<TurnContext>,
call_id: String,
args: RequestPermissionsArgs,
cwd: AbsolutePathBuf,
cancellation_token: CancellationToken,
) -> Option<RequestPermissionsResponse> {
let turn_environment = match args.environment_id.as_deref() {
Some(environment_id) => turn_context
.environments
.turn_environments
.iter()
.find(|environment| environment.environment_id == environment_id),
None => turn_context.environments.primary(),
};
let Some(turn_environment) = turn_environment else {
return Some(RequestPermissionsResponse {
permissions: RequestPermissionProfile::default(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
});
};
let mut environment = turn_environment.selection();
environment.cwd = cwd;
self.request_permissions_for_environment(
turn_context,
call_id,
args,
environment,
cancellation_token,
)
.await
}
#[expect(
clippy::await_holding_invalid_type,
reason = "active turn checks and turn state updates must remain atomic"
@@ -2370,10 +2401,11 @@ impl Session {
let response = Self::normalize_request_permissions_response(
entry.requested_permissions,
response,
entry.cwd.as_path(),
entry.environment.cwd.as_path(),
);
self.record_granted_request_permissions_for_turn(
&response,
&entry.environment.environment_id,
originating_turn_state.as_ref(),
)
.await;
@@ -2417,6 +2449,7 @@ impl Session {
async fn record_granted_request_permissions_for_turn(
&self,
response: &RequestPermissionsResponse,
environment_id: &str,
originating_turn_state: Option<&Arc<Mutex<crate::state::TurnState>>>,
) {
if response.permissions.is_empty() {
@@ -2428,7 +2461,7 @@ impl Session {
let mut ts = turn_state.lock().await;
let permissions: AdditionalPermissionProfile =
response.permissions.clone().into();
ts.record_granted_permissions(permissions);
ts.record_granted_permissions(environment_id, permissions);
if response.strict_auto_review {
ts.enable_strict_auto_review();
}
@@ -2436,7 +2469,10 @@ impl Session {
}
PermissionGrantScope::Session => {
let mut state = self.state.lock().await;
state.record_granted_permissions(response.permissions.clone().into());
state.record_granted_permissions(
environment_id,
response.permissions.clone().into(),
);
}
}
}
@@ -2445,11 +2481,14 @@ impl Session {
clippy::await_holding_invalid_type,
reason = "active turn reads must stay consistent with the matching turn state"
)]
pub(crate) async fn granted_turn_permissions(&self) -> Option<AdditionalPermissionProfile> {
pub(crate) async fn granted_turn_permissions(
&self,
environment_id: &str,
) -> Option<AdditionalPermissionProfile> {
let active = self.active_turn.lock().await;
let active = active.as_ref()?;
let ts = active.turn_state.lock().await;
ts.granted_permissions()
ts.granted_permissions(environment_id)
}
#[expect(
@@ -2465,9 +2504,12 @@ impl Session {
ts.strict_auto_review_enabled()
}
pub(crate) async fn granted_session_permissions(&self) -> Option<AdditionalPermissionProfile> {
pub(crate) async fn granted_session_permissions(
&self,
environment_id: &str,
) -> Option<AdditionalPermissionProfile> {
let state = self.state.lock().await;
state.granted_permissions()
state.granted_permissions(environment_id)
}
#[expect(
+256 -9
View File
@@ -73,6 +73,7 @@ use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolPayload;
use crate::tools::handlers::CreateGoalHandler;
use crate::tools::handlers::ExecCommandHandler;
use crate::tools::handlers::RequestPermissionsHandler;
use crate::tools::handlers::ShellCommandHandler;
use crate::tools::handlers::UpdateGoalHandler;
use crate::tools::registry::ToolExecutor;
@@ -5155,7 +5156,12 @@ async fn notify_request_permissions_response_ignores_unmatched_call_id() {
)
.await;
assert_eq!(session.granted_turn_permissions().await, None);
assert_eq!(
session
.granted_turn_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID)
.await,
None
);
}
#[tokio::test]
@@ -5182,16 +5188,84 @@ async fn record_granted_request_permissions_for_turn_uses_originating_turn() {
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
},
codex_exec_server::LOCAL_ENVIRONMENT_ID,
Some(&originating_turn_state),
)
.await;
assert_eq!(
originating_turn_state.lock().await.granted_permissions(),
originating_turn_state
.lock()
.await
.granted_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID),
Some(requested_permissions.into())
);
assert_eq!(current_turn_state.lock().await.granted_permissions(), None);
assert_eq!(session.granted_turn_permissions().await, None);
assert_eq!(
current_turn_state
.lock()
.await
.granted_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID),
None
);
assert_eq!(
session
.granted_turn_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID)
.await,
None
);
}
#[tokio::test]
async fn request_permission_grants_are_environment_keyed() {
let (session, _turn_context) = make_session_and_context().await;
let originating_active_turn = ActiveTurn::default();
let originating_turn_state = Arc::clone(&originating_active_turn.turn_state);
*session.active_turn.lock().await = Some(originating_active_turn);
let requested_permissions = RequestPermissionProfile {
network: Some(codex_protocol::models::NetworkPermissions {
enabled: Some(true),
}),
..RequestPermissionProfile::default()
};
session
.record_granted_request_permissions_for_turn(
&codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: requested_permissions.clone(),
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
},
"remote",
Some(&originating_turn_state),
)
.await;
{
let turn_state = originating_turn_state.lock().await;
assert_eq!(
turn_state.granted_permissions("remote"),
Some(requested_permissions.clone().into())
);
assert_eq!(turn_state.granted_permissions("local"), None);
}
session
.record_granted_request_permissions_for_turn(
&codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: requested_permissions.clone(),
scope: PermissionGrantScope::Session,
strict_auto_review: false,
},
"remote",
/*originating_turn_state*/ None,
)
.await;
assert_eq!(
session.granted_session_permissions("remote").await,
Some(requested_permissions.into())
);
assert_eq!(session.granted_session_permissions("local").await, None);
}
#[tokio::test]
@@ -5214,6 +5288,7 @@ async fn enable_strict_auto_review_for_turn_uses_originating_turn() {
scope: PermissionGrantScope::Turn,
strict_auto_review: true,
},
codex_exec_server::LOCAL_ENVIRONMENT_ID,
Some(&originating_turn_state),
)
.await;
@@ -5290,11 +5365,17 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests()
let turn_context = Arc::clone(&turn_context);
let call_id = call_id.clone();
async move {
let environment = turn_context
.environments
.primary()
.expect("primary environment")
.selection();
session
.request_permissions(
.request_permissions_for_environment(
&turn_context,
call_id,
codex_protocol::request_permissions::RequestPermissionsArgs {
environment_id: None,
reason: Some("need network".to_string()),
permissions: RequestPermissionProfile {
network: Some(codex_protocol::models::NetworkPermissions {
@@ -5303,6 +5384,7 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests()
..RequestPermissionProfile::default()
},
},
environment,
CancellationToken::new(),
)
.await
@@ -5317,6 +5399,10 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests()
panic!("expected request_permissions event");
};
assert_eq!(request.call_id, call_id);
assert_eq!(
request.environment_id.as_deref(),
Some(codex_exec_server::LOCAL_ENVIRONMENT_ID)
);
#[allow(deprecated)]
let turn_cwd = turn_context.cwd.clone();
assert_eq!(request.cwd, Some(turn_cwd));
@@ -5333,6 +5419,142 @@ async fn request_permissions_emits_event_when_granular_policy_allows_requests()
assert_eq!(response, Some(expected_response));
}
#[tokio::test]
async fn request_permissions_tool_resolves_relative_paths_against_selected_environment() {
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
*session.active_turn.lock().await = Some(ActiveTurn::default());
let environment_cwd = {
#[allow(deprecated)]
let legacy_cwd = turn_context.cwd.clone();
legacy_cwd.join("request-permissions-environment")
};
std::fs::create_dir_all(environment_cwd.as_path()).expect("create environment cwd");
let turn_context_mut = Arc::get_mut(&mut turn_context).expect("single thread settings ref");
turn_context_mut
.approval_policy
.set(AskForApproval::Granular(GranularApprovalConfig {
sandbox_approval: true,
rules: true,
skill_approval: true,
request_permissions: true,
mcp_elicitations: true,
}))
.expect("test setup should allow updating approval policy");
turn_context_mut.environments.turn_environments[0].environment_id = "remote".to_string();
turn_context_mut.environments.turn_environments[0].cwd = environment_cwd.clone();
let call_id = "call-1".to_string();
let handler = RequestPermissionsHandler;
let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new()));
let handle = tokio::spawn({
let session = Arc::clone(&session);
let turn_context = Arc::clone(&turn_context);
let tracker = Arc::clone(&tracker);
let call_id = call_id.clone();
async move {
handler
.handle(ToolInvocation {
session,
turn: turn_context,
cancellation_token: CancellationToken::new(),
tracker,
call_id,
tool_name: codex_tools::ToolName::plain("request_permissions"),
source: ToolCallSource::Direct,
payload: ToolPayload::Function {
arguments: json!({
"environment_id": "remote",
"reason": "need write",
"permissions": {
"file_system": {
"entries": [{
"path": {
"type": "path",
"path": "relative.txt",
},
"access": "write",
}],
},
},
})
.to_string(),
},
})
.await
}
});
let request_event = tokio::time::timeout(StdDuration::from_secs(1), rx.recv())
.await
.expect("request_permissions event timed out")
.expect("request_permissions event missing");
let EventMsg::RequestPermissions(request) = request_event.msg else {
panic!("expected request_permissions event");
};
let expected_permissions = RequestPermissionProfile {
file_system: Some(FileSystemPermissions {
entries: vec![FileSystemSandboxEntry {
path: FileSystemPath::Path {
path: environment_cwd.join("relative.txt"),
},
access: FileSystemAccessMode::Write,
}],
glob_scan_max_depth: None,
}),
..Default::default()
};
assert_eq!(request.environment_id.as_deref(), Some("remote"));
assert_eq!(request.permissions, expected_permissions);
session
.notify_request_permissions_response(
&request.call_id,
codex_protocol::request_permissions::RequestPermissionsResponse {
permissions: request.permissions,
scope: PermissionGrantScope::Turn,
strict_auto_review: false,
},
)
.await;
tokio::time::timeout(StdDuration::from_secs(1), handle)
.await
.expect("request_permissions handler timed out")
.expect("request_permissions handler join error")
.expect("request_permissions handler should succeed");
}
#[tokio::test]
async fn request_permissions_tool_rejects_unknown_environment_id() {
let (session, turn_context) = make_session_and_context().await;
let result = RequestPermissionsHandler
.handle(ToolInvocation {
session: Arc::new(session),
turn: Arc::new(turn_context),
cancellation_token: CancellationToken::new(),
tracker: Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())),
call_id: "call-1".to_string(),
tool_name: codex_tools::ToolName::plain("request_permissions"),
source: ToolCallSource::Direct,
payload: ToolPayload::Function {
arguments: json!({
"environment_id": "missing",
"permissions": {
"network": {
"enabled": true,
},
},
})
.to_string(),
},
})
.await;
let Err(FunctionCallError::RespondToModel(output)) = result else {
panic!("expected unknown environment id to be rejected");
};
assert_eq!(output, "unknown turn environment id `missing`");
}
#[tokio::test]
async fn request_permissions_response_materializes_session_cwd_grants_before_recording() {
let (session, mut turn_context, rx) = make_session_and_context_with_rx().await;
@@ -5371,14 +5593,21 @@ async fn request_permissions_response_materializes_session_cwd_grants_before_rec
let call_id = call_id.clone();
let requested_permissions = requested_permissions.clone();
async move {
let environment = turn_context
.environments
.primary()
.expect("primary environment")
.selection();
session
.request_permissions(
.request_permissions_for_environment(
&turn_context,
call_id,
codex_protocol::request_permissions::RequestPermissionsArgs {
environment_id: None,
reason: Some("need cwd write".to_string()),
permissions: requested_permissions,
},
environment,
CancellationToken::new(),
)
.await
@@ -5392,6 +5621,10 @@ async fn request_permissions_response_materializes_session_cwd_grants_before_rec
let EventMsg::RequestPermissions(request) = request_event.msg else {
panic!("expected request_permissions event");
};
assert_eq!(
request.environment_id.as_deref(),
Some(codex_exec_server::LOCAL_ENVIRONMENT_ID)
);
let request_cwd = request.cwd.clone().expect("request cwd");
session
@@ -5425,7 +5658,9 @@ async fn request_permissions_response_materializes_session_cwd_grants_before_rec
assert_eq!(response, Some(expected_response));
assert_eq!(
session.granted_session_permissions().await,
session
.granted_session_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID)
.await,
Some(expected_permissions.into())
);
}
@@ -5449,11 +5684,17 @@ async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_req
let session = Arc::new(session);
let turn_context = Arc::new(turn_context);
let call_id = "call-1".to_string();
let environment = turn_context
.environments
.primary()
.expect("primary environment")
.selection();
let response = session
.request_permissions(
.request_permissions_for_environment(
&turn_context,
call_id,
codex_protocol::request_permissions::RequestPermissionsArgs {
environment_id: None,
reason: Some("need network".to_string()),
permissions: RequestPermissionProfile {
network: Some(codex_protocol::models::NetworkPermissions {
@@ -5462,6 +5703,7 @@ async fn request_permissions_is_auto_denied_when_granular_policy_blocks_tool_req
..RequestPermissionProfile::default()
},
},
environment,
CancellationToken::new(),
)
.await;
@@ -10450,7 +10692,12 @@ async fn rejects_escalated_permissions_when_policy_not_on_request() {
);
pretty_assertions::assert_eq!(output, expected);
pretty_assertions::assert_eq!(session.granted_turn_permissions().await, None);
pretty_assertions::assert_eq!(
session
.granted_turn_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID)
.await,
None
);
// The rejection should not poison the non-escalated path for the same
// command. Force DangerFullAccess so this check stays focused on approval
@@ -124,15 +124,22 @@ async fn request_permissions_routes_to_guardian_when_reviewer_is_enabled() {
}),
..RequestPermissionProfile::default()
};
let environment = turn_context
.environments
.primary()
.expect("primary environment")
.selection();
let response = tokio::time::timeout(
Duration::from_secs(45),
session.request_permissions(
session.request_permissions_for_environment(
&turn_context,
"perm-call-1".to_string(),
RequestPermissionsArgs {
environment_id: None,
reason: Some("need network".to_string()),
permissions: requested_permissions.clone(),
},
environment,
CancellationToken::new(),
),
)
@@ -148,7 +155,9 @@ async fn request_permissions_routes_to_guardian_when_reviewer_is_enabled() {
})
);
assert_eq!(
session.granted_turn_permissions().await,
session
.granted_turn_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID)
.await,
Some(requested_permissions.into())
);
@@ -211,14 +220,21 @@ async fn request_permissions_guardian_review_stops_when_cancelled() {
let requested_permissions = requested_permissions.clone();
let cancellation_token = cancellation_token.clone();
async move {
let environment = turn_context
.environments
.primary()
.expect("primary environment")
.selection();
session
.request_permissions(
.request_permissions_for_environment(
&turn_context,
"perm-call-cancelled".to_string(),
RequestPermissionsArgs {
environment_id: None,
reason: Some("need network".to_string()),
permissions: requested_permissions,
},
environment,
cancellation_token,
)
.await
@@ -246,7 +262,12 @@ async fn request_permissions_guardian_review_stops_when_cancelled() {
.expect("request_permissions should stop when cancelled")
.expect("request_permissions task should not panic");
assert_eq!(response, None);
assert_eq!(session.granted_turn_permissions().await, None);
assert_eq!(
session
.granted_turn_permissions(codex_exec_server::LOCAL_ENVIRONMENT_ID)
.await,
None
);
}
#[tokio::test]
@@ -380,6 +401,7 @@ async fn strict_auto_review_turn_grant_forces_guardian_for_shell_command_policy_
scope: PermissionGrantScope::Turn,
strict_auto_review: true,
},
codex_exec_server::LOCAL_ENVIRONMENT_ID,
Some(&originating_turn_state),
)
.await;
@@ -564,12 +586,15 @@ async fn shell_command_allows_sticky_turn_permissions_without_inline_request_per
let mut active_turn = session.active_turn.lock().await;
let active_turn = active_turn.as_mut().expect("active turn");
let mut turn_state = active_turn.turn_state.lock().await;
turn_state.record_granted_permissions(PermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
..Default::default()
});
turn_state.record_granted_permissions(
codex_exec_server::LOCAL_ENVIRONMENT_ID,
PermissionProfile {
network: Some(NetworkPermissions {
enabled: Some(true),
}),
..Default::default()
},
);
}
let session = Arc::new(session);
+3 -1
View File
@@ -369,7 +369,7 @@ pub(crate) async fn run_turn(
// Aborted turn is reported via a different event.
break;
}
Err(CodexErr::InvalidImageRequest()) => {
Err(codex_error @ CodexErr::InvalidImageRequest()) => {
{
let mut state = sess.state.lock().await;
error_or_panic(
@@ -380,6 +380,7 @@ pub(crate) async fn run_turn(
}
}
sess.track_turn_codex_error(turn_context.as_ref(), &codex_error);
let error = CodexErrorInfo::BadRequest;
sess.emit_turn_error_lifecycle(turn_context.as_ref(), error.clone())
.await;
@@ -405,6 +406,7 @@ pub(crate) async fn run_turn(
{
warn!("failed to usage-limit active goal after usage-limit error: {err}");
}
sess.track_turn_codex_error(turn_context.as_ref(), &e);
let event = EventMsg::Error(e.to_error_event(/*message_prefix*/ None));
sess.send_event(&turn_context, event).await;
// let the user continue the conversation
+24 -7
View File
@@ -3,6 +3,7 @@
use codex_protocol::models::AdditionalPermissionProfile;
use codex_protocol::models::ResponseItem;
use codex_sandboxing::policy_transforms::merge_permission_profiles;
use std::collections::HashMap;
use std::collections::HashSet;
use std::collections::VecDeque;
@@ -37,7 +38,7 @@ pub(crate) struct SessionState {
pub(crate) startup_prewarm: Option<SessionStartupPrewarmHandle>,
pub(crate) active_connector_selection: HashSet<String>,
pub(crate) pending_session_start_sources: VecDeque<codex_hooks::SessionStartSource>,
granted_permissions: Option<AdditionalPermissionProfile>,
granted_permissions_by_environment_id: HashMap<String, AdditionalPermissionProfile>,
next_turn_is_first: bool,
}
@@ -57,7 +58,7 @@ impl SessionState {
startup_prewarm: None,
active_connector_selection: HashSet::new(),
pending_session_start_sources: VecDeque::new(),
granted_permissions: None,
granted_permissions_by_environment_id: HashMap::new(),
next_turn_is_first: true,
}
}
@@ -235,13 +236,29 @@ impl SessionState {
self.pending_session_start_sources.pop_front()
}
pub(crate) fn record_granted_permissions(&mut self, permissions: AdditionalPermissionProfile) {
self.granted_permissions =
merge_permission_profiles(self.granted_permissions.as_ref(), Some(&permissions));
pub(crate) fn record_granted_permissions(
&mut self,
environment_id: &str,
permissions: AdditionalPermissionProfile,
) {
let granted_permissions = merge_permission_profiles(
self.granted_permissions_by_environment_id
.get(environment_id),
Some(&permissions),
);
if let Some(granted_permissions) = granted_permissions {
self.granted_permissions_by_environment_id
.insert(environment_id.to_string(), granted_permissions);
}
}
pub(crate) fn granted_permissions(&self) -> Option<AdditionalPermissionProfile> {
self.granted_permissions.clone()
pub(crate) fn granted_permissions(
&self,
environment_id: &str,
) -> Option<AdditionalPermissionProfile> {
self.granted_permissions_by_environment_id
.get(environment_id)
.cloned()
}
}
+25 -9
View File
@@ -1,6 +1,5 @@
//! Turn-scoped state and active turn metadata scaffolding.
use codex_sandboxing::policy_transforms::merge_permission_profiles;
use std::collections::HashMap;
use std::sync::Arc;
use tokio::sync::Mutex;
@@ -10,11 +9,12 @@ use tokio_util::task::AbortOnDropHandle;
use codex_extension_api::ExtensionData;
use codex_protocol::dynamic_tools::DynamicToolResponse;
use codex_protocol::protocol::TurnEnvironmentSelection;
use codex_protocol::request_permissions::RequestPermissionProfile;
use codex_protocol::request_permissions::RequestPermissionsResponse;
use codex_protocol::request_user_input::RequestUserInputResponse;
use codex_rmcp_client::ElicitationResponse;
use codex_utils_absolute_path::AbsolutePathBuf;
use codex_sandboxing::policy_transforms::merge_permission_profiles;
use rmcp::model::RequestId;
use tokio::sync::oneshot;
@@ -90,7 +90,7 @@ pub(crate) struct TurnState {
pending_dynamic_tools: HashMap<String, oneshot::Sender<DynamicToolResponse>>,
pub(crate) pending_input: TurnInputQueue,
mailbox_delivery_phase: MailboxDeliveryPhase,
granted_permissions: Option<AdditionalPermissionProfile>,
granted_permissions_by_environment_id: HashMap<String, AdditionalPermissionProfile>,
strict_auto_review_enabled: bool,
pub(crate) tool_calls: u64,
pub(crate) has_memory_citation: bool,
@@ -100,7 +100,7 @@ pub(crate) struct TurnState {
pub(crate) struct PendingRequestPermissions {
pub(crate) tx_response: oneshot::Sender<RequestPermissionsResponse>,
pub(crate) requested_permissions: RequestPermissionProfile,
pub(crate) cwd: AbsolutePathBuf,
pub(crate) environment: TurnEnvironmentSelection,
}
impl TurnState {
@@ -204,13 +204,29 @@ impl TurnState {
self.mailbox_delivery_phase = phase;
}
pub(crate) fn record_granted_permissions(&mut self, permissions: AdditionalPermissionProfile) {
self.granted_permissions =
merge_permission_profiles(self.granted_permissions.as_ref(), Some(&permissions));
pub(crate) fn record_granted_permissions(
&mut self,
environment_id: &str,
permissions: AdditionalPermissionProfile,
) {
let granted_permissions = merge_permission_profiles(
self.granted_permissions_by_environment_id
.get(environment_id),
Some(&permissions),
);
if let Some(granted_permissions) = granted_permissions {
self.granted_permissions_by_environment_id
.insert(environment_id.to_string(), granted_permissions);
}
}
pub(crate) fn granted_permissions(&self) -> Option<AdditionalPermissionProfile> {
self.granted_permissions.clone()
pub(crate) fn granted_permissions(
&self,
environment_id: &str,
) -> Option<AdditionalPermissionProfile> {
self.granted_permissions_by_environment_id
.get(environment_id)
.cloned()
}
pub(crate) fn enable_strict_auto_review(&mut self) {
+45 -27
View File
@@ -131,6 +131,7 @@ pub(crate) async fn persist_image_generation_item(
turn_context: &TurnContext,
image_item: &mut ImageGenerationItem,
) -> Option<AbsolutePathBuf> {
image_item.saved_path = None;
let session_id = sess.conversation_id.to_string();
match save_image_generation_result(
&turn_context.config.codex_home,
@@ -163,15 +164,12 @@ pub(crate) async fn persist_image_generation_item(
}
}
pub(crate) async fn finalize_image_generation_item(
async fn record_image_generation_instructions(
sess: &Session,
turn_context: &TurnContext,
image_item: &mut ImageGenerationItem,
image_item: &ImageGenerationItem,
) {
if persist_image_generation_item(sess, turn_context, image_item)
.await
.is_none()
{
if image_item.saved_path.is_none() {
return;
}
let session_id = sess.conversation_id.to_string();
@@ -530,27 +528,16 @@ pub(crate) async fn handle_non_tool_response_item(
| ResponseItem::WebSearchCall { .. }
| ResponseItem::ImageGenerationCall { .. } => {
let mut turn_item = parse_turn_item(item)?;
if let TurnItemContributorPolicy::Run(turn_store) = contributor_policy {
apply_turn_item_contributors(sess, turn_store, &mut turn_item).await;
}
if let TurnItem::AgentMessage(agent_message) = &mut turn_item {
let combined = agent_message
.content
.iter()
.map(|entry| match entry {
codex_protocol::items::AgentMessageContent::Text { text } => text.as_str(),
})
.collect::<String>();
let (stripped, memory_citation) =
strip_hidden_assistant_markup_and_parse_memory_citation(&combined, plan_mode);
agent_message.content =
vec![codex_protocol::items::AgentMessageContent::Text { text: stripped }];
if agent_message.memory_citation.is_none() {
agent_message.memory_citation = memory_citation;
}
}
if let TurnItem::ImageGeneration(image_item) = &mut turn_item {
finalize_image_generation_item(sess, turn_context, image_item).await;
finalize_turn_item(
sess,
turn_context,
contributor_policy,
&mut turn_item,
plan_mode,
)
.await;
if let TurnItem::ImageGeneration(image_item) = &turn_item {
record_image_generation_instructions(sess, turn_context, image_item).await;
}
Some(turn_item)
}
@@ -564,6 +551,37 @@ pub(crate) async fn handle_non_tool_response_item(
}
}
pub(crate) async fn finalize_turn_item(
sess: &Session,
turn_context: &TurnContext,
contributor_policy: TurnItemContributorPolicy<'_>,
turn_item: &mut TurnItem,
plan_mode: bool,
) {
if let TurnItemContributorPolicy::Run(turn_store) = contributor_policy {
apply_turn_item_contributors(sess, turn_store, turn_item).await;
}
if let TurnItem::AgentMessage(agent_message) = &mut *turn_item {
let combined = agent_message
.content
.iter()
.map(|entry| match entry {
codex_protocol::items::AgentMessageContent::Text { text } => text.as_str(),
})
.collect::<String>();
let (stripped, memory_citation) =
strip_hidden_assistant_markup_and_parse_memory_citation(&combined, plan_mode);
agent_message.content =
vec![codex_protocol::items::AgentMessageContent::Text { text: stripped }];
if agent_message.memory_citation.is_none() {
agent_message.memory_citation = memory_citation;
}
}
if let TurnItem::ImageGeneration(image_item) = &mut *turn_item {
persist_image_generation_item(sess, turn_context, image_item).await;
}
}
pub(crate) fn last_assistant_message_from_item(
item: &ResponseItem,
plan_mode: bool,
@@ -265,6 +265,7 @@ fn apply_patch_payload_command(payload: &ToolPayload) -> Option<String> {
async fn effective_patch_permissions(
session: &Session,
turn: &TurnContext,
environment_id: &str,
action: &ApplyPatchAction,
cwd: &AbsolutePathBuf,
) -> (
@@ -274,8 +275,14 @@ async fn effective_patch_permissions(
) {
let file_paths = file_paths_for_action(action);
let granted_permissions = merge_permission_profiles(
session.granted_session_permissions().await.as_ref(),
session.granted_turn_permissions().await.as_ref(),
session
.granted_session_permissions(environment_id)
.await
.as_ref(),
session
.granted_turn_permissions(environment_id)
.await
.as_ref(),
);
let base_file_system_sandbox_policy = turn.file_system_sandbox_policy();
let file_system_sandbox_policy = effective_file_system_sandbox_policy(
@@ -284,6 +291,7 @@ async fn effective_patch_permissions(
);
let effective_additional_permissions = apply_granted_turn_permissions(
session,
environment_id,
cwd.as_path(),
crate::sandboxing::SandboxPermissions::UseDefault,
write_permissions_for_paths(&file_paths, &file_system_sandbox_policy, cwd),
@@ -353,8 +361,14 @@ impl ToolExecutor<ToolInvocation> for ApplyPatchHandler {
{
codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => {
let (file_paths, effective_additional_permissions, file_system_sandbox_policy) =
effective_patch_permissions(session.as_ref(), turn.as_ref(), &changes, &cwd)
.await;
effective_patch_permissions(
session.as_ref(),
turn.as_ref(),
&turn_environment.environment_id,
&changes,
&cwd,
)
.await;
match apply_patch::apply_patch(turn.as_ref(), &file_system_sandbox_policy, changes)
.await
{
@@ -506,7 +520,14 @@ pub(crate) async fn intercept_apply_patch(
{
codex_apply_patch::MaybeApplyPatchVerified::Body(changes) => {
let (approval_keys, effective_additional_permissions, file_system_sandbox_policy) =
effective_patch_permissions(session.as_ref(), turn.as_ref(), &changes, cwd).await;
effective_patch_permissions(
session.as_ref(),
turn.as_ref(),
&turn_environment.environment_id,
&changes,
cwd,
)
.await;
match apply_patch::apply_patch(turn.as_ref(), &file_system_sandbox_policy, changes)
.await
{
@@ -4,7 +4,6 @@ use std::sync::Weak;
use codex_protocol::items::TurnItem;
use codex_tools::ConversationHistory;
use codex_tools::ExtensionTurnItem;
use codex_tools::ImageGenerationCompletionFuture;
use codex_tools::ToolCall as ExtensionToolCall;
use codex_tools::ToolName;
use codex_tools::ToolSearchInfo;
@@ -12,12 +11,11 @@ use codex_tools::ToolSpec;
use codex_tools::TurnItemEmissionFuture;
use codex_tools::TurnItemEmitter;
use crate::context::ContextualUserFragment;
use crate::context::ImageGenerationInstructions;
use crate::function_tool::FunctionCallError;
use crate::session::session::Session;
use crate::session::turn_context::TurnContext;
use crate::stream_events_utils::persist_image_generation_item;
use crate::stream_events_utils::TurnItemContributorPolicy;
use crate::stream_events_utils::finalize_turn_item;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolOutput;
use crate::tools::context::ToolPayload;
@@ -76,6 +74,10 @@ struct CoreTurnItemEmitter {
fn extension_turn_item(item: ExtensionTurnItem) -> TurnItem {
match item {
ExtensionTurnItem::WebSearch(item) => TurnItem::WebSearch(item),
ExtensionTurnItem::ImageGeneration(mut item) => {
item.saved_path = None;
TurnItem::ImageGeneration(item)
}
}
}
@@ -85,8 +87,9 @@ impl TurnItemEmitter for CoreTurnItemEmitter {
let (Some(session), Some(turn)) = (self.session.upgrade(), self.turn.upgrade()) else {
return;
};
let item = extension_turn_item(item);
session.emit_turn_item_started(turn.as_ref(), &item).await;
session
.emit_turn_item_started(turn.as_ref(), &extension_turn_item(item))
.await;
})
}
@@ -95,54 +98,18 @@ impl TurnItemEmitter for CoreTurnItemEmitter {
let (Some(session), Some(turn)) = (self.session.upgrade(), self.turn.upgrade()) else {
return;
};
let item = extension_turn_item(item);
let mut item = extension_turn_item(item);
finalize_turn_item(
session.as_ref(),
turn.as_ref(),
TurnItemContributorPolicy::Run(turn.extension_data.as_ref()),
&mut item,
turn.collaboration_mode.mode == codex_protocol::config_types::ModeKind::Plan,
)
.await;
session.emit_turn_item_completed(turn.as_ref(), item).await;
})
}
fn image_generation_completed<'a>(
&'a self,
call_id: String,
prompt: String,
result: String,
) -> ImageGenerationCompletionFuture<'a> {
Box::pin(async move {
let (Some(session), Some(turn)) = (self.session.upgrade(), self.turn.upgrade()) else {
return None;
};
let mut item = codex_protocol::items::ImageGenerationItem {
id: call_id,
status: "completed".to_string(),
revised_prompt: Some(prompt),
result,
saved_path: None,
};
let output_hint =
persist_image_generation_item(session.as_ref(), turn.as_ref(), &mut item)
.await
.map(|saved_path| {
let output_dir = saved_path
.parent()
.unwrap_or_else(|| turn.config.codex_home.clone());
ImageGenerationInstructions::new(output_dir.display(), saved_path.display())
.body()
});
let started_item = codex_protocol::items::ImageGenerationItem {
id: item.id.clone(),
status: "in_progress".to_string(),
revised_prompt: None,
result: String::new(),
saved_path: None,
};
session
.emit_turn_item_started(turn.as_ref(), &TurnItem::ImageGeneration(started_item))
.await;
session
.emit_turn_item_completed(turn.as_ref(), TurnItem::ImageGeneration(item))
.await;
output_hint
})
}
}
async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
@@ -167,6 +134,8 @@ async fn to_extension_call(invocation: &ToolInvocation) -> ExtensionToolCall {
mod tests {
use std::sync::Arc;
use codex_extension_api::ExtensionData;
use codex_extension_api::TurnItemContributor;
use codex_protocol::items::TurnItem;
use codex_protocol::items::WebSearchItem;
use codex_protocol::models::ContentItem;
@@ -174,10 +143,13 @@ mod tests {
use codex_protocol::models::WebSearchAction;
use codex_protocol::protocol::EventMsg;
use codex_tools::ExtensionTurnItem;
use codex_utils_absolute_path::test_support::PathExt;
use codex_utils_absolute_path::test_support::test_path_buf;
use pretty_assertions::assert_eq;
use serde_json::json;
use tokio::sync::Mutex;
use super::CoreTurnItemEmitter;
use super::ExtensionToolAdapter;
use crate::tools::context::ToolCallSource;
use crate::tools::context::ToolInvocation;
@@ -409,8 +381,54 @@ mod tests {
assert_eq!(end.action, expected.action);
}
struct ImageGenerationExtensionExecutor {
output_hint: Arc<Mutex<Option<String>>>,
struct ImageGenerationExtensionExecutor;
#[derive(Debug)]
struct ExtensionTurnItemContributorRan;
struct RecordExtensionTurnItemContributor;
#[async_trait::async_trait]
impl TurnItemContributor for RecordExtensionTurnItemContributor {
async fn contribute(
&self,
_thread_store: &ExtensionData,
turn_store: &ExtensionData,
_item: &mut TurnItem,
) -> Result<(), String> {
turn_store.insert(ExtensionTurnItemContributorRan);
Ok(())
}
}
#[tokio::test]
async fn extension_completion_runs_turn_item_contributors() {
let (mut session, turn) = crate::session::tests::make_session_and_context().await;
let mut builder = codex_extension_api::ExtensionRegistryBuilder::new();
builder.turn_item_contributor(Arc::new(RecordExtensionTurnItemContributor));
session.services.extensions = Arc::new(builder.build());
let session = Arc::new(session);
let turn = Arc::new(turn);
let emitter = CoreTurnItemEmitter {
session: Arc::downgrade(&session),
turn: Arc::downgrade(&turn),
};
codex_tools::TurnItemEmitter::emit_completed(
&emitter,
ExtensionTurnItem::WebSearch(WebSearchItem {
id: "search-1".to_string(),
query: "contributors".to_string(),
action: WebSearchAction::Other,
}),
)
.await;
assert!(
turn.extension_data
.get::<ExtensionTurnItemContributorRan>()
.is_some()
);
}
#[async_trait::async_trait]
@@ -434,15 +452,28 @@ mod tests {
&self,
call: codex_tools::ToolCall,
) -> Result<Box<dyn codex_tools::ToolOutput>, codex_tools::FunctionCallError> {
let output_hint = call
.turn_item_emitter
.image_generation_completed(
call.call_id,
"A tiny blue square".to_string(),
"cG5n".to_string(),
)
call.turn_item_emitter
.emit_started(ExtensionTurnItem::ImageGeneration(
codex_protocol::items::ImageGenerationItem {
id: call.call_id.clone(),
status: "in_progress".to_string(),
revised_prompt: None,
result: String::new(),
saved_path: None,
},
))
.await;
call.turn_item_emitter
.emit_completed(ExtensionTurnItem::ImageGeneration(
codex_protocol::items::ImageGenerationItem {
id: call.call_id,
status: "completed".to_string(),
revised_prompt: Some("A tiny blue square".to_string()),
result: "cG5n".to_string(),
saved_path: Some(test_path_buf("/tmp/extension-claimed.png").abs()),
},
))
.await;
*self.output_hint.lock().await = output_hint;
Ok(Box::new(codex_tools::JsonToolOutput::new(
json!({ "ok": true }),
)))
@@ -451,10 +482,7 @@ mod tests {
#[tokio::test]
async fn image_generation_publication_is_finalized_by_core() {
let output_hint = Arc::new(Mutex::new(None));
let handler = ExtensionToolAdapter::new(Arc::new(ImageGenerationExtensionExecutor {
output_hint: Arc::clone(&output_hint),
}));
let handler = ExtensionToolAdapter::new(Arc::new(ImageGenerationExtensionExecutor));
let (session, turn, rx) = crate::session::tests::make_session_and_context_with_rx().await;
let expected_path = crate::stream_events_utils::image_generation_artifact_path(
&turn.config.codex_home,
@@ -521,17 +549,5 @@ mod tests {
std::fs::read(&expected_path).expect("generated artifact should be saved"),
b"png"
);
assert_eq!(
*output_hint.lock().await,
Some(format!(
"Generated images are saved to {} as {} by default.\n\
If you need to use a generated image at another path, copy it and leave the original in place unless the user explicitly asks you to delete it.",
expected_path
.parent()
.expect("generated image path should have a parent")
.display(),
expected_path.display(),
))
);
}
}
+4 -3
View File
@@ -250,7 +250,8 @@ pub(super) fn implicit_granted_permissions(
pub(super) async fn apply_granted_turn_permissions(
session: &Session,
cwd: &std::path::Path,
environment_id: &str,
cwd: &Path,
sandbox_permissions: SandboxPermissions,
additional_permissions: Option<AdditionalPermissionProfile>,
) -> EffectiveAdditionalPermissions {
@@ -262,8 +263,8 @@ pub(super) async fn apply_granted_turn_permissions(
};
}
let granted_session_permissions = session.granted_session_permissions().await;
let granted_turn_permissions = session.granted_turn_permissions().await;
let granted_session_permissions = session.granted_session_permissions(environment_id).await;
let granted_turn_permissions = session.granted_turn_permissions(environment_id).await;
let granted_permissions = merge_permission_profiles(
granted_session_permissions.as_ref(),
granted_turn_permissions.as_ref(),
@@ -6,16 +6,25 @@ use crate::tools::context::FunctionToolOutput;
use crate::tools::context::ToolInvocation;
use crate::tools::context::ToolPayload;
use crate::tools::context::boxed_tool_output;
use crate::tools::handlers::parse_arguments;
use crate::tools::handlers::parse_arguments_with_base_path;
use crate::tools::handlers::resolve_tool_environment;
use crate::tools::handlers::shell_spec::create_request_permissions_tool;
use crate::tools::handlers::shell_spec::request_permissions_tool_description;
use crate::tools::registry::CoreToolRuntime;
use crate::tools::registry::ToolExecutor;
use codex_tools::ToolName;
use codex_tools::ToolSpec;
use serde::Deserialize;
pub struct RequestPermissionsHandler;
#[derive(Deserialize)]
struct RequestPermissionsEnvironmentArgs {
#[serde(default, rename = "environment_id", alias = "environmentId")]
environment_id: Option<String>,
}
#[async_trait::async_trait]
impl ToolExecutor<ToolInvocation> for RequestPermissionsHandler {
fn tool_name(&self) -> ToolName {
@@ -48,9 +57,16 @@ impl ToolExecutor<ToolInvocation> for RequestPermissionsHandler {
}
};
#[allow(deprecated)]
let environment_args: RequestPermissionsEnvironmentArgs = parse_arguments(&arguments)?;
let Some(turn_environment) =
resolve_tool_environment(turn.as_ref(), environment_args.environment_id.as_deref())?
else {
return Err(FunctionCallError::RespondToModel(
"request_permissions requires a primary environment".to_string(),
));
};
let mut args: RequestPermissionsArgs =
parse_arguments_with_base_path(&arguments, &turn.cwd)?;
parse_arguments_with_base_path(&arguments, &turn_environment.cwd)?;
args.permissions = normalize_additional_permissions(args.permissions.into())
.map(codex_protocol::request_permissions::RequestPermissionProfile::from)
.map_err(FunctionCallError::RespondToModel)?;
@@ -61,7 +77,13 @@ impl ToolExecutor<ToolInvocation> for RequestPermissionsHandler {
}
let response = session
.request_permissions(&turn, call_id, args, cancellation_token)
.request_permissions_for_environment(
&turn,
call_id,
args,
turn_environment.selection(),
cancellation_token,
)
.await
.ok_or_else(|| {
FunctionCallError::RespondToModel(
+2 -2
View File
@@ -84,10 +84,10 @@ async fn run_exec_like(args: RunExecLikeArgs) -> Result<FunctionToolOutput, Func
let exec_permission_approvals_enabled =
session.features().enabled(Feature::ExecPermissionApprovals);
let requested_additional_permissions = additional_permissions.clone();
#[allow(deprecated)]
let effective_additional_permissions = apply_granted_turn_permissions(
session.as_ref(),
turn.cwd.as_path(),
&turn_environment.environment_id,
exec_params.cwd.as_path(),
exec_params.sandbox_permissions,
additional_permissions,
)
@@ -229,6 +229,13 @@ pub fn create_request_permissions_tool(description: String) -> ToolSpec {
"Optional short explanation for why additional permissions are needed.".to_string(),
)),
),
(
"environment_id".to_string(),
JsonSchema::string(Some(
"Environment id from <environment_context>. Omit to use the primary environment."
.to_string(),
)),
),
("permissions".to_string(), permission_profile_schema()),
]);
@@ -247,7 +254,7 @@ pub fn create_request_permissions_tool(description: String) -> ToolSpec {
}
pub fn request_permissions_tool_description() -> String {
"Request additional filesystem or network permissions from the user and wait for the client to grant a subset of the requested permission profile. Granted permissions apply automatically to later shell-like commands in the current turn, or for the rest of the session if the client approves them at session scope."
"Request additional filesystem or network permissions from the user and wait for the client to grant a subset of the requested permission profile. Use environment_id to target a specific attached environment; omit it to use the primary environment. Relative filesystem paths resolve against the selected environment cwd. Granted permissions apply automatically to later shell-like commands in the current turn, or for the rest of the session if the client approves them at session scope."
.to_string()
}
@@ -172,6 +172,13 @@ fn request_permissions_tool_includes_full_permission_schema() {
"Optional short explanation for why additional permissions are needed.".to_string(),
)),
),
(
"environment_id".to_string(),
JsonSchema::string(Some(
"Environment id from <environment_context>. Omit to use the primary environment."
.to_string(),
)),
),
("permissions".to_string(), permission_profile_schema()),
]);
@@ -173,6 +173,7 @@ impl ToolExecutor<ToolInvocation> for ExecCommandHandler {
let requested_additional_permissions = additional_permissions.clone();
let effective_additional_permissions = apply_granted_turn_permissions(
context.session.as_ref(),
&turn_environment.environment_id,
cwd.as_path(),
sandbox_permissions,
additional_permissions,
-1
View File
@@ -13,7 +13,6 @@ pub use capabilities::ResponseItemInjector;
pub use codex_tools::ConversationHistory;
pub use codex_tools::ExtensionTurnItem;
pub use codex_tools::FunctionCallError;
pub use codex_tools::ImageGenerationCompletionFuture;
pub use codex_tools::JsonToolOutput;
pub use codex_tools::NoopTurnItemEmitter;
pub use codex_tools::ResponsesApiTool;
+5 -12
View File
@@ -26,7 +26,6 @@ use crate::IMAGE_GEN_NAMESPACE;
use crate::IMAGEGEN_TOOL_NAME;
const RESULT: &str = "cG5n";
const OUTPUT_HINT: &str = "Generated images are saved to /tmp as /tmp/call-1.png by default.";
#[test]
fn uses_reserved_image_gen_namespace() {
@@ -55,10 +54,9 @@ fn generate_uses_fixed_request_defaults() {
}
#[test]
fn generated_output_returns_image_input_and_output_hint() {
fn generated_output_returns_image_input() {
let output = GeneratedImageOutput {
result: RESULT.to_string(),
output_hint: Some(OUTPUT_HINT.to_string()),
};
let ResponseInputItem::FunctionCallOutput {
@@ -73,15 +71,10 @@ fn generated_output_returns_image_input_and_output_hint() {
};
assert_eq!(
content_items,
vec![
FunctionCallOutputContentItem::InputImage {
image_url: format!("data:image/png;base64,{RESULT}"),
detail: Some(DEFAULT_IMAGE_DETAIL),
},
FunctionCallOutputContentItem::InputText {
text: OUTPUT_HINT.to_string(),
},
]
vec![FunctionCallOutputContentItem::InputImage {
image_url: format!("data:image/png;base64,{RESULT}"),
detail: Some(DEFAULT_IMAGE_DETAIL),
}]
);
}
+22 -15
View File
@@ -3,6 +3,7 @@ use codex_api::ImageEditRequest;
use codex_api::ImageGenerationRequest;
use codex_api::ImageQuality;
use codex_api::ImageUrl;
use codex_extension_api::ExtensionTurnItem;
use codex_extension_api::FunctionCallError;
use codex_extension_api::ToolCall;
use codex_extension_api::ToolExecutor;
@@ -11,6 +12,7 @@ use codex_extension_api::ToolOutput;
use codex_extension_api::ToolPayload;
use codex_extension_api::ToolSpec;
use codex_extension_api::parse_tool_input_schema;
use codex_protocol::items::ImageGenerationItem;
use codex_protocol::models::ContentItem;
use codex_protocol::models::DEFAULT_IMAGE_DETAIL;
use codex_protocol::models::FunctionCallOutputBody;
@@ -84,6 +86,15 @@ impl ToolExecutor<ToolCall> for ImageGenerationTool {
async fn handle(&self, call: ToolCall) -> Result<Box<dyn ToolOutput>, FunctionCallError> {
let args = parse_args(&call)?;
let request = request_for_action(&args, call.conversation_history.items())?;
call.turn_item_emitter
.emit_started(ExtensionTurnItem::ImageGeneration(ImageGenerationItem {
id: call.call_id.clone(),
status: "in_progress".to_string(),
revised_prompt: None,
result: String::new(),
saved_path: None,
}))
.await;
let response = match request {
ImageRequest::Generate(request) => self.backend.generate(request).await,
ImageRequest::Edit(request) => self.backend.edit(request).await,
@@ -96,14 +107,16 @@ impl ToolExecutor<ToolCall> for ImageGenerationTool {
"image generation returned no image data".to_string(),
));
};
let output_hint = call
.turn_item_emitter
.image_generation_completed(call.call_id.clone(), args.prompt, result.clone())
call.turn_item_emitter
.emit_completed(ExtensionTurnItem::ImageGeneration(ImageGenerationItem {
id: call.call_id.clone(),
status: "completed".to_string(),
revised_prompt: Some(args.prompt),
result: result.clone(),
saved_path: None,
}))
.await;
Ok(Box::new(GeneratedImageOutput {
result,
output_hint,
}))
Ok(Box::new(GeneratedImageOutput { result }))
}
}
@@ -284,7 +297,6 @@ fn imagegen_tool_spec() -> ToolSpec {
struct GeneratedImageOutput {
result: String,
output_hint: Option<String>,
}
impl ToolOutput for GeneratedImageOutput {
@@ -298,17 +310,12 @@ impl ToolOutput for GeneratedImageOutput {
true
}
/// Returns generated bytes and persisted-artifact context for model follow-up.
/// Returns generated bytes for model follow-up.
fn to_response_item(&self, call_id: &str, _payload: &ToolPayload) -> ResponseInputItem {
let mut content = vec![FunctionCallOutputContentItem::InputImage {
let content = vec![FunctionCallOutputContentItem::InputImage {
image_url: format!("data:image/png;base64,{}", self.result),
detail: Some(DEFAULT_IMAGE_DETAIL),
}];
if let Some(output_hint) = &self.output_hint {
content.push(FunctionCallOutputContentItem::InputText {
text: output_hint.clone(),
});
}
ResponseInputItem::FunctionCallOutput {
call_id: call_id.to_string(),
output: FunctionCallOutputPayload {
@@ -48,6 +48,14 @@ impl From<AdditionalPermissionProfile> for RequestPermissionProfile {
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, JsonSchema, TS)]
pub struct RequestPermissionsArgs {
#[serde(
default,
rename = "environment_id",
alias = "environmentId",
skip_serializing_if = "Option::is_none"
)]
#[ts(optional)]
pub environment_id: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub reason: Option<String>,
pub permissions: RequestPermissionProfile,
@@ -71,6 +79,15 @@ pub struct RequestPermissionsEvent {
/// Uses `#[serde(default)]` for backwards compatibility.
#[serde(default)]
pub turn_id: String,
#[serde(
default,
rename = "environmentId",
alias = "environment_id",
skip_serializing_if = "Option::is_none"
)]
#[ts(optional)]
#[ts(rename = "environmentId")]
pub environment_id: Option<String>,
#[ts(type = "number")]
pub started_at_ms: i64,
#[serde(skip_serializing_if = "Option::is_none")]
-1
View File
@@ -64,7 +64,6 @@ pub use responses_api::mcp_tool_to_responses_api_tool;
pub use responses_api::tool_definition_to_responses_api_tool;
pub use tool_call::ConversationHistory;
pub use tool_call::ExtensionTurnItem;
pub use tool_call::ImageGenerationCompletionFuture;
pub use tool_call::NoopTurnItemEmitter;
pub use tool_call::ToolCall;
pub use tool_call::TurnItemEmissionFuture;
+5 -23
View File
@@ -1,6 +1,7 @@
use crate::FunctionCallError;
use crate::ToolName;
use crate::ToolPayload;
use codex_protocol::items::ImageGenerationItem;
use codex_protocol::items::WebSearchItem;
use codex_protocol::models::ResponseItem;
use codex_utils_output_truncation::TruncationPolicy;
@@ -29,20 +30,14 @@ impl ConversationHistory {
/// Future returned when an extension tool emits a visible turn-item lifecycle event.
pub type TurnItemEmissionFuture<'a> = Pin<Box<dyn Future<Output = ()> + Send + 'a>>;
/// Future returned when an image-generation extension publishes completed image bytes.
pub type ImageGenerationCompletionFuture<'a> =
Pin<Box<dyn Future<Output = Option<String>> + Send + 'a>>;
/// Visible turn items that an extension fully owns and may emit as-is.
///
/// Add only item kinds that require no additional host finalization before
/// persistence or client delivery. Richer items need a host-owned publish path.
/// Visible turn items that an extension may publish into the host lifecycle.
#[derive(Clone, Debug, PartialEq)]
pub enum ExtensionTurnItem {
WebSearch(WebSearchItem),
ImageGeneration(ImageGenerationItem),
}
/// Host-provided capability for extension tools to emit finalized visible turn items.
/// Host-provided capability for extension tools to emit visible turn items.
///
/// Implementations route lifecycle events through the host's normal item event
/// pipeline, including any persistence and client delivery owned by the host.
@@ -50,21 +45,8 @@ pub trait TurnItemEmitter: Send + Sync {
/// Emits the beginning of one visible turn item.
fn emit_started<'a>(&'a self, item: ExtensionTurnItem) -> TurnItemEmissionFuture<'a>;
/// Emits the completion of one visible turn item.
/// Emits one visible turn item after host-owned finalization.
fn emit_completed<'a>(&'a self, item: ExtensionTurnItem) -> TurnItemEmissionFuture<'a>;
/// Publishes image bytes for host persistence and visible completion.
///
/// Returns persisted-artifact context for the extension's model-facing
/// function output when the host saves the generated image successfully.
fn image_generation_completed<'a>(
&'a self,
_call_id: String,
_prompt: String,
_result: String,
) -> ImageGenerationCompletionFuture<'a> {
Box::pin(std::future::ready(None))
}
}
/// Turn-item emitter used when a caller does not expose visible item emission.
@@ -489,6 +489,7 @@ mod tests {
thread_id: "thread-1".to_string(),
turn_id: "turn-1".to_string(),
item_id: "perm-1".to_string(),
environment_id: None,
started_at_ms: 0,
cwd: absolute_path(if cfg!(windows) { r"C:\tmp" } else { "/tmp" }),
reason: None,
+8
View File
@@ -1856,12 +1856,20 @@ impl App {
));
}
ApprovalRequest::Permissions {
environment_id,
permissions,
reason,
..
} => {
let _ = tui.enter_alt_screen();
let mut lines = Vec::new();
if let Some(environment_id) = environment_id {
lines.push(Line::from(vec![
"Environment: ".into(),
environment_id.bold(),
]));
lines.push(Line::from(""));
}
if let Some(reason) = reason {
lines.push(Line::from(vec!["Reason: ".into(), reason.italic()]));
lines.push(Line::from(""));
+5 -1
View File
@@ -2539,6 +2539,7 @@ async fn inactive_thread_permissions_approval_preserves_file_system_permissions(
thread_id: thread_id.to_string(),
turn_id: "turn-approval".to_string(),
item_id: "call-approval".to_string(),
environment_id: Some("remote".to_string()),
started_at_ms: 0,
cwd: test_absolute_path("/tmp"),
reason: Some("Need access to .git".to_string()),
@@ -2557,7 +2558,9 @@ async fn inactive_thread_permissions_approval_preserves_file_system_permissions(
};
let Some(ThreadInteractiveRequest::Approval(ApprovalRequest::Permissions {
permissions, ..
environment_id,
permissions,
..
})) = app
.interactive_request_for_thread_request(thread_id, &request)
.await
@@ -2565,6 +2568,7 @@ async fn inactive_thread_permissions_approval_preserves_file_system_permissions(
panic!("expected permissions approval request");
};
assert_eq!(environment_id.as_deref(), Some("remote"));
assert_eq!(
permissions,
RequestPermissionProfile {
+1
View File
@@ -310,6 +310,7 @@ impl App {
thread_id,
thread_label,
call_id: params.item_id.clone(),
environment_id: params.environment_id.clone(),
reason: params.reason.clone(),
permissions: params.permissions.clone().into(),
}),
@@ -84,6 +84,7 @@ pub(crate) enum ApprovalRequest {
thread_id: ThreadId,
thread_label: Option<String>,
call_id: String,
environment_id: Option<String>,
reason: Option<String>,
permissions: RequestPermissionProfile,
},
@@ -713,6 +714,7 @@ fn build_header(request: &ApprovalRequest) -> Box<dyn Renderable> {
}
ApprovalRequest::Permissions {
thread_label,
environment_id,
reason,
permissions,
..
@@ -725,6 +727,13 @@ fn build_header(request: &ApprovalRequest) -> Box<dyn Renderable> {
]));
header.push(Line::from(""));
}
if let Some(environment_id) = environment_id {
header.push(Line::from(vec![
"Environment: ".into(),
environment_id.clone().bold(),
]));
header.push(Line::from(""));
}
if let Some(reason) = reason {
header.push(Line::from(vec!["Reason: ".into(), reason.clone().italic()]));
header.push(Line::from(""));
@@ -1227,6 +1236,7 @@ mod tests {
thread_id: ThreadId::new(),
thread_label: None,
call_id: "test".to_string(),
environment_id: None,
reason: Some("need workspace access".to_string()),
permissions: RequestPermissionProfile {
network: Some(NetworkPermissions {
+1
View File
@@ -864,6 +864,7 @@ fn request_permissions_from_params(
RequestPermissionsEvent {
turn_id: params.turn_id,
call_id: params.item_id,
environment_id: params.environment_id,
started_at_ms: params.started_at_ms,
reason: params.reason,
permissions: params.permissions.into(),
@@ -281,6 +281,7 @@ fn app_server_request_permissions_preserves_file_system_permissions() {
thread_id: "thread-1".to_string(),
turn_id: "turn-1".to_string(),
item_id: "item-1".to_string(),
environment_id: Some("remote".to_string()),
started_at_ms: 0,
cwd: cwd.clone(),
reason: Some("Select a workspace root".to_string()),
@@ -310,6 +311,7 @@ fn app_server_request_permissions_preserves_file_system_permissions() {
}
);
assert_eq!(request.cwd, Some(cwd));
assert_eq!(request.environment_id.as_deref(), Some("remote"));
}
#[tokio::test]
@@ -441,6 +441,7 @@ impl ChatWidget {
thread_id: self.thread_id.unwrap_or_default(),
thread_label: None,
call_id: ev.call_id,
environment_id: ev.environment_id,
reason: ev.reason,
permissions: ev.permissions,
};