diff --git a/.changeset/mcp-stdio-edit-ui.md b/.changeset/mcp-stdio-edit-ui.md new file mode 100644 index 000000000..1a071950f --- /dev/null +++ b/.changeset/mcp-stdio-edit-ui.md @@ -0,0 +1,26 @@ +--- +"executor": patch +--- + +**Stdio MCP integrations can be edited from the UI** + +The integration Edit sheet showed stdio servers as read-only text and told you +to remove and recreate the integration to change its command. Fixing a typo in +an argument, moving a server to a new path, or adding a static environment +variable meant editing `executor.jsonc` by hand, or losing the integration's +connections and tool policies to a delete-and-re-add. + +The sheet now edits the command, its arguments, the working directory, and the +declared environment map, staged and applied by the sheet's own Save like the +remote editor beside it. Arguments use the same quote-aware parsing as the add +flow, so an argument containing spaces survives a round trip. + +The environment field edits the DECLARED static variables only. A stdio server +receives those plus a small fixed base set — it does not inherit executor's +environment — and secret values still belong to the connection, entered per +account against the server's declared `stdio_env` method. + +Saving revises the integration config, which is already enough to rebuild the +tool catalog: connections whose catalog predates the revision re-list on their +next read, so an edited command's tools are correct without an explicit +refresh. diff --git a/apps/local/vite.config.ts b/apps/local/vite.config.ts index 29f7a28d2..bfd23d57d 100644 --- a/apps/local/vite.config.ts +++ b/apps/local/vite.config.ts @@ -1,5 +1,6 @@ import { readFileSync } from "node:fs"; -import { resolve } from "node:path"; +import { createRequire } from "node:module"; +import { dirname, resolve } from "node:path"; import { Readable } from "node:stream"; import { fileURLToPath } from "node:url"; import { defineConfig, type Plugin } from "vite"; @@ -42,6 +43,19 @@ const EXECUTOR_GITHUB_URL = ( const REPO_ROOT = fileURLToPath(new URL("../..", import.meta.url)); const APP_ROOT = fileURLToPath(new URL("../../packages/app/", import.meta.url)); +// js-yaml reaches the browser through @executor-js/plugin-openapi's UI, so it +// belongs in the pre-bundle below. Vite resolves `optimizeDeps.include` from +// `root`, which here is packages/app, and bun's isolated install keeps js-yaml +// in the openapi plugin's own node_modules — out of reach of both a plain +// "js-yaml" specifier and the " > " form from that directory. Point +// the bare specifier at the package the plugin already resolves, so `root` can +// name it. It is the same copy either way, so nothing about resolution changes. +const JS_YAML_DIR = dirname( + createRequire(fileURLToPath(new URL("../../packages/plugins/openapi/", import.meta.url))).resolve( + "js-yaml", + ), +); + const oauthClientMetadataResponse = (requestUrl: string, webRequest: Request): Response => new Response( JSON.stringify( @@ -215,6 +229,25 @@ export default defineConfig({ outDir: resolve(import.meta.dirname, "dist"), emptyOutDir: true, }, + // Deps vite only discovers once a lazy-loaded React chunk actually renders + // (e.g. opening the integration Edit sheet, or the MCP/OpenAPI add-source + // flow). Discovering one mid-run forces a re-optimize and a full page + // reload, which throws away whatever the person was doing — an open sheet + // and its form state included. Pre-bundle them at boot so vite never + // discovers them mid-run. Keep in sync with apps/host-selfhost and + // apps/cloud. + optimizeDeps: { + include: [ + "effect/Match", + "effect/Predicate", + "effect/Exit", + "effect/Option", + "effect/Cause", + "effect/Data", + "effect/Schema", + "js-yaml", + ], + }, define: { "import.meta.env.VITE_APP_VERSION": JSON.stringify(EXECUTOR_VERSION), // The local app IS the npm-installed CLI, so its update card shows the npm @@ -229,6 +262,7 @@ export default defineConfig({ }, resolve: { tsconfigPaths: true, + alias: { "js-yaml": JS_YAML_DIR }, }, server: { port: parseInt(process.env.PORT ?? "5173", 10), diff --git a/e2e/local/stdio-mcp-edit-ui.test.ts b/e2e/local/stdio-mcp-edit-ui.test.ts new file mode 100644 index 000000000..6c86bb9e3 --- /dev/null +++ b/e2e/local/stdio-mcp-edit-ui.test.ts @@ -0,0 +1,138 @@ +// Local-only — the integration Edit sheet for a STDIO MCP server, driven in a +// real browser. `local` is the only surface that enables stdio MCP +// (`dangerouslyAllowStdioMCP: true`), so it is the only place this sheet can be +// exercised end to end. +// +// The sheet used to say "Stdio MCP integrations cannot be edited. Remove and +// recreate the integration with the updated command." (#812) — changing a +// command meant editing `executor.jsonc` by hand, or losing the integration's +// connections and policies to a delete-and-re-add. +// +// The assertion is deliberately not "the field accepted my text". The scenario +// edits the DECLARED env map through the form and then reads the tool catalog: +// the fixture advertises `saw_declared_env` only when that variable is present +// in the spawned child's environment, so the tool appearing proves the edited +// config travelled the whole way — form, config write, respawn, rediscovery. +import { fileURLToPath } from "node:url"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { HttpApiClient } from "effect/unstable/httpapi"; +import { FetchHttpClient, HttpClient, HttpClientRequest } from "effect/unstable/http"; +import { composePluginApi } from "@executor-js/api/server"; +import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api"; + +import { scenario } from "../src/scenario"; +import { Browser, Cli, RunDir, Target } from "../src/services"; +import { withLocalServer } from "./local-server"; + +const api = composePluginApi([mcpHttpPlugin()] as const); + +const FIXTURE = fileURLToPath(new URL("./fixtures/stdio-mcp-server.mjs", import.meta.url)); + +const SECRET = "s3cr3t-typed-into-the-edit-sheet"; + +scenario( + "Local · a stdio MCP server's command and environment are editable from the integration Edit sheet", + { timeout: 300_000 }, + Effect.gen(function* () { + const cli = yield* Cli; + const browser = yield* Browser; + const target = yield* Target; + const runDir = yield* RunDir; + const identity = yield* target.newIdentity(); + + yield* withLocalServer(cli, runDir, (server) => + Effect.gen(function* () { + const client = yield* HttpApiClient.make(api, { + baseUrl: new URL("/api", server.origin).toString(), + transformClient: HttpClient.mapRequest((request) => + HttpClientRequest.setHeader(request, "authorization", `Bearer ${server.token}`), + ), + }).pipe(Effect.provide(FetchHttpClient.layer)); + + const slug = "e2e-stdio-editable"; + + // A plain stdio server: no declared env, so the env-gated tool is absent + // until the edit adds the variable. + yield* client.mcp.addServer({ + payload: { + transport: "stdio", + name: "E2E Stdio Editable", + command: "node", + args: [FIXTURE], + slug, + }, + }); + + const before = yield* client.tools.list({ query: { integration: slug } }); + expect( + before.map((t) => t.name), + "the server starts with its base tool and no declared env", + ).toContain("echo_tool"); + expect( + before.map((t) => t.name), + "the env-gated tool is absent before the edit", + ).not.toContain("saw_declared_env"); + + yield* browser.session(identity, async ({ page, step }) => { + await step("Open the stdio integration from the console", async () => { + await page.goto(server.url, { waitUntil: "domcontentloaded" }); + await page.getByTestId(`integration-entry-${slug}`).first().click(); + // Wait for the detail page's own data, not just its shell: clicking + // Edit while the accounts panel is still a skeleton races the layout + // shift that lands when it resolves. + await page.getByText("default").first().waitFor({ timeout: 30_000 }); + await page.getByRole("button", { name: "Edit" }).waitFor({ timeout: 30_000 }); + }); + + await step("Open the Edit sheet — the stdio command is editable", async () => { + await page.getByRole("button", { name: "Edit" }).click(); + await page.getByText("Edit integration").waitFor({ timeout: 30_000 }); + await page.getByText("Server command").waitFor({ timeout: 30_000 }); + // The read-only dead end this replaces. + expect( + await page.getByText("Stdio MCP integrations cannot be edited").count(), + "the read-only message is gone", + ).toBe(0); + expect( + await page.getByRole("textbox", { name: "Command" }).inputValue(), + "the stored command is loaded into the form", + ).toBe("node"); + }); + + await step("Declare an environment variable and save", async () => { + await page + .getByRole("textbox", { name: "Environment variables" }) + .fill(`EXECUTOR_E2E_SECRET=${SECRET}`); + await page.getByRole("button", { name: "Save" }).click(); + await page.getByText("Server command").waitFor({ state: "hidden", timeout: 30_000 }); + }); + }); + + // The edit persisted as the DECLARED static env map on the config. + const stored = yield* client.mcp.getServer({ params: { slug } }); + expect( + stored?.config.transport === "stdio" ? stored.config.env : undefined, + "the sheet wrote the declared env map", + ).toEqual({ EXECUTOR_E2E_SECRET: SECRET }); + expect( + stored?.config.transport === "stdio" ? stored.config.command : undefined, + "the command it did not touch is unchanged", + ).toBe("node"); + + // And it reached the spawned server: the fixture gates this tool's very + // existence on that variable being in its own environment. + const after = yield* client.tools.list({ query: { integration: slug } }); + expect( + after.map((t) => t.name), + "the edited environment reached the respawned server and the catalog was rebuilt", + ).toContain("saw_declared_env"); + expect( + after.map((t) => t.name), + "the rebuild kept the tools that still exist", + ).toContain("echo_tool"); + }), + ); + }), +); diff --git a/e2e/local/stdio-mcp.test.ts b/e2e/local/stdio-mcp.test.ts index c7b55bdf3..ad1e8ef10 100644 --- a/e2e/local/stdio-mcp.test.ts +++ b/e2e/local/stdio-mcp.test.ts @@ -222,6 +222,48 @@ scenario( autoTools.map((t) => t.name), "auto negotiation falls back to legacy and still discovers tools", ).toContain("echo_tool"); + expect( + autoTools.map((t) => t.name), + "this server declared no env, so the env-gated tool is absent to begin with", + ).not.toContain("saw_declared_env"); + + // --- Editing a stdio server's config (what the integration Edit sheet + // now does instead of telling you to remove and recreate). The tool + // catalog is persisted per connection, so a plain config replace is + // only enough because core stamps `config_revised_at` on a config + // write and every connection whose catalog predates that stamp is + // rebuilt on the next read. The edit path therefore needs NO explicit + // refresh of its own — this asserts that, so nobody adds one back. + // + // Adding a declared env var is the lever: the fixture advertises + // `saw_declared_env` only when that variable reached the child, so the + // tool appearing with no further action proves both halves — the new + // config reached the spawn, and the catalog was rebuilt from it. --- + const editedConfig = { + ...autoStored!.config, + env: { EXECUTOR_E2E_SECRET: SECRET }, + } as typeof autoStored.config; + + yield* client.mcp.configureServer({ + params: { slug: autoSlug }, + payload: { config: editedConfig }, + }); + + const editedStored = yield* client.mcp.getServer({ params: { slug: autoSlug } }); + expect( + JSON.stringify(editedStored?.config ?? {}), + "the edit persisted, and left the untouched negotiation mode alone", + ).toContain('"versionNegotiation":"auto"'); + + const editedTools = yield* client.tools.list({ query: { integration: autoSlug } }); + expect( + editedTools.map((t) => t.name), + "the edited config reached the spawn and the catalog was rediscovered", + ).toContain("saw_declared_env"); + expect( + editedTools.map((t) => t.name), + "rediscovery replaced the catalog rather than dropping what still exists", + ).toContain("echo_tool"); }), { env: DAEMON_ENV }, ); diff --git a/packages/plugins/mcp/src/react/AddMcpIntegration.tsx b/packages/plugins/mcp/src/react/AddMcpIntegration.tsx index ccd67b9b7..a2479c85b 100644 --- a/packages/plugins/mcp/src/react/AddMcpIntegration.tsx +++ b/packages/plugins/mcp/src/react/AddMcpIntegration.tsx @@ -41,6 +41,7 @@ import { McpRemoteIntegrationFields } from "./McpRemoteIntegrationFields"; import { McpRequestHeadersEditor } from "./McpRequestHeadersEditor"; import { mcpHeadersFromRows, type McpHeaderRow } from "./request-headers"; import { mcpAuthMethodInputFromEditorValue, mcpWireAuthInput } from "./auth-method-config"; +import { parseStdioArgs } from "./stdio-fields"; import { isProbableMcpEndpoint } from "./probe-url"; import { cloudflareNeedsCodemodeOptOut } from "../sdk/cloudflare-codemode"; import { mcpPresets, type McpPreset } from "../sdk/presets"; @@ -62,19 +63,6 @@ function findPreset(id: string | undefined): McpPreset | undefined { return mcpPresets.find((p) => p.id === id); } -// Splits the raw args field into tokens, honoring double-quoted groups so an -// argument with spaces stays intact. -function parseStdioArgs(raw: string): string[] { - if (!raw.trim()) return []; - const args: string[] = []; - const regex = /[^\s"]+|"([^"]*)"/g; - let match; - while ((match = regex.exec(raw)) !== null) { - args.push(match[1] ?? match[0]); - } - return args; -} - // --------------------------------------------------------------------------- // State machine (remote flow) // --------------------------------------------------------------------------- diff --git a/packages/plugins/mcp/src/react/EditMcpIntegration.tsx b/packages/plugins/mcp/src/react/EditMcpIntegration.tsx index e1cb718a8..e982c1ca5 100644 --- a/packages/plugins/mcp/src/react/EditMcpIntegration.tsx +++ b/packages/plugins/mcp/src/react/EditMcpIntegration.tsx @@ -13,10 +13,16 @@ import { type AuthMethodRow, type AuthMethodSeed, } from "@executor-js/react/components/auth-method-list-editor"; -import { Badge } from "@executor-js/react/components/badge"; -import { FormErrorAlert } from "@executor-js/react/lib/integration-add"; +import { + CardStack, + CardStackContent, + CardStackEntryField, +} from "@executor-js/react/components/card-stack"; +import { Input } from "@executor-js/react/components/input"; +import { Textarea } from "@executor-js/react/components/textarea"; +import { errorMessageFromExit, FormErrorAlert } from "@executor-js/react/lib/integration-add"; -import { configureMcpAuth, mcpServerAtom } from "./atoms"; +import { configureMcpAuth, configureMcpServer, mcpServerAtom } from "./atoms"; import type { McpAuthMethod, McpCanonicalAuthMethodInput, @@ -27,6 +33,7 @@ import { mcpAuthMethodInputFromEditorValue, mcpWireAuthInput, } from "./auth-method-config"; +import { formatStdioArgs, formatStdioEnv, parseStdioArgs, parseStdioEnv } from "./stdio-fields"; type McpServer = { readonly slug: IntegrationSlug; @@ -174,30 +181,160 @@ function RemoteEdit(props: { } // --------------------------------------------------------------------------- -// Stdio read-only view +// Stdio edit — the command, its arguments, the working directory, and the +// DECLARED static environment. Secret env vars are not edited here: a stdio +// server declares them as a `stdio_env` auth method and their values live on +// the connection, managed from the integration page's accounts hub. Changes +// are staged and applied by the sheet's Save, like the remote editor above. // --------------------------------------------------------------------------- -function StdioReadOnly(props: { - server: McpServer & { config: Extract }; +type McpStdioConfig = Extract; + +/** Order-independent identity for a declared env map, so re-ordering the lines + * in the field is not reported as a change. */ +const envIdentity = (env: Readonly> | undefined): string => + Object.entries(env ?? {}) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([key, value]) => `${key}=${value}`) + .join("\n"); + +function StdioEdit(props: { + server: McpServer & { config: McpStdioConfig }; + onPendingChange?: EditSheetSectionProps["onPendingChange"]; }) { - const { command, args } = props.server.config; + const { server } = props; + const doConfigure = useAtomSet(configureMcpServer, { mode: "promiseExit" }); + + const [command, setCommand] = useState(server.config.command); + const [args, setArgs] = useState(() => formatStdioArgs(server.config.args)); + const [cwd, setCwd] = useState(server.config.cwd ?? ""); + const [env, setEnv] = useState(() => formatStdioEnv(server.config.env)); + const [error, setError] = useState(null); + + // The edited config. `configureServer` replaces the whole blob, so anything + // this form does not surface (`versionNegotiation`, `authenticationTemplate`) + // is carried through untouched. Empty optional fields are omitted rather than + // written as `undefined`. + const edited = useMemo(() => { + const { args: _args, cwd: _cwd, env: _env, ...rest } = server.config; + const nextArgs = parseStdioArgs(args); + const nextEnv = parseStdioEnv(env); + const nextCwd = cwd.trim(); + return { + ...rest, + command: command.trim(), + ...(nextArgs.length > 0 ? { args: nextArgs } : {}), + ...(Object.keys(nextEnv).length > 0 ? { env: nextEnv } : {}), + ...(nextCwd !== "" ? { cwd: nextCwd } : {}), + }; + }, [args, command, cwd, env, server.config]); + + const changed = + edited.command !== server.config.command || + formatStdioArgs(edited.args) !== formatStdioArgs(server.config.args) || + (edited.cwd ?? "") !== (server.config.cwd ?? "") || + envIdentity(edited.env) !== envIdentity(server.config.env); + + // Staged apply, run by the sheet's Save. Persisting the config re-runs + // discovery on every connection, so the tool catalog matches the new command. + const applyStaged = useCallback(async (): Promise => { + setError(null); + if (edited.command === "") { + setError("A command is required."); + return { ok: false }; + } + const exit = await doConfigure({ + params: { slug: server.slug }, + payload: { config: edited }, + reactivityKeys: integrationWriteKeys, + }); + if (Exit.isFailure(exit)) { + setError(errorMessageFromExit(exit, "Failed to update the server command")); + return { ok: false }; + } + return { ok: true, summary: "Server command updated." }; + }, [doConfigure, edited, server.slug]); + + const onPendingChangeRef = useRef(props.onPendingChange); + onPendingChangeRef.current = props.onPendingChange; + useEffect(() => { + onPendingChangeRef.current?.(changed ? applyStaged : null); + return () => onPendingChangeRef.current?.(null); + }, [changed, applyStaged]); + return ( -
+

Server command

- Stdio MCP integrations cannot be edited. Remove and recreate the integration with the - updated command. + Changes apply when you save. The server's tools are then rediscovered with the new + command.

-
-

- {command} {(args ?? []).join(" ")} -

- - stdio - -
+ + + + + setCommand((e.target as HTMLInputElement).value)} + placeholder="npx" + className="font-mono text-sm" + /> + + + + setArgs((e.target as HTMLInputElement).value)} + placeholder="-y chrome-devtools-mcp@latest" + className="font-mono text-sm" + /> + + + + setCwd((e.target as HTMLInputElement).value)} + placeholder="/path/to/server" + className="font-mono text-sm" + /> + + + +