mirror of
https://github.com/openai/codex.git
synced 2026-10-25 18:28:38 +00:00
permissions: move workspace roots onto thread state
This commit is contained in:
@@ -448,12 +448,12 @@ async fn start_uninitialized(args: InProcessStartArgs) -> IoResult<InProcessClie
|
||||
let was_initialized = session.initialized();
|
||||
processor
|
||||
.process_client_request(
|
||||
IN_PROCESS_CONNECTION_ID,
|
||||
*request,
|
||||
Arc::clone(&session),
|
||||
&outbound_initialized,
|
||||
)
|
||||
.await;
|
||||
IN_PROCESS_CONNECTION_ID,
|
||||
*request,
|
||||
Arc::clone(&session),
|
||||
&outbound_initialized,
|
||||
)
|
||||
.await;
|
||||
let opted_out_notification_methods_snapshot =
|
||||
session.opted_out_notification_methods();
|
||||
let experimental_api_enabled =
|
||||
@@ -797,80 +797,119 @@ mod tests {
|
||||
start_test_client_with_capacity(session_source, DEFAULT_IN_PROCESS_CHANNEL_CAPACITY).await
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn in_process_start_initializes_and_handles_typed_v2_request() {
|
||||
let client = start_test_client(SessionSource::Cli).await;
|
||||
let response = client
|
||||
.request(ClientRequest::ConfigRequirementsRead {
|
||||
request_id: RequestId::Integer(1),
|
||||
params: None,
|
||||
fn run_current_thread_test_with_stack<F>(name: &str, future: F)
|
||||
where
|
||||
F: Future<Output = ()> + Send + 'static,
|
||||
{
|
||||
const TEST_STACK_SIZE_BYTES: usize = 4 * 1024 * 1024;
|
||||
|
||||
let handle = std::thread::Builder::new()
|
||||
.name(name.to_string())
|
||||
.stack_size(TEST_STACK_SIZE_BYTES)
|
||||
.spawn(move || {
|
||||
let runtime = tokio::runtime::Builder::new_current_thread()
|
||||
.enable_all()
|
||||
.build()
|
||||
.expect("current-thread runtime should build");
|
||||
runtime.block_on(Box::pin(future));
|
||||
})
|
||||
.await
|
||||
.expect("request transport should work")
|
||||
.expect("request should succeed");
|
||||
assert!(response.is_object());
|
||||
.expect("stack-sized test thread should spawn");
|
||||
|
||||
let _parsed: ConfigRequirementsReadResponse =
|
||||
serde_json::from_value(response).expect("response should match v2 schema");
|
||||
client
|
||||
.shutdown()
|
||||
.await
|
||||
.expect("in-process runtime should shutdown cleanly");
|
||||
assert!(handle.join().is_ok(), "{name} thread panicked");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn in_process_start_uses_requested_session_source_for_thread_start() {
|
||||
for (requested_source, expected_source) in [
|
||||
(SessionSource::Cli, ApiSessionSource::Cli),
|
||||
(SessionSource::Exec, ApiSessionSource::Exec),
|
||||
] {
|
||||
let client = start_test_client(requested_source).await;
|
||||
let response = client
|
||||
.request(ClientRequest::ThreadStart {
|
||||
request_id: RequestId::Integer(2),
|
||||
params: ThreadStartParams {
|
||||
ephemeral: Some(true),
|
||||
..ThreadStartParams::default()
|
||||
},
|
||||
})
|
||||
.await
|
||||
.expect("request transport should work")
|
||||
.expect("thread/start should succeed");
|
||||
let parsed: ThreadStartResponse =
|
||||
serde_json::from_value(response).expect("thread/start response should parse");
|
||||
assert_eq!(parsed.thread.source, expected_source);
|
||||
client
|
||||
.shutdown()
|
||||
.await
|
||||
.expect("in-process runtime should shutdown cleanly");
|
||||
}
|
||||
#[test]
|
||||
fn in_process_start_initializes_and_handles_typed_v2_request() {
|
||||
run_current_thread_test_with_stack(
|
||||
"in_process_start_initializes_and_handles_typed_v2_request",
|
||||
async {
|
||||
let client = start_test_client(SessionSource::Cli).await;
|
||||
let response = client
|
||||
.request(ClientRequest::ConfigRequirementsRead {
|
||||
request_id: RequestId::Integer(1),
|
||||
params: None,
|
||||
})
|
||||
.await
|
||||
.expect("request transport should work")
|
||||
.expect("request should succeed");
|
||||
assert!(response.is_object());
|
||||
|
||||
let _parsed: ConfigRequirementsReadResponse =
|
||||
serde_json::from_value(response).expect("response should match v2 schema");
|
||||
client
|
||||
.shutdown()
|
||||
.await
|
||||
.expect("in-process runtime should shutdown cleanly");
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn in_process_start_clamps_zero_channel_capacity() {
|
||||
let client =
|
||||
start_test_client_with_capacity(SessionSource::Cli, /*channel_capacity*/ 0).await;
|
||||
let response = loop {
|
||||
match client
|
||||
.request(ClientRequest::ConfigRequirementsRead {
|
||||
request_id: RequestId::Integer(4),
|
||||
params: None,
|
||||
})
|
||||
.await
|
||||
{
|
||||
Ok(response) => break response.expect("request should succeed"),
|
||||
Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => {
|
||||
tokio::task::yield_now().await;
|
||||
#[test]
|
||||
fn in_process_start_uses_requested_session_source_for_thread_start() {
|
||||
run_current_thread_test_with_stack(
|
||||
"in_process_start_uses_requested_session_source_for_thread_start",
|
||||
async {
|
||||
for (requested_source, expected_source) in [
|
||||
(SessionSource::Cli, ApiSessionSource::Cli),
|
||||
(SessionSource::Exec, ApiSessionSource::Exec),
|
||||
] {
|
||||
let client = start_test_client(requested_source).await;
|
||||
let response = client
|
||||
.request(ClientRequest::ThreadStart {
|
||||
request_id: RequestId::Integer(2),
|
||||
params: ThreadStartParams {
|
||||
ephemeral: Some(true),
|
||||
..ThreadStartParams::default()
|
||||
},
|
||||
})
|
||||
.await
|
||||
.expect("request transport should work")
|
||||
.expect("thread/start should succeed");
|
||||
let parsed: ThreadStartResponse = serde_json::from_value(response)
|
||||
.expect("thread/start response should parse");
|
||||
assert_eq!(parsed.thread.source, expected_source);
|
||||
client
|
||||
.shutdown()
|
||||
.await
|
||||
.expect("in-process runtime should shutdown cleanly");
|
||||
}
|
||||
Err(err) => panic!("request transport should work: {err}"),
|
||||
}
|
||||
};
|
||||
let _parsed: ConfigRequirementsReadResponse =
|
||||
serde_json::from_value(response).expect("response should match v2 schema");
|
||||
client
|
||||
.shutdown()
|
||||
.await
|
||||
.expect("in-process runtime should shutdown cleanly");
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn in_process_start_clamps_zero_channel_capacity() {
|
||||
run_current_thread_test_with_stack(
|
||||
"in_process_start_clamps_zero_channel_capacity",
|
||||
async {
|
||||
let client = start_test_client_with_capacity(
|
||||
SessionSource::Cli,
|
||||
/*channel_capacity*/ 0,
|
||||
)
|
||||
.await;
|
||||
let response = loop {
|
||||
match client
|
||||
.request(ClientRequest::ConfigRequirementsRead {
|
||||
request_id: RequestId::Integer(4),
|
||||
params: None,
|
||||
})
|
||||
.await
|
||||
{
|
||||
Ok(response) => break response.expect("request should succeed"),
|
||||
Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => {
|
||||
tokio::task::yield_now().await;
|
||||
}
|
||||
Err(err) => panic!("request transport should work: {err}"),
|
||||
}
|
||||
};
|
||||
let _parsed: ConfigRequirementsReadResponse =
|
||||
serde_json::from_value(response).expect("response should match v2 schema");
|
||||
client
|
||||
.shutdown()
|
||||
.await
|
||||
.expect("in-process runtime should shutdown cleanly");
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -581,7 +581,7 @@ pub async fn run_main_with_transport_options(
|
||||
});
|
||||
}
|
||||
if let Some(warning) =
|
||||
codex_core::config::system_bwrap_warning(config.permissions.permission_profile.get())
|
||||
codex_core::config::system_bwrap_warning(config.permissions.permission_profile_ref())
|
||||
{
|
||||
config_warnings.push(ConfigWarningNotification {
|
||||
summary: warning,
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use std::collections::HashSet;
|
||||
use std::future::Future;
|
||||
use std::pin::Pin;
|
||||
use std::sync::Arc;
|
||||
use std::sync::OnceLock;
|
||||
use std::sync::atomic::AtomicBool;
|
||||
@@ -501,13 +502,13 @@ impl MessageProcessor {
|
||||
self.account_processor.clear_external_auth();
|
||||
}
|
||||
|
||||
pub(crate) async fn process_request(
|
||||
self: &Arc<Self>,
|
||||
pub(crate) fn process_request<'a>(
|
||||
self: &'a Arc<Self>,
|
||||
connection_id: ConnectionId,
|
||||
request: JSONRPCRequest,
|
||||
transport: &AppServerTransport,
|
||||
transport: &'a AppServerTransport,
|
||||
session: Arc<ConnectionSessionState>,
|
||||
) {
|
||||
) -> Pin<Box<dyn Future<Output = ()> + Send + 'a>> {
|
||||
let request_method = request.method.as_str();
|
||||
tracing::trace!(
|
||||
?connection_id,
|
||||
@@ -525,52 +526,55 @@ impl MessageProcessor {
|
||||
tracestate: trace.tracestate.clone(),
|
||||
});
|
||||
let request_context = RequestContext::new(request_id.clone(), request_span, request_trace);
|
||||
Self::run_request_with_context(
|
||||
Arc::clone(&self.outgoing),
|
||||
request_context.clone(),
|
||||
async {
|
||||
let codex_request = serde_json::to_value(&request)
|
||||
.map_err(|err| invalid_request(format!("Invalid request: {err}")))
|
||||
.and_then(|request_json| {
|
||||
serde_json::from_value::<ClientRequest>(request_json)
|
||||
.map_err(|err| invalid_request(format!("Invalid request: {err}")))
|
||||
});
|
||||
let result = match codex_request {
|
||||
Ok(codex_request) => {
|
||||
// Websocket callers finalize outbound readiness in lib.rs after mirroring
|
||||
// session state into outbound state and sending initialize notifications to
|
||||
// this specific connection. Passing `None` avoids marking the connection
|
||||
// ready too early from inside the shared request handler.
|
||||
self.handle_client_request(
|
||||
request_id.clone(),
|
||||
codex_request,
|
||||
Arc::clone(&session),
|
||||
/*outbound_initialized*/ None,
|
||||
request_context.clone(),
|
||||
)
|
||||
.await
|
||||
Box::pin(async move {
|
||||
Self::run_request_with_context(
|
||||
Arc::clone(&self.outgoing),
|
||||
request_context.clone(),
|
||||
Box::pin(async move {
|
||||
let codex_request = serde_json::to_value(&request)
|
||||
.map_err(|err| invalid_request(format!("Invalid request: {err}")))
|
||||
.and_then(|request_json| {
|
||||
serde_json::from_value::<ClientRequest>(request_json)
|
||||
.map_err(|err| invalid_request(format!("Invalid request: {err}")))
|
||||
});
|
||||
let result = match codex_request {
|
||||
Ok(codex_request) => {
|
||||
// Websocket callers finalize outbound readiness in lib.rs after
|
||||
// mirroring session state into outbound state and sending initialize
|
||||
// notifications to this specific connection. Passing `None` avoids
|
||||
// marking the connection ready too early from inside the shared
|
||||
// request handler.
|
||||
self.handle_client_request(
|
||||
request_id.clone(),
|
||||
codex_request,
|
||||
Arc::clone(&session),
|
||||
/*outbound_initialized*/ None,
|
||||
request_context.clone(),
|
||||
)
|
||||
.await
|
||||
}
|
||||
Err(error) => Err(error),
|
||||
};
|
||||
if let Err(error) = result {
|
||||
self.outgoing.send_error(request_id.clone(), error).await;
|
||||
}
|
||||
Err(error) => Err(error),
|
||||
};
|
||||
if let Err(error) = result {
|
||||
self.outgoing.send_error(request_id.clone(), error).await;
|
||||
}
|
||||
},
|
||||
)
|
||||
.await;
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
})
|
||||
}
|
||||
|
||||
/// Handles a typed request path used by in-process embedders.
|
||||
///
|
||||
/// This bypasses JSON request deserialization but keeps identical request
|
||||
/// semantics by delegating to `handle_client_request`.
|
||||
pub(crate) async fn process_client_request(
|
||||
self: &Arc<Self>,
|
||||
pub(crate) fn process_client_request<'a>(
|
||||
self: &'a Arc<Self>,
|
||||
connection_id: ConnectionId,
|
||||
request: ClientRequest,
|
||||
session: Arc<ConnectionSessionState>,
|
||||
outbound_initialized: &AtomicBool,
|
||||
) {
|
||||
outbound_initialized: &'a AtomicBool,
|
||||
) -> Pin<Box<dyn Future<Output = ()> + Send + 'a>> {
|
||||
let request_id = ConnectionRequestId {
|
||||
connection_id,
|
||||
request_id: request.id().clone(),
|
||||
@@ -584,28 +588,30 @@ impl MessageProcessor {
|
||||
request_id = ?request_id.request_id,
|
||||
"app-server typed request"
|
||||
);
|
||||
Self::run_request_with_context(
|
||||
Arc::clone(&self.outgoing),
|
||||
request_context.clone(),
|
||||
async {
|
||||
// In-process clients do not have the websocket transport loop that performs
|
||||
// post-initialize bookkeeping, so they still finalize outbound readiness in
|
||||
// the shared request handler.
|
||||
let result = self
|
||||
.handle_client_request(
|
||||
request_id.clone(),
|
||||
request,
|
||||
Arc::clone(&session),
|
||||
Some(outbound_initialized),
|
||||
request_context.clone(),
|
||||
)
|
||||
.await;
|
||||
if let Err(error) = result {
|
||||
self.outgoing.send_error(request_id.clone(), error).await;
|
||||
}
|
||||
},
|
||||
)
|
||||
.await;
|
||||
Box::pin(async move {
|
||||
Self::run_request_with_context(
|
||||
Arc::clone(&self.outgoing),
|
||||
request_context.clone(),
|
||||
Box::pin(async move {
|
||||
// In-process clients do not have the websocket transport loop that performs
|
||||
// post-initialize bookkeeping, so they still finalize outbound readiness in
|
||||
// the shared request handler.
|
||||
let result = self
|
||||
.handle_client_request(
|
||||
request_id.clone(),
|
||||
request,
|
||||
Arc::clone(&session),
|
||||
Some(outbound_initialized),
|
||||
request_context.clone(),
|
||||
)
|
||||
.await;
|
||||
if let Err(error) = result {
|
||||
self.outgoing.send_error(request_id.clone(), error).await;
|
||||
}
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) async fn process_notification(&self, notification: JSONRPCNotification) {
|
||||
@@ -621,13 +627,11 @@ impl MessageProcessor {
|
||||
tracing::info!("<- typed notification: {:?}", notification);
|
||||
}
|
||||
|
||||
async fn run_request_with_context<F>(
|
||||
async fn run_request_with_context<'a>(
|
||||
outgoing: Arc<OutgoingMessageSender>,
|
||||
request_context: RequestContext,
|
||||
request_fut: F,
|
||||
) where
|
||||
F: Future<Output = ()>,
|
||||
{
|
||||
request_fut: Pin<Box<dyn Future<Output = ()> + Send + 'a>>,
|
||||
) {
|
||||
outgoing
|
||||
.register_request_context(request_context.clone())
|
||||
.await;
|
||||
|
||||
@@ -632,77 +632,80 @@ fn thread_start_jsonrpc_span_exports_server_span_and_parents_children() -> Resul
|
||||
)
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
#[serial(app_server_tracing)]
|
||||
async fn turn_start_jsonrpc_span_parents_core_turn_spans() -> Result<()> {
|
||||
let mut harness = TracingHarness::new().await?;
|
||||
let thread_start_response = harness.start_thread(/*request_id*/ 2, /*trace*/ None).await;
|
||||
let thread_id = thread_start_response.thread.id.clone();
|
||||
#[test]
|
||||
fn turn_start_jsonrpc_span_parents_core_turn_spans() -> Result<()> {
|
||||
run_current_thread_test_with_stack("turn_start_jsonrpc_span_parents_core_turn_spans", async {
|
||||
let mut harness = TracingHarness::new().await?;
|
||||
let thread_start_response = harness.start_thread(/*request_id*/ 2, /*trace*/ None).await;
|
||||
let thread_id = thread_start_response.thread.id.clone();
|
||||
|
||||
harness.reset_tracing();
|
||||
harness.reset_tracing();
|
||||
|
||||
let RemoteTrace {
|
||||
trace_id: remote_trace_id,
|
||||
parent_span_id: remote_parent_span_id,
|
||||
context: remote_trace,
|
||||
} = RemoteTrace::new("00000000000000000000000000000077", "0000000000000088");
|
||||
let turn_start_response: TurnStartResponse = harness
|
||||
.request(
|
||||
ClientRequest::TurnStart {
|
||||
request_id: RequestId::Integer(3),
|
||||
params: TurnStartParams {
|
||||
environments: None,
|
||||
thread_id,
|
||||
input: vec![UserInput::Text {
|
||||
text: "hello".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
responsesapi_client_metadata: None,
|
||||
cwd: None,
|
||||
approval_policy: None,
|
||||
sandbox_policy: None,
|
||||
permissions: None,
|
||||
approvals_reviewer: None,
|
||||
model: None,
|
||||
service_tier: None,
|
||||
effort: None,
|
||||
summary: None,
|
||||
personality: None,
|
||||
output_schema: None,
|
||||
collaboration_mode: None,
|
||||
let RemoteTrace {
|
||||
trace_id: remote_trace_id,
|
||||
parent_span_id: remote_parent_span_id,
|
||||
context: remote_trace,
|
||||
} = RemoteTrace::new("00000000000000000000000000000077", "0000000000000088");
|
||||
let turn_start_response: TurnStartResponse = harness
|
||||
.request(
|
||||
ClientRequest::TurnStart {
|
||||
request_id: RequestId::Integer(3),
|
||||
params: TurnStartParams {
|
||||
environments: None,
|
||||
thread_id,
|
||||
input: vec![UserInput::Text {
|
||||
text: "hello".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
responsesapi_client_metadata: None,
|
||||
cwd: None,
|
||||
workspace_roots: None,
|
||||
approval_policy: None,
|
||||
sandbox_policy: None,
|
||||
permissions: None,
|
||||
approvals_reviewer: None,
|
||||
model: None,
|
||||
service_tier: None,
|
||||
effort: None,
|
||||
summary: None,
|
||||
personality: None,
|
||||
output_schema: None,
|
||||
collaboration_mode: None,
|
||||
},
|
||||
},
|
||||
},
|
||||
Some(remote_trace),
|
||||
)
|
||||
.await;
|
||||
let spans = wait_for_exported_spans(harness.tracing, |spans| {
|
||||
spans.iter().any(|span| {
|
||||
span.span_kind == SpanKind::Server
|
||||
&& span_attr(span, "rpc.method") == Some("turn/start")
|
||||
&& span.span_context.trace_id() == remote_trace_id
|
||||
}) && spans.iter().any(|span| {
|
||||
span_attr(span, "codex.op") == Some("user_input")
|
||||
&& span.span_context.trace_id() == remote_trace_id
|
||||
Some(remote_trace),
|
||||
)
|
||||
.await;
|
||||
let spans = wait_for_exported_spans(harness.tracing, |spans| {
|
||||
spans.iter().any(|span| {
|
||||
span.span_kind == SpanKind::Server
|
||||
&& span_attr(span, "rpc.method") == Some("turn/start")
|
||||
&& span.span_context.trace_id() == remote_trace_id
|
||||
}) && spans.iter().any(|span| {
|
||||
span_attr(span, "codex.op") == Some("user_input")
|
||||
&& span.span_context.trace_id() == remote_trace_id
|
||||
})
|
||||
})
|
||||
.await;
|
||||
|
||||
let server_request_span =
|
||||
find_rpc_span_with_trace(&spans, SpanKind::Server, "turn/start", remote_trace_id);
|
||||
let core_turn_span =
|
||||
find_span_with_trace(&spans, remote_trace_id, "codex.op=user_input", |span| {
|
||||
span_attr(span, "codex.op") == Some("user_input")
|
||||
});
|
||||
|
||||
assert_eq!(server_request_span.parent_span_id, remote_parent_span_id);
|
||||
assert!(server_request_span.parent_span_is_remote);
|
||||
assert_eq!(server_request_span.span_context.trace_id(), remote_trace_id);
|
||||
assert_eq!(
|
||||
span_attr(server_request_span, "turn.id"),
|
||||
Some(turn_start_response.turn.id.as_str())
|
||||
);
|
||||
assert_span_descends_from(&spans, core_turn_span, server_request_span);
|
||||
harness.shutdown().await;
|
||||
|
||||
Ok(())
|
||||
})
|
||||
.await;
|
||||
|
||||
let server_request_span =
|
||||
find_rpc_span_with_trace(&spans, SpanKind::Server, "turn/start", remote_trace_id);
|
||||
let core_turn_span =
|
||||
find_span_with_trace(&spans, remote_trace_id, "codex.op=user_input", |span| {
|
||||
span_attr(span, "codex.op") == Some("user_input")
|
||||
});
|
||||
|
||||
assert_eq!(server_request_span.parent_span_id, remote_parent_span_id);
|
||||
assert!(server_request_span.parent_span_is_remote);
|
||||
assert_eq!(server_request_span.span_context.trace_id(), remote_trace_id);
|
||||
assert_eq!(
|
||||
span_attr(server_request_span, "turn.id"),
|
||||
Some(turn_start_response.turn.id.as_str())
|
||||
);
|
||||
assert_span_descends_from(&spans, core_turn_span, server_request_span);
|
||||
harness.shutdown().await;
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -103,8 +103,6 @@ use codex_app_server_protocol::MockExperimentalMethodParams;
|
||||
use codex_app_server_protocol::MockExperimentalMethodResponse;
|
||||
use codex_app_server_protocol::ModelListParams;
|
||||
use codex_app_server_protocol::ModelListResponse;
|
||||
use codex_app_server_protocol::PermissionProfileModificationParams;
|
||||
use codex_app_server_protocol::PermissionProfileSelectionParams;
|
||||
use codex_app_server_protocol::PluginDetail;
|
||||
use codex_app_server_protocol::PluginInstallParams;
|
||||
use codex_app_server_protocol::PluginInstallResponse;
|
||||
|
||||
@@ -164,7 +164,7 @@ impl CommandExecRequestProcessor {
|
||||
let started_network_proxy = match self.config.permissions.network.as_ref() {
|
||||
Some(spec) => match spec
|
||||
.start_proxy(
|
||||
self.config.permissions.permission_profile.get(),
|
||||
self.config.permissions.permission_profile_ref(),
|
||||
/*policy_decider*/ None,
|
||||
/*blocked_request_observer*/ None,
|
||||
managed_network_requirements_enabled,
|
||||
@@ -200,7 +200,8 @@ impl CommandExecRequestProcessor {
|
||||
} else {
|
||||
ExecCapturePolicy::ShellTool
|
||||
};
|
||||
let sandbox_cwd = if permission_profile.is_some() {
|
||||
let has_request_permission_profile = permission_profile.is_some();
|
||||
let sandbox_cwd = if has_request_permission_profile {
|
||||
cwd.clone()
|
||||
} else {
|
||||
self.config.cwd.clone()
|
||||
@@ -243,28 +244,20 @@ impl CommandExecRequestProcessor {
|
||||
);
|
||||
self.config
|
||||
.permissions
|
||||
.permission_profile
|
||||
.permission_profile_constraint()
|
||||
.can_set(&effective_permission_profile)
|
||||
.map_err(|err| invalid_request(format!("invalid permission profile: {err}")))?;
|
||||
effective_permission_profile
|
||||
} else if let Some(policy) = sandbox_policy.map(|policy| policy.to_core()) {
|
||||
} else if let Some(policy) = sandbox_policy {
|
||||
let legacy_policy = policy.to_core();
|
||||
self.config
|
||||
.permissions
|
||||
.can_set_legacy_sandbox_policy(&policy, &sandbox_cwd)
|
||||
.can_set_legacy_sandbox_policy(&legacy_policy, &sandbox_cwd)
|
||||
.map_err(|err| invalid_request(format!("invalid sandbox policy: {err}")))?;
|
||||
let file_system_sandbox_policy =
|
||||
codex_protocol::permissions::FileSystemSandboxPolicy::from_legacy_sandbox_policy_for_cwd(&policy, &sandbox_cwd);
|
||||
let network_sandbox_policy =
|
||||
codex_protocol::permissions::NetworkSandboxPolicy::from(&policy);
|
||||
let permission_profile =
|
||||
codex_protocol::models::PermissionProfile::from_runtime_permissions_with_enforcement(
|
||||
codex_protocol::models::SandboxEnforcement::from_legacy_sandbox_policy(&policy),
|
||||
&file_system_sandbox_policy,
|
||||
network_sandbox_policy,
|
||||
);
|
||||
let permission_profile = policy.to_permission_profile_for_cwd(&sandbox_cwd);
|
||||
self.config
|
||||
.permissions
|
||||
.permission_profile
|
||||
.permission_profile_constraint()
|
||||
.can_set(&permission_profile)
|
||||
.map_err(|err| invalid_request(format!("invalid sandbox policy: {err}")))?;
|
||||
permission_profile
|
||||
@@ -283,10 +276,16 @@ impl CommandExecRequestProcessor {
|
||||
None => None,
|
||||
};
|
||||
|
||||
let workspace_roots = if has_request_permission_profile {
|
||||
vec![sandbox_cwd.clone()]
|
||||
} else {
|
||||
self.config.workspace_roots.clone()
|
||||
};
|
||||
let exec_request = codex_core::exec::build_exec_request(
|
||||
exec_params,
|
||||
&effective_permission_profile,
|
||||
&sandbox_cwd,
|
||||
&workspace_roots,
|
||||
&codex_linux_sandbox_exe,
|
||||
use_legacy_landlock,
|
||||
)
|
||||
|
||||
@@ -604,11 +604,15 @@ pub(super) async fn handle_pending_thread_resume_request(
|
||||
permission_profile,
|
||||
active_permission_profile,
|
||||
cwd,
|
||||
workspace_roots,
|
||||
reasoning_effort,
|
||||
..
|
||||
} = pending.config_snapshot;
|
||||
let instruction_sources = pending.instruction_sources;
|
||||
let sandbox = thread_response_sandbox_policy(&permission_profile, cwd.as_path());
|
||||
let sandbox =
|
||||
thread_response_sandbox_policy(&permission_profile, &workspace_roots, cwd.as_path());
|
||||
let response_permission_profile =
|
||||
thread_response_permission_profile(&permission_profile, &workspace_roots);
|
||||
let active_permission_profile =
|
||||
thread_response_active_permission_profile(active_permission_profile);
|
||||
let session_id = conversation.session_configured().session_id.to_string();
|
||||
@@ -624,7 +628,8 @@ pub(super) async fn handle_pending_thread_resume_request(
|
||||
approval_policy: approval_policy.into(),
|
||||
approvals_reviewer: approvals_reviewer.into(),
|
||||
sandbox,
|
||||
permission_profile: Some(permission_profile.into()),
|
||||
permission_profile: Some(response_permission_profile.into()),
|
||||
workspace_roots,
|
||||
active_permission_profile,
|
||||
reasoning_effort,
|
||||
};
|
||||
|
||||
@@ -802,6 +802,7 @@ impl ThreadRequestProcessor {
|
||||
model_provider,
|
||||
service_tier,
|
||||
cwd,
|
||||
workspace_roots,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox,
|
||||
@@ -835,6 +836,7 @@ impl ThreadRequestProcessor {
|
||||
model_provider,
|
||||
service_tier,
|
||||
cwd,
|
||||
workspace_roots,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox,
|
||||
@@ -1160,8 +1162,13 @@ impl ThreadRequestProcessor {
|
||||
|
||||
let sandbox = thread_response_sandbox_policy(
|
||||
&config_snapshot.permission_profile,
|
||||
&config_snapshot.workspace_roots,
|
||||
config_snapshot.cwd.as_path(),
|
||||
);
|
||||
let permission_profile = thread_response_permission_profile(
|
||||
&config_snapshot.permission_profile,
|
||||
&config_snapshot.workspace_roots,
|
||||
);
|
||||
let active_permission_profile =
|
||||
thread_response_active_permission_profile(config_snapshot.active_permission_profile);
|
||||
|
||||
@@ -1175,7 +1182,8 @@ impl ThreadRequestProcessor {
|
||||
approval_policy: config_snapshot.approval_policy.into(),
|
||||
approvals_reviewer: config_snapshot.approvals_reviewer.into(),
|
||||
sandbox,
|
||||
permission_profile: Some(config_snapshot.permission_profile.into()),
|
||||
permission_profile: Some(permission_profile.into()),
|
||||
workspace_roots: config_snapshot.workspace_roots,
|
||||
active_permission_profile,
|
||||
reasoning_effort: config_snapshot.reasoning_effort,
|
||||
};
|
||||
@@ -1212,10 +1220,11 @@ impl ThreadRequestProcessor {
|
||||
model_provider: Option<String>,
|
||||
service_tier: Option<Option<String>>,
|
||||
cwd: Option<String>,
|
||||
workspace_roots: Option<Vec<AbsolutePathBuf>>,
|
||||
approval_policy: Option<codex_app_server_protocol::AskForApproval>,
|
||||
approvals_reviewer: Option<codex_app_server_protocol::ApprovalsReviewer>,
|
||||
sandbox: Option<SandboxMode>,
|
||||
permissions: Option<PermissionProfileSelectionParams>,
|
||||
permissions: Option<String>,
|
||||
base_instructions: Option<String>,
|
||||
developer_instructions: Option<String>,
|
||||
personality: Option<Personality>,
|
||||
@@ -1225,6 +1234,8 @@ impl ThreadRequestProcessor {
|
||||
model_provider,
|
||||
service_tier,
|
||||
cwd: cwd.map(PathBuf::from),
|
||||
workspace_roots: workspace_roots
|
||||
.map(|roots| roots.into_iter().map(|root| root.to_path_buf()).collect()),
|
||||
approval_policy: approval_policy
|
||||
.map(codex_app_server_protocol::AskForApproval::to_core),
|
||||
approvals_reviewer: approvals_reviewer
|
||||
@@ -2349,6 +2360,7 @@ impl ThreadRequestProcessor {
|
||||
model_provider,
|
||||
service_tier,
|
||||
cwd,
|
||||
workspace_roots,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox,
|
||||
@@ -2384,6 +2396,7 @@ impl ThreadRequestProcessor {
|
||||
model_provider,
|
||||
service_tier,
|
||||
cwd,
|
||||
workspace_roots,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox,
|
||||
@@ -2505,8 +2518,13 @@ impl ThreadRequestProcessor {
|
||||
let config_snapshot = codex_thread.config_snapshot().await;
|
||||
let sandbox = thread_response_sandbox_policy(
|
||||
&config_snapshot.permission_profile,
|
||||
&config_snapshot.workspace_roots,
|
||||
config_snapshot.cwd.as_path(),
|
||||
);
|
||||
let permission_profile = thread_response_permission_profile(
|
||||
&config_snapshot.permission_profile,
|
||||
&config_snapshot.workspace_roots,
|
||||
);
|
||||
let active_permission_profile = thread_response_active_permission_profile(
|
||||
config_snapshot.active_permission_profile,
|
||||
);
|
||||
@@ -2525,7 +2543,8 @@ impl ThreadRequestProcessor {
|
||||
approval_policy: session_configured.approval_policy.into(),
|
||||
approvals_reviewer: session_configured.approvals_reviewer.into(),
|
||||
sandbox,
|
||||
permission_profile: Some(config_snapshot.permission_profile.into()),
|
||||
permission_profile: Some(permission_profile.into()),
|
||||
workspace_roots: config_snapshot.workspace_roots,
|
||||
active_permission_profile,
|
||||
reasoning_effort: session_configured.reasoning_effort,
|
||||
};
|
||||
@@ -2985,6 +3004,7 @@ impl ThreadRequestProcessor {
|
||||
model_provider,
|
||||
service_tier,
|
||||
cwd,
|
||||
workspace_roots,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox,
|
||||
@@ -3050,6 +3070,7 @@ impl ThreadRequestProcessor {
|
||||
model_provider,
|
||||
service_tier,
|
||||
cwd,
|
||||
workspace_roots,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox,
|
||||
@@ -3168,8 +3189,13 @@ impl ThreadRequestProcessor {
|
||||
let config_snapshot = forked_thread.config_snapshot().await;
|
||||
let sandbox = thread_response_sandbox_policy(
|
||||
&config_snapshot.permission_profile,
|
||||
&config_snapshot.workspace_roots,
|
||||
config_snapshot.cwd.as_path(),
|
||||
);
|
||||
let permission_profile = thread_response_permission_profile(
|
||||
&config_snapshot.permission_profile,
|
||||
&config_snapshot.workspace_roots,
|
||||
);
|
||||
let active_permission_profile =
|
||||
thread_response_active_permission_profile(config_snapshot.active_permission_profile);
|
||||
|
||||
@@ -3183,7 +3209,8 @@ impl ThreadRequestProcessor {
|
||||
approval_policy: session_configured.approval_policy.into(),
|
||||
approvals_reviewer: session_configured.approvals_reviewer.into(),
|
||||
sandbox,
|
||||
permission_profile: Some(config_snapshot.permission_profile.into()),
|
||||
permission_profile: Some(permission_profile.into()),
|
||||
workspace_roots: config_snapshot.workspace_roots,
|
||||
active_permission_profile,
|
||||
reasoning_effort: session_configured.reasoning_effort,
|
||||
};
|
||||
|
||||
@@ -631,6 +631,7 @@ mod thread_processor_behavior_tests {
|
||||
model_provider: None,
|
||||
service_tier: Some(Some("priority".to_string())),
|
||||
cwd: None,
|
||||
workspace_roots: None,
|
||||
approval_policy: None,
|
||||
approvals_reviewer: None,
|
||||
sandbox: None,
|
||||
@@ -651,6 +652,7 @@ mod thread_processor_behavior_tests {
|
||||
permission_profile: codex_protocol::models::PermissionProfile::Disabled,
|
||||
active_permission_profile: None,
|
||||
cwd,
|
||||
workspace_roots: Vec::new(),
|
||||
ephemeral: false,
|
||||
reasoning_effort: None,
|
||||
personality: None,
|
||||
|
||||
@@ -177,37 +177,38 @@ pub(super) fn thread_response_active_permission_profile(
|
||||
|
||||
pub(super) fn apply_permission_profile_selection_to_config_overrides(
|
||||
overrides: &mut ConfigOverrides,
|
||||
permissions: Option<PermissionProfileSelectionParams>,
|
||||
permissions: Option<String>,
|
||||
) {
|
||||
let Some(PermissionProfileSelectionParams::Profile { id, modifications }) = permissions else {
|
||||
let Some(id) = permissions else {
|
||||
return;
|
||||
};
|
||||
overrides.default_permissions = Some(id);
|
||||
overrides
|
||||
.additional_writable_roots
|
||||
.extend(modifications.unwrap_or_default().into_iter().map(
|
||||
|modification| match modification {
|
||||
PermissionProfileModificationParams::AdditionalWritableRoot { path } => {
|
||||
path.to_path_buf()
|
||||
}
|
||||
},
|
||||
));
|
||||
}
|
||||
|
||||
pub(super) fn thread_response_sandbox_policy(
|
||||
permission_profile: &codex_protocol::models::PermissionProfile,
|
||||
workspace_roots: &[AbsolutePathBuf],
|
||||
cwd: &Path,
|
||||
) -> codex_app_server_protocol::SandboxPolicy {
|
||||
let file_system_policy = permission_profile.file_system_sandbox_policy();
|
||||
let materialized_permission_profile =
|
||||
thread_response_permission_profile(permission_profile, workspace_roots);
|
||||
let file_system_policy = materialized_permission_profile.file_system_sandbox_policy();
|
||||
let sandbox_policy = codex_sandboxing::compatibility_sandbox_policy_for_permission_profile(
|
||||
permission_profile,
|
||||
&materialized_permission_profile,
|
||||
&file_system_policy,
|
||||
permission_profile.network_sandbox_policy(),
|
||||
materialized_permission_profile.network_sandbox_policy(),
|
||||
cwd,
|
||||
);
|
||||
sandbox_policy.into()
|
||||
}
|
||||
|
||||
pub(super) fn thread_response_permission_profile(
|
||||
permission_profile: &codex_protocol::models::PermissionProfile,
|
||||
workspace_roots: &[AbsolutePathBuf],
|
||||
) -> codex_protocol::models::PermissionProfile {
|
||||
permission_profile.materialize_project_roots_with_workspace_roots(workspace_roots)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
fn parse_datetime(timestamp: Option<&str>) -> Option<DateTime<Utc>> {
|
||||
timestamp.and_then(|ts| {
|
||||
|
||||
@@ -357,6 +357,7 @@ impl TurnRequestProcessor {
|
||||
let turn_has_input = !mapped_items.is_empty();
|
||||
|
||||
let has_any_overrides = params.cwd.is_some()
|
||||
|| params.workspace_roots.is_some()
|
||||
|| params.approval_policy.is_some()
|
||||
|| params.approvals_reviewer.is_some()
|
||||
|| params.sandbox_policy.is_some()
|
||||
@@ -375,6 +376,7 @@ impl TurnRequestProcessor {
|
||||
}
|
||||
|
||||
let cwd = params.cwd;
|
||||
let workspace_roots = params.workspace_roots;
|
||||
let approval_policy = params.approval_policy.map(AskForApproval::to_core);
|
||||
let approvals_reviewer = params
|
||||
.approvals_reviewer
|
||||
@@ -432,6 +434,9 @@ impl TurnRequestProcessor {
|
||||
thread
|
||||
.validate_turn_context_overrides(CodexThreadTurnContextOverrides {
|
||||
cwd: cwd.clone(),
|
||||
workspace_roots: workspace_roots
|
||||
.clone()
|
||||
.map(|roots| roots.into_iter().map(|root| root.to_path_buf()).collect()),
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox_policy: sandbox_policy.clone(),
|
||||
@@ -457,6 +462,7 @@ impl TurnRequestProcessor {
|
||||
final_output_json_schema: params.output_schema,
|
||||
responsesapi_client_metadata: params.responsesapi_client_metadata,
|
||||
cwd,
|
||||
workspace_roots,
|
||||
approval_policy,
|
||||
approvals_reviewer,
|
||||
sandbox_policy,
|
||||
|
||||
@@ -135,6 +135,7 @@ pub fn create_fake_rollout_with_source(
|
||||
forked_from_id: None,
|
||||
timestamp: meta_rfc3339.to_string(),
|
||||
cwd: PathBuf::from("/"),
|
||||
workspace_roots: Vec::new(),
|
||||
originator: "codex".to_string(),
|
||||
cli_version: "0.0.0".to_string(),
|
||||
source,
|
||||
@@ -219,6 +220,7 @@ pub fn create_fake_rollout_with_text_elements(
|
||||
forked_from_id: None,
|
||||
timestamp: meta_rfc3339.to_string(),
|
||||
cwd: PathBuf::from("/"),
|
||||
workspace_roots: Vec::new(),
|
||||
originator: "codex".to_string(),
|
||||
cli_version: "0.0.0".to_string(),
|
||||
source: SessionSource::Cli,
|
||||
|
||||
@@ -130,6 +130,7 @@ async fn get_conversation_summary_by_thread_id_reads_pathless_store_thread() ->
|
||||
dynamic_tools: Vec::new(),
|
||||
metadata: ThreadPersistenceMetadata {
|
||||
cwd: None,
|
||||
workspace_roots: Vec::new(),
|
||||
model_provider: "test-provider".to_string(),
|
||||
memory_mode: ThreadMemoryMode::Disabled,
|
||||
},
|
||||
|
||||
@@ -83,8 +83,7 @@ async fn process_spawn_returns_before_exit_and_emits_exit_notification() -> Resu
|
||||
.await?;
|
||||
assert_eq!(response.result, serde_json::json!({}));
|
||||
|
||||
wait_for_file(&probe_file).await?;
|
||||
assert_eq!(std::fs::read_to_string(&probe_file)?, "process");
|
||||
wait_for_file_contents(&probe_file, "process").await?;
|
||||
std::fs::write(&release_file, "release")?;
|
||||
|
||||
let exited = read_process_exited(&mut mcp).await?;
|
||||
@@ -239,9 +238,9 @@ async fn read_process_exited(mcp: &mut McpProcess) -> Result<ProcessExitedNotifi
|
||||
serde_json::from_value(params).context("deserialize process/exited notification")
|
||||
}
|
||||
|
||||
async fn wait_for_file(path: &Path) -> Result<()> {
|
||||
async fn wait_for_file_contents(path: &Path, expected: &str) -> Result<()> {
|
||||
timeout(DEFAULT_READ_TIMEOUT, async {
|
||||
while !path.exists() {
|
||||
while std::fs::read_to_string(path).ok().as_deref() != Some(expected) {
|
||||
sleep(Duration::from_millis(20)).await;
|
||||
}
|
||||
})
|
||||
|
||||
@@ -614,6 +614,7 @@ async fn skills_changed_notification_is_emitted_after_skill_change() -> Result<(
|
||||
model_provider: None,
|
||||
service_tier: None,
|
||||
cwd: None,
|
||||
workspace_roots: None,
|
||||
approval_policy: None,
|
||||
approvals_reviewer: None,
|
||||
sandbox: None,
|
||||
|
||||
@@ -29,6 +29,7 @@ use codex_app_server_protocol::TurnStatus;
|
||||
use codex_app_server_protocol::UserInput;
|
||||
use codex_config::types::AuthCredentialsStoreMode;
|
||||
use codex_login::REFRESH_TOKEN_URL_OVERRIDE_ENV_VAR;
|
||||
use codex_utils_absolute_path::AbsolutePathBuf;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use serde_json::json;
|
||||
@@ -216,6 +217,103 @@ async fn thread_fork_creates_new_thread_and_emits_started() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_fork_preserves_persisted_workspace_roots_when_request_omits_them() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
|
||||
let repo = TempDir::new()?;
|
||||
let shared = TempDir::new()?;
|
||||
let workspace_roots = vec![
|
||||
AbsolutePathBuf::try_from(repo.path())?,
|
||||
AbsolutePathBuf::try_from(shared.path())?,
|
||||
];
|
||||
|
||||
let mut mcp = McpProcess::new(codex_home.path()).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
|
||||
|
||||
let start_id = mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
cwd: Some(repo.path().display().to_string()),
|
||||
permissions: Some(":workspace".to_string()),
|
||||
workspace_roots: Some(workspace_roots.clone()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let start_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(start_id)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadStartResponse { thread, .. } = to_response::<ThreadStartResponse>(start_resp)?;
|
||||
let rollout_path = thread.path.clone().expect("thread path");
|
||||
|
||||
let turn_id = mcp
|
||||
.send_turn_start_request(TurnStartParams {
|
||||
thread_id: thread.id.clone(),
|
||||
input: vec![UserInput::Text {
|
||||
text: "seed history".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(turn_id)),
|
||||
)
|
||||
.await??;
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_notification_message("turn/completed"),
|
||||
)
|
||||
.await??;
|
||||
for _ in 0..50 {
|
||||
let contents = std::fs::read_to_string(rollout_path.as_path())?;
|
||||
if contents.contains("turn_context") {
|
||||
break;
|
||||
}
|
||||
tokio::time::sleep(std::time::Duration::from_millis(20)).await;
|
||||
}
|
||||
let contents = std::fs::read_to_string(rollout_path.as_path())?;
|
||||
if !contents.contains("turn_context") {
|
||||
anyhow::bail!(
|
||||
"turn context was not persisted to rollout {}",
|
||||
rollout_path.display()
|
||||
);
|
||||
}
|
||||
|
||||
let fork_id = mcp
|
||||
.send_thread_fork_request(ThreadForkParams {
|
||||
thread_id: thread.id,
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let fork_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(fork_id)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadForkResponse {
|
||||
workspace_roots: forked_roots,
|
||||
permission_profile,
|
||||
..
|
||||
} = to_response::<ThreadForkResponse>(fork_resp)?;
|
||||
|
||||
assert_eq!(forked_roots, workspace_roots);
|
||||
let permission_profile: codex_protocol::models::PermissionProfile = permission_profile
|
||||
.expect("fork response should include exact permissions")
|
||||
.into();
|
||||
assert!(
|
||||
permission_profile
|
||||
.file_system_sandbox_policy()
|
||||
.can_write_path_with_cwd(shared.path(), repo.path())
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_fork_can_load_source_by_path() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
|
||||
@@ -610,6 +610,7 @@ sqlite = true
|
||||
codex_home: codex_home.path().to_path_buf(),
|
||||
sqlite_home: codex_home.path().to_path_buf(),
|
||||
cwd: codex_home.path().to_path_buf(),
|
||||
workspace_roots: Vec::new(),
|
||||
model_provider_id: "mock_provider".to_string(),
|
||||
generate_memories: false,
|
||||
};
|
||||
|
||||
@@ -1291,6 +1291,7 @@ async fn seed_pathless_store_thread(
|
||||
dynamic_tools: Vec::new(),
|
||||
metadata: ThreadPersistenceMetadata {
|
||||
cwd: None,
|
||||
workspace_roots: Vec::new(),
|
||||
model_provider: "test-provider".to_string(),
|
||||
memory_mode: ThreadMemoryMode::Disabled,
|
||||
},
|
||||
|
||||
@@ -1474,6 +1474,7 @@ stream_max_retries = 0
|
||||
forked_from_id: None,
|
||||
timestamp: "2025-01-05T12:00:00Z".to_string(),
|
||||
cwd: repo_path.clone(),
|
||||
workspace_roots: Vec::new(),
|
||||
originator: "codex".to_string(),
|
||||
cli_version: "0.0.0".to_string(),
|
||||
source: RolloutSessionSource::Cli,
|
||||
@@ -2970,6 +2971,64 @@ async fn thread_resume_supports_history_and_overrides() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_resume_preserves_persisted_workspace_roots_when_request_omits_them() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
let codex_home = TempDir::new()?;
|
||||
create_config_toml(codex_home.path(), &server.uri())?;
|
||||
|
||||
let repo = TempDir::new()?;
|
||||
let shared = TempDir::new()?;
|
||||
let workspace_roots = vec![
|
||||
AbsolutePathBuf::try_from(repo.path())?,
|
||||
AbsolutePathBuf::try_from(shared.path())?,
|
||||
];
|
||||
|
||||
let RestartedThreadFixture {
|
||||
mut mcp, thread_id, ..
|
||||
} = start_materialized_thread_with_params_and_restart(
|
||||
codex_home.path(),
|
||||
"seed history",
|
||||
ThreadStartParams {
|
||||
model: Some("gpt-5.4".to_string()),
|
||||
cwd: Some(repo.path().display().to_string()),
|
||||
permissions: Some(":workspace".to_string()),
|
||||
workspace_roots: Some(workspace_roots.clone()),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
|
||||
let resume_id = mcp
|
||||
.send_thread_resume_request(ThreadResumeParams {
|
||||
thread_id,
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let resume_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(resume_id)),
|
||||
)
|
||||
.await??;
|
||||
let ThreadResumeResponse {
|
||||
workspace_roots: resumed_roots,
|
||||
permission_profile,
|
||||
..
|
||||
} = to_response::<ThreadResumeResponse>(resume_resp)?;
|
||||
|
||||
assert_eq!(resumed_roots, workspace_roots);
|
||||
let permission_profile: codex_protocol::models::PermissionProfile = permission_profile
|
||||
.expect("resume response should include exact permissions")
|
||||
.into();
|
||||
assert!(
|
||||
permission_profile
|
||||
.file_system_sandbox_policy()
|
||||
.can_write_path_with_cwd(shared.path(), repo.path())
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
struct RestartedThreadFixture {
|
||||
mcp: McpProcess,
|
||||
thread_id: String,
|
||||
@@ -2980,16 +3039,27 @@ struct RestartedThreadFixture {
|
||||
async fn start_materialized_thread_and_restart(
|
||||
codex_home: &Path,
|
||||
seed_text: &str,
|
||||
) -> Result<RestartedThreadFixture> {
|
||||
start_materialized_thread_with_params_and_restart(
|
||||
codex_home,
|
||||
seed_text,
|
||||
ThreadStartParams {
|
||||
model: Some("gpt-5.4".to_string()),
|
||||
..Default::default()
|
||||
},
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
async fn start_materialized_thread_with_params_and_restart(
|
||||
codex_home: &Path,
|
||||
seed_text: &str,
|
||||
start_params: ThreadStartParams,
|
||||
) -> Result<RestartedThreadFixture> {
|
||||
let mut first_mcp = McpProcess::new(codex_home).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, first_mcp.initialize()).await??;
|
||||
|
||||
let start_id = first_mcp
|
||||
.send_thread_start_request(ThreadStartParams {
|
||||
model: Some("gpt-5.4".to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
let start_id = first_mcp.send_thread_start_request(start_params).await?;
|
||||
let start_resp: JSONRPCResponse = timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
first_mcp.read_stream_until_response_message(RequestId::Integer(start_id)),
|
||||
@@ -3036,18 +3106,29 @@ async fn start_materialized_thread_and_restart(
|
||||
.path
|
||||
.ok_or_else(|| anyhow::anyhow!("thread path missing from thread/start response"))?;
|
||||
let updated_at = thread.updated_at;
|
||||
for _ in 0..50 {
|
||||
let contents = std::fs::read_to_string(rollout_file_path.as_path())?;
|
||||
if contents.contains("turn_context") {
|
||||
drop(first_mcp);
|
||||
|
||||
let mut second_mcp = McpProcess::new(codex_home).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, second_mcp.initialize()).await??;
|
||||
|
||||
return Ok(RestartedThreadFixture {
|
||||
mcp: second_mcp,
|
||||
thread_id,
|
||||
rollout_file_path: rollout_file_path.to_path_buf(),
|
||||
updated_at,
|
||||
});
|
||||
}
|
||||
tokio::time::sleep(Duration::from_millis(20)).await;
|
||||
}
|
||||
|
||||
drop(first_mcp);
|
||||
|
||||
let mut second_mcp = McpProcess::new(codex_home).await?;
|
||||
timeout(DEFAULT_READ_TIMEOUT, second_mcp.initialize()).await??;
|
||||
|
||||
Ok(RestartedThreadFixture {
|
||||
mcp: second_mcp,
|
||||
thread_id,
|
||||
rollout_file_path: rollout_file_path.to_path_buf(),
|
||||
updated_at,
|
||||
})
|
||||
anyhow::bail!(
|
||||
"turn context was not persisted to rollout {}",
|
||||
rollout_file_path.display()
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -44,6 +44,7 @@ use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use std::fs::FileTimes;
|
||||
use std::fs::OpenOptions;
|
||||
use std::future::Future;
|
||||
use std::path::Path;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
@@ -54,6 +55,28 @@ use uuid::Uuid;
|
||||
|
||||
const DEFAULT_READ_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30);
|
||||
|
||||
fn run_current_thread_test_with_stack<F>(name: &str, future: F) -> Result<()>
|
||||
where
|
||||
F: Future<Output = Result<()>> + Send + 'static,
|
||||
{
|
||||
const TEST_STACK_SIZE_BYTES: usize = 4 * 1024 * 1024;
|
||||
|
||||
let handle = std::thread::Builder::new()
|
||||
.name(name.to_string())
|
||||
.stack_size(TEST_STACK_SIZE_BYTES)
|
||||
.spawn(move || -> Result<()> {
|
||||
let runtime = tokio::runtime::Builder::new_current_thread()
|
||||
.enable_all()
|
||||
.build()?;
|
||||
runtime.block_on(Box::pin(future))
|
||||
})?;
|
||||
|
||||
match handle.join() {
|
||||
Ok(result) => result,
|
||||
Err(_) => Err(anyhow::anyhow!("{name} thread panicked")),
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_unarchive_moves_rollout_back_into_sessions_directory() -> Result<()> {
|
||||
let server = create_mock_responses_server_repeating_assistant("Done").await;
|
||||
@@ -197,97 +220,103 @@ async fn thread_unarchive_moves_rollout_back_into_sessions_directory() -> Result
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn thread_unarchive_preserves_pathless_store_metadata() -> Result<()> {
|
||||
let codex_home = TempDir::new()?;
|
||||
let store_id = Uuid::new_v4().to_string();
|
||||
create_config_toml_with_in_memory_thread_store(codex_home.path(), &store_id)?;
|
||||
let store = InMemoryThreadStore::for_id(store_id.clone());
|
||||
let _in_memory_store = InMemoryThreadStoreId { store_id };
|
||||
let thread_id = ThreadId::from_string("00000000-0000-4000-8000-000000000126")?;
|
||||
let parent_thread_id = ThreadId::from_string("00000000-0000-4000-8000-000000000127")?;
|
||||
store
|
||||
.create_thread(CreateThreadParams {
|
||||
thread_id,
|
||||
forked_from_id: Some(parent_thread_id),
|
||||
source: SessionSource::Cli,
|
||||
thread_source: None,
|
||||
base_instructions: BaseInstructions::default(),
|
||||
dynamic_tools: Vec::new(),
|
||||
metadata: ThreadPersistenceMetadata {
|
||||
cwd: None,
|
||||
model_provider: "test-provider".to_string(),
|
||||
memory_mode: ThreadMemoryMode::Disabled,
|
||||
},
|
||||
event_persistence_mode: ThreadEventPersistenceMode::default(),
|
||||
})
|
||||
.await?;
|
||||
store
|
||||
.update_thread_metadata(UpdateThreadMetadataParams {
|
||||
thread_id,
|
||||
patch: ThreadMetadataPatch {
|
||||
name: Some(Some("named pathless thread".to_string())),
|
||||
..Default::default()
|
||||
},
|
||||
include_archived: true,
|
||||
})
|
||||
.await?;
|
||||
#[test]
|
||||
fn thread_unarchive_preserves_pathless_store_metadata() -> Result<()> {
|
||||
run_current_thread_test_with_stack(
|
||||
"thread_unarchive_preserves_pathless_store_metadata",
|
||||
async {
|
||||
let codex_home = TempDir::new()?;
|
||||
let store_id = Uuid::new_v4().to_string();
|
||||
create_config_toml_with_in_memory_thread_store(codex_home.path(), &store_id)?;
|
||||
let store = InMemoryThreadStore::for_id(store_id.clone());
|
||||
let _in_memory_store = InMemoryThreadStoreId { store_id };
|
||||
let thread_id = ThreadId::from_string("00000000-0000-4000-8000-000000000126")?;
|
||||
let parent_thread_id = ThreadId::from_string("00000000-0000-4000-8000-000000000127")?;
|
||||
store
|
||||
.create_thread(CreateThreadParams {
|
||||
thread_id,
|
||||
forked_from_id: Some(parent_thread_id),
|
||||
source: SessionSource::Cli,
|
||||
thread_source: None,
|
||||
base_instructions: BaseInstructions::default(),
|
||||
dynamic_tools: Vec::new(),
|
||||
metadata: ThreadPersistenceMetadata {
|
||||
cwd: None,
|
||||
workspace_roots: Vec::new(),
|
||||
model_provider: "test-provider".to_string(),
|
||||
memory_mode: ThreadMemoryMode::Disabled,
|
||||
},
|
||||
event_persistence_mode: ThreadEventPersistenceMode::default(),
|
||||
})
|
||||
.await?;
|
||||
store
|
||||
.update_thread_metadata(UpdateThreadMetadataParams {
|
||||
thread_id,
|
||||
patch: ThreadMetadataPatch {
|
||||
name: Some(Some("named pathless thread".to_string())),
|
||||
..Default::default()
|
||||
},
|
||||
include_archived: true,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let loader_overrides = LoaderOverrides::without_managed_config_for_tests();
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.loader_overrides(loader_overrides.clone())
|
||||
.build()
|
||||
.await?;
|
||||
let client = in_process::start(InProcessStartArgs {
|
||||
arg0_paths: Arg0DispatchPaths::default(),
|
||||
config: Arc::new(config),
|
||||
cli_overrides: Vec::new(),
|
||||
loader_overrides,
|
||||
strict_config: false,
|
||||
cloud_requirements: CloudRequirementsLoader::default(),
|
||||
thread_config_loader: Arc::new(codex_config::NoopThreadConfigLoader),
|
||||
feedback: CodexFeedback::new(),
|
||||
log_db: None,
|
||||
state_db: None,
|
||||
environment_manager: Arc::new(EnvironmentManager::default_for_tests()),
|
||||
config_warnings: Vec::new(),
|
||||
session_source: SessionSource::Cli,
|
||||
enable_codex_api_key_env: false,
|
||||
initialize: InitializeParams {
|
||||
client_info: ClientInfo {
|
||||
name: "codex-app-server-tests".to_string(),
|
||||
title: None,
|
||||
version: "0.1.0".to_string(),
|
||||
},
|
||||
capabilities: Some(InitializeCapabilities {
|
||||
experimental_api: true,
|
||||
..Default::default()
|
||||
}),
|
||||
let loader_overrides = LoaderOverrides::without_managed_config_for_tests();
|
||||
let config = ConfigBuilder::default()
|
||||
.codex_home(codex_home.path().to_path_buf())
|
||||
.fallback_cwd(Some(codex_home.path().to_path_buf()))
|
||||
.loader_overrides(loader_overrides.clone())
|
||||
.build()
|
||||
.await?;
|
||||
let client = in_process::start(InProcessStartArgs {
|
||||
arg0_paths: Arg0DispatchPaths::default(),
|
||||
config: Arc::new(config),
|
||||
cli_overrides: Vec::new(),
|
||||
loader_overrides,
|
||||
strict_config: false,
|
||||
cloud_requirements: CloudRequirementsLoader::default(),
|
||||
thread_config_loader: Arc::new(codex_config::NoopThreadConfigLoader),
|
||||
feedback: CodexFeedback::new(),
|
||||
log_db: None,
|
||||
state_db: None,
|
||||
environment_manager: Arc::new(EnvironmentManager::default_for_tests()),
|
||||
config_warnings: Vec::new(),
|
||||
session_source: SessionSource::Cli,
|
||||
enable_codex_api_key_env: false,
|
||||
initialize: InitializeParams {
|
||||
client_info: ClientInfo {
|
||||
name: "codex-app-server-tests".to_string(),
|
||||
title: None,
|
||||
version: "0.1.0".to_string(),
|
||||
},
|
||||
capabilities: Some(InitializeCapabilities {
|
||||
experimental_api: true,
|
||||
..Default::default()
|
||||
}),
|
||||
},
|
||||
channel_capacity: in_process::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let result = client
|
||||
.request(ClientRequest::ThreadUnarchive {
|
||||
request_id: RequestId::Integer(1),
|
||||
params: ThreadUnarchiveParams {
|
||||
thread_id: thread_id.to_string(),
|
||||
},
|
||||
})
|
||||
.await?
|
||||
.expect("thread/unarchive should succeed");
|
||||
let ThreadUnarchiveResponse { thread } = serde_json::from_value(result)?;
|
||||
|
||||
assert_eq!(thread.id, thread_id.to_string());
|
||||
assert_eq!(thread.path, None);
|
||||
assert_eq!(thread.forked_from_id, Some(parent_thread_id.to_string()));
|
||||
assert_eq!(thread.name, Some("named pathless thread".to_string()));
|
||||
|
||||
client.shutdown().await?;
|
||||
Ok(())
|
||||
},
|
||||
channel_capacity: in_process::DEFAULT_IN_PROCESS_CHANNEL_CAPACITY,
|
||||
})
|
||||
.await?;
|
||||
|
||||
let result = client
|
||||
.request(ClientRequest::ThreadUnarchive {
|
||||
request_id: RequestId::Integer(1),
|
||||
params: ThreadUnarchiveParams {
|
||||
thread_id: thread_id.to_string(),
|
||||
},
|
||||
})
|
||||
.await?
|
||||
.expect("thread/unarchive should succeed");
|
||||
let ThreadUnarchiveResponse { thread } = serde_json::from_value(result)?;
|
||||
|
||||
assert_eq!(thread.id, thread_id.to_string());
|
||||
assert_eq!(thread.path, None);
|
||||
assert_eq!(thread.forked_from_id, Some(parent_thread_id.to_string()));
|
||||
assert_eq!(thread.name, Some("named pathless thread".to_string()));
|
||||
|
||||
client.shutdown().await?;
|
||||
Ok(())
|
||||
)
|
||||
}
|
||||
|
||||
fn create_config_toml(codex_home: &Path, server_uri: &str) -> std::io::Result<()> {
|
||||
|
||||
@@ -34,7 +34,6 @@ use codex_app_server_protocol::JSONRPCNotification;
|
||||
use codex_app_server_protocol::JSONRPCResponse;
|
||||
use codex_app_server_protocol::PatchApplyStatus;
|
||||
use codex_app_server_protocol::PatchChangeKind;
|
||||
use codex_app_server_protocol::PermissionProfileSelectionParams;
|
||||
use codex_app_server_protocol::RequestId;
|
||||
use codex_app_server_protocol::ServerRequest;
|
||||
use codex_app_server_protocol::ServerRequestResolvedNotification;
|
||||
@@ -329,9 +328,27 @@ async fn turn_start_emits_thread_scoped_warning_notification_for_trimmed_skills(
|
||||
let warning: WarningNotification =
|
||||
serde_json::from_value(params).expect("deserialize warning notification");
|
||||
assert_eq!(warning.thread_id.as_deref(), Some(thread.id.as_str()));
|
||||
assert_eq!(
|
||||
warning.message,
|
||||
"Exceeded skills context budget of 2%. All skill descriptions were removed and 7 additional skills were not included in the model-visible skills list."
|
||||
const WARNING_PREFIX: &str =
|
||||
"Exceeded skills context budget of 2%. All skill descriptions were removed and ";
|
||||
const WARNING_SUFFIX: &str =
|
||||
" additional skills were not included in the model-visible skills list.";
|
||||
assert!(
|
||||
warning.message.starts_with(WARNING_PREFIX),
|
||||
"unexpected warning prefix: {}",
|
||||
warning.message
|
||||
);
|
||||
assert!(
|
||||
warning.message.ends_with(WARNING_SUFFIX),
|
||||
"unexpected warning suffix: {}",
|
||||
warning.message
|
||||
);
|
||||
let trimmed_count = warning.message
|
||||
[WARNING_PREFIX.len()..warning.message.len() - WARNING_SUFFIX.len()]
|
||||
.parse::<usize>()
|
||||
.expect("warning should include a numeric trimmed skills count");
|
||||
assert!(
|
||||
trimmed_count > 0,
|
||||
"warning should report at least one trimmed skill"
|
||||
);
|
||||
|
||||
timeout(
|
||||
@@ -779,10 +796,7 @@ async fn turn_start_rejects_invalid_permission_selection_before_starting_turn()
|
||||
text: "Hello".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
permissions: Some(PermissionProfileSelectionParams::Profile {
|
||||
id: ":danger-no-sandbox".to_string(),
|
||||
modifications: None,
|
||||
}),
|
||||
permissions: Some(":danger-no-sandbox".to_string()),
|
||||
..Default::default()
|
||||
})
|
||||
.await?;
|
||||
@@ -1890,13 +1904,14 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> {
|
||||
}],
|
||||
responsesapi_client_metadata: None,
|
||||
cwd: Some(first_cwd.clone()),
|
||||
workspace_roots: Some(vec![first_cwd.try_into()?]),
|
||||
approval_policy: Some(codex_app_server_protocol::AskForApproval::Never),
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![first_cwd.try_into()?],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
legacy_writable_roots: Vec::new(),
|
||||
}),
|
||||
permissions: None,
|
||||
model: Some("mock-model".to_string()),
|
||||
@@ -1931,6 +1946,7 @@ async fn turn_start_updates_sandbox_and_cwd_between_turns_v2() -> Result<()> {
|
||||
}],
|
||||
responsesapi_client_metadata: None,
|
||||
cwd: Some(second_cwd.clone()),
|
||||
workspace_roots: None,
|
||||
approval_policy: Some(codex_app_server_protocol::AskForApproval::Never),
|
||||
approvals_reviewer: None,
|
||||
sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::DangerFullAccess),
|
||||
|
||||
@@ -533,12 +533,13 @@ async fn turn_start_shell_zsh_fork_subcommand_decline_marks_parent_declined_v2()
|
||||
text_elements: Vec::new(),
|
||||
}],
|
||||
cwd: Some(workspace.clone()),
|
||||
workspace_roots: Some(vec![workspace.clone().try_into()?]),
|
||||
approval_policy: Some(codex_app_server_protocol::AskForApproval::UnlessTrusted),
|
||||
sandbox_policy: Some(codex_app_server_protocol::SandboxPolicy::WorkspaceWrite {
|
||||
writable_roots: vec![workspace.clone().try_into()?],
|
||||
network_access: false,
|
||||
exclude_tmpdir_env_var: false,
|
||||
exclude_slash_tmp: false,
|
||||
legacy_writable_roots: Vec::new(),
|
||||
}),
|
||||
model: Some("mock-model".to_string()),
|
||||
effort: Some(codex_protocol::openai_models::ReasoningEffort::Medium),
|
||||
|
||||
Reference in New Issue
Block a user