Surface OpenAPI argument errors to the caller instead of redacting them

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<number>;
+  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");
+    }),
+  );
+});

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,14 +209,22 @@ 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",
         message: "Tool arguments did not match the input schema.",
         details: { issues },
       };
     }
+    const preflight = openApiPreflightMessage(cause);
+    if (preflight) {
+      return {
+        code: "invalid_tool_arguments",
+        message: preflight,
+      };
+    }
   }
   return null;
 };

packages/core/sdk/src/executor.ts

@@ -2935,6 +2935,17 @@ export const createExecutor = <const TPlugins extends readonly AnyPlugin[] = rea
         });
     };
 
+    // The single source of truth for "will enforceApproval pause this call".
+    // Read before pre-approval arg validation so the extra validation pass
+    // only runs for calls that would otherwise burn a user approval.
+    const approvalRequired = (
+      annotations: ToolAnnotations | undefined,
+      policy: EffectivePolicy,
+    ): boolean => {
+      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 = <const TPlugins extends readonly AnyPlugin[] = rea
       handler: ElicitationHandler,
     ) =>
       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 = <const TPlugins extends readonly AnyPlugin[] = rea
             .pipe(wrapInvocationError);
           resolvedAnnotations = map[String(parsed.tool)] ?? annotations;
         }
+        // When this call is about to pause for approval, validate args
+        // first: a call that can only fail (missing required path param /
+        // body) must be rejected here, not after the user grants an approval
+        // that then goes to waste. Non-pausing calls skip this — invokeTool
+        // raises the identical failure moments later without the extra pass.
+        if (approvalRequired(resolvedAnnotations, policy) && runtime.plugin.validateToolArgs) {
+          yield* runtime.plugin
+            .validateToolArgs({ ctx: runtime.ctx, toolRow: row, args })
+            .pipe(wrapInvocationError);
+        }
         yield* enforceApproval(resolvedAnnotations, address, args, policy, handler);
 
         // Resolve every named credential input (`variable → value`); `value` is

packages/core/sdk/src/index.ts

@@ -288,6 +288,7 @@ export {
   type StaticToolInput,
   type ConfigureIntegrationHandlerInput,
   type InvokeToolInput,
+  type ValidateToolArgsInput,
   type ConnectionLifecycleInput,
   type IntegrationConfigureDecl,
   type IntegrationConfigureSchema,

packages/core/sdk/src/plugin.ts

@@ -406,6 +406,15 @@ export interface InvokeToolInput<TStore = unknown> {
   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<TStore = unknown> {
+  readonly ctx: PluginCtx<TStore>;
+  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<TStore>) => Effect.Effect<unknown, unknown>;
 
+  /** 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<TStore>,
+  ) => Effect.Effect<void, unknown>;
+
   /** Bulk resolve annotations for a set of tool rows under one connection. */
   readonly resolveAnnotations?: (input: {
     readonly ctx: PluginCtx<TStore>;

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<OpenapiStore>;
+  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<string, unknown>, {});
+  });
+
 export const resolveOpenApiBackedAnnotations = (input: {
   readonly ctx: PluginCtx<OpenapiStore>;
   readonly integration: string;

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";