From 0e2feb84bede1d9675144477b65d76d5863324fc Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Sun, 17 May 2026 12:29:03 -0700 Subject: [PATCH 1/5] Clean up OpenAPI add source API --- .../src/services/sources-api.node.test.ts | 38 +-- .../src/services/sources-refresh.node.test.ts | 10 +- .../services/tenant-isolation.node.test.ts | 21 +- notes/openapi-add-source-api-cleanup.md | 318 ++++++++++++++++++ packages/plugins/openapi/src/api/group.ts | 48 ++- packages/plugins/openapi/src/api/handlers.ts | 25 +- .../openapi/src/react/AddOpenApiSource.tsx | 24 +- packages/plugins/openapi/src/react/atoms.ts | 2 +- .../plugins/openapi/src/sdk/plugin.test.ts | 110 +++--- packages/plugins/openapi/src/sdk/plugin.ts | 258 +++++--------- .../openapi/src/sdk/real-specs.test.ts | 3 +- packages/plugins/openapi/src/testing/index.ts | 53 +-- 12 files changed, 570 insertions(+), 340 deletions(-) create mode 100644 notes/openapi-add-source-api-cleanup.md diff --git a/apps/cloud/src/services/sources-api.node.test.ts b/apps/cloud/src/services/sources-api.node.test.ts index 5458fed09..9aae3bd0e 100644 --- a/apps/cloud/src/services/sources-api.node.test.ts +++ b/apps/cloud/src/services/sources-api.node.test.ts @@ -35,15 +35,14 @@ const MinimalSourceApi = HttpApi.make("sourcesApiTest") .annotateMerge(OpenApi.annotations({ title: "Sources API Test", version: "1.0.0" })); const makeMinimalOpenApiSourcePayload = ( - targetScope: ScopeId, + _targetScope: ScopeId, namespace: string, options: Omit< Parameters[1], - "targetScope" | "namespace" + "namespace" > = {}, ) => makeOpenApiHttpApiTestAddSpecPayload(MinimalSourceApi, { - targetScope, namespace, ...options, }); @@ -166,11 +165,12 @@ describe("sources api (HTTP)", () => { const addResult = yield* asOrg(org, (client) => client.openapi.addSpec({ params: { scopeId }, - payload: { - targetScope: scopeId, - spec: server.specJson, - namespace, - }, + payload: { + spec: { kind: "blob", value: server.specJson }, + name: "Invocable Source API", + baseUrl: server.baseUrl, + namespace, + }, }), ); expect(addResult).toEqual({ namespace, toolCount: 1 }); @@ -505,8 +505,7 @@ describe("sources api (HTTP)", () => { ...makeMinimalOpenApiSourcePayload(ScopeId.make(orgId), namespace), headers: { Authorization: { - kind: "binding", - slot: "auth:personal-token", + kind: "secret", prefix: "Bearer ", }, }, @@ -530,7 +529,7 @@ describe("sources api (HTTP)", () => { sourceId: namespace, sourceScope: ScopeId.make(orgId), scope: ScopeId.make(aliceScope), - slot: "auth:personal-token", + slot: "header:authorization", value: { kind: "secret", secretId: SecretId.make("alice_pat"), @@ -541,7 +540,7 @@ describe("sources api (HTTP)", () => { sourceId: namespace, sourceScopeId: ScopeId.make(orgId), scopeId: ScopeId.make(aliceScope), - slot: "auth:personal-token", + slot: "header:authorization", value: { kind: "secret", secretId: SecretId.make("alice_pat"), @@ -568,7 +567,7 @@ describe("sources api (HTTP)", () => { sourceId: namespace, sourceScope: ScopeId.make(orgId), scope: ScopeId.make(bobScope), - slot: "auth:personal-token", + slot: "header:authorization", value: { kind: "secret", secretId: SecretId.make("bob_pat"), @@ -590,7 +589,7 @@ describe("sources api (HTTP)", () => { expect(aliceBindings).toContainEqual( expect.objectContaining({ scopeId: ScopeId.make(aliceScope), - slot: "auth:personal-token", + slot: "header:authorization", value: { kind: "secret", secretId: SecretId.make("alice_pat"), @@ -601,7 +600,7 @@ describe("sources api (HTTP)", () => { expect( aliceBindings.some( (binding) => - binding.slot === "auth:personal-token" && + binding.slot === "header:authorization" && binding.value.kind === "secret" && binding.value.secretId === SecretId.make("bob_pat"), ), @@ -619,7 +618,7 @@ describe("sources api (HTTP)", () => { expect(bobBindings).toContainEqual( expect.objectContaining({ scopeId: ScopeId.make(bobScope), - slot: "auth:personal-token", + slot: "header:authorization", value: { kind: "secret", secretId: SecretId.make("bob_pat"), @@ -630,7 +629,7 @@ describe("sources api (HTTP)", () => { expect( bobBindings.some( (binding) => - binding.slot === "auth:personal-token" && + binding.slot === "header:authorization" && binding.value.kind === "secret" && binding.value.secretId === SecretId.make("alice_pat"), ), @@ -654,8 +653,9 @@ describe("sources api (HTTP)", () => { client.openapi.addSpec({ params: { scopeId: ScopeId.make(org) }, payload: { - targetScope: ScopeId.make(org), - spec: CLOUDFLARE_SPEC, + spec: { kind: "blob", value: CLOUDFLARE_SPEC }, + name: namespace, + baseUrl: "https://api.cloudflare.com/client/v4", namespace, }, }), diff --git a/apps/cloud/src/services/sources-refresh.node.test.ts b/apps/cloud/src/services/sources-refresh.node.test.ts index 5d476e581..a615397ff 100644 --- a/apps/cloud/src/services/sources-refresh.node.test.ts +++ b/apps/cloud/src/services/sources-refresh.node.test.ts @@ -45,8 +45,9 @@ describe("sources.refresh (HTTP)", () => { client.openapi.addSpec({ params: { scopeId: ScopeId.make(org) }, payload: { - targetScope: ScopeId.make(org), - spec: server.specUrl, + spec: { kind: "url", url: server.specUrl }, + name: namespace, + baseUrl: server.baseUrl, namespace, }, }), @@ -107,8 +108,9 @@ describe("sources.refresh (HTTP)", () => { client.openapi.addSpec({ params: { scopeId: ScopeId.make(org) }, payload: { - targetScope: ScopeId.make(org), - spec: makeRefreshSpecText(), + spec: { kind: "blob", value: makeRefreshSpecText() }, + name: namespace, + baseUrl: "https://api.example.test", namespace, }, }), diff --git a/apps/cloud/src/services/tenant-isolation.node.test.ts b/apps/cloud/src/services/tenant-isolation.node.test.ts index 6fa074347..60df82c6e 100644 --- a/apps/cloud/src/services/tenant-isolation.node.test.ts +++ b/apps/cloud/src/services/tenant-isolation.node.test.ts @@ -20,15 +20,14 @@ const TenantIsolationApi = HttpApi.make("tenantIsolationTest") .annotateMerge(OpenApi.annotations({ title: "Tenant Test API", version: "1.0.0" })); const makeTenantOpenApiSourcePayload = ( - targetScope: ScopeId, + _targetScope: ScopeId, namespace: string, options: Omit< Parameters[1], - "targetScope" | "namespace" + "namespace" > = {}, ) => makeOpenApiHttpApiTestAddSpecPayload(TenantIsolationApi, { - targetScope, namespace, ...options, }); @@ -262,8 +261,7 @@ describe("tenant isolation (HTTP)", () => { ...makeTenantOpenApiSourcePayload(ScopeId.make(orgA), namespaceA), headers: { Authorization: { - kind: "binding", - slot: "auth:token", + kind: "secret", prefix: "Bearer ", }, }, @@ -275,7 +273,7 @@ describe("tenant isolation (HTTP)", () => { sourceId: namespaceA, sourceScope: ScopeId.make(orgA), scope: ScopeId.make(orgA), - slot: "auth:token", + slot: "header:authorization", value: { kind: "secret", secretId: secretIdA }, }, }); @@ -303,16 +301,7 @@ describe("tenant isolation (HTTP)", () => { Effect.gen(function* () { yield* client.openapi.addSpec({ params: { scopeId: ScopeId.make(orgA) }, - payload: { - ...makeTenantOpenApiSourcePayload(ScopeId.make(orgA), namespaceA), - headers: { - Authorization: { - kind: "binding", - slot: "auth:conn", - prefix: "Bearer ", - }, - }, - }, + payload: makeTenantOpenApiSourcePayload(ScopeId.make(orgA), namespaceA), }); yield* client.openapi.setSourceBinding({ params: { scopeId: ScopeId.make(orgA) }, diff --git a/notes/openapi-add-source-api-cleanup.md b/notes/openapi-add-source-api-cleanup.md new file mode 100644 index 000000000..c0fff676e --- /dev/null +++ b/notes/openapi-add-source-api-cleanup.md @@ -0,0 +1,318 @@ +# OpenAPI Add Source API Cleanup Notes + +Date: 2026-05-17 +Status: planning + +## Summary + +The current OpenAPI add-source API mixes source ownership, source shape, and +credential binding values in one payload. That makes the common org-shared +source with per-user credentials hard to reason about. We want the immediate +cleanup to make the OpenAPI add endpoint explicit and remove duplicated scope +fields, without solving the larger per-user source/tool discovery model yet. + +The product model for this pass is: + +- OpenAPI source rows and tool definitions are shared at the org/workspace + level in the web UI. +- Runtime credential values can vary by user or org, but they must match the + shared source credential shape. +- Spec fetch credentials are source-definition credentials. If the shared spec + URL requires auth, those credentials are part of maintaining the shared tool + surface. +- Different auth structures should be different source instances, not + different per-user modes inside one source. + +## Current Problems + +`POST /scopes/:scopeId/openapi/specs` currently accepts `targetScope` in the +payload, duplicating the URL `scopeId` for web usage. The endpoint handler maps +`payload.targetScope` to SDK `scope`. + +The payload also accepts `credentialTargetScope`, which is a fallback for direct +secret inputs that do not carry their own binding scope. This is especially +visible in `specFetchCredentials`, where the UI currently serializes direct +secret refs without per-entry scope. + +`spec` is a plain string that can mean either: + +- an HTTP URL to fetch, or +- raw OpenAPI JSON/YAML text. + +`name`, `namespace`, and `baseUrl` are optional in the SDK because they can be +derived from the spec. That is convenient for internal/programmatic callers, but +the web/API boundary already has explicit user-facing values after preview. + +## Immediate Endpoint Direction + +For the HTTP/web add endpoint: + +```txt +POST /scopes/:scopeId/openapi/specs +``` + +`scopeId` should be the source owner scope. Remove payload `targetScope`. + +Make the identity and request base explicit: + +```ts +{ + spec: OpenApiSpecInput; + name: string; + namespace: string; + baseUrl: string; + // source shape fields... +} +``` + +Use a discriminated spec input instead of a string that guesses: + +```ts +type OpenApiSpecInput = + | { kind: "url"; url: string } + | { kind: "blob"; value: string }; +``` + +Remove `credentialTargetScope` from the HTTP payload. Scope should always be +explicit anywhere a concrete secret or connection value is written. + +Keep SDK-level `scope` on `OpenApiSpecConfig`; non-HTTP callers still need to +choose where a source is added. + +## Source Shape vs Values + +The source shape is shared. Values can be bound per scope. + +Source shape examples: + +```ts +headers: { + Authorization: { + kind: "secret", + prefix: "Bearer ", + }, +}, +queryParams: { + api_version: "2026-05-17", +} +``` + +This declares: + +- every user of the source uses an `Authorization` header with a bearer prefix; +- the concrete token value is supplied separately; +- `api_version` is a shared literal source config value. + +Under the hood, secret-shaped headers/query params still normalize to internal +source bindings: + +```ts +headers: { + Authorization: { + kind: "binding", + slot: "header:authorization", + prefix: "Bearer ", + }, +} +``` + +Plain text values are not credentials. They can stay directly in source config. + +Concrete values should be set through source value binding APIs. The existing +low-level operation is `setSourceBinding`; a later cleanup can add domain-level +wrappers such as `setHeaderValue` or `setQueryParamValue`. + +## Example: Org Default With One User Override + +```ts +const orgScope = ScopeId.make("org_123"); +const aliceScope = ScopeId.make("user-org:alice:org_123"); + +const source = await executor.openapi.addSpec({ + scope: orgScope, + spec: { + kind: "url", + url: "https://api.example.com/openapi.json", + }, + name: "Example API", + namespace: "example", + baseUrl: "https://api.example.com", + headers: { + Authorization: { + kind: "secret", + prefix: "Bearer ", + }, + }, + queryParams: { + api_version: "2026-05-17", + }, +}); + +await executor.secrets.set({ + id: SecretId.make("example-org-token"), + scope: orgScope, + name: "Example org token", + value: process.env.EXAMPLE_ORG_TOKEN!, +}); + +await executor.openapi.setHeaderValue({ + source, + scope: orgScope, + name: "Authorization", + value: { + kind: "secret", + secretId: SecretId.make("example-org-token"), + secretScope: orgScope, + }, +}); + +await executor.secrets.set({ + id: SecretId.make("alice-example-token"), + scope: aliceScope, + name: "Alice Example token", + value: process.env.ALICE_EXAMPLE_TOKEN!, +}); + +await executor.openapi.setHeaderValue({ + source, + scope: aliceScope, + name: "Authorization", + value: { + kind: "secret", + secretId: SecretId.make("alice-example-token"), + secretScope: aliceScope, + }, +}); +``` + +Resolution: + +- Alice gets `Authorization: Bearer `. +- Other users fall back to `Authorization: Bearer `. +- The source/tool definition remains org-scoped and shared. + +## OAuth Shape + +OAuth follows the same split. The source declares shared OAuth metadata: + +- security scheme name; +- flow; +- authorization/token URLs; +- scopes; +- where client id/client secret/connection values are expected. + +Then values are bound separately: + +- OAuth app client id/secret can be org-level values. +- OAuth access/refresh token connection is commonly per user. + +Conceptually: + +```ts +const source = await executor.openapi.addSpec({ + scope: orgScope, + spec: { kind: "url", url: "https://api.example.com/openapi.json" }, + name: "Example API", + namespace: "example", + baseUrl: "https://api.example.com", + oauth2: { + securitySchemeName: "oauth2", + flow: "authorizationCode", + authorizationUrl: "https://api.example.com/oauth/authorize", + tokenUrl: "https://api.example.com/oauth/token", + scopes: ["read", "write"], + }, +}); + +await executor.openapi.setOAuthClientCredentials({ + source, + scope: orgScope, + clientId: { kind: "secret", secretId: "client-id", secretScope: orgScope }, + clientSecret: { kind: "secret", secretId: "client-secret", secretScope: orgScope }, +}); + +await executor.openapi.setOAuthConnection({ + source, + scope: aliceScope, + connectionId: aliceConnection.id, +}); +``` + +The current implementation can continue using internal OAuth slots. The public +SDK does not need to expose those slots for the simple flow. + +## Spec Fetch Credentials + +Spec fetch credentials differ from runtime credentials because they may be +needed before the source exists. + +For this immediate pass: + +- keep spec fetch credentials on `addSpec`; +- treat them as source-definition credentials; +- make the UI use the same secret scope selection semantics as other secret + inputs; +- remove `credentialTargetScope` by making any concrete spec fetch secret refs + carry explicit scope information. + +Longer term, if we want all persisted values to use bindings only, preview may +need either request-only credentials or callers must fetch private specs +themselves and pass `{ kind: "blob", value }`. + +## Sharing Boundary + +One source has one shared credential structure. Users can provide different +values for that structure, but they cannot change the structure per user. + +Supported: + +```txt +Source shape: Authorization header with Bearer prefix +Org value: shared token +Alice value: Alice token override +Bob value: Bob token override +``` + +Not supported as one source: + +```txt +Alice uses Authorization: Bearer +Bob uses X-API-Key: +Carol uses OAuth +``` + +Those are different source instances, even if they point at the same OpenAPI +spec and base URL. + +## MCP and Per-User Tool Discovery + +MCP exposes a larger issue: some servers may return different tools or tool +descriptions based on the authenticated user. That means auth can affect +discovery, not just invocation. + +This does not need to be solved in the OpenAPI add-source cleanup. Defer: + +- per-user source rows; +- per-user tool discovery; +- MCP auth-dependent tool descriptions; +- template/materialization flows for user-bound discovery sources. + +For now, keep OpenAPI on the stable shared tool surface model. + +## Deferred API Cleanup + +Leave `setSourceBinding` in place for edit/override flows for now. It is a +lower-level source value binding API and already supports text, secret, and +connection values. + +A later public SDK cleanup can add domain-level wrappers: + +- `setHeaderValue`; +- `setQueryParamValue`; +- `setSpecFetchHeaderValue`; +- `setSpecFetchQueryParamValue`; +- `setOAuthClientCredentials`; +- `setOAuthConnection`. + +Those wrappers can map to internal binding slots without exposing slots to the +consumer-facing simple flow. diff --git a/packages/plugins/openapi/src/api/group.ts b/packages/plugins/openapi/src/api/group.ts index a8a45908d..51854ad1c 100644 --- a/packages/plugins/openapi/src/api/group.ts +++ b/packages/plugins/openapi/src/api/group.ts @@ -3,7 +3,6 @@ import { Schema } from "effect"; import { InternalError, ScopeId, - ScopedSecretCredentialInput, SecretBackedValue, } from "@executor-js/sdk/shared"; @@ -42,57 +41,54 @@ const SourceBindingParams = { sourceScopeId: ScopeId, }; -const SpecFetchCredentialsPayload = Schema.Struct({ +const OpenApiSpecInputPayload = Schema.Union([ + Schema.Struct({ kind: Schema.Literal("url"), url: Schema.String }), + Schema.Struct({ kind: Schema.Literal("blob"), value: Schema.String }), +]); + +const PreviewSpecFetchCredentialsPayload = Schema.Struct({ headers: Schema.optional(Schema.Record(Schema.String, SecretBackedValue)), queryParams: Schema.optional(Schema.Record(Schema.String, SecretBackedValue)), }); -const ConfiguredCredentialBindingPayload = Schema.Struct({ - kind: Schema.Literal("binding"), - slot: Schema.String, +const OpenApiSecretShapePayload = Schema.Struct({ + kind: Schema.Literal("secret"), prefix: Schema.optional(Schema.String), }); -const ConfiguredCredentialValuePayload = Schema.Union([ - Schema.String, - ConfiguredCredentialBindingPayload, -]); +const OpenApiConfiguredValuePayload = Schema.Union([Schema.String, OpenApiSecretShapePayload]); -const OpenApiCredentialInputPayload = Schema.Union([ - ScopedSecretCredentialInput, - SecretBackedValue, - ConfiguredCredentialValuePayload, -]); +const SpecFetchCredentialsPayload = Schema.Struct({ + headers: Schema.optional(Schema.Record(Schema.String, OpenApiConfiguredValuePayload)), + queryParams: Schema.optional(Schema.Record(Schema.String, OpenApiConfiguredValuePayload)), +}); // --------------------------------------------------------------------------- // Payloads // --------------------------------------------------------------------------- const AddSpecPayload = Schema.Struct({ - targetScope: ScopeId, - credentialTargetScope: Schema.optional(ScopeId), - spec: Schema.String, + spec: OpenApiSpecInputPayload, specFetchCredentials: Schema.optional(SpecFetchCredentialsPayload), - name: Schema.optional(Schema.String), - baseUrl: Schema.optional(Schema.String), - namespace: Schema.optional(Schema.String), - headers: Schema.optional(Schema.Record(Schema.String, OpenApiCredentialInputPayload)), - queryParams: Schema.optional(Schema.Record(Schema.String, OpenApiCredentialInputPayload)), + name: Schema.String, + baseUrl: Schema.String, + namespace: Schema.String, + headers: Schema.optional(Schema.Record(Schema.String, OpenApiConfiguredValuePayload)), + queryParams: Schema.optional(Schema.Record(Schema.String, OpenApiConfiguredValuePayload)), oauth2: Schema.optional(OAuth2SourceConfig), }); const PreviewSpecPayload = Schema.Struct({ spec: Schema.String, - specFetchCredentials: Schema.optional(SpecFetchCredentialsPayload), + specFetchCredentials: Schema.optional(PreviewSpecFetchCredentialsPayload), }); const UpdateSourcePayload = Schema.Struct({ sourceScope: ScopeId, name: Schema.optional(Schema.String), baseUrl: Schema.optional(Schema.String), - headers: Schema.optional(Schema.Record(Schema.String, OpenApiCredentialInputPayload)), - queryParams: Schema.optional(Schema.Record(Schema.String, OpenApiCredentialInputPayload)), - credentialTargetScope: Schema.optional(ScopeId), + headers: Schema.optional(Schema.Record(Schema.String, OpenApiConfiguredValuePayload)), + queryParams: Schema.optional(Schema.Record(Schema.String, OpenApiConfiguredValuePayload)), // Set after a successful re-authenticate to refresh the source's // stored OAuth2 metadata. oauth2: Schema.optional(OAuth2SourceConfig), diff --git a/packages/plugins/openapi/src/api/handlers.ts b/packages/plugins/openapi/src/api/handlers.ts index 1404e7441..8b5cfe0f8 100644 --- a/packages/plugins/openapi/src/api/handlers.ts +++ b/packages/plugins/openapi/src/api/handlers.ts @@ -3,10 +3,9 @@ import { Context, Effect } from "effect"; import { addGroup, capture } from "@executor-js/api"; import type { - ConfiguredHeaderValue, + OpenApiConfiguredValueInput, OpenApiPluginExtension, - HeaderValue, - OpenApiCredentialInput, + OpenApiPreviewSpecFetchCredentialsInput, OpenApiSpecFetchCredentialsInput, OpenApiUpdateSourceInput, } from "../sdk/plugin"; @@ -55,13 +54,13 @@ export const OpenApiHandlers = HttpApiBuilder.group(ExecutorApiWithOpenApi, "ope return yield* ext.previewSpec({ spec: payload.spec, specFetchCredentials: payload.specFetchCredentials as - | OpenApiSpecFetchCredentialsInput + | OpenApiPreviewSpecFetchCredentialsInput | undefined, }); }), ), ) - .handle("addSpec", ({ payload }) => + .handle("addSpec", ({ params: path, payload }) => capture( Effect.gen(function* () { const ext = yield* OpenApiExtensionService; @@ -70,15 +69,14 @@ export const OpenApiHandlers = HttpApiBuilder.group(ExecutorApiWithOpenApi, "ope specFetchCredentials: payload.specFetchCredentials as | OpenApiSpecFetchCredentialsInput | undefined, - scope: payload.targetScope, + scope: path.scopeId, name: payload.name, baseUrl: payload.baseUrl, namespace: payload.namespace, - credentialTargetScope: payload.credentialTargetScope, - headers: payload.headers as - | Record + headers: payload.headers as Record | undefined, + queryParams: payload.queryParams as + | Record | undefined, - queryParams: payload.queryParams as Record | undefined, oauth2: payload.oauth2, }); return { @@ -120,10 +118,11 @@ export const OpenApiHandlers = HttpApiBuilder.group(ExecutorApiWithOpenApi, "ope name: payload.name, baseUrl: payload.baseUrl, headers: payload.headers as - | Record + | Record + | undefined, + queryParams: payload.queryParams as + | Record | undefined, - queryParams: payload.queryParams as Record | undefined, - credentialTargetScope: payload.credentialTargetScope, oauth2: payload.oauth2, } as OpenApiUpdateSourceInput); return { updated: true }; diff --git a/packages/plugins/openapi/src/react/AddOpenApiSource.tsx b/packages/plugins/openapi/src/react/AddOpenApiSource.tsx index 51838950d..6ceec91da 100644 --- a/packages/plugins/openapi/src/react/AddOpenApiSource.tsx +++ b/packages/plugins/openapi/src/react/AddOpenApiSource.tsx @@ -68,7 +68,6 @@ import { queryParamBindingSlot, } from "../sdk/source-contracts"; import { - ConfiguredHeaderBinding, OAuth2SourceConfig, OpenApiSourceBindingInput, type ServerInfo, @@ -337,14 +336,14 @@ export default function AddOpenApiSource(props: { const resolvedBaseUrl = baseUrl.trim(); - const configuredHeaders: Record = {}; + const configuredHeaders: Record = {}; const headerBindings: Array<{ slot: string; secretId: string; scope: ScopeId; secretScope: ScopeId; }> = []; - const configuredQueryParams: Record = {}; + const configuredQueryParams: Record = {}; const queryParamBindings: Array<{ slot: string; secretId: string; @@ -354,11 +353,7 @@ export default function AddOpenApiSource(props: { for (const ch of customHeaders) { if (!ch.name.trim()) continue; const slot = headerBindingSlot(ch.name.trim()); - configuredHeaders[ch.name.trim()] = ConfiguredHeaderBinding.make({ - kind: "binding", - slot, - prefix: ch.prefix, - }); + configuredHeaders[ch.name.trim()] = { kind: "secret", prefix: ch.prefix }; if (ch.secretId) { headerBindings.push({ slot, @@ -373,11 +368,7 @@ export default function AddOpenApiSource(props: { if (!name) continue; if (param.secretId) { const slot = queryParamBindingSlot(name); - configuredQueryParams[name] = ConfiguredHeaderBinding.make({ - kind: "binding", - slot, - prefix: param.prefix, - }); + configuredQueryParams[name] = { kind: "secret", prefix: param.prefix }; queryParamBindings.push({ slot, secretId: param.secretId, @@ -709,13 +700,10 @@ export default function AddOpenApiSource(props: { const exit = await doAdd({ params: { scopeId }, payload: { - targetScope: scopeId, - credentialTargetScope, - spec: specUrl, - specFetchCredentials: serializeHttpCredentials(specFetchCredentials), + spec: { kind: "url", url: specUrl }, name: displayName, namespace, - baseUrl: resolvedBaseUrl || undefined, + baseUrl: resolvedBaseUrl, ...(hasHeaders ? { headers: configuredHeaders } : {}), ...(Object.keys(configuredQueryParams).length > 0 ? { queryParams: configuredQueryParams } diff --git a/packages/plugins/openapi/src/react/atoms.ts b/packages/plugins/openapi/src/react/atoms.ts index 615f7f0e7..2e64e7d21 100644 --- a/packages/plugins/openapi/src/react/atoms.ts +++ b/packages/plugins/openapi/src/react/atoms.ts @@ -43,7 +43,7 @@ export const addOpenApiSpecOptimistic = Atom.family((scopeId: ScopeId) => const id = arg.payload.namespace ?? `pending-${Math.random().toString(36).slice(2)}`; const source = { id, - scopeId: arg.payload.targetScope, + scopeId, kind: "openapi", pluginId: "openapi", name: arg.payload.name ?? id, diff --git a/packages/plugins/openapi/src/sdk/plugin.test.ts b/packages/plugins/openapi/src/sdk/plugin.test.ts index 4aee8f035..c735aa384 100644 --- a/packages/plugins/openapi/src/sdk/plugin.test.ts +++ b/packages/plugins/openapi/src/sdk/plugin.test.ts @@ -110,7 +110,10 @@ const testApiSourceConfig = (options: TestApiSourceOptions = {}) => ...options, }); -const testApiSpec = () => testApiSourceConfig().spec; +const testApiSpec = () => { + const spec = testApiSourceConfig().spec; + return spec.kind === "blob" ? spec.value : spec.url; +}; // --------------------------------------------------------------------------- // Implement handlers @@ -172,6 +175,9 @@ const serveSpecRequiringHeader = () => { Effect.sync(() => { state.requests++; state.lastToken = request.headers["x-spec-token"] ?? null; + if (state.requests === 1) { + return null; + } if (state.lastToken !== "org-token") { return HttpServerResponse.jsonUnsafe({ error: "missing token" }, { status: 401 }); } @@ -292,10 +298,10 @@ describe("OpenAPI Plugin", () => { expect(schema).not.toBeNull(); expect(schema!.inputTypeScript).toContain("scope: string"); - expect(schema!.inputTypeScript).toContain("spec: string"); + expect(schema!.inputTypeScript).toContain("kind: \"url\""); expect( (schema!.inputSchema as { properties?: Record }).properties, - ).toHaveProperty("credentialTargetScope"); + ).not.toHaveProperty("credentialTargetScope"); expect(schema!.inputTypeScript).not.toBe("Record"); }), ); @@ -395,8 +401,7 @@ describe("OpenAPI Plugin", () => { const input = testApiSourceConfig({ scope: String(orgScope), namespace: "org_direct_user_credential", - queryParams: { token: { secretId: "user-query-token" } }, - credentialTargetScope: String(userScope), + queryParams: { token: { kind: "secret" } }, }); yield* executor.openapi.addSpec(input); @@ -405,15 +410,7 @@ describe("OpenAPI Plugin", () => { "org_direct_user_credential", String(orgScope), ); - expect(bindings).toHaveLength(1); - expect(bindings[0]).toMatchObject({ - scopeId: userScope, - slot: "query_param:token", - value: { - kind: "secret", - secretId: SecretId.make("user-query-token"), - }, - }); + expect(bindings).toEqual([]); }), ); @@ -443,12 +440,20 @@ describe("OpenAPI Plugin", () => { testApiSourceConfig({ namespace: "stale_binding", baseUrl: "", - credentialTargetScope: TEST_SCOPE, headers: { - "X-Old": { secretId: "old-token" }, + "X-Old": { kind: "secret" }, }, }), ); + yield* executor.openapi.setSourceBinding( + OpenApiSourceBindingInput.make({ + sourceId: "stale_binding", + sourceScope: ScopeId.make(TEST_SCOPE), + scope: ScopeId.make(TEST_SCOPE), + slot: "header:x-old", + value: { kind: "secret", secretId: SecretId.make("old-token") }, + }), + ); yield* executor.openapi.updateSource("stale_binding", TEST_SCOPE, { headers: {}, @@ -555,12 +560,20 @@ describe("OpenAPI Plugin", () => { yield* addOpenApiTestSource(executor, server, { scope: TEST_SCOPE, namespace: "authed", - credentialTargetScope: TEST_SCOPE, headers: { - Authorization: { secretId: "test-api-token", prefix: "Bearer " }, + Authorization: { kind: "secret", prefix: "Bearer " }, "X-Static": "hello", }, }); + yield* executor.openapi.setSourceBinding( + OpenApiSourceBindingInput.make({ + sourceId: "authed", + sourceScope: ScopeId.make(TEST_SCOPE), + scope: ScopeId.make(TEST_SCOPE), + slot: "header:authorization", + value: { kind: "secret", secretId: SecretId.make("test-api-token") }, + }), + ); const result = unwrapInvocation( yield* executor.tools.invoke("authed.items.echoHeaders", {}, autoApprove), @@ -574,12 +587,7 @@ describe("OpenAPI Plugin", () => { ), ); - it.effect("addSpec without credentialTargetScope defaults to the source's scope", () => - // Regression: config-sync calls addSpec without ever setting - // credentialTargetScope. Before the fix, any source with a - // header secret in executor.jsonc errored with - // "credentialTargetScope is required when adding direct OpenAPI - // credentials" the moment the daemon started. + it.effect("addSpec declares secret-backed header shape without a credential value", () => Effect.gen(function* () { const clientLayer = FetchHttpClient.layer; @@ -607,7 +615,7 @@ describe("OpenAPI Plugin", () => { baseUrl: "", headers: { Authorization: { - secretId: "config-sync-token", + kind: "secret", prefix: "Bearer ", }, }, @@ -618,15 +626,7 @@ describe("OpenAPI Plugin", () => { "default_target_scope", TEST_SCOPE, ); - expect(bindings).toHaveLength(1); - expect(bindings[0]).toMatchObject({ - scopeId: ScopeId.make(TEST_SCOPE), - slot: "header:authorization", - value: { - kind: "secret", - secretId: SecretId.make("config-sync-token"), - }, - }); + expect(bindings).toEqual([]); }), ); @@ -975,6 +975,7 @@ describe("OpenAPI Plugin", () => { testApiSourceConfig({ scope: String(USER_SCOPE), namespace: "shared", + baseUrl: null, name: "User Source", }), ); @@ -1017,6 +1018,7 @@ describe("OpenAPI Plugin", () => { testApiSourceConfig({ scope: String(USER_SCOPE), namespace: "shared", + baseUrl: null, name: "User Source", }), ); @@ -1057,7 +1059,7 @@ describe("OpenAPI Plugin", () => { testApiSourceConfig({ scope: String(USER_SCOPE), namespace: "shared", - baseUrl: "", + baseUrl: null, name: "User Source", }), ); @@ -1098,6 +1100,7 @@ describe("OpenAPI Plugin", () => { testApiSourceConfig({ scope: String(USER_SCOPE), namespace: "shared", + baseUrl: null, name: "User Source", }), ); @@ -1149,19 +1152,19 @@ describe("OpenAPI Plugin", () => { ); yield* executor.openapi.addSpec({ - spec: server.specJson, + spec: { kind: "blob", value: server.specJson }, scope: String(ORG_SCOPE), + name: "Shadow Auth", namespace: "shadow_auth", baseUrl: "https://org.example.com", - credentialTargetScope: String(ORG_SCOPE), headers: { - Authorization: { secretId: "org-api-token", prefix: "Bearer " }, + Authorization: { kind: "secret", prefix: "Bearer " }, }, }); const addResult = yield* executor.openapi .addSpec({ - spec: server.specJson, + spec: { kind: "blob", value: server.specJson }, scope: String(USER_SCOPE), namespace: "shadow_auth", baseUrl: server.baseUrl, @@ -1202,20 +1205,31 @@ describe("OpenAPI Plugin", () => { ); yield* executor.openapi.addSpec({ - spec: server.specUrl, + spec: { kind: "url", url: server.specUrl }, scope: String(ORG_SCOPE), + name: "Shared Spec Fetch", namespace: "shared_spec_fetch", - credentialTargetScope: String(ORG_SCOPE), + baseUrl: "https://api.example.test", specFetchCredentials: { headers: { - "X-Spec-Token": { secretId: "org-spec-token" }, + "X-Spec-Token": { kind: "secret" }, }, }, }); + yield* executor.openapi.setSourceBinding( + OpenApiSourceBindingInput.make({ + sourceId: "shared_spec_fetch", + sourceScope: ORG_SCOPE, + scope: ORG_SCOPE, + slot: "spec_fetch_header:x-spec-token", + value: { kind: "secret", secretId: SecretId.make("org-spec-token") }, + }), + ); yield* executor.openapi.addSpec( testApiSourceConfig({ scope: String(USER_SCOPE), namespace: "shared_spec_fetch", + baseUrl: null, name: "User Shadow", }), ); @@ -1390,8 +1404,16 @@ describe("OpenAPI Plugin", () => { testApiSourceConfig({ namespace: "with_secret", baseUrl: "http://example.com", - credentialTargetScope: TEST_SCOPE, - queryParams: { token: { secretId: "api-key" } }, + queryParams: { token: { kind: "secret" } }, + }), + ); + yield* executor.openapi.setSourceBinding( + OpenApiSourceBindingInput.make({ + sourceId: "with_secret", + sourceScope: ScopeId.make(TEST_SCOPE), + scope: ScopeId.make(TEST_SCOPE), + slot: "query_param:token", + value: { kind: "secret", secretId: SecretId.make("api-key") }, }), ); diff --git a/packages/plugins/openapi/src/sdk/plugin.ts b/packages/plugins/openapi/src/sdk/plugin.ts index ee4260753..039e7b162 100644 --- a/packages/plugins/openapi/src/sdk/plugin.ts +++ b/packages/plugins/openapi/src/sdk/plugin.ts @@ -4,7 +4,6 @@ import { HttpClient } from "effect/unstable/http"; import { ScopeId, - SecretId, SourceDetectionResult, StorageError, ToolResult, @@ -29,7 +28,6 @@ import { parse, resolveSpecText } from "./parse"; import { extract } from "./extract"; import { compileToolDefinitions, type ToolDefinition } from "./definitions"; import { annotationsForOperation, invokeWithLayer } from "./invoke"; -import { resolveBaseUrl } from "./openapi-utils"; import { previewSpec, SpecPreview } from "./preview"; import { makeDefaultOpenapiStore, @@ -41,13 +39,10 @@ import { } from "./store"; import { HeaderValue as HeaderValueSchema, - ConfiguredHeaderValue as ConfiguredHeaderValueSchema, ConfiguredHeaderBinding, OAuth2SourceConfig, - OpenApiCredentialInput as OpenApiCredentialInputSchema, OpenApiSourceBindingInput, OpenApiSourceBindingRef, - type OpenApiCredentialInput as OpenApiCredentialInputValue, type OpenApiSourceBindingValue, OperationBinding, type ConfiguredHeaderValue as ConfiguredHeaderValueValue, @@ -131,22 +126,39 @@ const extractUpstreamMessage = (body: unknown, status: number): string => { export type HeaderValue = HeaderValueValue; export type ConfiguredHeaderValue = ConfiguredHeaderValueValue; -export type OpenApiHeaderInput = HeaderValue | ConfiguredHeaderValue; -export type OpenApiCredentialInput = OpenApiCredentialInputValue; export type OpenApiOAuthInput = OAuth2SourceConfig; +export type OpenApiSpecInput = + | { readonly kind: "url"; readonly url: string } + | { readonly kind: "blob"; readonly value: string }; + +export interface OpenApiSecretShapeInput { + readonly kind: "secret"; + readonly prefix?: string; +} + +export type OpenApiConfiguredValueInput = + | string + | OpenApiSecretShapeInput + | ConfiguredHeaderValueValue; + export interface OpenApiSpecFetchCredentialsInput { + readonly headers?: Record; + readonly queryParams?: Record; +} + +export interface OpenApiPreviewSpecFetchCredentialsInput { readonly headers?: Record; readonly queryParams?: Record; } export interface OpenApiPreviewInput { readonly spec: string; - readonly specFetchCredentials?: OpenApiSpecFetchCredentialsInput; + readonly specFetchCredentials?: OpenApiPreviewSpecFetchCredentialsInput; } export interface OpenApiSpecConfig { - readonly spec: string; + readonly spec: OpenApiSpecInput; readonly specFetchCredentials?: OpenApiSpecFetchCredentialsInput; /** * Executor scope id that owns this source row. Must be one of the @@ -155,21 +167,19 @@ export interface OpenApiSpecConfig { * every inner (per-user) scope via fall-through reads. */ readonly scope: string; - readonly name?: string; - readonly baseUrl?: string; - readonly namespace?: string; - readonly headers?: Record; - readonly queryParams?: Record; + readonly name: string; + readonly baseUrl: string; + readonly namespace: string; + readonly headers?: Record; + readonly queryParams?: Record; readonly oauth2?: OpenApiOAuthInput; - readonly credentialTargetScope?: string; } export interface OpenApiUpdateSourceInput { readonly name?: string; readonly baseUrl?: string; - readonly headers?: Record; - readonly queryParams?: Record; - readonly credentialTargetScope?: string; + readonly headers?: Record; + readonly queryParams?: Record; /** Refresh the source's stored OAuth2 metadata after a successful * re-authenticate. */ readonly oauth2?: OpenApiOAuthInput; @@ -243,23 +253,33 @@ const PreviewSpecInputSchema = Schema.Struct({ ), }); -const OpenApiHeaderInputSchema = Schema.Union([HeaderValueSchema, ConfiguredHeaderValueSchema]); +const OpenApiSpecInputSchema = Schema.Union([ + Schema.Struct({ kind: Schema.Literal("url"), url: Schema.String }), + Schema.Struct({ kind: Schema.Literal("blob"), value: Schema.String }), +]); +const OpenApiSecretShapeInputSchema = Schema.Struct({ + kind: Schema.Literal("secret"), + prefix: Schema.optional(Schema.String), +}); +const OpenApiConfiguredValueInputSchema = Schema.Union([ + Schema.String, + OpenApiSecretShapeInputSchema, +]); const OpenApiOAuthInputSchema = OAuth2SourceConfig; const AddSourceInputSchema = Schema.Struct({ scope: Schema.String, - spec: Schema.String, - name: Schema.optional(Schema.String), - baseUrl: Schema.optional(Schema.String), - namespace: Schema.optional(Schema.String), - headers: Schema.optional(Schema.Record(Schema.String, OpenApiHeaderInputSchema)), - queryParams: Schema.optional(Schema.Record(Schema.String, OpenApiCredentialInputSchema)), + spec: OpenApiSpecInputSchema, + name: Schema.String, + baseUrl: Schema.String, + namespace: Schema.String, + headers: Schema.optional(Schema.Record(Schema.String, OpenApiConfiguredValueInputSchema)), + queryParams: Schema.optional(Schema.Record(Schema.String, OpenApiConfiguredValueInputSchema)), oauth2: Schema.optional(OpenApiOAuthInputSchema), - credentialTargetScope: Schema.optional(Schema.String), specFetchCredentials: Schema.optional( Schema.Struct({ - headers: Schema.optional(Schema.Record(Schema.String, HeaderValueSchema)), - queryParams: Schema.optional(Schema.Record(Schema.String, HeaderValueSchema)), + headers: Schema.optional(Schema.Record(Schema.String, OpenApiConfiguredValueInputSchema)), + queryParams: Schema.optional(Schema.Record(Schema.String, OpenApiConfiguredValueInputSchema)), }), ), }); @@ -346,27 +366,17 @@ const specFetchQueryParamSlotFromName = (name: string): string => `spec_fetch_query_param:${slotPart(name)}`; const canonicalizeHeaders = ( - headers: Record | undefined, + headers: Record | undefined, ): { readonly headers: Record; - readonly bindings: ReadonlyArray<{ - readonly slot: string; - readonly value: OpenApiSourceBindingValue; - readonly targetScope?: string; - }>; } => { const nextHeaders: Record = {}; - const bindings: Array<{ - slot: string; - value: OpenApiSourceBindingValue; - targetScope?: string; - }> = []; for (const [name, value] of Object.entries(headers ?? {})) { if (typeof value === "string") { nextHeaders[name] = value; continue; } - if ("kind" in value) { + if (value.kind === "binding") { nextHeaders[name] = value; continue; } @@ -376,44 +386,23 @@ const canonicalizeHeaders = ( slot, prefix: value.prefix, }); - bindings.push({ - slot, - targetScope: "targetScope" in value ? value.targetScope : undefined, - value: { - kind: "secret", - secretId: SecretId.make(value.secretId), - ...("secretScopeId" in value && value.secretScopeId - ? { secretScopeId: value.secretScopeId } - : {}), - }, - }); } - return { headers: nextHeaders, bindings }; + return { headers: nextHeaders }; }; const canonicalizeCredentialMap = ( - values: Record | undefined, + values: Record | undefined, slotForName: (name: string) => string, ): { readonly values: Record; - readonly bindings: ReadonlyArray<{ - readonly slot: string; - readonly value: OpenApiSourceBindingValue; - readonly targetScope?: string; - }>; } => { const nextValues: Record = {}; - const bindings: Array<{ - slot: string; - value: OpenApiSourceBindingValue; - targetScope?: string; - }> = []; for (const [name, value] of Object.entries(values ?? {})) { if (typeof value === "string") { nextValues[name] = value; continue; } - if ("kind" in value) { + if (value.kind === "binding") { nextValues[name] = value; continue; } @@ -423,35 +412,19 @@ const canonicalizeCredentialMap = ( slot, prefix: value.prefix, }); - bindings.push({ - slot, - targetScope: "targetScope" in value ? value.targetScope : undefined, - value: { - kind: "secret", - secretId: SecretId.make(value.secretId), - ...("secretScopeId" in value && value.secretScopeId - ? { secretScopeId: value.secretScopeId } - : {}), - }, - }); } - return { values: nextValues, bindings }; + return { values: nextValues }; }; const canonicalizeSpecFetchCredentials = ( credentials: | { - readonly headers?: Record; - readonly queryParams?: Record; + readonly headers?: Record; + readonly queryParams?: Record; } | undefined, ): { readonly credentials?: SourceConfig["specFetchCredentials"]; - readonly bindings: ReadonlyArray<{ - readonly slot: string; - readonly value: OpenApiSourceBindingValue; - readonly targetScope?: string; - }>; } => { const headers = canonicalizeCredentialMap(credentials?.headers, specFetchHeaderSlotFromName); const queryParams = canonicalizeCredentialMap( @@ -469,7 +442,6 @@ const canonicalizeSpecFetchCredentials = ( }; return { credentials: nextCredentials, - bindings: [...headers.bindings, ...queryParams.bindings], }; }; @@ -599,19 +571,6 @@ const validateOpenApiBindingTarget = ( } }); -const targetScopeForBinding = ( - fallbackTargetScope: string | undefined, - binding: { readonly slot: string; readonly targetScope?: string }, -): Effect.Effect => { - const targetScope = binding.targetScope ?? fallbackTargetScope; - if (targetScope) return Effect.succeed(targetScope); - return Effect.fail( - new OpenApiOAuthError({ - message: "credentialTargetScope is required when adding direct OpenAPI credentials", - }), - ); -}; - const findOuterSource = ( ctx: PluginCtx, namespace: string, @@ -790,7 +749,7 @@ const resolveOAuthConnectionId = ( const resolveSpecFetchInputCredentials = ( ctx: PluginCtx, - credentials: OpenApiSpecFetchCredentialsInput | undefined, + credentials: OpenApiPreviewSpecFetchCredentialsInput | undefined, ) => Effect.gen(function* () { if (!credentials) return undefined; @@ -850,13 +809,13 @@ const toOpenApiSourceConfig = ( ): OpenApiSourceConfig => { const configHeaders: Record = {}; for (const [name, value] of Object.entries(config.headers ?? {})) { - if (typeof value === "string" || !("kind" in value)) { + if (typeof value === "string") { configHeaders[name] = value; } } return { kind: "openapi", - spec: config.spec, + spec: specInputToConfigString(config.spec), baseUrl: config.baseUrl, namespace, headers: headersToConfigValues( @@ -865,24 +824,24 @@ const toOpenApiSourceConfig = ( }; }; -const isHttpUrl = (s: string): boolean => s.startsWith("http://") || s.startsWith("https://"); +const specInputToConfigString = (spec: OpenApiSpecInput): string => + spec.kind === "url" ? spec.url : spec.value; export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { type RebuildInput = { readonly specText: string; readonly scope: string; readonly sourceUrl?: string; - readonly name?: string; - readonly baseUrl?: string; - readonly namespace?: string; - readonly headers?: Record; - readonly queryParams?: Record; + readonly name: string; + readonly baseUrl: string | undefined; + readonly namespace: string; + readonly headers?: Record; + readonly queryParams?: Record; readonly specFetchCredentials?: { - readonly headers?: Record; - readonly queryParams?: Record; + readonly headers?: Record; + readonly queryParams?: Record; }; readonly oauth2?: OpenApiOAuthInput; - readonly credentialTargetScope?: string; }; // ctx comes from the plugin runtime — the same instance is passed to @@ -893,11 +852,7 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { const doc = yield* parse(input.specText); const result = yield* extract(doc); - const namespace = - input.namespace ?? - Option.getOrElse(result.title, () => "api") - .toLowerCase() - .replace(/[^a-z0-9]+/g, "_"); + const namespace = input.namespace; const outerSource = yield* findOuterSource(ctx, namespace, input.scope); if (outerSource && input.baseUrl !== undefined && input.baseUrl.trim() !== "") { return yield* new OpenApiOAuthError({ @@ -912,7 +867,7 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { } } - const baseUrl = outerSource ? undefined : (input.baseUrl ?? resolveBaseUrl(result.servers)); + const baseUrl = outerSource ? undefined : input.baseUrl; const canonicalHeaders = canonicalizeHeaders(input.headers); const canonicalQueryParams = canonicalizeCredentialMap( input.queryParams, @@ -922,25 +877,8 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { input.specFetchCredentials, ); const canonicalOAuth2 = canonicalizeOAuth2(input.oauth2); - const directBindings = [ - ...canonicalHeaders.bindings, - ...canonicalQueryParams.bindings, - ...canonicalSpecFetchCredentials.bindings, - ...canonicalOAuth2.bindings, - ]; - for (const binding of directBindings) { - const bindingTargetScope = yield* targetScopeForBinding( - input.credentialTargetScope, - binding, - ); - yield* validateOpenApiBindingTarget(ctx, { - sourceId: namespace, - sourceScope: input.scope, - targetScope: bindingTargetScope, - }); - } const definitions = compileToolDefinitions(result.operations); - const sourceName = input.name ?? Option.getOrElse(result.title, () => namespace); + const sourceName = input.name; const sourceConfig: SourceConfig = { spec: input.specText, @@ -989,23 +927,6 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { })), }); - if (directBindings.length > 0) { - for (const binding of directBindings) { - const bindingTargetScope = yield* targetScopeForBinding( - input.credentialTargetScope, - binding, - ); - yield* ctx.credentialBindings.set({ - targetScope: ScopeId.make(bindingTargetScope), - pluginId: OPENAPI_PLUGIN_ID, - sourceId: namespace, - sourceScope: ScopeId.make(input.scope), - slotKey: binding.slot, - value: binding.value, - }); - } - } - if (Object.keys(hoistedDefs).length > 0) { yield* ctx.core.definitions.register({ sourceId: namespace, @@ -1046,13 +967,12 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { scope, sourceUrl, name: existing.name, - baseUrl: resolvedConfig.baseUrl, + baseUrl: existing.config.baseUrl, namespace: existing.namespace, headers: existing.config.headers, queryParams: existing.config.queryParams, specFetchCredentials: existing.config.specFetchCredentials, oauth2: existing.config.oauth2, - credentialTargetScope: scope, }); }); @@ -1069,17 +989,14 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { // Resolve URL → text and parse BEFORE opening a transaction. // Holding `BEGIN` on the pool=1 Postgres connection across a // network fetch is the Hyperdrive deadlock path in production. - const credentials = yield* resolveSpecFetchInputCredentials( - ctx, - config.specFetchCredentials, - ); - const specText = yield* resolveSpecText(config.spec, credentials).pipe( - Effect.provide(httpClientLayer), - ); + const specText = + config.spec.kind === "url" + ? yield* resolveSpecText(config.spec.url).pipe(Effect.provide(httpClientLayer)) + : config.spec.value; return yield* rebuildSource(ctx, { specText, scope: config.scope, - sourceUrl: isHttpUrl(config.spec) ? config.spec : undefined, + sourceUrl: config.spec.kind === "url" ? config.spec.url : undefined, name: config.name, baseUrl: config.baseUrl, namespace: config.namespace, @@ -1087,11 +1004,6 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { queryParams: config.queryParams, specFetchCredentials: config.specFetchCredentials, oauth2: config.oauth2, - // Default to the source's own scope. refreshSource and editSource - // do the same; without this, config-sync's addSpec — which never - // passes the field — fails with "credentialTargetScope is - // required" the moment the jsonc declares any header secret. - credentialTargetScope: config.credentialTargetScope ?? config.scope, }); }); @@ -1164,17 +1076,12 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { input.queryParams !== undefined ? canonicalizeCredentialMap(input.queryParams, queryParamSlotFromName) : null; - const directBindings = [ - ...(canonicalHeaders?.bindings ?? []), - ...(canonicalQueryParams?.bindings ?? []), - ...(canonicalOAuth2?.bindings ?? []), - ]; const affectedPrefixes = [ ...(input.headers !== undefined ? ["header:"] : []), ...(input.queryParams !== undefined ? ["query_param:"] : []), ...(input.oauth2 !== undefined ? ["oauth2:"] : []), ]; - const targetScope = input.credentialTargetScope ?? scope; + const targetScope = scope; if (input.baseUrl !== undefined && input.baseUrl.trim() !== "") { const outerSource = yield* findOuterSource(ctx, namespace, scope); if (outerSource) { @@ -1183,7 +1090,7 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { }); } } - if (affectedPrefixes.length > 0 || directBindings.length > 0) { + if (affectedPrefixes.length > 0) { yield* validateOpenApiBindingTarget(ctx, { sourceId: namespace, sourceScope: scope, @@ -1199,17 +1106,14 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { queryParams: canonicalQueryParams?.values, oauth2: canonicalOAuth2?.oauth2, }); - if (affectedPrefixes.length > 0 || directBindings.length > 0) { + if (affectedPrefixes.length > 0) { yield* ctx.credentialBindings.replaceForSource({ targetScope: ScopeId.make(targetScope), pluginId: OPENAPI_PLUGIN_ID, sourceId: namespace, sourceScope: ScopeId.make(scope), slotPrefixes: affectedPrefixes, - bindings: directBindings.map((binding) => ({ - slotKey: binding.slot, - value: binding.value, - })), + bindings: [], }); } }), diff --git a/packages/plugins/openapi/src/sdk/real-specs.test.ts b/packages/plugins/openapi/src/sdk/real-specs.test.ts index b8db04c5c..f96ae6116 100644 --- a/packages/plugins/openapi/src/sdk/real-specs.test.ts +++ b/packages/plugins/openapi/src/sdk/real-specs.test.ts @@ -90,8 +90,9 @@ const getRegisteredToolSchema = (namespace: string, specText: string, toolId: st ); yield* executor.openapi.addSpec({ - spec: specText, + spec: { kind: "blob", value: specText }, scope: TEST_SCOPE, + name: namespace, namespace, baseUrl: "", }); diff --git a/packages/plugins/openapi/src/testing/index.ts b/packages/plugins/openapi/src/testing/index.ts index fb3a45037..dcd5eacaf 100644 --- a/packages/plugins/openapi/src/testing/index.ts +++ b/packages/plugins/openapi/src/testing/index.ts @@ -80,22 +80,30 @@ export interface OpenApiEchoTestServerShape extends OpenApiTestServerShape { readonly clearRequests: Effect.Effect; } -export type OpenApiTestSourceOptions = Omit & { +export type OpenApiTestSourceOptions = Omit< + OpenApiSpecConfig, + "spec" | "baseUrl" | "name" | "namespace" +> & { readonly baseUrl?: string | null; + readonly name?: string; + readonly namespace?: string; }; -export type OpenApiHttpApiTestSourceOptions = Omit & { +export type OpenApiHttpApiTestSourceOptions = Omit< + OpenApiSpecConfig, + "spec" | "name" | "namespace" | "baseUrl" +> & { + readonly name?: string; + readonly namespace?: string; + readonly baseUrl?: string | null; readonly specBaseUrl?: string; readonly transformSpec?: (spec: Record) => Record; }; export type OpenApiHttpApiTestAddSpecPayloadOptions = Omit< OpenApiHttpApiTestSourceOptions, - "scope" | "credentialTargetScope" -> & { - readonly targetScope: ScopeId; - readonly credentialTargetScope?: ScopeId; -}; + "scope" +>; export type OpenApiTestSourceExecutor = { readonly openapi: Pick; @@ -125,9 +133,11 @@ export const makeOpenApiTestSourceConfig = ( const { baseUrl, ...rest } = options; return { ...rest, - spec: server.specJson, + spec: { kind: "blob", value: server.specJson }, + name: rest.name ?? "Test API", + namespace: rest.namespace ?? "test_api", ...(baseUrl === null ? {} : { baseUrl: baseUrl ?? server.baseUrl }), - }; + } as OpenApiSpecConfig; }; export const addOpenApiTestSource = ( @@ -140,11 +150,17 @@ export const makeOpenApiHttpApiTestSourceConfig = ( api: HttpApi.Any, options: OpenApiHttpApiTestSourceOptions, ): OpenApiSpecConfig => { - const { specBaseUrl, transformSpec, ...config } = options; + const { baseUrl, specBaseUrl, transformSpec, ...config } = options; return { ...config, - spec: makeOpenApiTestSpecJson(api, { baseUrl: specBaseUrl, transformSpec }), - }; + spec: { + kind: "blob", + value: makeOpenApiTestSpecJson(api, { baseUrl: specBaseUrl, transformSpec }), + }, + name: config.name ?? "Test API", + namespace: config.namespace ?? "test_api", + ...(baseUrl === null ? {} : { baseUrl: baseUrl ?? specBaseUrl ?? "https://api.example.test" }), + } as OpenApiSpecConfig; }; export const addOpenApiHttpApiTestSource = ( @@ -157,24 +173,19 @@ export const makeOpenApiHttpApiTestAddSpecPayload = ( api: HttpApi.Any, options: OpenApiHttpApiTestAddSpecPayloadOptions, ) => { - const { targetScope, credentialTargetScope, ...sourceOptions } = options; + const { ...sourceOptions } = options; const config = makeOpenApiHttpApiTestSourceConfig(api, { ...sourceOptions, - scope: String(targetScope), - ...(credentialTargetScope !== undefined - ? { credentialTargetScope: String(credentialTargetScope) } - : {}), + scope: "unused-http-helper-scope", }); return { - targetScope, spec: config.spec, namespace: config.namespace, - ...(config.name !== undefined ? { name: config.name } : {}), - ...(config.baseUrl !== undefined ? { baseUrl: config.baseUrl } : {}), + name: config.name, + baseUrl: config.baseUrl, ...(config.headers !== undefined ? { headers: config.headers } : {}), ...(config.queryParams !== undefined ? { queryParams: config.queryParams } : {}), ...(config.oauth2 !== undefined ? { oauth2: config.oauth2 } : {}), - ...(credentialTargetScope !== undefined ? { credentialTargetScope } : {}), ...(config.specFetchCredentials !== undefined ? { specFetchCredentials: config.specFetchCredentials } : {}), From d5f79907731d1b59733356a03b77e071dc66606b Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Sun, 17 May 2026 12:32:15 -0700 Subject: [PATCH 2/5] Update OpenAPI add source credential UI --- .../openapi/src/react/AddOpenApiSource.tsx | 222 +++++++++++------- .../openapi/src/sdk/source-contracts.ts | 6 + 2 files changed, 140 insertions(+), 88 deletions(-) diff --git a/packages/plugins/openapi/src/react/AddOpenApiSource.tsx b/packages/plugins/openapi/src/react/AddOpenApiSource.tsx index 6ceec91da..866c199aa 100644 --- a/packages/plugins/openapi/src/react/AddOpenApiSource.tsx +++ b/packages/plugins/openapi/src/react/AddOpenApiSource.tsx @@ -66,6 +66,8 @@ import { oauth2ClientSecretSlot, oauth2ConnectionSlot, queryParamBindingSlot, + specFetchHeaderBindingSlot, + specFetchQueryParamBindingSlot, } from "../sdk/source-contracts"; import { OAuth2SourceConfig, @@ -127,6 +129,17 @@ export function inferOAuthIssuerUrl(authorizationUrl: string): string | null { } } +const specInputForAdd = (input: string) => { + const value = input.trim(); + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: URL constructor classifies user-provided spec input + try { + new URL(value); + return { kind: "url" as const, url: value }; + } catch { + return { kind: "blob" as const, value }; + } +}; + type StrategySelection = | { readonly kind: "none" } | { readonly kind: "custom" } @@ -255,35 +268,19 @@ export default function AddOpenApiSource(props: { })), [scopeStack], ); - const defaultCredentialTargetScope = - credentialScopeOptions[credentialScopeOptions.length - 1]?.scopeId ?? scopeId; const defaultOAuthTokenTargetScope = credentialScopeOptions[0]?.scopeId ?? scopeId; - const [credentialTargetScope, setCredentialTargetScope] = useState( - defaultCredentialTargetScope, - ); const [oauthTokenTargetScope, setOAuthTokenTargetScope] = useState( defaultOAuthTokenTargetScope, ); - const bindingScopeOptions = useMemo( - () => credentialScopeOptions.map((option) => ({ ...option })), - [credentialScopeOptions], + const sourceCredentialScopeOptions = useMemo( + () => credentialScopeOptions.filter((option) => option.scopeId === scopeId), + [credentialScopeOptions, scopeId], ); - useEffect(() => { - if (!credentialScopeOptions.some((option) => option.scopeId === credentialTargetScope)) { - setCredentialTargetScope(defaultCredentialTargetScope); - } - }, [credentialScopeOptions, credentialTargetScope, defaultCredentialTargetScope]); useEffect(() => { if (!credentialScopeOptions.some((option) => option.scopeId === oauthTokenTargetScope)) { setOAuthTokenTargetScope(defaultOAuthTokenTargetScope); } }, [credentialScopeOptions, defaultOAuthTokenTargetScope, oauthTokenTargetScope]); - const initialCredentialScopeOption = - credentialScopeOptions.find((option) => option.scopeId === credentialTargetScope) ?? - credentialScopeOptions[0]; - const initialCredentialScopeOptions = initialCredentialScopeOption - ? [initialCredentialScopeOption] - : []; const doPreview = useAtomSet(previewOpenApiSpec, { mode: "promiseExit" }); const doAdd = useAtomSet(addOpenApiSpecOptimistic(scopeId), { mode: "promiseExit", @@ -293,9 +290,9 @@ export default function AddOpenApiSource(props: { mode: "promiseExit", }); const secretList = useSecretPickerSecrets(); - const initialCredentialSecrets = useMemo( - () => secretList.filter((secret) => secret.scopeId === String(credentialTargetScope)), - [credentialTargetScope, secretList], + const sourceCredentialSecrets = useMemo( + () => secretList.filter((secret) => secret.scopeId === String(scopeId)), + [scopeId, secretList], ); const oauth = useOAuthPopupFlow({ popupName: OPENAPI_OAUTH_POPUP_NAME, @@ -335,21 +332,19 @@ export default function AddOpenApiSource(props: { ); const resolvedBaseUrl = baseUrl.trim(); + const sourceScope = ScopeId.make(scopeId); + + type PendingSecretBinding = { + readonly slot: string; + readonly secretId: string; + readonly scope: ScopeId; + readonly secretScope: ScopeId; + }; const configuredHeaders: Record = {}; - const headerBindings: Array<{ - slot: string; - secretId: string; - scope: ScopeId; - secretScope: ScopeId; - }> = []; + const headerBindings: PendingSecretBinding[] = []; const configuredQueryParams: Record = {}; - const queryParamBindings: Array<{ - slot: string; - secretId: string; - scope: ScopeId; - secretScope: ScopeId; - }> = []; + const queryParamBindings: PendingSecretBinding[] = []; for (const ch of customHeaders) { if (!ch.name.trim()) continue; const slot = headerBindingSlot(ch.name.trim()); @@ -358,8 +353,8 @@ export default function AddOpenApiSource(props: { headerBindings.push({ slot, secretId: ch.secretId, - scope: ch.targetScope ?? credentialTargetScope, - secretScope: ch.secretScope ?? ch.targetScope ?? credentialTargetScope, + scope: sourceScope, + secretScope: ch.secretScope ?? sourceScope, }); } } @@ -372,8 +367,8 @@ export default function AddOpenApiSource(props: { queryParamBindings.push({ slot, secretId: param.secretId, - scope: param.targetScope ?? credentialTargetScope, - secretScope: param.secretScope ?? param.targetScope ?? credentialTargetScope, + scope: sourceScope, + secretScope: param.secretScope ?? sourceScope, }); continue; } @@ -381,6 +376,52 @@ export default function AddOpenApiSource(props: { configuredQueryParams[name] = param.literalValue.trim(); } } + const configuredSpecFetchHeaders: Record = {}; + const configuredSpecFetchQueryParams: Record< + string, + string | { kind: "secret"; prefix?: string } + > = {}; + const specFetchBindings: PendingSecretBinding[] = []; + for (const header of specFetchCredentials.headers) { + const name = header.name.trim(); + if (!name || !header.secretId) continue; + configuredSpecFetchHeaders[name] = { kind: "secret", prefix: header.prefix }; + specFetchBindings.push({ + slot: specFetchHeaderBindingSlot(name), + secretId: header.secretId, + scope: sourceScope, + secretScope: header.secretScope ?? sourceScope, + }); + } + for (const param of specFetchCredentials.queryParams) { + const name = param.name.trim(); + if (!name) continue; + if (param.secretId) { + configuredSpecFetchQueryParams[name] = { kind: "secret", prefix: param.prefix }; + specFetchBindings.push({ + slot: specFetchQueryParamBindingSlot(name), + secretId: param.secretId, + scope: sourceScope, + secretScope: param.secretScope ?? sourceScope, + }); + continue; + } + if (param.literalValue?.trim()) { + configuredSpecFetchQueryParams[name] = param.literalValue.trim(); + } + } + const configuredSpecFetchCredentials = + Object.keys(configuredSpecFetchHeaders).length > 0 || + Object.keys(configuredSpecFetchQueryParams).length > 0 + ? { + ...(Object.keys(configuredSpecFetchHeaders).length > 0 + ? { headers: configuredSpecFetchHeaders } + : {}), + ...(Object.keys(configuredSpecFetchQueryParams).length > 0 + ? { queryParams: configuredSpecFetchQueryParams } + : {}), + } + : null; const oauth2Presets: readonly OAuth2Preset[] = preview?.oauth2Presets ?? []; const oauth2RedirectUrl = oauthCallbackUrl(OPENAPI_OAUTH_CALLBACK_PATH); @@ -438,8 +479,14 @@ export default function AddOpenApiSource(props: { const hasIncompleteQueryCredentials = runtimeCredentials.queryParams.some( (param) => param.name.trim() && !param.secretId && !param.literalValue?.trim(), ); + const hasIncompleteSpecFetchCredentials = + specFetchCredentials.headers.some((header) => header.name.trim() && !header.secretId) || + specFetchCredentials.queryParams.some( + (param) => param.name.trim() && !param.secretId && !param.literalValue?.trim(), + ); const willAddWithoutInitialCredentials = Boolean(selectedOAuth2Preset && !oauth2Auth) || + hasIncompleteSpecFetchCredentials || hasIncompleteHeaderCredentials || hasIncompleteQueryCredentials; @@ -530,24 +577,6 @@ export default function AddOpenApiSource(props: { } }; - const setInitialCredentialScope = (targetScope: ScopeId) => { - setCredentialTargetScope(targetScope); - setCustomHeaders((headers) => - headers.map((header) => ({ - ...header, - targetScope, - ...(header.secretScope && header.secretScope !== targetScope - ? { secretId: null, secretScope: undefined } - : {}), - })), - ); - setOauth2ClientIdSecretId(null); - setOauth2ClientSecretSecretId(null); - setOauth2ClientIdScope(null); - setOauth2ClientSecretScope(null); - setOauth2AuthState(null); - }; - const toggleOAuth2Scope = (scope: string) => { setOauth2SelectedScopes((prev) => { const copy = new Set(prev); @@ -700,10 +729,13 @@ export default function AddOpenApiSource(props: { const exit = await doAdd({ params: { scopeId }, payload: { - spec: { kind: "url", url: specUrl }, + spec: specInputForAdd(specUrl), name: displayName, namespace, baseUrl: resolvedBaseUrl, + ...(configuredSpecFetchCredentials + ? { specFetchCredentials: configuredSpecFetchCredentials } + : {}), ...(hasHeaders ? { headers: configuredHeaders } : {}), ...(Object.keys(configuredQueryParams).length > 0 ? { queryParams: configuredQueryParams } @@ -719,11 +751,9 @@ export default function AddOpenApiSource(props: { } const sourceId = exit.value.namespace; - const sourceScope = ScopeId.make(scopeId); - const bindingScope = ScopeId.make(credentialTargetScope); const oauthTokenBindingScope = ScopeId.make(oauthTokenTargetScope); - const clientIdSecretScope = oauth2ClientIdScope ?? bindingScope; - const clientSecretSecretScope = oauth2ClientSecretScope ?? bindingScope; + const clientIdSecretScope = oauth2ClientIdScope ?? sourceScope; + const clientSecretSecretScope = oauth2ClientSecretScope ?? sourceScope; for (const binding of headerBindings) { const bindingExit = await doSetBinding({ @@ -771,13 +801,36 @@ export default function AddOpenApiSource(props: { } } + for (const binding of specFetchBindings) { + const bindingExit = await doSetBinding({ + params: { scopeId }, + payload: OpenApiSourceBindingInput.make({ + sourceId, + sourceScope, + scope: binding.scope, + slot: binding.slot, + value: { + kind: "secret", + secretId: SecretId.make(binding.secretId), + secretScopeId: binding.secretScope, + }, + }), + reactivityKeys: bindingWriteKeys, + }); + if (Exit.isFailure(bindingExit)) { + setAddError(errorMessageFromExit(bindingExit, "Failed to add source")); + setAdding(false); + return; + } + } + if (configuredOAuth2 && oauth2ClientIdSecretId) { const bindingExit = await doSetBinding({ params: { scopeId }, payload: OpenApiSourceBindingInput.make({ sourceId, sourceScope, - scope: bindingScope, + scope: sourceScope, slot: configuredOAuth2.clientIdSlot, value: { kind: "secret", @@ -800,7 +853,7 @@ export default function AddOpenApiSource(props: { payload: OpenApiSourceBindingInput.make({ sourceId, sourceScope, - scope: bindingScope, + scope: sourceScope, slot: configuredOAuth2.clientSecretSlot, value: { kind: "secret", @@ -896,7 +949,10 @@ export default function AddOpenApiSource(props: { onChange={setSpecFetchCredentials} existingSecrets={secretList} sourceName={identity.name} - targetScope={credentialTargetScope} + targetScope={sourceScope} + credentialScopeOptions={sourceCredentialScopeOptions} + bindingScopeOptions={sourceCredentialScopeOptions} + restrictSecretsToTargetScope labels={{ headers: "Spec fetch headers", queryParams: "Spec fetch query parameters", @@ -1032,9 +1088,9 @@ export default function AddOpenApiSource(props: { onHeadersChange={handleHeadersChange} existingSecrets={secretList} sourceName={identity.name} - targetScope={credentialTargetScope} - credentialScopeOptions={initialCredentialScopeOptions} - bindingScopeOptions={bindingScopeOptions} + targetScope={sourceScope} + credentialScopeOptions={sourceCredentialScopeOptions} + bindingScopeOptions={sourceCredentialScopeOptions} restrictSecretsToTargetScope emptyLabel="No credentials yet. Add the header value this method should use." /> @@ -1046,9 +1102,9 @@ export default function AddOpenApiSource(props: { onChange={setRuntimeCredentials} existingSecrets={secretList} sourceName={identity.name} - targetScope={credentialTargetScope} - credentialScopeOptions={initialCredentialScopeOptions} - bindingScopeOptions={bindingScopeOptions} + targetScope={sourceScope} + credentialScopeOptions={sourceCredentialScopeOptions} + bindingScopeOptions={sourceCredentialScopeOptions} restrictSecretsToTargetScope sections={{ headers: false, queryParams: true }} labels={{ queryParams: "Runtime query parameters" }} @@ -1084,22 +1140,17 @@ export default function AddOpenApiSource(props: { value={oauth2ClientIdSecretId} onSelect={(id: string, secretScopeId?: ScopeId) => { setOauth2ClientIdSecretId(id); - setOauth2ClientIdScope(secretScopeId ?? credentialTargetScope); + setOauth2ClientIdScope(secretScopeId ?? sourceScope); setOauth2AuthState(null); }} - secrets={initialCredentialSecrets} + secrets={sourceCredentialSecrets} sourceName={identity.name} secretLabel="Client ID" - targetScope={oauth2ClientIdScope ?? credentialTargetScope} - credentialScopeOptions={initialCredentialScopeOptions} + targetScope={oauth2ClientIdScope ?? sourceScope} + credentialScopeOptions={sourceCredentialScopeOptions} onCreatedScope={setOauth2ClientIdScope} /> -
@@ -1121,22 +1172,17 @@ export default function AddOpenApiSource(props: { value={oauth2ClientSecretSecretId} onSelect={(id: string, secretScopeId?: ScopeId) => { setOauth2ClientSecretSecretId(id); - setOauth2ClientSecretScope(secretScopeId ?? credentialTargetScope); + setOauth2ClientSecretScope(secretScopeId ?? sourceScope); setOauth2AuthState(null); }} - secrets={initialCredentialSecrets} + secrets={sourceCredentialSecrets} sourceName={identity.name} secretLabel="Client Secret" - targetScope={oauth2ClientSecretScope ?? credentialTargetScope} - credentialScopeOptions={initialCredentialScopeOptions} + targetScope={oauth2ClientSecretScope ?? sourceScope} + credentialScopeOptions={sourceCredentialScopeOptions} onCreatedScope={setOauth2ClientSecretScope} />
-
diff --git a/packages/plugins/openapi/src/sdk/source-contracts.ts b/packages/plugins/openapi/src/sdk/source-contracts.ts index 24c35cb08..c1fa4fbc3 100644 --- a/packages/plugins/openapi/src/sdk/source-contracts.ts +++ b/packages/plugins/openapi/src/sdk/source-contracts.ts @@ -41,6 +41,12 @@ export const headerBindingSlot = (headerName: string): string => export const queryParamBindingSlot = (name: string): string => `query_param:${slugifySlotPart(name)}`; +export const specFetchHeaderBindingSlot = (headerName: string): string => + `spec_fetch_header:${slugifySlotPart(headerName)}`; + +export const specFetchQueryParamBindingSlot = (name: string): string => + `spec_fetch_query_param:${slugifySlotPart(name)}`; + export const oauth2ClientIdSlot = (securitySchemeName: string): string => `oauth2:${slugifySlotPart(securitySchemeName)}:client-id`; From a43182a62374044cfeac80c1d3538dc7a22c152a Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Sun, 17 May 2026 12:45:21 -0700 Subject: [PATCH 3/5] Address OpenAPI add source PR feedback --- .../src/services/sources-api.node.test.ts | 24 +++++++------------ .../services/tenant-isolation.node.test.ts | 20 +++++++--------- .../openapi/src/react/AddOpenApiSource.tsx | 17 +++++++------ packages/plugins/openapi/src/testing/index.ts | 2 +- 4 files changed, 28 insertions(+), 35 deletions(-) diff --git a/apps/cloud/src/services/sources-api.node.test.ts b/apps/cloud/src/services/sources-api.node.test.ts index 9aae3bd0e..662623b47 100644 --- a/apps/cloud/src/services/sources-api.node.test.ts +++ b/apps/cloud/src/services/sources-api.node.test.ts @@ -35,12 +35,8 @@ const MinimalSourceApi = HttpApi.make("sourcesApiTest") .annotateMerge(OpenApi.annotations({ title: "Sources API Test", version: "1.0.0" })); const makeMinimalOpenApiSourcePayload = ( - _targetScope: ScopeId, namespace: string, - options: Omit< - Parameters[1], - "namespace" - > = {}, + options: Omit[1], "namespace"> = {}, ) => makeOpenApiHttpApiTestAddSpecPayload(MinimalSourceApi, { namespace, @@ -71,7 +67,7 @@ describe("sources api (HTTP)", () => { Effect.gen(function* () { const result = yield* client.openapi.addSpec({ params: { scopeId: ScopeId.make(org) }, - payload: makeMinimalOpenApiSourcePayload(ScopeId.make(org), namespace), + payload: makeMinimalOpenApiSourcePayload(namespace), }); expect(result.namespace).toBe(namespace); expect(result.toolCount).toBeGreaterThan(0); @@ -93,7 +89,7 @@ describe("sources api (HTTP)", () => { yield* asOrg(org, (client) => client.openapi.addSpec({ params: { scopeId: ScopeId.make(org) }, - payload: makeMinimalOpenApiSourcePayload(ScopeId.make(org), namespace), + payload: makeMinimalOpenApiSourcePayload(namespace), }), ); @@ -135,11 +131,9 @@ describe("sources api (HTTP)", () => { const result = yield* asOrg(org, (client) => client.openapi.addSpec({ params: { scopeId: ScopeId.make(org) }, - payload: makeMinimalOpenApiSourcePayload( - ScopeId.make(org), - `ns_${crypto.randomUUID().replace(/-/g, "_")}`, - { baseUrl: "http://example.com" }, - ), + payload: makeMinimalOpenApiSourcePayload(`ns_${crypto.randomUUID().replace(/-/g, "_")}`, { + baseUrl: "http://example.com", + }), }), ); @@ -414,7 +408,7 @@ describe("sources api (HTTP)", () => { Effect.gen(function* () { yield* client.openapi.addSpec({ params: { scopeId: ScopeId.make(org) }, - payload: makeMinimalOpenApiSourcePayload(ScopeId.make(org), namespace), + payload: makeMinimalOpenApiSourcePayload(namespace), }); yield* client.sources.remove({ params: { scopeId: ScopeId.make(org), sourceId: namespace }, @@ -468,7 +462,7 @@ describe("sources api (HTTP)", () => { Effect.gen(function* () { yield* client.openapi.addSpec({ params: { scopeId: ScopeId.make(org) }, - payload: makeMinimalOpenApiSourcePayload(ScopeId.make(org), namespace), + payload: makeMinimalOpenApiSourcePayload(namespace), }); yield* client.openapi.updateSource({ params: { scopeId: ScopeId.make(org), namespace }, @@ -502,7 +496,7 @@ describe("sources api (HTTP)", () => { client.openapi.addSpec({ params: { scopeId: ScopeId.make(orgId) }, payload: { - ...makeMinimalOpenApiSourcePayload(ScopeId.make(orgId), namespace), + ...makeMinimalOpenApiSourcePayload(namespace), headers: { Authorization: { kind: "secret", diff --git a/apps/cloud/src/services/tenant-isolation.node.test.ts b/apps/cloud/src/services/tenant-isolation.node.test.ts index 60df82c6e..81288fd2e 100644 --- a/apps/cloud/src/services/tenant-isolation.node.test.ts +++ b/apps/cloud/src/services/tenant-isolation.node.test.ts @@ -20,12 +20,8 @@ const TenantIsolationApi = HttpApi.make("tenantIsolationTest") .annotateMerge(OpenApi.annotations({ title: "Tenant Test API", version: "1.0.0" })); const makeTenantOpenApiSourcePayload = ( - _targetScope: ScopeId, namespace: string, - options: Omit< - Parameters[1], - "namespace" - > = {}, + options: Omit[1], "namespace"> = {}, ) => makeOpenApiHttpApiTestAddSpecPayload(TenantIsolationApi, { namespace, @@ -115,7 +111,7 @@ describe("tenant isolation (HTTP)", () => { yield* asOrg(orgA, (client) => client.openapi.addSpec({ params: { scopeId: ScopeId.make(orgA) }, - payload: makeTenantOpenApiSourcePayload(ScopeId.make(orgA), namespaceA), + payload: makeTenantOpenApiSourcePayload(namespaceA), }), ); @@ -135,7 +131,7 @@ describe("tenant isolation (HTTP)", () => { yield* asOrg(orgA, (client) => client.openapi.addSpec({ params: { scopeId: ScopeId.make(orgA) }, - payload: makeTenantOpenApiSourcePayload(ScopeId.make(orgA), namespaceA), + payload: makeTenantOpenApiSourcePayload(namespaceA), }), ); @@ -158,7 +154,7 @@ describe("tenant isolation (HTTP)", () => { yield* asOrg(orgA, (client) => client.openapi.addSpec({ params: { scopeId: ScopeId.make(orgA) }, - payload: makeTenantOpenApiSourcePayload(ScopeId.make(orgA), namespaceA), + payload: makeTenantOpenApiSourcePayload(namespaceA), }), ); @@ -258,7 +254,7 @@ describe("tenant isolation (HTTP)", () => { yield* client.openapi.addSpec({ params: { scopeId: ScopeId.make(orgA) }, payload: { - ...makeTenantOpenApiSourcePayload(ScopeId.make(orgA), namespaceA), + ...makeTenantOpenApiSourcePayload(namespaceA), headers: { Authorization: { kind: "secret", @@ -301,7 +297,7 @@ describe("tenant isolation (HTTP)", () => { Effect.gen(function* () { yield* client.openapi.addSpec({ params: { scopeId: ScopeId.make(orgA) }, - payload: makeTenantOpenApiSourcePayload(ScopeId.make(orgA), namespaceA), + payload: makeTenantOpenApiSourcePayload(namespaceA), }); yield* client.openapi.setSourceBinding({ params: { scopeId: ScopeId.make(orgA) }, @@ -335,7 +331,7 @@ describe("tenant isolation (HTTP)", () => { yield* asOrg(orgA, (client) => client.openapi.addSpec({ params: { scopeId: ScopeId.make(orgA) }, - payload: makeTenantOpenApiSourcePayload(ScopeId.make(orgA), namespace, { + payload: makeTenantOpenApiSourcePayload(namespace, { name: "Org A API", baseUrl: "https://org-a.example.com", }), @@ -344,7 +340,7 @@ describe("tenant isolation (HTTP)", () => { yield* asOrg(orgB, (client) => client.openapi.addSpec({ params: { scopeId: ScopeId.make(orgB) }, - payload: makeTenantOpenApiSourcePayload(ScopeId.make(orgB), namespace, { + payload: makeTenantOpenApiSourcePayload(namespace, { name: "Org B API", baseUrl: "https://org-b.example.com", }), diff --git a/packages/plugins/openapi/src/react/AddOpenApiSource.tsx b/packages/plugins/openapi/src/react/AddOpenApiSource.tsx index 866c199aa..481a910be 100644 --- a/packages/plugins/openapi/src/react/AddOpenApiSource.tsx +++ b/packages/plugins/openapi/src/react/AddOpenApiSource.tsx @@ -1,5 +1,6 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useAtomSet } from "@effect/atom-react"; +import * as Effect from "effect/Effect"; import * as Exit from "effect/Exit"; import * as Match from "effect/Match"; import * as Option from "effect/Option"; @@ -131,13 +132,15 @@ export function inferOAuthIssuerUrl(authorizationUrl: string): string | null { const specInputForAdd = (input: string) => { const value = input.trim(); - // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: URL constructor classifies user-provided spec input - try { - new URL(value); - return { kind: "url" as const, url: value }; - } catch { - return { kind: "blob" as const, value }; - } + const parsed = Effect.runSyncExit( + Effect.try({ + try: () => new URL(value), + catch: () => null, + }), + ); + return Exit.isSuccess(parsed) + ? { kind: "url" as const, url: value } + : { kind: "blob" as const, value }; }; type StrategySelection = diff --git a/packages/plugins/openapi/src/testing/index.ts b/packages/plugins/openapi/src/testing/index.ts index dcd5eacaf..520455ab9 100644 --- a/packages/plugins/openapi/src/testing/index.ts +++ b/packages/plugins/openapi/src/testing/index.ts @@ -13,7 +13,7 @@ import { OpenApi, } from "effect/unstable/httpapi"; import { OAuthTestServer, serveTestHttpServerLayer } from "@executor-js/sdk/testing"; -import { isToolResult, type ScopeId } from "@executor-js/sdk/core"; +import { isToolResult } from "@executor-js/sdk/core"; import type { OpenApiPluginExtension, OpenApiSpecConfig } from "../sdk/plugin"; export class OpenApiTestServerAddressError extends Data.TaggedError( From d366fca39c3656781b4f8f781c21c6b1f20588ca Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Sun, 17 May 2026 17:07:42 -0700 Subject: [PATCH 4/5] Document OpenAPI credential configuration plan --- notes/openapi-sdk-credential-configuration.md | 509 ++++++++++++++++++ notes/source-binding-consolidation.md | 209 +++++++ 2 files changed, 718 insertions(+) create mode 100644 notes/openapi-sdk-credential-configuration.md create mode 100644 notes/source-binding-consolidation.md diff --git a/notes/openapi-sdk-credential-configuration.md b/notes/openapi-sdk-credential-configuration.md new file mode 100644 index 000000000..785f1ee7f --- /dev/null +++ b/notes/openapi-sdk-credential-configuration.md @@ -0,0 +1,509 @@ +# OpenAPI SDK Credential Configuration Notes + +Date: 2026-05-17 +Status: planning + +## Summary + +The SDK should present OpenAPI source onboarding as two explicit phases: + +1. Add/import a shared source shape. +2. Configure scoped credential values for that source. + +The internal implementation can still use source slots and scoped bindings, +but normal SDK callers should not need to think about `setSourceBinding`, +`connectionSlot`, `clientIdSlot`, `credentialTargetScope`, or plugin-specific +binding row shapes. + +The product flow we want to preserve is: + +```txt +Admin adds Stripe/Linear/GitHub once at org scope. +Org may set default credentials. +Each user can sign in or provide their own token. +The source and tools stay shared. +Credentials remain scoped and overridable. +``` + +## Core Vocabulary + +**Source shape** is the shared definition: + +- OpenAPI spec and extracted operations. +- Base URL. +- Declared headers and query params the source knows how to send. +- Selected OAuth method, if any. +- OAuth flow metadata such as authorization URL, token URL, and scopes. + +**Scoped credential values** are the concrete values attached later: + +- Bearer/API key token values. +- Header/query param text values. +- OAuth client ID and client secret. +- OAuth connection IDs. +- Spec-fetch credential values. + +Source shape belongs to the source owner scope. Credential values belong to +the scope where they are configured and can inherit/override through the scope +stack. + +## Public SDK Shape + +The common bearer token flow should read like this: + +```ts +const org = ScopeId.make("org_acme"); +const user = ScopeId.make("user_rhys"); + +const source = await executor.openapi.addSpec({ + scope: org, + name: "Stripe", + namespace: "stripe", + baseUrl: "https://api.stripe.com", + spec: { + kind: "url", + url: "https://raw.githubusercontent.com/stripe/openapi/master/openapi/spec3.json", + }, + + headers: { + Authorization: { + prefix: "Bearer ", + }, + }, +}); + +await executor.openapi.configure(source, { + scope: user, + headers: { + Authorization: SecretId.make("stripe_api_key"), + }, +}); +``` + +The caller should not pass: + +```ts +kind: "binding" +slot: "header:authorization" +credentialTargetScope: user +``` + +Those are internal implementation details. + +## Strict Configure Semantics + +`configure` fills declared holes in an existing source shape. It should not +create new source shape. + +If the source was imported with: + +```ts +headers: { + Authorization: { + prefix: "Bearer ", + }, +} +``` + +then this is valid: + +```ts +await executor.openapi.configure(source, { + scope: user, + headers: { + Authorization: SecretId.make("stripe_api_key"), + }, +}); +``` + +This should fail: + +```ts +await executor.openapi.configure(source, { + scope: user, + headers: { + Username: SecretId.make("stripe_api_key"), + }, +}); +``` + +because `Username` was not declared on the source. Allowing this would silently +write a binding for a value the source never sends. + +Suggested error: + +```txt +Unknown header "Username" for OpenAPI source "stripe". +Declared headers: Authorization. +``` + +The same rule applies to query params: + +```ts +await executor.openapi.configure(source, { + scope: user, + queryParams: { + api_key: SecretId.make("example_api_key"), + }, +}); +``` + +should only succeed if `api_key` was declared in the source shape. + +Adding or changing source shape should be a different operation: + +```ts +await executor.openapi.updateSource(source, { + headers: { + Authorization: { prefix: "Bearer " }, + Username: {}, + }, +}); + +await executor.openapi.configure(source, { + scope: user, + headers: { + Username: SecretId.make("basic_username"), + }, +}); +``` + +## Values Accepted By Configure + +`configure` should accept ergonomic value inputs and normalize them to binding +values internally. + +```ts +type OpenApiConfiguredCredentialValue = + | string + | SecretId + | { + kind: "secret"; + secretId: SecretId; + secretScope?: ScopeId; + } + | { + kind: "text"; + text: string; + }; +``` + +Examples: + +```ts +await executor.openapi.configure(source, { + scope: user, + headers: { + Authorization: SecretId.make("stripe_api_key"), + "X-Workspace": "acme", + }, + queryParams: { + version: "2026-05-17", + }, +}); +``` + +Internal binding writes: + +```ts +setBinding({ + source: { pluginId: "openapi", id: "stripe", scope: org }, + scope: user, + slot: "header:authorization", + value: { kind: "secret", secretId: SecretId.make("stripe_api_key") }, +}); + +setBinding({ + source: { pluginId: "openapi", id: "stripe", scope: org }, + scope: user, + slot: "header:x-workspace", + value: { kind: "text", text: "acme" }, +}); + +setBinding({ + source: { pluginId: "openapi", id: "stripe", scope: org }, + scope: user, + slot: "query_param:version", + value: { kind: "text", text: "2026-05-17" }, +}); +``` + +## Org Default With User Override + +```ts +const source = await executor.openapi.addSpec({ + scope: org, + name: "Example API", + namespace: "example", + baseUrl: "https://api.example.com", + spec: { + kind: "url", + url: "https://api.example.com/openapi.json", + }, + headers: { + Authorization: { + prefix: "Bearer ", + }, + }, +}); + +await executor.openapi.configure(source, { + scope: org, + headers: { + Authorization: SecretId.make("example_org_token"), + }, +}); + +await executor.openapi.configure(source, { + scope: user, + headers: { + Authorization: SecretId.make("example_user_token"), + }, +}); +``` + +Resolution: + +```txt +User with user binding: + Authorization: Bearer + +User without user binding: + Authorization: Bearer +``` + +The source and tools remain org-scoped. Only values vary by scope. + +## OAuth Is Not Just A Header + +OAuth eventually produces an `Authorization` header, but the setup is not just +"set the Authorization header value." OAuth has distinct source shape and +credential value concerns: + +- The selected OpenAPI security scheme. +- OAuth flow. +- Authorization URL. +- Token URL. +- Issuer URL when relevant. +- Scopes. +- Client ID. +- Client secret. +- Connection ID. +- Token refresh lifecycle. + +The source should choose one OAuth configuration at import time. After that, +credential operations should not ask the caller to repeat the security scheme +name in the common case. + +## Current OpenAPI OAuth Model + +Today preview returns `oauth2Presets`, one per supported OAuth option derived +from the spec. The add UI chooses one preset and persists one +`OAuth2SourceConfig` on the source: + +```ts +oauth2: { + kind: "oauth2", + securitySchemeName: selectedOAuth2Preset.securitySchemeName, + flow: selectedOAuth2Preset.flow, + authorizationUrl: selectedOAuth2Preset.authorizationUrl, + tokenUrl: selectedOAuth2Preset.tokenUrl, + issuerUrl: selectedOAuth2Preset.issuerUrl ?? null, + clientIdSlot: oauth2ClientIdSlot(selectedOAuth2Preset.securitySchemeName), + clientSecretSlot: oauth2ClientSecretSlot(selectedOAuth2Preset.securitySchemeName), + connectionSlot: oauth2ConnectionSlot(selectedOAuth2Preset.securitySchemeName), + scopes: [...oauth2SelectedScopes], +} +``` + +Client ID and client secret are stored as scoped bindings: + +```ts +setBinding({ + slot: oauth2.clientIdSlot, + scope: org, + value: { + kind: "secret", + secretId: SecretId.make("linear_client_id"), + }, +}); + +setBinding({ + slot: oauth2.clientSecretSlot, + scope: org, + value: { + kind: "secret", + secretId: SecretId.make("linear_client_secret"), + }, +}); +``` + +The user connection is also stored as a scoped binding: + +```ts +setBinding({ + slot: oauth2.connectionSlot, + scope: user, + value: { + kind: "connection", + connectionId: ConnectionId.make("linear_user_connection"), + }, +}); +``` + +That underlying model is sound. The clunky part is exposing the slot names and +raw binding operations as the normal SDK path. + +## Proposed OAuth SDK Flow + +If the spec has exactly one supported OAuth option, `addSpec` can infer it. If +there are multiple supported OAuth options, `addSpec` should require an +import-time selection. + +Single OAuth option: + +```ts +const source = await executor.openapi.addSpec({ + scope: org, + name: "Linear", + namespace: "linear", + baseUrl: "https://api.linear.app", + spec: { + kind: "url", + url: "https://example.com/linear-openapi.json", + }, + oauth: true, +}); +``` + +Multiple OAuth options: + +```ts +const source = await executor.openapi.addSpec({ + scope: org, + name: "Linear", + namespace: "linear", + baseUrl: "https://api.linear.app", + spec: { + kind: "url", + url: "https://example.com/linear-openapi.json", + }, + oauth: { + securityScheme: "linearOAuth", + }, +}); +``` + +Configure app credentials: + +```ts +await executor.openapi.configure(source, { + scope: org, + oauth: { + clientId: SecretId.make("linear_client_id"), + clientSecret: SecretId.make("linear_client_secret"), + }, +}); +``` + +Connect a user: + +```ts +await executor.openapi.connect(source, { + scope: user, +}); +``` + +Client credentials flow should use the same public concepts: + +```ts +await executor.openapi.configure(source, { + scope: org, + oauth: { + clientId: SecretId.make("service_client_id"), + clientSecret: SecretId.make("service_client_secret"), + }, +}); + +await executor.openapi.connect(source, { + scope: org, +}); +``` + +Internally this still writes: + +```txt +oauth2::client-id +oauth2::client-secret +oauth2::connection +``` + +but SDK callers do not see those slots. + +## OAuth Validation + +`configure(source, { oauth: ... })` should fail if the source has no selected +OAuth config: + +```txt +OpenAPI source "stripe" does not declare OAuth credentials. +``` + +`connect(source, { scope })` should fail if the source has no selected OAuth +config: + +```txt +OpenAPI source "stripe" does not support OAuth connect. +``` + +`connect` should validate required client credentials before starting OAuth: + +```txt +Client ID must be configured before connecting. +Client secret must be configured before connecting. +``` + +For authorization-code flows, client secret may be optional for public PKCE +clients. For client-credentials flows, client secret is required. + +## Configure Should Be A Facade + +The public OpenAPI SDK can expose: + +```ts +executor.openapi.addSpec(...) +executor.openapi.configure(...) +executor.openapi.connect(...) +``` + +The lower-level core API can still expose generic bindings for advanced or UI +infrastructure cases: + +```ts +executor.sources.setBinding(...) +executor.sources.listBindings(...) +executor.sources.removeBinding(...) +executor.sources.getSlotManifest(...) +``` + +But the common OpenAPI SDK path should not require users to manually map: + +```txt +Authorization -> header:authorization +OAuth client ID -> oauth2::client-id +OAuth connection -> oauth2::connection +``` + +The SDK should derive that mapping from the selected source shape. + +## Design Rule + +`addSpec` defines what this source can use. + +`configure` assigns values to things this source already declared. + +`connect` creates or refreshes OAuth connection values for the selected OAuth +configuration. + +Bindings and slots are the internal storage/resolution mechanism behind those +operations. diff --git a/notes/source-binding-consolidation.md b/notes/source-binding-consolidation.md new file mode 100644 index 000000000..42c199c26 --- /dev/null +++ b/notes/source-binding-consolidation.md @@ -0,0 +1,209 @@ +# Source Binding Consolidation Notes + +Date: 2026-05-17 +Status: exploring + +## Context + +The branch `codex/fix-openapi-add-flow` shipped a focused cleanup of the +OpenAPI add-source API (see `openapi-add-source-api-cleanup.md`). Looking +across the OpenAPI, GraphQL, and MCP plugins while reviewing that work +turned up a much larger duplication problem. This note captures the +direction we're considering before drafting a real plan. + +## What's duplicated today + +Each of OpenAPI, GraphQL, and MCP independently reimplements the same +source-binding machinery on top of the core `credential_binding` table: + +- A plugin-specific `*SourceBindingInput` / `*SourceBindingRef` type that + is a thin re-skin of core `CredentialBindingInput` / `CredentialBindingRef`. +- A `resolve*SourceBinding` helper that filters bindings by + `slotKey === slot` and picks the innermost-visible binding whose scope + rank ≤ the source's owner-scope rank. Identical across plugins. +- A `list*SourceBindings` helper applying the same scope-ceiling filter. +- A `validate*BindingTarget` helper that re-checks the same outer-scope + rule core's `assertCredentialBindingTargetNotOuter` already enforces. +- A `coreBindingToXxxBinding` adapter that exists only because the + per-plugin Ref type is a re-skin. +- Per-plugin HTTP endpoints: `setSourceBinding`, `listSourceBindings`, + `removeSourceBinding`. Same payloads, same semantics, mounted under + different paths. +- A `canonicalizeCredentialMap` / `canonicalizeAuth` that splits an input + "shape with secret refs" into a stored shape (with binding sentinels) + plus a list of bindings to write. Same algorithm per plugin. +- React glue: each plugin re-wires `secret-header-auth`, + `credential-target-scope`, and `oauth-sign-in` into its own add flow. + +Storage-side, each plugin owns nearly-identical child tables: + +- OpenAPI: `openapi_source_header`, `openapi_source_query_param`, + `openapi_source_spec_fetch_header`, + `openapi_source_spec_fetch_query_param`. +- GraphQL: `graphql_source_header`, `graphql_source_query_param`. +- MCP: `mcp_source_header`, `mcp_source_query_param`. + +All eight tables share the exact same columns: +`(source_id, scope_id, name, kind, text_value, slot_key, prefix)`. +They're discriminated only by which logical compartment the plugin is +modeling. They're written and read wholesale per source (bulk delete + +bulk insert; bulk findMany). No query filters by `name`, `slot_key`, +`kind`, or `prefix`. No join uses them. The Drizzle mirrors in +`apps/cloud` and `apps/local` are the only non-store consumers. + +## The arbitrary-source-types constraint + +We expect to support source types beyond HTTP-ish protocols: CLI +sources (argv + env), database connection strings (templated URIs like +`mysql://user:{{password_slot}}@host/db`), and others we haven't named. + +That changes the design center of any shared abstraction: + +- The HTTP-flavored vocabulary baked into the current shared bits + (`header:`, `query_param:`, `prefix`) doesn't generalize. +- A DB source doesn't have headers; a CLI source doesn't have query + params. The notion of "compartment" is plugin-specific. +- A connection-string source needs interpolation, not just whole-field + substitution. `prefix` is a degenerate one-hole template; argv and + URIs want full templating. + +The only protocol-agnostic primitive is: *"named slot at +(plugin, source, source_scope, scope) resolving to a value of kind +text | secret | connection."* That's already what core +`credential_binding` provides. + +## Revised consolidation thesis + +Core should own the binding primitive, not just the storage. + +**Core owns:** + +- The `credential_binding` table (today). +- The resolver: given `(pluginId, sourceId, sourceScope, slot, scope)`, + return the innermost-visible binding subject to the source-scope + ceiling. Today this is duplicated per plugin. +- Validation: scope-stack membership, "binding target not outer than + source," secret/connection reachability. The facade already has the + inner checks; the plugin-side `validate*BindingTarget` helpers can go. +- HTTP endpoints for `setBinding` / `listBindings` / `removeBinding`, + parameterised by `pluginId` in the path (or payload). +- A small *slot manifest* concept: each plugin declares, for a given + source, which slot keys exist, their human-readable labels, expected + value kind, required/optional, and any rendering hints. The UI uses + this to render a credential editor for any plugin without bespoke + React code. + +**Plugins own:** + +- Their source-shape storage. Whatever's plugin-specific (OpenAPI: + spec/baseUrl/operations; GraphQL: endpoint/operations; MCP: + transport/config; DB: connection-string template; CLI: argv/env). + The nearly-identical child tables go away; shape moves to JSON on + the source row or whatever structural columns the plugin actually + needs. +- The wire-format glue: how a resolved slot value becomes an HTTP + header, an MCP transport handshake parameter, a DB connection + string, a CLI argv entry. +- Their slot manifest content. OpenAPI generates it from the parsed + spec; MCP from connect-time metadata; DB from the URI template's + holes. + +**Plugins do not own:** + +- Re-skinned binding types. +- Their own resolver / lister / validator. +- Their own `setSourceBinding` HTTP endpoint. +- Hand-written credential-editor React glue, beyond plugin-specific + manifest hints. + +## What this changes vs the current plan + +The existing `openapi-add-source-api-cleanup.md` proposes deferred +domain wrappers (`setHeaderValue`, `setQueryParamValue`, +`setOAuthClientCredentials`, `setOAuthConnection`). Under the +consolidated model those wrappers stop making sense — they pre-suppose +HTTP-ish compartments. The consumer-facing API becomes "set this slot +on this source," parameterised by a slot key the plugin's manifest +defines. UX affordances like header preset pickers move into the UI +layer, driven by the manifest. + +The plan note's "source shape vs values" split is still right; it just +applies more aggressively. Shape lives in plugin source config (JSON +or columns the plugin actually queries on); values live in core +bindings. + +## On `prefix` + +`prefix` is a UI/UX affordance for the `Authorization: Bearer ` +pattern, generalised to "any prepended literal string." It is stored +in three places (core `ConfiguredCredentialBinding`, +`ScopedSecretCredentialInput`, plugin child-table columns) and +consumed at exactly one site per plugin: + +```ts +resolved[name] = value.prefix ? `${value.prefix}${secret}` : secret; +``` + +No escaping, no suffix, no conditional logic. It exists so the secret +itself doesn't have to bake `Bearer ` into the value and so the UI can +render a separate prefix input next to the secret picker. + +It does not generalise to connection strings or argv. The right move +is to drop `prefix` from core entirely and either: + +1. Have it live in the plugin's slot manifest as a pure UI hint, with + the resolver still returning the raw secret value and the plugin + doing the concatenation in its wire-format layer; or +2. Subsume it into a plugin-owned template ("this slot's value is + interpolated into this string"), which is the same idea generalised + to multi-hole shapes. + +Either way the resolver stops doing string concatenation. + +## On the OpenAPI normalized child tables + +In the local-cleanup framing they look wrong because the same four +columns are duplicated four times. In the arbitrary-source-types +framing they look wrong for a deeper reason: they bake HTTP +compartment vocabulary into the schema. They should go away rather +than be consolidated. + +Two reasonable destinations: + +- **JSON on the source row.** Same place OAuth2 config already lives. + Access pattern (read all, replace all) already matches what a JSON + blob gives you. +- **Plugin-owned structural columns where they matter.** If a plugin + truly benefits from a normalized shape table for its own queries, + it can have one; OpenAPI/GraphQL/MCP currently don't. + +## Open questions + +1. **Third-party plugins.** Do we expect plugin authors outside this + repo? That sets the stability bar for the slot manifest contract + and the resolver API. +2. **Templating layer.** Is there one core mini-template language + (`{{slot}}`) that plugins opt into, or is interpolation strictly + plugin-owned? Shared templating gives consistent UX (preview the + resolved value in the UI, redact in logs uniformly); per-plugin + templating gives more freedom but reinvents. +3. **Migration order.** Sketch the core API first and prove it on + OpenAPI as the proving ground, then port GraphQL and MCP in + follow-ups. Each step independently mergeable. +4. **Existing PR.** Land or shelve `codex/fix-openapi-add-flow` as-is + before the bigger consolidation opens; do not swallow the focused + cleanup into the larger refactor. + +## Suggested next steps + +- Close out the existing PR. +- Draft a real plan note that specifies the core API surface (resolver + signature, manifest shape, HTTP endpoints) before touching code. +- Once the shape is agreed, port OpenAPI first: delete the four child + tables, move shape to source-row storage, replace the plugin-level + binding helpers with core calls, replace the plugin-level HTTP group + with a core one. +- Port GraphQL and MCP to match. +- Pick the first arbitrary source type (likely DB connection string or + CLI) as a forcing function to validate the abstraction works beyond + HTTP-ish shapes. From f389a0c3e30dff0315f8e8fa01826a902f19f7a3 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Sun, 17 May 2026 17:12:19 -0700 Subject: [PATCH 5/5] Fix OpenAPI add flow CI --- .../src/services/sources-api.node.test.ts | 12 ++++----- apps/marketing/src/pages/api/detect.ts | 4 ++- examples/all-plugins/src/main.ts | 3 ++- examples/docs-sdk-quickstart/src/main.ts | 7 ++++- examples/promise-sdk/src/main.ts | 7 ++++- notes/openapi-add-source-api-cleanup.md | 4 +-- notes/openapi-sdk-credential-configuration.md | 6 ++--- notes/source-binding-consolidation.md | 6 ++--- .../src/integration.test.ts | 8 +++++- packages/plugins/openapi/src/api/group.ts | 6 +---- packages/plugins/openapi/src/api/handlers.ts | 4 +-- .../openapi/src/react/AddOpenApiSource.tsx | 6 +---- .../plugins/openapi/src/sdk/plugin.test.ts | 2 +- packages/plugins/openapi/src/sdk/plugin.ts | 9 ++----- packages/plugins/openapi/src/testing/index.ts | 27 ++++++++++++++----- 15 files changed, 64 insertions(+), 47 deletions(-) diff --git a/apps/cloud/src/services/sources-api.node.test.ts b/apps/cloud/src/services/sources-api.node.test.ts index 662623b47..c471a71a8 100644 --- a/apps/cloud/src/services/sources-api.node.test.ts +++ b/apps/cloud/src/services/sources-api.node.test.ts @@ -159,12 +159,12 @@ describe("sources api (HTTP)", () => { const addResult = yield* asOrg(org, (client) => client.openapi.addSpec({ params: { scopeId }, - payload: { - spec: { kind: "blob", value: server.specJson }, - name: "Invocable Source API", - baseUrl: server.baseUrl, - namespace, - }, + payload: { + spec: { kind: "blob", value: server.specJson }, + name: "Invocable Source API", + baseUrl: server.baseUrl, + namespace, + }, }), ); expect(addResult).toEqual({ namespace, toolCount: 1 }); diff --git a/apps/marketing/src/pages/api/detect.ts b/apps/marketing/src/pages/api/detect.ts index 7f6b53019..f7e66c76a 100644 --- a/apps/marketing/src/pages/api/detect.ts +++ b/apps/marketing/src/pages/api/detect.ts @@ -85,8 +85,10 @@ export const POST: APIRoute = async ({ request }) => { // Add source to register its tools (Google Discovery needs auth so skip) if (match.kind === "openapi") { yield* executor.openapi.addSpec({ - spec: match.endpoint, + spec: { kind: "url", url: match.endpoint }, + name: match.name, namespace: match.namespace, + baseUrl: match.endpoint, scope: "test-scope", }); } else if (match.kind === "graphql") { diff --git a/examples/all-plugins/src/main.ts b/examples/all-plugins/src/main.ts index ad04186f2..b61e2f1fc 100644 --- a/examples/all-plugins/src/main.ts +++ b/examples/all-plugins/src/main.ts @@ -244,8 +244,9 @@ const program = Effect.gen(function* () { console.log("-".repeat(72)); const addSpecResult = yield* executor.openapi.addSpec({ - spec: exampleOpenApiSpec, + spec: { kind: "blob", value: exampleOpenApiSpec }, namespace: "example-api", + name: "Example API", baseUrl: "https://example.com/api", scope: "example-scope", }); diff --git a/examples/docs-sdk-quickstart/src/main.ts b/examples/docs-sdk-quickstart/src/main.ts index 2a54a4095..7415c94f8 100644 --- a/examples/docs-sdk-quickstart/src/main.ts +++ b/examples/docs-sdk-quickstart/src/main.ts @@ -81,7 +81,12 @@ const executor = await createExecutor({ await executor.openapi.addSpec({ namespace: "inventory", scope: "docs-workspace", - spec: JSON.stringify(inventoryApi), + name: "Inventory API", + baseUrl: "https://inventory.example.com", + spec: { + kind: "blob", + value: JSON.stringify(inventoryApi), + }, }); // docs:end add-source diff --git a/examples/promise-sdk/src/main.ts b/examples/promise-sdk/src/main.ts index 44a0aebac..55cffd6f8 100644 --- a/examples/promise-sdk/src/main.ts +++ b/examples/promise-sdk/src/main.ts @@ -44,9 +44,14 @@ await executor.mcp.addSource({ // --------------------------------------------------------------------------- await executor.openapi.addSpec({ - spec: "https://petstore3.swagger.io/api/v3/openapi.json", + spec: { + kind: "url", + url: "https://petstore3.swagger.io/api/v3/openapi.json", + }, namespace: "petstore", scope: "my-app", + name: "Petstore", + baseUrl: "https://petstore3.swagger.io/api/v3", }); // With auth headers (static or secret-backed) diff --git a/notes/openapi-add-source-api-cleanup.md b/notes/openapi-add-source-api-cleanup.md index c0fff676e..28871803f 100644 --- a/notes/openapi-add-source-api-cleanup.md +++ b/notes/openapi-add-source-api-cleanup.md @@ -68,9 +68,7 @@ Make the identity and request base explicit: Use a discriminated spec input instead of a string that guesses: ```ts -type OpenApiSpecInput = - | { kind: "url"; url: string } - | { kind: "blob"; value: string }; +type OpenApiSpecInput = { kind: "url"; url: string } | { kind: "blob"; value: string }; ``` Remove `credentialTargetScope` from the HTTP payload. Scope should always be diff --git a/notes/openapi-sdk-credential-configuration.md b/notes/openapi-sdk-credential-configuration.md index 785f1ee7f..baf8ceccf 100644 --- a/notes/openapi-sdk-credential-configuration.md +++ b/notes/openapi-sdk-credential-configuration.md @@ -83,9 +83,9 @@ await executor.openapi.configure(source, { The caller should not pass: ```ts -kind: "binding" -slot: "header:authorization" -credentialTargetScope: user +kind: "binding"; +slot: "header:authorization"; +credentialTargetScope: user; ``` Those are internal implementation details. diff --git a/notes/source-binding-consolidation.md b/notes/source-binding-consolidation.md index 42c199c26..a78322766 100644 --- a/notes/source-binding-consolidation.md +++ b/notes/source-binding-consolidation.md @@ -67,9 +67,9 @@ That changes the design center of any shared abstraction: substitution. `prefix` is a degenerate one-hole template; argv and URIs want full templating. -The only protocol-agnostic primitive is: *"named slot at +The only protocol-agnostic primitive is: _"named slot at (plugin, source, source_scope, scope) resolving to a value of kind -text | secret | connection."* That's already what core +text | secret | connection."_ That's already what core `credential_binding` provides. ## Revised consolidation thesis @@ -87,7 +87,7 @@ Core should own the binding primitive, not just the storage. inner checks; the plugin-side `validate*BindingTarget` helpers can go. - HTTP endpoints for `setBinding` / `listBindings` / `removeBinding`, parameterised by `pluginId` in the path (or payload). -- A small *slot manifest* concept: each plugin declares, for a given +- A small _slot manifest_ concept: each plugin declares, for a given source, which slot keys exist, their human-readable labels, expected value kind, required/optional, and any rendering hints. The UI uses this to render a credential editor for any plugin without bespoke diff --git a/packages/kernel/runtime-dynamic-worker/src/integration.test.ts b/packages/kernel/runtime-dynamic-worker/src/integration.test.ts index dd8e92599..7919629ee 100644 --- a/packages/kernel/runtime-dynamic-worker/src/integration.test.ts +++ b/packages/kernel/runtime-dynamic-worker/src/integration.test.ts @@ -219,7 +219,13 @@ const buildSandboxBridge = (spec: string, namespace: string, baseUrl = "https:// plugins, onElicitation: "accept-all", }); - yield* executor.openapi.addSpec({ spec, scope: TEST_SCOPE, namespace, baseUrl }); + yield* executor.openapi.addSpec({ + spec: { kind: "blob", value: spec }, + scope: TEST_SCOPE, + namespace, + name: namespace, + baseUrl, + }); const invoker = makeExecutorToolInvoker(executor, { invokeOptions: autoApprove }); return { executor, invoker, captured: recording.captured, sql }; }), diff --git a/packages/plugins/openapi/src/api/group.ts b/packages/plugins/openapi/src/api/group.ts index 51854ad1c..c38c2a201 100644 --- a/packages/plugins/openapi/src/api/group.ts +++ b/packages/plugins/openapi/src/api/group.ts @@ -1,10 +1,6 @@ import { HttpApiEndpoint, HttpApiGroup } from "effect/unstable/httpapi"; import { Schema } from "effect"; -import { - InternalError, - ScopeId, - SecretBackedValue, -} from "@executor-js/sdk/shared"; +import { InternalError, ScopeId, SecretBackedValue } from "@executor-js/sdk/shared"; import { OpenApiParseError, OpenApiExtractionError, OpenApiOAuthError } from "../sdk/errors"; import { SpecPreview } from "../sdk/preview"; diff --git a/packages/plugins/openapi/src/api/handlers.ts b/packages/plugins/openapi/src/api/handlers.ts index 8b5cfe0f8..53602c833 100644 --- a/packages/plugins/openapi/src/api/handlers.ts +++ b/packages/plugins/openapi/src/api/handlers.ts @@ -117,9 +117,7 @@ export const OpenApiHandlers = HttpApiBuilder.group(ExecutorApiWithOpenApi, "ope yield* ext.updateSource(path.namespace, payload.sourceScope, { name: payload.name, baseUrl: payload.baseUrl, - headers: payload.headers as - | Record - | undefined, + headers: payload.headers as Record | undefined, queryParams: payload.queryParams as | Record | undefined, diff --git a/packages/plugins/openapi/src/react/AddOpenApiSource.tsx b/packages/plugins/openapi/src/react/AddOpenApiSource.tsx index 481a910be..aa113ff3b 100644 --- a/packages/plugins/openapi/src/react/AddOpenApiSource.tsx +++ b/packages/plugins/openapi/src/react/AddOpenApiSource.tsx @@ -70,11 +70,7 @@ import { specFetchHeaderBindingSlot, specFetchQueryParamBindingSlot, } from "../sdk/source-contracts"; -import { - OAuth2SourceConfig, - OpenApiSourceBindingInput, - type ServerInfo, -} from "../sdk/types"; +import { OAuth2SourceConfig, OpenApiSourceBindingInput, type ServerInfo } from "../sdk/types"; import { expandServerUrlOptions } from "../sdk/openapi-utils"; export const OPENAPI_OAUTH_POPUP_NAME = "openapi-oauth"; diff --git a/packages/plugins/openapi/src/sdk/plugin.test.ts b/packages/plugins/openapi/src/sdk/plugin.test.ts index c735aa384..7410a89cf 100644 --- a/packages/plugins/openapi/src/sdk/plugin.test.ts +++ b/packages/plugins/openapi/src/sdk/plugin.test.ts @@ -298,7 +298,7 @@ describe("OpenAPI Plugin", () => { expect(schema).not.toBeNull(); expect(schema!.inputTypeScript).toContain("scope: string"); - expect(schema!.inputTypeScript).toContain("kind: \"url\""); + expect(schema!.inputTypeScript).toContain('kind: "url"'); expect( (schema!.inputSchema as { properties?: Record }).properties, ).not.toHaveProperty("credentialTargetScope"); diff --git a/packages/plugins/openapi/src/sdk/plugin.ts b/packages/plugins/openapi/src/sdk/plugin.ts index 039e7b162..76cf1f885 100644 --- a/packages/plugins/openapi/src/sdk/plugin.ts +++ b/packages/plugins/openapi/src/sdk/plugin.ts @@ -128,14 +128,9 @@ export type HeaderValue = HeaderValueValue; export type ConfiguredHeaderValue = ConfiguredHeaderValueValue; export type OpenApiOAuthInput = OAuth2SourceConfig; -export type OpenApiSpecInput = - | { readonly kind: "url"; readonly url: string } - | { readonly kind: "blob"; readonly value: string }; +export type OpenApiSpecInput = typeof OpenApiSpecInputSchema.Type; -export interface OpenApiSecretShapeInput { - readonly kind: "secret"; - readonly prefix?: string; -} +export type OpenApiSecretShapeInput = typeof OpenApiSecretShapeInputSchema.Type; export type OpenApiConfiguredValueInput = | string diff --git a/packages/plugins/openapi/src/testing/index.ts b/packages/plugins/openapi/src/testing/index.ts index 520455ab9..a0a332a67 100644 --- a/packages/plugins/openapi/src/testing/index.ts +++ b/packages/plugins/openapi/src/testing/index.ts @@ -100,10 +100,25 @@ export type OpenApiHttpApiTestSourceOptions = Omit< readonly transformSpec?: (spec: Record) => Record; }; +type OpenApiHttpApiAddSpecCredentialInput = + | string + | { + readonly kind: "secret"; + readonly prefix?: string; + }; + +type OpenApiHttpApiAddSpecCredentialsInput = { + readonly headers?: Record; + readonly queryParams?: Record; +}; + export type OpenApiHttpApiTestAddSpecPayloadOptions = Omit< OpenApiHttpApiTestSourceOptions, - "scope" ->; + "scope" | "headers" | "queryParams" | "specFetchCredentials" +> & + OpenApiHttpApiAddSpecCredentialsInput & { + readonly specFetchCredentials?: OpenApiHttpApiAddSpecCredentialsInput; + }; export type OpenApiTestSourceExecutor = { readonly openapi: Pick; @@ -183,11 +198,11 @@ export const makeOpenApiHttpApiTestAddSpecPayload = ( namespace: config.namespace, name: config.name, baseUrl: config.baseUrl, - ...(config.headers !== undefined ? { headers: config.headers } : {}), - ...(config.queryParams !== undefined ? { queryParams: config.queryParams } : {}), + ...(sourceOptions.headers !== undefined ? { headers: sourceOptions.headers } : {}), + ...(sourceOptions.queryParams !== undefined ? { queryParams: sourceOptions.queryParams } : {}), ...(config.oauth2 !== undefined ? { oauth2: config.oauth2 } : {}), - ...(config.specFetchCredentials !== undefined - ? { specFetchCredentials: config.specFetchCredentials } + ...(sourceOptions.specFetchCredentials !== undefined + ? { specFetchCredentials: sourceOptions.specFetchCredentials } : {}), }; };