Skip to content
Merged
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
4 changes: 4 additions & 0 deletions .mcp.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
"sentry": {
"type": "http",
"url": "https://mcp.sentry.dev/mcp"
},
"desktop-automation": {
"command": "bun",
"args": ["run", "packages/desktop-mcp/src/bin.ts"]
}
}
}
1 change: 1 addition & 0 deletions apps/desktop/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
"@superset/agent": "workspace:*",
"@superset/auth": "workspace:*",
"@superset/db": "workspace:*",
"@superset/desktop-mcp": "workspace:*",
"@superset/durable-session": "workspace:*",
"@superset/local-db": "workspace:*",
"@superset/shared": "workspace:*",
Expand Down
7 changes: 7 additions & 0 deletions apps/desktop/src/lib/electron-app/factories/app/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,10 @@ PLATFORM.IS_WINDOWS &&
);

app.commandLine.appendSwitch("force-color-profile", "srgb");

// Enable CDP for desktop automation MCP (playwright-core connects via this port)
if (env.NODE_ENV === "development") {
const cdpPort = String(process.env.DESKTOP_AUTOMATION_PORT || 9223);
app.commandLine.appendSwitch("remote-debugging-port", cdpPort);
app.commandLine.appendSwitch("remote-allow-origins", "*");
}
Comment on lines +86 to +91
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Use the parsed env object instead of raw process.env.

Line 88 reads process.env.DESKTOP_AUTOMATION_PORT directly, bypassing the Zod-validated env already imported on line 7 and used elsewhere in this file. This duplicates the default 9223 (already in env.shared.ts) and skips coercion/validation. Also, the comment says "playwright-core" but this PR uses puppeteer-core.

