From 437a06b73325a21d371f9df0bb51871222f18389 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Thu=E1=BA=ADn=20Ph=C3=A1t?= Date: Mon, 27 Apr 2026 10:17:00 +0700 Subject: [PATCH] fix: keep executor.jsonc in sync with source add/remove Two related bugs caused executor.jsonc and the runtime DB to drift out of sync, so sources resurrected after a UI delete and OAuth auth blocks vanished after a restart. 1. Engine-level removeSource (openapi, mcp, graphql) only deleted plugin storage rows. The SDK-level removeSpec/removeSource paths also call configFile.removeSource, but the engine hook (used by the HTTP `sources.remove` handler and the web UI delete button) never did. Effect: deleted source reappeared on next boot from executor.jsonc. 2. apps/local config-sync rebuilt the executor.mcp.addSource call for remote MCP sources without passing source.auth. addSource then wrote back to executor.jsonc with auth=undefined, so the {kind:"oauth2", connectionId} block silently disappeared on the first restart and the source went 401 with zero tools. Fixes: - Plugin engine-hook removeSource now mirrors removeSpec/removeSource behavior and calls options.configFile?.removeSource(sourceId) after the storage delete (openapi, mcp, graphql). - config-sync translates McpAuthConfig -> McpConnectionAuth via a new translateMcpAuth helper and forwards it to executor.mcp.addSource. Tests: - packages/plugins/openapi/src/sdk/plugin.test.ts: regression test asserts engine-level executor.sources.remove triggers configFile.removeSource. - apps/local/src/server/config-sync.test.ts: covers all McpAuthConfig variants (none, header w/ + w/o secret-public-ref prefix, oauth2, undefined). --- apps/local/src/server/config-sync.test.ts | 50 +++++++++++++++++++ apps/local/src/server/config-sync.ts | 25 ++++++++++ packages/plugins/graphql/src/sdk/plugin.ts | 7 ++- packages/plugins/mcp/src/sdk/plugin.ts | 3 ++ .../plugins/openapi/src/sdk/plugin.test.ts | 44 ++++++++++++++++ packages/plugins/openapi/src/sdk/plugin.ts | 7 ++- 6 files changed, 134 insertions(+), 2 deletions(-) create mode 100644 apps/local/src/server/config-sync.test.ts diff --git a/apps/local/src/server/config-sync.test.ts b/apps/local/src/server/config-sync.test.ts new file mode 100644 index 000000000..3e62b5208 --- /dev/null +++ b/apps/local/src/server/config-sync.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from "vitest"; + +import { translateMcpAuth } from "./config-sync"; + +describe("translateMcpAuth", () => { + it("returns undefined when no auth is configured", () => { + expect(translateMcpAuth(undefined)).toBeUndefined(); + }); + + it("preserves the kind=none variant", () => { + expect(translateMcpAuth({ kind: "none" })).toEqual({ kind: "none" }); + }); + + it("preserves oauth2 connectionId so the source keeps its OAuth link across boots", () => { + expect( + translateMcpAuth({ kind: "oauth2", connectionId: "mcp-oauth2-linear" }), + ).toEqual({ kind: "oauth2", connectionId: "mcp-oauth2-linear" }); + }); + + it("strips the secret-public-ref prefix from header auth", () => { + expect( + translateMcpAuth({ + kind: "header", + headerName: "Authorization", + secret: "secret-public-ref:my-token", + prefix: "Bearer ", + }), + ).toEqual({ + kind: "header", + headerName: "Authorization", + secretId: "my-token", + prefix: "Bearer ", + }); + }); + + it("passes through a raw secret id when no prefix is present", () => { + expect( + translateMcpAuth({ + kind: "header", + headerName: "X-Api-Key", + secret: "raw-id", + }), + ).toEqual({ + kind: "header", + headerName: "X-Api-Key", + secretId: "raw-id", + prefix: undefined, + }); + }); +}); diff --git a/apps/local/src/server/config-sync.ts b/apps/local/src/server/config-sync.ts index 22b36266c..4e55326f8 100644 --- a/apps/local/src/server/config-sync.ts +++ b/apps/local/src/server/config-sync.ts @@ -14,8 +14,10 @@ import type { SourceConfig, ExecutorFileConfig, ConfigHeaderValue, + McpAuthConfig, } from "@executor/config"; import { SECRET_REF_PREFIX } from "@executor/config"; +import type { McpConnectionAuth } from "@executor/plugin-mcp"; import type { LocalExecutor } from "./executor"; @@ -53,6 +55,28 @@ const translateHeaders = ( return out; }; +// MCP auth translation: file format → plugin format. The header variant +// stores credentials as `secret-public-ref:`; the plugin SDK takes the +// raw secret id. The oauth2 variant is structurally identical. +export const translateMcpAuth = ( + auth: McpAuthConfig | undefined, +): McpConnectionAuth | undefined => { + if (!auth) return undefined; + if (auth.kind === "none") return { kind: "none" }; + if (auth.kind === "header") { + const secretId = auth.secret.startsWith(SECRET_REF_PREFIX) + ? auth.secret.slice(SECRET_REF_PREFIX.length) + : auth.secret; + return { + kind: "header", + headerName: auth.headerName, + secretId, + prefix: auth.prefix, + }; + } + return { kind: "oauth2", connectionId: auth.connectionId }; +}; + // --------------------------------------------------------------------------- // Config path resolution // --------------------------------------------------------------------------- @@ -129,6 +153,7 @@ const addSourceFromConfig = ( queryParams: source.queryParams, headers: source.headers, namespace: source.namespace, + auth: translateMcpAuth(source.auth), }).pipe(Effect.asVoid); } }; diff --git a/packages/plugins/graphql/src/sdk/plugin.ts b/packages/plugins/graphql/src/sdk/plugin.ts index 575e20593..33a50d753 100644 --- a/packages/plugins/graphql/src/sdk/plugin.ts +++ b/packages/plugins/graphql/src/sdk/plugin.ts @@ -565,7 +565,12 @@ export const graphqlPlugin = definePlugin( }), removeSource: ({ ctx, sourceId, scope }) => - ctx.storage.removeSource(sourceId, scope), + Effect.gen(function* () { + yield* ctx.storage.removeSource(sourceId, scope); + if (options?.configFile) { + yield* options.configFile.removeSource(sourceId); + } + }), detect: ({ url }) => Effect.gen(function* () { diff --git a/packages/plugins/mcp/src/sdk/plugin.ts b/packages/plugins/mcp/src/sdk/plugin.ts index 841a563aa..75fdc554b 100644 --- a/packages/plugins/mcp/src/sdk/plugin.ts +++ b/packages/plugins/mcp/src/sdk/plugin.ts @@ -1267,6 +1267,9 @@ export const mcpPlugin = definePlugin( Effect.gen(function* () { yield* ctx.storage.removeBindingsByNamespace(sourceId, scope); yield* ctx.storage.removeSource(sourceId, scope); + if (options?.configFile) { + yield* options.configFile.removeSource(sourceId); + } }), refreshSource: () => Effect.void, diff --git a/packages/plugins/openapi/src/sdk/plugin.test.ts b/packages/plugins/openapi/src/sdk/plugin.test.ts index 5fee6eae8..25a74a69d 100644 --- a/packages/plugins/openapi/src/sdk/plugin.test.ts +++ b/packages/plugins/openapi/src/sdk/plugin.test.ts @@ -24,6 +24,7 @@ import { type SecretProvider, type Where, } from "@executor/sdk"; +import type { ConfigFileSink } from "@executor/config"; const TEST_SCOPE = "test-scope"; import { openApiPlugin } from "./plugin"; @@ -488,6 +489,49 @@ layer(TestLayer)("OpenAPI Plugin", (it) => { }), ); + it.effect( + "executor.sources.remove writes back to configFile (engine-level remove)", + () => + Effect.gen(function* () { + const httpClient = yield* HttpClient.HttpClient; + const clientLayer = Layer.succeed(HttpClient.HttpClient, httpClient); + + const removeCalls: string[] = []; + const upsertCalls: string[] = []; + const configFile: ConfigFileSink = { + upsertSource: (source) => + Effect.sync(() => { + upsertCalls.push(source.namespace ?? ""); + }), + removeSource: (namespace) => + Effect.sync(() => { + removeCalls.push(namespace); + }), + }; + + const executor = yield* createExecutor( + makeTestConfig({ + plugins: [ + openApiPlugin({ httpClientLayer: clientLayer, configFile }), + memorySecretsPlugin(), + ] as const, + }), + ); + + yield* executor.openapi.addSpec({ + spec: specJson, + scope: TEST_SCOPE, + namespace: "removable", + baseUrl: "", + }); + expect(upsertCalls).toEqual(["removable"]); + + yield* executor.sources.remove("removable"); + + expect(removeCalls).toEqual(["removable"]); + }), + ); + // ------------------------------------------------------------------------- // Multi-scope shadowing — regression suite covering the bug class where // store reads/writes that don't pin scope_id collapse onto whichever row diff --git a/packages/plugins/openapi/src/sdk/plugin.ts b/packages/plugins/openapi/src/sdk/plugin.ts index 3f6571e6a..e4d5841fc 100644 --- a/packages/plugins/openapi/src/sdk/plugin.ts +++ b/packages/plugins/openapi/src/sdk/plugin.ts @@ -1392,7 +1392,12 @@ export const openApiPlugin = definePlugin( }), removeSource: ({ ctx, sourceId, scope }) => - ctx.storage.removeSource(sourceId, scope), + Effect.gen(function* () { + yield* ctx.storage.removeSource(sourceId, scope); + if (options?.configFile) { + yield* options.configFile.removeSource(sourceId); + } + }), // Re-fetch the spec from its origin URL (captured at addSpec time) // and replay the same parse → extract → upsertSource → register