Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 32 additions & 2 deletions apps/desktop/src/lib/trpc/routers/browser-automation/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { EventEmitter } from "node:events";
import { readFileSync } from "node:fs";
import { existsSync, readFileSync } from "node:fs";
import { homedir } from "node:os";
import { join } from "node:path";
import { join, resolve } from "node:path";
import {
browserAutomationBindings,
projects,
Expand All @@ -11,6 +11,7 @@ import {
} from "@superset/local-db";
import { observable } from "@trpc/server/observable";
import { and, eq, ne } from "drizzle-orm";
import { app } from "electron";
import { localDb } from "main/lib/local-db";
import { getProcessName, getProcessTree } from "main/lib/terminal/port-scanner";
import { getTerminalHostClient } from "main/lib/terminal-host/client";
Expand Down Expand Up @@ -288,6 +289,33 @@ const CLAUDE_SETTINGS_JSON_PATH = join(homedir(), ".claude", "settings.json");
const CLAUDE_CONFIG_PATHS = [CLAUDE_USER_JSON_PATH, CLAUDE_SETTINGS_JSON_PATH];
const CODEX_CONFIG_PATH = join(homedir(), ".codex", "config.toml");

/**
* Resolve the absolute command Claude / Codex should spawn to start the
* superset-browser MCP server. `desktop-mcp` is a workspace-private bin
* (TypeScript, no compiled output), so it is not on PATH. We point
* users at `bun <repo>/packages/desktop-mcp/src/bin.ts` instead — bun
* can execute TypeScript directly.
*
* In packaged production builds the source tree isn't available, so we
* fall back to the bare `desktop-mcp` name and let the user pick the
* command themselves. (Browser automation is dev-only today.)
*/
function resolveDesktopMcpCommand(): {
command: string;
args: string[];
available: boolean;
} {
if (!app.isPackaged) {
// `app.getAppPath()` is the `apps/desktop` directory in dev.
const repoRoot = resolve(app.getAppPath(), "../..");
const binPath = join(repoRoot, "packages/desktop-mcp/src/bin.ts");
if (existsSync(binPath)) {
return { command: "bun", args: [binPath], available: true };
}
}
return { command: "desktop-mcp", args: [], available: false };
}

export interface TerminalAgentSession {
paneId: string;
workspaceId: string;
Expand Down Expand Up @@ -374,12 +402,14 @@ export const createBrowserAutomationRouter = () => {
claudeReadyByWorkspaceId[workspaceId] = localScope || projectScope;
}
const codexReady = detectCodexMcp(CODEX_CONFIG_PATH);
const serverCommand = resolveDesktopMcpCommand();
return {
claudeHomeReady,
claudeReadyByWorkspaceId,
codexReady,
claudeConfigPath: CLAUDE_USER_JSON_PATH,
codexConfigPath: CODEX_CONFIG_PATH,
serverCommand,
};
}),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { electronTrpc } from "renderer/lib/electron-trpc";
import {
type AutomationSession,
getSnippetForSession,
type ServerCommand,
useBrowserAutomationStore,
} from "renderer/stores/browser-automation";
import { useTabsStore } from "renderer/stores/tabs/store";
Expand Down Expand Up @@ -143,10 +144,14 @@ export function SessionConnectModal({
}
};

const serverCommand = mcpStatus?.serverCommand;

