Skip to content

fix(mcp): distinguish MCP configuration from OAuth authentication - #214

Open
gjtorikian wants to merge 5 commits into
mainfrom
fix/codex-mcp-setup-recovery
Open

fix(mcp): distinguish MCP configuration from OAuth authentication#214
gjtorikian wants to merge 5 commits into
mainfrom
fix/codex-mcp-setup-recovery

Conversation

@gjtorikian

Copy link
Copy Markdown
Contributor

Summary

  • report MCP server definitions as configured instead of implying OAuth is complete
  • recover when Codex persists configuration but its OAuth flow is interrupted
  • validate the effective Codex endpoint and surface safe host-shell recovery commands
  • retain legacy JSON outcome and installed fields for compatibility
  • link setup and doctor output to the canonical WorkOS MCP guide

Addresses #213.

Testing

  • bun run format:check
  • bun run lint
  • bun run typecheck
  • bun run test
  • bun run build

Comment thread src/lib/mcp-clients.ts Outdated
@greptile-apps

greptile-apps Bot commented Jul 30, 2026

Copy link
Copy Markdown

Greptile Summary

The PR separates MCP configuration from OAuth status and strengthens Codex recovery handling.

  • Verifies the effective Codex endpoint before treating a failed or colliding add operation as successful.
  • Adds structured authentication metadata, recovery guidance, and compatibility-preserving JSON fields.
  • Updates setup, status, doctor, help, tests, and documentation to consistently report MCP definitions as configured rather than authenticated.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/lib/mcp-clients.ts Adds Codex endpoint introspection and ensures non-zero add and collision recovery only succeed when the surviving definition uses the intended endpoint.
src/commands/mcp.ts Reports configuration and authentication separately while retaining the legacy installed field for JSON consumers.
src/commands/setup.ts Propagates structured MCP configuration metadata and client-owned OAuth recovery guidance through setup output.
src/doctor/checks/mcp.ts Generalizes effective-endpoint validation and annotates configured clients whose OAuth state cannot be verified.
src/doctor/output.ts Renders MCP definitions as configured and links users to recovery documentation when OAuth remains unverified.
src/doctor/types.ts Extends the public doctor report with configuration, authentication, and documentation fields while preserving installed compatibility.
src/bin.ts Updates MCP command help terminology to describe configuration rather than installation.
src/utils/help-json.ts Keeps agent-facing JSON help synchronized with the updated MCP command description.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Run Codex MCP add] --> B{Exit status}
  B -->|Success| C[Report configured]
  B -->|Failure or collision| D{WorkOS entry present?}
  D -->|No| E[Report failure]
  D -->|Yes| F[Read effective endpoint]
  F -->|Expected WorkOS URL| G[Report configured and OAuth action required]
  F -->|Wrong or unreadable URL| E
  C --> H[Show OAuth recovery guidance]
  G --> H
Loading

Reviews (5): Last reviewed commit: "refactor(mcp): drive the OAuth caveat of..." | Re-trigger Greptile

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) <noreply@anthropic.com>
@gjtorikian gjtorikian changed the title Distinguish MCP configuration from OAuth authentication fix(mcp): distinguish MCP configuration from OAuth authentication Jul 30, 2026
gjtorikian and others added 3 commits July 30, 2026 13:18
…nfigured

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) <noreply@anthropic.com>
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) <noreply@anthropic.com>
`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) <noreply@anthropic.com>
@gjtorikian
gjtorikian requested a review from nicknisi July 30, 2026 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant