Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions packages/agent-core-v2/src/mcpCore/config-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@
* Owns the `McpServerConfig` schema and its transport variants. These describe
* the shape of MCP server entries as they appear in configuration (whether in
* `config.toml` or an MCP-specific config file).
*
* Remote variants accept `auth: "oauth"`, mirroring v1: OAuth is still
* discovered from a remote server's 401 response; the flag records that the
* user explicitly chose OAuth, so static `headers` on the same entry are
* treated as plain request headers (capability/identity declarations) rather
* than as the server's credentials.
*/

import { z } from 'zod';
Expand Down Expand Up @@ -37,6 +43,7 @@ export const McpServerHttpConfigSchema = z.object({
transport: z.literal('http'),
url: z.string().url(),
headers: StringRecordSchema.optional(),
auth: z.literal('oauth').optional(),
bearerTokenEnvVar: z.string().min(1).optional(),
...McpServerCommonFields,
});
Expand All @@ -47,6 +54,7 @@ export const McpServerSseConfigSchema = z.object({
transport: z.literal('sse'),
url: z.string().url(),
headers: StringRecordSchema.optional(),
auth: z.literal('oauth').optional(),
bearerTokenEnvVar: z.string().min(1).optional(),
...McpServerCommonFields,
});
Expand Down
2 changes: 1 addition & 1 deletion packages/agent-core-v2/src/mcpCore/connection-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ export class McpConnectionManager implements McpConnectionView {
if (this.oauthService === undefined) return false;
if (!isRemoteMcpConfig(entry.config)) return false;
if (entry.config.bearerTokenEnvVar !== undefined) return false;
if (entry.config.headers !== undefined) return false;
if (entry.config.headers !== undefined && entry.config.auth !== 'oauth') return false;
return isUnauthorizedLikeError(error);
Comment on lines +413 to 414

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reclassify auth-expiry disconnects as needs-auth

This only changes the initial 401 path. If an auth: 'oauth' server is already connected and later its refresh token expires, the SDK reports the failure through onUnexpectedClose, and watchForUnexpectedClose() still hard-codes failed, so the entry never re-enters needs-auth and /mcp-config login cannot recover it.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Real limitation, but not a v1/v2 gap: v1's watchForUnexpectedClose hard-codes failed identically (packages/agent-core/src/mcp/connection-manager.ts, same shape), so reclassifying mid-session auth-expiry disconnects would take v2 beyond the v1 behavior this PR ports — and the fix belongs in both engines. Recovery does exist today: the next reconnect hits the startup 401 path, which this PR flips to needs-auth. Happy to file a shared-engine follow-up for the mid-session classification if maintainers want it.

}

Expand Down
59 changes: 59 additions & 0 deletions packages/agent-core-v2/test/mcpCore/connection-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,65 @@ describe('McpConnectionManager', () => {
}
}, 15000);

it('marks an explicitly OAuth HTTP server as needs-auth when non-auth headers accompany a 401', async () => {
const server: HttpServer = createHttpServer((_req, res) => {
res.writeHead(401, {
'content-type': 'application/json',
'www-authenticate':
'Bearer realm="mcp", resource_metadata="http://x/.well-known/oauth-protected-resource"',
});
res.end(JSON.stringify({ error: 'unauthorized' }));
});
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
const port = (server.address() as HttpAddress).port;
const oauthService = new McpOAuthService({ store: createMemoryMcpOAuthStore() });
const cm = new McpConnectionManager({ oauthService });
try {
await cm.connectAll({
gated: {
transport: 'http',
url: `http://127.0.0.1:${port}/mcp`,
headers: { 'X-Tenant': 'example' },
auth: 'oauth',
startupTimeoutMs: 5_000,
},
});
const entry = cm.get('gated');
expect(entry?.status).toBe('needs-auth');
expect(entry?.error).toContain('run /mcp-config login gated');
expect(entry?.toolCount).toBe(0);
} finally {
await cm.shutdown();
await closeServer(server);
}
}, 15000);

it('keeps a headers-only HTTP server failed (not needs-auth) on 401', async () => {
const server: HttpServer = createHttpServer((_req, res) => {
res.writeHead(401, { 'content-type': 'application/json' });
res.end(JSON.stringify({ error: 'unauthorized' }));
});
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
const port = (server.address() as HttpAddress).port;
const oauthService = new McpOAuthService({ store: createMemoryMcpOAuthStore() });
const cm = new McpConnectionManager({ oauthService });
try {
await cm.connectAll({
keyed: {
transport: 'http',
url: `http://127.0.0.1:${port}/mcp`,
headers: { Authorization: 'Bearer static-key' },
startupTimeoutMs: 5_000,
},
});
const entry = cm.get('keyed');
expect(entry?.status).toBe('failed');
} finally {
await cm.shutdown();
await closeServer(server);
}
}, 15000);

it('flips SSE servers into needs-auth when the server returns 401 and no static token is set', async () => {
const server: HttpServer = createHttpServer((_req, res) => {
res.writeHead(401, {
Expand Down
Loading