const handleCopySnippet = async () => {
if (!session) return;
try {
await navigator.clipboard.writeText(getSnippetForSession(session));
await navigator.clipboard.writeText(
getSnippetForSession(session, serverCommand),
);
Comment on lines +147 to +154
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

serverCommand.available を尊重して、失敗するスニペットのコピーを防いでください。

mcpStatus が未ロード、または resolver が available: false を返した場合でも、現在は getSnippetForSession のフォールバックで bare desktop-mcp が表示/コピーされます。これは今回直した /mcp failed の経路を再発させます。

修正案
-	const serverCommand = mcpStatus?.serverCommand;
+	const serverCommand = mcpStatus?.serverCommand ?? null;

 	const handleCopySnippet = async () => {
-		if (!session) return;
+		if (!session) return;
+		if (!serverCommand?.available) {
+			toast.error("MCP server command is still unavailable");
+			return;
+		}
 		try {
 			await navigator.clipboard.writeText(
 				getSnippetForSession(session, serverCommand),
 			);
 								<SetupPanel
 									session={session}
 									mcpConfigPath={
 										session.provider === "Codex"
 											? (mcpStatus?.codexConfigPath ?? null)
 											: (mcpStatus?.claudeConfigPath ?? null)
 									}
 									serverCommand={serverCommand}
 									onCopy={handleCopySnippet}
 								/>
 function SetupPanel({
 	session,
 	mcpConfigPath,
 	serverCommand,
 	onCopy,
 }: {
 	session: AutomationSession;
 	mcpConfigPath: string | null;
-	serverCommand?: ServerCommand;
+	serverCommand: ServerCommand | null;
 	onCopy: () => void;
 }) {
-	const snippet = getSnippetForSession(session, serverCommand);
+	const snippet = serverCommand?.available
+		? getSnippetForSession(session, serverCommand)
+		: "MCP server command is not available in this build.";
-					<Button size="sm" variant="outline" onClick={onCopy}>
+					<Button
+						size="sm"
+						variant="outline"
+						onClick={onCopy}
+						disabled={!serverCommand?.available}
+					>

Also applies to: 252-253, 457-465, 517-520

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/BrowserPane/components/SessionConnectModal/SessionConnectModal.tsx`
around lines 147 - 154, The copy/snippet flow ignores mcpStatus/resolver
availability and may produce a failing "desktop-mcp" snippet; update
handleCopySnippet (and the other similar handlers that call
getSnippetForSession) to check mcpStatus?.serverCommand?.available (or
serverCommand.available) before calling getSnippetForSession and prevent
copying/show a clear error / disable the copy action when available is false or
mcpStatus is not loaded; ensure you reference serverCommand and
getSnippetForSession so the guard is applied everywhere this snippet is
generated.

toast.success("Configuration snippet copied");
} catch {
toast.error("Failed to copy snippet");
Expand Down Expand Up @@ -244,6 +249,7 @@ export function SessionConnectModal({
? (mcpStatus?.codexConfigPath ?? null)
: (mcpStatus?.claudeConfigPath ?? null)
}
serverCommand={serverCommand}
onCopy={handleCopySnippet}
/>
)
Expand Down Expand Up @@ -448,13 +454,15 @@ function DetailItem({
function SetupPanel({
session,
mcpConfigPath,
serverCommand,
onCopy,
}: {
session: AutomationSession;
mcpConfigPath: string | null;
serverCommand?: ServerCommand;
onCopy: () => void;
}) {
const snippet = getSnippetForSession(session);
const snippet = getSnippetForSession(session, serverCommand);
return (
<div className="flex flex-col gap-3">
<div className="rounded-xl border p-3 bg-card/60">
Expand Down
36 changes: 27 additions & 9 deletions apps/desktop/src/renderer/stores/browser-automation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,18 +64,36 @@ export const useBrowserAutomationStore = create<BrowserAutomationUiState>(
}),
);

export function getSnippetForSession(session: AutomationSession): string {
export interface ServerCommand {
command: string;
args: string[];
available: boolean;
}

function tomlString(value: string): string {
return `"${value.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`;
}

function shellQuote(value: string): string {
return /^[\w@./:+-]+$/.test(value)
? value
: `'${value.replace(/'/g, "'\\''")}'`;
}

export function getSnippetForSession(
session: AutomationSession,
server?: ServerCommand,
): string {
const cmd = server?.command ?? "desktop-mcp";
const args = server?.args ?? [];
if (session.provider === "Codex") {
// TOML is section-based, so appending this block to an existing
// config.toml is safe. The bin comes from packages/desktop-mcp.
const argsToml = args.map(tomlString).join(", ");
return `[mcp_servers.superset-browser]
command = "desktop-mcp"
args = []`;
command = ${tomlString(cmd)}
args = [${argsToml}]`;
}
// For Claude Code we recommend `claude mcp add` so the user's
// ~/.claude.json is updated in-place instead of manually merging a
// standalone JSON document (which would produce invalid JSON).
return `claude mcp add superset-browser -s local -- desktop-mcp`;
const parts = [cmd, ...args].map(shellQuote).join(" ");
return `claude mcp add superset-browser -s local -- ${parts}`;
}

function formatRelativeTime(ts: number | null | undefined): string {
Expand Down
Loading