fix(mcp): inject stored OAuth tokens into ACP agent MCP servers (#445)

## Summary
🤖 Generated with [Nori](https://www.npmjs.com/package/nori-ai)

- **Fix OAuth tokens not reaching ACP agent:** Stored OAuth tokens (from
browser login via keyring or `.credentials.json`) are now loaded and
injected as `Authorization: Bearer` headers when converting MCP server
configs to SACP protocol types. Previously only `bearer_token_env_var`
was checked — tokens from OAuth login were stored but never sent to the
agent.
- **Fix disabled servers being sent:** MCP servers with `enabled ==
false` are now filtered out of the SACP conversion instead of being sent
to the agent.
- **Fix OAuth branding:** DCR client name changed from "Codex" to "Nori"
so OAuth login pages show the correct app name. Keyring service name
changed to "Nori TUI MCP Credentials".

## Test Plan
- [x] Unit tests for OAuth token injection
(`http_server_with_stored_oauth_tokens_gets_auth_header`)
- [x] Unit test for bearer token env var precedence over stored OAuth
(`bearer_token_env_var_takes_precedence_over_stored_oauth`)
- [x] Unit test for disabled server filtering
(`disabled_servers_are_excluded`)
- [x] TUI verification via tmux: `/mcp` picker renders, HTTP server can
be added, config is persisted to `config.toml`
- [ ] Manual test: Add Linear MCP server, complete OAuth login, restart
session, verify agent can use Linear tools

Share Nori with your team: https://www.npmjs.com/package/nori-skillsets

Co-authored-by: Nori <contact@tilework.tech>

Author: theahura

nori-rs/Cargo.lock

@@ -23,6 +23,7 @@ codex-protocol = { workspace = true }
 codex-utils-string = { workspace = true }
 futures = { workspace = true }
 nori-protocol = { workspace = true }
+oauth2 = "5"
 serde = { workspace = true, features = ["derive"] }
 serde_json = { workspace = true }
 thiserror = { workspace = true }
@@ -44,6 +45,7 @@ libc = { workspace = true }
 [dev-dependencies]
 filetime = "0.2"
 pretty_assertions = { workspace = true }
+rmcp = { workspace = true }
 serial_test = { workspace = true }
 tempfile = { workspace = true }
 tokio-test = { workspace = true }

nori-rs/acp/Cargo.toml

@@ -568,9 +568,16 @@ CLI-configured MCP servers (from `config.toml`) are converted to ACP schema type
 | Transport | SACP Type | Key Fields |
 |-----------|-----------|------------|
 | `Stdio` | `McpServer::Stdio` | command, args, env (explicit key-value pairs + env vars resolved from process environment) |
-| `StreamableHttp` | `McpServer::Http` | url, headers (static headers + env-resolved headers + bearer token from env var as `Authorization: Bearer` header) |
+| `StreamableHttp` | `McpServer::Http` | url, headers (static headers + env-resolved headers + bearer token + OAuth token as `Authorization: Bearer` header) |
 
-Environment variable references (`bearer_token_env_var`, `env_http_headers`, `env_vars`) are resolved eagerly from the current process environment at conversion time. Missing variables are logged as warnings and skipped -- they do not cause errors. The `client_id` and `client_secret_env_var` fields on `StreamableHttp` are not forwarded to the agent -- they are only used by the TUI/rmcp-client layer for OAuth login flows (see `@/nori-rs/rmcp-client/docs.md`). All servers are included regardless of the `enabled` flag; the agent decides how to handle them. Results are sorted by server name for deterministic ordering.
+Disabled servers (`enabled == false`) are filtered out before conversion. Environment variable references (`bearer_token_env_var`, `env_http_headers`, `env_vars`) are resolved eagerly from the current process environment at conversion time. Missing variables are logged as warnings and skipped -- they do not cause errors. The `client_id` and `client_secret_env_var` fields on `StreamableHttp` are not forwarded to the agent -- they are only used by the TUI/rmcp-client layer for OAuth login flows (see `@/nori-rs/rmcp-client/docs.md`). Results are sorted by server name for deterministic ordering.
+
+**OAuth token injection for HTTP servers:** For `StreamableHttp` servers, `to_sacp_mcp_servers()` resolves authentication headers using a priority chain:
+1. `bearer_token_env_var` -- if present and the env var resolves, used as `Authorization: Bearer` header
+2. Stored OAuth tokens -- if no bearer token env var was resolved, `load_oauth_tokens()` from `@/nori-rs/rmcp-client/src/oauth.rs` is called to load credentials from the system keyring or `CODEX_HOME/.credentials.json` fallback file
+3. No auth -- if neither source produces a token, the server is forwarded without an `Authorization` header
+
+This means `to_sacp_mcp_servers()` has side effects (reads from keyring/file system) rather than being a pure config transformation. The `acp` crate depends on `codex-rmcp-client`'s `load_oauth_tokens` for this purpose.
 
 `create_session()` accepts a `mcp_servers: Vec<McpServer>` parameter that is populated by calling `to_sacp_mcp_servers()` at each session creation site:
 - `spawn_and_relay.rs` -- initial session creation during backend spawn

nori-rs/acp/docs.md

@@ -9,21 +9,28 @@ use std::collections::HashMap;
 
 use codex_core::config::types::McpServerConfig;
 use codex_core::config::types::McpServerTransportConfig;
+use codex_rmcp_client::OAuthCredentialsStoreMode;
+use codex_rmcp_client::load_oauth_tokens;
+use oauth2::TokenResponse;
 use sacp::schema as acp;
 use tracing::warn;
 
 /// Convert CLI MCP server configs into SACP protocol `McpServer` values
 /// suitable for inclusion in a `NewSessionRequest`.
 ///
-/// All servers are included regardless of their `enabled` flag — the agent
-/// decides how to handle them.
+/// Disabled servers (`enabled == false`) are excluded.
 ///
 /// Environment variable references (`bearer_token_env_var`, `env_http_headers`,
 /// `env_vars`) are resolved eagerly from the current process environment.
 /// Missing variables are logged as warnings and skipped.
+///
+/// For HTTP servers without a `bearer_token_env_var`, stored OAuth tokens
+/// (from keyring or credential file) are loaded and injected as an
+/// `Authorization: Bearer` header.
 pub fn to_sacp_mcp_servers(servers: &HashMap<String, McpServerConfig>) -> Vec<acp::McpServer> {
     let mut result: Vec<acp::McpServer> = servers
         .iter()
+        .filter(|(_, config)| config.enabled)
         .map(|(name, config)| convert_one(name, config))
         .collect();
     // Sort for deterministic ordering (HashMap iteration is random).
@@ -106,13 +113,15 @@ fn convert_one(name: &str, config: &McpServerConfig) -> acp::McpServer {
             }
 
             // Bearer token from env var → Authorization header.
+            let mut has_bearer = false;
             if let Some(token_env_var) = bearer_token_env_var {
                 match std::env::var(token_env_var) {
                     Ok(token) => {
                         headers.push(acp::HttpHeader::new(
                             "Authorization".to_string(),
                             format!("Bearer {token}"),
                         ));
+                        has_bearer = true;
                     }
                     Err(_) => {
                         warn!(
@@ -122,6 +131,35 @@ fn convert_one(name: &str, config: &McpServerConfig) -> acp::McpServer {
                 }
             }
 
+            // Fall back to stored OAuth tokens if no bearer token env var was resolved.
+            if !has_bearer {
+                match load_oauth_tokens(name, url, OAuthCredentialsStoreMode::Auto) {
+                    Ok(Some(tokens)) => {
+                        if let Some(expires_at) = tokens.expires_at {
+                            let now_ms = std::time::SystemTime::now()
+                                .duration_since(std::time::UNIX_EPOCH)
+                                .unwrap_or_default()
+                                .as_millis() as u64;
+                            if expires_at <= now_ms {
+                                warn!(
+                                    "MCP server '{name}': stored OAuth token has expired; \
+                                     re-authenticate via /mcp"
+                                );
+                            }
+                        }
+                        let access_token = tokens.token_response.0.access_token().secret();
+                        headers.push(acp::HttpHeader::new(
+                            "Authorization".to_string(),
+                            format!("Bearer {access_token}"),
+                        ));
+                    }
+                    Ok(None) => {}
+                    Err(e) => {
+                        warn!("MCP server '{name}': failed to load stored OAuth tokens: {e}");
+                    }
+                }
+            }
+
             acp::McpServer::Http(acp::McpServerHttp::new(name, url.as_str()).headers(headers))
         }
     }
@@ -131,6 +169,7 @@ fn convert_one(name: &str, config: &McpServerConfig) -> acp::McpServer {
 mod tests {
     use super::*;
     use pretty_assertions::assert_eq;
+    use serial_test::serial;
 
     fn stdio_config(
         command: &str,
@@ -309,7 +348,7 @@ mod tests {
     }
 
     #[test]
-    fn disabled_servers_are_included() {
+    fn disabled_servers_are_excluded() {
         let mut servers = HashMap::new();
         servers.insert(
             "disabled-server".to_string(),
@@ -327,9 +366,159 @@ mod tests {
                 disabled_tools: None,
             },
         );
+        servers.insert(
+            "enabled-server".to_string(),
+            stdio_config("echo", vec![], None, vec![]),
+        );
+
+        let result = to_sacp_mcp_servers(&servers);
+        assert_eq!(result.len(), 1);
+        match &result[0] {
+            acp::McpServer::Stdio(s) => {
+                assert_eq!(s.name, "enabled-server");
+            }
+            other => panic!("Expected Stdio, got {other:?}"),
+        }
+    }
+
+    #[test]
+    #[serial]
+    fn http_server_with_stored_oauth_tokens_gets_auth_header() {
+        use codex_rmcp_client::OAuthCredentialsStoreMode;
+        use codex_rmcp_client::StoredOAuthTokens;
+        use codex_rmcp_client::WrappedOAuthTokenResponse;
+        use codex_rmcp_client::save_oauth_tokens;
+        use oauth2::AccessToken;
+        use oauth2::EmptyExtraTokenFields;
+        use oauth2::basic::BasicTokenType;
+        use rmcp::transport::auth::OAuthTokenResponse;
+
+        let tmp_dir = tempfile::tempdir().unwrap();
+        let tmp_path = tmp_dir.path().to_str().unwrap().to_string();
+
+        // Point CODEX_HOME at temp dir so file-based credential storage works
+        let old_codex_home = std::env::var("CODEX_HOME").ok();
+        unsafe { std::env::set_var("CODEX_HOME", &tmp_path) };
+
+        let server_name = "linear";
+        let server_url = "https://mcp.linear.app/mcp";
+
+        // Store OAuth tokens for this server
+        let token_response = OAuthTokenResponse::new(
+            AccessToken::new("test-oauth-access-token".to_string()),
+            BasicTokenType::Bearer,
+            EmptyExtraTokenFields {},
+        );
+        let stored = StoredOAuthTokens {
+            server_name: server_name.to_string(),
+            url: server_url.to_string(),
+            client_id: "test-client-id".to_string(),
+            token_response: WrappedOAuthTokenResponse(token_response),
+            expires_at: None,
+        };
+        save_oauth_tokens(server_name, &stored, OAuthCredentialsStoreMode::File).unwrap();
+
+        // Create an HTTP server config without bearer_token_env_var
+        let mut servers = HashMap::new();
+        servers.insert(
+            server_name.to_string(),
+            http_config(server_url, None, None, None),
+        );
+
+        let result = to_sacp_mcp_servers(&servers);
+        assert_eq!(result.len(), 1);
+
+        match &result[0] {
+            acp::McpServer::Http(s) => {
+                assert_eq!(s.name, "linear");
+                assert_eq!(s.url, server_url);
+                assert_eq!(s.headers.len(), 1);
+                assert_eq!(s.headers[0].name, "Authorization");
+                assert_eq!(s.headers[0].value, "Bearer test-oauth-access-token");
+            }
+            other => panic!("Expected Http, got {other:?}"),
+        }
+
+        // Cleanup
+        match old_codex_home {
+            Some(val) => unsafe { std::env::set_var("CODEX_HOME", val) },
+            None => unsafe { std::env::remove_var("CODEX_HOME") },
+        }
+    }
+
+    #[test]
+    #[serial]
+    fn bearer_token_env_var_takes_precedence_over_stored_oauth() {
+        use codex_rmcp_client::OAuthCredentialsStoreMode;
+        use codex_rmcp_client::StoredOAuthTokens;
+        use codex_rmcp_client::WrappedOAuthTokenResponse;
+        use codex_rmcp_client::save_oauth_tokens;
+        use oauth2::AccessToken;
+        use oauth2::EmptyExtraTokenFields;
+        use oauth2::basic::BasicTokenType;
+        use rmcp::transport::auth::OAuthTokenResponse;
+
+        let tmp_dir = tempfile::tempdir().unwrap();
+        let tmp_path = tmp_dir.path().to_str().unwrap().to_string();
+
+        let old_codex_home = std::env::var("CODEX_HOME").ok();
+        unsafe { std::env::set_var("CODEX_HOME", &tmp_path) };
+
+        let server_name = "my-server";
+        let server_url = "https://mcp.example.com";
+
+        // Store OAuth tokens
+        let token_response = OAuthTokenResponse::new(
+            AccessToken::new("oauth-token-should-be-ignored".to_string()),
+            BasicTokenType::Bearer,
+            EmptyExtraTokenFields {},
+        );
+        let stored = StoredOAuthTokens {
+            server_name: server_name.to_string(),
+            url: server_url.to_string(),
+            client_id: "test-client-id".to_string(),
+            token_response: WrappedOAuthTokenResponse(token_response),
+            expires_at: None,
+        };
+        save_oauth_tokens(server_name, &stored, OAuthCredentialsStoreMode::File).unwrap();
+
+        // Also set a bearer token env var
+        unsafe { std::env::set_var("TEST_PRECEDENCE_TOKEN", "env-var-token-wins") };
+
+        let mut servers = HashMap::new();
+        servers.insert(
+            server_name.to_string(),
+            http_config(
+                server_url,
+                Some("TEST_PRECEDENCE_TOKEN".to_string()),
+                None,
+                None,
+            ),
+        );
 
         let result = to_sacp_mcp_servers(&servers);
         assert_eq!(result.len(), 1);
+
+        match &result[0] {
+            acp::McpServer::Http(s) => {
+                // Should have exactly one Authorization header from env var, not OAuth
+                let auth_headers: Vec<_> = s
+                    .headers
+                    .iter()
+                    .filter(|h| h.name == "Authorization")
+                    .collect();
+                assert_eq!(auth_headers.len(), 1);
+                assert_eq!(auth_headers[0].value, "Bearer env-var-token-wins");
+            }
+            other => panic!("Expected Http, got {other:?}"),
+        }
+
+        // Cleanup
+        unsafe { std::env::remove_var("TEST_PRECEDENCE_TOKEN") };
+        match old_codex_home {
+            Some(val) => unsafe { std::env::set_var("CODEX_HOME", val) },
+            None => unsafe { std::env::remove_var("CODEX_HOME") },
+        }
     }
 
     #[test]

nori-rs/acp/src/connection/mcp.rs

@@ -8,7 +8,8 @@ The rmcp-client crate provides a high-level MCP client for connecting to remote
 
 ### How it fits into the larger codebase
 
-Used by `@/nori-rs/core/` (`mcp_connection_manager.rs`) to establish connections to configured MCP servers that provide additional tools to the AI model.
+- Used by `@/nori-rs/core/` (`mcp_connection_manager.rs`) to establish connections to configured MCP servers that provide additional tools to the AI model
+- Used by `@/nori-rs/acp/src/connection/mcp.rs` to load stored OAuth tokens at session creation time, so the ACP agent receives credentials for MCP servers that were authenticated via the OAuth browser flow
 
 ### Core Implementation
 
@@ -23,6 +24,7 @@ Used by `@/nori-rs/core/` (`mcp_connection_manager.rs`) to establish connections
 - Two OAuth login entry points, both backed by the same `wait_for_callback_or_cancel()` mechanism (biased `tokio::select!` between callback, cancel signal, and 5-minute timeout):
   - `perform_oauth_login()` - Blocking/interactive flow that uses `println!` for status and `stdin` Enter for cancellation. Used by CLI contexts where TUI suspension is acceptable.
   - `start_oauth_login()` - Non-blocking async flow that returns an `OAuthLoginHandle`. The handle exposes a `cancel_tx: Option<oneshot::Sender<()>>` for programmatic cancellation and a `task: JoinHandle<Result<()>>` for awaiting completion. Used by the TUI to run OAuth inline without suspending the terminal. Accepts optional `client_id` and `client_secret` parameters to select between two OAuth paths (see below).
+- Dynamic client registration (DCR) uses `"Nori"` as the client name presented to OAuth servers
 - Token refresh handling via `OAuthPersistor`, which is called after every MCP request
 
 **Two OAuth Credential Paths** (`perform_oauth_login.rs`):
@@ -43,8 +45,11 @@ The pre-configured path uses `discover_oauth_metadata()` to fetch the server's `
 **Program Resolution** (`program_resolver.rs`): Resolves MCP server executables from configuration.
 
 **Credential Storage** (`oauth.rs`):
-- `OAuthCredentialsStoreMode` - Keyring vs file storage
-- `save_oauth_tokens()` / `delete_oauth_tokens()` - Credential management
+- `OAuthCredentialsStoreMode` - Keyring vs file storage (`Auto`, `File`, `Keyring`)
+- `save_oauth_tokens()` / `load_oauth_tokens()` / `delete_oauth_tokens()` - Credential management
+- `load_oauth_tokens` is public and used by `@/codex-rs/acp/src/connection/mcp.rs` to inject stored OAuth tokens when forwarding MCP server configs to ACP agents
+- Keyring service name: `"Nori TUI MCP Credentials"`. Credentials stored under the previous service name are not migrated
+- Fallback file: `CODEX_HOME/.credentials.json` (used when keyring is unavailable or fails)
 
 ### Things to Know
 

nori-rs/rmcp-client/docs.md

@@ -14,7 +14,7 @@ pub use oauth::OAuthCredentialsStoreMode;
 pub use oauth::StoredOAuthTokens;
 pub use oauth::WrappedOAuthTokenResponse;
 pub use oauth::delete_oauth_tokens;
-pub(crate) use oauth::load_oauth_tokens;
+pub use oauth::load_oauth_tokens;
 pub use oauth::save_oauth_tokens;
 pub use perform_oauth_login::OAuthLoginHandle;
 pub use perform_oauth_login::perform_oauth_login;

nori-rs/rmcp-client/src/lib.rs

@@ -49,7 +49,7 @@ use tokio::sync::Mutex;
 
 use crate::find_codex_home::find_codex_home;
 
-const KEYRING_SERVICE: &str = "Codex MCP Credentials";
+const KEYRING_SERVICE: &str = "Nori TUI MCP Credentials";
 const REFRESH_SKEW_MILLIS: u64 = 30_000;
 
 #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
@@ -90,7 +90,7 @@ impl PartialEq for WrappedOAuthTokenResponse {
     }
 }
 
-pub(crate) fn load_oauth_tokens(
+pub fn load_oauth_tokens(
     server_name: &str,
     url: &str,
     store_mode: OAuthCredentialsStoreMode,

nori-rs/rmcp-client/src/oauth.rs

@@ -104,7 +104,7 @@ pub async fn start_oauth_login(
         // Dynamic registration path: use OAuthState from rmcp.
         let mut oauth_state = OAuthState::new(&server_url, Some(http_client)).await?;
         oauth_state
-            .start_authorization(&scope_refs, &redirect_uri, Some("Codex"))
+            .start_authorization(&scope_refs, &redirect_uri, Some("Nori"))
             .await?;
         let auth_url = oauth_state.get_authorization_url().await?;
 
@@ -348,7 +348,7 @@ pub async fn perform_oauth_login(
     let mut oauth_state = OAuthState::new(server_url, Some(http_client)).await?;
     let scope_refs: Vec<&str> = scopes.iter().map(String::as_str).collect();
     oauth_state
-        .start_authorization(&scope_refs, &redirect_uri, Some("Codex"))
+        .start_authorization(&scope_refs, &redirect_uri, Some("Nori"))
         .await?;
     let auth_url = oauth_state.get_authorization_url().await?;