diff --git a/e2e/scenarios/openapi-server-selection-ui.test.ts b/e2e/scenarios/openapi-server-selection-ui.test.ts new file mode 100644 index 000000000..e7f76f065 --- /dev/null +++ b/e2e/scenarios/openapi-server-selection-ui.test.ts @@ -0,0 +1,72 @@ +// Cross-target (browser): the UI side of per-call server selection. When a +// pasted spec declares more than one server, the Add OpenAPI source form turns +// the Base URL field into a picker over those servers and relabels it an +// optional override — the host is otherwise resolved per tool call. The session +// video + per-step screenshots are the artifact; this scenario skips on targets +// without a browser surface (selfhost today). +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; + +import { scenario } from "../src/scenario"; +import { Browser, Target } from "../src/services"; + +// Two servers — production and staging — so the form offers a base-URL picker +// instead of the single locked input a one-server spec gets. +const multiServerSpec = JSON.stringify({ + openapi: "3.0.3", + info: { title: "Regions API", version: "1.0.0" }, + servers: [ + { url: "https://api.example.com", description: "Production" }, + { url: "https://staging.example.com", description: "Staging" }, + ], + paths: { + "/ping": { get: { operationId: "ping", responses: { "200": { description: "pong" } } } }, + }, +}); + +scenario( + "OpenAPI · the add form offers a server picker and an optional base URL for a multi-server spec", + {}, + Effect.scoped( + Effect.gen(function* () { + const target = yield* Target; + const browser = yield* Browser; + const identity = yield* target.newIdentity(); + + yield* browser.session(identity, async ({ page, step }) => { + await step("Open the Add OpenAPI source form", async () => { + await page.goto("/integrations/add/openapi", { waitUntil: "networkidle" }); + await page.getByPlaceholder("https://api.example.com/openapi.json").waitFor(); + }); + + await step("Paste a spec that declares two servers", async () => { + await page.getByPlaceholder("https://api.example.com/openapi.json").fill(multiServerSpec); + // The form auto-analyzes (debounced) and renders the preview details, + // where the base URL is now an OPTIONAL override. + await page.getByText("Base URL override (optional)").waitFor({ timeout: 20_000 }); + }); + + await step("The base URL is optional — the server is chosen per call", async () => { + // The hint spells out that leaving it empty defers the host (and its + // variables) to each tool call: the heart of per-call selection. + await page.getByText(/leave empty to choose the server.*per tool call/i).waitFor(); + }); + + await step("The field is a picker over the spec's two servers", async () => { + // Opening the combobox reveals both declared servers as choices. + await page.getByPlaceholder("https://api.example.com", { exact: true }).click(); + const options = page.getByRole("option"); + await options.filter({ hasText: "staging.example.com" }).waitFor({ timeout: 10_000 }); + const labels = await options.allInnerTexts(); + expect( + labels.join(" | "), + "both declared servers are offered as base-URL choices", + ).toEqual(expect.stringContaining("https://staging.example.com")); + expect(labels.join(" | "), "production is offered too").toEqual( + expect.stringContaining("https://api.example.com"), + ); + }); + }); + }), + ), +); diff --git a/e2e/scenarios/openapi-server-selection.test.ts b/e2e/scenarios/openapi-server-selection.test.ts new file mode 100644 index 000000000..7b0c92b75 --- /dev/null +++ b/e2e/scenarios/openapi-server-selection.test.ts @@ -0,0 +1,169 @@ +// Cross-target: when a spec declares more than one server, per-call server +// selection becomes a first-class tool input. The generated tool advertises an +// optional `server` selector — a `url` enum over the declared servers plus the +// `{variables}` drawn from the templated one — so an agent can choose where each +// call goes. Adding the spec with NO baseUrl also proves the base URL is now an +// optional override: the host is resolved per call from the spec's servers. +// +// Entirely through the typed client: addSpec → connection (via a `from` provider +// reference, so no vault round-trip — works against the cloud stub) → read the +// tool's schema and assert the `server` input this feature introduces. +import { randomBytes, randomUUID } from "node:crypto"; + +import { expect } from "@effect/vitest"; +import { Effect } from "effect"; +import { composePluginApi } from "@executor-js/api/server"; +import { openApiHttpPlugin } from "@executor-js/plugin-openapi/api"; +import { + AuthTemplateSlug, + ConnectionName, + IntegrationSlug, + ProviderItemId, +} from "@executor-js/sdk/shared"; + +import { scenario } from "../src/scenario"; +import { Api, Target } from "../src/services"; + +const api = composePluginApi([openApiHttpPlugin()] as const); + +// The operation inherits two document-level servers: a fixed production URL and +// a templated regional sandbox whose `{region}` is a per-call variable. +const PROD_SERVER = "https://api.example.test/v1"; +const SANDBOX_SERVER = "https://{region}.sandbox.example.test/v1"; + +const multiServerSpec = (): string => + JSON.stringify({ + openapi: "3.0.3", + info: { title: "Regions API", version: "1.0.0" }, + servers: [ + { url: PROD_SERVER, description: "Production" }, + { + url: SANDBOX_SERVER, + description: "Regional sandbox", + variables: { + region: { default: "us", enum: ["us", "eu", "ap"], description: "Sandbox region" }, + }, + }, + ], + paths: { + "/ping": { + get: { + operationId: "ping", + summary: "Health check", + responses: { "200": { description: "pong" } }, + }, + }, + }, + }); + +// Minimal structural view of the JSON Schema we assert against. +type ServerInputSchema = { + readonly properties?: { + readonly server?: { + readonly properties?: { + readonly url?: { readonly enum?: readonly unknown[]; readonly default?: unknown }; + readonly variables?: { + readonly properties?: Record< + string, + { readonly enum?: readonly unknown[]; readonly default?: unknown } + >; + }; + }; + }; + }; +}; + +scenario( + "OpenAPI · a multi-server spec advertises a per-call server selector on its tools", + {}, + Effect.gen(function* () { + const target = yield* Target; + const { client } = yield* Api; + const identity = yield* target.newIdentity(); + const apiClient = yield* client(api, identity); + + // Unique slug per run: selfhost shares the bootstrap-admin identity, so the + // prefix keeps parallel/repeated runs out of each other's catalogs. + const slug = `openapi-scn-servers-${randomBytes(4).toString("hex")}`; + + yield* Effect.ensuring( + Effect.gen(function* () { + // Add the spec with NO baseUrl — the host is resolved per call from the + // spec's servers, so a base URL is purely an optional override now. + const added = yield* apiClient.openapi.addSpec({ + payload: { + spec: { kind: "blob", value: multiServerSpec() }, + slug, + authenticationTemplate: [ + { + slug: "apiKey", + type: "apiKey", + headers: { "x-api-key": [{ type: "variable", name: "token" }] }, + }, + ], + }, + }); + expect(added.toolCount, "the spec's operation became a tool").toBeGreaterThan(0); + + // The catalog stamps tools once a connection exists; a `from` provider + // reference avoids any vault round-trip. + const providers = yield* apiClient.providers.list(); + expect(providers.length, "a credential provider is available").toBeGreaterThan(0); + yield* apiClient.connections.create({ + payload: { + owner: "org", + name: ConnectionName.make("main"), + integration: IntegrationSlug.make(slug), + template: AuthTemplateSlug.make("apiKey"), + from: { provider: providers[0]!, id: ProviderItemId.make(randomUUID()) }, + }, + }); + + // Locate the generated tool and read its full input schema. + const tools = yield* apiClient.tools.list({ query: {} }); + const ping = tools.find( + (tool) => String(tool.integration) === slug && String(tool.name).includes("ping"), + ); + expect(ping?.address, "the ping tool is in the catalog").toBeDefined(); + + const view = yield* apiClient.tools.schema({ query: { address: ping!.address } }); + const input = view.inputSchema as ServerInputSchema; + const serverInput = input.properties?.server?.properties; + + // The per-call `server` input exists: `url` is an enum over BOTH declared + // servers (raw templates), defaulting to the first. + expect(serverInput?.url?.enum, "server.url enumerates the declared servers").toEqual([ + PROD_SERVER, + SANDBOX_SERVER, + ]); + expect(serverInput?.url?.default, "server.url defaults to the first server").toBe( + PROD_SERVER, + ); + + // …and the templated server's `{region}` surfaces as a per-call variable + // carrying its spec enum and default. + const region = serverInput?.variables?.properties?.region; + expect(region?.enum, "server.variables.region carries the spec enum").toEqual([ + "us", + "eu", + "ap", + ]); + expect(region?.default, "server.variables.region keeps the spec default").toBe("us"); + }), + // Selfhost shares one bootstrap admin, so this scenario must not leak its + // connection or integration into other scenarios' zero-state assertions. + Effect.gen(function* () { + yield* apiClient.connections + .remove({ + params: { + owner: "org", + integration: IntegrationSlug.make(slug), + name: ConnectionName.make("main"), + }, + }) + .pipe(Effect.ignore); + yield* apiClient.openapi.removeSpec({ params: { slug } }).pipe(Effect.ignore); + }), + ); + }), +); diff --git a/packages/plugins/openapi/src/react/AddOpenApiSource.tsx b/packages/plugins/openapi/src/react/AddOpenApiSource.tsx index 3632a16b9..cac9b4698 100644 --- a/packages/plugins/openapi/src/react/AddOpenApiSource.tsx +++ b/packages/plugins/openapi/src/react/AddOpenApiSource.tsx @@ -45,11 +45,10 @@ import { type GoogleOpenApiPreset, } from "../sdk/google-presets"; import type { SpecPreview } from "../sdk/preview"; -import { type Authentication, type ServerInfo } from "../sdk/types"; -import { expandServerUrlOptions } from "../sdk/openapi-utils"; -import { detectedAuthenticationTemplates, firstBaseUrlForPreview } from "../sdk/derive-auth"; +import { type Authentication } from "../sdk/types"; +import { resolveServerUrl } from "../sdk/openapi-utils"; +import { detectedAuthenticationTemplates } from "../sdk/derive-auth"; -const GOOGLE_BUNDLE_BASE_URL = "https://www.googleapis.com/"; const GOOGLE_BUNDLE_FAVICON = "https://fonts.gstatic.com/s/i/productlogos/googleg/v6/192px.svg"; // The bundle picker opens with the featured Google APIs pre-checked. @@ -103,9 +102,6 @@ const specInputForAdd = (input: string) => { : { kind: "blob" as const, value }; }; -const expandServerOptions = (server: ServerInfo) => - expandServerUrlOptions(server).map((value) => ({ value, label: value })); - // --------------------------------------------------------------------------- // Component — single progressive form. Post-redesign: preview → addSpec // (register the integration catalog entry with ALL detected auth methods) → @@ -136,7 +132,7 @@ export default function AddOpenApiSource(props: { // After analysis const [preview, setPreview] = useState(null); - const [baseUrl, setBaseUrl] = useState(isGoogleBundlePreset ? GOOGLE_BUNDLE_BASE_URL : ""); + const [baseUrl, setBaseUrl] = useState(""); const identityFallbackName = isGoogleBundlePreset ? "Google" : preview @@ -198,10 +194,20 @@ export default function AddOpenApiSource(props: { // ---- Derived state ---- - const servers: readonly ServerInfo[] = preview?.servers ?? []; - const baseUrlOptions = Array.from( - new Map(servers.flatMap(expandServerOptions).map((option) => [option.value, option])).values(), - ); + const previewHasNoServers = preview !== null && preview.servers.length === 0; + // Offer the spec's servers (resolved with defaults) as base-URL choices when + // there's more than one; a single/no server uses a plain input. + const baseUrlOptions = + preview && preview.servers.length > 1 + ? preview.servers.map((server) => { + const url = resolveServerUrl(server.url, Option.getOrUndefined(server.variables), {}); + return { value: url, label: url }; + }) + : undefined; + const firstServer = preview?.servers[0]; + const firstServerUrl = firstServer + ? resolveServerUrl(firstServer.url, Option.getOrUndefined(firstServer.variables), {}) + : ""; const previewPresetIcon = openApiPresets.find( (preset) => preset.url && normalizePresetUrl(preset.url) === normalizePresetUrl(specUrl), @@ -275,7 +281,12 @@ export default function AddOpenApiSource(props: { const hasPreviewOrBundle = isGoogleBundlePreset ? bundleDiscoveryUrls.length > 0 : preview !== null; - const canAdd = hasPreviewOrBundle && resolvedBaseUrl.length > 0 && !slugAlreadyExists; + // The base URL is optional when the spec declares servers (resolved per call); + // required only when it doesn't. + const canAdd = + hasPreviewOrBundle && + !slugAlreadyExists && + (!previewHasNoServers || resolvedBaseUrl.length > 0); // ---- Handlers ---- @@ -291,7 +302,7 @@ export default function AddOpenApiSource(props: { } const result = exit.value; setPreview(result); - setBaseUrl(firstBaseUrlForPreview(result)); + setBaseUrl(""); setAnalyzing(false); }; @@ -423,9 +434,9 @@ export default function AddOpenApiSource(props: { identity={identity} baseUrl={resolvedBaseUrl} onBaseUrlChange={setBaseUrl} + baseUrlLabel="Base URL override (optional)" faviconIcon={GOOGLE_BUNDLE_FAVICON} faviconUrl={resolvedBaseUrl} - baseUrlMissingMessage="A base URL is required to make requests." /> ) : preview ? ( { setSpecUrl(value); @@ -448,8 +469,7 @@ export default function AddOpenApiSource(props: { setBaseUrl(""); }} faviconIcon={previewPresetIcon} - faviconUrl={resolvedBaseUrl} - baseUrlMissingMessage="A base URL is required to make requests." + faviconUrl={resolvedBaseUrl || firstServerUrl} /> ) : null} diff --git a/packages/plugins/openapi/src/react/OpenApiSourceDetailsFields.tsx b/packages/plugins/openapi/src/react/OpenApiSourceDetailsFields.tsx index fd59dbdb8..b66c6d6f2 100644 --- a/packages/plugins/openapi/src/react/OpenApiSourceDetailsFields.tsx +++ b/packages/plugins/openapi/src/react/OpenApiSourceDetailsFields.tsx @@ -25,6 +25,9 @@ export function OpenApiSourceDetailsFields(props: { readonly baseUrl: string; readonly onBaseUrlChange: (value: string) => void; readonly baseUrlOptions?: readonly FreeformComboboxOption[]; + readonly baseUrlLabel?: string; + readonly baseUrlPlaceholder?: string; + readonly baseUrlHint?: string; readonly specUrl?: string; readonly onSpecUrlChange?: (value: string) => void; readonly faviconIcon?: string | null; @@ -35,8 +38,6 @@ export function OpenApiSourceDetailsFields(props: { readonly baseUrlMissingMessage?: string; readonly footer?: string; }) { - const baseUrlOptions = props.baseUrlOptions ?? []; - return ( @@ -61,13 +62,13 @@ export function OpenApiSourceDetailsFields(props: { namespaceReadOnly={props.namespaceReadOnly} />
- - {baseUrlOptions.length > 0 ? ( + + {props.baseUrlOptions && props.baseUrlOptions.length > 0 ? ( @@ -75,7 +76,7 @@ export function OpenApiSourceDetailsFields(props: { props.onBaseUrlChange((e.target as HTMLInputElement).value)} - placeholder="https://api.example.com" + placeholder={props.baseUrlPlaceholder ?? "https://api.example.com"} className="font-mono text-sm" /> )} @@ -85,6 +86,9 @@ export function OpenApiSourceDetailsFields(props: { {props.baseUrlMissingMessage}

)} + {props.baseUrlHint && ( +

{props.baseUrlHint}

+ )}
{props.specUrl !== undefined && props.onSpecUrlChange && ( diff --git a/packages/plugins/openapi/src/sdk/config.ts b/packages/plugins/openapi/src/sdk/config.ts index eb7894b45..22dc26ef2 100644 --- a/packages/plugins/openapi/src/sdk/config.ts +++ b/packages/plugins/openapi/src/sdk/config.ts @@ -46,7 +46,7 @@ export const OpenApiIntegrationConfigSchema = Schema.Struct({ sourceUrl: Schema.optional(Schema.String), /** Google Discovery bundle URLs, when the spec came from a Google bundle. */ googleDiscoveryUrls: Schema.optional(Schema.Array(Schema.String)), - /** Base URL override; falls back to the spec's first server. */ + /** Optional base URL override. */ baseUrl: Schema.optional(Schema.String), /** Static headers applied to every request (no secret material). */ headers: Schema.optional(Schema.Record(Schema.String, Schema.String)), diff --git a/packages/plugins/openapi/src/sdk/derive-auth.ts b/packages/plugins/openapi/src/sdk/derive-auth.ts index 8bd6336ba..6fd07735a 100644 --- a/packages/plugins/openapi/src/sdk/derive-auth.ts +++ b/packages/plugins/openapi/src/sdk/derive-auth.ts @@ -11,7 +11,7 @@ import { AuthTemplateSlug, type OAuthAuthentication } from "@executor-js/sdk/sha import type { HeaderPreset, OAuth2Preset, SpecPreview } from "./preview"; import type { APIKeyAuthentication, Authentication } from "./types"; -import { expandServerUrlOptions } from "./openapi-utils"; +import { resolveServerUrl } from "./openapi-utils"; // --------------------------------------------------------------------------- // OpenAPI url helpers — specs sometimes ship relative OAuth endpoints; resolve @@ -134,7 +134,9 @@ export const detectedAuthenticationTemplates = ( export const firstBaseUrlForPreview = (preview: SpecPreview): string => { const firstServer = preview.servers[0]; - return firstServer ? (expandServerUrlOptions(firstServer)[0] ?? "") : ""; + return firstServer + ? resolveServerUrl(firstServer.url, Option.getOrUndefined(firstServer.variables), {}) + : ""; }; /** The fallback `addSpec` uses when no explicit template was passed: every diff --git a/packages/plugins/openapi/src/sdk/extract.ts b/packages/plugins/openapi/src/sdk/extract.ts index efe38faf1..2720a890d 100644 --- a/packages/plugins/openapi/src/sdk/extract.ts +++ b/packages/plugins/openapi/src/sdk/extract.ts @@ -6,7 +6,6 @@ import { declaredContents, DocResolver, preferredResponseContent, - resolveBaseUrl, type OperationObject, type ParameterObject, type PathItemObject, @@ -169,9 +168,60 @@ const extractOutputSchema = (operation: OperationObject, r: DocResolver): unknow // Input schema builder // --------------------------------------------------------------------------- +// Optional `server` input — host selection + server-URL variables. Undefined +// when there's nothing to configure (a single server with no variables). +const buildServerInputProperty = ( + servers: readonly ServerInfo[], +): Record | undefined => { + const variableDefs: Record = {}; + for (const server of servers) { + for (const [name, v] of Object.entries(Option.getOrUndefined(server.variables) ?? {})) { + if (!(name in variableDefs)) variableDefs[name] = v; + } + } + const hasMultiple = servers.length > 1; + const variableNames = Object.keys(variableDefs); + if (!hasMultiple && variableNames.length === 0) return undefined; + + const properties: Record = {}; + if (hasMultiple) { + properties.url = { + type: "string", + enum: servers.map((server) => server.url), + default: servers[0]!.url, + description: "Which of the spec's servers to send the request to.", + }; + } + if (variableNames.length > 0) { + properties.variables = { + type: "object", + additionalProperties: false, + properties: Object.fromEntries( + Object.entries(variableDefs).map(([name, v]) => [ + name, + { + type: "string", + default: v.default, + ...(Option.isSome(v.enum) ? { enum: v.enum.value } : {}), + ...(Option.isSome(v.description) ? { description: v.description.value } : {}), + }, + ]), + ), + description: "Values for the server URL `{variables}`; spec defaults apply when omitted.", + }; + } + return { + type: "object", + additionalProperties: false, + properties, + description: "Optional host selection and server-URL variables for this request.", + }; +}; + const buildInputSchema = ( parameters: readonly OperationParameter[], requestBody: OperationRequestBody | undefined, + servers: readonly ServerInfo[], ): Record | undefined => { const properties: Record = {}; const required: string[] = []; @@ -181,6 +231,10 @@ const buildInputSchema = ( if (param.required) required.push(param.name); } + // A path/query parameter named `server` takes precedence over the host input. + const serverProperty = buildServerInputProperty(servers); + if (serverProperty && !("server" in properties)) properties.server = serverProperty; + if (requestBody) { properties.body = Option.getOrElse(requestBody.schema, () => ({ type: "object" })); if (requestBody.required) required.push("body"); @@ -283,17 +337,16 @@ const extractServerList = (servers: readonly ServerObject[] | undefined): Server const extractServers = (doc: ParsedDocument): ServerInfo[] => extractServerList(doc.servers); -const extractOperationBaseUrl = ( +const operationServers = ( pathItem: PathItemObject, operation: OperationObject, -): string | undefined => { - const operationServers = extractServerList(operation.servers); - if (operationServers.length > 0) return resolveBaseUrl(operationServers); - - const pathServers = extractServerList(pathItem.servers); - if (pathServers.length > 0) return resolveBaseUrl(pathServers); - - return undefined; + docServers: readonly ServerInfo[], +): readonly ServerInfo[] => { + const operationLevel = extractServerList(operation.servers); + if (operationLevel.length > 0) return operationLevel; + const pathLevel = extractServerList(pathItem.servers); + if (pathLevel.length > 0) return pathLevel; + return docServers; }; // --------------------------------------------------------------------------- @@ -310,6 +363,7 @@ export const extract = Effect.fn("OpenApi.extract")(function* (doc: ParsedDocume } const r = new DocResolver(doc); + const docServers = extractServers(doc); const operations: ExtractedOperation[] = []; for (const [pathTemplate, pathItem] of Object.entries(paths).sort(([a], [b]) => @@ -323,7 +377,8 @@ export const extract = Effect.fn("OpenApi.extract")(function* (doc: ParsedDocume const parameters = extractParameters(pathItem, operation, r); const requestBody = extractRequestBody(operation, r); - const inputSchema = buildInputSchema(parameters, requestBody); + const servers = operationServers(pathItem, operation, docServers); + const inputSchema = buildInputSchema(parameters, requestBody, servers); const outputSchema = extractOutputSchema(operation, r); const tags = (operation.tags ?? []).filter((t) => t.trim().length > 0); const operationPathTemplate = explicitPathTemplate(operation) ?? pathTemplate; @@ -333,7 +388,7 @@ export const extract = Effect.fn("OpenApi.extract")(function* (doc: ParsedDocume operationId: OperationId.make(deriveOperationId(method, pathTemplate, operation)), toolPath: Option.fromNullishOr(explicitToolPath(operation)), method, - baseUrl: extractOperationBaseUrl(pathItem, operation), + servers, pathTemplate: operationPathTemplate, summary: Option.fromNullishOr(operation.summary), description: Option.fromNullishOr(operation.description), @@ -351,7 +406,7 @@ export const extract = Effect.fn("OpenApi.extract")(function* (doc: ParsedDocume return ExtractionResult.make({ title: Option.fromNullishOr(doc.info?.title), version: Option.fromNullishOr(doc.info?.version), - servers: extractServers(doc), + servers: docServers, operations, }); }); diff --git a/packages/plugins/openapi/src/sdk/google-discovery.test.ts b/packages/plugins/openapi/src/sdk/google-discovery.test.ts index e9a84e639..0f50fc571 100644 --- a/packages/plugins/openapi/src/sdk/google-discovery.test.ts +++ b/packages/plugins/openapi/src/sdk/google-discovery.test.ts @@ -436,9 +436,9 @@ it.effect("bundles Google Discovery documents into one Google OpenAPI source", ( const extractedChatMessage = extracted.operations.find( (candidate) => candidate.operationId === "chat.spaces.messages.get", ); - expect(extractedGmail?.baseUrl).toBe("https://gmail.googleapis.com/"); + expect(extractedGmail?.servers[0]?.url).toBe("https://gmail.googleapis.com/"); expect(extractedChatMessage?.pathTemplate).toBe("/v1/{+name}"); - expect(extractedChatMessage?.baseUrl).toBe("https://chat.googleapis.com/"); + expect(extractedChatMessage?.servers[0]?.url).toBe("https://chat.googleapis.com/"); // v2: the bundled oauth scopes are carried on the oauth auth template. const oauthTemplate = result.authenticationTemplate?.find((entry) => entry.kind === "oauth2"); expect(oauthTemplate?.kind === "oauth2" ? oauthTemplate.scopes : undefined).toEqual([ diff --git a/packages/plugins/openapi/src/sdk/index.ts b/packages/plugins/openapi/src/sdk/index.ts index cc3ed7c3d..319d2d355 100644 --- a/packages/plugins/openapi/src/sdk/index.ts +++ b/packages/plugins/openapi/src/sdk/index.ts @@ -40,7 +40,7 @@ export { } from "./preview"; export { DocResolver, - resolveBaseUrl, + resolveServerUrl, substituteUrlVariables, preferredContent, } from "./openapi-utils"; diff --git a/packages/plugins/openapi/src/sdk/invoke.ts b/packages/plugins/openapi/src/sdk/invoke.ts index 498f53695..df36cf13e 100644 --- a/packages/plugins/openapi/src/sdk/invoke.ts +++ b/packages/plugins/openapi/src/sdk/invoke.ts @@ -2,12 +2,14 @@ import { Effect, Layer, Option } from "effect"; import { HttpClient, HttpClientRequest } from "effect/unstable/http"; import { OpenApiInvocationError } from "./errors"; +import { resolveServerUrl } from "./openapi-utils"; import { type EncodingObject, type OperationBinding, InvocationResult, type MediaBinding, type OperationParameter, + type ServerInfo, } from "./types"; // --------------------------------------------------------------------------- @@ -572,8 +574,34 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* ( }); }); +// Connection `baseUrl` wins; otherwise the call's chosen server (`server.url`, or +// the first) resolved with its `{variables}` (call values, else spec defaults). +const resolveRequestHost = ( + servers: readonly ServerInfo[], + serverArg: unknown, + baseUrl: string, +): string => { + if (baseUrl) return baseUrl; + if (servers.length === 0) return ""; + + const arg = ( + typeof serverArg === "object" && serverArg !== null && !Array.isArray(serverArg) + ? serverArg + : {} + ) as { url?: unknown; variables?: unknown }; + const chosen = servers.find((server) => server.url === arg.url) ?? servers[0]!; + + const overrides: Record = {}; + if (typeof arg.variables === "object" && arg.variables !== null) { + for (const [name, value] of Object.entries(arg.variables as Record)) { + if (value != null && value !== "") overrides[name] = String(value); + } + } + return resolveServerUrl(chosen.url, Option.getOrUndefined(chosen.variables), overrides); +}; + // --------------------------------------------------------------------------- -// Invoke with a provided HttpClient layer + optional baseUrl prefix +// Invoke with a provided HttpClient layer + per-call host resolution // --------------------------------------------------------------------------- export const invokeWithLayer = ( @@ -584,8 +612,7 @@ export const invokeWithLayer = ( sourceQueryParams: Record, httpClientLayer: Layer.Layer, ) => { - const operationBaseUrl = operation.baseUrl; - const effectiveBaseUrl = operationBaseUrl ?? baseUrl; + const effectiveBaseUrl = resolveRequestHost(operation.servers ?? [], args.server, baseUrl); const clientWithBaseUrl = effectiveBaseUrl ? Layer.effect( HttpClient.HttpClient, diff --git a/packages/plugins/openapi/src/sdk/openapi-utils.ts b/packages/plugins/openapi/src/sdk/openapi-utils.ts index 3617496e4..39ed375e6 100644 --- a/packages/plugins/openapi/src/sdk/openapi-utils.ts +++ b/packages/plugins/openapi/src/sdk/openapi-utils.ts @@ -4,7 +4,6 @@ // Wraps the openapi-types V3/V3_1 union mess and provides clean ref resolution. // --------------------------------------------------------------------------- -import { Option } from "effect"; import type { OpenAPIV3, OpenAPIV3_1 } from "openapi-types"; import type { ParsedDocument } from "./parse"; import type { ServerVariable } from "./types"; @@ -65,49 +64,20 @@ export const substituteUrlVariables = (url: string, values: Record>; -}; - -export const expandServerUrlOptions = ( - server: ServerLike, - limit = OPENAPI_MAX_SERVER_VARIABLE_OPTIONS, -): readonly string[] => { - if (!Option.isSome(server.variables)) return [server.url]; - let urls: readonly string[] = [server.url]; - for (const [name, variable] of Object.entries(server.variables.value)) { - const enumValues = - typeof variable === "string" ? [] : Option.getOrElse(variable.enum, () => []); - const values = - enumValues.length > 0 - ? enumValues - : [typeof variable === "string" ? variable : variable.default]; - const next: string[] = []; - for (const url of urls) { - for (const value of values) { - next.push(url.replaceAll(`{${name}}`, value)); - if (next.length >= limit) return next; - } - } - urls = next; - } - return urls; -}; - -export const resolveBaseUrl = (servers: readonly ServerLike[]): string => { - const server = servers[0]; - if (!server) return ""; - - if (!Option.isSome(server.variables)) return server.url; - +/** Resolve a templated server URL, filling each `{var}` from `overrides` when + * non-empty, otherwise the variable's spec default. URLs without placeholders + * pass through unchanged. */ +export const resolveServerUrl = ( + templateUrl: string, + variables: Record | undefined, + overrides: Record, +): string => { const values: Record = {}; - for (const [name, v] of Object.entries(server.variables.value)) { - values[name] = typeof v === "string" ? v : v.default; + for (const [name, v] of Object.entries(variables ?? {})) values[name] = v.default; + for (const [name, value] of Object.entries(overrides)) { + if (value) values[name] = value; } - return substituteUrlVariables(server.url, values); + return substituteUrlVariables(templateUrl, values); }; // --------------------------------------------------------------------------- diff --git a/packages/plugins/openapi/src/sdk/plugin.test.ts b/packages/plugins/openapi/src/sdk/plugin.test.ts index c3578da05..354b67804 100644 --- a/packages/plugins/openapi/src/sdk/plugin.test.ts +++ b/packages/plugins/openapi/src/sdk/plugin.test.ts @@ -559,28 +559,32 @@ describe("OpenAPI Plugin", () => { ), ); - it.effect("addSpec defaults baseUrl to the spec's first server when omitted", () => + it.effect("addSpec omits baseUrl and resolves the host per call from the spec's servers", () => Effect.scoped( Effect.gen(function* () { const server = yield* servePluginTestApi(); const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() })); - // No baseUrl passed — the spec's own servers entry must fill it, or - // every invocation on the integration fails with no host to call. - // oxlint-disable-next-line executor/no-json-parse -- boundary: test fixture surgery on the test server's own spec JSON - const spec = JSON.parse(server.specJson) as Record; - const specWithServer = JSON.stringify({ - ...spec, - servers: [{ url: server.baseUrl }], + // No baseUrl override: the spec declares `servers`, so the host is + // resolved per call from the operation's servers rather than baked into + // the connection config. `baseUrl: null` suppresses the test helper's + // default connection-level override. + const conn = yield* addOpenApiTestConnection(executor, server, { + slug: "per_call_host", + baseUrl: null, }); - yield* executor.openapi.addSpec({ - spec: { kind: "blob", value: specWithServer }, - slug: "derived_base_api", - }); + // The override is absent… + const config = yield* executor.openapi.getConfig("per_call_host"); + expect(config?.baseUrl).toBeUndefined(); - const config = yield* executor.openapi.getConfig("derived_base_api"); - expect(config?.baseUrl).toBe(server.baseUrl); + // …yet the integration is still invocable: the request reaches the + // spec's server host with no baked baseUrl. + const result = unwrapInvocation( + yield* executor.execute(conn.address("items.listItems"), {}), + ); + expect(result.error).toBeNull(); + expect(result.data).toEqual(ITEMS); }), ), ); diff --git a/packages/plugins/openapi/src/sdk/plugin.ts b/packages/plugins/openapi/src/sdk/plugin.ts index e80e81d9f..69d3e6536 100644 --- a/packages/plugins/openapi/src/sdk/plugin.ts +++ b/packages/plugins/openapi/src/sdk/plugin.ts @@ -481,7 +481,7 @@ const normalizeOpenApiRefs = (node: unknown): unknown => { const toBinding = (def: ToolDefinition): OperationBinding => OperationBinding.make({ method: def.operation.method, - baseUrl: def.operation.baseUrl, + servers: def.operation.servers, pathTemplate: def.operation.pathTemplate, parameters: [...def.operation.parameters], requestBody: def.operation.requestBody, @@ -717,8 +717,10 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { // Defaults the add page derives from its preview, applied here so // headless callers (MCP, API) get the same integration the UI's // add flow would produce — see e2e/scenarios/connect-handoff.test.ts: - // - baseUrl: the spec's first server (else tools have no host to - // call and every invocation fails with "HTTP request failed") + // - effectiveBaseUrl: the spec's first server, used to anchor the + // derived auth template's absolute URLs. It is NOT stored as the + // connection baseUrl — the request host is resolved per call from + // the operation's extracted `servers`. // - authenticationTemplate: the spec's declared security schemes // (else the Add-connection modal is a dead "No authentication" // end with nowhere to paste a credential) @@ -762,7 +764,10 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { ...(specInputToGoogleBundle(config.spec) !== undefined ? { googleDiscoveryUrls: specInputToGoogleBundle(config.spec) } : {}), - ...(effectiveBaseUrl ? { baseUrl: effectiveBaseUrl } : {}), + // baseUrl is an optional override only. The host is otherwise + // resolved per call from the operation's `servers` (extracted from + // the spec), so we never bake a derived base URL into the config. + ...(config.baseUrl ? { baseUrl: config.baseUrl } : {}), ...(config.headers ? { headers: config.headers } : {}), ...(config.queryParams ? { queryParams: config.queryParams } : {}), // Prefer the caller's explicit template; otherwise adopt the one the diff --git a/packages/plugins/openapi/src/sdk/query-serialization.test.ts b/packages/plugins/openapi/src/sdk/query-serialization.test.ts index 07b6c6dd0..ed7f08b82 100644 --- a/packages/plugins/openapi/src/sdk/query-serialization.test.ts +++ b/packages/plugins/openapi/src/sdk/query-serialization.test.ts @@ -4,7 +4,7 @@ import { FetchHttpClient } from "effect/unstable/http"; import { createServer, type Server } from "node:http"; import { invokeWithLayer } from "./invoke"; -import { OperationBinding, OperationParameter } from "./types"; +import { OperationBinding, OperationParameter, ServerInfo } from "./types"; const withServer = ( f: (input: { readonly baseUrl: string; readonly requests: string[] }) => Promise, @@ -37,6 +37,7 @@ it.effect("serializes form-exploded query arrays as repeated parameters", () => withServer(async ({ baseUrl, requests }) => { const operation = OperationBinding.make({ method: "get", + servers: [], pathTemplate: "/messages/{id}", requestBody: Option.none(), parameters: [ @@ -101,7 +102,9 @@ it.effect("uses operation base URL and preserves reserved path expansion when al withServer(async ({ baseUrl, requests }) => { const operation = OperationBinding.make({ method: "get", - baseUrl, + servers: [ + ServerInfo.make({ url: baseUrl, description: Option.none(), variables: Option.none() }), + ], pathTemplate: "/v1/{+name}", requestBody: Option.none(), parameters: [ @@ -124,7 +127,8 @@ it.effect("uses operation base URL and preserves reserved path expansion when al { name: "spaces/AAA/messages/BBB", }, - "https://unused.example", + // No connection override → the request targets the operation's server. + "", {}, {}, FetchHttpClient.layer, @@ -136,3 +140,79 @@ it.effect("uses operation base URL and preserves reserved path expansion when al }), ), ); + +it.effect("targets the server chosen by the call's `server.url`", () => + Effect.promise(() => + withServer(async ({ baseUrl, requests }) => { + const operation = OperationBinding.make({ + method: "get", + // First server is a dead host; the call must select the live one. + servers: [ + ServerInfo.make({ + url: "https://unused.example", + description: Option.none(), + variables: Option.none(), + }), + ServerInfo.make({ url: baseUrl, description: Option.none(), variables: Option.none() }), + ], + pathTemplate: "/ping", + requestBody: Option.none(), + parameters: [], + }); + + await Effect.runPromise( + invokeWithLayer(operation, { server: { url: baseUrl } }, "", {}, {}, FetchHttpClient.layer), + ); + + const url = new URL(requests[0]!, "http://executor.test"); + expect(url.pathname).toBe("/ping"); + }), + ), +); + +it.effect("a connection base URL overrides the operation's servers", () => + Effect.promise(() => + withServer(async ({ baseUrl, requests }) => { + const operation = OperationBinding.make({ + method: "get", + servers: [ + ServerInfo.make({ + url: "https://unused.example", + description: Option.none(), + variables: Option.none(), + }), + ], + pathTemplate: "/ping", + requestBody: Option.none(), + parameters: [], + }); + + // The live server is the connection override; it wins over the spec server. + await Effect.runPromise( + invokeWithLayer(operation, {}, baseUrl, {}, {}, FetchHttpClient.layer), + ); + + expect(new URL(requests[0]!, "http://executor.test").pathname).toBe("/ping"); + }), + ), +); + +it.effect("falls back to the base URL for bindings persisted without servers", () => + Effect.promise(() => + withServer(async ({ baseUrl, requests }) => { + // Old binding shape: no `servers` field at all. + const operation = OperationBinding.make({ + method: "get", + pathTemplate: "/ping", + requestBody: Option.none(), + parameters: [], + }); + + await Effect.runPromise( + invokeWithLayer(operation, {}, baseUrl, {}, {}, FetchHttpClient.layer), + ); + + expect(new URL(requests[0]!, "http://executor.test").pathname).toBe("/ping"); + }), + ), +); diff --git a/packages/plugins/openapi/src/sdk/server-url-resolution.test.ts b/packages/plugins/openapi/src/sdk/server-url-resolution.test.ts new file mode 100644 index 000000000..7cbe7846f --- /dev/null +++ b/packages/plugins/openapi/src/sdk/server-url-resolution.test.ts @@ -0,0 +1,253 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Effect, Option } from "effect"; +import { FetchHttpClient } from "effect/unstable/http"; + +import { extract } from "./extract"; +import { resolveServerUrl } from "./openapi-utils"; +import { parse } from "./parse"; +import { previewSpec as previewSpecRaw } from "./preview"; +import { ServerVariable } from "./types"; + +const previewSpec = (input: string) => + previewSpecRaw(input).pipe(Effect.provide(FetchHttpClient.layer)); + +const serverVariable = (defaultValue: string): ServerVariable => + ServerVariable.make({ default: defaultValue, enum: Option.none(), description: Option.none() }); + +// --------------------------------------------------------------------------- +// resolveServerUrl — invoke-time template resolution +// --------------------------------------------------------------------------- + +describe("resolveServerUrl", () => { + const vars = { + tenant: serverVariable("default-tenant"), + region: serverVariable("us-east-1"), + }; + + it("fills placeholders from variable defaults when no override is given", () => { + expect(resolveServerUrl("https://{tenant}.{region}.api.example.com", vars, {})).toBe( + "https://default-tenant.us-east-1.api.example.com", + ); + }); + + it("prefers connection overrides over defaults", () => { + expect( + resolveServerUrl("https://{tenant}.{region}.api.example.com", vars, { + tenant: "acme", + region: "eu-west-1", + }), + ).toBe("https://acme.eu-west-1.api.example.com"); + }); + + it("ignores empty overrides and keeps the default", () => { + expect( + resolveServerUrl("https://{tenant}.{region}.api.example.com", vars, { tenant: "" }), + ).toBe("https://default-tenant.us-east-1.api.example.com"); + }); + + it("returns a URL with no placeholders unchanged", () => { + expect(resolveServerUrl("https://api.example.com", undefined, { tenant: "acme" })).toBe( + "https://api.example.com", + ); + }); +}); + +// --------------------------------------------------------------------------- +// Operation applicable servers — operation/path override else document servers +// --------------------------------------------------------------------------- + +// Raw JSON so the path-level `servers` override is easy to express. +const specWithPathOverride = { + openapi: "3.0.0", + info: { title: "Example", version: "1.0" }, + servers: [{ url: "https://api.example.com" }], + paths: { + "/items": { + get: { operationId: "listItems", responses: { "200": { description: "ok" } } }, + }, + "/query": { + servers: [ + { + url: "https://{tenant}.{region}.api.example.com", + variables: { tenant: { default: "default-tenant" }, region: { default: "us-east-1" } }, + }, + ], + post: { operationId: "runQuery", responses: { "200": { description: "ok" } } }, + }, + }, +}; + +describe("extract — operation applicable servers", () => { + it.effect("inherits the document servers when there is no override", () => + Effect.gen(function* () { + // @effect-diagnostics-next-line preferSchemaOverJson:off + const doc = yield* parse(JSON.stringify(specWithPathOverride)); + const result = yield* extract(doc); + + const listItems = result.operations.find((op) => op.operationId === "listItems")!; + expect(listItems.servers.map((s) => s.url)).toEqual(["https://api.example.com"]); + }), + ); + + it.effect("carries a path-level override's servers as templates with variables", () => + Effect.gen(function* () { + // @effect-diagnostics-next-line preferSchemaOverJson:off + const doc = yield* parse(JSON.stringify(specWithPathOverride)); + const result = yield* extract(doc); + + const runQuery = result.operations.find((op) => op.operationId === "runQuery")!; + expect(runQuery.servers.map((s) => s.url)).toEqual([ + "https://{tenant}.{region}.api.example.com", + ]); + const vars = Option.getOrThrow(runQuery.servers[0]!.variables); + expect(vars.tenant?.default).toBe("default-tenant"); + expect(vars.region?.default).toBe("us-east-1"); + + // The host resolves per call: defaults, or call-supplied overrides. + const server = runQuery.servers[0]!; + expect(resolveServerUrl(server.url, Option.getOrUndefined(server.variables), {})).toBe( + "https://default-tenant.us-east-1.api.example.com", + ); + expect( + resolveServerUrl(server.url, Option.getOrUndefined(server.variables), { + tenant: "acme", + region: "eu-west-1", + }), + ).toBe("https://acme.eu-west-1.api.example.com"); + }), + ); +}); + +// --------------------------------------------------------------------------- +// Preview surfaces each top-level server with its own variables +// --------------------------------------------------------------------------- + +describe("previewSpec — server variables", () => { + it.effect("carries variables per top-level server, excluding operation overrides", () => + Effect.gen(function* () { + const spec = { + openapi: "3.0.0", + info: { title: "Example", version: "1.0" }, + servers: [ + { url: "https://api.example.com", description: "Control plane" }, + { + url: "https://{branch}.{region}.example.com", + variables: { branch: { default: "main" }, region: { default: "us-east-1" } }, + }, + ], + paths: { + "/query": { + servers: [ + { + url: "https://{tenant}.gw.example.com", + variables: { tenant: { default: "default-tenant" } }, + }, + ], + post: { operationId: "runQuery", responses: { "200": { description: "ok" } } }, + }, + }, + }; + // @effect-diagnostics-next-line preferSchemaOverJson:off + const preview = yield* previewSpec(JSON.stringify(spec)); + + // The plain control-plane server has no variables. + const control = preview.servers.find((s) => s.url === "https://api.example.com")!; + expect(Option.getOrNull(control.variables)).toBeNull(); + + // The gateway server carries only its own branch/region. + const gateway = preview.servers.find((s) => s.url.startsWith("https://{branch}"))!; + expect(Object.keys(Option.getOrThrow(gateway.variables))).toEqual(["branch", "region"]); + + // The operation override's `tenant` never appears in the top-level servers. + const names = preview.servers.flatMap((s) => + Object.keys(Option.getOrElse(s.variables, () => ({}))), + ); + expect(names).not.toContain("tenant"); + }), + ); +}); + +// --------------------------------------------------------------------------- +// The `server` input property exposes per-call host selection + variables +// --------------------------------------------------------------------------- + +type ServerInputSchema = { + readonly required?: readonly string[]; + readonly properties: { + readonly server?: { + readonly properties: { + readonly url?: { readonly enum: readonly string[]; readonly default: string }; + readonly variables?: { + readonly properties: Record< + string, + { readonly default?: string; readonly enum?: readonly string[] } + >; + }; + }; + }; + }; +}; + +describe("buildInputSchema — server property", () => { + it.effect("exposes a server picker and variables for multiple/templated servers", () => + Effect.gen(function* () { + const spec = { + openapi: "3.0.0", + info: { title: "Example", version: "1.0" }, + servers: [ + { url: "https://api.example.com" }, + { + url: "https://{branch}.{region}.example.com", + variables: { + branch: { default: "main" }, + region: { default: "us", enum: ["us", "eu"] }, + }, + }, + ], + paths: { + "/items": { + get: { operationId: "listItems", responses: { "200": { description: "ok" } } }, + }, + }, + }; + // @effect-diagnostics-next-line preferSchemaOverJson:off + const result = yield* extract(yield* parse(JSON.stringify(spec))); + const op = result.operations.find((o) => o.operationId === "listItems")!; + const schema = Option.getOrThrow(op.inputSchema) as ServerInputSchema; + + expect(schema.properties.server?.properties.url?.enum).toEqual([ + "https://api.example.com", + "https://{branch}.{region}.example.com", + ]); + const vars = schema.properties.server?.properties.variables?.properties; + expect(vars?.branch?.default).toBe("main"); + expect(vars?.region?.enum).toEqual(["us", "eu"]); + // The host stays optional — defaults apply when the call omits `server`. + expect(schema.required ?? []).not.toContain("server"); + }), + ); + + it.effect("omits the server property for a single concrete server", () => + Effect.gen(function* () { + const spec = { + openapi: "3.0.0", + info: { title: "Example", version: "1.0" }, + servers: [{ url: "https://api.example.com" }], + paths: { + "/items/{id}": { + get: { + operationId: "getItem", + parameters: [{ name: "id", in: "path", required: true, schema: { type: "string" } }], + responses: { "200": { description: "ok" } }, + }, + }, + }, + }; + // @effect-diagnostics-next-line preferSchemaOverJson:off + const result = yield* extract(yield* parse(JSON.stringify(spec))); + const op = result.operations.find((o) => o.operationId === "getItem")!; + const schema = Option.getOrThrow(op.inputSchema) as ServerInputSchema; + expect(schema.properties.server).toBeUndefined(); + }), + ); +}); diff --git a/packages/plugins/openapi/src/sdk/types.ts b/packages/plugins/openapi/src/sdk/types.ts index 7008f73df..048ee6f8d 100644 --- a/packages/plugins/openapi/src/sdk/types.ts +++ b/packages/plugins/openapi/src/sdk/types.ts @@ -135,11 +135,25 @@ export const OperationRequestBody = Schema.Struct({ }); export type OperationRequestBody = typeof OperationRequestBody.Type; +export const ServerVariable = Schema.Struct({ + default: Schema.String, + enum: Schema.OptionFromOptional(Schema.Array(Schema.String)), + description: Schema.OptionFromOptional(Schema.String), +}); +export type ServerVariable = typeof ServerVariable.Type; + +export const ServerInfo = Schema.Struct({ + url: Schema.String, + description: Schema.OptionFromOptional(Schema.String), + variables: Schema.OptionFromOptional(Schema.Record(Schema.String, ServerVariable)), +}); +export type ServerInfo = typeof ServerInfo.Type; + export const ExtractedOperation = Schema.Struct({ operationId: OperationId, toolPath: Schema.OptionFromOptional(Schema.String), method: HttpMethod, - baseUrl: Schema.optional(Schema.String), + servers: Schema.Array(ServerInfo), pathTemplate: Schema.String, summary: Schema.OptionFromOptional(Schema.String), description: Schema.OptionFromOptional(Schema.String), @@ -152,20 +166,6 @@ export const ExtractedOperation = Schema.Struct({ }); export type ExtractedOperation = typeof ExtractedOperation.Type; -export const ServerVariable = Schema.Struct({ - default: Schema.String, - enum: Schema.OptionFromOptional(Schema.Array(Schema.String)), - description: Schema.OptionFromOptional(Schema.String), -}); -export type ServerVariable = typeof ServerVariable.Type; - -export const ServerInfo = Schema.Struct({ - url: Schema.String, - description: Schema.OptionFromOptional(Schema.String), - variables: Schema.OptionFromOptional(Schema.Record(Schema.String, ServerVariable)), -}); -export type ServerInfo = typeof ServerInfo.Type; - export const ExtractionResult = Schema.Struct({ title: Schema.OptionFromOptional(Schema.String), version: Schema.OptionFromOptional(Schema.String), @@ -180,7 +180,7 @@ export type ExtractionResult = typeof ExtractionResult.Type; export const OperationBinding = Schema.Struct({ method: HttpMethod, - baseUrl: Schema.optional(Schema.String), + servers: Schema.optional(Schema.Array(ServerInfo)), pathTemplate: Schema.String, parameters: Schema.Array(OperationParameter), requestBody: Schema.OptionFromOptional(OperationRequestBody),