修复客户端MCP连接状态竞态
为每个 DirectProject MCP 连接分配运行时 token 按扩展项与连接 token 校验启动状态回写 扩展启停、重命名和删除时失效旧连接 owner 补充旧连接迟到状态回归测试
This commit is contained in:
@@ -619,6 +619,7 @@ struct CodexAppServerInner {
|
||||
workspace_path: std::path::PathBuf,
|
||||
workspace_mode: CodexAppServerWorkspaceMode,
|
||||
client_mcp_server_ids_by_name: HashMap<String, String>,
|
||||
client_mcp_connection_id: Option<String>,
|
||||
client_mcp_startup_statuses: Mutex<HashMap<String, String>>,
|
||||
client_mcp_startup_notify: Notify,
|
||||
initial_client_mcp_startup_waited: AtomicBool,
|
||||
@@ -1604,6 +1605,9 @@ impl CodexAppServerConnection {
|
||||
.iter()
|
||||
.map(|server| (server.name.clone(), server.extension_id.clone()))
|
||||
.collect::<HashMap<_, _>>();
|
||||
let client_mcp_connection_id = (!client_mcp_servers.is_empty()
|
||||
&& workspace_mode == CodexAppServerWorkspaceMode::DirectProject)
|
||||
.then(|| uuid::Uuid::new_v4().simple().to_string());
|
||||
if workspace_override.is_some() && workspace_mode.allows_workspace_writes() {
|
||||
trust_isolated_game_creator_codex_workspace(
|
||||
&isolated_codex_home,
|
||||
@@ -1751,6 +1755,7 @@ impl CodexAppServerConnection {
|
||||
workspace_path,
|
||||
workspace_mode,
|
||||
client_mcp_server_ids_by_name,
|
||||
client_mcp_connection_id: client_mcp_connection_id.clone(),
|
||||
client_mcp_startup_statuses: Mutex::new(HashMap::new()),
|
||||
client_mcp_startup_notify: Notify::new(),
|
||||
initial_client_mcp_startup_waited: AtomicBool::new(false),
|
||||
@@ -1758,6 +1763,18 @@ impl CodexAppServerConnection {
|
||||
tool_bridge,
|
||||
_skill_roots: skill_roots,
|
||||
});
|
||||
if let Some(connection_id) = client_mcp_connection_id.as_deref() {
|
||||
if let Err(error) = crate::client_extensions::claim_client_mcp_connection(
|
||||
client_mcp_servers
|
||||
.iter()
|
||||
.map(|server| server.extension_id.clone()),
|
||||
connection_id,
|
||||
) {
|
||||
shutdown_game_creator_codex_app_server_inner(&inner, "客户端 MCP 连接登记失败")
|
||||
.await;
|
||||
return Err(platform_llm::LlmError::InvalidConfig(error));
|
||||
}
|
||||
}
|
||||
tokio::spawn(read_game_creator_codex_app_server_stdout(
|
||||
Arc::downgrade(&inner),
|
||||
stdout,
|
||||
@@ -2675,10 +2692,13 @@ async fn read_game_creator_codex_app_server_stdout(
|
||||
inner.client_mcp_startup_notify.notify_waiters();
|
||||
}
|
||||
if let Some(extension_id) = inner.client_mcp_server_ids_by_name.get(name) {
|
||||
let _ = crate::client_extensions::record_client_mcp_startup_status(
|
||||
extension_id,
|
||||
status,
|
||||
);
|
||||
if let Some(connection_id) = inner.client_mcp_connection_id.as_deref() {
|
||||
let _ = crate::client_extensions::record_client_mcp_startup_status(
|
||||
extension_id,
|
||||
connection_id,
|
||||
status,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
continue;
|
||||
@@ -2923,6 +2943,7 @@ async fn fail_game_creator_codex_app_server_connection(
|
||||
if inner.closed.swap(true, Ordering::AcqRel) {
|
||||
return;
|
||||
}
|
||||
release_client_mcp_connection_for_inner(&inner);
|
||||
let exit_status = inner
|
||||
.child
|
||||
.lock()
|
||||
@@ -2949,6 +2970,7 @@ async fn shutdown_game_creator_codex_app_server_inner(
|
||||
reason: &str,
|
||||
) {
|
||||
inner.closed.store(true, Ordering::Release);
|
||||
release_client_mcp_connection_for_inner(inner);
|
||||
for (_, pending) in inner.pending.lock().await.drain() {
|
||||
let _ = pending.sender.send(Err(reason.to_string()));
|
||||
}
|
||||
@@ -2962,6 +2984,16 @@ async fn shutdown_game_creator_codex_app_server_inner(
|
||||
}
|
||||
}
|
||||
|
||||
fn release_client_mcp_connection_for_inner(inner: &Arc<CodexAppServerInner>) {
|
||||
let Some(connection_id) = inner.client_mcp_connection_id.as_deref() else {
|
||||
return;
|
||||
};
|
||||
let _ = crate::client_extensions::release_client_mcp_connection(
|
||||
inner.client_mcp_server_ids_by_name.values().cloned(),
|
||||
connection_id,
|
||||
);
|
||||
}
|
||||
|
||||
pub(in crate::agent) async fn request_game_creator_agent_codex_app_server(
|
||||
snapshot: &AgentRuntimeProviderRequestSnapshot,
|
||||
llm: &GameCreatorLlmConfig,
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
use serde::{Deserialize, Serialize};
|
||||
use sha2::{Digest, Sha256};
|
||||
use std::collections::{BTreeMap, BTreeSet};
|
||||
use std::collections::{BTreeMap, BTreeSet, HashMap};
|
||||
use std::fs;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::{Mutex, OnceLock};
|
||||
@@ -14,6 +14,7 @@ const CLIENT_EXTENSIONS_SCHEMA_VERSION: &str = "direct-project-client-extensions
|
||||
const CLIENT_EXTENSIONS_SOURCES_DIR_NAME: &str = "sources";
|
||||
const RESERVED_MCP_SERVER_NAME: &str = "agc_tools";
|
||||
static CLIENT_EXTENSIONS_INDEX_LOCK: OnceLock<Mutex<()>> = OnceLock::new();
|
||||
static CLIENT_MCP_CONNECTION_OWNERS: OnceLock<Mutex<HashMap<String, String>>> = OnceLock::new();
|
||||
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
pub(crate) struct ClientMcpRuntimeServer {
|
||||
@@ -144,6 +145,73 @@ fn client_extensions_index_lock() -> &'static Mutex<()> {
|
||||
CLIENT_EXTENSIONS_INDEX_LOCK.get_or_init(|| Mutex::new(()))
|
||||
}
|
||||
|
||||
fn client_mcp_connection_owners() -> &'static Mutex<HashMap<String, String>> {
|
||||
CLIENT_MCP_CONNECTION_OWNERS.get_or_init(|| Mutex::new(HashMap::new()))
|
||||
}
|
||||
|
||||
pub(crate) fn claim_client_mcp_connection(
|
||||
extension_ids: impl IntoIterator<Item = String>,
|
||||
connection_id: &str,
|
||||
) -> Result<(), String> {
|
||||
let root = extensions_root()?;
|
||||
claim_client_mcp_connection_at(&root, extension_ids, connection_id)
|
||||
}
|
||||
|
||||
fn claim_client_mcp_connection_at(
|
||||
root: &Path,
|
||||
extension_ids: impl IntoIterator<Item = String>,
|
||||
connection_id: &str,
|
||||
) -> Result<(), String> {
|
||||
let _index_guard = client_extensions_index_lock()
|
||||
.lock()
|
||||
.map_err(|_| "客户端扩展索引锁已损坏".to_string())?;
|
||||
let index = read_index(&root)?;
|
||||
let enabled_ids = index
|
||||
.items
|
||||
.iter()
|
||||
.filter(|item| item.extension_type == "mcp" && item.enabled)
|
||||
.map(|item| item.id.as_str())
|
||||
.collect::<BTreeSet<_>>();
|
||||
let mut owners = client_mcp_connection_owners()
|
||||
.lock()
|
||||
.map_err(|_| "客户端 MCP 连接锁已损坏".to_string())?;
|
||||
for extension_id in extension_ids {
|
||||
if enabled_ids.contains(extension_id.as_str()) {
|
||||
owners.insert(extension_id, connection_id.to_string());
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(crate) fn release_client_mcp_connection(
|
||||
extension_ids: impl IntoIterator<Item = String>,
|
||||
connection_id: &str,
|
||||
) -> Result<(), String> {
|
||||
let _index_guard = client_extensions_index_lock()
|
||||
.lock()
|
||||
.map_err(|_| "客户端扩展索引锁已损坏".to_string())?;
|
||||
let mut owners = client_mcp_connection_owners()
|
||||
.lock()
|
||||
.map_err(|_| "客户端 MCP 连接锁已损坏".to_string())?;
|
||||
for extension_id in extension_ids {
|
||||
if owners.get(&extension_id).map(String::as_str) == Some(connection_id) {
|
||||
owners.remove(&extension_id);
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn invalidate_client_mcp_connection(extension_id: &str) -> Result<(), String> {
|
||||
let _index_guard = client_extensions_index_lock()
|
||||
.lock()
|
||||
.map_err(|_| "客户端扩展索引锁已损坏".to_string())?;
|
||||
client_mcp_connection_owners()
|
||||
.lock()
|
||||
.map_err(|_| "客户端 MCP 连接锁已损坏".to_string())?
|
||||
.remove(extension_id);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn read_client_extension_index_locked<T>(
|
||||
root: &Path,
|
||||
reader: impl FnOnce(&Path, &ClientExtensionIndex) -> Result<T, String>,
|
||||
@@ -1094,9 +1162,26 @@ fn apply_client_mcp_startup_status(
|
||||
|
||||
pub(crate) fn record_client_mcp_startup_status(
|
||||
extension_id: &str,
|
||||
connection_id: &str,
|
||||
status: &str,
|
||||
) -> Result<(), String> {
|
||||
update_client_extension_index(|_, index| {
|
||||
let root = extensions_root()?;
|
||||
record_client_mcp_startup_status_at(&root, extension_id, connection_id, status)
|
||||
}
|
||||
|
||||
fn record_client_mcp_startup_status_at(
|
||||
root: &Path,
|
||||
extension_id: &str,
|
||||
connection_id: &str,
|
||||
status: &str,
|
||||
) -> Result<(), String> {
|
||||
update_client_extension_index_at(root, |_, index| {
|
||||
let owners = client_mcp_connection_owners()
|
||||
.lock()
|
||||
.map_err(|_| "客户端 MCP 连接锁已损坏".to_string())?;
|
||||
if owners.get(extension_id).map(String::as_str) != Some(connection_id) {
|
||||
return Ok(((), false));
|
||||
}
|
||||
let changed = apply_client_mcp_startup_status(index, extension_id, status)?;
|
||||
Ok(((), changed))
|
||||
})
|
||||
@@ -1346,6 +1431,7 @@ fn set_client_extension_enabled_at(
|
||||
id: &str,
|
||||
enabled: bool,
|
||||
) -> Result<ClientExtensionItem, String> {
|
||||
invalidate_client_mcp_connection(id.trim())?;
|
||||
update_client_extension_index_at(root, |_, index| {
|
||||
let item = index.items.iter_mut().find(|item| item.id == id.trim())
|
||||
.ok_or_else(|| "未找到客户端扩展".to_string())?;
|
||||
@@ -1378,6 +1464,7 @@ fn rename_client_extension_at(
|
||||
id: &str,
|
||||
normalized: &str,
|
||||
) -> Result<ClientExtensionItem, String> {
|
||||
invalidate_client_mcp_connection(id.trim())?;
|
||||
update_client_extension_index_at(root, |_, index| {
|
||||
let item_index = index.items.iter().position(|item| item.id == id.trim())
|
||||
.ok_or_else(|| "未找到客户端扩展".to_string())?;
|
||||
@@ -1423,6 +1510,7 @@ pub(crate) fn remove_client_extension(id: String) -> Result<(), String> {
|
||||
}
|
||||
|
||||
fn remove_client_extension_at(root: &Path, id: &str) -> Result<(), String> {
|
||||
invalidate_client_mcp_connection(id.trim())?;
|
||||
update_client_extension_index_at(root, |_, index| {
|
||||
let item_index = index.items.iter().position(|item| item.id == id.trim())
|
||||
.ok_or_else(|| "未找到客户端扩展".to_string())?;
|
||||
@@ -1940,6 +2028,73 @@ mod tests {
|
||||
assert_eq!(index.items[1].last_error, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn stale_mcp_startup_status_from_old_connection_is_ignored() {
|
||||
let (_directory, root) = test_extension_root();
|
||||
let extension_id = "mcp-stale-status";
|
||||
update_client_extension_index_at(&root, |_, index| {
|
||||
index.items.push(StoredExtensionItem {
|
||||
id: extension_id.to_string(),
|
||||
source_id: "source-1".to_string(),
|
||||
extension_type: "mcp".to_string(),
|
||||
name: "search".to_string(),
|
||||
original_name: "search".to_string(),
|
||||
source_relative_path: "config.toml".to_string(),
|
||||
enabled: true,
|
||||
fingerprint: "content-a".to_string(),
|
||||
last_error: None,
|
||||
mcp_config: Some(serde_json::json!({"command": "search"})),
|
||||
});
|
||||
Ok(((), true))
|
||||
})
|
||||
.expect("insert MCP extension");
|
||||
|
||||
claim_client_mcp_connection_at(
|
||||
&root,
|
||||
[extension_id.to_string()],
|
||||
"connection-old",
|
||||
)
|
||||
.expect("claim old MCP connection");
|
||||
record_client_mcp_startup_status_at(
|
||||
&root,
|
||||
extension_id,
|
||||
"connection-old",
|
||||
"failed",
|
||||
)
|
||||
.expect("record old failure");
|
||||
|
||||
claim_client_mcp_connection_at(
|
||||
&root,
|
||||
[extension_id.to_string()],
|
||||
"connection-new",
|
||||
)
|
||||
.expect("claim new MCP connection");
|
||||
record_client_mcp_startup_status_at(
|
||||
&root,
|
||||
extension_id,
|
||||
"connection-old",
|
||||
"ready",
|
||||
)
|
||||
.expect("ignore stale ready status");
|
||||
let index = read_index(&root).expect("read status after stale callback");
|
||||
assert_eq!(
|
||||
index.items[0].last_error.as_deref(),
|
||||
Some("MCP Server 启动失败")
|
||||
);
|
||||
|
||||
record_client_mcp_startup_status_at(
|
||||
&root,
|
||||
extension_id,
|
||||
"connection-new",
|
||||
"ready",
|
||||
)
|
||||
.expect("record current ready status");
|
||||
let index = read_index(&root).expect("read status after current callback");
|
||||
assert_eq!(index.items[0].last_error, None);
|
||||
release_client_mcp_connection([extension_id.to_string()], "connection-new")
|
||||
.expect("release current MCP connection");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn client_extension_index_updates_read_latest_snapshot() {
|
||||
let directory = tempfile::tempdir().expect("temporary extension root");
|
||||
|
||||
@@ -334,7 +334,7 @@ Skill root 是当前启用 Skill 集合的完整投影;每次准备时先清
|
||||
|
||||
运行时只转换 Codex 原生能够直接使用的标准字段。STDIO 支持 `command / args / env / env_vars / cwd`;HTTP 支持 `url / bearer_token_env_var / http_headers / env_http_headers`;两类都支持原生启动超时、工具超时和工具筛选字段。客户端启用状态是运行时权威,每个第三方项固定 `enabled=true`、`required=false`,并沿用 DirectProject 无逐工具弹窗的自动批准方式。第三方 MCP 配置写入本次隔离 `CODEX_HOME/config.toml`;DirectProject 启动命令同时以 `-c mcp_optional_startup_grace_ms=120000` 传入同一值,命令行 override 是 Codex 最终生效来源。DirectProject 在首次 `turn/start` 前消费 app-server 已有的 `mcpServer/startupStatus/updated` 通知,等待每个已启用第三方 MCP 进入 `ready`、`failed` 或 `cancelled`;最长等待 `120000` 毫秒,超时后继续对话。ready 的 MCP 进入首轮,失败或超时的 MCP 被跳过并记录状态。该配置只作用于本次 DirectProject 隔离运行,不修改用户全局 Codex 配置。DirectProject 系统提示会声明客户端扩展列表中已启用的第三方 MCP;用户明确指定 Server 或工具时,模型只在当前可用工具中查找,找不到则如实说明,不伪造结果。
|
||||
|
||||
客户端扩展索引的所有读改写操作在进程内使用同一把互斥锁串行化;原子写文件只负责避免半截文件,不能替代这层读改写协调。每个运行时 MCP 同时保留客户端扩展项 ID,app-server 的启动状态通知按“连接建立时的 Server 名称 → 扩展项 ID”映射回写;迟到的旧连接通知找不到已删除或已重导入的新项时直接忽略。已启用 MCP 的连接池 fingerprint 包含扩展项 ID 和来源 ID,避免删除后重导入相同内容时复用旧连接。
|
||||
客户端扩展索引的所有读改写操作在进程内使用同一把互斥锁串行化;原子写文件只负责避免半截文件,不能替代这层读改写协调。每个运行时 MCP 同时保留客户端扩展项 ID 和当前 app-server 连接 token,启动状态通知按“连接建立时的 Server 名称 → 扩展项 ID + 连接 token”映射回写;迟到的旧连接通知、扩展启停/删除后的失效 token 直接忽略。连接 token 只存在进程内 owner registry,不写入持久化索引。已启用 MCP 的连接池 fingerprint 包含扩展项 ID 和来源 ID,避免删除后重导入相同内容时复用旧连接。
|
||||
|
||||
用户导入来源中的相对 `cwd` 按该 MCP 配置文件所在目录解析;未声明 `cwd` 的 STDIO Server 默认以该目录启动,从而保留脚本参数的原生相对路径语义。客户端不替用户猜测普通文件如何启动。
|
||||
|
||||
|
||||
Reference in New Issue
Block a user