From ace73a2b4e0a0f89d8a07e986c18849e4f6423bd Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Thu, 30 Jul 2026 11:52:23 -0400 Subject: [PATCH 1/5] fix(mcp): distinguish configuration from OAuth --- README.md | 6 +- src/bin.ts | 2 +- src/commands/mcp.spec.ts | 63 +++++++++++++++--- src/commands/mcp.ts | 51 +++++++++++++-- src/commands/setup.spec.ts | 36 ++++++++++- src/commands/setup.ts | 19 +++++- src/doctor/checks/mcp.spec.ts | 65 +++++++++++-------- src/doctor/checks/mcp.ts | 31 +++++---- src/doctor/issues.spec.ts | 15 +++-- src/doctor/issues.ts | 3 +- src/doctor/output.spec.ts | 30 +++++++++ src/doctor/output.ts | 10 ++- src/doctor/types.ts | 10 ++- src/lib/constants.ts | 1 + src/lib/mcp-clients.ts | 117 +++++++++++++++++++++++++++++----- src/utils/help-json.ts | 2 +- 16 files changed, 373 insertions(+), 88 deletions(-) diff --git a/README.md b/README.md index 8dfc8638..bebc5083 100644 --- a/README.md +++ b/README.md @@ -92,8 +92,12 @@ Commands: env Manage environment configurations (add, remove, switch, list, claim) doctor Diagnose WorkOS integration issues skills Manage WorkOS skills for coding agents (install, uninstall, list) + mcp Manage the WorkOS MCP server in coding agents + setup Set up WorkOS skills and the MCP server -Skills auto-install to detected coding agents on `workos install` and `workos auth login`. Use `workos skills list` to check status, `workos doctor` to detect stale skills, or `workos doctor --fix` to refresh them in place (constrained to `workos/` and `workos-widgets/`). +`workos setup` installs WorkOS skills and configures the MCP server only after consent. Use `workos skills list` to check skill status, `workos mcp status` to check whether the server definition is configured, or `workos doctor --fix` to refresh stale skills. + +MCP configuration and OAuth authentication are separate states. For Codex, complete or refresh OAuth with `codex mcp login workos` in your normal host shell. The WorkOS CLI does not inspect Codex credentials and cannot prove that OAuth is usable. See the [WorkOS MCP setup and recovery guide](https://workos.com/docs/mcp) for user-global and trusted-project-only configuration. Resource Management: organization (org) Manage organizations diff --git a/src/bin.ts b/src/bin.ts index 79f31fba..0e206108 100644 --- a/src/bin.ts +++ b/src/bin.ts @@ -473,7 +473,7 @@ async function runCli(): Promise { registerSubcommand( yargs, 'install', - 'Add the WorkOS MCP server to detected coding agents', + 'Configure the WorkOS MCP server in detected coding agents', (y) => y.option('agent', { alias: 'a', diff --git a/src/commands/mcp.spec.ts b/src/commands/mcp.spec.ts index 500dd36b..3a0d0bf7 100644 --- a/src/commands/mcp.spec.ts +++ b/src/commands/mcp.spec.ts @@ -17,7 +17,7 @@ vi.mock('../utils/ui.js', () => { }; return { default: { - log: { info: record, success: record, error: record, warn: record, step: record, message: record }, + log: { info: record, success: record, error: record, warn: record, hint: record, step: record, message: record }, }, }; }); @@ -200,9 +200,22 @@ describe('Claude Code client', () => { describe('Codex client', () => { const codex = () => clientByKey('codex'); - it('add maps a clean exit to installed', async () => { + it('add maps a clean exit to configured with unverified OAuth metadata', async () => { mockExec(() => ({ status: 0 })); - expect((await codex().add()).outcome).toBe('installed'); + const result = await codex().add(); + expect(result).toMatchObject({ + outcome: 'installed', + configuration: { scope: 'user', authentication: 'unknown' }, + recovery: { + docsUrl: 'https://workos.com/docs/mcp', + hints: expect.arrayContaining([ + expect.objectContaining({ + command: 'codex mcp login workos', + hostShellRequired: true, + }), + ]), + }, + }); }); it('add maps a version gap (unknown --url) to failed', async () => { @@ -212,7 +225,7 @@ describe('Codex client', () => { expect(res.error).toContain('--url'); }); - it('add reports installed when a non-zero exit still persisted the server (OAuth-flow timeout)', async () => { + it('add reports configured with host action required when OAuth times out after persistence', async () => { // Codex writes the config, then its post-add OAuth flow blocks and times // out with a non-zero status — but `mcp list` proves the server landed. mockExec((_c, args) => { @@ -220,7 +233,10 @@ describe('Codex client', () => { if (args.includes('list')) return { status: 0, stdout: 'workos https://mcp.workos.com/mcp enabled' }; return { status: 0 }; }); - expect((await codex().add()).outcome).toBe('installed'); + expect(await codex().add()).toMatchObject({ + outcome: 'installed', + configuration: { scope: 'user', authentication: 'action-required' }, + }); }); it('add stays failed on a non-zero exit when the server did NOT land', async () => { @@ -240,6 +256,28 @@ describe('Codex client', () => { expect(await codex().isInstalled()).toBe(true); }); + it('reads the effective Codex MCP URL from get --json', async () => { + mockExec((_c, args) => { + if (args.includes('get')) { + return { + status: 0, + stdout: JSON.stringify({ + name: 'workos', + transport: { type: 'streamable_http', url: 'https://mcp.workos.com/mcp' }, + }), + }; + } + return { status: 0 }; + }); + + expect(await codex().getConfiguredUrl()).toBe('https://mcp.workos.com/mcp'); + }); + + it('returns null when Codex get output is unavailable or malformed', async () => { + mockExec(() => ({ status: 0, stdout: 'not-json' })); + expect(await codex().getConfiguredUrl()).toBeNull(); + }); + it('remove maps "No MCP server named" (exit 0) to not-installed', async () => { mockExec(() => ({ status: 0, stdout: "No MCP server named 'workos' found." })); expect((await codex().remove()).outcome).toBe('not-installed'); @@ -330,6 +368,9 @@ describe('runMcpInstall / runMcpRemove (human mode)', () => { expect(joined).toContain('Claude Code'); expect(joined).toContain('Codex'); expect(joined).toContain('Cursor'); + expect(joined).toContain('Codex: configured (user scope)'); + expect(joined).toContain('codex mcp login workos'); + expect(joined).toContain('https://workos.com/docs/mcp'); }); it('prints the no-agents message and exits 0 when none are detected', async () => { @@ -421,7 +462,7 @@ describe('JSON output mode', () => { expect(output.data.agents[0].outcome).toBe('removed'); }); - it('runMcpStatus emits { data: { agents: [...] } } with availability + install flags', async () => { + it('runMcpStatus distinguishes configured state while retaining the legacy installed flag', async () => { makeDir('.cursor'); writeCursorConfig('{ "mcpServers": { "workos": { "url": "https://w" } } }'); mockExec((_c, args) => (args[0] === '--version' ? { status: 1 } : { status: 0 })); @@ -429,8 +470,14 @@ describe('JSON output mode', () => { const output = JSON.parse(consoleOutput[0]); expect(output.data.agents).toHaveLength(3); const cursor = output.data.agents.find((a: { agent: string }) => a.agent === 'cursor'); - expect(cursor).toMatchObject({ agent: 'cursor', displayName: 'Cursor', available: true, installed: true }); + expect(cursor).toMatchObject({ + agent: 'cursor', + displayName: 'Cursor', + available: true, + configured: true, + installed: true, + }); const claude = output.data.agents.find((a: { agent: string }) => a.agent === 'claude-code'); - expect(claude).toMatchObject({ available: false, installed: false }); + expect(claude).toMatchObject({ available: false, configured: false, installed: false }); }); }); diff --git a/src/commands/mcp.ts b/src/commands/mcp.ts index 30e3f055..6348425a 100644 --- a/src/commands/mcp.ts +++ b/src/commands/mcp.ts @@ -9,6 +9,7 @@ import { type McpAgentKey, type McpClientResult, } from '../lib/mcp-clients.js'; +import { MCP_DOCS_URL, MCP_SERVER_URL } from '../lib/constants.js'; /** * `workos mcp install | remove | status` handlers. @@ -58,7 +59,8 @@ function reportResults(message: string, results: McpClientResult[]): void { outputSuccess(message, { agents: results }); } else { for (const r of results) { - const line = `${r.displayName}: ${MCP_OUTCOME_LABELS[r.outcome]}`; + const scope = r.configuration ? ` (${r.configuration.scope} scope)` : ''; + const line = `${r.displayName}: ${MCP_OUTCOME_LABELS[r.outcome]}${scope}`; if (r.outcome === 'failed') { ui.log.error(r.error ? `${line} — ${r.error}` : line); } else if (r.outcome === 'installed' || r.outcome === 'removed' || r.outcome === 'already-installed') { @@ -67,6 +69,7 @@ function reportResults(message: string, results: McpClientResult[]): void { ui.log.info(line); } } + reportRecovery(results); } if (results.some((r) => r.outcome === 'failed')) { @@ -74,6 +77,21 @@ function reportResults(message: string, results: McpClientResult[]): void { } } +function reportRecovery(results: McpClientResult[]): void { + for (const result of results) { + if (!result.recovery || !result.configuration) continue; + if (result.configuration.authentication === 'action-required') { + ui.log.warn(`${result.displayName}: configuration was written, but OAuth did not complete in this process.`); + } else { + ui.log.info(`${result.displayName}: configuration does not prove that OAuth is complete.`); + } + for (const hint of result.recovery.hints) { + ui.log.hint(hint.command ? `${hint.description}: ${hint.command}` : hint.description); + } + ui.log.hint(`Setup and recovery guide: ${result.recovery.docsUrl}`); + } +} + export async function runMcpInstall(options: McpCommandOptions = {}): Promise { const filter = resolveAgentFilter(options.agent); const clients = await detectMcpClients(filter); @@ -110,8 +128,21 @@ export async function runMcpStatus(): Promise { const agents = await Promise.all( clients.map(async (client) => { const available = await client.isAvailable(); - const installed = available ? await client.isInstalled() : false; - return { agent: client.key, displayName: client.displayName, available, installed }; + const configured = available ? await client.isInstalled() : false; + const configuredUrl = configured ? await client.getConfiguredUrl() : null; + const endpointValid = configuredUrl === null ? null : configuredUrl === MCP_SERVER_URL; + const authentication = client.key === 'codex' && configured ? 'not-verified' : null; + return { + agent: client.key, + displayName: client.displayName, + available, + configured, + // Retained for compatibility with existing JSON consumers. + installed: configured, + configuredUrl, + endpointValid, + authentication, + }; }), ); @@ -121,7 +152,17 @@ export async function runMcpStatus(): Promise { } outputTable( - [{ header: 'Agent' }, { header: 'Available' }, { header: 'Installed' }], - agents.map((a) => [a.displayName, a.available ? 'yes' : 'no', a.installed ? 'yes' : 'no']), + [{ header: 'Agent' }, { header: 'Available' }, { header: 'Configured' }, { header: 'Authentication' }], + agents.map((a) => [ + a.displayName, + a.available ? 'yes' : 'no', + a.configured ? 'yes' : 'no', + a.configured ? (a.authentication ?? 'client-managed') : '—', + ]), ); + + if (agents.some((agent) => agent.authentication === 'not-verified')) { + ui.log.hint(`Codex OAuth is not verified by this command. Run: codex mcp login workos`); + ui.log.hint(`Setup and recovery guide: ${MCP_DOCS_URL}`); + } } diff --git a/src/commands/setup.spec.ts b/src/commands/setup.spec.ts index 66f07eed..28593d6b 100644 --- a/src/commands/setup.spec.ts +++ b/src/commands/setup.spec.ts @@ -54,10 +54,10 @@ vi.mock('../lib/mcp-clients.js', () => ({ detectMcpClients: vi.fn(), MCP_AGENT_KEYS: ['claude-code', 'codex', 'cursor'], MCP_OUTCOME_LABELS: { - installed: 'installed', - 'already-installed': 'already installed', + installed: 'configured', + 'already-installed': 'already configured', removed: 'removed', - 'not-installed': 'not installed', + 'not-installed': 'not configured', skipped: 'skipped', failed: 'failed', }, @@ -329,6 +329,36 @@ describe('runSetup — command trigger', () => { expect(target.add).toHaveBeenCalledOnce(); }); + it('reports structured Codex OAuth recovery after configuration', async () => { + const target = mcpTarget({ + key: 'codex', + displayName: 'Codex', + add: vi.fn(async () => ({ + agent: 'codex', + displayName: 'Codex', + outcome: 'installed', + configuration: { scope: 'user', authentication: 'action-required' }, + recovery: { + docsUrl: 'https://workos.com/docs/mcp', + hints: [ + { + description: 'Complete or refresh WorkOS OAuth in your normal host shell', + command: 'codex mcp login workos', + hostShellRequired: true, + }, + ], + }, + })), + }); + vi.mocked(detectMcpClients).mockResolvedValue([target as any]); + + await runSetup({ trigger: 'command', mcpOnly: true, assumeYes: true }); + + expect(ui.log.success).toHaveBeenCalledWith('MCP server: Codex — configured (user scope)'); + expect(ui.log.hint).toHaveBeenCalledWith(expect.stringContaining('codex mcp login workos')); + expect(ui.log.hint).toHaveBeenCalledWith(expect.stringContaining('https://workos.com/docs/mcp')); + }); + it('rejects unknown --agents values', async () => { await expect(runSetup({ trigger: 'command', agents: ['bogus'] })).rejects.toThrow('exit:unknown_agent'); }); diff --git a/src/commands/setup.ts b/src/commands/setup.ts index ab863608..668e858a 100644 --- a/src/commands/setup.ts +++ b/src/commands/setup.ts @@ -224,15 +224,28 @@ function reportResults(skills: SkillSummary | null, mcp: McpClientResult[]): voi ui.log.success(`${skills.count} ${word} installed for ${skills.agents.join(', ')}`); } for (const r of mcp) { - const line = `MCP server: ${r.displayName} — ${MCP_OUTCOME_LABELS[r.outcome]}`; + const scope = r.configuration ? ` (${r.configuration.scope} scope)` : ''; + const line = `MCP server: ${r.displayName} — ${MCP_OUTCOME_LABELS[r.outcome]}${scope}`; if (r.outcome === 'failed') { ui.log.error(r.error ? `${line} (${r.error})` : line); } else { ui.log.success(line); } } - if (mcp.some((r) => r.outcome === 'installed' || r.outcome === 'already-installed')) { - ui.log.hint('Your agent will authorize WorkOS via OAuth on first MCP use.'); + for (const result of mcp) { + if (!result.recovery || !result.configuration) continue; + if (result.configuration.authentication === 'action-required') { + ui.log.hint(`${result.displayName} configuration was written, but OAuth still requires host-shell action.`); + } else { + ui.log.hint(`${result.displayName} configuration does not prove that OAuth is complete.`); + } + for (const hint of result.recovery.hints) { + ui.log.hint(hint.command ? `${hint.description}: ${hint.command}` : hint.description); + } + ui.log.hint(`Setup and recovery guide: ${result.recovery.docsUrl}`); + } + if (mcp.some((r) => (r.outcome === 'installed' || r.outcome === 'already-installed') && r.recovery === undefined)) { + ui.log.hint('Complete WorkOS OAuth in your agent before using the MCP server.'); } } diff --git a/src/doctor/checks/mcp.spec.ts b/src/doctor/checks/mcp.spec.ts index a59d0a1f..cb2c5c2d 100644 --- a/src/doctor/checks/mcp.spec.ts +++ b/src/doctor/checks/mcp.spec.ts @@ -1,24 +1,22 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; let detectResult: unknown[] = []; -let cursorUrl: string | null = null; const detectMcpClientsMock = vi.fn(() => Promise.resolve(detectResult)); -const getCursorConfiguredUrlMock = vi.fn(() => Promise.resolve(cursorUrl)); vi.mock('../../lib/mcp-clients.js', () => ({ detectMcpClients: (...a: unknown[]) => detectMcpClientsMock(...(a as [])), - getCursorConfiguredUrl: (...a: unknown[]) => getCursorConfiguredUrlMock(...(a as [])), })); -const { MCP_SERVER_URL } = await import('../../lib/constants.js'); +const { MCP_DOCS_URL, MCP_SERVER_URL } = await import('../../lib/constants.js'); const { checkMcp } = await import('./mcp.js'); -function fakeClient(key: string, displayName: string, installed: boolean) { +function fakeClient(key: string, displayName: string, configured: boolean, configuredUrl: string | null = null) { return { key, displayName, isAvailable: vi.fn(() => Promise.resolve(true)), - isInstalled: vi.fn(() => Promise.resolve(installed)), + isInstalled: vi.fn(() => Promise.resolve(configured)), + getConfiguredUrl: vi.fn(() => Promise.resolve(configuredUrl)), add: vi.fn(), remove: vi.fn(), }; @@ -27,9 +25,7 @@ function fakeClient(key: string, displayName: string, installed: boolean) { beforeEach(() => { vi.clearAllMocks(); detectResult = []; - cursorUrl = null; detectMcpClientsMock.mockImplementation(() => Promise.resolve(detectResult)); - getCursorConfiguredUrlMock.mockImplementation(() => Promise.resolve(cursorUrl)); }); describe('checkMcp', () => { @@ -38,32 +34,37 @@ describe('checkMcp', () => { expect(await checkMcp()).toBeNull(); }); - it('reports available + installed per detected agent, with the server URL', async () => { + it('reports available + configured per detected agent, with the server and docs URLs', async () => { detectResult = [fakeClient('claude-code', 'Claude Code', true), fakeClient('codex', 'Codex', false)]; const result = await checkMcp(); expect(result).toEqual({ serverUrl: MCP_SERVER_URL, + docsUrl: MCP_DOCS_URL, agents: [ - { agent: 'Claude Code', available: true, installed: true }, - { agent: 'Codex', available: true, installed: false }, + { agent: 'Claude Code', available: true, configured: true, installed: true, misconfigured: false }, + { agent: 'Codex', available: true, configured: false, installed: false }, ], }); }); it('flags a Cursor entry with an unexpected URL as misconfigured', async () => { - detectResult = [fakeClient('cursor', 'Cursor', true)]; - cursorUrl = 'https://evil.example.com/mcp'; + detectResult = [fakeClient('cursor', 'Cursor', true, 'https://evil.example.com/mcp')]; const result = await checkMcp(); - expect(result!.agents[0]).toEqual({ agent: 'Cursor', available: true, installed: true, misconfigured: true }); + expect(result!.agents[0]).toEqual({ + agent: 'Cursor', + available: true, + configured: true, + installed: true, + misconfigured: true, + }); }); it('does not flag Cursor when the configured URL matches', async () => { - detectResult = [fakeClient('cursor', 'Cursor', true)]; - cursorUrl = MCP_SERVER_URL; + detectResult = [fakeClient('cursor', 'Cursor', true, MCP_SERVER_URL)]; const result = await checkMcp(); @@ -72,7 +73,6 @@ describe('checkMcp', () => { it('does not flag Cursor when the URL cannot be read (null)', async () => { detectResult = [fakeClient('cursor', 'Cursor', true)]; - cursorUrl = null; const result = await checkMcp(); @@ -80,28 +80,43 @@ describe('checkMcp', () => { }); it('never reads the URL (nor flags) when Cursor lacks the server', async () => { - detectResult = [fakeClient('cursor', 'Cursor', false)]; + const cursor = fakeClient('cursor', 'Cursor', false); + detectResult = [cursor]; const result = await checkMcp(); expect(result!.agents[0].misconfigured).toBeUndefined(); - expect(getCursorConfiguredUrlMock).not.toHaveBeenCalled(); + expect(cursor.getConfiguredUrl).not.toHaveBeenCalled(); + }); + + it('validates the Codex URL and marks OAuth as not verified', async () => { + detectResult = [fakeClient('codex', 'Codex', true, 'https://wrong.example.com/mcp')]; + + const result = await checkMcp(); + + expect(result!.agents[0]).toEqual({ + agent: 'Codex', + available: true, + configured: true, + installed: true, + misconfigured: true, + authentication: 'not-verified', + }); }); - it('handles the mixed matrix: one installed, one missing, one misconfigured', async () => { + it('handles the mixed matrix: one configured, one missing, one misconfigured', async () => { detectResult = [ fakeClient('claude-code', 'Claude Code', true), fakeClient('codex', 'Codex', false), - fakeClient('cursor', 'Cursor', true), + fakeClient('cursor', 'Cursor', true, 'https://stale.example.com/mcp'), ]; - cursorUrl = 'https://stale.example.com/mcp'; const result = await checkMcp(); expect(result!.agents).toEqual([ - { agent: 'Claude Code', available: true, installed: true }, - { agent: 'Codex', available: true, installed: false }, - { agent: 'Cursor', available: true, installed: true, misconfigured: true }, + { agent: 'Claude Code', available: true, configured: true, installed: true, misconfigured: false }, + { agent: 'Codex', available: true, configured: false, installed: false }, + { agent: 'Cursor', available: true, configured: true, installed: true, misconfigured: true }, ]); }); }); diff --git a/src/doctor/checks/mcp.ts b/src/doctor/checks/mcp.ts index 84f438e2..549dfe1c 100644 --- a/src/doctor/checks/mcp.ts +++ b/src/doctor/checks/mcp.ts @@ -1,5 +1,5 @@ -import { detectMcpClients, getCursorConfiguredUrl } from '../../lib/mcp-clients.js'; -import { MCP_SERVER_URL } from '../../lib/constants.js'; +import { detectMcpClients } from '../../lib/mcp-clients.js'; +import { MCP_DOCS_URL, MCP_SERVER_URL } from '../../lib/constants.js'; import type { McpInfo, McpAgentMcpStatus } from '../types.js'; /** @@ -11,9 +11,9 @@ import type { McpInfo, McpAgentMcpStatus } from '../types.js'; * touch MCP in this phase). * * Absent MCP is deliberately NOT an issue: the user may have declined the - * offer. Only Cursor exposes a readable config, so URL drift ("misconfigured") - * is detected for Cursor alone; issues.ts derives a warning from it. Per-agent - * shell-outs carry the 10s timeout from the client library, keeping doctor fast. + * offer. URL drift ("misconfigured") is detected when a client exposes its + * effective URL; issues.ts derives a warning from it. Per-agent shell-outs + * carry the 10s timeout from the client library, keeping doctor fast. */ export async function checkMcp(): Promise { const clients = await detectMcpClients(); @@ -21,18 +21,23 @@ export async function checkMcp(): Promise { const agents = await Promise.all( clients.map(async (client): Promise => { - const installed = await client.isInstalled(); - const status: McpAgentMcpStatus = { agent: client.displayName, available: true, installed }; - // Cursor is the only client whose config we can read, so it's the only - // one we can flag for URL drift. A missing/unreadable URL (null) is not - // "unexpected" — only a present-but-different URL counts. - if (client.key === 'cursor' && installed) { - const url = await getCursorConfiguredUrl(); + const configured = await client.isInstalled(); + const status: McpAgentMcpStatus = { + agent: client.displayName, + available: true, + configured, + installed: configured, + }; + if (configured) { + const url = await client.getConfiguredUrl(); status.misconfigured = url !== null && url !== MCP_SERVER_URL; } + if (client.key === 'codex' && configured) { + status.authentication = 'not-verified'; + } return status; }), ); - return { serverUrl: MCP_SERVER_URL, agents }; + return { serverUrl: MCP_SERVER_URL, docsUrl: MCP_DOCS_URL, agents }; } diff --git a/src/doctor/issues.spec.ts b/src/doctor/issues.spec.ts index 59c54276..3c9bdd29 100644 --- a/src/doctor/issues.spec.ts +++ b/src/doctor/issues.spec.ts @@ -97,7 +97,8 @@ describe('detectIssues', () => { const report = baseReport() as DoctorReport; report.mcp = { serverUrl: 'https://mcp.workos.com/mcp', - agents: [{ agent: 'Cursor', available: true, installed: true, misconfigured: true }], + docsUrl: 'https://workos.com/docs/mcp', + agents: [{ agent: 'Cursor', available: true, configured: true, installed: true, misconfigured: true }], }; return report; } @@ -125,7 +126,7 @@ describe('detectIssues', () => { beforeEach(clearNpmEnv); afterEach(restoreNpmEnv); - it('adds no issue when MCP is absent or merely not installed', () => { + it('adds no issue when MCP is absent or merely not configured', () => { const report = baseReport(); // No mcp field at all. expect(detectIssues(report).some((i) => i.code === 'MCP_MISCONFIGURED')).toBe(false); @@ -133,9 +134,10 @@ describe('detectIssues', () => { // Detected agents, none misconfigured (some simply lack the server). report.mcp = { serverUrl: 'https://mcp.workos.com/mcp', + docsUrl: 'https://workos.com/docs/mcp', agents: [ - { agent: 'Claude Code', available: true, installed: false }, - { agent: 'Cursor', available: true, installed: true, misconfigured: false }, + { agent: 'Claude Code', available: true, configured: false, installed: false }, + { agent: 'Cursor', available: true, configured: true, installed: true, misconfigured: false }, ], }; expect(detectIssues(report).some((i) => i.code === 'MCP_MISCONFIGURED')).toBe(false); @@ -145,9 +147,10 @@ describe('detectIssues', () => { const report = baseReport(); report.mcp = { serverUrl: 'https://mcp.workos.com/mcp', + docsUrl: 'https://workos.com/docs/mcp', agents: [ - { agent: 'Claude Code', available: true, installed: true }, - { agent: 'Cursor', available: true, installed: true, misconfigured: true }, + { agent: 'Claude Code', available: true, configured: true, installed: true }, + { agent: 'Cursor', available: true, configured: true, installed: true, misconfigured: true }, ], }; diff --git a/src/doctor/issues.ts b/src/doctor/issues.ts index a8b9fa3d..af66d6d5 100644 --- a/src/doctor/issues.ts +++ b/src/doctor/issues.ts @@ -181,7 +181,8 @@ export function detectIssues(report: Omit): // MCP server URL drift — warn ONLY when a configured entry points at an // unexpected URL. Absent MCP is never an issue: the user may have declined - // the offer, and doctor reports state without judging. + // the offer, and doctor reports state without judging. Authentication remains + // client-managed and is not inferred from configuration presence. if (report.mcp) { const misconfigured = report.mcp.agents.filter((a) => a.misconfigured); if (misconfigured.length > 0) { diff --git a/src/doctor/output.spec.ts b/src/doctor/output.spec.ts index fc98e086..010f764f 100644 --- a/src/doctor/output.spec.ts +++ b/src/doctor/output.spec.ts @@ -85,4 +85,34 @@ describe('doctor output', () => { log.mockRestore(); }); + + it('reports MCP configuration separately from unverified Codex OAuth', () => { + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + + formatReport( + report({ + mcp: { + serverUrl: 'https://mcp.workos.com/mcp', + docsUrl: 'https://workos.com/docs/mcp', + agents: [ + { + agent: 'Codex', + available: true, + configured: true, + installed: true, + authentication: 'not-verified', + }, + ], + }, + }), + ); + + const output = log.mock.calls.map((call) => String(call[0])).join('\n'); + expect(output).toContain('Codex: configured (OAuth not verified)'); + expect(output).toContain('Recovery:'); + expect(output).toContain('https://workos.com/docs/mcp'); + expect(output).not.toContain('Codex: installed'); + + log.mockRestore(); + }); }); diff --git a/src/doctor/output.ts b/src/doctor/output.ts index 360164e0..de4e61e3 100644 --- a/src/doctor/output.ts +++ b/src/doctor/output.ts @@ -204,12 +204,16 @@ export function formatReport(report: DoctorReport, options?: FormatOptions): voi for (const agent of report.mcp.agents) { if (agent.misconfigured) { console.log(` ${Chalk.yellow('!')} ${agent.agent}: configured with an unexpected URL`); - } else if (agent.installed) { - console.log(` ${Chalk.green('✓')} ${agent.agent}: installed`); + } else if (agent.configured) { + const authentication = agent.authentication === 'not-verified' ? ' (OAuth not verified)' : ''; + console.log(` ${Chalk.green('✓')} ${agent.agent}: configured${authentication}`); } else { - console.log(` ${agent.agent}: ${Chalk.dim('not installed')}`); + console.log(` ${agent.agent}: ${Chalk.dim('not configured')}`); } } + if (report.mcp.agents.some((agent) => agent.authentication === 'not-verified')) { + console.log(` ${Chalk.dim('Recovery:')} ${report.mcp.docsUrl}`); + } } // Verbose mode additions diff --git a/src/doctor/types.ts b/src/doctor/types.ts index 45d9e795..a7f6975f 100644 --- a/src/doctor/types.ts +++ b/src/doctor/types.ts @@ -156,15 +156,21 @@ export interface McpAgentMcpStatus { agent: string; /** Agent is usable on this machine. */ available: boolean; - /** The WorkOS MCP server is present in this agent's config. */ + /** The WorkOS MCP server definition is present in this agent's effective config. */ + configured: boolean; + /** Legacy JSON field retained for compatibility; equivalent to configured. */ installed: boolean; - /** Cursor only: the entry exists but points at an unexpected URL. */ + /** The entry exists but points at an unexpected URL. */ misconfigured?: boolean; + /** Client-managed authentication cannot be verified without accessing credentials. */ + authentication?: 'not-verified'; } export interface McpInfo { /** The URL the WorkOS MCP server should be configured with. */ serverUrl: string; + /** Canonical setup and recovery documentation. */ + docsUrl: string; agents: McpAgentMcpStatus[]; } diff --git a/src/lib/constants.ts b/src/lib/constants.ts index 12efb8bc..4c74eca1 100644 --- a/src/lib/constants.ts +++ b/src/lib/constants.ts @@ -45,6 +45,7 @@ export const OAUTH_PORT = settings.legacy.oauthPort; */ export const MCP_SERVER_NAME = 'workos'; export const MCP_SERVER_URL = 'https://mcp.workos.com/mcp'; +export const MCP_DOCS_URL = 'https://workos.com/docs/mcp'; /** * Shared description for the `migrations` command, referenced by both the yargs diff --git a/src/lib/mcp-clients.ts b/src/lib/mcp-clients.ts index a28a148c..00ee4a85 100644 --- a/src/lib/mcp-clients.ts +++ b/src/lib/mcp-clients.ts @@ -3,7 +3,7 @@ import { join } from 'path'; import { access, mkdir, readFile, writeFile } from 'fs/promises'; import * as jsonc from 'jsonc-parser'; import { execFileNoThrow } from '../utils/exec-file.js'; -import { MCP_SERVER_NAME, MCP_SERVER_URL } from './constants.js'; +import { MCP_DOCS_URL, MCP_SERVER_NAME, MCP_SERVER_URL } from './constants.js'; /** * Client-writer library for the WorkOS MCP server. @@ -41,25 +41,45 @@ export type McpAgentKey = 'claude-code' | 'codex' | 'cursor'; export type McpOutcome = 'installed' | 'already-installed' | 'removed' | 'not-installed' | 'skipped' | 'failed'; /** - * Human phrasing for each outcome (JSON mode emits the raw `outcome` value). - * Lives with the `McpOutcome` type so every consumer (`mcp` command, `setup`) - * stays in lockstep when an outcome is added. + * Human phrasing for each outcome. The existing JSON values are retained for + * compatibility, but "installed" means the server definition is configured; + * it does not prove that client-managed OAuth completed successfully. */ export const MCP_OUTCOME_LABELS: Record = { - installed: 'installed', - 'already-installed': 'already installed', + installed: 'configured', + 'already-installed': 'already configured', removed: 'removed', - 'not-installed': 'not installed', + 'not-installed': 'not configured', skipped: 'skipped', failed: 'failed', }; +export type McpAuthenticationState = 'unknown' | 'action-required'; + +export interface McpRecoveryHint { + description: string; + command?: string; + hostShellRequired?: boolean; +} + +export interface McpRecovery { + docsUrl: string; + hints: McpRecoveryHint[]; +} + export interface McpClientResult { agent: McpAgentKey; displayName: string; outcome: McpOutcome; /** stderr/message excerpt when `outcome === 'failed'`. */ error?: string; + /** Configuration metadata for clients whose authentication is managed separately. */ + configuration?: { + scope: 'user' | 'project' | 'unknown'; + authentication: McpAuthenticationState; + }; + /** Safe next steps that do not expose or inspect client-managed credentials. */ + recovery?: McpRecovery; } export interface McpClientTarget { @@ -69,6 +89,8 @@ export interface McpClientTarget { isAvailable(): Promise; /** The WorkOS server is present in this client's config. */ isInstalled(): Promise; + /** Effective configured URL when the client exposes it, otherwise null. */ + getConfiguredUrl(): Promise; add(): Promise; remove(): Promise; } @@ -94,6 +116,39 @@ function excerpt(raw: string): string { return firstLine.trim().slice(0, 300); } +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function parseCodexConfiguredUrl(stdout: string): string | null { + try { + const parsed: unknown = JSON.parse(stdout); + if (!isRecord(parsed) || !isRecord(parsed.transport)) return null; + return typeof parsed.transport.url === 'string' ? parsed.transport.url : null; + } catch { + return null; + } +} + +const CODEX_RECOVERY: McpRecovery = { + docsUrl: MCP_DOCS_URL, + hints: [ + { + description: 'Confirm that Codex discovered the intended server definition', + command: `codex mcp get ${MCP_SERVER_NAME}`, + }, + { + description: 'Complete or refresh WorkOS OAuth in your normal host shell', + command: `codex mcp login ${MCP_SERVER_NAME}`, + hostShellRequired: true, + }, + { + description: 'Verify the WorkOS endpoint and OAuth authentication type', + command: 'codex mcp list', + }, + ], +}; + /** * Does a CLI `mcp list` output contain our server? * @@ -124,13 +179,31 @@ function createCliClient(config: { configDir: string; addArgs: string[]; removeArgs: string[]; + getUrlArgs?: string[]; + parseConfiguredUrl?: (stdout: string) => string | null; + configuredScope?: 'user' | 'project' | 'unknown'; + recovery?: McpRecovery; }): McpClientTarget { - const { key, displayName, binary, configDir, addArgs, removeArgs } = config; - const result = (outcome: McpOutcome, error?: string): McpClientResult => ({ + const { + key, + displayName, + binary, + configDir, + addArgs, + removeArgs, + getUrlArgs, + parseConfiguredUrl, + configuredScope, + recovery, + } = config; + const result = (outcome: McpOutcome, error?: string, authentication?: McpAuthenticationState): McpClientResult => ({ agent: key, displayName, outcome, ...(error ? { error } : {}), + ...(authentication && configuredScope + ? { configuration: { scope: configuredScope, authentication }, recovery } + : {}), }); async function checkInstalled(): Promise { @@ -150,20 +223,26 @@ function createCliClient(config: { return res.status === 0; }, isInstalled: checkInstalled, + async getConfiguredUrl() { + if (!getUrlArgs || !parseConfiguredUrl) return null; + const res = await execFileNoThrow(binary, getUrlArgs, { timeout: EXEC_TIMEOUT_MS }); + if (res.status !== 0) return null; + return parseConfiguredUrl(res.stdout); + }, async add() { const res = await execFileNoThrow(binary, addArgs, { timeout: EXEC_TIMEOUT_MS }); - if (res.status === 0) return result('installed'); + if (res.status === 0) return result('installed', undefined, 'unknown'); // Idempotent: an "already exists" collision is a success, not a failure. const combined = `${res.stdout}\n${res.stderr}`.toLowerCase(); if (combined.includes('already exists') || combined.includes('already configured')) { - return result('already-installed'); + return result('already-installed', undefined, 'unknown'); } // A non-zero exit doesn't always mean the write failed: Codex persists the // server config and THEN starts an OAuth flow that blocks (no browser / // callback available here) until our timeout kills it with a non-zero // status. Confirm against the actual config before declaring failure — // more robust than matching each client's success wording. - if (await checkInstalled()) return result('installed'); + if (await checkInstalled()) return result('installed', undefined, 'action-required'); // Otherwise a real failure — an old client lacking `--transport http` / // `--url`, a bad invocation, etc. Reported, never thrown. return result('failed', excerpt(res.stderr || res.stdout)); @@ -201,6 +280,10 @@ function createCodexClient(): McpClientTarget { configDir: '.codex', addArgs: ['mcp', 'add', MCP_SERVER_NAME, '--url', MCP_SERVER_URL], removeArgs: ['mcp', 'remove', MCP_SERVER_NAME], + getUrlArgs: ['mcp', 'get', MCP_SERVER_NAME, '--json'], + parseConfiguredUrl: parseCodexConfiguredUrl, + configuredScope: 'user', + recovery: CODEX_RECOVERY, }); } @@ -258,6 +341,7 @@ function createCursorClient(): McpClientTarget { return false; } }, + getConfiguredUrl: getCursorConfiguredUrl, async add() { const path = configPath(); try { @@ -309,10 +393,11 @@ export function createMcpClients(): McpClientTarget[] { * The URL the WorkOS server is configured with in Cursor's `~/.cursor/mcp.json`, * or null when the file/entry is absent, unreadable, or unparseable. * - * Cursor is the only client whose config we read directly (the CLI clients don't - * expose per-entry URLs), so this powers doctor's URL-drift ("misconfigured") - * check without a second jsonc reader duplicating the config schema. Read-only - * and never throws — a problem reading just yields null. + * This powers Cursor's URL-drift ("misconfigured") check without a second jsonc + * reader duplicating the config schema. Codex exposes its effective URL through + * `codex mcp get`; Claude Code does not currently expose a machine-readable + * per-entry URL here. Read-only and never throws — a problem reading just yields + * null. */ export async function getCursorConfiguredUrl(): Promise { try { diff --git a/src/utils/help-json.ts b/src/utils/help-json.ts index 5037c570..ae30d1b3 100644 --- a/src/utils/help-json.ts +++ b/src/utils/help-json.ts @@ -191,7 +191,7 @@ const commands: CommandSchema[] = [ commands: [ { name: 'install', - description: 'Add the WorkOS MCP server to detected coding agents', + description: 'Configure the WorkOS MCP server in detected coding agents', options: [ { name: 'agent', From 444a7f82b30c7a108cb11b3b99e2ccc242c810d4 Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Thu, 30 Jul 2026 13:08:15 -0400 Subject: [PATCH 2/5] fix(mcp): verify endpoint before treating a failed add as configured A non-zero `mcp add` was reclassified as configured whenever an entry of the same name existed. A stale entry from an earlier install satisfies that check, so a genuinely failed add reported success and setup could be marked complete with the intended endpoint never applied. Compare the client's effective endpoint when it can report one: a mismatch is decisive and stays failed. A null reading (no introspection, or `get` failed) is not contradicting evidence, so it keeps the existing name-only interpretation. Co-Authored-By: Claude Opus 5 (1M context) --- src/commands/mcp.spec.ts | 46 ++++++++++++++++++++++++++++++++++++++++ src/lib/mcp-clients.ts | 32 ++++++++++++++++++++++------ 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/src/commands/mcp.spec.ts b/src/commands/mcp.spec.ts index 3a0d0bf7..95ba6eb4 100644 --- a/src/commands/mcp.spec.ts +++ b/src/commands/mcp.spec.ts @@ -239,6 +239,52 @@ describe('Codex client', () => { }); }); + it('add stays failed when a stale entry survives the failed add at the wrong endpoint', async () => { + // The name is present, but it belongs to an older definition — the add + // never applied our endpoint, so this must not read as freshly configured. + mockExec((_c, args) => { + if (args.includes('add')) return { status: 1, stderr: 'boom' }; + if (args.includes('list')) return { status: 0, stdout: 'workos https://stale.example/mcp enabled' }; + if (args.includes('get')) { + return { + status: 0, + stdout: JSON.stringify({ + name: 'workos', + transport: { type: 'streamable_http', url: 'https://stale.example/mcp' }, + }), + }; + } + return { status: 0 }; + }); + + const res = await codex().add(); + expect(res.outcome).toBe('failed'); + expect(res.error).toContain('https://stale.example/mcp'); + expect(res.error).toContain('https://mcp.workos.com/mcp'); + }); + + it('add reports configured when the surviving entry matches the intended endpoint', async () => { + mockExec((_c, args) => { + if (args.includes('add')) return { status: 1, stdout: 'timed out waiting for authentication' }; + if (args.includes('list')) return { status: 0, stdout: 'workos https://mcp.workos.com/mcp enabled' }; + if (args.includes('get')) { + return { + status: 0, + stdout: JSON.stringify({ + name: 'workos', + transport: { type: 'streamable_http', url: 'https://mcp.workos.com/mcp' }, + }), + }; + } + return { status: 0 }; + }); + + expect(await codex().add()).toMatchObject({ + outcome: 'installed', + configuration: { scope: 'user', authentication: 'action-required' }, + }); + }); + it('add stays failed on a non-zero exit when the server did NOT land', async () => { mockExec((_c, args) => { if (args.includes('add')) return { status: 1, stderr: 'boom' }; diff --git a/src/lib/mcp-clients.ts b/src/lib/mcp-clients.ts index 00ee4a85..659c4c12 100644 --- a/src/lib/mcp-clients.ts +++ b/src/lib/mcp-clients.ts @@ -212,6 +212,13 @@ function createCliClient(config: { return listHasServer(res.stdout, MCP_SERVER_NAME); } + async function readConfiguredUrl(): Promise { + if (!getUrlArgs || !parseConfiguredUrl) return null; + const res = await execFileNoThrow(binary, getUrlArgs, { timeout: EXEC_TIMEOUT_MS }); + if (res.status !== 0) return null; + return parseConfiguredUrl(res.stdout); + } + return { key, displayName, @@ -223,12 +230,7 @@ function createCliClient(config: { return res.status === 0; }, isInstalled: checkInstalled, - async getConfiguredUrl() { - if (!getUrlArgs || !parseConfiguredUrl) return null; - const res = await execFileNoThrow(binary, getUrlArgs, { timeout: EXEC_TIMEOUT_MS }); - if (res.status !== 0) return null; - return parseConfiguredUrl(res.stdout); - }, + getConfiguredUrl: readConfiguredUrl, async add() { const res = await execFileNoThrow(binary, addArgs, { timeout: EXEC_TIMEOUT_MS }); if (res.status === 0) return result('installed', undefined, 'unknown'); @@ -242,7 +244,23 @@ function createCliClient(config: { // callback available here) until our timeout kills it with a non-zero // status. Confirm against the actual config before declaring failure — // more robust than matching each client's success wording. - if (await checkInstalled()) return result('installed', undefined, 'action-required'); + if (await checkInstalled()) { + // Presence of the name alone isn't proof our write landed: a stale + // entry from an earlier install survives a genuinely failed add and + // would otherwise be reported as freshly configured. When the client + // can report its effective endpoint, a mismatch is decisive — the add + // failed and the old definition is still in place. A null reading + // (client can't introspect, or `get` failed) is not contradicting + // evidence, so it keeps the name-only interpretation. + const configuredUrl = await readConfiguredUrl(); + if (configuredUrl !== null && configuredUrl !== MCP_SERVER_URL) { + return result( + 'failed', + `${binary} mcp add failed and "${MCP_SERVER_NAME}" still points at ${configuredUrl} (expected ${MCP_SERVER_URL})`, + ); + } + return result('installed', undefined, 'action-required'); + } // Otherwise a real failure — an old client lacking `--transport http` / // `--url`, a bad invocation, etc. Reported, never thrown. return result('failed', excerpt(res.stderr || res.stdout)); From a72ce9e7860e1765a7eb081c1d1ea733414aea18 Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Thu, 30 Jul 2026 13:18:49 -0400 Subject: [PATCH 3/5] fix(mcp): require a readable endpoint before claiming a failed add configured An unreadable `mcp get` still fell through to an installed result, so a failed Codex add whose endpoint could not be confirmed was reported as configured and could mark setup complete against a stale entry. Split "answered null" from "was never askable". A client that exposes its effective endpoint has to earn the configured verdict: mismatch and unreadable both stay failed. Claude Code, which has no introspection at all, keeps the name-only fallback. Co-Authored-By: Claude Opus 5 (1M context) --- src/commands/mcp.spec.ts | 59 +++++++++++++++++++++++++--------------- src/lib/mcp-clients.ts | 28 +++++++++++-------- 2 files changed, 54 insertions(+), 33 deletions(-) diff --git a/src/commands/mcp.spec.ts b/src/commands/mcp.spec.ts index 95ba6eb4..8d9a4088 100644 --- a/src/commands/mcp.spec.ts +++ b/src/commands/mcp.spec.ts @@ -168,6 +168,19 @@ describe('Claude Code client', () => { expect(res.error).toContain('--transport'); }); + it('add falls back to name-only presence for a client that cannot report its endpoint', async () => { + // Claude Code exposes no endpoint introspection, so the listed name is the + // best evidence available — it keeps the benefit of the doubt that a + // Codex-style unreadable endpoint does not get. + mockExec((_c, args) => { + if (args.includes('add')) return { status: 1, stderr: 'boom' }; + if (args.includes('list')) return { status: 0, stdout: 'workos: https://mcp.workos.com/mcp (HTTP) - Connected' }; + return { status: 0 }; + }); + + expect((await claude().add()).outcome).toBe('installed'); + }); + it('isInstalled matches the workos: list line', async () => { mockExec(() => ({ status: 0, @@ -231,6 +244,15 @@ describe('Codex client', () => { mockExec((_c, args) => { if (args.includes('add')) return { status: 1, stdout: "Added global MCP server 'workos'." }; if (args.includes('list')) return { status: 0, stdout: 'workos https://mcp.workos.com/mcp enabled' }; + if (args.includes('get')) { + return { + status: 0, + stdout: JSON.stringify({ + name: 'workos', + transport: { type: 'streamable_http', url: 'https://mcp.workos.com/mcp' }, + }), + }; + } return { status: 0 }; }); expect(await codex().add()).toMatchObject({ @@ -239,6 +261,21 @@ describe('Codex client', () => { }); }); + it('add stays failed when the surviving endpoint cannot be read back', async () => { + // Codex can be asked for its effective endpoint, so a `get` we can't parse + // is an unverified claim, not a success — the entry may still be stale. + mockExec((_c, args) => { + if (args.includes('add')) return { status: 1, stderr: 'boom' }; + if (args.includes('list')) return { status: 0, stdout: 'workos https://mcp.workos.com/mcp enabled' }; + if (args.includes('get')) return { status: 1, stderr: 'unknown flag --json' }; + return { status: 0 }; + }); + + const res = await codex().add(); + expect(res.outcome).toBe('failed'); + expect(res.error).toContain('could not read'); + }); + it('add stays failed when a stale entry survives the failed add at the wrong endpoint', async () => { // The name is present, but it belongs to an older definition — the add // never applied our endpoint, so this must not read as freshly configured. @@ -263,28 +300,6 @@ describe('Codex client', () => { expect(res.error).toContain('https://mcp.workos.com/mcp'); }); - it('add reports configured when the surviving entry matches the intended endpoint', async () => { - mockExec((_c, args) => { - if (args.includes('add')) return { status: 1, stdout: 'timed out waiting for authentication' }; - if (args.includes('list')) return { status: 0, stdout: 'workos https://mcp.workos.com/mcp enabled' }; - if (args.includes('get')) { - return { - status: 0, - stdout: JSON.stringify({ - name: 'workos', - transport: { type: 'streamable_http', url: 'https://mcp.workos.com/mcp' }, - }), - }; - } - return { status: 0 }; - }); - - expect(await codex().add()).toMatchObject({ - outcome: 'installed', - configuration: { scope: 'user', authentication: 'action-required' }, - }); - }); - it('add stays failed on a non-zero exit when the server did NOT land', async () => { mockExec((_c, args) => { if (args.includes('add')) return { status: 1, stderr: 'boom' }; diff --git a/src/lib/mcp-clients.ts b/src/lib/mcp-clients.ts index 659c4c12..0d92f7b3 100644 --- a/src/lib/mcp-clients.ts +++ b/src/lib/mcp-clients.ts @@ -212,6 +212,10 @@ function createCliClient(config: { return listHasServer(res.stdout, MCP_SERVER_NAME); } + /** Whether this client exposes its effective endpoint at all — the difference + * between "answered null" (unreadable) and "was never askable". */ + const canReadConfiguredUrl = Boolean(getUrlArgs && parseConfiguredUrl); + async function readConfiguredUrl(): Promise { if (!getUrlArgs || !parseConfiguredUrl) return null; const res = await execFileNoThrow(binary, getUrlArgs, { timeout: EXEC_TIMEOUT_MS }); @@ -247,19 +251,21 @@ function createCliClient(config: { if (await checkInstalled()) { // Presence of the name alone isn't proof our write landed: a stale // entry from an earlier install survives a genuinely failed add and - // would otherwise be reported as freshly configured. When the client - // can report its effective endpoint, a mismatch is decisive — the add - // failed and the old definition is still in place. A null reading - // (client can't introspect, or `get` failed) is not contradicting - // evidence, so it keeps the name-only interpretation. + // would otherwise be reported as freshly configured. So the name is + // only sufficient for a client that can't be asked anything better + // (Claude Code). For a client that CAN report its effective endpoint, + // "configured" has to be earned — a mismatch means the old definition + // is still in place, and an unreadable answer is not a yes. Both stay + // failed rather than claiming a write we never confirmed. const configuredUrl = await readConfiguredUrl(); - if (configuredUrl !== null && configuredUrl !== MCP_SERVER_URL) { - return result( - 'failed', - `${binary} mcp add failed and "${MCP_SERVER_NAME}" still points at ${configuredUrl} (expected ${MCP_SERVER_URL})`, - ); + if (configuredUrl === MCP_SERVER_URL || !canReadConfiguredUrl) { + return result('installed', undefined, 'action-required'); } - return result('installed', undefined, 'action-required'); + const detail = + configuredUrl === null + ? `could not read the effective ${MCP_SERVER_NAME} endpoint to confirm it` + : `"${MCP_SERVER_NAME}" still points at ${configuredUrl} (expected ${MCP_SERVER_URL})`; + return result('failed', `${binary} mcp add failed and ${detail}`); } // Otherwise a real failure — an old client lacking `--transport http` / // `--url`, a bad invocation, etc. Reported, never thrown. From 20d4e208725b64fe7005386a2ab07cc18c283c29 Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Thu, 30 Jul 2026 13:22:51 -0400 Subject: [PATCH 4/5] fix(mcp): confirm an "already exists" collision is our own entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The collision branch returned already-installed on the add command's wording alone, so a stale entry under our name reported as already configured and let setup persist completion against the wrong endpoint — the same defect the non-zero-exit path was just fixed for. Extract the endpoint check both paths need into staleEntryReason() and apply it to the collision branch too. Co-Authored-By: Claude Opus 5 (1M context) --- src/commands/mcp.spec.ts | 40 ++++++++++++++++++++++++++++++++++ src/lib/mcp-clients.ts | 46 ++++++++++++++++++++++++---------------- 2 files changed, 68 insertions(+), 18 deletions(-) diff --git a/src/commands/mcp.spec.ts b/src/commands/mcp.spec.ts index 8d9a4088..68252779 100644 --- a/src/commands/mcp.spec.ts +++ b/src/commands/mcp.spec.ts @@ -261,6 +261,46 @@ describe('Codex client', () => { }); }); + it('add rejects an "already exists" collision that points at a stale endpoint', async () => { + // A name collision only counts as idempotent success if the entry we + // collided with is actually ours. + mockExec((_c, args) => { + if (args.includes('add')) return { status: 1, stderr: "MCP server 'workos' already exists" }; + if (args.includes('get')) { + return { + status: 0, + stdout: JSON.stringify({ + name: 'workos', + transport: { type: 'streamable_http', url: 'https://stale.example/mcp' }, + }), + }; + } + return { status: 0 }; + }); + + const res = await codex().add(); + expect(res.outcome).toBe('failed'); + expect(res.error).toContain('https://stale.example/mcp'); + }); + + it('add accepts an "already exists" collision at the intended endpoint', async () => { + mockExec((_c, args) => { + if (args.includes('add')) return { status: 1, stderr: "MCP server 'workos' already exists" }; + if (args.includes('get')) { + return { + status: 0, + stdout: JSON.stringify({ + name: 'workos', + transport: { type: 'streamable_http', url: 'https://mcp.workos.com/mcp' }, + }), + }; + } + return { status: 0 }; + }); + + expect((await codex().add()).outcome).toBe('already-installed'); + }); + it('add stays failed when the surviving endpoint cannot be read back', async () => { // Codex can be asked for its effective endpoint, so a `get` we can't parse // is an unverified claim, not a success — the entry may still be stale. diff --git a/src/lib/mcp-clients.ts b/src/lib/mcp-clients.ts index 0d92f7b3..d3fa1069 100644 --- a/src/lib/mcp-clients.ts +++ b/src/lib/mcp-clients.ts @@ -223,6 +223,27 @@ function createCliClient(config: { return parseConfiguredUrl(res.stdout); } + /** + * An entry under our name exists — but is it ours? Both `add` paths that + * infer success from an existing entry (a collision, and a non-zero exit + * whose write may still have landed) have to answer this, or they report a + * stale definition from an earlier install as freshly configured. + * + * Returns null when the entry can be treated as ours: either the endpoint + * matches, or the client exposes no way to ask (Claude Code), where the + * listed name is the best evidence available. Otherwise returns the reason + * it can't be — a mismatch, or an answer we couldn't read, which for an + * introspectable client is not a yes. + */ + async function staleEntryReason(): Promise { + if (!canReadConfiguredUrl) return null; + const configuredUrl = await readConfiguredUrl(); + if (configuredUrl === MCP_SERVER_URL) return null; + return configuredUrl === null + ? `could not read the effective ${MCP_SERVER_NAME} endpoint to confirm it` + : `"${MCP_SERVER_NAME}" still points at ${configuredUrl} (expected ${MCP_SERVER_URL})`; + } + return { key, displayName, @@ -238,9 +259,12 @@ function createCliClient(config: { async add() { const res = await execFileNoThrow(binary, addArgs, { timeout: EXEC_TIMEOUT_MS }); if (res.status === 0) return result('installed', undefined, 'unknown'); - // Idempotent: an "already exists" collision is a success, not a failure. + // Idempotent: an "already exists" collision is a success, not a failure — + // but only once the colliding entry is confirmed to be ours. const combined = `${res.stdout}\n${res.stderr}`.toLowerCase(); if (combined.includes('already exists') || combined.includes('already configured')) { + const stale = await staleEntryReason(); + if (stale) return result('failed', `${binary} mcp add found an existing entry and ${stale}`); return result('already-installed', undefined, 'unknown'); } // A non-zero exit doesn't always mean the write failed: Codex persists the @@ -249,23 +273,9 @@ function createCliClient(config: { // status. Confirm against the actual config before declaring failure — // more robust than matching each client's success wording. if (await checkInstalled()) { - // Presence of the name alone isn't proof our write landed: a stale - // entry from an earlier install survives a genuinely failed add and - // would otherwise be reported as freshly configured. So the name is - // only sufficient for a client that can't be asked anything better - // (Claude Code). For a client that CAN report its effective endpoint, - // "configured" has to be earned — a mismatch means the old definition - // is still in place, and an unreadable answer is not a yes. Both stay - // failed rather than claiming a write we never confirmed. - const configuredUrl = await readConfiguredUrl(); - if (configuredUrl === MCP_SERVER_URL || !canReadConfiguredUrl) { - return result('installed', undefined, 'action-required'); - } - const detail = - configuredUrl === null - ? `could not read the effective ${MCP_SERVER_NAME} endpoint to confirm it` - : `"${MCP_SERVER_NAME}" still points at ${configuredUrl} (expected ${MCP_SERVER_URL})`; - return result('failed', `${binary} mcp add failed and ${detail}`); + const stale = await staleEntryReason(); + if (stale) return result('failed', `${binary} mcp add failed and ${stale}`); + return result('installed', undefined, 'action-required'); } // Otherwise a real failure — an old client lacking `--transport http` / // `--url`, a bad invocation, etc. Reported, never thrown. From 42157469d21ec8f9a0ed22e7b0b4f2ab0908f451 Mon Sep 17 00:00:00 2001 From: "Garen J. Torikian" Date: Thu, 30 Jul 2026 14:37:17 -0400 Subject: [PATCH 5/5] refactor(mcp): drive the OAuth caveat off a client capability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `authentication: 'not-verified'` was stamped by comparing `client.key` to 'codex' in two places, and `mcp status` hardcoded `codex mcp login workos` even though the client library already held that exact command. The CLI cannot verify OAuth for *any* client — it never reads agent credentials — so annotating Codex alone made it read as the broken one when it was the only honest one. Move the fact onto `McpClientTarget` as `authenticationVerifiable` (false everywhere today) plus the client's own `recovery`, so callers report the caveat without knowing which client they hold, and a client that grows a real signal flips one literal. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 2 +- src/commands/mcp.spec.ts | 49 ++++++++++++++++++++++++- src/commands/mcp.ts | 68 +++++++++++++++++++++++++---------- src/doctor/checks/mcp.spec.ts | 57 ++++++++++++++++++++++++++--- src/doctor/checks/mcp.ts | 9 +++-- src/lib/mcp-clients.ts | 16 +++++++++ 6 files changed, 173 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index bebc5083..3df20538 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,7 @@ Commands: `workos setup` installs WorkOS skills and configures the MCP server only after consent. Use `workos skills list` to check skill status, `workos mcp status` to check whether the server definition is configured, or `workos doctor --fix` to refresh stale skills. -MCP configuration and OAuth authentication are separate states. For Codex, complete or refresh OAuth with `codex mcp login workos` in your normal host shell. The WorkOS CLI does not inspect Codex credentials and cannot prove that OAuth is usable. See the [WorkOS MCP setup and recovery guide](https://workos.com/docs/mcp) for user-global and trusted-project-only configuration. +MCP configuration and OAuth authentication are separate states. The WorkOS CLI never inspects a coding agent's credentials, so "configured" means the server definition is in place — it cannot prove that OAuth is usable in any agent. Each agent owns its own OAuth; with Codex, for example, complete or refresh it with `codex mcp login workos` in your normal host shell. See the [WorkOS MCP setup and recovery guide](https://workos.com/docs/mcp) for user-global and trusted-project-only configuration. Resource Management: organization (org) Manage organizations diff --git a/src/commands/mcp.spec.ts b/src/commands/mcp.spec.ts index 68252779..9c383861 100644 --- a/src/commands/mcp.spec.ts +++ b/src/commands/mcp.spec.ts @@ -577,8 +577,55 @@ describe('JSON output mode', () => { available: true, configured: true, installed: true, + // Cursor's OAuth is no more verifiable than Codex's, so it carries the + // same caveat — the annotation follows the capability, not the agent name. + authentication: 'not-verified', }); const claude = output.data.agents.find((a: { agent: string }) => a.agent === 'claude-code'); - expect(claude).toMatchObject({ available: false, configured: false, installed: false }); + expect(claude).toMatchObject({ + available: false, + configured: false, + installed: false, + authentication: null, + }); + }); +}); + +describe('runMcpStatus (human mode)', () => { + it('names every unverified client and takes recovery commands from the client', async () => { + makeDir('.codex'); + makeDir('.cursor'); + writeCursorConfig('{ "mcpServers": { "workos": { "url": "https://mcp.workos.com/mcp" } } }'); + mockExec((_c, args) => { + if (args[0] === '--version') return { status: 0 }; + if (args.includes('list')) return { status: 0, stdout: 'workos https://mcp.workos.com/mcp enabled' }; + if (args.includes('get')) { + return { + status: 0, + stdout: JSON.stringify({ + name: 'workos', + transport: { type: 'streamable_http', url: 'https://mcp.workos.com/mcp' }, + }), + }; + } + return { status: 0 }; + }); + + await runMcpStatus(); + + const joined = uiLogs.join('\n'); + expect(joined).toContain('OAuth is managed by'); + expect(joined).toContain('Codex'); + expect(joined).toContain('Cursor'); + expect(joined).toContain('codex mcp login workos'); + expect(joined).toContain('https://workos.com/docs/mcp'); + }); + + it('prints no OAuth caveat when nothing is configured', async () => { + mockExec((_c, args) => (args[0] === '--version' ? { status: 1 } : { status: 0 })); + + await runMcpStatus(); + + expect(uiLogs.join('\n')).not.toContain('OAuth is managed by'); }); }); diff --git a/src/commands/mcp.ts b/src/commands/mcp.ts index 6348425a..65674cb9 100644 --- a/src/commands/mcp.ts +++ b/src/commands/mcp.ts @@ -8,6 +8,7 @@ import { MCP_OUTCOME_LABELS, type McpAgentKey, type McpClientResult, + type McpRecovery, } from '../lib/mcp-clients.js'; import { MCP_DOCS_URL, MCP_SERVER_URL } from '../lib/constants.js'; @@ -77,7 +78,18 @@ function reportResults(message: string, results: McpClientResult[]): void { } } +/** + * Print a client's own recovery steps. Every command string comes from the + * client library, so no caller here spells out an agent's CLI invocation. + */ +function printRecoveryHints(recovery: McpRecovery): void { + for (const hint of recovery.hints) { + ui.log.hint(hint.command ? `${hint.description}: ${hint.command}` : hint.description); + } +} + function reportRecovery(results: McpClientResult[]): void { + const docsUrls = new Set(); for (const result of results) { if (!result.recovery || !result.configuration) continue; if (result.configuration.authentication === 'action-required') { @@ -85,10 +97,11 @@ function reportRecovery(results: McpClientResult[]): void { } else { ui.log.info(`${result.displayName}: configuration does not prove that OAuth is complete.`); } - for (const hint of result.recovery.hints) { - ui.log.hint(hint.command ? `${hint.description}: ${hint.command}` : hint.description); - } - ui.log.hint(`Setup and recovery guide: ${result.recovery.docsUrl}`); + printRecoveryHints(result.recovery); + docsUrls.add(result.recovery.docsUrl); + } + for (const url of docsUrls) { + ui.log.hint(`Setup and recovery guide: ${url}`); } } @@ -125,26 +138,32 @@ export async function runMcpStatus(): Promise { // availability + install flags, so `createMcpClients()` — not // `detectMcpClients()` — is the right source here. const clients = createMcpClients(); - const agents = await Promise.all( + const probed = await Promise.all( clients.map(async (client) => { const available = await client.isAvailable(); const configured = available ? await client.isInstalled() : false; const configuredUrl = configured ? await client.getConfiguredUrl() : null; const endpointValid = configuredUrl === null ? null : configuredUrl === MCP_SERVER_URL; - const authentication = client.key === 'codex' && configured ? 'not-verified' : null; + // A client that cannot report its OAuth state gets the caveat, whichever + // client it is — the CLI never reads agent credentials. + const authentication = configured && !client.authenticationVerifiable ? 'not-verified' : null; return { - agent: client.key, - displayName: client.displayName, - available, - configured, - // Retained for compatibility with existing JSON consumers. - installed: configured, - configuredUrl, - endpointValid, - authentication, + client, + status: { + agent: client.key, + displayName: client.displayName, + available, + configured, + // Retained for compatibility with existing JSON consumers. + installed: configured, + configuredUrl, + endpointValid, + authentication, + }, }; }), ); + const agents = probed.map((p) => p.status); if (isJsonMode()) { outputJson({ data: { agents } }); @@ -161,8 +180,21 @@ export async function runMcpStatus(): Promise { ]), ); - if (agents.some((agent) => agent.authentication === 'not-verified')) { - ui.log.hint(`Codex OAuth is not verified by this command. Run: codex mcp login workos`); - ui.log.hint(`Setup and recovery guide: ${MCP_DOCS_URL}`); + const unverified = probed.filter((p) => p.status.authentication === 'not-verified'); + if (unverified.length === 0) return; + + ui.log.hint( + `OAuth is managed by ${unverified.map((p) => p.client.displayName).join(', ')} and is not verified by this command.`, + ); + const docsUrls = new Set(); + for (const { client } of unverified) { + if (!client.recovery) continue; + printRecoveryHints(client.recovery); + docsUrls.add(client.recovery.docsUrl); + } + // Clients without their own recovery steps still get the canonical guide. + if (docsUrls.size === 0) docsUrls.add(MCP_DOCS_URL); + for (const url of docsUrls) { + ui.log.hint(`Setup and recovery guide: ${url}`); } } diff --git a/src/doctor/checks/mcp.spec.ts b/src/doctor/checks/mcp.spec.ts index cb2c5c2d..bfd58fe8 100644 --- a/src/doctor/checks/mcp.spec.ts +++ b/src/doctor/checks/mcp.spec.ts @@ -10,13 +10,20 @@ vi.mock('../../lib/mcp-clients.js', () => ({ const { MCP_DOCS_URL, MCP_SERVER_URL } = await import('../../lib/constants.js'); const { checkMcp } = await import('./mcp.js'); -function fakeClient(key: string, displayName: string, configured: boolean, configuredUrl: string | null = null) { +function fakeClient( + key: string, + displayName: string, + configured: boolean, + configuredUrl: string | null = null, + authenticationVerifiable = false, +) { return { key, displayName, isAvailable: vi.fn(() => Promise.resolve(true)), isInstalled: vi.fn(() => Promise.resolve(configured)), getConfiguredUrl: vi.fn(() => Promise.resolve(configuredUrl)), + authenticationVerifiable, add: vi.fn(), remove: vi.fn(), }; @@ -43,7 +50,14 @@ describe('checkMcp', () => { serverUrl: MCP_SERVER_URL, docsUrl: MCP_DOCS_URL, agents: [ - { agent: 'Claude Code', available: true, configured: true, installed: true, misconfigured: false }, + { + agent: 'Claude Code', + available: true, + configured: true, + installed: true, + misconfigured: false, + authentication: 'not-verified', + }, { agent: 'Codex', available: true, configured: false, installed: false }, ], }); @@ -60,6 +74,7 @@ describe('checkMcp', () => { configured: true, installed: true, misconfigured: true, + authentication: 'not-verified', }); }); @@ -89,7 +104,7 @@ describe('checkMcp', () => { expect(cursor.getConfiguredUrl).not.toHaveBeenCalled(); }); - it('validates the Codex URL and marks OAuth as not verified', async () => { + it('validates the URL and marks OAuth not verified for any unverifiable client', async () => { detectResult = [fakeClient('codex', 'Codex', true, 'https://wrong.example.com/mcp')]; const result = await checkMcp(); @@ -104,6 +119,24 @@ describe('checkMcp', () => { }); }); + it('omits the caveat for a client that can report its own OAuth state', async () => { + // The annotation is driven by the client capability, not by its name — a + // client that grows a real signal is exempt without touching this check. + detectResult = [fakeClient('claude-code', 'Claude Code', true, MCP_SERVER_URL, true)]; + + const result = await checkMcp(); + + expect(result!.agents[0].authentication).toBeUndefined(); + }); + + it('never annotates a client that is not configured', async () => { + detectResult = [fakeClient('codex', 'Codex', false)]; + + const result = await checkMcp(); + + expect(result!.agents[0].authentication).toBeUndefined(); + }); + it('handles the mixed matrix: one configured, one missing, one misconfigured', async () => { detectResult = [ fakeClient('claude-code', 'Claude Code', true), @@ -114,9 +147,23 @@ describe('checkMcp', () => { const result = await checkMcp(); expect(result!.agents).toEqual([ - { agent: 'Claude Code', available: true, configured: true, installed: true, misconfigured: false }, + { + agent: 'Claude Code', + available: true, + configured: true, + installed: true, + misconfigured: false, + authentication: 'not-verified', + }, { agent: 'Codex', available: true, configured: false, installed: false }, - { agent: 'Cursor', available: true, configured: true, installed: true, misconfigured: true }, + { + agent: 'Cursor', + available: true, + configured: true, + installed: true, + misconfigured: true, + authentication: 'not-verified', + }, ]); }); }); diff --git a/src/doctor/checks/mcp.ts b/src/doctor/checks/mcp.ts index 549dfe1c..26ed9303 100644 --- a/src/doctor/checks/mcp.ts +++ b/src/doctor/checks/mcp.ts @@ -14,6 +14,11 @@ import type { McpInfo, McpAgentMcpStatus } from '../types.js'; * offer. URL drift ("misconfigured") is detected when a client exposes its * effective URL; issues.ts derives a warning from it. Per-agent shell-outs * carry the 10s timeout from the client library, keeping doctor fast. + * + * A configured entry is annotated `not-verified` whenever the client cannot + * report its OAuth state — a client capability, never a hardcoded agent name, + * so the caveat is applied uniformly instead of singling out the one client we + * happen to have written recovery steps for. */ export async function checkMcp(): Promise { const clients = await detectMcpClients(); @@ -31,9 +36,7 @@ export async function checkMcp(): Promise { if (configured) { const url = await client.getConfiguredUrl(); status.misconfigured = url !== null && url !== MCP_SERVER_URL; - } - if (client.key === 'codex' && configured) { - status.authentication = 'not-verified'; + if (!client.authenticationVerifiable) status.authentication = 'not-verified'; } return status; }), diff --git a/src/lib/mcp-clients.ts b/src/lib/mcp-clients.ts index d3fa1069..f60bb6b0 100644 --- a/src/lib/mcp-clients.ts +++ b/src/lib/mcp-clients.ts @@ -91,6 +91,19 @@ export interface McpClientTarget { isInstalled(): Promise; /** Effective configured URL when the client exposes it, otherwise null. */ getConfiguredUrl(): Promise; + /** + * Whether this CLI can observe the client's OAuth state. False for every + * client today: credentials live inside each client and we never read them, so + * a present server definition proves nothing about authentication. + * + * It lives on the target rather than being inferred from `key` so callers can + * report the caveat without knowing which client they hold, and so a client + * that grows a usable signal (Claude Code's `mcp list` already prints a + * per-entry connection state) flips to true here alone. + */ + readonly authenticationVerifiable: boolean; + /** Client-specific next steps when authentication needs attention. */ + readonly recovery?: McpRecovery; add(): Promise; remove(): Promise; } @@ -256,6 +269,8 @@ function createCliClient(config: { }, isInstalled: checkInstalled, getConfiguredUrl: readConfiguredUrl, + authenticationVerifiable: false, + ...(recovery ? { recovery } : {}), async add() { const res = await execFileNoThrow(binary, addArgs, { timeout: EXEC_TIMEOUT_MS }); if (res.status === 0) return result('installed', undefined, 'unknown'); @@ -376,6 +391,7 @@ function createCursorClient(): McpClientTarget { } }, getConfiguredUrl: getCursorConfiguredUrl, + authenticationVerifiable: false, async add() { const path = configPath(); try {