Proposed fix
-// Enable CDP for desktop automation MCP (playwright-core connects via this port)
+// Enable CDP for desktop automation MCP (puppeteer-core connects via this port)
 if (env.NODE_ENV === "development") {
-	const cdpPort = String(process.env.DESKTOP_AUTOMATION_PORT || 9223);
+	const cdpPort = String(env.DESKTOP_AUTOMATION_PORT);
 	app.commandLine.appendSwitch("remote-debugging-port", cdpPort);
 	app.commandLine.appendSwitch("remote-allow-origins", "*");
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Enable CDP for desktop automation MCP (playwright-core connects via this port)
if (env.NODE_ENV === "development") {
const cdpPort = String(process.env.DESKTOP_AUTOMATION_PORT || 9223);
app.commandLine.appendSwitch("remote-debugging-port", cdpPort);
app.commandLine.appendSwitch("remote-allow-origins", "*");
}
// Enable CDP for desktop automation MCP (puppeteer-core connects via this port)
if (env.NODE_ENV === "development") {
const cdpPort = String(env.DESKTOP_AUTOMATION_PORT);
app.commandLine.appendSwitch("remote-debugging-port", cdpPort);
app.commandLine.appendSwitch("remote-allow-origins", "*");
}
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/electron-app/factories/app/setup.ts` around lines 86 -
91, Replace the raw process.env usage with the validated env import and update
the comment: use env.DESKTOP_AUTOMATION_PORT (already validated/coerced in
env.shared) instead of process.env.DESKTOP_AUTOMATION_PORT when building cdpPort
in the setup block that calls app.commandLine.appendSwitch, and change the
comment text from "playwright-core" to "puppeteer-core" to reflect the correct
automation library.

2 changes: 2 additions & 0 deletions apps/desktop/src/shared/env.shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const envSchema = z.object({
DESKTOP_VITE_PORT: z.coerce.number().default(5173),
DESKTOP_NOTIFICATIONS_PORT: z.coerce.number().default(5174),
ELECTRIC_PORT: z.coerce.number().default(5133),
DESKTOP_AUTOMATION_PORT: z.coerce.number().default(9223),
// Workspace name for instance isolation
SUPERSET_WORKSPACE_NAME: z.string().default("superset"),
});
Expand All @@ -36,6 +37,7 @@ export const env = envSchema.parse({
DESKTOP_VITE_PORT: process.env.DESKTOP_VITE_PORT,
DESKTOP_NOTIFICATIONS_PORT: process.env.DESKTOP_NOTIFICATIONS_PORT,
ELECTRIC_PORT: process.env.ELECTRIC_PORT,
DESKTOP_AUTOMATION_PORT: process.env.DESKTOP_AUTOMATION_PORT,
SUPERSET_WORKSPACE_NAME: process.env.SUPERSET_WORKSPACE_NAME,
});

Expand Down
92 changes: 92 additions & 0 deletions bun.lock

Large diffs are not rendered by default.

28 changes: 28 additions & 0 deletions packages/desktop-mcp/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{
"name": "@superset/desktop-mcp",
"version": "0.1.0",
"private": true,
"type": "module",
"bin": {
"desktop-mcp": "./src/bin.ts"
},
"exports": {
".": {
"types": "./src/index.ts",
"default": "./src/index.ts"
}
},
"scripts": {
"typecheck": "tsc --noEmit --emitDeclarationOnly false"
},
"dependencies": {
"@modelcontextprotocol/sdk": "^1.25.3",
"puppeteer-core": "^24.37.3",
"zod": "^4.3.5"
},
"devDependencies": {
"@superset/typescript": "workspace:*",
"@types/node": "^24.9.1",
"typescript": "^5.9.3"
}
}
7 changes: 7 additions & 0 deletions packages/desktop-mcp/src/bin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/usr/bin/env node
import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js";
import { createMcpServer } from "./mcp/index.js";

const server = createMcpServer();
const transport = new StdioServerTransport();
await server.connect(transport);
13 changes: 13 additions & 0 deletions packages/desktop-mcp/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export { createMcpServer } from "./mcp/index.js";
export type {
ClickResponse,
ConsoleLogEntry,
ConsoleLogsResponse,
DomElement,
DomResponse,
EvaluateResponse,
NavigateResponse,
ScreenshotResponse,
TypeResponse,
WindowInfoResponse,
} from "./zod.js";
59 changes: 59 additions & 0 deletions packages/desktop-mcp/src/mcp/connection/connection-manager.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import puppeteer, { type Browser, type Page } from "puppeteer-core";
import { ConsoleCapture } from "../console-capture/index.js";
import { FocusLock } from "../focus-lock/index.js";

const CDP_PORT = Number(process.env.DESKTOP_AUTOMATION_PORT) || 9223;

/**
* Manages a CDP connection to the Electron renderer via puppeteer-core.
*
* - Lazy connect on first tool call (Electron might not be running yet)
* - Auto-reconnect if connection drops (Electron restart/hot reload)
* - Re-injects focus lock and console capture on reconnect
*/
export class ConnectionManager {
private browser: Browser | null = null;
private page: Page | null = null;

readonly consoleCapture = new ConsoleCapture();
readonly focusLock = new FocusLock();

async getPage(): Promise<Page> {
if (this.page && this.browser?.connected) {
await this.focusLock.inject(this.page);
return this.page;
}
return this.connect();
}
Comment on lines +21 to +27
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Race condition: concurrent getPage() calls can trigger multiple connect() invocations.

If two MCP tool calls arrive simultaneously while this.page is null, both will pass the check on Line 22 and call connect() concurrently. This results in duplicate CDP connections, with the second overwriting this.browser/this.page and orphaning the first connection's disconnect handler.

A simple connection promise guard resolves this:

Proposed fix
 export class ConnectionManager {
 	private browser: Browser | null = null;
 	private page: Page | null = null;
+	private connecting: Promise<Page> | null = null;

 	readonly consoleCapture = new ConsoleCapture();
 	readonly focusLock = new FocusLock();

 	async getPage(): Promise<Page> {
 		if (this.page && this.browser?.connected) {
 			await this.focusLock.inject(this.page);
 			return this.page;
 		}
-		return this.connect();
+		if (!this.connecting) {
+			this.connecting = this.connect().finally(() => {
+				this.connecting = null;
+			});
+		}
+		return this.connecting;
 	}
🤖 Prompt for AI Agents
In `@packages/desktop-mcp/src/mcp/connection/connection-manager.ts` around lines
21 - 27, The getPage() method has a race where concurrent calls can both detect
no page and call connect() concurrently; introduce a connection guard (e.g.,
this.connectingPromise) inside the ConnectionManager so that getPage() sets
this.connectingPromise = this.connect() when no existing promise exists, awaits
that single promise for all callers, and then clears the guard on success or
failure; ensure you still call this.focusLock.inject(this.page) after the
awaited promise resolves and clear the connectingPromise in a finally block to
avoid stalling future attempts.


private async connect(): Promise<Page> {
this.browser = await puppeteer.connect({
browserURL: `http://127.0.0.1:${CDP_PORT}`,
protocolTimeout: 60_000,
defaultViewport: null,
});
const pages = await this.browser.pages();

