From b3123d6f4e73297503845703b00e021711c94d00 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Wed, 1 Jul 2026 15:14:22 -0700 Subject: [PATCH 1/2] Surface pre-flight OpenAPI invocation errors as expected tool failures Pre-flight OpenApiInvocationError raises (missing/unresolved path params, missing or malformed request body) happen before any HTTP request and their messages are generated locally from the spec, but the dispatcher only classified ToolNotFoundError, ToolBlockedError, and validation-issue ToolInvocationError as expected failures. These caller-argument errors fell into the defect-redaction branch, so the model saw an opaque "Internal tool error [id]" with no hint that the fix is renaming or adding an argument. Classify them as invalid_tool_arguments with the real message, keyed on the pre-flight shape: statusCode None and no cause. Transport failures (None + cause carrying the raw client error) and post-response failures (Some status) keep flowing through redaction. --- .../execution/src/tool-invoker.leak.test.ts | 143 +++++++++++++++++- packages/core/execution/src/tool-invoker.ts | 33 +++- 2 files changed, 174 insertions(+), 2 deletions(-) diff --git a/packages/core/execution/src/tool-invoker.leak.test.ts b/packages/core/execution/src/tool-invoker.leak.test.ts index 650728f22..a31c8c123 100644 --- a/packages/core/execution/src/tool-invoker.leak.test.ts +++ b/packages/core/execution/src/tool-invoker.leak.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "@effect/vitest"; -import { Data, Effect, Schema } from "effect"; +import { Data, Effect, Option, Schema } from "effect"; import { ElicitationResponse, createExecutor, definePlugin } from "@executor-js/sdk"; import { makeTestConfig } from "@executor-js/sdk/testing"; @@ -20,6 +20,17 @@ class FakePluginInvocationError extends Data.TaggedError("PluginInvocationError" readonly cause: unknown; }> {} +// Mirrors the openapi plugin's OpenApiInvocationError shape (the dispatcher +// matches it structurally by tag, not by class identity, since this package +// does not depend on the plugin). Pre-flight raises carry statusCode: None +// and no cause and must surface verbatim; transport/post-response failures +// must stay opaque. +class FakeOpenApiInvocationError extends Data.TaggedError("OpenApiInvocationError")<{ + readonly message: string; + readonly statusCode: Option.Option; + readonly cause?: unknown; +}> {} + const leakyPlugin = definePlugin(() => ({ id: "leaky-test" as const, storage: () => ({}), @@ -66,6 +77,59 @@ const leakyPlugin = definePlugin(() => ({ ), ), }, + { + name: "failsPreflightPathParam", + description: "", + inputSchema: EmptyInputSchema, + handler: () => + Effect.fail( + new FakeOpenApiInvocationError({ + message: "Missing required path parameter: datasourceId", + statusCode: Option.none(), + }), + ), + }, + { + name: "failsPreflightBody", + description: "", + inputSchema: EmptyInputSchema, + handler: () => + Effect.fail( + new FakeOpenApiInvocationError({ + message: "Missing required request body", + statusCode: Option.none(), + }), + ), + }, + { + name: "failsOpenApiTransport", + description: "", + inputSchema: EmptyInputSchema, + handler: () => + Effect.fail( + new FakeOpenApiInvocationError({ + message: "HTTP request failed", + statusCode: Option.none(), + cause: { + _tag: "RequestError", + request: { url: "https://internal.service.local/v1?token=tok-456" }, + }, + }), + ), + }, + { + name: "failsOpenApiPostResponse", + description: "", + inputSchema: EmptyInputSchema, + handler: () => + Effect.fail( + new FakeOpenApiInvocationError({ + message: "Failed to read response body", + statusCode: Option.some(502), + cause: { _tag: "ResponseError", note: "internal parser detail" }, + }), + ), + }, ], }, ], @@ -111,3 +175,80 @@ describe("internal-error leak audit (opaque defects)", () => { }), ); }); + +describe("openapi pre-flight invocation errors (expected failures)", () => { + it.effect("missing path parameter surfaces verbatim as invalid_tool_arguments", () => + Effect.gen(function* () { + const executor = yield* createExecutor(makeTestConfig({ plugins: [leakyPlugin()] as const })); + const invoker = makeExecutorToolInvoker(executor, { + invokeOptions: { onElicitation: acceptAll }, + }); + + const result = yield* invoker.invoke({ path: "leaky.failsPreflightPathParam", args: {} }); + expect(result).toEqual({ + ok: false, + error: { + code: "invalid_tool_arguments", + message: "Missing required path parameter: datasourceId", + }, + }); + }), + ); + + it.effect("missing request body surfaces verbatim as invalid_tool_arguments", () => + Effect.gen(function* () { + const executor = yield* createExecutor(makeTestConfig({ plugins: [leakyPlugin()] as const })); + const invoker = makeExecutorToolInvoker(executor, { + invokeOptions: { onElicitation: acceptAll }, + }); + + const result = yield* invoker.invoke({ path: "leaky.failsPreflightBody", args: {} }); + expect(result).toEqual({ + ok: false, + error: { + code: "invalid_tool_arguments", + message: "Missing required request body", + }, + }); + }), + ); + + it.effect("transport failure (statusCode None but cause present) stays opaque", () => + Effect.gen(function* () { + const executor = yield* createExecutor(makeTestConfig({ plugins: [leakyPlugin()] as const })); + const invoker = makeExecutorToolInvoker(executor, { + invokeOptions: { onElicitation: acceptAll }, + }); + + const err = yield* Effect.flip( + invoker.invoke({ path: "leaky.failsOpenApiTransport", args: {} }), + ); + expect(err).toBeInstanceOf(ExecutionToolError); + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: leak test inspects the rendered message to assert it is the opaque generic + const msg = (err as { message: string }).message; + expect(msg).toMatch(/^Internal tool error \[[0-9a-f]{8}\]$/); + expect(msg).not.toContain("internal.service.local"); + expect(msg).not.toContain("tok-456"); + expect(msg).not.toContain("HTTP request failed"); + }), + ); + + it.effect("post-response failure (statusCode Some) stays opaque", () => + Effect.gen(function* () { + const executor = yield* createExecutor(makeTestConfig({ plugins: [leakyPlugin()] as const })); + const invoker = makeExecutorToolInvoker(executor, { + invokeOptions: { onElicitation: acceptAll }, + }); + + const err = yield* Effect.flip( + invoker.invoke({ path: "leaky.failsOpenApiPostResponse", args: {} }), + ); + expect(err).toBeInstanceOf(ExecutionToolError); + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: leak test inspects the rendered message to assert it is the opaque generic + const msg = (err as { message: string }).message; + expect(msg).toMatch(/^Internal tool error \[[0-9a-f]{8}\]$/); + expect(msg).not.toContain("Failed to read response body"); + expect(msg).not.toContain("internal parser detail"); + }), + ); +}); diff --git a/packages/core/execution/src/tool-invoker.ts b/packages/core/execution/src/tool-invoker.ts index 8ce6d31bc..be6105d6d 100644 --- a/packages/core/execution/src/tool-invoker.ts +++ b/packages/core/execution/src/tool-invoker.ts @@ -148,6 +148,29 @@ const validationIssues = (value: unknown): readonly unknown[] | null => { return Array.isArray(issues) ? issues : null; }; +// Pre-flight OpenAPI invocation failures (missing/unresolved path params, +// missing or malformed request body) are caller-argument errors raised before +// any HTTP request is sent. Their messages are generated locally from the +// spec (no upstream data, no transport internals), so they are safe to +// surface, and actionable: the fix is renaming or adding an argument. +// Discriminator: pre-flight raises carry `statusCode: None` and no `cause`. +// Transport failures share `statusCode: None` but wrap the raw client error +// (which can hold internal URLs) as `cause`, and post-response failures carry +// a status code; both stay on the opaque-defect path. Matched structurally +// because this package does not depend on the openapi plugin. +const openApiPreflightMessage = (value: unknown): string | null => { + if (!Predicate.isTagged(value, "OpenApiInvocationError")) return null; + const err = value as { + readonly message?: unknown; + readonly statusCode?: unknown; + readonly cause?: unknown; + }; + if (err.cause !== undefined) return null; + if (!Predicate.isTagged(err.statusCode, "None")) return null; + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: structural match on the openapi plugin's tagged error; the pre-flight message is locally generated (no upstream data) and is the payload being surfaced + return typeof err.message === "string" && err.message.length > 0 ? err.message : null; +}; + const credentialResolutionToolFailure = (input: { readonly label: string; readonly message: string; @@ -186,7 +209,8 @@ const expectedToolFailure = (value: unknown): ToolError | null => { }; } if (Predicate.isTagged(value, "ToolInvocationError")) { - const issues = validationIssues((value as { readonly cause?: unknown }).cause); + const cause = (value as { readonly cause?: unknown }).cause; + const issues = validationIssues(cause); if (issues) { return { code: "invalid_tool_arguments", @@ -194,6 +218,13 @@ const expectedToolFailure = (value: unknown): ToolError | null => { details: { issues }, }; } + const preflight = openApiPreflightMessage(cause); + if (preflight) { + return { + code: "invalid_tool_arguments", + message: preflight, + }; + } } return null; }; From 6e78677a8661351948c1aed51f5cab6376f74832 Mon Sep 17 00:00:00 2001 From: Rhys Sullivan <39114868+RhysSullivan@users.noreply.github.com> Date: Wed, 1 Jul 2026 16:09:05 -0700 Subject: [PATCH 2/2] Validate OpenAPI args before pausing for approval Approval elicitation ran before any argument validation, so a call with a missing required path parameter or request body paused for user approval and then failed on the exact pre-flight check the approval could never satisfy (observed with queryCreate: approved, then "Missing required request body"). Add an optional validateToolArgs plugin hook that core calls only when the call is about to pause for approval (same predicate enforceApproval applies, now shared as approvalRequired). The openapi plugin implements it by running the request-building pass extracted from invoke (buildRequest: path params, request body, base64 payloads) and discarding the built request, so the validation semantics are exactly the ones invocation applies rather than a parallel re-implementation. Legacy rows without a persisted binding skip validation instead of re-parsing the spec. --- packages/core/sdk/src/executor.ts | 24 +++++++++- packages/core/sdk/src/index.ts | 1 + packages/core/sdk/src/plugin.ts | 19 ++++++++ packages/plugins/openapi/src/sdk/backing.ts | 23 +++++++++- packages/plugins/openapi/src/sdk/index.ts | 3 +- packages/plugins/openapi/src/sdk/invoke.ts | 46 ++++++++++++++----- .../plugins/openapi/src/sdk/plugin.test.ts | 34 ++++++++++++++ packages/plugins/openapi/src/sdk/plugin.ts | 4 ++ 8 files changed, 138 insertions(+), 16 deletions(-) diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 5dfd80b19..530462a8a 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -2935,6 +2935,17 @@ export const createExecutor = { + if (policy.action === "approve") return false; + return policy.action === "require_approval" || annotations?.requiresApproval === true; + }; + const enforceApproval = ( annotations: ToolAnnotations | undefined, address: ToolAddress, @@ -2943,9 +2954,8 @@ export const createExecutor = Effect.gen(function* () { - if (policy.action === "approve") return; + if (!approvalRequired(annotations, policy)) return; const policyForcesApproval = policy.action === "require_approval"; - if (!policyForcesApproval && !annotations?.requiresApproval) return; const message = annotations?.approvalDescription ? annotations.approvalDescription : policyForcesApproval && policy.pattern @@ -3154,6 +3164,16 @@ export const createExecutor = { readonly elicit: Elicit; } +/** Input for `validateToolArgs` — no credential/elicit: validation runs + * before approval enforcement and credential resolution, so it must depend + * on nothing but the tool row and the caller's args. */ +export interface ValidateToolArgsInput { + readonly ctx: PluginCtx; + readonly toolRow: ToolInvocationRow; + readonly args: unknown; +} + /** Called when the executor removes / refreshes a connection owned by this * plugin's integration — plugin-side cleanup or re-resolution only; the * executor handles the core tool rows. */ @@ -522,6 +531,16 @@ export interface PluginSpec< * address. The plugin applies `input.credential` to the outbound request. */ readonly invokeTool?: (input: InvokeToolInput) => Effect.Effect; + /** Validate a dynamic tool's args before the executor enforces approval, + * so a call guaranteed to fail is rejected instead of pausing for an + * approval the user can only waste. Fail with the same error `invokeTool` + * would raise for those args; succeed (void) when the args would reach the + * wire. Must be side-effect free: no credential use, no elicitation, no + * outbound request. Omit to skip pre-approval validation. */ + readonly validateToolArgs?: ( + input: ValidateToolArgsInput, + ) => Effect.Effect; + /** Bulk resolve annotations for a set of tool rows under one connection. */ readonly resolveAnnotations?: (input: { readonly ctx: PluginCtx; diff --git a/packages/plugins/openapi/src/sdk/backing.ts b/packages/plugins/openapi/src/sdk/backing.ts index 0018671e9..63ed632b6 100644 --- a/packages/plugins/openapi/src/sdk/backing.ts +++ b/packages/plugins/openapi/src/sdk/backing.ts @@ -28,7 +28,7 @@ import { streamOperationBindingsFromStructure, } from "./extract"; import { compileToolDefinitions, type ToolDefinition } from "./definitions"; -import { annotationsForOperation, invokeWithLayer } from "./invoke"; +import { annotationsForOperation, buildRequest, invokeWithLayer } from "./invoke"; import { parse, type ParsedDocument } from "./parse"; import { parseEntry, structuralSplit, type KeepPathItem, type SpecStructure } from "./split"; import { type OpenapiStore, type StoredOperation } from "./store"; @@ -645,6 +645,27 @@ export const invokeOpenApiBackedTool = (input: { }); }); +/** Pre-approval argument validation: run the exact request-building pass + * `invoke` runs (path params, request body, base64 payloads) and discard the + * built request. Fails with the same pre-flight OpenApiInvocationError the + * real invocation would raise, so a call guaranteed to fail is rejected + * before the executor pauses it for approval. Legacy rows without a + * persisted binding skip validation (re-parsing the spec here would repeat + * invokeTool's heavy fallback for no user-visible gain). */ +export const validateOpenApiBackedToolArgs = (input: { + readonly ctx: PluginCtx; + readonly toolRow: { readonly integration: string; readonly name: string }; + readonly args: unknown; +}) => + Effect.gen(function* () { + const operation = yield* input.ctx.storage.getOperation( + input.toolRow.integration, + input.toolRow.name, + ); + if (!operation) return; + yield* buildRequest(operation.binding, (input.args ?? {}) as Record, {}); + }); + export const resolveOpenApiBackedAnnotations = (input: { readonly ctx: PluginCtx; readonly integration: string; diff --git a/packages/plugins/openapi/src/sdk/index.ts b/packages/plugins/openapi/src/sdk/index.ts index ea9ef5041..f2657a10d 100644 --- a/packages/plugins/openapi/src/sdk/index.ts +++ b/packages/plugins/openapi/src/sdk/index.ts @@ -12,7 +12,7 @@ export { type ByteRange, type KeepPathItem, } from "./split"; -export { invoke, invokeWithLayer, annotationsForOperation } from "./invoke"; +export { invoke, invokeWithLayer, buildRequest, annotationsForOperation } from "./invoke"; export { buildDefsJsonStreaming, compileAndPersistOpenApiOperations, @@ -28,6 +28,7 @@ export { openApiToolDefsFromCompiled, resolveOpenApiBackedAnnotations, resolveOpenApiBackedTools, + validateOpenApiBackedToolArgs, type CompiledOpenApiSpec, type OpenApiPersistResult, } from "./backing"; diff --git a/packages/plugins/openapi/src/sdk/invoke.ts b/packages/plugins/openapi/src/sdk/invoke.ts index bf2d0c5dd..0ba054d53 100644 --- a/packages/plugins/openapi/src/sdk/invoke.ts +++ b/packages/plugins/openapi/src/sdk/invoke.ts @@ -643,25 +643,22 @@ const applyRequestBody = ( }; // --------------------------------------------------------------------------- -// Public API — invoke a single operation +// Public API — build the outbound request for an operation +// +// The full pre-flight phase of an invocation: read/validate caller args +// (path params, request body, base64 payloads) and construct the +// HttpClientRequest. Pure — nothing is sent. Exposed separately so the +// executor's approval flow can validate args BEFORE raising an approval +// elicitation, guaranteeing the validation semantics are exactly what +// `invoke` applies (one code path, not a parallel re-implementation). // --------------------------------------------------------------------------- -export const invoke = Effect.fn("OpenApi.invoke")(function* ( +export const buildRequest = Effect.fn("OpenApi.buildRequest")(function* ( operation: OperationBinding, args: Record, resolvedHeaders: Record, sourceQueryParams: Record = {}, ) { - const client = yield* HttpClient.HttpClient; - - yield* Effect.annotateCurrentSpan({ - "http.method": operation.method.toUpperCase(), - "http.route": operation.pathTemplate, - "plugin.openapi.method": operation.method.toUpperCase(), - "plugin.openapi.path_template": operation.pathTemplate, - "plugin.openapi.headers.resolved_count": Object.keys(resolvedHeaders).length, - }); - const resolvedPath = yield* resolvePath(operation.pathTemplate, args, operation.parameters); const path = resolvedPath.startsWith("/") ? resolvedPath : `/${resolvedPath}`; @@ -808,6 +805,31 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* ( request = applyHeaders(request, resolvedHeaders); + return request; +}); + +// --------------------------------------------------------------------------- +// Public API — invoke a single operation +// --------------------------------------------------------------------------- + +export const invoke = Effect.fn("OpenApi.invoke")(function* ( + operation: OperationBinding, + args: Record, + resolvedHeaders: Record, + sourceQueryParams: Record = {}, +) { + const client = yield* HttpClient.HttpClient; + + yield* Effect.annotateCurrentSpan({ + "http.method": operation.method.toUpperCase(), + "http.route": operation.pathTemplate, + "plugin.openapi.method": operation.method.toUpperCase(), + "plugin.openapi.path_template": operation.pathTemplate, + "plugin.openapi.headers.resolved_count": Object.keys(resolvedHeaders).length, + }); + + const request = yield* buildRequest(operation, args, resolvedHeaders, sourceQueryParams); + const response = yield* client.execute(request).pipe( Effect.mapError( (err) => diff --git a/packages/plugins/openapi/src/sdk/plugin.test.ts b/packages/plugins/openapi/src/sdk/plugin.test.ts index 05519ce2e..5829c648a 100644 --- a/packages/plugins/openapi/src/sdk/plugin.test.ts +++ b/packages/plugins/openapi/src/sdk/plugin.test.ts @@ -565,6 +565,40 @@ describe("OpenAPI Plugin", () => { ), ); + it.effect("rejects invalid args before raising the approval elicitation", () => + Effect.scoped( + Effect.gen(function* () { + const server = yield* servePluginTestApi(); + const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() })); + + const conn = yield* addOpenApiTestConnection(executor, server, { slug: "test" }); + const calls = { count: 0 }; + // createItem requires approval (POST) and a request body. Omitting + // the body must fail pre-flight validation WITHOUT consuming an + // approval: the user would otherwise approve a call that can only + // fail. + const failure = yield* executor + .execute( + conn.address("items.createItem"), + {}, + { + onElicitation: () => + Effect.sync(() => { + calls.count++; + return { action: "accept" as const, content: {} }; + }), + }, + ) + .pipe(Effect.flip); + + expect(calls.count).toBe(0); + expect(Predicate.isTagged(failure, "ToolInvocationError")).toBe(true); + // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: asserts the exact caller-facing message the pre-flight failure carries + expect((failure as { message: string }).message).toBe("Missing required request body"); + }), + ), + ); + it.effect("describes OpenAPI invocation results payload-first with http meta beside data", () => Effect.scoped( Effect.gen(function* () { diff --git a/packages/plugins/openapi/src/sdk/plugin.ts b/packages/plugins/openapi/src/sdk/plugin.ts index a6de92ddd..d5bd42e09 100644 --- a/packages/plugins/openapi/src/sdk/plugin.ts +++ b/packages/plugins/openapi/src/sdk/plugin.ts @@ -44,6 +44,7 @@ import { openApiStoredOperationsFromCompiled, resolveOpenApiBackedAnnotations, resolveOpenApiBackedTools, + validateOpenApiBackedToolArgs, } from "./backing"; import { resolveServerUrl } from "./openapi-utils"; @@ -1031,6 +1032,9 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { }); }, + validateToolArgs: ({ ctx: validateCtx, toolRow, args }) => + validateOpenApiBackedToolArgs({ ctx: validateCtx, toolRow, args }), + resolveAnnotations: ({ ctx: annotationsCtx, integration, toolRows }) => resolveOpenApiBackedAnnotations({ ctx: annotationsCtx,