mirror of
https://github.com/openai/codex.git
synced 2026-10-28 18:58:46 +00:00
Fallback for older app-server MCP inventory
This commit is contained in:
@@ -234,7 +234,7 @@ fn sample_thread_start_response(
|
||||
model_provider: "openai".to_string(),
|
||||
service_tier: None,
|
||||
cwd: test_path_buf("/tmp").abs(),
|
||||
mcp_server_names: Vec::new(),
|
||||
mcp_server_names: None,
|
||||
runtime_workspace_roots: Vec::new(),
|
||||
instruction_sources: Vec::new(),
|
||||
approval_policy: AppServerAskForApproval::OnRequest,
|
||||
|
||||
@@ -309,7 +309,7 @@ fn sample_thread_start_response() -> ClientResponsePayload {
|
||||
model_provider: "openai".to_string(),
|
||||
service_tier: None,
|
||||
cwd: test_path_buf("/tmp").abs(),
|
||||
mcp_server_names: Vec::new(),
|
||||
mcp_server_names: None,
|
||||
runtime_workspace_roots: Vec::new(),
|
||||
instruction_sources: Vec::new(),
|
||||
approval_policy: AppServerAskForApproval::OnRequest,
|
||||
|
||||
+5
-2
@@ -19532,12 +19532,15 @@
|
||||
"type": "array"
|
||||
},
|
||||
"mcpServerNames": {
|
||||
"default": [],
|
||||
"default": null,
|
||||
"description": "Names of the effective MCP servers in the thread's initial runtime.",
|
||||
"items": {
|
||||
"type": "string"
|
||||
},
|
||||
"type": "array"
|
||||
"type": [
|
||||
"array",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"model": {
|
||||
"type": "string"
|
||||
|
||||
+5
-2
@@ -17311,12 +17311,15 @@
|
||||
"type": "array"
|
||||
},
|
||||
"mcpServerNames": {
|
||||
"default": [],
|
||||
"default": null,
|
||||
"description": "Names of the effective MCP servers in the thread's initial runtime.",
|
||||
"items": {
|
||||
"type": "string"
|
||||
},
|
||||
"type": "array"
|
||||
"type": [
|
||||
"array",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"model": {
|
||||
"type": "string"
|
||||
|
||||
@@ -2431,12 +2431,15 @@
|
||||
"type": "array"
|
||||
},
|
||||
"mcpServerNames": {
|
||||
"default": [],
|
||||
"default": null,
|
||||
"description": "Names of the effective MCP servers in the thread's initial runtime.",
|
||||
"items": {
|
||||
"type": "string"
|
||||
},
|
||||
"type": "array"
|
||||
"type": [
|
||||
"array",
|
||||
"null"
|
||||
]
|
||||
},
|
||||
"model": {
|
||||
"type": "string"
|
||||
|
||||
@@ -12,7 +12,7 @@ import type { Thread } from "./Thread";
|
||||
export type ThreadStartResponse = {thread: Thread, model: string, modelProvider: string, serviceTier: string | null, cwd: AbsolutePathBuf, /**
|
||||
* Names of the effective MCP servers in the thread's initial runtime.
|
||||
*/
|
||||
mcpServerNames: Array<string>, /**
|
||||
mcpServerNames: Array<string> | null, /**
|
||||
* Environment-native paths to instruction source files currently loaded for this thread.
|
||||
*/
|
||||
instructionSources: Array<LegacyAppPathString>, approvalPolicy: AskForApproval, /**
|
||||
|
||||
@@ -2608,7 +2608,7 @@ mod tests {
|
||||
model_provider: "openai".to_string(),
|
||||
service_tier: None,
|
||||
cwd,
|
||||
mcp_server_names: vec!["example".to_string()],
|
||||
mcp_server_names: Some(vec!["example".to_string()]),
|
||||
runtime_workspace_roots: Vec::new(),
|
||||
instruction_sources: vec![
|
||||
codex_utils_path_uri::LegacyAppPathString::from_abs_path(&absolute_path(
|
||||
|
||||
@@ -3978,6 +3978,7 @@ fn thread_lifecycle_responses_default_missing_optional_fields() {
|
||||
serde_json::from_value(response.clone()).expect("thread/fork response");
|
||||
|
||||
assert_eq!(start.instruction_sources, Vec::<LegacyAppPathString>::new());
|
||||
assert_eq!(start.mcp_server_names, None);
|
||||
assert_eq!(start.thread.parent_thread_id, None);
|
||||
assert_eq!(start.thread.recency_at, None);
|
||||
assert_eq!(
|
||||
|
||||
@@ -175,7 +175,7 @@ pub struct ThreadStartResponse {
|
||||
pub cwd: AbsolutePathBuf,
|
||||
/// Names of the effective MCP servers in the thread's initial runtime.
|
||||
#[serde(default)]
|
||||
pub mcp_server_names: Vec<String>,
|
||||
pub mcp_server_names: Option<Vec<String>>,
|
||||
/// Thread-scoped runtime workspace roots used to materialize
|
||||
/// `:workspace_roots`.
|
||||
#[experimental("thread/start.runtimeWorkspaceRoots")]
|
||||
|
||||
@@ -1290,7 +1290,7 @@ impl ThreadRequestProcessor {
|
||||
model_provider: config_snapshot.model_provider_id,
|
||||
service_tier: config_snapshot.service_tier,
|
||||
cwd,
|
||||
mcp_server_names,
|
||||
mcp_server_names: Some(mcp_server_names),
|
||||
runtime_workspace_roots: config_snapshot.workspace_roots,
|
||||
instruction_sources,
|
||||
approval_policy: config_snapshot.approval_policy.into(),
|
||||
|
||||
@@ -81,7 +81,7 @@ url = "{mcp_server_url}/mcp"
|
||||
let thread_start_response: ThreadStartResponse = to_response(thread_start_response)?;
|
||||
assert_eq!(
|
||||
thread_start_response.mcp_server_names,
|
||||
vec!["some-server".to_string()]
|
||||
Some(vec!["some-server".to_string()])
|
||||
);
|
||||
|
||||
let request_id = mcp
|
||||
|
||||
@@ -782,7 +782,7 @@ fn sample_thread_start_response() -> ThreadStartResponse {
|
||||
model_provider: "openai".to_string(),
|
||||
service_tier: None,
|
||||
cwd: test_path_buf("/tmp").abs(),
|
||||
mcp_server_names: Vec::new(),
|
||||
mcp_server_names: None,
|
||||
runtime_workspace_roots: Vec::new(),
|
||||
instruction_sources: Vec::new(),
|
||||
approval_policy: codex_app_server_protocol::AskForApproval::OnRequest,
|
||||
|
||||
@@ -517,6 +517,29 @@ impl App {
|
||||
turns,
|
||||
mcp_server_names,
|
||||
}) => {
|
||||
let mcp_server_names = match mcp_server_names {
|
||||
Some(mcp_server_names) => mcp_server_names,
|
||||
None if self.chat_widget.startup_draft_needs_mcp_server_names() => {
|
||||
match super::background_requests::fetch_all_mcp_server_statuses(
|
||||
app_server.request_handle(),
|
||||
McpServerStatusDetail::ToolsAndAuthOnly,
|
||||
Some(session.thread_id),
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok(statuses) => {
|
||||
statuses.into_iter().map(|status| status.name).collect()
|
||||
}
|
||||
Err(err) => {
|
||||
tracing::warn!(
|
||||
"failed to load startup MCP server names from an older app server: {err}"
|
||||
);
|
||||
Vec::new()
|
||||
}
|
||||
}
|
||||
}
|
||||
None => Vec::new(),
|
||||
};
|
||||
self.chat_widget
|
||||
.set_startup_draft_expected_mcp_servers(mcp_server_names);
|
||||
self.enqueue_primary_thread_session(session, turns).await?;
|
||||
|
||||
@@ -3529,7 +3529,7 @@ async fn primary_thread_ignores_child_mcp_startup_notifications() {
|
||||
AppServerStartedThread {
|
||||
session: test_thread_session(child_thread_id, test_path_buf("/tmp/child")),
|
||||
turns: Vec::new(),
|
||||
mcp_server_names: Vec::new(),
|
||||
mcp_server_names: None,
|
||||
},
|
||||
&mut child_snapshot,
|
||||
)
|
||||
@@ -5496,7 +5496,7 @@ async fn refreshed_snapshot_session_persists_resumed_turns() {
|
||||
AppServerStartedThread {
|
||||
session: resumed_session.clone(),
|
||||
turns: resumed_turns.clone(),
|
||||
mcp_server_names: Vec::new(),
|
||||
mcp_server_names: None,
|
||||
},
|
||||
&mut snapshot,
|
||||
)
|
||||
|
||||
@@ -165,7 +165,7 @@ async fn restored_startup_draft_waits_for_effective_mcp_settlement() {
|
||||
Ok(AppServerStartedThread {
|
||||
session: test_thread_session(thread_id, test_path_buf("/tmp/project")),
|
||||
turns: Vec::new(),
|
||||
mcp_server_names: vec!["test".to_string()],
|
||||
mcp_server_names: Some(vec!["test".to_string()]),
|
||||
}),
|
||||
)
|
||||
.await
|
||||
@@ -230,6 +230,44 @@ async fn restored_startup_draft_waits_for_effective_mcp_settlement() {
|
||||
assert_eq!(app.chat_widget.composer_text_with_pending(), "");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn restored_startup_draft_loads_mcp_names_from_older_app_servers() {
|
||||
let (mut app, _app_event_rx, mut op_rx) = make_test_app_with_channels().await;
|
||||
app.pending_startup_thread_start = true;
|
||||
app.chat_widget
|
||||
.set_queue_submissions_until_session_configured(/*queue*/ true);
|
||||
app.chat_widget
|
||||
.restore_startup_draft("captured during startup");
|
||||
|
||||
let mut app_server = Box::pin(crate::start_embedded_app_server_for_picker(
|
||||
app.chat_widget.config_ref(),
|
||||
))
|
||||
.await
|
||||
.expect("embedded app server");
|
||||
let mut started = app_server
|
||||
.start_thread(app.chat_widget.config_ref())
|
||||
.await
|
||||
.expect("startup thread");
|
||||
started.mcp_server_names = None;
|
||||
|
||||
app.handle_startup_thread_started(&mut app_server, Ok(started))
|
||||
.await
|
||||
.expect("older app-server startup should attach");
|
||||
app.chat_widget
|
||||
.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
|
||||
match next_user_turn_op(&mut op_rx) {
|
||||
Op::UserTurn { items, .. } => assert_eq!(
|
||||
items,
|
||||
vec![UserInput::Text {
|
||||
text: "captured during startup".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}]
|
||||
),
|
||||
other => panic!("expected restored startup input submission, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn startup_without_restored_draft_uses_normal_queued_submission() {
|
||||
let (mut app, _app_event_rx, _op_rx) = make_test_app_with_channels().await;
|
||||
@@ -306,7 +344,7 @@ fn stale_startup_thread_started_removes_local_routing_state() -> Result<()> {
|
||||
Ok(AppServerStartedThread {
|
||||
session: test_thread_session(stale_thread_id, test_path_buf("/tmp/project")),
|
||||
turns: Vec::new(),
|
||||
mcp_server_names: Vec::new(),
|
||||
mcp_server_names: None,
|
||||
}),
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -203,7 +203,7 @@ impl ThreadParamsMode {
|
||||
pub(crate) struct AppServerStartedThread {
|
||||
pub(crate) session: ThreadSessionState,
|
||||
pub(crate) turns: Vec<Turn>,
|
||||
pub(crate) mcp_server_names: Vec<String>,
|
||||
pub(crate) mcp_server_names: Option<Vec<String>>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
@@ -1548,7 +1548,7 @@ async fn started_thread_from_resume_response(
|
||||
Ok(AppServerStartedThread {
|
||||
session,
|
||||
turns: response.thread.turns,
|
||||
mcp_server_names: Vec::new(),
|
||||
mcp_server_names: None,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -1564,7 +1564,7 @@ async fn started_thread_from_fork_response(
|
||||
Ok(AppServerStartedThread {
|
||||
session,
|
||||
turns: response.thread.turns,
|
||||
mcp_server_names: Vec::new(),
|
||||
mcp_server_names: None,
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@@ -25,6 +25,10 @@ impl ChatWidget {
|
||||
self.finish_startup_draft_protection_if_ready();
|
||||
}
|
||||
|
||||
pub(crate) fn startup_draft_needs_mcp_server_names(&self) -> bool {
|
||||
self.startup_draft_pending_mcp_servers.is_some()
|
||||
}
|
||||
|
||||
pub(super) fn finish_startup_draft_protection_if_ready(&mut self) {
|
||||
let startup_ready = self.is_session_configured()
|
||||
&& self
|
||||
|
||||
Reference in New Issue
Block a user