Skip to content

fix(oauth): honor discovered CIMD support - #80

Merged
aryasaatvik merged 3 commits into
devfrom
fix/mcp-cimd-discovery-handoff
Aug 25, 2026
Merged

fix(oauth): honor discovered CIMD support#80
aryasaatvik merged 3 commits into
devfrom
fix/mcp-cimd-discovery-handoff

Conversation

@aryasaatvik

@aryasaatvik aryasaatvik commented Aug 25, 2026

Copy link
Copy Markdown
Owner

Summary

Honor OAuth discovery that advertises Client ID Metadata Documents, so MCP integrations such as Samva connect without falling through to manual app registration.

Flow

Connect click
  -> reserve popup
  -> probe OAuth metadata
  -> CIMD advertised: create or reuse the public metadata client
  -> otherwise: use DCR when registration_endpoint exists
  -> start OAuth in the reserved popup

The CIMD branch wins when both mechanisms are advertised and never calls dynamic registration.

Validation

  • bunx vitest run src/components/add-account-modal.test.ts — 45 tests passed
  • bun run typecheck
  • ./node_modules/.bin/oxlint -c .oxlintrc.jsonc packages/core/sdk/src/testing/oauth-test-server.ts e2e/selfhost/mcp-oauth-cimd-connect.test.ts --deny-warnings
  • ./node_modules/.bin/oxfmt --check packages/core/sdk/src/testing/oauth-test-server.ts e2e/selfhost/mcp-oauth-cimd-connect.test.ts
  • ./node_modules/.bin/vitest run --project selfhost selfhost/mcp-oauth-cimd-connect.test.ts — browser E2E passed with recording and trace

@aryasaatvik
aryasaatvik marked this pull request as ready for review August 25, 2026 15:17
@greptile-apps

greptile-apps Bot commented Aug 25, 2026

Copy link
Copy Markdown

Greptile Summary

This PR extends automatic discovered OAuth setup to prefer Client ID Metadata Documents when advertised, while retaining Dynamic Client Registration for other servers.

  • Extracts CIMD client resolution so manual and automatic connection flows share creation and reuse behavior.
  • Preserves the popup reservation across discovery and client setup.
  • Adds focused orchestration coverage for CIMD precedence over DCR and updates DCR outcome assertions.

Confidence Score: 4/5

The PR appears safe to merge from a functional perspective, with the non-blocking requirement to add the repository-mandated end-to-end UI validation and recording.

The automatic CIMD selection and DCR fallback are coherently implemented and unit-tested, but the actual modal interaction, popup handoff, and recovery states are not covered by the required user-visible end-to-end evidence.

Files Needing Attention: packages/react/src/components/add-account-modal.tsx, packages/react/src/components/add-account-modal.test.ts

Important Files Changed

Filename Overview
packages/react/src/components/add-account-modal.tsx Adds CIMD-aware automatic OAuth orchestration and wires it into the shared account modal; the implementation is focused, but the changed user interaction lacks the required end-to-end artifact.
packages/react/src/components/add-account-modal.test.ts Adds direct orchestration coverage proving CIMD precedence and popup-reservation reuse, but does not exercise the rendered modal flow.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  Click[Connect click] --> Reserve[Reserve OAuth popup]
  Reserve --> Probe[Probe OAuth discovery]
  Probe -->|CIMD advertised| CIMD[Create or reuse metadata client]
  Probe -->|No CIMD, DCR available| DCR[Register dynamic client]
  Probe -->|Neither available| Fallback[Show manual app recovery]
  CIMD --> Start[Start OAuth in reserved popup]
  DCR --> Start
Loading

Fix all with Greploop Fix All in Codex Fix All in Cursor Cloud Agents Fix All in Claude Code Fix All in Cursor

Reviews (1): Last reviewed commit: "refactor(oauth): use the canonical probe..." | Re-trigger Greptile

Comment on lines 3075 to +3078
) : dcrActive ? (
<Button
type="button"
onClick={() => void handleDcrConnect()}
onClick={() => void handleAutomaticOAuthConnect()}

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 Missing end-to-end OAuth coverage

The new Connect interaction is covered only through the extracted orchestrator test, leaving the rendered modal state, popup handoff, and failure recovery unvalidated across the shared browser hosts. Add the repository-required end-to-end scenario and browsable recording or trace for this user-visible flow.

Context Used: AGENTS.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Codex Fix in Cursor Cloud Agents Fix in Claude Code Fix in Cursor

@aryasaatvik
aryasaatvik merged commit 1831d36 into dev Aug 25, 2026
2 of 35 checks passed
aryasaatvik added a commit that referenced this pull request Aug 27, 2026
Honor OAuth discovery that advertises Client ID Metadata Documents, so
MCP integrations such as Samva connect without falling through to manual
app registration.

```text
Connect click
  -> reserve popup
  -> probe OAuth metadata
  -> CIMD advertised: create or reuse the public metadata client
  -> otherwise: use DCR when registration_endpoint exists
  -> start OAuth in the reserved popup
```

The CIMD branch wins when both mechanisms are advertised and never calls
dynamic registration.

- `bunx vitest run src/components/add-account-modal.test.ts` — 45 tests
passed
- `bun run typecheck`
- `./node_modules/.bin/oxlint -c .oxlintrc.jsonc
packages/core/sdk/src/testing/oauth-test-server.ts
e2e/selfhost/mcp-oauth-cimd-connect.test.ts --deny-warnings`
- `./node_modules/.bin/oxfmt --check
packages/core/sdk/src/testing/oauth-test-server.ts
e2e/selfhost/mcp-oauth-cimd-connect.test.ts`
- `./node_modules/.bin/vitest run --project selfhost
selfhost/mcp-oauth-cimd-connect.test.ts` — browser E2E passed with
recording and trace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant