feat(mcp): clerk mcp install/list/uninstall/run + doctor MCP check#307
Conversation
🦋 Changeset detectedLatest commit: e22734c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds MCP server support to the Clerk CLI: new subcommands (clerk mcp install/list/uninstall), a JSON-backed client factory and five client integrations (Claude Code, Cursor, VS Code, Windsurf, Gemini), per-client config read/write helpers, collection of installed entries, an initialize-handshake probe supporting JSON and SSE with a 10s timeout, a doctor check (checkMcp), environment/profile mcpUrl wiring, CLI wiring, comprehensive tests, and documentation including a changeset and README updates. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Comment |
49da9bd to
173bdb4
Compare
|
!snapshot |
b4bc06c to
4f53e9f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/cli-core/src/commands/mcp/clients/user-scope.test.ts (1)
2-2: ⚡ Quick winUse Bun file APIs here instead of
node:fsread/write helpers.These tests currently use
readFile/writeFilefromnode:fs/promises, which goes against the repo’s Bun-specific file I/O guideline.Bun.file(...).json()/text()andBun.write(...)would keep this aligned with the rest of the codebase. As per coding guidelines,**/*.{ts,tsx,js,jsx}: PreferBun.fileovernode:fs's readFile/writeFile for file operations.Also applies to: 45-46, 64-66
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli-core/src/commands/mcp/clients/user-scope.test.ts` at line 2, Replace node:fs/promises read/write helpers imported as mkdir, mkdtemp, readFile, rm, writeFile in user-scope.test.ts with Bun file APIs: use Bun.file(path).text()/json() for reads and Bun.write/Bun.writeSync (or Bun.file(path).write(...)) for writes, and keep using mkdtemp/mkdir/rm equivalents via Bun or the existing helpers if available; update all call sites that use readFile/writeFile (and any promise-based usages) to the corresponding Bun.file(...).text()/json() and Bun.write(...) patterns so the tests follow the repo’s Bun-specific I/O guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli-core/src/commands/mcp/clients/user-scope.test.ts`:
- Around line 7-12: The top-level module mock of node:os (mock.module("node:os",
...)) is never torn down causing cross-test leakage; add a cleanup via
afterAll(() => mock.restoreModule("node:os")) (or equivalent) to restore the
original module and avoid order-dependent failures, and update file I/O in this
test to use Bun APIs instead of node:fs/promises—replace readFile/writeFile
calls with Bun.file(path).json()/text() for reads and Bun.file(path).write(...)
or Bun.write(...) for writes so the test uses Bun's file operations correctly;
reference mock.module, mock.restoreModule, mockHome and the test file's existing
fs read/write usages when making the changes.
---
Nitpick comments:
In `@packages/cli-core/src/commands/mcp/clients/user-scope.test.ts`:
- Line 2: Replace node:fs/promises read/write helpers imported as mkdir,
mkdtemp, readFile, rm, writeFile in user-scope.test.ts with Bun file APIs: use
Bun.file(path).text()/json() for reads and Bun.write/Bun.writeSync (or
Bun.file(path).write(...)) for writes, and keep using mkdtemp/mkdir/rm
equivalents via Bun or the existing helpers if available; update all call sites
that use readFile/writeFile (and any promise-based usages) to the corresponding
Bun.file(...).text()/json() and Bun.write(...) patterns so the tests follow the
repo’s Bun-specific I/O guideline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 33d9756c-11d6-436b-8465-7e750f438029
📒 Files selected for processing (34)
.changeset/mcp-install.mdREADME.mdpackages/cli-core/src/cli-program.tspackages/cli-core/src/commands/doctor/README.mdpackages/cli-core/src/commands/doctor/check-mcp.tspackages/cli-core/src/commands/doctor/context.test.tspackages/cli-core/src/commands/doctor/index.tspackages/cli-core/src/commands/mcp/README.mdpackages/cli-core/src/commands/mcp/clients/claude-code.tspackages/cli-core/src/commands/mcp/clients/clients.test.tspackages/cli-core/src/commands/mcp/clients/cursor.tspackages/cli-core/src/commands/mcp/clients/gemini.tspackages/cli-core/src/commands/mcp/clients/json-config.tspackages/cli-core/src/commands/mcp/clients/make-json-client.test.tspackages/cli-core/src/commands/mcp/clients/make-json-client.tspackages/cli-core/src/commands/mcp/clients/paths.tspackages/cli-core/src/commands/mcp/clients/registry.tspackages/cli-core/src/commands/mcp/clients/types.tspackages/cli-core/src/commands/mcp/clients/user-scope.test.tspackages/cli-core/src/commands/mcp/clients/vscode.tspackages/cli-core/src/commands/mcp/clients/windsurf.tspackages/cli-core/src/commands/mcp/collect.tspackages/cli-core/src/commands/mcp/index.tspackages/cli-core/src/commands/mcp/install.test.tspackages/cli-core/src/commands/mcp/install.tspackages/cli-core/src/commands/mcp/list.test.tspackages/cli-core/src/commands/mcp/list.tspackages/cli-core/src/commands/mcp/probe.test.tspackages/cli-core/src/commands/mcp/probe.tspackages/cli-core/src/commands/mcp/shared.tspackages/cli-core/src/commands/mcp/uninstall.test.tspackages/cli-core/src/commands/mcp/uninstall.tspackages/cli-core/src/lib/environment.tspackages/cli-core/src/lib/errors.ts
✅ Files skipped from review due to trivial changes (4)
- .changeset/mcp-install.md
- packages/cli-core/src/commands/doctor/README.md
- README.md
- packages/cli-core/src/commands/mcp/README.md
🚧 Files skipped from review as they are similar to previous changes (26)
- packages/cli-core/src/commands/mcp/list.test.ts
- packages/cli-core/src/commands/mcp/clients/registry.ts
- packages/cli-core/src/commands/mcp/index.ts
- packages/cli-core/src/commands/mcp/clients/claude-code.ts
- packages/cli-core/src/commands/mcp/clients/cursor.ts
- packages/cli-core/src/commands/mcp/list.ts
- packages/cli-core/src/commands/mcp/clients/vscode.ts
- packages/cli-core/src/commands/doctor/check-mcp.ts
- packages/cli-core/src/commands/mcp/clients/gemini.ts
- packages/cli-core/src/commands/mcp/clients/windsurf.ts
- packages/cli-core/src/commands/mcp/clients/types.ts
- packages/cli-core/src/commands/mcp/probe.test.ts
- packages/cli-core/src/lib/errors.ts
- packages/cli-core/src/commands/doctor/index.ts
- packages/cli-core/src/commands/mcp/clients/paths.ts
- packages/cli-core/src/commands/mcp/install.ts
- packages/cli-core/src/lib/environment.ts
- packages/cli-core/src/commands/mcp/collect.ts
- packages/cli-core/src/commands/mcp/uninstall.ts
- packages/cli-core/src/commands/mcp/clients/clients.test.ts
- packages/cli-core/src/commands/mcp/probe.ts
- packages/cli-core/src/commands/mcp/install.test.ts
- packages/cli-core/src/cli-program.ts
- packages/cli-core/src/commands/mcp/clients/json-config.ts
- packages/cli-core/src/commands/mcp/clients/make-json-client.test.ts
- packages/cli-core/src/commands/mcp/uninstall.test.ts
a6e7532 to
bdf8faf
Compare
|
!snapshot |
Snapshot publishednpm install -g clerk@2.0.1-snapshot.bdf8faf
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli-core/src/commands/mcp/clients/clients.test.ts`:
- Around line 65-96: The VS Code test is leaking to the real config because
vscodeUserDir() respects XDG_CONFIG_HOME/APPDATA; update the tests to isolate
that by setting XDG_CONFIG_HOME and APPDATA to a directory under mockHome (or
delete them) before running the VS Code-specific assertions and restore them
after; ensure the expectation for the VS Code path does not call vscodeUserDir()
(i.e. compute the expected path explicitly inside the test instead of reusing
expectedPath()/vscodeUserDir()) so the test verifies the function under test
rather than mirroring the same helper.
In `@packages/cli-core/src/commands/mcp/clients/paths.ts`:
- Around line 26-35: vscodeUserDir currently uses the nullish coalescing
operator (??) with process.env.APPDATA and process.env.XDG_CONFIG_HOME which
treats empty strings as set; update vscodeUserDir to treat empty or
whitespace-only env vars as unset by checking truthiness/trim (e.g. derive local
vars like appData = process.env.APPDATA?.trim() ? process.env.APPDATA :
undefined and xdg = process.env.XDG_CONFIG_HOME?.trim() ?
process.env.XDG_CONFIG_HOME : undefined) and then fall back to join(homedir(),
...) in the switch cases; reference the vscodeUserDir function and its uses of
process.env.APPDATA and process.env.XDG_CONFIG_HOME, and ensure platform(),
homedir(), and join() behavior is preserved.
In `@packages/cli-core/src/lib/environment.ts`:
- Around line 154-165: getMcpUrl currently lets an empty CLERK_MCP_URL ("") win
due to the nullish coalescing operator; treat empty/whitespace-only env values
as unset by trimming and checking truthiness before falling back. Update
getMcpUrl (function name: getMcpUrl) to compute const env =
process.env.CLERK_MCP_URL?.trim(); then return env && env.length ? env :
getCurrentEnv().mcpUrl ?? DEFAULT_MCP_URL so blank overrides no longer produce
an unusable empty string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2826916f-5886-46c5-96c7-da4b8e7b1a92
📒 Files selected for processing (23)
.changeset/mcp-install.mdpackages/cli-core/src/cli-program.tspackages/cli-core/src/commands/doctor/check-mcp.tspackages/cli-core/src/commands/mcp/README.mdpackages/cli-core/src/commands/mcp/clients/claude-code.tspackages/cli-core/src/commands/mcp/clients/clients.test.tspackages/cli-core/src/commands/mcp/clients/cursor.tspackages/cli-core/src/commands/mcp/clients/gemini.tspackages/cli-core/src/commands/mcp/clients/json-config.tspackages/cli-core/src/commands/mcp/clients/make-json-client.test.tspackages/cli-core/src/commands/mcp/clients/make-json-client.tspackages/cli-core/src/commands/mcp/clients/paths.tspackages/cli-core/src/commands/mcp/clients/user-scope.test.tspackages/cli-core/src/commands/mcp/clients/vscode.tspackages/cli-core/src/commands/mcp/clients/windsurf.tspackages/cli-core/src/commands/mcp/install.test.tspackages/cli-core/src/commands/mcp/install.tspackages/cli-core/src/commands/mcp/list.test.tspackages/cli-core/src/commands/mcp/list.tspackages/cli-core/src/commands/mcp/shared.tspackages/cli-core/src/commands/mcp/uninstall.test.tspackages/cli-core/src/lib/environment.tspackages/cli-core/src/lib/errors.ts
💤 Files with no reviewable changes (3)
- packages/cli-core/src/commands/mcp/clients/gemini.ts
- packages/cli-core/src/commands/mcp/clients/windsurf.ts
- packages/cli-core/src/cli-program.ts
✅ Files skipped from review due to trivial changes (2)
- .changeset/mcp-install.md
- packages/cli-core/src/commands/mcp/README.md
🚧 Files skipped from review as they are similar to previous changes (12)
- packages/cli-core/src/commands/mcp/clients/cursor.ts
- packages/cli-core/src/lib/errors.ts
- packages/cli-core/src/commands/mcp/clients/vscode.ts
- packages/cli-core/src/commands/mcp/list.ts
- packages/cli-core/src/commands/doctor/check-mcp.ts
- packages/cli-core/src/commands/mcp/uninstall.test.ts
- packages/cli-core/src/commands/mcp/clients/make-json-client.test.ts
- packages/cli-core/src/commands/mcp/clients/user-scope.test.ts
- packages/cli-core/src/commands/mcp/shared.ts
- packages/cli-core/src/commands/mcp/clients/json-config.ts
- packages/cli-core/src/commands/mcp/clients/make-json-client.ts
- packages/cli-core/src/commands/mcp/install.test.ts
b017038 to
a9840ca
Compare
|
!snapshot |
Snapshot publishednpm install -g clerk@2.0.1-snapshot.d637eb0
|
8a62c01 to
42756ac
Compare
19b2820 to
0d5281d
Compare
wyattjoh
left a comment
There was a problem hiding this comment.
Solid, well-tested feature surface. The error handling, URL hardening, and per-client config abstraction are all carefully done. A few non-blocking observations below, mostly around concurrency edge cases in the stdio bridge and the shared ~/.claude.json write path.
35a40ac to
240ce80
Compare
161868b to
c13e55a
Compare
wyattjoh
left a comment
There was a problem hiding this comment.
Strong, well-tested PR. The main cleanup is leftover --url surface from the "drop --url from public CLI" commit (a help example that now errors, an unactionable error message, the README, and the changeset), plus a couple of narrow correctness edge cases noted inline.
c13e55a to
a99ba57
Compare
6397087 to
4823b0d
Compare
c70a7f1 to
89d5811
Compare
6ac2848 to
e42209c
Compare
… + doctor MCP check Registers the Clerk remote MCP server (https://mcp.clerk.com/mcp) in AI editors and agent CLIs, with an MCP reachability check in `clerk doctor`. Supported clients (all user-global): Claude Code, Cursor, GitHub Copilot / VS Code (`--client vscode` or `copilot`), Windsurf, Gemini, Codex, opencode, OpenClaw, Warp, and Hermes Agent. Registration is delegated to each client's own CLI where a non-interactive one exists (`claude`/`gemini`/`codex`/`openclaw`/`hermes` `mcp add`, `code --add-mcp`), with PATH-based detection and no file-write fallback; Cursor, Windsurf, Warp, and opencode (interactive-only CLI) get atomic file writes. Install always converges via remove-then-add. Client CLIs are spawned with stdin closed and a 15s timeout; Windows `.cmd` shims run through `cmd.exe /c`; Hermes gets its confirm prompt answered via stdin and a post-add verification because its cancel path exits 0 without saving. Every client stores the same URL-less bridge descriptor `{command: "clerk", args: ["mcp", "run"]}`. `clerk mcp run` is a built-in stdio<->Streamable-HTTP bridge (replaces npx mcp-remote) that resolves its target at spawn time (CLERK_MCP_URL > env profile mcpUrl > hosted default), serializes stdout frames, caps both stdin lines and upstream SSE/JSON bodies at 16 MiB, and surfaces pre-session 401/403 as a clear error while answering in-session auth failures per-request. Reads stay file-based (JSON/TOML/YAML codecs; TOML and YAML are read-only since those clients' CLIs own both mutations). `--name` is allowlisted ([A-Za-z0-9_-], no leading dash) because it is spliced into client CLI argv and used as a config key. `list --json` reports { entries, failures } so an unreadable config surfaces structurally; install/uninstall carry the same failures array and exit non-zero only when every targeted client fails. Agent mode never prompts and always emits JSON with machine-readable error codes plus a docs URL (clerk.com/docs/guides/ai/mcp/clerk-mcp-server). Verified live against the real client CLIs on macOS: full uninstall/install round-trips with zero failures; opencode reports "clerk connected" and Hermes "clerk enabled" through their own CLIs; OpenClaw's dialect verified in a sandboxed OPENCLAW_STATE_DIR; bridge handshake against the hosted server returns "Clerk MCP Server".
e42209c to
42ad04d
Compare
…verified removes, SDK-typed tests
Summary
Adds a
clerk mcpcommand group that connects the Clerk remote MCP server (https://mcp.clerk.com/mcp) to 10 AI clients, plus an MCP health check inclerk doctor.clerk mcp install— register the server. Human mode: interactive multiselect over detected clients; agent mode /--all/--client <id>(repeatable): non-interactive. Install always converges: whatever entry sits under the name is replaced (remove-then-add through the client's CLI, or an overwrite for file-backed clients) — there is no--forceand no conflict state.clerk mcp list— show every Clerk-flavored entry (namedclerkor pointing at a*.clerk.comhost). JSON output is{ entries, failures }, so an unreadable client config surfaces structurally instead of reading as "nothing installed".clerk mcp uninstall— remove the entry. Human mode prompts with the clients that actually have it (nothing pre-checked); agent mode /--alltargets every client. Nothing matched → warm hint, exit 0.clerk mcp run— a built-in stdio↔Streamable-HTTP bridge that editors spawn (replacesnpx mcp-remote; no npx dependency). Every client config stores the same URL-less descriptor{ "command": "clerk", "args": ["mcp", "run"] }; the bridge resolves its target at spawn time (CLERK_MCP_URL> env profilemcpUrl> hosted default).clerk doctor— MCP check probes each distinct configured URL via theinitializehandshake when an entry is installed (skips otherwise; warns, never fails), and distinguishes "config unreadable" from "nothing installed".Registration is delegated to each client's own CLI
Clients that ship a non-interactive registration CLI are driven through it — the client owns its config format and write safety. The rest get direct file writes. There is no file-write fallback for CLI-backed clients: a missing binary fails that client with an actionable error (
mcp_client_cli_not_found+ docs link), and detection is PATH-based so the picker only offers clients we can actually drive.--clientidclaudeclaude mcp add --scope userclaude mcp removecursorvscode(copilot)code --add-mcp '<json>'windsurfgeminigemini mcp add --scope usergemini mcp removecodexcodex mcp add(TOML config)codex mcp removeopencodemcp addis an interactive-only wizard)openclawopenclaw mcp add --no-probeopenclaw mcp unsetwarp~/.warp/.mcp.json(no registration CLI)hermeshermes mcp add(stdin-piped confirm + post-add verify)hermes mcp removeAll entries are user-global, so the server is available in every project.
Design notes
.cmd/.batshims are launched throughcmd.exe /c.mcp addends in a confirm prompt and exits 0 on cancel without saving — so the affirmative answer is piped to stdin and the config is re-read after add; a silent no-op becomesmcp_client_cli_failed. Its YAML config is read-only to us (both mutations go through its CLI), so hand-written YAML is never rewritten.list,doctor, and the uninstall picker parse config files directly (JSON/TOML/YAML codecs, incl. OpenClaw's nestedmcp.serversmap). Legacy descriptor shapes ({url},{serverUrl},mcp-remoteargs,clerk mcp run --url) still round-trip.codeplus adocsUrlto the Clerk MCP setup docs (agent mode gets the.mdvariant) for the manual fallback.failures: [{ client, error }]array in install/uninstall/list JSON; the command exits non-zero only when every targeted client fails.Test plan
bun run format:check,bun run lint,bun run typecheck— greenbun run test— 2095 tests green, including per-client argv contracts for all six CLI dialects, file shapes for the four file-backed clients, nested-topKey + YAML codec units, corrupt-config failure threading, Hermes verify-add, and Windows.cmd-shim spawningclaude/gemini/codex/code(duplicate-add/remove semantics),hermes(prompt + lying exit code discovered this way),openclaw(sandboxedOPENCLAW_STATE_DIR)uninstall --all→install --allacross all detected clients with zero failures; opencode's own CLI reports✓ clerk connectedand Hermes reportsclerk ✓ enabled; bridge handshake against the hosted server returnsClerk MCP Server