History

Surface OpenAPI argument errors to the caller instead of redacting them (#1243)

* 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. * 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.

Rhys Sullivan committed 4 months ago

Commit e39dbcf1 parent fd16b2b10 files · +312 −18Browse files

Showing the changes to packages/plugins/openapi/src (5 of 10 files). Show all

Unified · Split

5 files
  1. packages/plugins/openapi/src/sdk/backing.ts +22 −1
  2. packages/plugins/openapi/src/sdk/index.ts +2 −1
  3. packages/plugins/openapi/src/sdk/invoke.ts +34 −12
  4. packages/plugins/openapi/src/sdk/plugin.test.ts +34 −0
  5. packages/plugins/openapi/src/sdk/plugin.ts +4 −0
packages/plugins/openapi/src/sdk/backing.ts+22−1
@@ -28,7 +28,7 @@ import { · 27 unmodified lines
2828 streamOperationBindingsFromStructure,
2929 } from "./extract";
3030 import { compileToolDefinitions, type ToolDefinition } from "./definitions";
31−import { annotationsForOperation, invokeWithLayer } from "./invoke";
31+import { annotationsForOperation, buildRequest, invokeWithLayer } from "./invoke";
3232 import { parse, type ParsedDocument } from "./parse";
3333 import { parseEntry, structuralSplit, type KeepPathItem, type SpecStructure } from "./split";
3434 import { type OpenapiStore, type StoredOperation } from "./store";
@@ -645,6 +645,27 @@ export const invokeOpenApiBackedTool = (input: { · 610 unmodified lines
645645 });
646646 });
647647
648+/** Pre-approval argument validation: run the exact request-building pass
649+ * `invoke` runs (path params, request body, base64 payloads) and discard the
650+ * built request. Fails with the same pre-flight OpenApiInvocationError the
651+ * real invocation would raise, so a call guaranteed to fail is rejected
652+ * before the executor pauses it for approval. Legacy rows without a
653+ * persisted binding skip validation (re-parsing the spec here would repeat
654+ * invokeTool's heavy fallback for no user-visible gain). */
655+export const validateOpenApiBackedToolArgs = (input: {
656+ readonly ctx: PluginCtx<OpenapiStore>;
657+ readonly toolRow: { readonly integration: string; readonly name: string };
658+ readonly args: unknown;
659+}) =>
660+ Effect.gen(function* () {
661+ const operation = yield* input.ctx.storage.getOperation(
662+ input.toolRow.integration,
663+ input.toolRow.name,
664+ );
665+ if (!operation) return;
666+ yield* buildRequest(operation.binding, (input.args ?? {}) as Record<string, unknown>, {});
667+ });
668+
648669 export const resolveOpenApiBackedAnnotations = (input: {
649670 readonly ctx: PluginCtx<OpenapiStore>;
650671 readonly integration: string;
packages/plugins/openapi/src/sdk/index.ts+2−1
@@ -12,7 +12,7 @@ export { · 11 unmodified lines
1212 type ByteRange,
1313 type KeepPathItem,
1414 } from "./split";
15−export { invoke, invokeWithLayer, annotationsForOperation } from "./invoke";
15+export { invoke, invokeWithLayer, buildRequest, annotationsForOperation } from "./invoke";
1616 export {
1717 buildDefsJsonStreaming,
1818 compileAndPersistOpenApiOperations,
@@ -28,6 +28,7 @@ export { · 9 unmodified lines
2828 openApiToolDefsFromCompiled,
2929 resolveOpenApiBackedAnnotations,
3030 resolveOpenApiBackedTools,
31+ validateOpenApiBackedToolArgs,
3132 type CompiledOpenApiSpec,
3233 type OpenApiPersistResult,
3334 } from "./backing";
packages/plugins/openapi/src/sdk/invoke.ts+34−12
@@ -643,25 +643,22 @@ const applyRequestBody = ( · 642 unmodified lines
643643 };
644644
645645 // ---------------------------------------------------------------------------
646−// Public API — invoke a single operation
646+// Public API — build the outbound request for an operation
647+//
648+// The full pre-flight phase of an invocation: read/validate caller args
649+// (path params, request body, base64 payloads) and construct the
650+// HttpClientRequest. Pure — nothing is sent. Exposed separately so the
651+// executor's approval flow can validate args BEFORE raising an approval
652+// elicitation, guaranteeing the validation semantics are exactly what
653+// `invoke` applies (one code path, not a parallel re-implementation).
647654 // ---------------------------------------------------------------------------
648655
649−export const invoke = Effect.fn("OpenApi.invoke")(function* (
656+export const buildRequest = Effect.fn("OpenApi.buildRequest")(function* (
650657 operation: OperationBinding,
651658 args: Record<string, unknown>,
652659 resolvedHeaders: Record<string, string>,
653660 sourceQueryParams: Record<string, string> = {},
654661 ) {
655− const client = yield* HttpClient.HttpClient;
656−
657− yield* Effect.annotateCurrentSpan({
658− "http.method": operation.method.toUpperCase(),
659− "http.route": operation.pathTemplate,
660− "plugin.openapi.method": operation.method.toUpperCase(),
661− "plugin.openapi.path_template": operation.pathTemplate,
662− "plugin.openapi.headers.resolved_count": Object.keys(resolvedHeaders).length,
663− });
664−
665662 const resolvedPath = yield* resolvePath(operation.pathTemplate, args, operation.parameters);
666663
667664 const path = resolvedPath.startsWith("/") ? resolvedPath : `/${resolvedPath}`;
@@ -808,6 +805,31 @@ export const invoke = Effect.fn("OpenApi.invoke")(function* ( · 140 unmodified lines
808805
809806 request = applyHeaders(request, resolvedHeaders);
810807
808+ return request;
809+});
810+
811+// ---------------------------------------------------------------------------
812+// Public API — invoke a single operation
813+// ---------------------------------------------------------------------------
814+
815+export const invoke = Effect.fn("OpenApi.invoke")(function* (
816+ operation: OperationBinding,
817+ args: Record<string, unknown>,
818+ resolvedHeaders: Record<string, string>,
819+ sourceQueryParams: Record<string, string> = {},
820+) {
821+ const client = yield* HttpClient.HttpClient;
822+
823+ yield* Effect.annotateCurrentSpan({
824+ "http.method": operation.method.toUpperCase(),
825+ "http.route": operation.pathTemplate,
826+ "plugin.openapi.method": operation.method.toUpperCase(),
827+ "plugin.openapi.path_template": operation.pathTemplate,
828+ "plugin.openapi.headers.resolved_count": Object.keys(resolvedHeaders).length,
829+ });
830+
831+ const request = yield* buildRequest(operation, args, resolvedHeaders, sourceQueryParams);
832+
811833 const response = yield* client.execute(request).pipe(
812834 Effect.mapError(
813835 (err) =>
packages/plugins/openapi/src/sdk/plugin.test.ts+34−0
@@ -565,6 +565,40 @@ describe("OpenAPI Plugin", () => { · 564 unmodified lines
565565 ),
566566 );
567567
568+ it.effect("rejects invalid args before raising the approval elicitation", () =>
569+ Effect.scoped(
570+ Effect.gen(function* () {
571+ const server = yield* servePluginTestApi();
572+ const executor = yield* createExecutor(makeTestConfig({ plugins: testPlugins() }));
573+
574+ const conn = yield* addOpenApiTestConnection(executor, server, { slug: "test" });
575+ const calls = { count: 0 };
576+ // createItem requires approval (POST) and a request body. Omitting
577+ // the body must fail pre-flight validation WITHOUT consuming an
578+ // approval: the user would otherwise approve a call that can only
579+ // fail.
580+ const failure = yield* executor
581+ .execute(
582+ conn.address("items.createItem"),
583+ {},
584+ {
585+ onElicitation: () =>
586+ Effect.sync(() => {
587+ calls.count++;
588+ return { action: "accept" as const, content: {} };
589+ }),
590+ },
591+ )
592+ .pipe(Effect.flip);
593+
594+ expect(calls.count).toBe(0);
595+ expect(Predicate.isTagged(failure, "ToolInvocationError")).toBe(true);
596+ // oxlint-disable-next-line executor/no-unknown-error-message -- boundary: asserts the exact caller-facing message the pre-flight failure carries
597+ expect((failure as { message: string }).message).toBe("Missing required request body");
598+ }),
599+ ),
600+ );
601+
568602 it.effect("describes OpenAPI invocation results payload-first with http meta beside data", () =>
569603 Effect.scoped(
570604 Effect.gen(function* () {
packages/plugins/openapi/src/sdk/plugin.ts+4−0
@@ -44,6 +44,7 @@ import { · 43 unmodified lines
4444 openApiStoredOperationsFromCompiled,
4545 resolveOpenApiBackedAnnotations,
4646 resolveOpenApiBackedTools,
47+ validateOpenApiBackedToolArgs,
4748 } from "./backing";
4849 import { resolveServerUrl } from "./openapi-utils";
4950
@@ -1031,6 +1032,9 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { · 981 unmodified lines
10311032 });
10321033 },
10331034
1035+ validateToolArgs: ({ ctx: validateCtx, toolRow, args }) =>
1036+ validateOpenApiBackedToolArgs({ ctx: validateCtx, toolRow, args }),
1037+
10341038 resolveAnnotations: ({ ctx: annotationsCtx, integration, toolRows }) =>
10351039 resolveOpenApiBackedAnnotations({
10361040 ctx: annotationsCtx,

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t