mirror of
https://github.com/openai/codex.git
synced 2026-11-02 19:43:24 +00:00
Merge remote-tracking branch 'origin/main' into dev/python-sdk-goal-turns
# Conflicts: # codex-rs/deny.toml
This commit is contained in:
Generated
+50
-36
File diff suppressed because one or more lines are too long
@@ -1,9 +1,10 @@
|
||||
[advisories]
|
||||
# Reviewed 2026-04-15. Keep this list in sync with ../deny.toml.
|
||||
# Reviewed 2026-06-07. Keep this list in sync with ../deny.toml.
|
||||
ignore = [
|
||||
"RUSTSEC-2024-0388", # derivative 2.2.0 via starlark; upstream crate is unmaintained
|
||||
"RUSTSEC-2025-0057", # fxhash 0.2.1 via starlark_map; upstream crate is unmaintained
|
||||
"RUSTSEC-2024-0388", # derivative 2.2.0 via starlark/starlark_syntax; upstream crate is unmaintained
|
||||
"RUSTSEC-2025-0057", # fxhash 0.2.1 via starlark_map/bm25; upstream crate is unmaintained
|
||||
"RUSTSEC-2024-0436", # paste 1.0.15 via starlark/ratatui; upstream crate is unmaintained
|
||||
"RUSTSEC-2023-0089", # atomic-polyfill via postcard/heapless/pagable; upstream crate is unmaintained
|
||||
"RUSTSEC-2024-0320", # yaml-rust via syntect; remove when syntect drops or updates it
|
||||
"RUSTSEC-2025-0141", # bincode via syntect; remove when syntect drops or updates it
|
||||
"RUSTSEC-2026-0118", # hickory-proto via rama-dns/rama-tcp; remove when rama updates to hickory 0.26.1 or hickory-net
|
||||
|
||||
Generated
+527
-352
File diff suppressed because it is too large
Load Diff
+1
-1
@@ -384,7 +384,7 @@ sqlx = { version = "0.9.0", default-features = false, features = [
|
||||
"time",
|
||||
"uuid",
|
||||
] }
|
||||
starlark = "0.13.0"
|
||||
starlark = { version = "0.14.2", default-features = false }
|
||||
strum = "0.27.2"
|
||||
strum_macros = "0.28.0"
|
||||
supports-color = "3.0.2"
|
||||
|
||||
@@ -2234,6 +2234,12 @@
|
||||
},
|
||||
"status": {
|
||||
"$ref": "#/definitions/McpServerStartupState"
|
||||
},
|
||||
"threadId": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
+6
@@ -11462,6 +11462,12 @@
|
||||
},
|
||||
"status": {
|
||||
"$ref": "#/definitions/v2/McpServerStartupState"
|
||||
},
|
||||
"threadId": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
+6
@@ -7964,6 +7964,12 @@
|
||||
},
|
||||
"status": {
|
||||
"$ref": "#/definitions/McpServerStartupState"
|
||||
},
|
||||
"threadId": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
+6
@@ -23,6 +23,12 @@
|
||||
},
|
||||
"status": {
|
||||
"$ref": "#/definitions/McpServerStartupState"
|
||||
},
|
||||
"threadId": {
|
||||
"type": [
|
||||
"string",
|
||||
"null"
|
||||
]
|
||||
}
|
||||
},
|
||||
"required": [
|
||||
|
||||
+1
-1
@@ -3,4 +3,4 @@
|
||||
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
|
||||
import type { McpServerStartupState } from "./McpServerStartupState";
|
||||
|
||||
export type McpServerStatusUpdatedNotification = { name: string, status: McpServerStartupState, error: string | null, };
|
||||
export type McpServerStatusUpdatedNotification = { threadId: string | null, name: string, status: McpServerStartupState, error: string | null, };
|
||||
|
||||
@@ -235,6 +235,7 @@ pub enum McpServerStartupState {
|
||||
#[serde(rename_all = "camelCase")]
|
||||
#[ts(export_to = "v2/")]
|
||||
pub struct McpServerStatusUpdatedNotification {
|
||||
pub thread_id: Option<String>,
|
||||
pub name: String,
|
||||
pub status: McpServerStartupState,
|
||||
pub error: Option<String>,
|
||||
|
||||
@@ -2040,6 +2040,33 @@ fn mcp_server_status_serializes_absent_server_info_as_null() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_server_status_updated_accepts_missing_thread_id() {
|
||||
let notification: McpServerStatusUpdatedNotification = serde_json::from_value(json!({
|
||||
"name": "optional_broken",
|
||||
"status": "failed",
|
||||
"error": "handshake failed",
|
||||
}))
|
||||
.expect("notification without threadId should deserialize");
|
||||
|
||||
let expected = McpServerStatusUpdatedNotification {
|
||||
thread_id: None,
|
||||
name: "optional_broken".to_string(),
|
||||
status: McpServerStartupState::Failed,
|
||||
error: Some("handshake failed".to_string()),
|
||||
};
|
||||
assert_eq!(notification, expected);
|
||||
assert_eq!(
|
||||
serde_json::to_value(notification).expect("notification should serialize"),
|
||||
json!({
|
||||
"threadId": null,
|
||||
"name": "optional_broken",
|
||||
"status": "failed",
|
||||
"error": "handshake failed",
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_server_status_serializes_absent_server_info_metadata_as_null() {
|
||||
let response = ListMcpServerStatusResponse {
|
||||
|
||||
@@ -1235,7 +1235,7 @@ Because audio is intentionally separate from `ThreadItem`, clients can opt out o
|
||||
|
||||
### MCP server startup events
|
||||
|
||||
- `mcpServer/startupStatus/updated` — `{ name, status, error }` when app-server observes an MCP server startup transition. `status` is one of `starting`, `ready`, `failed`, or `cancelled`. `error` is `null` except for `failed`.
|
||||
- `mcpServer/startupStatus/updated` — `{ threadId, name, status, error }` when app-server observes an MCP server startup transition. `threadId` identifies the owning thread when startup is thread-scoped and is `null` when startup is app-scoped. `status` is one of `starting`, `ready`, `failed`, or `cancelled`. `error` is `null` except for `failed`.
|
||||
|
||||
### Turn events
|
||||
|
||||
@@ -1787,7 +1787,7 @@ Codex supports these authentication modes. The current mode is surfaced in `acco
|
||||
- `account/rateLimits/updated` (notify) — emitted whenever a user's ChatGPT rate limits change. This is a sparse rolling update; merge available values into the most recent `account/rateLimits/read` response or refetch that snapshot.
|
||||
- `account/sendAddCreditsNudgeEmail` — ask ChatGPT to email the workspace owner about depleted credits or a reached usage limit.
|
||||
- `mcpServer/oauthLogin/completed` (notify) — emitted after a `mcpServer/oauth/login` flow finishes for a server; payload includes `{ name, success, error? }`.
|
||||
- `mcpServer/startupStatus/updated` (notify) — emitted when a configured MCP server's startup status changes for a loaded thread; payload includes `{ name, status, error }` where `status` is `starting`, `ready`, `failed`, or `cancelled`.
|
||||
- `mcpServer/startupStatus/updated` (notify) — emitted when a configured MCP server's startup status changes; payload includes `{ threadId, name, status, error }`, where `threadId` is the owning thread when startup is thread-scoped and `null` when it is app-scoped, and `status` is `starting`, `ready`, `failed`, or `cancelled`.
|
||||
|
||||
### 1) Check auth state
|
||||
|
||||
|
||||
@@ -212,6 +212,7 @@ pub(crate) async fn apply_bespoke_event_handling(
|
||||
}
|
||||
};
|
||||
let notification = McpServerStatusUpdatedNotification {
|
||||
thread_id: Some(conversation_id.to_string()),
|
||||
name: update.server,
|
||||
status,
|
||||
error,
|
||||
|
||||
@@ -708,7 +708,7 @@ async fn thread_start_emits_mcp_server_status_updated_notifications() -> Result<
|
||||
.send_thread_start_request(ThreadStartParams::default())
|
||||
.await?;
|
||||
|
||||
let _: ThreadStartResponse = to_response(
|
||||
let start_response: ThreadStartResponse = to_response(
|
||||
timeout(
|
||||
DEFAULT_READ_TIMEOUT,
|
||||
mcp.read_stream_until_response_message(RequestId::Integer(req_id)),
|
||||
@@ -745,6 +745,7 @@ async fn thread_start_emits_mcp_server_status_updated_notifications() -> Result<
|
||||
assert_eq!(
|
||||
starting,
|
||||
McpServerStatusUpdatedNotification {
|
||||
thread_id: Some(start_response.thread.id.clone()),
|
||||
name: "optional_broken".to_string(),
|
||||
status: McpServerStartupState::Starting,
|
||||
error: None,
|
||||
@@ -777,6 +778,7 @@ async fn thread_start_emits_mcp_server_status_updated_notifications() -> Result<
|
||||
let ServerNotification::McpServerStatusUpdated(failed) = failed else {
|
||||
anyhow::bail!("unexpected notification variant");
|
||||
};
|
||||
assert_eq!(failed.thread_id, Some(start_response.thread.id));
|
||||
assert_eq!(failed.name, "optional_broken");
|
||||
assert_eq!(failed.status, McpServerStartupState::Failed);
|
||||
assert!(
|
||||
|
||||
+4
-3
@@ -70,12 +70,13 @@ feature-depth = 1
|
||||
# A list of advisory IDs to ignore. Note that ignored advisories will still
|
||||
# output a note when they are encountered.
|
||||
ignore = [
|
||||
# Reviewed 2026-04-15. Keep this list in sync with .cargo/audit.toml.
|
||||
# Reviewed 2026-06-07. Keep this list in sync with .cargo/audit.toml.
|
||||
# Each exception must identify the dependency path and removal condition.
|
||||
{ id = "RUSTSEC-2024-0388", reason = "derivative is unmaintained; pulled in via starlark v0.13.0 used by execpolicy/cli/core; no fixed release yet" },
|
||||
{ id = "RUSTSEC-2025-0057", reason = "fxhash is unmaintained; pulled in via starlark_map/starlark v0.13.0 used by execpolicy/cli/core; no fixed release yet" },
|
||||
{ id = "RUSTSEC-2024-0388", reason = "derivative is unmaintained; pulled in via starlark/starlark_syntax v0.14.2 used by execpolicy/cli/core; no fixed starlark release yet" },
|
||||
{ id = "RUSTSEC-2025-0057", reason = "fxhash is unmaintained; pulled in via starlark_map under starlark v0.14.2 used by execpolicy/cli/core and bm25 used by core; remove when upstream dependencies drop fxhash" },
|
||||
{ id = "RUSTSEC-2024-0436", reason = "paste is unmaintained; pulled in via ratatui/rmcp/starlark used by tui/execpolicy; no fixed release yet" },
|
||||
{ id = "RUSTSEC-2026-0173", reason = "proc-macro-error2 is unmaintained; pulled in via i18n-embed-fl v0.9.4 and age v0.11.2 used by codex-secrets; age v0.11.3 still depends on it and no fixed stable release exists" },
|
||||
{ id = "RUSTSEC-2023-0089", reason = "atomic-polyfill is unmaintained; pulled in via postcard/heapless/pagable under starlark v0.14.2 used by execpolicy/cli/core; no fixed starlark release yet" },
|
||||
# TODO(fcoury): remove this exception when syntect drops yaml-rust and bincode, or updates to versions that have fixed the vulnerabilities.
|
||||
{ id = "RUSTSEC-2024-0320", reason = "yaml-rust is unmaintained; pulled in via syntect v5.3.0 used by codex-tui for syntax highlighting; no fixed release yet" },
|
||||
{ id = "RUSTSEC-2025-0141", reason = "bincode is unmaintained; pulled in via syntect v5.3.0 used by codex-tui for syntax highlighting; no fixed release yet" },
|
||||
|
||||
@@ -95,7 +95,7 @@ impl ArgMatcherCardinality {
|
||||
}
|
||||
|
||||
impl<'v> AllocValue<'v> for ArgMatcher {
|
||||
fn alloc_value(self, heap: &'v Heap) -> Value<'v> {
|
||||
fn alloc_value(self, heap: Heap<'v>) -> Value<'v> {
|
||||
heap.alloc_simple(self)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -66,7 +66,7 @@ impl<'v> UnpackValue<'v> for Opt {
|
||||
}
|
||||
|
||||
impl<'v> AllocValue<'v> for Opt {
|
||||
fn alloc_value(self, heap: &'v Heap) -> Value<'v> {
|
||||
fn alloc_value(self, heap: Heap<'v>) -> Value<'v> {
|
||||
heap.alloc_simple(self)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -15,7 +15,6 @@ use starlark::environment::Module;
|
||||
use starlark::eval::Evaluator;
|
||||
use starlark::syntax::AstModule;
|
||||
use starlark::syntax::Dialect;
|
||||
use starlark::values::Heap;
|
||||
use starlark::values::list::UnpackList;
|
||||
use starlark::values::none::NoneType;
|
||||
use std::cell::RefCell;
|
||||
@@ -41,31 +40,28 @@ impl PolicyParser {
|
||||
let globals = GlobalsBuilder::extended_by(&[LibraryExtension::Typing])
|
||||
.with(policy_builtins)
|
||||
.build();
|
||||
let module = Module::new();
|
||||
|
||||
let heap = Heap::new();
|
||||
|
||||
module.set("ARG_OPAQUE_VALUE", heap.alloc(ArgMatcher::OpaqueNonFile));
|
||||
module.set("ARG_RFILE", heap.alloc(ArgMatcher::ReadableFile));
|
||||
module.set("ARG_WFILE", heap.alloc(ArgMatcher::WriteableFile));
|
||||
module.set("ARG_RFILES", heap.alloc(ArgMatcher::ReadableFiles));
|
||||
module.set(
|
||||
"ARG_RFILES_OR_CWD",
|
||||
heap.alloc(ArgMatcher::ReadableFilesOrCwd),
|
||||
);
|
||||
module.set("ARG_POS_INT", heap.alloc(ArgMatcher::PositiveInteger));
|
||||
module.set("ARG_SED_COMMAND", heap.alloc(ArgMatcher::SedCommand));
|
||||
module.set(
|
||||
"ARG_UNVERIFIED_VARARGS",
|
||||
heap.alloc(ArgMatcher::UnverifiedVarargs),
|
||||
);
|
||||
|
||||
let policy_builder = PolicyBuilder::new();
|
||||
{
|
||||
Module::with_temp_heap(|module| {
|
||||
let heap = module.heap();
|
||||
module.set("ARG_OPAQUE_VALUE", heap.alloc(ArgMatcher::OpaqueNonFile));
|
||||
module.set("ARG_RFILE", heap.alloc(ArgMatcher::ReadableFile));
|
||||
module.set("ARG_WFILE", heap.alloc(ArgMatcher::WriteableFile));
|
||||
module.set("ARG_RFILES", heap.alloc(ArgMatcher::ReadableFiles));
|
||||
module.set(
|
||||
"ARG_RFILES_OR_CWD",
|
||||
heap.alloc(ArgMatcher::ReadableFilesOrCwd),
|
||||
);
|
||||
module.set("ARG_POS_INT", heap.alloc(ArgMatcher::PositiveInteger));
|
||||
module.set("ARG_SED_COMMAND", heap.alloc(ArgMatcher::SedCommand));
|
||||
module.set(
|
||||
"ARG_UNVERIFIED_VARARGS",
|
||||
heap.alloc(ArgMatcher::UnverifiedVarargs),
|
||||
);
|
||||
|
||||
let mut eval = Evaluator::new(&module);
|
||||
eval.extra = Some(&policy_builder);
|
||||
eval.eval_module(ast, &globals)?;
|
||||
}
|
||||
eval.eval_module(ast, &globals).map(|_| ())
|
||||
})?;
|
||||
let policy = policy_builder.build();
|
||||
policy.map_err(|e| starlark::Error::new_kind(starlark::ErrorKind::Other(e.into())))
|
||||
}
|
||||
|
||||
@@ -65,12 +65,13 @@ impl PolicyParser {
|
||||
)
|
||||
.map_err(Error::Starlark)?;
|
||||
let globals = GlobalsBuilder::standard().with(policy_builtins).build();
|
||||
let module = Module::new();
|
||||
{
|
||||
Module::with_temp_heap(|module| {
|
||||
let mut eval = Evaluator::new(&module);
|
||||
eval.extra = Some(&self.builder);
|
||||
eval.eval_module(ast, &globals).map_err(Error::Starlark)?;
|
||||
}
|
||||
eval.eval_module(ast, &globals)
|
||||
.map(|_| ())
|
||||
.map_err(Error::Starlark)
|
||||
})?;
|
||||
self.builder
|
||||
.borrow()
|
||||
.validate_pending_examples_from(pending_validation_count)?;
|
||||
|
||||
@@ -35,6 +35,7 @@ pub(super) fn server_request_thread_id(request: &ServerRequest) -> Option<Thread
|
||||
pub(super) enum ServerNotificationThreadTarget {
|
||||
Thread(ThreadId),
|
||||
InvalidThreadId(String),
|
||||
AppScoped,
|
||||
Global,
|
||||
}
|
||||
|
||||
@@ -147,8 +148,13 @@ pub(super) fn server_notification_thread_target(
|
||||
}
|
||||
ServerNotification::Warning(notification) => notification.thread_id.as_deref(),
|
||||
ServerNotification::GuardianWarning(notification) => Some(notification.thread_id.as_str()),
|
||||
ServerNotification::McpServerStatusUpdated(notification) => {
|
||||
match notification.thread_id.as_deref() {
|
||||
Some(thread_id) => Some(thread_id),
|
||||
None => return ServerNotificationThreadTarget::AppScoped,
|
||||
}
|
||||
}
|
||||
ServerNotification::SkillsChanged(_)
|
||||
| ServerNotification::McpServerStatusUpdated(_)
|
||||
| ServerNotification::McpServerOauthLoginCompleted(_)
|
||||
| ServerNotification::AccountUpdated(_)
|
||||
| ServerNotification::AccountRateLimitsUpdated(_)
|
||||
@@ -184,6 +190,8 @@ mod tests {
|
||||
use crate::test_support::PathBufExt;
|
||||
use crate::test_support::test_path_buf;
|
||||
use codex_app_server_protocol::GuardianWarningNotification;
|
||||
use codex_app_server_protocol::McpServerStartupState;
|
||||
use codex_app_server_protocol::McpServerStatusUpdatedNotification;
|
||||
use codex_app_server_protocol::ServerNotification;
|
||||
use codex_app_server_protocol::ThreadSettings;
|
||||
use codex_app_server_protocol::ThreadSettingsUpdatedNotification;
|
||||
@@ -259,6 +267,37 @@ mod tests {
|
||||
assert_eq!(target, ServerNotificationThreadTarget::Thread(thread_id));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_startup_notifications_route_to_threads() {
|
||||
let thread_id = ThreadId::new();
|
||||
let notification =
|
||||
ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification {
|
||||
thread_id: Some(thread_id.to_string()),
|
||||
name: "sentry".to_string(),
|
||||
status: McpServerStartupState::Failed,
|
||||
error: Some("sentry is not logged in".to_string()),
|
||||
});
|
||||
|
||||
let target = server_notification_thread_target(¬ification);
|
||||
|
||||
assert_eq!(target, ServerNotificationThreadTarget::Thread(thread_id));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn mcp_startup_notifications_without_threads_are_app_scoped() {
|
||||
let notification =
|
||||
ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification {
|
||||
thread_id: None,
|
||||
name: "sentry".to_string(),
|
||||
status: McpServerStartupState::Failed,
|
||||
error: Some("sentry is not logged in".to_string()),
|
||||
});
|
||||
|
||||
let target = server_notification_thread_target(¬ification);
|
||||
|
||||
assert_eq!(target, ServerNotificationThreadTarget::AppScoped);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thread_settings_updated_notifications_route_to_threads() {
|
||||
let thread_id = ThreadId::new();
|
||||
|
||||
@@ -15,10 +15,9 @@ use codex_app_server_protocol::ServerNotification;
|
||||
use codex_app_server_protocol::ServerRequest;
|
||||
|
||||
impl App {
|
||||
fn refresh_mcp_startup_expected_servers_from_config(&mut self) {
|
||||
pub(super) fn refresh_mcp_startup_expected_servers_from_config(&mut self) {
|
||||
let enabled_config_mcp_servers: Vec<String> = self
|
||||
.chat_widget
|
||||
.config_ref()
|
||||
.config
|
||||
.mcp_servers
|
||||
.get()
|
||||
.iter()
|
||||
@@ -141,6 +140,12 @@ impl App {
|
||||
);
|
||||
return;
|
||||
}
|
||||
ServerNotificationThreadTarget::AppScoped => {
|
||||
tracing::debug!(
|
||||
"ignoring app-scoped MCP startup notification without a TUI app-level target"
|
||||
);
|
||||
return;
|
||||
}
|
||||
ServerNotificationThreadTarget::Global => {}
|
||||
}
|
||||
|
||||
|
||||
@@ -3353,24 +3353,33 @@ async fn side_thread_snapshot_skips_session_header_preamble() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn side_thread_ignores_global_mcp_startup_notifications() {
|
||||
async fn primary_thread_ignores_child_mcp_startup_notifications() {
|
||||
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
while app_event_rx.try_recv().is_ok() {}
|
||||
let sentry_config = toml::from_str::<toml::Value>("command = 'true'")
|
||||
.expect("test MCP config should parse")
|
||||
.try_into()
|
||||
.expect("test MCP config should deserialize");
|
||||
app.config
|
||||
.mcp_servers
|
||||
.set(std::collections::HashMap::from([(
|
||||
"sentry".to_string(),
|
||||
sentry_config,
|
||||
)]))
|
||||
.expect("test MCP servers should accept any configuration");
|
||||
let app_server = crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref())
|
||||
.await
|
||||
.expect("embedded app server");
|
||||
let parent_thread_id = ThreadId::new();
|
||||
let side_thread_id = ThreadId::new();
|
||||
let child_thread_id = ThreadId::new();
|
||||
app.primary_thread_id = Some(parent_thread_id);
|
||||
app.active_thread_id = Some(side_thread_id);
|
||||
app.side_threads
|
||||
.insert(side_thread_id, SideThreadState::new(parent_thread_id));
|
||||
app.sync_side_thread_ui();
|
||||
app.active_thread_id = Some(parent_thread_id);
|
||||
|
||||
app.handle_app_server_event(
|
||||
&app_server,
|
||||
codex_app_server_client::AppServerEvent::ServerNotification(
|
||||
ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification {
|
||||
thread_id: Some(child_thread_id.to_string()),
|
||||
name: "sentry".to_string(),
|
||||
status: McpServerStartupState::Failed,
|
||||
error: Some("sentry is not logged in".to_string()),
|
||||
@@ -3380,6 +3389,168 @@ async fn side_thread_ignores_global_mcp_startup_notifications() {
|
||||
.await;
|
||||
|
||||
assert!(app_event_rx.try_recv().is_err());
|
||||
let mut child_snapshot = app
|
||||
.thread_event_channels
|
||||
.get(&child_thread_id)
|
||||
.expect("child thread channel should be created")
|
||||
.store
|
||||
.lock()
|
||||
.await
|
||||
.snapshot();
|
||||
assert!(
|
||||
matches!(
|
||||
child_snapshot.events.as_slice(),
|
||||
[ThreadBufferedEvent::Notification(
|
||||
ServerNotification::McpServerStatusUpdated(_)
|
||||
)]
|
||||
),
|
||||
"child MCP startup notification should be buffered for the child thread"
|
||||
);
|
||||
|
||||
app.apply_refreshed_snapshot_thread(
|
||||
child_thread_id,
|
||||
AppServerStartedThread {
|
||||
session: test_thread_session(child_thread_id, test_path_buf("/tmp/child")),
|
||||
turns: Vec::new(),
|
||||
},
|
||||
&mut child_snapshot,
|
||||
)
|
||||
.await;
|
||||
app.replay_thread_snapshot(child_snapshot, /*resume_restored_queue*/ false);
|
||||
|
||||
let mut rendered_cells = Vec::new();
|
||||
while let Ok(event) = app_event_rx.try_recv() {
|
||||
if let AppEvent::InsertHistoryCell(cell) = event {
|
||||
rendered_cells.push(lines_to_single_string(&cell.display_lines(/*width*/ 120)));
|
||||
}
|
||||
}
|
||||
let rendered = rendered_cells.join("\n");
|
||||
assert_eq!(app.chat_widget.thread_id(), Some(child_thread_id));
|
||||
assert_eq!(rendered.matches("sentry is not logged in").count(), 1);
|
||||
assert_eq!(
|
||||
rendered
|
||||
.matches("MCP startup incomplete (failed: sentry)")
|
||||
.count(),
|
||||
1
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn app_scoped_mcp_startup_notifications_do_not_render_in_active_thread() {
|
||||
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
while app_event_rx.try_recv().is_ok() {}
|
||||
let app_server = crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref())
|
||||
.await
|
||||
.expect("embedded app server");
|
||||
let thread_id = ThreadId::new();
|
||||
app.primary_thread_id = Some(thread_id);
|
||||
app.active_thread_id = Some(thread_id);
|
||||
|
||||
app.handle_app_server_event(
|
||||
&app_server,
|
||||
codex_app_server_client::AppServerEvent::ServerNotification(
|
||||
ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification {
|
||||
thread_id: None,
|
||||
name: "sentry".to_string(),
|
||||
status: McpServerStartupState::Failed,
|
||||
error: Some("sentry is not logged in".to_string()),
|
||||
}),
|
||||
),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(app_event_rx.try_recv().is_err());
|
||||
assert_eq!(
|
||||
app.chat_widget.active_cell_transcript_lines(/*width*/ 120),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn active_side_thread_renders_live_mcp_startup_notifications() {
|
||||
let (mut app, mut app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
while app_event_rx.try_recv().is_ok() {}
|
||||
let sentry_config = toml::from_str::<toml::Value>("command = 'true'")
|
||||
.expect("test MCP config should parse")
|
||||
.try_into()
|
||||
.expect("test MCP config should deserialize");
|
||||
app.config
|
||||
.mcp_servers
|
||||
.set(std::collections::HashMap::from([(
|
||||
"sentry".to_string(),
|
||||
sentry_config,
|
||||
)]))
|
||||
.expect("test MCP servers should accept any configuration");
|
||||
let app_server = crate::start_embedded_app_server_for_picker(app.chat_widget.config_ref())
|
||||
.await
|
||||
.expect("embedded app server");
|
||||
let parent_thread_id = ThreadId::new();
|
||||
let side_thread_id = ThreadId::new();
|
||||
app.primary_thread_id = Some(parent_thread_id);
|
||||
app.side_threads
|
||||
.insert(side_thread_id, SideThreadState::new(parent_thread_id));
|
||||
app.ensure_thread_channel(side_thread_id);
|
||||
app.activate_thread_channel(side_thread_id).await;
|
||||
app.replay_thread_snapshot(
|
||||
ThreadEventSnapshot {
|
||||
session: Some(test_thread_session(
|
||||
side_thread_id,
|
||||
test_path_buf("/tmp/side"),
|
||||
)),
|
||||
turns: Vec::new(),
|
||||
events: Vec::new(),
|
||||
input_state: None,
|
||||
},
|
||||
/*resume_restored_queue*/ false,
|
||||
);
|
||||
app.sync_side_thread_ui();
|
||||
|
||||
for status in [
|
||||
McpServerStartupState::Starting,
|
||||
McpServerStartupState::Failed,
|
||||
] {
|
||||
app.handle_app_server_event(
|
||||
&app_server,
|
||||
codex_app_server_client::AppServerEvent::ServerNotification(
|
||||
ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification {
|
||||
thread_id: Some(side_thread_id.to_string()),
|
||||
name: "sentry".to_string(),
|
||||
status,
|
||||
error: matches!(status, McpServerStartupState::Failed)
|
||||
.then(|| "sentry is not logged in".to_string()),
|
||||
}),
|
||||
),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
let mut active_thread_events = Vec::new();
|
||||
let active_thread_rx = app
|
||||
.active_thread_rx
|
||||
.as_mut()
|
||||
.expect("side thread receiver should be active");
|
||||
while let Ok(event) = active_thread_rx.try_recv() {
|
||||
active_thread_events.push(event);
|
||||
}
|
||||
for event in active_thread_events {
|
||||
app.handle_thread_event_now(event);
|
||||
}
|
||||
|
||||
let mut rendered_cells = Vec::new();
|
||||
while let Ok(event) = app_event_rx.try_recv() {
|
||||
if let AppEvent::InsertHistoryCell(cell) = event {
|
||||
rendered_cells.push(lines_to_single_string(&cell.display_lines(/*width*/ 120)));
|
||||
}
|
||||
}
|
||||
let rendered = rendered_cells.join("\n");
|
||||
assert!(app.chat_widget.side_conversation_active());
|
||||
assert_eq!(rendered.matches("sentry is not logged in").count(), 1);
|
||||
assert_eq!(
|
||||
rendered
|
||||
.matches("MCP startup incomplete (failed: sentry)")
|
||||
.count(),
|
||||
1
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
|
||||
@@ -50,6 +50,7 @@ impl ThreadEventStore {
|
||||
ThreadBufferedEvent::Request(_)
|
||||
| ThreadBufferedEvent::Notification(ServerNotification::HookStarted(_))
|
||||
| ThreadBufferedEvent::Notification(ServerNotification::HookCompleted(_))
|
||||
| ThreadBufferedEvent::Notification(ServerNotification::McpServerStatusUpdated(_))
|
||||
| ThreadBufferedEvent::FeedbackSubmission(_)
|
||||
)
|
||||
}
|
||||
@@ -590,4 +591,31 @@ mod tests {
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn thread_event_store_rebase_preserves_mcp_startup_notifications() {
|
||||
let thread_id = ThreadId::new();
|
||||
let notification = ServerNotification::McpServerStatusUpdated(
|
||||
codex_app_server_protocol::McpServerStatusUpdatedNotification {
|
||||
thread_id: Some(thread_id.to_string()),
|
||||
name: "sentry".to_string(),
|
||||
status: codex_app_server_protocol::McpServerStartupState::Failed,
|
||||
error: Some("sentry is not logged in".to_string()),
|
||||
},
|
||||
);
|
||||
let mut store = ThreadEventStore::new(/*capacity*/ 8);
|
||||
store.push_notification(notification.clone());
|
||||
|
||||
store.rebase_buffer_after_session_refresh();
|
||||
|
||||
let snapshot = store.snapshot();
|
||||
let actual = match snapshot.events.as_slice() {
|
||||
[ThreadBufferedEvent::Notification(actual)] => actual,
|
||||
other => panic!("expected one buffered MCP notification, saw: {other:?}"),
|
||||
};
|
||||
assert_eq!(
|
||||
serde_json::to_value(actual).expect("MCP notification should serialize"),
|
||||
serde_json::to_value(notification).expect("MCP notification should serialize"),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1261,6 +1261,7 @@ impl App {
|
||||
snapshot: ThreadEventSnapshot,
|
||||
resume_restored_queue: bool,
|
||||
) {
|
||||
self.refresh_mcp_startup_expected_servers_from_config();
|
||||
let should_buffer_replay = self.terminal_resize_reflow_enabled()
|
||||
&& (!snapshot.turns.is_empty() || !snapshot.events.is_empty());
|
||||
if should_buffer_replay {
|
||||
|
||||
@@ -83,7 +83,13 @@ impl ChatWidget {
|
||||
// per-server failures immediately.
|
||||
let mut startup_status = self.mcp_startup_status.take().unwrap_or_default();
|
||||
if let McpStartupStatus::Failed { error } = &status {
|
||||
self.on_warning(error);
|
||||
let already_reported = matches!(
|
||||
startup_status.get(&server),
|
||||
Some(McpStartupStatus::Failed { error: previous }) if previous == error
|
||||
);
|
||||
if !already_reported {
|
||||
self.on_warning(error);
|
||||
}
|
||||
}
|
||||
startup_status.insert(server, status);
|
||||
startup_status
|
||||
|
||||
@@ -6,12 +6,6 @@ impl ChatWidget {
|
||||
notification: ServerNotification,
|
||||
replay_kind: Option<ReplayKind>,
|
||||
) {
|
||||
if self.active_side_conversation
|
||||
&& replay_kind.is_none()
|
||||
&& matches!(notification, ServerNotification::McpServerStatusUpdated(_))
|
||||
{
|
||||
return;
|
||||
}
|
||||
let from_replay = replay_kind.is_some();
|
||||
let is_resume_initial_replay =
|
||||
matches!(replay_kind, Some(ReplayKind::ResumeInitialMessages));
|
||||
|
||||
@@ -4,6 +4,7 @@ use pretty_assertions::assert_eq;
|
||||
fn notify_mcp_status(chat: &mut ChatWidget, name: &str, status: McpServerStartupState) {
|
||||
chat.handle_server_notification(
|
||||
ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification {
|
||||
thread_id: Some("thread-1".to_string()),
|
||||
name: name.to_string(),
|
||||
status,
|
||||
error: None,
|
||||
@@ -15,6 +16,7 @@ fn notify_mcp_status(chat: &mut ChatWidget, name: &str, status: McpServerStartup
|
||||
fn notify_mcp_status_error(chat: &mut ChatWidget, name: &str, error: &str) {
|
||||
chat.handle_server_notification(
|
||||
ServerNotification::McpServerStatusUpdated(McpServerStatusUpdatedNotification {
|
||||
thread_id: Some("thread-1".to_string()),
|
||||
name: name.to_string(),
|
||||
status: McpServerStartupState::Failed,
|
||||
error: Some(error.to_string()),
|
||||
@@ -23,6 +25,42 @@ fn notify_mcp_status_error(chat: &mut ChatWidget, name: &str, error: &str) {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_startup_dedupes_same_round_duplicate_failure_warning() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.show_welcome_banner = false;
|
||||
chat.set_mcp_startup_expected_servers(["alpha".to_string(), "beta".to_string()]);
|
||||
|
||||
notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting);
|
||||
notify_mcp_status_error(
|
||||
&mut chat,
|
||||
"alpha",
|
||||
"MCP client for `alpha` failed to start: handshake failed",
|
||||
);
|
||||
notify_mcp_status_error(
|
||||
&mut chat,
|
||||
"alpha",
|
||||
"MCP client for `alpha` failed to start: handshake failed",
|
||||
);
|
||||
|
||||
let failure_text = drain_insert_history(&mut rx)
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>();
|
||||
assert_eq!(
|
||||
failure_text,
|
||||
"⚠ MCP client for `alpha` failed to start: handshake failed\n"
|
||||
);
|
||||
|
||||
notify_mcp_status(&mut chat, "beta", McpServerStartupState::Ready);
|
||||
|
||||
let summary_text = drain_insert_history(&mut rx)
|
||||
.iter()
|
||||
.map(|lines| lines_to_single_string(lines))
|
||||
.collect::<String>();
|
||||
assert_eq!(summary_text, "⚠ MCP startup incomplete (failed: alpha)\n");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn mcp_startup_header_booting_snapshot() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
@@ -110,6 +148,11 @@ async fn app_server_mcp_startup_failure_renders_warning_history() {
|
||||
assert!(drain_insert_history(&mut rx).is_empty());
|
||||
assert!(chat.bottom_pane.is_task_running());
|
||||
|
||||
notify_mcp_status_error(
|
||||
&mut chat,
|
||||
"alpha",
|
||||
"MCP client for `alpha` failed to start: handshake failed",
|
||||
);
|
||||
notify_mcp_status_error(
|
||||
&mut chat,
|
||||
"alpha",
|
||||
|
||||
Reference in New Issue
Block a user