Skip to content

Commit ecb87de

Browse files
feat(mcp): add edit support for stdio sources (#1775)
* feat(mcp): add edit support for stdio sources - Add StdioEditForm with Save/Cancel buttons for stdio MCP sources - Implement updateStdioServer backend function using Effect - Connect UI to backend with proper error handling - Enable canEdit for stdio sources - Use shared Input, Label, and Textarea components - 42/42 MCP tests passing Closes #812 * Rework the stdio edit sheet onto the current shape Edit the command, arguments, working directory, and declared env map through the sheet's staged Save instead of a second inline form, using the add flow's CardStack fields and its quote-aware argument parsing (now shared). Drop the parallel updateStdioServer endpoint: configureServer already replaces an integration config. It now also re-runs discovery on the connections, since the tool catalog is derived from the config it just replaced. The env field edits the declared static map only. A stdio server receives that plus a fixed base set, so the copy no longer implies it inherits executor's environment. * Drop the rediscovery call; core already revises the catalog An integrations config write stamps config_revised_at, and every connection whose catalog predates the stamp re-lists on its next read. Refreshing from the plugin duplicated that, eagerly, on the save path. The local stdio scenario passes either way, which is how the duplication showed up. Cover the edit in that scenario instead: it adds a declared env var, and the fixture only advertises saw_declared_env when that variable reached the child, so the tool appearing after a plain config replace shows the rebuild happens with no refresh of its own. * Drive the stdio edit sheet in a browser e2e Local is the only surface with stdio MCP enabled, so the sheet gets its scenario there. It edits the declared env through the form and then reads the tool catalog: the fixture advertises saw_declared_env only when that variable is in the spawned child environment, so the tool appearing proves the edit travelled form to config to respawn. Name the four fields with aria-label. CardStackEntryField renders its label without htmlFor, so the controls had no accessible name at all. * Pre-bundle the local dev server's late-discovered deps The local app's dev server discovered effect/Match, effect/Predicate and js-yaml only when a lazy React chunk first rendered them. Vite then re-optimized and full-reloaded the page. Opening the integration Edit sheet is one of those chunks, so the reload landed on the open sheet and closed it, and the new stdio edit scenario waited 30 seconds for a field that had been thrown away. Pre-bundle the same list apps/host-selfhost and apps/cloud already carry, so nothing is discovered mid-run. --------- Co-authored-by: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com>
1 parent 2168029 commit ecb87de

8 files changed

Lines changed: 521 additions & 36 deletions

File tree

.changeset/mcp-stdio-edit-ui.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
"executor": patch
3+
---
4+
5+
**Stdio MCP integrations can be edited from the UI**
6+
7+
The integration Edit sheet showed stdio servers as read-only text and told you
8+
to remove and recreate the integration to change its command. Fixing a typo in
9+
an argument, moving a server to a new path, or adding a static environment
10+
variable meant editing `executor.jsonc` by hand, or losing the integration's
11+
connections and tool policies to a delete-and-re-add.
12+
13+
The sheet now edits the command, its arguments, the working directory, and the
14+
declared environment map, staged and applied by the sheet's own Save like the
15+
remote editor beside it. Arguments use the same quote-aware parsing as the add
16+
flow, so an argument containing spaces survives a round trip.
17+
18+
The environment field edits the DECLARED static variables only. A stdio server
19+
receives those plus a small fixed base set — it does not inherit executor's
20+
environment — and secret values still belong to the connection, entered per
21+
account against the server's declared `stdio_env` method.
22+
23+
Saving revises the integration config, which is already enough to rebuild the
24+
tool catalog: connections whose catalog predates the revision re-list on their
25+
next read, so an edited command's tools are correct without an explicit
26+
refresh.

apps/local/vite.config.ts

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { readFileSync } from "node:fs";
2-
import { resolve } from "node:path";
2+
import { createRequire } from "node:module";
3+
import { dirname, resolve } from "node:path";
34
import { Readable } from "node:stream";
45
import { fileURLToPath } from "node:url";
56
import { defineConfig, type Plugin } from "vite";
@@ -42,6 +43,19 @@ const EXECUTOR_GITHUB_URL = (
4243
const REPO_ROOT = fileURLToPath(new URL("../..", import.meta.url));
4344
const APP_ROOT = fileURLToPath(new URL("../../packages/app/", import.meta.url));
4445

46+
// js-yaml reaches the browser through @executor-js/plugin-openapi's UI, so it
47+
// belongs in the pre-bundle below. Vite resolves `optimizeDeps.include` from
48+
// `root`, which here is packages/app, and bun's isolated install keeps js-yaml
49+
// in the openapi plugin's own node_modules — out of reach of both a plain
50+
// "js-yaml" specifier and the "<pkg> > <dep>" form from that directory. Point
51+
// the bare specifier at the package the plugin already resolves, so `root` can
52+
// name it. It is the same copy either way, so nothing about resolution changes.
53+
const JS_YAML_DIR = dirname(
54+
createRequire(fileURLToPath(new URL("../../packages/plugins/openapi/", import.meta.url))).resolve(
55+
"js-yaml",
56+
),
57+
);
58+
4559
const oauthClientMetadataResponse = (requestUrl: string, webRequest: Request): Response =>
4660
new Response(
4761
JSON.stringify(
@@ -215,6 +229,25 @@ export default defineConfig({
215229
outDir: resolve(import.meta.dirname, "dist"),
216230
emptyOutDir: true,
217231
},
232+
// Deps vite only discovers once a lazy-loaded React chunk actually renders
233+
// (e.g. opening the integration Edit sheet, or the MCP/OpenAPI add-source
234+
// flow). Discovering one mid-run forces a re-optimize and a full page
235+
// reload, which throws away whatever the person was doing — an open sheet
236+
// and its form state included. Pre-bundle them at boot so vite never
237+
// discovers them mid-run. Keep in sync with apps/host-selfhost and
238+
// apps/cloud.
239+
optimizeDeps: {
240+
include: [
241+
"effect/Match",
242+
"effect/Predicate",
243+
"effect/Exit",
244+
"effect/Option",
245+
"effect/Cause",
246+
"effect/Data",
247+
"effect/Schema",
248+
"js-yaml",
249+
],
250+
},
218251
define: {
219252
"import.meta.env.VITE_APP_VERSION": JSON.stringify(EXECUTOR_VERSION),
220253
// The local app IS the npm-installed CLI, so its update card shows the npm
@@ -229,6 +262,7 @@ export default defineConfig({
229262
},
230263
resolve: {
231264
tsconfigPaths: true,
265+
alias: { "js-yaml": JS_YAML_DIR },
232266
},
233267
server: {
234268
port: parseInt(process.env.PORT ?? "5173", 10),
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
// Local-only — the integration Edit sheet for a STDIO MCP server, driven in a
2+
// real browser. `local` is the only surface that enables stdio MCP
3+
// (`dangerouslyAllowStdioMCP: true`), so it is the only place this sheet can be
4+
// exercised end to end.
5+
//
6+
// The sheet used to say "Stdio MCP integrations cannot be edited. Remove and
7+
// recreate the integration with the updated command." (#812) — changing a
8+
// command meant editing `executor.jsonc` by hand, or losing the integration's
9+
// connections and policies to a delete-and-re-add.
10+
//
11+
// The assertion is deliberately not "the field accepted my text". The scenario
12+
// edits the DECLARED env map through the form and then reads the tool catalog:
13+
// the fixture advertises `saw_declared_env` only when that variable is present
14+
// in the spawned child's environment, so the tool appearing proves the edited
15+
// config travelled the whole way — form, config write, respawn, rediscovery.
16+
import { fileURLToPath } from "node:url";
17+
18+
import { expect } from "@effect/vitest";
19+
import { Effect } from "effect";
20+
import { HttpApiClient } from "effect/unstable/httpapi";
21+
import { FetchHttpClient, HttpClient, HttpClientRequest } from "effect/unstable/http";
22+
import { composePluginApi } from "@executor-js/api/server";
23+
import { mcpHttpPlugin } from "@executor-js/plugin-mcp/api";
24+
25+
import { scenario } from "../src/scenario";
26+
import { Browser, Cli, RunDir, Target } from "../src/services";
27+
import { withLocalServer } from "./local-server";
28+
29+
const api = composePluginApi([mcpHttpPlugin()] as const);
30+
31+
const FIXTURE = fileURLToPath(new URL("./fixtures/stdio-mcp-server.mjs", import.meta.url));
32+
33+
const SECRET = "s3cr3t-typed-into-the-edit-sheet";
34+
35+
scenario(
36+
"Local · a stdio MCP server's command and environment are editable from the integration Edit sheet",
37+
{ timeout: 300_000 },
38+
Effect.gen(function* () {
39+
const cli = yield* Cli;
40+
const browser = yield* Browser;
41+
const target = yield* Target;
42+
const runDir = yield* RunDir;
43+
const identity = yield* target.newIdentity();
44+
45+
yield* withLocalServer(cli, runDir, (server) =>
46+
Effect.gen(function* () {
47+
const client = yield* HttpApiClient.make(api, {
48+
baseUrl: new URL("/api", server.origin).toString(),
49+
transformClient: HttpClient.mapRequest((request) =>
50+
HttpClientRequest.setHeader(request, "authorization", `Bearer ${server.token}`),
51+
),
52+
}).pipe(Effect.provide(FetchHttpClient.layer));
53+
54+
const slug = "e2e-stdio-editable";
55+
56+
// A plain stdio server: no declared env, so the env-gated tool is absent
57+
// until the edit adds the variable.
58+
yield* client.mcp.addServer({
59+
payload: {
60+
transport: "stdio",
61+
name: "E2E Stdio Editable",
62+
command: "node",
63+
args: [FIXTURE],
64+
slug,
65+
},
66+
});
67+
68+
const before = yield* client.tools.list({ query: { integration: slug } });
69+
expect(
70+
before.map((t) => t.name),
71+
"the server starts with its base tool and no declared env",
72+
).toContain("echo_tool");
73+
expect(
74+
before.map((t) => t.name),
75+
"the env-gated tool is absent before the edit",
76+
).not.toContain("saw_declared_env");
77+
78+
yield* browser.session(identity, async ({ page, step }) => {
79+
await step("Open the stdio integration from the console", async () => {
80+
await page.goto(server.url, { waitUntil: "domcontentloaded" });
81+
await page.getByTestId(`integration-entry-${slug}`).first().click();
82+
// Wait for the detail page's own data, not just its shell: clicking
83+
// Edit while the accounts panel is still a skeleton races the layout
84+
// shift that lands when it resolves.
85+
await page.getByText("default").first().waitFor({ timeout: 30_000 });
86+
await page.getByRole("button", { name: "Edit" }).waitFor({ timeout: 30_000 });
87+
});
88+
89+
await step("Open the Edit sheet — the stdio command is editable", async () => {
90+
await page.getByRole("button", { name: "Edit" }).click();
91+
await page.getByText("Edit integration").waitFor({ timeout: 30_000 });
92+
await page.getByText("Server command").waitFor({ timeout: 30_000 });
93+
// The read-only dead end this replaces.
94+
expect(
95+
await page.getByText("Stdio MCP integrations cannot be edited").count(),
96+
"the read-only message is gone",
97+
).toBe(0);
98+
expect(
99+
await page.getByRole("textbox", { name: "Command" }).inputValue(),
100+
"the stored command is loaded into the form",
101+
).toBe("node");
102+
});
103+
104+
await step("Declare an environment variable and save", async () => {
105+
await page
106+
.getByRole("textbox", { name: "Environment variables" })
107+
.fill(`EXECUTOR_E2E_SECRET=${SECRET}`);
108+
await page.getByRole("button", { name: "Save" }).click();
109+
await page.getByText("Server command").waitFor({ state: "hidden", timeout: 30_000 });
110+
});
111+
});
112+
113+
// The edit persisted as the DECLARED static env map on the config.
114+
const stored = yield* client.mcp.getServer({ params: { slug } });
115+
expect(
116+
stored?.config.transport === "stdio" ? stored.config.env : undefined,
117+
"the sheet wrote the declared env map",
118+
).toEqual({ EXECUTOR_E2E_SECRET: SECRET });
119+
expect(
120+
stored?.config.transport === "stdio" ? stored.config.command : undefined,
121+
"the command it did not touch is unchanged",
122+
).toBe("node");
123+
124+
// And it reached the spawned server: the fixture gates this tool's very
125+
// existence on that variable being in its own environment.
126+
const after = yield* client.tools.list({ query: { integration: slug } });
127+
expect(
128+
after.map((t) => t.name),
129+
"the edited environment reached the respawned server and the catalog was rebuilt",
130+
).toContain("saw_declared_env");
131+
expect(
132+
after.map((t) => t.name),
133+
"the rebuild kept the tools that still exist",
134+
).toContain("echo_tool");
135+
}),
136+
);
137+
}),
138+
);

e2e/local/stdio-mcp.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,48 @@ scenario(
222222
autoTools.map((t) => t.name),
223223
"auto negotiation falls back to legacy and still discovers tools",
224224
).toContain("echo_tool");
225+
expect(
226+
autoTools.map((t) => t.name),
227+
"this server declared no env, so the env-gated tool is absent to begin with",
228+
).not.toContain("saw_declared_env");
229+
230+
// --- Editing a stdio server's config (what the integration Edit sheet
231+
// now does instead of telling you to remove and recreate). The tool
232+
// catalog is persisted per connection, so a plain config replace is
233+
// only enough because core stamps `config_revised_at` on a config
234+
// write and every connection whose catalog predates that stamp is
235+
// rebuilt on the next read. The edit path therefore needs NO explicit
236+
// refresh of its own — this asserts that, so nobody adds one back.
237+
//
238+
// Adding a declared env var is the lever: the fixture advertises
239+
// `saw_declared_env` only when that variable reached the child, so the
240+
// tool appearing with no further action proves both halves — the new
241+
// config reached the spawn, and the catalog was rebuilt from it. ---
242+
const editedConfig = {
243+
...autoStored!.config,
244+
env: { EXECUTOR_E2E_SECRET: SECRET },
245+
} as typeof autoStored.config;
246+
247+
yield* client.mcp.configureServer({
248+
params: { slug: autoSlug },
249+
payload: { config: editedConfig },
250+
});
251+
252+
const editedStored = yield* client.mcp.getServer({ params: { slug: autoSlug } });
253+
expect(
254+
JSON.stringify(editedStored?.config ?? {}),
255+
"the edit persisted, and left the untouched negotiation mode alone",
256+
).toContain('"versionNegotiation":"auto"');
257+
258+
const editedTools = yield* client.tools.list({ query: { integration: autoSlug } });
259+
expect(
260+
editedTools.map((t) => t.name),
261+
"the edited config reached the spawn and the catalog was rediscovered",
262+
).toContain("saw_declared_env");
263+
expect(
264+
editedTools.map((t) => t.name),
265+
"rediscovery replaced the catalog rather than dropping what still exists",
266+
).toContain("echo_tool");
225267
}),
226268
{ env: DAEMON_ENV },
227269
);

packages/plugins/mcp/src/react/AddMcpIntegration.tsx

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import { McpRemoteIntegrationFields } from "./McpRemoteIntegrationFields";
4141
import { McpRequestHeadersEditor } from "./McpRequestHeadersEditor";
4242
import { mcpHeadersFromRows, type McpHeaderRow } from "./request-headers";
4343
import { mcpAuthMethodInputFromEditorValue, mcpWireAuthInput } from "./auth-method-config";
44+
import { parseStdioArgs } from "./stdio-fields";
4445
import { isProbableMcpEndpoint } from "./probe-url";
4546
import { cloudflareNeedsCodemodeOptOut } from "../sdk/cloudflare-codemode";
4647
import { mcpPresets, type McpPreset } from "../sdk/presets";
@@ -62,19 +63,6 @@ function findPreset(id: string | undefined): McpPreset | undefined {
6263
return mcpPresets.find((p) => p.id === id);
6364
}
6465

65-
// Splits the raw args field into tokens, honoring double-quoted groups so an
66-
// argument with spaces stays intact.
67-
function parseStdioArgs(raw: string): string[] {
68-
if (!raw.trim()) return [];
69-
const args: string[] = [];
70-
const regex = /[^\s"]+|"([^"]*)"/g;
71-
let match;
72-
while ((match = regex.exec(raw)) !== null) {
73-
args.push(match[1] ?? match[0]);
74-
}
75-
return args;
76-
}
77-
7866
// ---------------------------------------------------------------------------
7967
// State machine (remote flow)
8068
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)