// Find the actual app page, skipping chrome-extension:// background pages
const appPage = pages.find(
(p) => !p.url().startsWith("chrome-extension://"),
);
if (!appPage) {
throw new Error(
`[desktop-mcp] No app pages found via CDP (found ${pages.length} pages, all extensions)`,
);
}
this.page = appPage;

this.consoleCapture.attach(this.page);
this.focusLock.attach(this.page);
await this.focusLock.inject(this.page);

this.browser.on("disconnected", () => {
this.browser = null;
this.page = null;
});

return this.page;
}
}
1 change: 1 addition & 0 deletions packages/desktop-mcp/src/mcp/connection/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { ConnectionManager } from "./connection-manager.js";
53 changes: 53 additions & 0 deletions packages/desktop-mcp/src/mcp/console-capture/console-capture.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import type { ConsoleMessage, Page } from "puppeteer-core";
import type { ConsoleLogEntry } from "../../zod.js";

const LEVEL_MAP: Record<string, number> = {
verbose: 0,
debug: 0,
info: 1,
log: 1,
warning: 2,
warn: 2,
error: 3,
};

export class ConsoleCapture {
private logs: ConsoleLogEntry[] = [];
private maxSize = 500;

attach(page: Page) {
page.on("console", (msg: ConsoleMessage) => {
const level = LEVEL_MAP[msg.type()] ?? 1;
const location = msg.location();
this.logs.push({
level,
message: msg.text(),
source: location.url ?? "",
line: location.lineNumber ?? 0,
timestamp: Date.now(),
});
if (this.logs.length > this.maxSize) this.logs.shift();
});
}

getLogs({
level,
limit,
}: {
level?: number;
limit?: number;
}): ConsoleLogEntry[] {
let filtered = this.logs;
if (level !== undefined) {
filtered = filtered.filter((log) => log.level === level);
}
if (limit !== undefined) {
filtered = filtered.slice(-limit);
}
return filtered;
}
Comment on lines +33 to +48
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

getLogs returns a direct reference to the internal array when no filters are applied.

On Line 40, let filtered = this.logs — when neither level nor limit is provided, the caller receives a direct reference to the internal logs array. External mutations would corrupt internal state.

Proposed fix
 	getLogs({
 		level,
 		limit,
 	}: {
 		level?: number;
 		limit?: number;
 	}): ConsoleLogEntry[] {
-		let filtered = this.logs;
+		let filtered: ConsoleLogEntry[] = this.logs;
 		if (level !== undefined) {
 			filtered = filtered.filter((log) => log.level === level);
 		}
 		if (limit !== undefined) {
 			filtered = filtered.slice(-limit);
 		}
+		// Return a copy if no filter was applied to avoid leaking internal state
+		return filtered === this.logs ? [...filtered] : filtered;
-		return filtered;
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
getLogs({
level,
limit,
}: {
level?: number;
limit?: number;
}): ConsoleLogEntry[] {
let filtered = this.logs;
if (level !== undefined) {
filtered = filtered.filter((log) => log.level === level);
}
if (limit !== undefined) {
filtered = filtered.slice(-limit);
}
return filtered;
}
getLogs({
level,
limit,
}: {
level?: number;
limit?: number;
}): ConsoleLogEntry[] {
let filtered: ConsoleLogEntry[] = this.logs;
if (level !== undefined) {
filtered = filtered.filter((log) => log.level === level);
}
if (limit !== undefined) {
filtered = filtered.slice(-limit);
}
// Return a copy if no filter was applied to avoid leaking internal state
return filtered === this.logs ? [...filtered] : filtered;
}
🤖 Prompt for AI Agents
In `@packages/desktop-mcp/src/mcp/console-capture/console-capture.ts` around lines
33 - 48, getLogs currently assigns filtered = this.logs which returns a direct
reference to the internal array; change getLogs (the function named getLogs
returning ConsoleLogEntry[]) to always return a copy instead of the original
(e.g., initialize filtered with a shallow copy like this.logs.slice() or
[...this.logs]) so callers never receive a reference to the internal this.logs;
keep the existing filtering by level and slicing for limit but ensure the
initial copy is used.


clear() {
this.logs = [];
}
}
1 change: 1 addition & 0 deletions packages/desktop-mcp/src/mcp/console-capture/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { ConsoleCapture } from "./console-capture.js";
92 changes: 92 additions & 0 deletions packages/desktop-mcp/src/mcp/dom-inspector/dom-inspector.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/**
* JavaScript source to inject into the renderer via page.evaluate().
* Walks the DOM and returns a flat list of visible elements with metadata.
*/
export const DOM_INSPECTOR_SCRIPT = `function inspectDom({ selector, interactiveOnly }) {
const INTERACTIVE_TAGS = new Set([
'A', 'BUTTON', 'INPUT', 'SELECT', 'TEXTAREA', 'DETAILS', 'SUMMARY'
]);
const INTERACTIVE_ROLES = new Set([
'button', 'link', 'checkbox', 'radio', 'tab', 'menuitem',
'switch', 'textbox', 'combobox', 'listbox', 'option', 'slider', 'spinbutton'
]);

const root = selector ? document.querySelector(selector) : document.body;
if (!root) return [];

const elements = [];
const walker = document.createTreeWalker(root, NodeFilter.SHOW_ELEMENT);

let node = walker.currentNode;
while (node) {
if (node instanceof HTMLElement) {
const rect = node.getBoundingClientRect();
const style = window.getComputedStyle(node);

// Skip invisible elements
if (rect.width === 0 && rect.height === 0) { node = walker.nextNode(); continue; }
if (style.display === 'none' || style.visibility === 'hidden') { node = walker.nextNode(); continue; }
if (parseFloat(style.opacity) === 0) { node = walker.nextNode(); continue; }

const tag = node.tagName.toLowerCase();
const role = node.getAttribute('role') || undefined;
const testId = node.getAttribute('data-testid') || undefined;
const isInteractive = INTERACTIVE_TAGS.has(node.tagName)
|| INTERACTIVE_ROLES.has(role || '')
|| node.hasAttribute('onclick')
|| node.getAttribute('tabindex') !== null;

if (interactiveOnly && !isInteractive) { node = walker.nextNode(); continue; }

// Build a unique CSS selector
let cssSelector;
if (node.id) {
cssSelector = '#' + CSS.escape(node.id);
} else if (testId) {
cssSelector = '[data-testid="' + testId + '"]';
} else {
Comment on lines +45 to +47
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

testId value is not escaped in the CSS selector string.

If data-testid contains a double-quote or backslash, the generated selector [data-testid="..."] will be malformed. Use CSS.escape() for consistency with how node.id is handled on Line 44.

Proposed fix
-        cssSelector = '[data-testid="' + testId + '"]';
+        cssSelector = '[data-testid="' + CSS.escape(testId) + '"]';
🤖 Prompt for AI Agents
In `@packages/desktop-mcp/src/mcp/dom-inspector/dom-inspector.ts` around lines 45
- 47, The code builds cssSelector with the raw testId which can break the
attribute selector when testId contains quotes/backslashes; update the branch
that sets cssSelector (the else if handling testId) to use CSS.escape(testId)
the same way node.id is handled (i.e., build the selector using '[data-testid="'
+ CSS.escape(testId) + '"]'). This ensures the generated selector is properly
escaped and robust.

const path = [];
let el = node;
while (el && el !== document.body) {
const parent = el.parentElement;
if (!parent) break;
const siblings = Array.from(parent.children).filter(s => s.tagName === el.tagName);
if (siblings.length > 1) {
const idx = siblings.indexOf(el) + 1;
path.unshift(el.tagName.toLowerCase() + ':nth-of-type(' + idx + ')');
} else {
path.unshift(el.tagName.toLowerCase());
}
el = parent;
}
cssSelector = path.join(' > ');
}

const text = (node.textContent || '').trim().slice(0, 200);

elements.push({
tag,
id: node.id || undefined,
classes: Array.from(node.classList),
text,
selector: cssSelector,
bounds: {
x: Math.round(rect.x),
y: Math.round(rect.y),
width: Math.round(rect.width),
height: Math.round(rect.height),
},
role,
testId,
interactive: isInteractive,
disabled: node.hasAttribute('disabled'),
checked: 'checked' in node ? node.checked : undefined,
focused: document.activeElement === node,
visible: true,
});
}
node = walker.nextNode();
}

return elements;
}`;
1 change: 1 addition & 0 deletions packages/desktop-mcp/src/mcp/dom-inspector/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { DOM_INSPECTOR_SCRIPT } from "./dom-inspector.js";
Loading
Loading