mirror of
https://github.com/openai/codex.git
synced 2026-11-13 14:46:14 +00:00
Distinguish unknown MCP authentication status (#36045)
## Why OAuth discovery failures do not establish that an MCP server lacks OAuth support. Reporting those failures as `unsupported` conflates an inconclusive check with a confirmed result. ## What changed - Add an `unknown` MCP authentication status across the protocol, app server, CLI, and TUI. - Preserve OAuth discovery errors so callers can report `unknown`, while retaining `unsupported` for servers known not to support OAuth. - Document the distinction in the app server API. ## Testing - Verify transient HTTP discovery errors are preserved. - Verify `codex mcp list --json` reports `unknown` when discovery is rate limited. GitOrigin-RevId: e4562985971606740538e542ec7eeee502111964
This commit is contained in:
1 parent
6493417150
commit
7ec480dda5
16 files changed
+58
-12
No files matched your search
+1
@@ -12777,6 +12777,7 @@
|
|||||||
},
|
},
|
||||||
"McpAuthStatus": {
|
"McpAuthStatus": {
|
||||||
"enum": [
|
"enum": [
|
||||||
|
"unknown",
|
||||||
"unsupported",
|
"unsupported",
|
||||||
"notLoggedIn",
|
"notLoggedIn",
|
||||||
"bearerToken",
|
"bearerToken",
|
||||||
|
|||||||
+1
@@ -9119,6 +9119,7 @@
|
|||||||
},
|
},
|
||||||
"McpAuthStatus": {
|
"McpAuthStatus": {
|
||||||
"enum": [
|
"enum": [
|
||||||
|
"unknown",
|
||||||
"unsupported",
|
"unsupported",
|
||||||
"notLoggedIn",
|
"notLoggedIn",
|
||||||
"bearerToken",
|
"bearerToken",
|
||||||
|
|||||||
@@ -3,6 +3,7 @@
|
|||||||
"definitions": {
|
"definitions": {
|
||||||
"McpAuthStatus": {
|
"McpAuthStatus": {
|
||||||
"enum": [
|
"enum": [
|
||||||
|
"unknown",
|
||||||
"unsupported",
|
"unsupported",
|
||||||
"notLoggedIn",
|
"notLoggedIn",
|
||||||
"bearerToken",
|
"bearerToken",
|
||||||
|
|||||||
@@ -2,4 +2,4 @@
|
|||||||
|
|
||||||
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
|
// This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually.
|
||||||
|
|
||||||
export type McpAuthStatus = "unsupported" | "notLoggedIn" | "bearerToken" | "oAuth";
|
export type McpAuthStatus = "unknown" | "unsupported" | "notLoggedIn" | "bearerToken" | "oAuth";
|
||||||
@@ -16,6 +16,7 @@ use ts_rs::TS;
|
|||||||
|
|
||||||
v2_enum_from_core!(
|
v2_enum_from_core!(
|
||||||
pub enum McpAuthStatus from codex_protocol::protocol::McpAuthStatus {
|
pub enum McpAuthStatus from codex_protocol::protocol::McpAuthStatus {
|
||||||
|
Unknown,
|
||||||
Unsupported,
|
Unsupported,
|
||||||
NotLoggedIn,
|
NotLoggedIn,
|
||||||
BearerToken,
|
BearerToken,
|
||||||
|
|||||||
@@ -2409,7 +2409,7 @@ fn mcp_server_status_serializes_absent_server_info_as_null() {
|
|||||||
tools: HashMap::new(),
|
tools: HashMap::new(),
|
||||||
resources: Vec::new(),
|
resources: Vec::new(),
|
||||||
resource_templates: Vec::new(),
|
resource_templates: Vec::new(),
|
||||||
auth_status: McpAuthStatus::Unsupported,
|
auth_status: McpAuthStatus::Unknown,
|
||||||
}],
|
}],
|
||||||
next_cursor: None,
|
next_cursor: None,
|
||||||
};
|
};
|
||||||
@@ -2423,7 +2423,7 @@ fn mcp_server_status_serializes_absent_server_info_as_null() {
|
|||||||
"tools": {},
|
"tools": {},
|
||||||
"resources": [],
|
"resources": [],
|
||||||
"resourceTemplates": [],
|
"resourceTemplates": [],
|
||||||
"authStatus": "unsupported",
|
"authStatus": "unknown",
|
||||||
}],
|
}],
|
||||||
"nextCursor": null,
|
"nextCursor": null,
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -241,7 +241,7 @@ Example with notification opt-out:
|
|||||||
- `mcpServer/oauth/login` — start an OAuth login for a configured MCP server; pass `threadId` to resolve servers from that thread's selected plugins and executor, and receive an `authorization_url` followed by `mcpServer/oauthLogin/completed` once the browser flow finishes.
|
- `mcpServer/oauth/login` — start an OAuth login for a configured MCP server; pass `threadId` to resolve servers from that thread's selected plugins and executor, and receive an `authorization_url` followed by `mcpServer/oauthLogin/completed` once the browser flow finishes.
|
||||||
- `tool/requestUserInput` — prompt the user with 1–3 short questions for a tool call and return their answers (experimental).
|
- `tool/requestUserInput` — prompt the user with 1–3 short questions for a tool call and return their answers (experimental).
|
||||||
- `config/mcpServer/reload` — reload MCP server config from disk and queue a refresh for loaded threads (applied on each thread's next active turn); returns `{}`. Use this after editing `config.toml` without restarting the server.
|
- `config/mcpServer/reload` — reload MCP server config from disk and queue a refresh for loaded threads (applied on each thread's next active turn); returns `{}`. Use this after editing `config.toml` without restarting the server.
|
||||||
- `mcpServerStatus/list` — enumerate configured MCP servers with their tools, auth status, server info, plus resources/resource templates for `full` detail; supports optional `threadId` and cursor+limit pagination. If `threadId` is omitted, the server reads from the latest global config directly. If `detail` is omitted, the server defaults to `full`.
|
- `mcpServerStatus/list` — enumerate configured MCP servers with their tools, auth status, server info, plus resources/resource templates for `full` detail; supports optional `threadId` and cursor+limit pagination. If `threadId` is omitted, the server reads from the latest global config directly. If `detail` is omitted, the server defaults to `full`. An `unknown` auth status means OAuth support could not be determined; `unsupported` means OAuth is known not to be supported.
|
||||||
- `mcpServer/resource/read` — read a resource from a configured MCP server by optional `threadId`, `server`, and `uri`, returning text/blob resource `contents`. If `threadId` is omitted, the server reads from the latest MCP config directly.
|
- `mcpServer/resource/read` — read a resource from a configured MCP server by optional `threadId`, `server`, and `uri`, returning text/blob resource `contents`. If `threadId` is omitted, the server reads from the latest MCP config directly.
|
||||||
- `mcpServer/tool/call` — call a tool on a thread's configured MCP server by `threadId`, `server`, `tool`, optional `arguments`, and optional `_meta`, returning the MCP tool result.
|
- `mcpServer/tool/call` — call a tool on a thread's configured MCP server by `threadId`, `server`, `tool`, optional `arguments`, and optional `_meta`, returning the MCP tool result.
|
||||||
- `windowsSandbox/setupStart` — start Windows sandbox setup for the selected mode (`elevated` or `unelevated`); accepts an optional absolute `cwd` to target setup for a specific workspace, returns `{ started: true }` immediately, and later emits `windowsSandbox/setupCompleted`.
|
- `windowsSandbox/setupStart` — start Windows sandbox setup for the selected mode (`elevated` or `unelevated`); accepts an optional absolute `cwd` to target setup for a specific workspace, returns `{ started: true }` immediately, and later emits `windowsSandbox/setupCompleted`.
|
||||||
|
|||||||
@@ -232,6 +232,36 @@ async fn list_discovers_local_oauth_server_through_environment_proxy() -> Result
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
|
async fn list_reports_unknown_auth_status_when_oauth_discovery_is_rate_limited() -> Result<()> {
|
||||||
|
let codex_home = TempDir::new()?;
|
||||||
|
let server = wiremock::MockServer::start().await;
|
||||||
|
wiremock::Mock::given(wiremock::matchers::method("GET"))
|
||||||
|
.and(wiremock::matchers::path("/mcp"))
|
||||||
|
.respond_with(wiremock::ResponseTemplate::new(429))
|
||||||
|
.expect(1)
|
||||||
|
.mount(&server)
|
||||||
|
.await;
|
||||||
|
configure_http_oauth_server(codex_home.path(), &format!("{}/mcp", server.uri())).await?;
|
||||||
|
|
||||||
|
codex_command(codex_home.path())?
|
||||||
|
.env("NO_PROXY", "127.0.0.1,localhost")
|
||||||
|
.env("no_proxy", "127.0.0.1,localhost")
|
||||||
|
.args([
|
||||||
|
"-c",
|
||||||
|
"mcp_oauth_credentials_store=\"file\"",
|
||||||
|
"mcp",
|
||||||
|
"list",
|
||||||
|
"--json",
|
||||||
|
])
|
||||||
|
.assert()
|
||||||
|
.success()
|
||||||
|
.stdout(contains(r#""auth_status": "unknown""#));
|
||||||
|
server.verify().await;
|
||||||
|
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
#[cfg(target_os = "macos")]
|
#[cfg(target_os = "macos")]
|
||||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||||
async fn list_with_macos_proxy_resolution_does_not_panic() -> Result<()> {
|
async fn list_with_macos_proxy_resolution_does_not_panic() -> Result<()> {
|
||||||
|
|||||||
@@ -62,6 +62,7 @@ pub(super) fn mcp_startup_failure_reason(
|
|||||||
}
|
}
|
||||||
Some(
|
Some(
|
||||||
McpAuthState::Unsupported
|
McpAuthState::Unsupported
|
||||||
|
| McpAuthState::Unknown
|
||||||
| McpAuthState::LoggedOut(McpLoginRequirement::Login)
|
| McpAuthState::LoggedOut(McpLoginRequirement::Login)
|
||||||
| McpAuthState::BearerToken
|
| McpAuthState::BearerToken
|
||||||
| McpAuthState::OAuth,
|
| McpAuthState::OAuth,
|
||||||
|
|||||||
@@ -190,7 +190,7 @@ where
|
|||||||
Ok(status) => status,
|
Ok(status) => status,
|
||||||
Err(error) => {
|
Err(error) => {
|
||||||
warn!("failed to determine auth status for MCP server `{name}`: {error:?}");
|
warn!("failed to determine auth status for MCP server `{name}`: {error:?}");
|
||||||
McpAuthState::Unsupported
|
McpAuthState::Unknown
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
let entry = McpAuthStatusEntry {
|
let entry = McpAuthStatusEntry {
|
||||||
|
|||||||
@@ -3770,6 +3770,7 @@ pub struct McpStartupFailure {
|
|||||||
#[serde(rename_all = "snake_case")]
|
#[serde(rename_all = "snake_case")]
|
||||||
#[ts(rename_all = "snake_case")]
|
#[ts(rename_all = "snake_case")]
|
||||||
pub enum McpAuthStatus {
|
pub enum McpAuthStatus {
|
||||||
|
Unknown,
|
||||||
Unsupported,
|
Unsupported,
|
||||||
NotLoggedIn,
|
NotLoggedIn,
|
||||||
BearerToken,
|
BearerToken,
|
||||||
@@ -3779,6 +3780,7 @@ pub enum McpAuthStatus {
|
|||||||
impl fmt::Display for McpAuthStatus {
|
impl fmt::Display for McpAuthStatus {
|
||||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||||
let text = match self {
|
let text = match self {
|
||||||
|
McpAuthStatus::Unknown => "Unknown",
|
||||||
McpAuthStatus::Unsupported => "Unsupported",
|
McpAuthStatus::Unsupported => "Unsupported",
|
||||||
McpAuthStatus::NotLoggedIn => "Not logged in",
|
McpAuthStatus::NotLoggedIn => "Not logged in",
|
||||||
McpAuthStatus::BearerToken => "Bearer token",
|
McpAuthStatus::BearerToken => "Bearer token",
|
||||||
|
|||||||
@@ -49,6 +49,7 @@ pub enum McpLoginRequirement {
|
|||||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||||
pub enum McpAuthState {
|
pub enum McpAuthState {
|
||||||
Unsupported,
|
Unsupported,
|
||||||
|
Unknown,
|
||||||
LoggedOut(McpLoginRequirement),
|
LoggedOut(McpLoginRequirement),
|
||||||
BearerToken,
|
BearerToken,
|
||||||
OAuth,
|
OAuth,
|
||||||
@@ -58,6 +59,7 @@ impl From<McpAuthState> for McpAuthStatus {
|
|||||||
fn from(value: McpAuthState) -> Self {
|
fn from(value: McpAuthState) -> Self {
|
||||||
match value {
|
match value {
|
||||||
McpAuthState::Unsupported => Self::Unsupported,
|
McpAuthState::Unsupported => Self::Unsupported,
|
||||||
|
McpAuthState::Unknown => Self::Unknown,
|
||||||
McpAuthState::LoggedOut(_) => Self::NotLoggedIn,
|
McpAuthState::LoggedOut(_) => Self::NotLoggedIn,
|
||||||
McpAuthState::BearerToken => Self::BearerToken,
|
McpAuthState::BearerToken => Self::BearerToken,
|
||||||
McpAuthState::OAuth => Self::OAuth,
|
McpAuthState::OAuth => Self::OAuth,
|
||||||
@@ -180,7 +182,7 @@ fn determine_auth_status_from_discovery(
|
|||||||
debug!(
|
debug!(
|
||||||
"failed to detect OAuth support for MCP server `{server_name}` at {url}: {error:?}"
|
"failed to detect OAuth support for MCP server `{server_name}` at {url}: {error:?}"
|
||||||
);
|
);
|
||||||
Ok(McpAuthState::Unsupported)
|
Err(error)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -576,7 +578,7 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn oauth_discovery_preserves_transient_http_errors() {
|
async fn determine_auth_status_preserves_transient_http_errors() {
|
||||||
for status in [
|
for status in [
|
||||||
StatusCode::REQUEST_TIMEOUT,
|
StatusCode::REQUEST_TIMEOUT,
|
||||||
StatusCode::TOO_EARLY,
|
StatusCode::TOO_EARLY,
|
||||||
@@ -590,22 +592,26 @@ mod tests {
|
|||||||
.mount(&server)
|
.mount(&server)
|
||||||
.await;
|
.await;
|
||||||
|
|
||||||
let error = discover_streamable_http_oauth(
|
let error = determine_streamable_http_auth_status(
|
||||||
|
"transient-http-error",
|
||||||
&format!("{}/mcp", server.uri()),
|
&format!("{}/mcp", server.uri()),
|
||||||
|
/*bearer_token_env_var*/ None,
|
||||||
/*http_headers*/ None,
|
/*http_headers*/ None,
|
||||||
/*env_http_headers*/ None,
|
/*env_http_headers*/ None,
|
||||||
|
OAuthCredentialsStoreMode::File,
|
||||||
|
AuthKeyringBackendKind::default(),
|
||||||
test_http_client(),
|
test_http_client(),
|
||||||
OAuthDiscoveryTimeout::LOCAL,
|
OAuthDiscoveryTimeout::LOCAL,
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
.expect_err("transient OAuth discovery failures must not become anonymous access");
|
.expect_err("transient OAuth discovery failures must not become unsupported access");
|
||||||
|
|
||||||
assert!(
|
assert!(
|
||||||
matches!(
|
matches!(
|
||||||
error.downcast_ref::<AuthError>(),
|
error.downcast_ref::<AuthError>(),
|
||||||
Some(AuthError::MetadataError(reason)) if reason.contains(status.as_str())
|
Some(AuthError::MetadataError(reason)) if reason.contains(status.as_str())
|
||||||
),
|
),
|
||||||
"OAuth discovery must preserve HTTP {status}: {error:#}"
|
"auth-status discovery must preserve HTTP {status}: {error:#}"
|
||||||
);
|
);
|
||||||
server.verify().await;
|
server.verify().await;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1263,6 +1263,7 @@ pub(super) fn mcp_inventory_maps_from_statuses(statuses: Vec<McpServerStatus>) -
|
|||||||
auth_statuses.insert(
|
auth_statuses.insert(
|
||||||
server_name.clone(),
|
server_name.clone(),
|
||||||
match status.auth_status {
|
match status.auth_status {
|
||||||
|
codex_app_server_protocol::McpAuthStatus::Unknown => McpAuthStatus::Unknown,
|
||||||
codex_app_server_protocol::McpAuthStatus::Unsupported => McpAuthStatus::Unsupported,
|
codex_app_server_protocol::McpAuthStatus::Unsupported => McpAuthStatus::Unsupported,
|
||||||
codex_app_server_protocol::McpAuthStatus::NotLoggedIn => McpAuthStatus::NotLoggedIn,
|
codex_app_server_protocol::McpAuthStatus::NotLoggedIn => McpAuthStatus::NotLoggedIn,
|
||||||
codex_app_server_protocol::McpAuthStatus::BearerToken => McpAuthStatus::BearerToken,
|
codex_app_server_protocol::McpAuthStatus::BearerToken => McpAuthStatus::BearerToken,
|
||||||
|
|||||||
@@ -16,6 +16,7 @@ impl HistoryCell for McpImageOutputCell {
|
|||||||
}
|
}
|
||||||
fn mcp_auth_status_label(status: McpAuthStatus) -> &'static str {
|
fn mcp_auth_status_label(status: McpAuthStatus) -> &'static str {
|
||||||
match status {
|
match status {
|
||||||
|
McpAuthStatus::Unknown => "Unknown",
|
||||||
McpAuthStatus::Unsupported => "Unsupported",
|
McpAuthStatus::Unsupported => "Unsupported",
|
||||||
McpAuthStatus::NotLoggedIn => "Not logged in",
|
McpAuthStatus::NotLoggedIn => "Not logged in",
|
||||||
McpAuthStatus::BearerToken => "Bearer token",
|
McpAuthStatus::BearerToken => "Bearer token",
|
||||||
@@ -545,6 +546,7 @@ pub(crate) fn new_mcp_tools_output_from_statuses(
|
|||||||
|
|
||||||
lines.push(header.into());
|
lines.push(header.into());
|
||||||
let auth_status = match status.auth_status {
|
let auth_status = match status.auth_status {
|
||||||
|
codex_app_server_protocol::McpAuthStatus::Unknown => McpAuthStatus::Unknown,
|
||||||
codex_app_server_protocol::McpAuthStatus::Unsupported => McpAuthStatus::Unsupported,
|
codex_app_server_protocol::McpAuthStatus::Unsupported => McpAuthStatus::Unsupported,
|
||||||
codex_app_server_protocol::McpAuthStatus::NotLoggedIn => McpAuthStatus::NotLoggedIn,
|
codex_app_server_protocol::McpAuthStatus::NotLoggedIn => McpAuthStatus::NotLoggedIn,
|
||||||
codex_app_server_protocol::McpAuthStatus::BearerToken => McpAuthStatus::BearerToken,
|
codex_app_server_protocol::McpAuthStatus::BearerToken => McpAuthStatus::BearerToken,
|
||||||
|
|||||||
+1
-1
@@ -7,5 +7,5 @@ expression: rendered
|
|||||||
🔌 MCP Tools
|
🔌 MCP Tools
|
||||||
|
|
||||||
• plugin_docs
|
• plugin_docs
|
||||||
• Auth: Unsupported
|
• Auth: Unknown
|
||||||
• Tools: lookup
|
• Tools: lookup
|
||||||
@@ -980,7 +980,7 @@ fn mcp_tools_output_from_statuses_renders_status_only_servers() {
|
|||||||
)]),
|
)]),
|
||||||
resources: Vec::new(),
|
resources: Vec::new(),
|
||||||
resource_templates: Vec::new(),
|
resource_templates: Vec::new(),
|
||||||
auth_status: codex_app_server_protocol::McpAuthStatus::Unsupported,
|
auth_status: codex_app_server_protocol::McpAuthStatus::Unknown,
|
||||||
}];
|
}];
|
||||||
|
|
||||||
let cell =
|
let cell =
|
||||||
|
|||||||
Reference in new issue
Block a user