Skip to content

Commit edc1122

Browse files
committed
refactor(mcp): adopt Clack gutter/rail conventions for install/list/uninstall
Follow the patterns the #305 Clack migration established: wrap install/list/uninstall human flows in withGutter() (guaranteed intro/outro cleanup, paused/failed outros on abort/error), render next steps via outro([...]) instead of a custom printNextSteps helper, and render the list table through the ui.* prompt rail (ui.message) like apps/list. No behavior change to JSON/agent output.
1 parent 8bbaa81 commit edc1122

7 files changed

Lines changed: 80 additions & 92 deletions

File tree

packages/cli-core/src/commands/mcp/install.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,21 +171,21 @@ describe("mcp install", () => {
171171

172172
test("prints next steps with a sign-in reminder after a human-mode install", async () => {
173173
await mcpInstall({ client: ["cursor"], url: URL_A });
174-
expect(captured.err).toContain("Next steps:");
174+
expect(captured.err).toContain("Next steps");
175175
expect(captured.err).toContain("Reload Cursor");
176176
expect(captured.err).toContain("sign in");
177177
});
178178

179179
test("omits next steps from JSON output", async () => {
180180
await mcpInstall({ client: ["cursor"], url: URL_A, json: true });
181-
expect(captured.err).not.toContain("Next steps:");
181+
expect(captured.err).not.toContain("Next steps");
182182
});
183183

184184
test("does not print next steps when the entry was unchanged", async () => {
185185
await mcpInstall({ client: ["cursor"], url: URL_A });
186186
captured.clear();
187187
await mcpInstall({ client: ["cursor"], url: URL_A });
188-
expect(captured.err).not.toContain("Next steps:");
188+
expect(captured.err).not.toContain("Next steps");
189189
});
190190

191191
test("rejects an unknown --client id", async () => {

packages/cli-core/src/commands/mcp/install.ts

Lines changed: 24 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,10 @@
88

99
import { log } from "../../lib/log.ts";
1010
import { cyan, dim, green, yellow } from "../../lib/color.ts";
11-
import { isAgent, isHuman } from "../../mode.ts";
12-
import { intro, outro } from "../../lib/spinner.ts";
11+
import { isAgent } from "../../mode.ts";
12+
import { withGutter } from "../../lib/spinner.ts";
1313
import {
1414
pickClients,
15-
printNextSteps,
1615
resolveName,
1716
resolveUrl,
1817
settleClients,
@@ -55,18 +54,15 @@ type ClientUpsert = { client: McpClient; result: UpsertResult };
5554
// Writing the config isn't enough — the editor must reload before it connects
5655
// (and sign in, if the server requires it). Surface that for every client we
5756
// just wrote, so "added" doesn't read as "done and working".
58-
function printInstallNextSteps(settled: ClientUpsert[]): void {
57+
function installNextSteps(settled: ClientUpsert[]): string[] {
5958
const activated = settled.filter(
6059
({ result }) => result.status === "added" || result.status === "updated",
6160
);
62-
if (activated.length === 0) return;
63-
64-
printNextSteps(activated.map(({ client }) => `${client.displayName}: ${client.activation}`));
65-
log.info(
66-
dim(
67-
"If the server requires authentication, your editor opens a browser to sign in on first connect.",
68-
),
69-
);
61+
if (activated.length === 0) return [];
62+
return [
63+
...activated.map(({ client }) => `${client.displayName}: ${client.activation}`),
64+
"If the server requires authentication, your editor opens a browser to sign in on first connect.",
65+
];
7066
}
7167

7268
export async function mcpInstall(options: McpOptions = {}): Promise<void> {
@@ -77,26 +73,24 @@ export async function mcpInstall(options: McpOptions = {}): Promise<void> {
7773
const force = Boolean(options.force);
7874
const json = wantsJson(options);
7975

80-
if (clients.length === 0 && json) {
81-
log.data(JSON.stringify({ url, name, results: [] }, null, 2));
82-
return;
83-
}
8476
if (clients.length === 0) {
85-
log.warn("No MCP clients selected.");
86-
return;
87-
}
88-
89-
if (isHuman() && !json) intro(`Installing Clerk MCP (${cyan(url)})`);
90-
91-
const settled = await settleClients(clients, (c) => c.upsert({ name, url }, cwd, force));
92-
const results = settled.map((s) => s.result);
93-
94-
if (json) {
95-
log.data(JSON.stringify({ url, name, results }, null, 2));
77+
if (json) log.data(JSON.stringify({ url, name, results: [] }, null, 2));
78+
else log.warn("No MCP clients selected.");
9679
return;
9780
}
9881

99-
settled.forEach(({ client, result }) => printResult(client, result));
100-
printInstallNextSteps(settled);
101-
outro("Done");
82+
await withGutter(
83+
`Installing Clerk MCP (${cyan(url)})`,
84+
async ({ setNextSteps }) => {
85+
const settled = await settleClients(clients, (c) => c.upsert({ name, url }, cwd, force));
86+
if (json) {
87+
log.data(JSON.stringify({ url, name, results: settled.map((s) => s.result) }, null, 2));
88+
return;
89+
}
90+
settled.forEach(({ client, result }) => printResult(client, result));
91+
const steps = installNextSteps(settled);
92+
if (steps.length > 0) setNextSteps(steps);
93+
},
94+
{ skip: json },
95+
);
10296
}

packages/cli-core/src/commands/mcp/list.test.ts

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
22
import { mkdtemp, rm } from "node:fs/promises";
33
import * as realOs from "node:os";
44
import { join } from "node:path";
5-
import { useCaptureLog } from "../../test/lib/stubs.ts";
5+
import { captureUi, useCaptureLog } from "../../test/lib/stubs.ts";
66

77
const mockIsAgent = mock();
88
mock.module("../../mode.ts", () => ({
@@ -24,6 +24,7 @@ const URL = "https://mcp.clerk.com/mcp";
2424

2525
describe("mcp list", () => {
2626
const captured = useCaptureLog();
27+
let uiCapture: ReturnType<typeof captureUi>;
2728
let cwd: string;
2829
let originalCwd: string;
2930

@@ -32,10 +33,13 @@ describe("mcp list", () => {
3233
cwd = await mkdtemp(join(realOs.tmpdir(), "clerk-mcp-list-"));
3334
mockHome = cwd;
3435
process.chdir(cwd);
36+
uiCapture = captureUi();
37+
uiCapture.install();
3538
mockIsAgent.mockReturnValue(true);
3639
});
3740

3841
afterEach(async () => {
42+
uiCapture.teardown();
3943
process.chdir(originalCwd);
4044
await rm(cwd, { recursive: true, force: true });
4145
mockIsAgent.mockReset();
@@ -60,25 +64,27 @@ describe("mcp list", () => {
6064
]);
6165
});
6266

63-
test("human-mode empty state writes the hint to stderr, nothing to stdout", async () => {
67+
test("human-mode empty state shows the hint on the prompt rail, nothing to stdout", async () => {
6468
mockIsAgent.mockReturnValue(false);
6569
await mcpList({});
6670
expect(captured.out).toBe("");
67-
expect(captured.err).toContain("No Clerk MCP entries");
71+
expect(uiCapture.out).toContain("No Clerk MCP entries");
6872
});
6973

70-
test("human-mode prints the formatted table to stdout after an install", async () => {
74+
test("human-mode renders the table and next steps after an install", async () => {
7175
mockIsAgent.mockReturnValue(true);
7276
await mcpInstall({ client: ["cursor"], url: URL });
7377
mockIsAgent.mockReturnValue(false);
7478
captured.clear();
7579
await mcpList({});
7680

77-
expect(captured.out).toContain("cursor");
78-
expect(captured.out).toContain("clerk");
79-
expect(captured.out).toContain(URL);
80-
expect(captured.err).toContain("1 entry");
81-
expect(captured.err).toContain("Next steps:");
82-
expect(captured.err).toContain("clerk doctor");
81+
// Table, count, and the outro next-steps all render on the clack prompt
82+
// rail; with captureUi installed, intro/outro write to that stream too.
83+
expect(uiCapture.out).toContain("cursor");
84+
expect(uiCapture.out).toContain("clerk");
85+
expect(uiCapture.out).toContain(URL);
86+
expect(uiCapture.out).toContain("1 entry");
87+
expect(uiCapture.out).toContain("Next steps");
88+
expect(uiCapture.out).toContain("clerk doctor");
8389
});
8490
});

packages/cli-core/src/commands/mcp/list.ts

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@
77

88
import { cyan, dim } from "../../lib/color.ts";
99
import { log } from "../../lib/log.ts";
10+
import { withGutter } from "../../lib/spinner.ts";
11+
import { ui } from "../../lib/ui.ts";
1012
import type { ListEntry } from "./clients/types.ts";
1113
import { collectEntries } from "./collect.ts";
12-
import { printNextSteps, wantsJson, type McpOptions } from "./shared.ts";
14+
import { wantsJson, type McpOptions } from "./shared.ts";
1315

1416
const COLUMN_PADDING = 2;
1517

@@ -31,36 +33,35 @@ function formatTable(entries: ListEntry[]): void {
3133
entries.map((e) => e.url),
3234
);
3335

34-
log.data(
35-
dim(`${"CLIENT".padEnd(clientWidth)}${"NAME".padEnd(nameWidth)}${"URL".padEnd(urlWidth)}PATH`),
36-
);
37-
for (const entry of entries) {
38-
const client = cyan(entry.client.padEnd(clientWidth));
39-
const name = entry.name.padEnd(nameWidth);
40-
const url = entry.url.padEnd(urlWidth);
41-
log.data(`${client}${name}${url}${dim(entry.configPath)}`);
42-
}
36+
const header = `${"CLIENT".padEnd(clientWidth)}${"NAME".padEnd(nameWidth)}${"URL".padEnd(urlWidth)}PATH`;
37+
const rows = entries.map((e) => {
38+
const client = cyan(e.client.padEnd(clientWidth));
39+
const name = e.name.padEnd(nameWidth);
40+
const url = e.url.padEnd(urlWidth);
41+
return `${client}${name}${url}${dim(e.configPath)}`;
42+
});
43+
44+
ui.message([dim(header), ...rows]);
4345
}
4446

4547
export async function mcpList(options: McpOptions = {}): Promise<void> {
46-
const cwd = process.cwd();
47-
const all: ListEntry[] = await collectEntries(cwd);
48+
const all: ListEntry[] = await collectEntries(process.cwd());
4849

4950
if (wantsJson(options)) {
5051
log.data(JSON.stringify(all, null, 2));
5152
return;
5253
}
5354

54-
if (all.length === 0) {
55-
log.warn("No Clerk MCP entries found. Run `clerk mcp install` to register one.");
56-
return;
57-
}
58-
59-
formatTable(all);
60-
log.info(`\n${all.length} entr${all.length === 1 ? "y" : "ies"}`);
61-
62-
printNextSteps([
63-
"Verify a server is reachable with `clerk doctor`.",
64-
"Remove an entry with `clerk mcp uninstall`.",
65-
]);
55+
await withGutter("Listing Clerk MCP entries", async ({ setNextSteps }) => {
56+
if (all.length === 0) {
57+
ui.warn("No Clerk MCP entries found. Run `clerk mcp install` to register one.");
58+
return;
59+
}
60+
formatTable(all);
61+
ui.message(`${all.length} entr${all.length === 1 ? "y" : "ies"}`);
62+
setNextSteps([
63+
"Verify a server is reachable with `clerk doctor`.",
64+
"Remove an entry with `clerk mcp uninstall`.",
65+
]);
66+
});
6667
}

packages/cli-core/src/commands/mcp/shared.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -98,14 +98,6 @@ export function wantsJson(options: McpOptions): boolean {
9898
return Boolean(options.json) || isAgent();
9999
}
100100

101-
/** Render a "Next steps:" block to stderr (human mode). No-op for an empty list. */
102-
export function printNextSteps(lines: string[]): void {
103-
if (lines.length === 0) return;
104-
log.blank();
105-
log.info("Next steps:");
106-
for (const line of lines) log.info(` ${line}`);
107-
}
108-
109101
/**
110102
* Run an async op against each client without letting one client's failure
111103
* abort the rest — `Promise.all` is fail-fast and would discard every other

packages/cli-core/src/commands/mcp/uninstall.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ describe("mcp uninstall", () => {
101101
mockIsAgent.mockReturnValue(false);
102102
captured.clear();
103103
await mcpUninstall({ client: ["cursor"] });
104-
expect(captured.err).toContain("Next steps:");
104+
expect(captured.err).toContain("Next steps");
105105
expect(captured.err).toContain("Reload Cursor");
106106
});
107107
});

packages/cli-core/src/commands/mcp/uninstall.ts

Lines changed: 11 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,10 @@
55
import { cyan, dim, green, yellow } from "../../lib/color.ts";
66
import { CliError, ERROR_CODE } from "../../lib/errors.ts";
77
import { log } from "../../lib/log.ts";
8-
import { isHuman } from "../../mode.ts";
9-
import { intro, outro } from "../../lib/spinner.ts";
8+
import { withGutter } from "../../lib/spinner.ts";
109
import { CLIENTS } from "./clients/registry.ts";
1110
import type { McpClient, RemoveResult } from "./clients/types.ts";
1211
import {
13-
printNextSteps,
1412
resolveClients,
1513
resolveName,
1614
settleClients,
@@ -32,12 +30,8 @@ export async function mcpUninstall(options: McpOptions = {}): Promise<void> {
3230
: Array.from(CLIENTS);
3331
const json = wantsJson(options);
3432

35-
if (isHuman() && !json) intro(`Removing MCP entry ${cyan(name)}`);
36-
3733
const settled = await settleClients(clients, (c) => c.remove(name, cwd));
3834
const results = settled.map((s) => s.result);
39-
if (isHuman() && !json) settled.forEach(({ client, result }) => printResult(client, result));
40-
4135
const removedCount = results.filter((r) => r.removed).length;
4236
const notInstalled = new CliError(`No MCP entry named "${name}" found in any client.`, {
4337
code: ERROR_CODE.MCP_NOT_INSTALLED,
@@ -49,14 +43,15 @@ export async function mcpUninstall(options: McpOptions = {}): Promise<void> {
4943
return;
5044
}
5145

52-
if (removedCount === 0) throw notInstalled;
53-
5446
// Removing the config entry doesn't drop a live editor session — it lingers
55-
// until the editor reloads. Tell the user for each client we removed from.
56-
printNextSteps(
57-
settled
58-
.filter(({ result }) => result.removed)
59-
.map(({ client }) => `Reload ${client.displayName} to drop the active connection.`),
60-
);
61-
outro("Done");
47+
// until the editor reloads. Surface that as a next step per removed client.
48+
await withGutter(`Removing MCP entry ${cyan(name)}`, async ({ setNextSteps }) => {
49+
if (removedCount === 0) throw notInstalled;
50+
settled.forEach(({ client, result }) => printResult(client, result));
51+
setNextSteps(
52+
settled
53+
.filter(({ result }) => result.removed)
54+
.map(({ client }) => `Reload ${client.displayName} to drop the active connection.`),
55+
);
56+
});
6257
}

0 commit comments

Comments
 (0)