diff --git a/apps/cloud/src/services/sources-api.node.test.ts b/apps/cloud/src/services/sources-api.node.test.ts index 0191f9a99..5458fed09 100644 --- a/apps/cloud/src/services/sources-api.node.test.ts +++ b/apps/cloud/src/services/sources-api.node.test.ts @@ -207,9 +207,15 @@ describe("sources api (HTTP)", () => { expect(execution.structured).toMatchObject({ status: "completed", result: { - message: "hello", - suffix: "world", - path: "/echo/hello", + ok: true, + data: { + status: 200, + data: { + message: "hello", + suffix: "world", + path: "/echo/hello", + }, + }, }, logs: [], }); @@ -315,7 +321,7 @@ describe("sources api (HTTP)", () => { expect(execution.isError).toBe(false); expect(execution.structured).toMatchObject({ status: "completed", - result: { hello: "Hello Ada" }, + result: { ok: true, data: { hello: "Hello Ada" } }, }); const requests = yield* server.requests; expect(requests.some((request) => request.payload.query?.includes("__schema"))).toBe(true); @@ -391,7 +397,8 @@ describe("sources api (HTTP)", () => { expect(execution.structured).toMatchObject({ status: "completed", result: { - content: [{ type: "text", text: "cloud-mcp-ok" }], + ok: true, + data: { content: [{ type: "text", text: "cloud-mcp-ok" }] }, }, }); expect((yield* server.requests).length).toBeGreaterThanOrEqual(2); diff --git a/apps/local/src/server/migrate-oauth-connections.test.ts b/apps/local/src/server/migrate-oauth-connections.test.ts index 7c35a9d7c..795467d5e 100644 --- a/apps/local/src/server/migrate-oauth-connections.test.ts +++ b/apps/local/src/server/migrate-oauth-connections.test.ts @@ -159,7 +159,7 @@ describe("0009_repair_openapi_oauth_cutover_residue", () => { `); db.prepare("INSERT INTO `openapi_source` (id, scope_id, oauth2) VALUES (?, ?, ?)").run( - "dealcloud_api", + "example_api", "org-1", JSON.stringify({ kind: "oauth2", @@ -174,27 +174,27 @@ describe("0009_repair_openapi_oauth_cutover_residue", () => { "INSERT INTO `connection` (id, scope_id, provider, provider_state, scope, updated_at) VALUES (?, ?, ?, ?, ?, ?)", ); insertConnection.run( - "openapi-oauth2-app-dealcloud_api", + "openapi-oauth2-app-example_api", "org-1", "oauth2", JSON.stringify({ kind: "client-credentials", - tokenEndpoint: "https://resolve.dealcloud.com/oauth/token", - clientIdSecretId: "dealcloud-client-id", - clientSecretSecretId: "dealcloud-client-secret", + tokenEndpoint: "https://auth.example.test/oauth/token", + clientIdSecretId: "example-client-id", + clientSecretSecretId: "example-client-secret", }), null, now, ); insertConnection.run( - "openapi-oauth2-app-dealcloud_api", + "openapi-oauth2-app-example_api", "user-org:user-jd:org-1", "openapi:oauth2", JSON.stringify({ kind: "client-credentials", - tokenEndpoint: "https://resolve.dealcloud.com/oauth/token", - clientIdSecretId: "dealcloud-client-id-jd", - clientSecretSecretId: "dealcloud-client-secret-jd", + tokenEndpoint: "https://auth.example.test/oauth/token", + clientIdSecretId: "example-client-id-jd", + clientSecretSecretId: "example-client-secret-jd", }), null, now, @@ -207,12 +207,12 @@ describe("0009_repair_openapi_oauth_cutover_residue", () => { "org-client-id", "org-1", "openapi", - "dealcloud_api", + "example_api", "org-1", "oauth2:oauth2:client-id", "secret", null, - "dealcloud-client-id-jd", + "example-client-id-jd", null, now, now, @@ -221,12 +221,12 @@ describe("0009_repair_openapi_oauth_cutover_residue", () => { "org-client-secret", "org-1", "openapi", - "dealcloud_api", + "example_api", "org-1", "oauth2:oauth2:client-secret", "secret", null, - "dealcloud-client-secret-jd", + "example-client-secret-jd", null, now, now, @@ -235,13 +235,13 @@ describe("0009_repair_openapi_oauth_cutover_residue", () => { "org-connection", "org-1", "openapi", - "dealcloud_api", + "example_api", "org-1", "oauth2:oauth2:connection", "connection", null, null, - "openapi-oauth2-app-dealcloud_api", + "openapi-oauth2-app-example_api", now, now, ); @@ -249,13 +249,13 @@ describe("0009_repair_openapi_oauth_cutover_residue", () => { "jd-connection", "user-org:user-jd:org-1", "openapi", - "dealcloud_api", + "example_api", "org-1", "oauth2:oauth2:connection", "connection", null, null, - "openapi-oauth2-app-dealcloud_api", + "openapi-oauth2-app-example_api", now, now, ); @@ -271,20 +271,20 @@ describe("0009_repair_openapi_oauth_cutover_residue", () => { .prepare( "SELECT scope_id, slot_key, kind, secret_id, connection_id FROM `credential_binding` WHERE source_id = ? ORDER BY scope_id, slot_key", ) - .all("dealcloud_api"); + .all("example_api"); expect(bindings).toEqual([ { scope_id: "org-1", slot_key: "oauth2:oauth2:client-id", kind: "secret", - secret_id: "dealcloud-client-id", + secret_id: "example-client-id", connection_id: null, }, { scope_id: "org-1", slot_key: "oauth2:oauth2:client-secret", kind: "secret", - secret_id: "dealcloud-client-secret", + secret_id: "example-client-secret", connection_id: null, }, { @@ -292,20 +292,20 @@ describe("0009_repair_openapi_oauth_cutover_residue", () => { slot_key: "oauth2:oauth2:connection", kind: "connection", secret_id: null, - connection_id: "openapi-oauth2-app-dealcloud_api", + connection_id: "openapi-oauth2-app-example_api", }, { scope_id: "user-org:user-jd:org-1", slot_key: "oauth2:oauth2:client-id", kind: "secret", - secret_id: "dealcloud-client-id-jd", + secret_id: "example-client-id-jd", connection_id: null, }, { scope_id: "user-org:user-jd:org-1", slot_key: "oauth2:oauth2:client-secret", kind: "secret", - secret_id: "dealcloud-client-secret-jd", + secret_id: "example-client-secret-jd", connection_id: null, }, { @@ -313,7 +313,7 @@ describe("0009_repair_openapi_oauth_cutover_residue", () => { slot_key: "oauth2:oauth2:connection", kind: "connection", secret_id: null, - connection_id: "openapi-oauth2-app-dealcloud_api", + connection_id: "openapi-oauth2-app-example_api", }, ]); }); diff --git a/packages/core/execution/src/description.ts b/packages/core/execution/src/description.ts index fdf05a4fd..97ad689ff 100644 --- a/packages/core/execution/src/description.ts +++ b/packages/core/execution/src/description.ts @@ -48,6 +48,7 @@ const formatDescription = (sources: readonly Source[]): string => { "- `tools.search()` returns paginated, ranked matches: `{ items, total, hasMore, nextOffset }`. Best-first. Use short intent phrases like `github issues`, `repo details`, or `create calendar event`.", '- When you already know the namespace, narrow with `tools.search({ namespace: "github", query: "issues" })`.', "- `tools.executor.sources.list()` returns the same paged shape: `{ items: [{ id, toolCount, ... }], total, hasMore, nextOffset }`.", + "- Tool calls return a value union: `{ ok: true, data }` for success or `{ ok: false, error: { code, message, status?, details?, retryable? } }` for expected tool/domain failures. Branch on `result.ok`.", "- If `hasMore` is true and you didn't find what you need, fetch the next page: `tools.search({ query, offset: nextOffset, limit })`. Same `offset` parameter on `tools.executor.sources.list({ offset, limit })`.", "- Always use the namespace prefix when calling tools: `tools..(args)`. Example: `tools.home_assistant_rest_api.states.getState(...)` — not `tools.states.getState(...)`.", "- The `tools` object is a lazy proxy — `Object.keys(tools)` won't work. Use `tools.search()` or `tools.executor.sources.list()` instead.", diff --git a/packages/core/execution/src/tool-invoker.leak.test.ts b/packages/core/execution/src/tool-invoker.leak.test.ts new file mode 100644 index 000000000..650728f22 --- /dev/null +++ b/packages/core/execution/src/tool-invoker.leak.test.ts @@ -0,0 +1,113 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Data, Effect, Schema } from "effect"; + +import { ElicitationResponse, createExecutor, definePlugin } from "@executor-js/sdk"; +import { makeTestConfig } from "@executor-js/sdk/testing"; +import { ExecutionToolError } from "./errors"; +import { makeExecutorToolInvoker } from "./tool-invoker"; + +const EmptyInputSchema = Schema.toStandardSchemaV1( + Schema.toStandardJSONSchemaV1(Schema.Struct({})), +); + +const acceptAll = () => Effect.succeed(ElicitationResponse.make({ action: "accept" })); + +// Plugin-internal tagged error whose `cause` carries internal diagnostics. +// The dispatcher must route this through the opaque-generic path so none of +// that context reaches the sandbox via Error.message. +class FakePluginInvocationError extends Data.TaggedError("PluginInvocationError")<{ + readonly message: string; + readonly cause: unknown; +}> {} + +const leakyPlugin = definePlugin(() => ({ + id: "leaky-test" as const, + storage: () => ({}), + staticSources: () => [ + { + id: "leaky", + kind: "in-memory", + name: "Leaky", + tools: [ + { + name: "failsWithCause", + description: "", + inputSchema: EmptyInputSchema, + handler: () => + Effect.fail( + new FakePluginInvocationError({ + message: "Upstream request failed", + cause: { + _tag: "InternalTransportError", + request: { + method: "GET", + url: "https://internal.service.local/v1/resources?trace=trace-123", + headers: { "x-internal-routing": "private-cluster" }, + }, + stack: "Error: connect failed\n at plugin-transport.ts:42:11", + note: "internal diagnostic detail", + }, + }), + ), + }, + { + name: "throwsRawError", + description: "", + inputSchema: EmptyInputSchema, + handler: () => + Effect.fail( + Object.assign( + // oxlint-disable-next-line executor/no-error-constructor -- boundary: leak test deliberately fails with a raw Error + crafted stack to assert the dispatcher's opaque-generic redaction + new Error("Internal: secret 'sk_live_abcd' rotation failed"), + { + stack: + "Error: Internal: secret 'sk_live_abcd' rotation failed\n at /home/svc/.../secret-store.ts:88", + }, + ), + ), + }, + ], + }, + ], +})); + +describe("internal-error leak audit (opaque defects)", () => { + it.effect("plugin tagged error: defect surfaces only as opaque generic + correlation id", () => + 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.failsWithCause", 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; + // Must be the canonical opaque shape: "Internal tool error []" + expect(msg).toMatch(/^Internal tool error \[[0-9a-f]{8}\]$/); + // Crucially, no internal context leaks + expect(msg).not.toContain("trace-123"); + expect(msg).not.toContain("private-cluster"); + expect(msg).not.toContain("internal.service.local"); + expect(msg).not.toContain("InternalTransportError"); + expect(msg).not.toContain("Upstream request failed"); + }), + ); + + it.effect("plain Error with stack: stack and message do NOT escape", () => + 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.throwsRawError", args: {} })); + // 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("secret-store.ts"); + expect(msg).not.toContain("at /home/"); + expect(msg).not.toContain("sk_live_abcd"); + }), + ); +}); diff --git a/packages/core/execution/src/tool-invoker.test.ts b/packages/core/execution/src/tool-invoker.test.ts index 686573407..dca052997 100644 --- a/packages/core/execution/src/tool-invoker.test.ts +++ b/packages/core/execution/src/tool-invoker.test.ts @@ -1,9 +1,11 @@ import { describe, expect, it } from "@effect/vitest"; import { Effect, Fiber, Schema } from "effect"; +import * as ts from "typescript"; import { ElicitationResponse, FormElicitation, + ToolResult, createExecutor, definePlugin, } from "@executor-js/sdk"; @@ -18,6 +20,10 @@ const RepoInputSchema = Schema.toStandardSchemaV1( Schema.toStandardJSONSchemaV1(Schema.Struct({ owner: Schema.String, repo: Schema.String })), ); +const RepoDetailsOutputSchema = Schema.toStandardSchemaV1( + Schema.toStandardJSONSchemaV1(Schema.Struct({ defaultBranch: Schema.String })), +); + const ContactInputSchema = Schema.toStandardSchemaV1( Schema.toStandardJSONSchemaV1(Schema.Struct({ email: Schema.String })), ); @@ -28,6 +34,58 @@ const EmptyInputSchema = Schema.toStandardSchemaV1( const acceptAll = () => Effect.succeed(ElicitationResponse.make({ action: "accept" })); +type DescribedToolContract = { + readonly outputTypeScript: string; + readonly typeScriptDefinitions: Record; +}; + +const typeCheckDescribedInvocation = ( + described: DescribedToolContract, + runtimeResult: unknown, + consumerSource: string, +): readonly string[] => { + const fileName = "described-tool-contract.ts"; + const source = [ + ...Object.entries(described.typeScriptDefinitions).map(([name, definition]) => { + return `type ${name} = ${definition};`; + }), + `type ToolOutput = ${described.outputTypeScript};`, + `const invokedResult: ToolOutput = ${JSON.stringify(runtimeResult)};`, + consumerSource, + ].join("\n"); + + const options: ts.CompilerOptions = { + module: ts.ModuleKind.ESNext, + noEmit: true, + skipLibCheck: true, + strict: true, + target: ts.ScriptTarget.ES2022, + }; + const host = ts.createCompilerHost(options); + const originalGetSourceFile = host.getSourceFile.bind(host); + const originalReadFile = host.readFile.bind(host); + const originalFileExists = host.fileExists.bind(host); + + host.getSourceFile = (candidate, languageVersion, onError, shouldCreateNewSourceFile) => { + if (candidate === fileName) { + return ts.createSourceFile(candidate, source, languageVersion, true); + } + return originalGetSourceFile(candidate, languageVersion, onError, shouldCreateNewSourceFile); + }; + host.readFile = (candidate) => (candidate === fileName ? source : originalReadFile(candidate)); + host.fileExists = (candidate) => candidate === fileName || originalFileExists(candidate); + + const program = ts.createProgram([fileName], options, host); + return ts.getPreEmitDiagnostics(program).map((diagnostic) => { + const message = ts.flattenDiagnosticMessageText(diagnostic.messageText, "\n"); + if (!diagnostic.file || diagnostic.start === undefined) { + return message; + } + const position = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start); + return `${diagnostic.file.fileName}:${position.line + 1}:${position.character + 1} ${message}`; + }); +}; + // --------------------------------------------------------------------------- // Test plugins — each one declares a namespace as a static source with N // tools. Handlers return static data; the suite only cares about discovery @@ -53,6 +111,7 @@ const githubPlugin = definePlugin(() => ({ name: "getRepositoryDetails", description: "Get repository details including the default branch", inputSchema: RepoInputSchema, + outputSchema: RepoDetailsOutputSchema, handler: () => Effect.succeed({ defaultBranch: "main" }), }, { @@ -106,13 +165,78 @@ const errorPlugin = definePlugin(() => ({ description: "Query rows", inputSchema: EmptyInputSchema, handler: () => - Effect.succeed({ - data: null, - error: { - message: 'Field with name "DisplayName" does not exist', + Effect.succeed( + ToolResult.fail({ code: "invalid_query", - }, - }), + message: 'Field with name "DisplayName" does not exist', + }), + ), + }, + ], + }, + ], +})); + +const structuredFailurePlugin = definePlugin(() => ({ + id: "structured-failure-test" as const, + storage: () => ({}), + staticSources: () => [ + { + id: "upstream", + kind: "in-memory", + name: "Upstream", + tools: [ + { + name: "nestedErrorBody", + description: "", + inputSchema: EmptyInputSchema, + handler: () => + Effect.succeed( + ToolResult.fail({ + code: "upstream_http_error", + status: 400, + message: 'The expression "foo" is not valid. Provide a valid expression.', + details: { + error: { + code: "invalidRequest", + message: 'The expression "foo" is not valid. Provide a valid expression.', + }, + }, + }), + ), + }, + { + name: "flatErrorBody", + description: "", + inputSchema: EmptyInputSchema, + handler: () => + Effect.succeed( + ToolResult.fail({ + code: "upstream_http_error", + status: 400, + message: "Field 'XYZ' does not exist", + details: { + errorCode: 400, + errorMessage: "Field 'XYZ' does not exist", + }, + }), + ), + }, + { + name: "errorsArrayBody", + description: "", + inputSchema: EmptyInputSchema, + handler: () => + Effect.succeed( + ToolResult.fail({ + code: "upstream_http_error", + status: 403, + message: "Insufficient scope", + details: { + errors: [{ status: "403", title: "Forbidden", detail: "Insufficient scope" }], + }, + }), + ), }, ], }, @@ -342,8 +466,102 @@ describe("tool discovery", () => { expect(described.name).toBe("listRepositoryIssues"); expect(described.description).toBe("List issues for a repository"); expect(described.inputTypeScript).toBe("{ owner: string; repo: string; }"); - expect(described.outputTypeScript).toBeUndefined(); - expect(described.typeScriptDefinitions).toBeUndefined(); + expect(described.outputTypeScript).toBe( + "{ ok: true; data: unknown } | { ok: false; error: ToolError }", + ); + expect(described.typeScriptDefinitions).toEqual({ + ToolError: + "{ code: string; message: string; status?: number; details?: unknown; retryable?: boolean }", + }); + }), + ); + + it.effect("describes a return type that accepts the sandbox invocation result", () => + Effect.gen(function* () { + const executor = yield* makeSearchExecutor(); + const engine = createExecutionEngine({ executor, codeExecutor }); + + const execution = yield* engine.execute( + [ + 'const details = await tools.describe.tool({ path: "github.getRepositoryDetails" });', + "const result = await tools.github.getRepositoryDetails({ owner: 'executor', repo: 'executor' });", + "return {", + " outputTypeScript: details.outputTypeScript,", + " typeScriptDefinitions: details.typeScriptDefinitions,", + " result,", + "};", + ].join("\n"), + { onElicitation: acceptAll }, + ); + + expect(execution.error).toBeUndefined(); + const observed = execution.result as DescribedToolContract & { readonly result: unknown }; + const diagnostics = typeCheckDescribedInvocation( + observed, + observed.result, + [ + "function readDefaultBranch(result: ToolOutput): string {", + " if (!result.ok) return result.error.message;", + " return result.data.defaultBranch;", + "}", + "readDefaultBranch(invokedResult);", + ].join("\n"), + ); + expect(diagnostics).toEqual([]); + }), + ); + + it.effect( + "describes an error-as-value return type that accepts sandbox invocation failures", + () => + Effect.gen(function* () { + const executor = yield* createExecutor( + makeTestConfig({ plugins: [errorPlugin()] as const }), + ); + const engine = createExecutionEngine({ executor, codeExecutor }); + + const execution = yield* engine.execute( + [ + 'const details = await tools.describe.tool({ path: "records.queryRows" });', + "const result = await tools.records.queryRows({});", + "return {", + " outputTypeScript: details.outputTypeScript,", + " typeScriptDefinitions: details.typeScriptDefinitions,", + " result,", + "};", + ].join("\n"), + { onElicitation: acceptAll }, + ); + + expect(execution.error).toBeUndefined(); + const observed = execution.result as DescribedToolContract & { readonly result: unknown }; + const diagnostics = typeCheckDescribedInvocation( + observed, + observed.result, + [ + "function readToolResult(result: ToolOutput): unknown {", + " if (!result.ok) return result.error.message;", + " return result.data;", + "}", + "readToolResult(invokedResult);", + ].join("\n"), + ); + expect(diagnostics).toEqual([]); + }), + ); + + it.effect("describes the ToolResult wrapper through the direct describe helper", () => + Effect.gen(function* () { + const executor = yield* makeSearchExecutor(); + const described = yield* describeTool(executor, "github.getRepositoryDetails"); + + expect(described.outputTypeScript).toBe( + "{ ok: true; data: { defaultBranch: string; } } | { ok: false; error: ToolError }", + ); + expect(described.typeScriptDefinitions).toEqual({ + ToolError: + "{ code: string; message: string; status?: number; details?: unknown; retryable?: boolean }", + }); }), ); @@ -405,20 +623,97 @@ describe("tool discovery", () => { }), ); - it.effect("converts message-bearing tool error results into execution errors", () => + it.effect("passes ToolResult.fail through to the sandbox as a value (no throw)", () => Effect.gen(function* () { const executor = yield* createExecutor(makeTestConfig({ plugins: [errorPlugin()] as const })); const invoker = makeExecutorToolInvoker(executor, { invokeOptions: { onElicitation: acceptAll }, }); - const error = yield* Effect.flip(invoker.invoke({ path: "records.queryRows", args: {} })); - - expect(error).toEqual( - expect.objectContaining({ + const result = yield* invoker.invoke({ path: "records.queryRows", args: {} }); + expect(result).toEqual({ + ok: false, + error: { + code: "invalid_query", message: 'Field with name "DisplayName" does not exist', - }), + }, + }); + }), + ); + + it.effect("preserves nested upstream error bodies through ToolResult.fail", () => + Effect.gen(function* () { + const executor = yield* createExecutor( + makeTestConfig({ plugins: [structuredFailurePlugin()] as const }), + ); + const invoker = makeExecutorToolInvoker(executor, { + invokeOptions: { onElicitation: acceptAll }, + }); + + const result = yield* invoker.invoke({ path: "upstream.nestedErrorBody", args: {} }); + expect(result).toEqual({ + ok: false, + error: { + code: "upstream_http_error", + status: 400, + message: 'The expression "foo" is not valid. Provide a valid expression.', + details: { + error: { + code: "invalidRequest", + message: 'The expression "foo" is not valid. Provide a valid expression.', + }, + }, + }, + }); + }), + ); + + it.effect("preserves flat upstream error bodies through ToolResult.fail", () => + Effect.gen(function* () { + const executor = yield* createExecutor( + makeTestConfig({ plugins: [structuredFailurePlugin()] as const }), + ); + const invoker = makeExecutorToolInvoker(executor, { + invokeOptions: { onElicitation: acceptAll }, + }); + + const result = yield* invoker.invoke({ path: "upstream.flatErrorBody", args: {} }); + expect(result).toEqual({ + ok: false, + error: { + code: "upstream_http_error", + status: 400, + message: "Field 'XYZ' does not exist", + details: { + errorCode: 400, + errorMessage: "Field 'XYZ' does not exist", + }, + }, + }); + }), + ); + + it.effect("preserves upstream errors arrays through ToolResult.fail", () => + Effect.gen(function* () { + const executor = yield* createExecutor( + makeTestConfig({ plugins: [structuredFailurePlugin()] as const }), ); + const invoker = makeExecutorToolInvoker(executor, { + invokeOptions: { onElicitation: acceptAll }, + }); + + const result = yield* invoker.invoke({ path: "upstream.errorsArrayBody", args: {} }); + expect(result).toEqual({ + ok: false, + error: { + code: "upstream_http_error", + status: 403, + message: "Insufficient scope", + details: { + errors: [{ status: "403", title: "Forbidden", detail: "Insufficient scope" }], + }, + }, + }); }), ); }); diff --git a/packages/core/execution/src/tool-invoker.ts b/packages/core/execution/src/tool-invoker.ts index d8f976e57..5c4b4f53d 100644 --- a/packages/core/execution/src/tool-invoker.ts +++ b/packages/core/execution/src/tool-invoker.ts @@ -1,4 +1,4 @@ -import { Effect, Match, Option, Predicate } from "effect"; +import { Effect, Predicate } from "effect"; import * as Cause from "effect/Cause"; import type { Executor, @@ -8,9 +8,32 @@ import type { InvokeOptions, Source, } from "@executor-js/sdk/core"; +import { isToolResult } from "@executor-js/sdk/core"; import type { SandboxToolInvoker } from "@executor-js/codemode-core"; import { ExecutionToolError } from "./errors"; +const OPAQUE_DEFECT_MESSAGE = "Internal tool error"; +const TOOL_ERROR_TYPESCRIPT = + "{ code: string; message: string; status?: number; details?: unknown; retryable?: boolean }"; + +const wrapOutputTypeScript = (outputTypeScript?: string): string => + `{ ok: true; data: ${outputTypeScript ?? "unknown"} } | { ok: false; error: ToolError }`; + +const withToolResultDefinitions = ( + definitions?: Record, +): Record => ({ + ...(definitions ?? {}), + ToolError: TOOL_ERROR_TYPESCRIPT, +}); + +const newCorrelationId = (): string => { + // 8-hex-char correlation id; enough entropy to disambiguate within a + // single deployment without leaking host process info. + return Math.floor(Math.random() * 0x1_0000_0000) + .toString(16) + .padStart(8, "0"); +}; + /** * Extract the source namespace from a tool path. Tool paths look like * "." or ".." — we take the first @@ -23,44 +46,6 @@ const extractSourceNamespace = (path: string): string => { return idx === -1 ? path : path.slice(0, idx); }; -const hasStringMessage = (value: unknown): value is { readonly message: string } => - value !== null && - typeof value === "object" && - "message" in value && - typeof value.message === "string"; - -const messageFromErrorLike = (value: unknown): string | undefined => { - if (hasStringMessage(value)) { - return value.message; - } - return undefined; -}; - -const renderToolErrorMessage = (error: unknown): string => - messageFromErrorLike(error) ?? - (typeof error === "undefined" ? "Tool execution failed" : renderUnknownPrimitive(error)); - -const renderUnknownPrimitive = (value: unknown): string => - Match.value(value).pipe( - Match.when(Match.string, (s) => s), - Match.whenOr(Match.number, Match.boolean, Match.bigint, Match.symbol, (x) => x.toString()), - Match.option, - Option.getOrElse(() => "Tool execution failed"), - ); - -type ToolResultEnvelope = { - readonly error?: unknown; - readonly data?: unknown; -}; - -const isToolResultEnvelope = (value: unknown): value is ToolResultEnvelope => - value !== null && typeof value === "object" && ("error" in value || "data" in value); - -const hasToolResultError = ( - value: ToolResultEnvelope, -): value is ToolResultEnvelope & { readonly error: unknown } => - value.error !== null && value.error !== undefined; - /** * Bridges QuickJS `tools.someSource.someOp(args)` calls into * `executor.tools.invoke(toolId, args)`. @@ -87,35 +72,46 @@ export const makeExecutorToolInvoker = ( const result = yield* executor.tools.invoke(path as ToolId, args, options.invokeOptions).pipe( Effect.catchCause((cause): Effect.Effect => { const err = cause.reasons.find(Cause.isFailReason)?.error; - if (!isElicitationDeclinedError(err)) { + if (isElicitationDeclinedError(err)) { return Effect.fail( new ExecutionToolError({ - message: renderToolErrorMessage(err), - cause: err ?? cause, + message: `Tool "${err.toolId}" requires approval but the request was ${err.action === "cancel" ? "cancelled" : "declined"} by the user.`, + cause: err, }), ); } - return Effect.fail( - new ExecutionToolError({ - message: `Tool "${err.toolId}" requires approval but the request was ${err.action === "cancel" ? "cancelled" : "declined"} by the user.`, - cause: err, + // Any other failure here is an infra/plugin defect. Emit an + // opaque generic with a correlation id so internal context (URLs + // with tokens, DB connection strings, file paths in stacks) + // can't leak through Error.message into the sandbox. The full + // cause is logged with the same correlation id so operators can + // still trace the failure. + const correlationId = newCorrelationId(); + return Effect.logError("tool dispatch failed", cause).pipe( + Effect.annotateLogs({ + "executor.correlation_id": correlationId, + "mcp.tool.name": path, }), + Effect.flatMap(() => + Effect.fail( + new ExecutionToolError({ + message: `${OPAQUE_DEFECT_MESSAGE} [${correlationId}]`, + cause: err ?? cause, + }), + ), + ), ); }), ); - if (!isToolResultEnvelope(result)) { + + // Strict: plugins emit ToolResult. Anything else is treated as a + // raw success value and wrapped — keeps the sandbox-facing contract + // uniform without forcing every tiny test plugin to import + // `ToolResult.ok`. + if (isToolResult(result)) { return result; } - if (hasToolResultError(result)) { - return yield* new ExecutionToolError({ - message: renderToolErrorMessage(result.error), - cause: result.error, - }); - } - if ("data" in result) { - return result.data; - } - return result; + return { ok: true, data: result }; }), }); @@ -513,7 +509,7 @@ export const describeTool = Effect.fn("executor.tools.describe")(function* ( name: schema.name ?? path, description: schema.description, inputTypeScript: schema.inputTypeScript, - outputTypeScript: schema.outputTypeScript, - typeScriptDefinitions: schema.typeScriptDefinitions, + outputTypeScript: wrapOutputTypeScript(schema.outputTypeScript), + typeScriptDefinitions: withToolResultDefinitions(schema.typeScriptDefinitions), }; }); diff --git a/packages/core/sdk/src/index.ts b/packages/core/sdk/src/index.ts index ecc0b141a..7297129e3 100644 --- a/packages/core/sdk/src/index.ts +++ b/packages/core/sdk/src/index.ts @@ -333,3 +333,10 @@ export { // Wire-level HTTP error schemas usable by plugin HttpApiGroup definitions. export { InternalError } from "./api-errors"; + +// ToolResult — typed value-based discriminated union for tool outcomes. +// The `Tool` value namespace exposes `Tool.ok` / `Tool.fail` constructors; +// the `Tool` type alias from `./types` is a separate row projection. +// TypeScript permits the two to share a name because one is purely a +// value and the other purely a type. +export { ToolResult, isToolResult, type ToolError } from "./tool-result"; diff --git a/packages/core/sdk/src/tool-result.test.ts b/packages/core/sdk/src/tool-result.test.ts new file mode 100644 index 000000000..c253203b5 --- /dev/null +++ b/packages/core/sdk/src/tool-result.test.ts @@ -0,0 +1,50 @@ +import { describe, expect, it } from "@effect/vitest"; +import { ToolResult, isToolResult } from "./tool-result"; + +describe("ToolResult", () => { + it("ok wraps a value", () => { + const r = ToolResult.ok({ count: 3 }); + expect(r).toEqual({ ok: true, data: { count: 3 } }); + expect(isToolResult(r)).toBe(true); + }); + + it("fail wraps a ToolError", () => { + const r = ToolResult.fail({ + code: "upstream_http_error", + status: 404, + message: "Not found", + details: { id: "x" }, + }); + expect(r).toEqual({ + ok: false, + error: { + code: "upstream_http_error", + status: 404, + message: "Not found", + details: { id: "x" }, + }, + }); + expect(isToolResult(r)).toBe(true); + }); + + it("isToolResult rejects unrelated shapes", () => { + expect(isToolResult(null)).toBe(false); + expect(isToolResult({})).toBe(false); + expect(isToolResult({ ok: true })).toBe(false); + expect(isToolResult({ ok: false })).toBe(false); + expect(isToolResult({ ok: false, error: {} })).toBe(false); + expect(isToolResult({ ok: false, error: { code: 1, message: "x" } })).toBe(false); + expect(isToolResult({ ok: "yes", data: 1 })).toBe(false); + }); + + it("isToolResult accepts both branches of the union", () => { + expect(isToolResult({ ok: true, data: 1 })).toBe(true); + expect(isToolResult({ ok: true, data: null })).toBe(true); + expect( + isToolResult({ + ok: false, + error: { code: "x", message: "y" }, + }), + ).toBe(true); + }); +}); diff --git a/packages/core/sdk/src/tool-result.ts b/packages/core/sdk/src/tool-result.ts new file mode 100644 index 000000000..c4ac138b3 --- /dev/null +++ b/packages/core/sdk/src/tool-result.ts @@ -0,0 +1,37 @@ +// --------------------------------------------------------------------------- +// ToolResult — typed value-based discriminated union returned by tool +// handlers and `invokeTool`. Domain success and expected failure both +// resolve through Effect's success channel; only true infra defects use +// the Effect failure channel. +// --------------------------------------------------------------------------- + +import { Schema } from "effect"; + +export const ToolErrorSchema = Schema.Struct({ + code: Schema.String, + message: Schema.String, + status: Schema.optional(Schema.Number), + details: Schema.optional(Schema.Unknown), + retryable: Schema.optional(Schema.Boolean), +}); + +export type ToolError = typeof ToolErrorSchema.Type; + +export type ToolResult = + | { readonly ok: true; readonly data: T } + | { readonly ok: false; readonly error: ToolError }; + +export const ToolResult = { + ok: (data: T): ToolResult => ({ ok: true, data }), + fail: (error: ToolError): ToolResult => ({ ok: false, error }), +} as const; + +const ToolResultSchema = Schema.Union([ + Schema.Struct({ ok: Schema.Literal(true), data: Schema.Unknown }), + Schema.Struct({ ok: Schema.Literal(false), error: ToolErrorSchema }), +]); + +const isUnknownToolResult = Schema.is(ToolResultSchema); + +export const isToolResult = (value: unknown): value is ToolResult => + isUnknownToolResult(value); diff --git a/packages/hosts/mcp/src/server.test.ts b/packages/hosts/mcp/src/server.test.ts index f6663bc0d..606487659 100644 --- a/packages/hosts/mcp/src/server.test.ts +++ b/packages/hosts/mcp/src/server.test.ts @@ -116,7 +116,7 @@ describe("MCP host server — client with elicitation", () => { }); }); - it("execute tool resolves failed engine effects as MCP error results", async () => { + it("execute tool surfaces failed engine effects as an opaque generic with correlation id", async () => { const engine = makeStubEngine({ execute: () => Effect.fail(new TestExecutionError({ message: "Unexpected token ':'" })), }); @@ -126,11 +126,11 @@ describe("MCP host server — client with elicitation", () => { name: "execute", arguments: { code: "const x: any = 1;" }, }); - expect(textOf(result)).toBe("Error: Unexpected token ':'"); - expect(result.structuredContent).toEqual({ - status: "error", - error: "Unexpected token ':'", - }); + const text = textOf(result); + expect(text).toMatch(/^Error: Internal tool error \[[0-9a-f]{8}\]$/); + expect(text).not.toContain("Unexpected token"); + const structured = (result.structuredContent as { readonly error?: string }).error ?? ""; + expect(structured).toMatch(/^Internal tool error \[[0-9a-f]{8}\]$/); expect(result.isError).toBe(true); }); }); @@ -146,11 +146,16 @@ describe("MCP host server — client with elicitation", () => { name: "execute", arguments: { code: "run" }, }); - expect(textOf(result)).toBe("Error: Tool execution failed"); - expect(result.structuredContent).toEqual({ + const text = textOf(result); + expect(text).toMatch(/^Error: Internal tool error \[[0-9a-f]{8}\]$/); + // Sensitive internal context must NOT leak through the MCP error path. + expect(text).not.toContain("secret internal detail"); + expect(result.structuredContent).toMatchObject({ status: "error", - error: "Tool execution failed", }); + const structuredError = (result.structuredContent as { readonly error?: string }).error ?? ""; + expect(structuredError).toMatch(/^Internal tool error \[[0-9a-f]{8}\]$/); + expect(structuredError).not.toContain("secret internal detail"); expect(result.isError).toBe(true); }); }); diff --git a/packages/hosts/mcp/src/server.ts b/packages/hosts/mcp/src/server.ts index abda5f550..5b6342e8d 100644 --- a/packages/hosts/mcp/src/server.ts +++ b/packages/hosts/mcp/src/server.ts @@ -260,20 +260,28 @@ const toMcpPausedResult = (formatted: ReturnType): structuredContent: formatted.structured, }); -const formatFailureMessage = (value: unknown): string | null => { - if (typeof value === "object" && value !== null && "message" in value) { - const message = (value as { readonly message?: unknown }).message; - if (typeof message === "string" && message.length > 0) return message; - } - if (typeof value === "string" && value.length > 0) return value; - return null; -}; +// `execute` failures reaching the MCP host are infra defects — domain +// failures from tools are now expressed as `ToolResult` values (success +// channel) and flow through `formatExecuteResult`. Emit an opaque +// generic plus a fresh correlation id and log the cause out-of-band so +// the model can't read internal context off `.message`. +const newCorrelationId = (): string => + Math.floor(Math.random() * 0x1_0000_0000) + .toString(16) + .padStart(8, "0"); const toMcpFailureResult = (cause: Cause.Cause): McpToolResult => { - const failure = cause.reasons.find(Cause.isFailReason); - const text = failure - ? (formatFailureMessage(failure.error) ?? "Tool execution failed") - : "Tool execution failed"; + const correlationId = newCorrelationId(); + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: best-effort defect logging must tolerate non-serializable causes + try { + console.error( + `[executor:mcp] execute defect correlation_id=${correlationId}`, + Cause.pretty(cause), + ); + } catch { + /* ignore logger failures */ + } + const text = `Internal tool error [${correlationId}]`; return { content: [{ type: "text", text: `Error: ${text}` }], structuredContent: { status: "error", error: text }, diff --git a/packages/kernel/core/src/strip-types.test.ts b/packages/kernel/core/src/strip-types.test.ts index 086d71391..83291dde2 100644 --- a/packages/kernel/core/src/strip-types.test.ts +++ b/packages/kernel/core/src/strip-types.test.ts @@ -59,11 +59,11 @@ describe("stripTypeScript", () => { // axiom://7bf76f79c5d807272781e9554040aab3 — typed annotation in // a function expression. const code = ` - const fetchDeals = async (sourceId: string): Promise> => { + const fetchResources = async (sourceId: string): Promise> => { const result = await tools.executor.sources.list(); return result.items; }; - return fetchDeals('dealcloud'); + return fetchResources('example-source'); `; const out = stripTypeScript(code); expect(out).not.toContain(": string"); diff --git a/packages/kernel/runtime-dynamic-worker/src/invocation.test.ts b/packages/kernel/runtime-dynamic-worker/src/invocation.test.ts index 3b48e0fd6..d2ee3aef4 100644 --- a/packages/kernel/runtime-dynamic-worker/src/invocation.test.ts +++ b/packages/kernel/runtime-dynamic-worker/src/invocation.test.ts @@ -4,6 +4,7 @@ import * as Cause from "effect/Cause"; import * as Data from "effect/Data"; import * as Effect from "effect/Effect"; import type { SandboxToolInvoker } from "@executor-js/codemode-core"; +import { ExecutionToolError } from "@executor-js/execution"; import { ToolDispatcher, makeDynamicWorkerExecutor, @@ -246,7 +247,7 @@ describe("makeDynamicWorkerExecutor", () => { expect(result.result).toBe(7); }); - it("surfaces tool errors in execution result", async () => { + it("surfaces infra defects through the worker bridge as an opaque generic", async () => { const executor = makeDynamicWorkerExecutor({ loader }); const invoker = failingInvoker("not authorized"); @@ -254,7 +255,28 @@ describe("makeDynamicWorkerExecutor", () => { executor.execute("async () => { return await tools.secret.read({}); }", invoker), ); - expect(result.error).toBe("not authorized"); + expect(result.error).toBe("Internal tool error"); + }); + + it("preserves public ExecutionToolError messages across the worker bridge", async () => { + const executor = makeDynamicWorkerExecutor({ loader }); + const invoker = { + invoke: () => + Effect.fail( + new ExecutionToolError({ + message: + "tools.search expects an object: { query?: string; namespace?: string; limit?: number; offset?: number }", + }), + ), + } satisfies SandboxToolInvoker; + + const result = await Effect.runPromise( + executor.execute("async () => await tools.search('github')", invoker), + ); + + expect(result.error).toBe( + "tools.search expects an object: { query?: string; namespace?: string; limit?: number; offset?: number }", + ); }); it("does not expose host error stack details to sandbox error handlers", async () => { @@ -284,11 +306,12 @@ describe("makeDynamicWorkerExecutor", () => { ); expect(result.error).toBeUndefined(); - expect(result.result).toMatchObject({ message: "not authorized" }); + expect(result.result).toMatchObject({ message: "Internal tool error" }); expect((result.result as { stack?: string }).stack).not.toContain("secret host stack"); + expect((result.result as { message?: string }).message).not.toContain("not authorized"); }); - it("surfaces object-shaped tool errors in execution result", async () => { + it("collapses object-shaped tool defects to an opaque generic", async () => { const executor = makeDynamicWorkerExecutor({ loader }); const invoker = { invoke: () => @@ -302,11 +325,11 @@ describe("makeDynamicWorkerExecutor", () => { executor.execute("async () => { return await tools.secret.read({}); }", invoker), ); - expect(result.error).toBe('{"code":"forbidden","detail":"missing team access"}'); + expect(result.error).toBe("Internal tool error"); expect(result.result).toBeNull(); }); - it("surfaces message-bearing object tool errors in execution result", async () => { + it("collapses message-bearing object tool defects to an opaque generic", async () => { const executor = makeDynamicWorkerExecutor({ loader }); const invoker = { invoke: () => @@ -320,7 +343,8 @@ describe("makeDynamicWorkerExecutor", () => { executor.execute("async () => { return await tools.records.query({}); }", invoker), ); - expect(result.error).toBe('Field with name "DisplayName" does not exist'); + expect(result.error).toBe("Internal tool error"); + expect(result.error).not.toContain("DisplayName"); expect(result.result).toBeNull(); }); @@ -366,7 +390,7 @@ describe("makeDynamicWorkerExecutor", () => { expect(result.error).toBe("Tool RPC payload contains a circular reference"); }); - it("returns an execution error for circular tool results", async () => { + it("returns an opaque generic when a tool result can't be serialized", async () => { const executor = makeDynamicWorkerExecutor({ loader }); const cyclic: Record = {}; cyclic.self = cyclic; @@ -377,7 +401,7 @@ describe("makeDynamicWorkerExecutor", () => { ); expect(result.result).toBeNull(); - expect(result.error).toBe("Tool RPC payload contains a circular reference"); + expect(result.error).toBe("Internal tool error"); }); it("respects timeout", async () => { diff --git a/packages/kernel/runtime-dynamic-worker/src/module-template.ts b/packages/kernel/runtime-dynamic-worker/src/module-template.ts index 23627e722..754eca1b4 100644 --- a/packages/kernel/runtime-dynamic-worker/src/module-template.ts +++ b/packages/kernel/runtime-dynamic-worker/src/module-template.ts @@ -127,6 +127,14 @@ export const buildExecutorModule = (body: string, timeoutMs: number): string => " for (const [k, v] of Object.entries(value)) out[k] = __decodeBinary(v, seen);", " return out;", " };", + " const __publicToolErrorMessage = (error) => {", + " const values = [error && error.primary, ...(Array.isArray(error && error.failures) ? error.failures : [])];", + " for (const value of values) {", + " if (value && value.__type === 'Error' && value.name === 'ExecutionToolError' && typeof value.message === 'string') return value.message;", + " }", + " if (error && typeof error.message === 'string' && error.message.startsWith('Internal tool error')) return error.message;", + " return null;", + " };", " const __makeToolsProxy = (path = []) => new Proxy(() => undefined, {", " get(_target, prop) {", " if (prop === 'then' || typeof prop === 'symbol') return undefined;", @@ -138,7 +146,7 @@ export const buildExecutorModule = (body: string, timeoutMs: number): string => " return (async () => {", " const encoded = await __encodeBinary(args[0]);", " const data = await __dispatcher.call(toolPath, encoded);", - " if (!data.ok) throw new Error(data.error && data.error.message ? data.error.message : 'Tool execution failed');", + " if (!data.ok) throw new Error(__publicToolErrorMessage(data.error) || 'Internal tool error');", " return __decodeBinary(data.result);", " })();", " },", diff --git a/packages/kernel/runtime-quickjs/src/index.test.ts b/packages/kernel/runtime-quickjs/src/index.test.ts index 4435d246b..30954f7eb 100644 --- a/packages/kernel/runtime-quickjs/src/index.test.ts +++ b/packages/kernel/runtime-quickjs/src/index.test.ts @@ -121,6 +121,52 @@ describe("quickjs executor", () => { }), ); + it.effect("internal defects reach the sandbox as an opaque generic only", () => + Effect.gen(function* () { + // Plugin defect carrying sensitive context. The bridge's reject + // path must strip everything except the canonical + // "Internal tool error []" shape — or fall back to the + // bare generic if the upstream invoker hasn't already stamped + // the correlation id (this test exercises the latter path + // because it bypasses makeExecutorToolInvoker). + const invoker: SandboxToolInvoker = { + invoke: () => + Effect.fail( + Object.assign( + new Error("Authorization: Bearer SECRET_TOKEN_xyz failed against host 10.0.0.5"), + { + stack: "Error\n at /home/svc/executor/packages/plugins/foo:142:11", + }, + ) as never, + ), + }; + + const result = yield* executor.execute( + ` + try { + await tools.leaky.call({}); + return "should not reach"; + } catch (e) { + return e.message; + } + `, + invoker, + ); + + expect(result.error).toBeUndefined(); + const message = String(result.result); + // Either the canonical opaque generic with a correlation id, or + // the bare fallback. Neither must contain any sensitive context. + expect( + message === "Internal tool error" || /^Internal tool error \[[0-9a-f]{8}\]$/.test(message), + ).toBe(true); + expect(message).not.toContain("SECRET_TOKEN_xyz"); + expect(message).not.toContain("Authorization"); + expect(message).not.toContain("10.0.0.5"); + expect(message).not.toContain("packages/plugins"); + }), + ); + it.effect("handles unknown tool path", () => Effect.gen(function* () { const invoker = makeTestInvoker({}); diff --git a/packages/kernel/runtime-quickjs/src/index.ts b/packages/kernel/runtime-quickjs/src/index.ts index af8683b4f..d90790123 100644 --- a/packages/kernel/runtime-quickjs/src/index.ts +++ b/packages/kernel/runtime-quickjs/src/index.ts @@ -47,6 +47,30 @@ const EXECUTION_FILENAME = "executor-quickjs-runtime.js"; const toError = (cause: unknown): Error => cause instanceof Error ? cause : new Error(String(cause)); +// Defect surfaced to the sandbox when a tool dispatch reaches the +// reject path. Two sources reach here: +// - `ExecutionToolError` emitted by the execution package — message +// is already either the canonical +// `Internal tool error []` opaque generic (plugin defects +// passed through `makeExecutorToolInvoker`'s catchCause) or an +// intentional public string (validators in engine.ts: +// "tools.search expects an object: ..."). +// - Anything else — assume infra defect and emit a bare opaque +// generic. +// `_tag === "ExecutionToolError"` is the in-band signal that the +// message was generated by code we control and is safe to pass +// through; we never read `.cause` / `.stack`. +const sandboxDefectMessage = (cause: unknown): string => { + if (cause !== null && typeof cause === "object") { + const tagged = cause as { readonly _tag?: unknown; readonly message?: unknown }; + // oxlint-disable-next-line executor/no-manual-tag-check, executor/no-unknown-error-message -- boundary: QuickJS reject path receives an already-rendered ExecutionToolError; we pass its in-band public message through and otherwise emit the opaque generic + if (tagged._tag === "ExecutionToolError" && typeof tagged.message === "string") { + return tagged.message; + } + } + return "Internal tool error"; +}; + const toErrorMessage = (cause: unknown): string => { if (typeof cause === "object" && cause !== null) { const message = @@ -216,7 +240,17 @@ const createToolBridge = ( return; } - const errorHandle = context.newError(toErrorMessage(cause)); + // The reject path is reserved for true infra defects. The + // upstream tool-invoker has already replaced the message with + // an opaque generic plus a correlation id, but defensively log + // the cause here and emit a stable generic if upstream changes. + const message = sandboxDefectMessage(cause); + try { + console.error("[executor:quickjs] tool dispatch defect", { path, cause }); + } catch { + /* ignore logger failures */ + } + const errorHandle = context.newError(message); deferred.reject(errorHandle); errorHandle.dispose(); }, diff --git a/packages/plugins/google-discovery/src/sdk/option-json-repro.test.ts b/packages/plugins/google-discovery/src/sdk/option-json.test.ts similarity index 96% rename from packages/plugins/google-discovery/src/sdk/option-json-repro.test.ts rename to packages/plugins/google-discovery/src/sdk/option-json.test.ts index 668322549..4a87e10df 100644 --- a/packages/plugins/google-discovery/src/sdk/option-json-repro.test.ts +++ b/packages/plugins/google-discovery/src/sdk/option-json.test.ts @@ -1,6 +1,6 @@ -// Reproduces the PR 706 bug class using Effect-native primitives only — +// Covers the Option JSON boundary using Effect-native primitives only: // no JSON.parse, no JSON.stringify, no node:fs on our side. We split the -// JSON boundary into two Effect schema steps: +// boundary into two Effect schema steps: // // 1. Schema.encodeEffect(Inner)(value) → encoded JS shape // 2. Schema.encodeEffect(UnknownFromJsonString) → JSON string @@ -13,7 +13,7 @@ // step 2's JSON-stringify (driven by Effect, not us) flattens the Option // to {_id,_tag,value}, and step 5 rejects the shape. // -// Run: vitest run packages/plugins/google-discovery/src/sdk/option-json-repro.test.ts +// Run: vitest run packages/plugins/google-discovery/src/sdk/option-json.test.ts import { describe, expect, it } from "@effect/vitest"; import * as NodeFileSystem from "@effect/platform-node/NodeFileSystem"; @@ -28,7 +28,7 @@ const fixed = { description: Option.some("hello") }; const withTmpFile = (fn: (path: string) => Effect.Effect) => Effect.gen(function* () { const fs = yield* FileSystem.FileSystem; - const dir = yield* fs.makeTempDirectoryScoped({ prefix: "option-repro-" }); + const dir = yield* fs.makeTempDirectoryScoped({ prefix: "option-json-" }); return yield* fn(`${dir}/binding.json`); }).pipe(Effect.scoped, Effect.provide(NodeFileSystem.layer)); diff --git a/packages/plugins/google-discovery/src/sdk/plugin.test.ts b/packages/plugins/google-discovery/src/sdk/plugin.test.ts index 55089f297..5b0b91eb1 100644 --- a/packages/plugins/google-discovery/src/sdk/plugin.test.ts +++ b/packages/plugins/google-discovery/src/sdk/plugin.test.ts @@ -430,10 +430,10 @@ describe("Google Discovery plugin", () => { "drive.files.get", { fileId: "123", fields: "id,name", prettyPrint: true }, autoApprove, - )) as { data: unknown; error: unknown }; + )) as { readonly ok: true; readonly data: { status: number; data: unknown } }; - expect(invocation.error).toBeNull(); - expect(invocation.data).toEqual({ id: "123", name: "Quarterly Plan" }); + expect(invocation.ok).toBe(true); + expect(invocation.data.data).toEqual({ id: "123", name: "Quarterly Plan" }); const apiRequest = handle.requests.find((request) => request.url.startsWith("/drive/v3/files/123"), diff --git a/packages/plugins/google-discovery/src/sdk/plugin.ts b/packages/plugins/google-discovery/src/sdk/plugin.ts index f44c7a25c..2f6c0f6e5 100644 --- a/packages/plugins/google-discovery/src/sdk/plugin.ts +++ b/packages/plugins/google-discovery/src/sdk/plugin.ts @@ -3,6 +3,7 @@ import { Effect, Option, Predicate, Schema } from "effect"; import { ScopeId, SourceDetectionResult, + ToolResult, Usage, definePlugin, resolveSecretBackedMap, @@ -29,6 +30,69 @@ import type { } from "./types"; import { GoogleDiscoveryStoredSourceData as GoogleDiscoveryStoredSourceDataSchema } from "./types"; +// --------------------------------------------------------------------------- +// Upstream-error message extraction +// --------------------------------------------------------------------------- + +const GOOGLE_BODY_CAP = 1024; +const UpstreamMessageBody = Schema.Struct({ message: Schema.String }); +const UpstreamErrorMessageBody = Schema.Struct({ errorMessage: Schema.String }); +const UpstreamNestedErrorBody = Schema.Struct({ error: UpstreamMessageBody }); +const UpstreamErrorsArrayBody = Schema.Struct({ + errors: Schema.Array( + Schema.Struct({ + detail: Schema.optional(Schema.String), + message: Schema.optional(Schema.String), + title: Schema.optional(Schema.String), + }), + ), +}); + +const decodeUpstreamMessageBody = Schema.decodeUnknownOption(UpstreamMessageBody); +const decodeUpstreamErrorMessageBody = Schema.decodeUnknownOption(UpstreamErrorMessageBody); +const decodeUpstreamNestedErrorBody = Schema.decodeUnknownOption(UpstreamNestedErrorBody); +const decodeUpstreamErrorsArrayBody = Schema.decodeUnknownOption(UpstreamErrorsArrayBody); + +const googleClampedStringify = (value: unknown): string => { + let s: string; + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: JSON.stringify may throw on cycles; fall back to String() so the upstream body can still be surfaced as ToolError.details fallback text + try { + s = JSON.stringify(value); + } catch { + s = String(value); + } + return s.length > GOOGLE_BODY_CAP ? `${s.slice(0, GOOGLE_BODY_CAP)}…` : s; +}; + +const firstNonEmpty = (...values: readonly (string | undefined)[]): string | undefined => + values.find((value) => value !== undefined && value.length > 0); + +const googleExtractUpstreamMessage = (body: unknown, status: number): string => { + if (typeof body === "string") { + return body.length > 0 ? body : `Upstream returned HTTP ${status}`; + } + const nested = Option.getOrUndefined(decodeUpstreamNestedErrorBody(body)); + const messageBody = Option.getOrUndefined(decodeUpstreamMessageBody(body)); + const errorMessageBody = Option.getOrUndefined(decodeUpstreamErrorMessageBody(body)); + const errorsBody = Option.getOrUndefined(decodeUpstreamErrorsArrayBody(body)); + const arrayMessage = errorsBody?.errors + .map(({ detail, message: upstreamMessage, title }) => + firstNonEmpty(detail, upstreamMessage, title), + ) + .find((message) => message !== undefined); + const message = firstNonEmpty( + nested?.error.message, + messageBody?.message, + errorMessageBody?.errorMessage, + arrayMessage, + ); + if (message !== undefined) return message; + if (body !== null && typeof body === "object") { + return googleClampedStringify(body); + } + return `Upstream returned HTTP ${status}`; +}; + // --------------------------------------------------------------------------- // Public input / output shapes // --------------------------------------------------------------------------- @@ -381,11 +445,27 @@ export const googleDiscoveryPlugin = definePlugin(() => ({ extension: makeGoogleDiscoveryPluginExtension, invokeTool: ({ ctx, toolRow, args }) => - invokeGoogleDiscoveryTool({ - ctx: ctx as PluginCtx, - toolId: toolRow.id, - toolScope: decodeString(toolRow.scope_id), - args, + Effect.gen(function* () { + const result = yield* invokeGoogleDiscoveryTool({ + ctx: ctx as PluginCtx, + toolId: toolRow.id, + toolScope: decodeString(toolRow.scope_id), + args, + }); + const ok = result.status >= 200 && result.status < 300; + if (!ok) { + return ToolResult.fail({ + code: "upstream_http_error", + status: result.status, + message: googleExtractUpstreamMessage(result.error, result.status), + details: result.error, + }); + } + return ToolResult.ok({ + status: result.status, + headers: result.headers, + data: result.data, + }); }), resolveAnnotations: ({ ctx, sourceId, toolRows }) => diff --git a/packages/plugins/graphql/src/sdk/plugin.test.ts b/packages/plugins/graphql/src/sdk/plugin.test.ts index df4d57fab..c97cd9c8e 100644 --- a/packages/plugins/graphql/src/sdk/plugin.test.ts +++ b/packages/plugins/graphql/src/sdk/plugin.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect } from "@effect/vitest"; import { Effect, Predicate } from "effect"; +import { HttpServerRequest, HttpServerResponse } from "effect/unstable/http"; import { ConnectionId, @@ -13,7 +14,7 @@ import { SecretId, TokenMaterial, } from "@executor-js/sdk"; -import { makeTestConfig } from "@executor-js/sdk/testing"; +import { makeTestConfig, serveTestHttpApp } from "@executor-js/sdk/testing"; import { memorySecretsPlugin } from "@executor-js/sdk/testing"; import { graphqlPlugin } from "./plugin"; @@ -196,9 +197,8 @@ describe("graphqlPlugin real protocol server", () => { }); expect(result).toEqual({ - status: 200, + ok: true, data: { hello: "Hello Ada" }, - errors: null, }); const requests = yield* server.requests; @@ -209,6 +209,46 @@ describe("graphqlPlugin real protocol server", () => { }), ); + it.effect("surfaces non-2xx invocation responses as ToolResult.fail", () => + Effect.gen(function* () { + const server = yield* serveTestHttpApp((request) => + Effect.gen(function* () { + const webRequest = yield* HttpServerRequest.toWeb(request); + const body = yield* Effect.promise(() => webRequest.text()); + if (body.includes("__schema")) { + return HttpServerResponse.jsonUnsafe({ data: introspectionResult }); + } + return HttpServerResponse.text("temporary upstream outage", { + status: 503, + contentType: "text/plain", + }); + }), + ); + const executor = yield* createExecutor( + makeTestConfig({ plugins: [graphqlPlugin()] as const }), + ); + + yield* executor.graphql.addSource({ + endpoint: server.url("/graphql"), + scope: TEST_SCOPE, + namespace: "http_error_graph", + }); + + const result = yield* executor.tools.invoke("http_error_graph.query.hello", { + name: "Ada", + }); + + expect(result).toMatchObject({ + ok: false, + error: { + code: "graphql_http_error", + status: 503, + message: "GraphQL request failed with HTTP 503", + }, + }); + }), + ); + it.effect("invokes OAuth-backed sources with a bearer token", () => Effect.gen(function* () { const server = yield* serveGreetingServer; @@ -251,9 +291,8 @@ describe("graphqlPlugin real protocol server", () => { }); expect(result).toEqual({ - status: 200, + ok: true, data: { hello: "Hello Ada" }, - errors: null, }); const requests = yield* server.requests; @@ -425,7 +464,7 @@ describe("graphqlPlugin", () => { }, { onElicitation: "accept-all" }, ); - expect(result).toEqual({ toolCount: 2, namespace: "via_static" }); + expect(result).toEqual({ ok: true, data: { toolCount: 2, namespace: "via_static" } }); expect(yield* executor.graphql.getSource("via_static", String(userScope))).toBeNull(); expect((yield* executor.graphql.getSource("via_static", String(orgScope)))?.scope).toBe( orgScope, @@ -729,7 +768,7 @@ describe("graphqlPlugin", () => { }); expect(result).toMatchObject({ - status: 200, + ok: true, data: { hello: "Hello Ada" }, }); const requests = yield* server.requests; @@ -843,7 +882,7 @@ describe("graphqlPlugin", () => { }); expect(result).toMatchObject({ - status: 200, + ok: true, data: { hello: "Hello Ada" }, }); const requests = yield* server.requests; @@ -915,7 +954,7 @@ describe("graphqlPlugin", () => { }); expect(result).toMatchObject({ - status: 200, + ok: true, data: { hello: "Hello Ada" }, }); const requests = yield* server.requests; diff --git a/packages/plugins/graphql/src/sdk/plugin.ts b/packages/plugins/graphql/src/sdk/plugin.ts index 3b0981729..76fb6cf35 100644 --- a/packages/plugins/graphql/src/sdk/plugin.ts +++ b/packages/plugins/graphql/src/sdk/plugin.ts @@ -12,6 +12,7 @@ import { SecretId, SourceDetectionResult, StorageError, + ToolResult, type PluginCtx, type StorageFailure, type ToolAnnotations, @@ -65,6 +66,19 @@ import { // Plugin config // --------------------------------------------------------------------------- +const GraphqlErrorBody = Schema.Struct({ message: Schema.String }); +const GraphqlErrorsBody = Schema.Array(Schema.Unknown); +const decodeGraphqlErrorBody = Schema.decodeUnknownOption(GraphqlErrorBody); +const decodeGraphqlErrorsBody = Schema.decodeUnknownOption(GraphqlErrorsBody); + +const decodeGraphqlErrors = (errors: unknown): readonly unknown[] | undefined => + Option.getOrUndefined(decodeGraphqlErrorsBody(errors)); + +const extractGraphqlErrorMessage = (errors: readonly unknown[]): string | undefined => + errors + .map((error) => Option.getOrUndefined(decodeGraphqlErrorBody(error))?.message) + .find((message) => message !== undefined && message.length > 0); + export type HeaderValue = HeaderValueValue; export type GraphqlCredentialValue = ConfiguredGraphqlCredentialValue; @@ -994,7 +1008,7 @@ export const graphqlPlugin = definePlugin((options?: GraphqlPluginOptions) => { }, inputSchema: StaticAddSourceInputStandardSchema, outputSchema: StaticAddSourceOutputStandardSchema, - execute: (input) => self.addSource(input), + execute: (input) => Effect.map(self.addSource(input), ToolResult.ok), }), ], }, @@ -1057,7 +1071,28 @@ export const graphqlPlugin = definePlugin((options?: GraphqlPluginOptions) => { httpClientLayer, ); - return result; + const errors = decodeGraphqlErrors(result.errors); + if (errors !== undefined && errors.length > 0) { + const firstMessage = extractGraphqlErrorMessage(errors); + return ToolResult.fail({ + code: "graphql_errors", + message: firstMessage !== undefined ? firstMessage : "GraphQL request returned errors", + details: { errors }, + }); + } + if (result.status < 200 || result.status >= 300) { + return ToolResult.fail({ + code: "graphql_http_error", + status: result.status, + message: `GraphQL request failed with HTTP ${result.status}`, + details: { + status: result.status, + data: result.data, + errors: result.errors, + }, + }); + } + return ToolResult.ok(result.data); }), resolveAnnotations: ({ ctx, sourceId, toolRows }) => diff --git a/packages/plugins/mcp/src/react/AddMcpSource.tsx b/packages/plugins/mcp/src/react/AddMcpSource.tsx index 2bb7f8ea3..6efbb94b0 100644 --- a/packages/plugins/mcp/src/react/AddMcpSource.tsx +++ b/packages/plugins/mcp/src/react/AddMcpSource.tsx @@ -55,6 +55,13 @@ import { MCP_OAUTH_CONNECTION_SLOT, type McpCredentialInput } from "../sdk/types const ErrorMessage = Schema.Struct({ message: Schema.String }); const decodeErrorMessage = Schema.decodeUnknownOption(ErrorMessage); +const STDIO_ENV_ESCAPE_REPLACEMENTS: Readonly> = { + "\\": "\\", + n: "\n", + r: "\r", + t: "\t", + '"': '"', +}; const errorMessageFromExit = (exit: Exit.Exit, fallback: string): string => Option.match(Option.flatMap(Exit.findErrorOption(exit), decodeErrorMessage), { @@ -525,22 +532,10 @@ export default function AddMcpSource(props: { const inner = value.slice(1, -1); if (quote === "'") return inner; - return inner.replace(/\\([\\nrt"])/g, (_, escaped: string) => { - switch (escaped) { - case "\\": - return "\\"; - case "n": - return "\n"; - case "r": - return "\r"; - case "t": - return "\t"; - case '"': - return '"'; - default: - return escaped; - } - }); + return inner.replace( + /\\([\\nrt"])/g, + (_, escaped: string) => STDIO_ENV_ESCAPE_REPLACEMENTS[escaped] ?? escaped, + ); }; const parseStdioEnv = (raw: string): Record | undefined => { diff --git a/packages/plugins/mcp/src/sdk/elicitation.test.ts b/packages/plugins/mcp/src/sdk/elicitation.test.ts index f1d92029e..34b008f4b 100644 --- a/packages/plugins/mcp/src/sdk/elicitation.test.ts +++ b/packages/plugins/mcp/src/sdk/elicitation.test.ts @@ -69,7 +69,8 @@ describe("MCP elicitation (end-to-end)", () => { const result = yield* executor.tools.invoke(gatedEcho!.id, { value: "hello" }, options); expect(result).toMatchObject({ - content: [{ type: "text", text: "approved:hello" }], + ok: true, + data: { content: [{ type: "text", text: "approved:hello" }] }, }); // At least one elicitation should be the MCP server's form expect(elicitationMessages.length).toBeGreaterThanOrEqual(1); @@ -95,7 +96,8 @@ describe("MCP elicitation (end-to-end)", () => { ); expect(result).toMatchObject({ - content: [{ type: "text", text: "denied:nope" }], + ok: true, + data: { content: [{ type: "text", text: "denied:nope" }] }, }); }), ); @@ -114,7 +116,32 @@ describe("MCP elicitation (end-to-end)", () => { ); expect(result).toMatchObject({ - content: [{ type: "text", text: "plain" }], + ok: true, + data: { content: [{ type: "text", text: "plain" }] }, + }); + }), + ); + + it.effect("successful tool invocation preserves structured MCP result fields", () => + Effect.gen(function* () { + const server = yield* serveElicitationTestServer; + const executor = yield* makeTestExecutor(server.url); + const tools = yield* executor.tools.list(); + const structuredEcho = tools.find((t) => t.name === "structured_echo")!; + + const result = yield* executor.tools.invoke( + structuredEcho.id, + { value: "plain" }, + { onElicitation: "accept-all" }, + ); + + expect(result).toMatchObject({ + ok: true, + data: { + content: [{ type: "text", text: "plain" }], + structuredContent: { value: "plain", upper: "PLAIN" }, + _meta: { trace: "kept" }, + }, }); }), ); diff --git a/packages/plugins/mcp/src/sdk/per-user-auth-isolation.test.ts b/packages/plugins/mcp/src/sdk/per-user-auth-isolation.test.ts index af155b7c1..fc75f6154 100644 --- a/packages/plugins/mcp/src/sdk/per-user-auth-isolation.test.ts +++ b/packages/plugins/mcp/src/sdk/per-user-auth-isolation.test.ts @@ -115,7 +115,8 @@ describe("per-user MCP auth isolation", () => { { onElicitation: "accept-all" }, ); expect(userAResult).toMatchObject({ - content: [{ type: "text", text: "ok:from-user-a" }], + ok: true, + data: { content: [{ type: "text", text: "ok:from-user-a" }] }, }); expect( (yield* server.requests) @@ -187,7 +188,8 @@ describe("per-user MCP auth isolation", () => { { onElicitation: "accept-all" }, ); expect(userAResult).toMatchObject({ - content: [{ type: "text", text: "ok:user-a-header" }], + ok: true, + data: { content: [{ type: "text", text: "ok:user-a-header" }] }, }); expect( (yield* server.requests) @@ -267,7 +269,8 @@ describe("per-user MCP auth isolation", () => { ); expect(result).toMatchObject({ - content: [{ type: "text", text: "ok:org-header" }], + ok: true, + data: { content: [{ type: "text", text: "ok:org-header" }] }, }); const invokeRequests = (yield* server.requests).slice(beforeInvoke); expect( diff --git a/packages/plugins/mcp/src/sdk/plugin.ts b/packages/plugins/mcp/src/sdk/plugin.ts index 3e9776091..ee13f962e 100644 --- a/packages/plugins/mcp/src/sdk/plugin.ts +++ b/packages/plugins/mcp/src/sdk/plugin.ts @@ -8,6 +8,7 @@ import { Predicate, Result, Scope, + Schema, ScopedCache, } from "effect"; import type { HttpClient } from "effect/unstable/http"; @@ -22,6 +23,7 @@ import { ScopeId, SecretId, SourceDetectionResult, + ToolResult, definePlugin, resolveSecretBackedMap as resolveSharedSecretBackedMap, type PluginCtx, @@ -193,6 +195,24 @@ const toBinding = (entry: McpToolManifestEntry): McpToolBinding => }); const MCP_PLUGIN_ID = "mcp"; +const McpTextContent = Schema.Struct({ type: Schema.Literal("text"), text: Schema.String }); +const McpToolCallEnvelope = Schema.Struct({ + isError: Schema.optional(Schema.Boolean), + content: Schema.optional(Schema.Array(Schema.Unknown)), +}); + +const decodeMcpTextContent = Schema.decodeUnknownOption(McpTextContent); +const decodeMcpToolCallEnvelope = Schema.decodeUnknownOption(McpToolCallEnvelope); + +const extractMcpErrorMessage = (content: unknown): string => { + if (Array.isArray(content)) { + for (const item of content) { + const decoded = Option.getOrUndefined(decodeMcpTextContent(item)); + if (decoded !== undefined && decoded.text.length > 0) return decoded.text; + } + } + return "MCP tool returned an error"; +}; /** Match `token` as a separator-bounded run inside a URL hostname or path, * used as a low-confidence detection hint when wire-shape detection fails. @@ -1726,7 +1746,7 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { }); } - return yield* invokeMcpTool({ + const raw = yield* invokeMcpTool({ toolId: toolRow.id, toolName: entry.binding.toolName, args, @@ -1764,6 +1784,16 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { pendingConnectors: runtime.pendingConnectors, elicit, }); + + const envelope = Option.getOrUndefined(decodeMcpToolCallEnvelope(raw)); + if (envelope?.isError === true) { + return ToolResult.fail({ + code: "mcp_tool_error", + message: extractMcpErrorMessage(envelope.content), + details: { content: envelope.content }, + }); + } + return ToolResult.ok(raw); }).pipe( Effect.withSpan("mcp.plugin.invoke_tool", { attributes: { diff --git a/packages/plugins/mcp/src/testing/server.ts b/packages/plugins/mcp/src/testing/server.ts index f4f46b35e..6222a9235 100644 --- a/packages/plugins/mcp/src/testing/server.ts +++ b/packages/plugins/mcp/src/testing/server.ts @@ -375,6 +375,23 @@ export const makeElicitationMcpServer = () => { }), ); + server.registerTool( + "structured_echo", + { + description: "Returns text plus structured data", + inputSchema: { value: z.string() }, + outputSchema: { + value: z.string(), + upper: z.string(), + }, + }, + async ({ value }: { value: string }) => ({ + content: [{ type: "text" as const, text: value }], + structuredContent: { value, upper: value.toUpperCase() }, + _meta: { trace: "kept" }, + }), + ); + return server; }; diff --git a/packages/plugins/openapi/src/sdk/client-credentials-oauth.test.ts b/packages/plugins/openapi/src/sdk/client-credentials-oauth.test.ts index 82aaeb424..e58d90fe0 100644 --- a/packages/plugins/openapi/src/sdk/client-credentials-oauth.test.ts +++ b/packages/plugins/openapi/src/sdk/client-credentials-oauth.test.ts @@ -26,6 +26,7 @@ import { makeTestConfig, serveOAuthTestServer } from "@executor-js/sdk/testing"; import { addOpenApiTestSource, serveOpenApiHttpApiTestServer, + unwrapInvocation, } from "@executor-js/plugin-openapi/testing"; import { openApiPlugin } from "./plugin"; @@ -226,25 +227,21 @@ describe("OpenAPI client_credentials OAuth", () => { ); // Invoking the tool injects the freshly-minted bearer via // ctx.connections.accessToken. - const result = (yield* userExec.tools.invoke( - "petstore.items.echoHeaders", - {}, - autoApprove, - )) as { - data: { authorization?: string } | null; - error: unknown; - }; + const result = unwrapInvocation( + yield* userExec.tools.invoke("petstore.items.echoHeaders", {}, autoApprove), + ); expect(result.error).toBeNull(); - const bearer = result.data?.authorization?.replace(/^Bearer\s+/i, ""); + const data = result.data as EchoHeaders | null; + const bearer = data?.authorization?.replace(/^Bearer\s+/i, ""); expect(bearer).toBeDefined(); expect(yield* oauth.acceptsAccessToken(bearer!)).toBe(true); // The connection lives at the innermost (user) scope, which // preserves per-user credential resolution: if each user has - // their own `dealcloud_client_id`/`dealcloud_client_secret` - // shadowed at their user scope, each user mints their own - // token. A single shared connection slot still lets every caller - // reach the right physical row through scoped credential bindings. + // their own OAuth client credentials shadowed at their user + // scope, each user mints their own token. A single shared + // connection slot still lets every caller reach the right + // physical row through scoped credential bindings. const userConnections = yield* userExec.connections.list(); const connection = userConnections.find((c) => c.id === completedConnection.connectionId); expect(connection).toBeDefined(); diff --git a/packages/plugins/openapi/src/sdk/multi-scope-bearer.test.ts b/packages/plugins/openapi/src/sdk/multi-scope-bearer.test.ts index 9ad28e255..35d648c7b 100644 --- a/packages/plugins/openapi/src/sdk/multi-scope-bearer.test.ts +++ b/packages/plugins/openapi/src/sdk/multi-scope-bearer.test.ts @@ -38,6 +38,7 @@ import { addOpenApiTestSource, makeOpenApiTestSourceConfig, serveOpenApiHttpApiTestServer, + unwrapInvocation, } from "@executor-js/plugin-openapi/testing"; import { openApiPlugin } from "./plugin"; @@ -277,25 +278,21 @@ describe("OpenAPI multi-scope bearer (Vercel-style)", () => { // yields her token; bob's scope yields his. Same source, same // tool, different injected bearer. // ------------------------------------------------------------- - const aliceResult = (yield* aliceExec.tools.invoke( - "vercel.projects.list", - {}, - autoApprove, - )) as { - data: { authorization?: string; token?: string } | null; - error: unknown; - }; + const aliceResult = unwrapInvocation( + yield* aliceExec.tools.invoke("vercel.projects.list", {}, autoApprove), + ); expect(aliceResult.error).toBeNull(); - expect(aliceResult.data?.authorization).toBe("Bearer alice-vercel-token"); - expect(aliceResult.data?.token).toBe("alice-team"); + const aliceData = aliceResult.data as EchoHeaders | null; + expect(aliceData?.authorization).toBe("Bearer alice-vercel-token"); + expect(aliceData?.token).toBe("alice-team"); - const bobResult = (yield* bobExec.tools.invoke("vercel.projects.list", {}, autoApprove)) as { - data: { authorization?: string; token?: string } | null; - error: unknown; - }; + const bobResult = unwrapInvocation( + yield* bobExec.tools.invoke("vercel.projects.list", {}, autoApprove), + ); expect(bobResult.error).toBeNull(); - expect(bobResult.data?.authorization).toBe("Bearer bob-vercel-token"); - expect(bobResult.data?.token).toBe("bob-team"); + const bobData = bobResult.data as EchoHeaders | null; + expect(bobData?.authorization).toBe("Bearer bob-vercel-token"); + expect(bobData?.token).toBe("bob-team"); // ------------------------------------------------------------- // 5. Scope attribution: each user's token is pinned to their @@ -450,20 +447,19 @@ describe("OpenAPI multi-scope bearer (Vercel-style)", () => { }), ); - const aliceResult = (yield* aliceExec.tools.invoke( - "vercel.projects.list", - {}, - autoApprove, - )) as { data: { authorization?: string } | null; error: unknown }; + const aliceResult = unwrapInvocation( + yield* aliceExec.tools.invoke("vercel.projects.list", {}, autoApprove), + ); expect(aliceResult.error).toBeNull(); - expect(aliceResult.data?.authorization).toBe("Bearer alice-vercel-token"); + expect((aliceResult.data as EchoHeaders | null)?.authorization).toBe( + "Bearer alice-vercel-token", + ); - const bobResult = (yield* bobExec.tools.invoke("vercel.projects.list", {}, autoApprove)) as { - data: { authorization?: string } | null; - error: unknown; - }; + const bobResult = unwrapInvocation( + yield* bobExec.tools.invoke("vercel.projects.list", {}, autoApprove), + ); expect(bobResult.error).toBeNull(); - expect(bobResult.data?.authorization).toBe("Bearer bob-vercel-token"); + expect((bobResult.data as EchoHeaders | null)?.authorization).toBe("Bearer bob-vercel-token"); }), ); @@ -558,13 +554,11 @@ describe("OpenAPI multi-scope bearer (Vercel-style)", () => { }), ); - const sharedResult = (yield* aliceExec.tools.invoke( - "vercel.projects.list", - {}, - autoApprove, - )) as { data: { authorization?: string } | null; error: unknown }; + const sharedResult = unwrapInvocation( + yield* aliceExec.tools.invoke("vercel.projects.list", {}, autoApprove), + ); expect(sharedResult.error).toBeNull(); - expect(sharedResult.data?.authorization).toBe("Bearer org-token"); + expect((sharedResult.data as EchoHeaders | null)?.authorization).toBe("Bearer org-token"); yield* aliceExec.secrets.set( SetSecretInput.make({ @@ -587,13 +581,13 @@ describe("OpenAPI multi-scope bearer (Vercel-style)", () => { }), ); - const overrideResult = (yield* aliceExec.tools.invoke( - "vercel.projects.list", - {}, - autoApprove, - )) as { data: { authorization?: string } | null; error: unknown }; + const overrideResult = unwrapInvocation( + yield* aliceExec.tools.invoke("vercel.projects.list", {}, autoApprove), + ); expect(overrideResult.error).toBeNull(); - expect(overrideResult.data?.authorization).toBe("Bearer alice-token"); + expect((overrideResult.data as EchoHeaders | null)?.authorization).toBe( + "Bearer alice-token", + ); yield* aliceExec.openapi.removeSourceBinding( "vercel", @@ -602,13 +596,11 @@ describe("OpenAPI multi-scope bearer (Vercel-style)", () => { String(aliceScope.id), ); - const fallbackResult = (yield* aliceExec.tools.invoke( - "vercel.projects.list", - {}, - autoApprove, - )) as { data: { authorization?: string } | null; error: unknown }; + const fallbackResult = unwrapInvocation( + yield* aliceExec.tools.invoke("vercel.projects.list", {}, autoApprove), + ); expect(fallbackResult.error).toBeNull(); - expect(fallbackResult.data?.authorization).toBe("Bearer org-token"); + expect((fallbackResult.data as EchoHeaders | null)?.authorization).toBe("Bearer org-token"); yield* aliceExec.openapi.setSourceBinding( OpenApiSourceBindingInput.make({ @@ -812,12 +804,11 @@ describe("OpenAPI multi-scope bearer (Vercel-style)", () => { }), ); - const result = (yield* aliceExec.tools.invoke("vercel.projects.list", {}, autoApprove)) as { - data: { authorization?: string } | null; - error: unknown; - }; + const result = unwrapInvocation( + yield* aliceExec.tools.invoke("vercel.projects.list", {}, autoApprove), + ); expect(result.error).toBeNull(); - expect(result.data?.authorization).toBe("Bearer alice-token"); + expect((result.data as EchoHeaders | null)?.authorization).toBe("Bearer alice-token"); }), ); @@ -914,13 +905,12 @@ describe("OpenAPI multi-scope bearer (Vercel-style)", () => { }), ); - const result = (yield* userExec.tools.invoke("vercel.projects.list", {}, autoApprove)) as { - data: { authorization?: string } | null; - error: unknown; - }; + const result = unwrapInvocation( + yield* userExec.tools.invoke("vercel.projects.list", {}, autoApprove), + ); expect(result.error).toBeNull(); - expect(result.data?.authorization).toBe("Bearer org-token"); + expect((result.data as EchoHeaders | null)?.authorization).toBe("Bearer org-token"); }), ); @@ -1013,13 +1003,12 @@ describe("OpenAPI multi-scope bearer (Vercel-style)", () => { }), ); - const result = (yield* userExec.tools.invoke("vercel.projects.list", {}, autoApprove)) as { - data: { authorization?: string } | null; - error: unknown; - }; + const result = unwrapInvocation( + yield* userExec.tools.invoke("vercel.projects.list", {}, autoApprove), + ); expect(result.error).toBeNull(); - expect(result.data?.authorization).toBe("Bearer org-choice"); + expect((result.data as EchoHeaders | null)?.authorization).toBe("Bearer org-choice"); }), ); }); diff --git a/packages/plugins/openapi/src/sdk/multi-scope-oauth.test.ts b/packages/plugins/openapi/src/sdk/multi-scope-oauth.test.ts index 6badab146..85cde6e4d 100644 --- a/packages/plugins/openapi/src/sdk/multi-scope-oauth.test.ts +++ b/packages/plugins/openapi/src/sdk/multi-scope-oauth.test.ts @@ -28,6 +28,7 @@ import { makeTestConfig, serveOAuthTestServer } from "@executor-js/sdk/testing"; import { addOpenApiTestSource, serveOpenApiHttpApiTestServer, + unwrapInvocation, } from "@executor-js/plugin-openapi/testing"; import { openApiPlugin } from "./plugin"; @@ -307,23 +308,25 @@ describe("OpenAPI multi-scope OAuth", () => { // 4. Invoke through each exec — Authorization must carry that // user's token. // ------------------------------------------------------------- - const aliceResult = (yield* aliceExec.tools.invoke( - "petstore.items.echoHeaders", - {}, - autoApprove, - )) as { data: { authorization?: string } | null; error: unknown }; + const aliceResult = unwrapInvocation( + yield* aliceExec.tools.invoke("petstore.items.echoHeaders", {}, autoApprove), + ); expect(aliceResult.error).toBeNull(); - const aliceBearer = aliceResult.data?.authorization?.replace(/^Bearer\s+/i, ""); + const aliceBearer = (aliceResult.data as EchoHeaders | null)?.authorization?.replace( + /^Bearer\s+/i, + "", + ); expect(aliceBearer).toBeDefined(); expect(yield* oauth.acceptsAccessToken(aliceBearer!)).toBe(true); - const bobResult = (yield* bobExec.tools.invoke( - "petstore.items.echoHeaders", - {}, - autoApprove, - )) as { data: { authorization?: string } | null; error: unknown }; + const bobResult = unwrapInvocation( + yield* bobExec.tools.invoke("petstore.items.echoHeaders", {}, autoApprove), + ); expect(bobResult.error).toBeNull(); - const bobBearer = bobResult.data?.authorization?.replace(/^Bearer\s+/i, ""); + const bobBearer = (bobResult.data as EchoHeaders | null)?.authorization?.replace( + /^Bearer\s+/i, + "", + ); expect(bobBearer).toBeDefined(); expect(yield* oauth.acceptsAccessToken(bobBearer!)).toBe(true); expect(bobBearer).not.toBe(aliceBearer); @@ -610,23 +613,25 @@ describe("OpenAPI multi-scope OAuth", () => { // (4) Each user's invocation resolves their OWN row and gets // their OWN token — not whatever the last signer happened to // mint. This is the core multi-user regression. - const aliceResult = (yield* aliceExec.tools.invoke( - "petstore.items.echoHeaders", - {}, - autoApprove, - )) as { data: { authorization?: string } | null; error: unknown }; + const aliceResult = unwrapInvocation( + yield* aliceExec.tools.invoke("petstore.items.echoHeaders", {}, autoApprove), + ); expect(aliceResult.error).toBeNull(); - const aliceBearer = aliceResult.data?.authorization?.replace(/^Bearer\s+/i, ""); + const aliceBearer = (aliceResult.data as EchoHeaders | null)?.authorization?.replace( + /^Bearer\s+/i, + "", + ); expect(aliceBearer).toBeDefined(); expect(yield* oauth.acceptsAccessToken(aliceBearer!)).toBe(true); - const bobResult = (yield* bobExec.tools.invoke( - "petstore.items.echoHeaders", - {}, - autoApprove, - )) as { data: { authorization?: string } | null; error: unknown }; + const bobResult = unwrapInvocation( + yield* bobExec.tools.invoke("petstore.items.echoHeaders", {}, autoApprove), + ); expect(bobResult.error).toBeNull(); - const bobBearer = bobResult.data?.authorization?.replace(/^Bearer\s+/i, ""); + const bobBearer = (bobResult.data as EchoHeaders | null)?.authorization?.replace( + /^Bearer\s+/i, + "", + ); expect(bobBearer).toBeDefined(); expect(yield* oauth.acceptsAccessToken(bobBearer!)).toBe(true); expect(bobBearer).not.toBe(aliceBearer); diff --git a/packages/plugins/openapi/src/sdk/oauth-refresh.test.ts b/packages/plugins/openapi/src/sdk/oauth-refresh.test.ts index 45ffbf994..0ba99b4f7 100644 --- a/packages/plugins/openapi/src/sdk/oauth-refresh.test.ts +++ b/packages/plugins/openapi/src/sdk/oauth-refresh.test.ts @@ -35,6 +35,7 @@ import { makeTestConfig, serveOAuthTestServer } from "@executor-js/sdk/testing"; import { addOpenApiTestSource, serveOpenApiHttpApiTestServer, + unwrapInvocation, } from "@executor-js/plugin-openapi/testing"; import { openApiPlugin } from "./plugin"; @@ -253,17 +254,16 @@ describe("OpenAPI oauth refresh", () => { }); yield* bindOAuthConnection(executor, scopeId, "conn-refresh-ok", auth); - const result = (yield* executor.tools.invoke( - "petstore.items.echoHeaders", - {}, - autoApprove, - )) as { data: { authorization?: string } | null; error: unknown }; + const result = unwrapInvocation( + yield* executor.tools.invoke("petstore.items.echoHeaders", {}, autoApprove), + ); expect(result.error).toBeNull(); + const data = result.data as EchoHeaders | null; // Proves the refresh landed: invoke carried the fresh token, // not the expired one we seeded. - expect(result.data?.authorization).not.toBe("Bearer expired-access-v1"); - const bearer = result.data?.authorization?.replace(/^Bearer\s+/i, ""); + expect(data?.authorization).not.toBe("Bearer expired-access-v1"); + const bearer = data?.authorization?.replace(/^Bearer\s+/i, ""); expect(bearer).toBeDefined(); expect(yield* oauth.acceptsAccessToken(bearer!)).toBe(true); const calls = refreshTokenRequests(yield* oauth.requests); @@ -313,12 +313,9 @@ describe("OpenAPI oauth refresh", () => { ); for (const r of invokes) { - const res = r as { - data: { authorization?: string } | null; - error: unknown; - }; + const res = unwrapInvocation(r); expect(res.error).toBeNull(); - const bearer = res.data?.authorization?.replace(/^Bearer\s+/i, ""); + const bearer = (res.data as EchoHeaders | null)?.authorization?.replace(/^Bearer\s+/i, ""); expect(bearer).toBeDefined(); expect(yield* oauth.acceptsAccessToken(bearer!)).toBe(true); } diff --git a/packages/plugins/openapi/src/sdk/plugin.test.ts b/packages/plugins/openapi/src/sdk/plugin.test.ts index 101b4d679..4aee8f035 100644 --- a/packages/plugins/openapi/src/sdk/plugin.test.ts +++ b/packages/plugins/openapi/src/sdk/plugin.test.ts @@ -25,6 +25,7 @@ import { addOpenApiTestSource, makeOpenApiHttpApiTestSourceConfig, serveOpenApiHttpApiTestServer, + unwrapInvocation, } from "../testing"; const autoApprove: InvokeOptions = { onElicitation: "accept-all" }; @@ -267,13 +268,15 @@ describe("OpenAPI Plugin", () => { }), ); - const result = (yield* executor.tools.invoke( - "executor.openapi.previewSpec", - { spec: testApiSpec() }, - autoApprove, - )) as { operationCount: number }; + const preview = unwrapInvocation( + yield* executor.tools.invoke( + "executor.openapi.previewSpec", + { spec: testApiSpec() }, + autoApprove, + ), + ).data as { operationCount: number }; - expect(result.operationCount).toBeGreaterThanOrEqual(2); + expect(preview.operationCount).toBeGreaterThanOrEqual(2); }), ); @@ -316,11 +319,13 @@ describe("OpenAPI Plugin", () => { }), ); - const result = (yield* executor.tools.invoke( - "executor.openapi.addSource", - testApiSourceConfig({ scope: String(orgScope), namespace: "runtime" }), - autoApprove, - )) as { sourceId: string; toolCount: number }; + const result = unwrapInvocation( + yield* executor.tools.invoke( + "executor.openapi.addSource", + testApiSourceConfig({ scope: String(orgScope), namespace: "runtime" }), + autoApprove, + ), + ).data as { sourceId: string; toolCount: number }; expect(result).toEqual({ sourceId: "runtime", toolCount: 4 }); expect(yield* executor.openapi.getSource("runtime", String(userScope))).toBeNull(); @@ -557,17 +562,12 @@ describe("OpenAPI Plugin", () => { }, }); - const result = (yield* executor.tools.invoke( - "authed.items.echoHeaders", - {}, - autoApprove, - )) as { - data: { authorization?: string; "x-static"?: string } | null; - error: unknown; - }; + const result = unwrapInvocation( + yield* executor.tools.invoke("authed.items.echoHeaders", {}, autoApprove), + ); expect(result.error).toBeNull(); - const data = result.data!; + const data = result.data as { authorization?: string; "x-static"?: string }; expect(data.authorization).toBe("Bearer secret-value-123"); expect(data["x-static"]).toBe("hello"); }), @@ -751,10 +751,9 @@ describe("OpenAPI Plugin", () => { namespace: "test", }); - const result = (yield* executor.tools.invoke("test.items.listItems", {}, autoApprove)) as { - data: unknown; - error: unknown; - }; + const result = unwrapInvocation( + yield* executor.tools.invoke("test.items.listItems", {}, autoApprove), + ); expect(result.error).toBeNull(); expect(result.data).toEqual(ITEMS); }), @@ -780,11 +779,9 @@ describe("OpenAPI Plugin", () => { namespace: "test", }); - const result = (yield* executor.tools.invoke( - "test.items.getItem", - { itemId: "2" }, - autoApprove, - )) as { data: unknown; error: unknown }; + const result = unwrapInvocation( + yield* executor.tools.invoke("test.items.getItem", { itemId: "2" }, autoApprove), + ); expect(result.error).toBeNull(); expect(result.data).toEqual({ id: 2, name: "Gadget" }); }), @@ -810,16 +807,18 @@ describe("OpenAPI Plugin", () => { namespace: "records", }); - const result = (yield* executor.tools.invoke( - "records.items.queryRows", - { - entryTypeId: "18538", - query: JSON.stringify([{ DisplayName: "Example" }]), - limit: 10, - skip: 0, - }, - autoApprove, - )) as { data: unknown; error: unknown }; + const result = unwrapInvocation( + yield* executor.tools.invoke( + "records.items.queryRows", + { + entryTypeId: "18538", + query: JSON.stringify([{ DisplayName: "Example" }]), + limit: 10, + skip: 0, + }, + autoApprove, + ), + ); expect(result.data).toBeNull(); expect(result.error).toEqual( diff --git a/packages/plugins/openapi/src/sdk/plugin.ts b/packages/plugins/openapi/src/sdk/plugin.ts index 691afe89f..ee4260753 100644 --- a/packages/plugins/openapi/src/sdk/plugin.ts +++ b/packages/plugins/openapi/src/sdk/plugin.ts @@ -7,6 +7,7 @@ import { SecretId, SourceDetectionResult, StorageError, + ToolResult, definePlugin, tool, resolveSecretBackedMap, @@ -57,6 +58,77 @@ import { // Plugin config // --------------------------------------------------------------------------- +const STRINGIFIED_BODY_CAP = 1024; +const UpstreamMessageBody = Schema.Struct({ message: Schema.String }); +const UpstreamErrorMessageBody = Schema.Struct({ errorMessage: Schema.String }); +const UpstreamNestedErrorBody = Schema.Struct({ error: UpstreamMessageBody }); +const UpstreamErrorsArrayBody = Schema.Struct({ + errors: Schema.Array( + Schema.Struct({ + detail: Schema.optional(Schema.String), + message: Schema.optional(Schema.String), + title: Schema.optional(Schema.String), + }), + ), +}); +const UpstreamDescriptionBody = Schema.Struct({ + detail: Schema.optional(Schema.String), + title: Schema.optional(Schema.String), + description: Schema.optional(Schema.String), +}); + +const decodeUpstreamMessageBody = Schema.decodeUnknownOption(UpstreamMessageBody); +const decodeUpstreamErrorMessageBody = Schema.decodeUnknownOption(UpstreamErrorMessageBody); +const decodeUpstreamNestedErrorBody = Schema.decodeUnknownOption(UpstreamNestedErrorBody); +const decodeUpstreamErrorsArrayBody = Schema.decodeUnknownOption(UpstreamErrorsArrayBody); +const decodeUpstreamDescriptionBody = Schema.decodeUnknownOption(UpstreamDescriptionBody); + +const clampedStringify = (value: unknown): string => { + let s: string; + // oxlint-disable-next-line executor/no-try-catch-or-throw -- boundary: JSON.stringify may throw on cycles; fall back to String() so the upstream body can still be surfaced as ToolError.details fallback text + try { + s = JSON.stringify(value); + } catch { + s = String(value); + } + return s.length > STRINGIFIED_BODY_CAP ? `${s.slice(0, STRINGIFIED_BODY_CAP)}…` : s; +}; + +const firstNonEmpty = (...values: readonly (string | undefined)[]): string | undefined => + values.find((value) => value !== undefined && value.length > 0); + +// Walk known upstream error-body shapes so ToolError.message stays concise +// while ToolError.details preserves the original body. +const extractUpstreamMessage = (body: unknown, status: number): string => { + if (typeof body === "string") { + return body.length > 0 ? body : `Upstream returned HTTP ${status}`; + } + const nested = Option.getOrUndefined(decodeUpstreamNestedErrorBody(body)); + const messageBody = Option.getOrUndefined(decodeUpstreamMessageBody(body)); + const errorMessageBody = Option.getOrUndefined(decodeUpstreamErrorMessageBody(body)); + const errorsBody = Option.getOrUndefined(decodeUpstreamErrorsArrayBody(body)); + const descriptionBody = Option.getOrUndefined(decodeUpstreamDescriptionBody(body)); + const arrayMessage = errorsBody?.errors + .map(({ detail, message: upstreamMessage, title }) => + firstNonEmpty(detail, upstreamMessage, title), + ) + .find((message) => message !== undefined); + const message = firstNonEmpty( + nested?.error.message, + messageBody?.message, + errorMessageBody?.errorMessage, + arrayMessage, + descriptionBody?.detail, + descriptionBody?.title, + descriptionBody?.description, + ); + if (message !== undefined) return message; + if (body !== null && typeof body === "object") { + return clampedStringify(body); + } + return `Upstream returned HTTP ${status}`; +}; + export type HeaderValue = HeaderValueValue; export type ConfiguredHeaderValue = ConfiguredHeaderValueValue; export type OpenApiHeaderInput = HeaderValue | ConfiguredHeaderValue; @@ -1193,7 +1265,7 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { name: "previewSpec", description: "Preview an OpenAPI document before adding it as a source", inputSchema: PreviewSpecInputStandardSchema, - execute: (input) => self.previewSpec(input), + execute: (input) => Effect.map(self.previewSpec(input), ToolResult.ok), }), tool({ name: "addSource", @@ -1204,7 +1276,7 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { }, inputSchema: AddSourceInputStandardSchema, outputSchema: AddSourceOutputStandardSchema, - execute: (input) => self.addSpec(input), + execute: (input) => Effect.map(self.addSpec(input), ToolResult.ok), }), ], }, @@ -1283,7 +1355,20 @@ export const openApiPlugin = definePlugin((options?: OpenApiPluginOptions) => { httpClientLayer, ); - return result; + const ok = result.status >= 200 && result.status < 300; + if (!ok) { + return ToolResult.fail({ + code: "upstream_http_error", + status: result.status, + message: extractUpstreamMessage(result.error, result.status), + details: result.error, + }); + } + return ToolResult.ok({ + status: result.status, + headers: result.headers, + data: result.data, + }); }), resolveAnnotations: ({ ctx, sourceId, toolRows }) => diff --git a/packages/plugins/openapi/src/sdk/upstream-failures.test.ts b/packages/plugins/openapi/src/sdk/upstream-failures.test.ts index 4d9740c2b..3a8e16cce 100644 --- a/packages/plugins/openapi/src/sdk/upstream-failures.test.ts +++ b/packages/plugins/openapi/src/sdk/upstream-failures.test.ts @@ -37,6 +37,7 @@ import { makeOpenApiTestSourceConfig, type OpenApiTestServerShape, serveOpenApiHttpApiTestServer, + unwrapInvocation, } from "../testing"; import { openApiPlugin } from "./plugin"; @@ -303,10 +304,10 @@ describe("OpenAPI upstream failure modes", () => { const { baseUrl } = yield* slowServer; const executor = yield* buildExecutor(baseUrl); - const result = yield* executor.tools.invoke("f.things.listThings", {}, autoApprove); - // Empty array via .data envelope or directly — accept either shape. - const data = (result as { data?: unknown }).data ?? result; - expect(data).toEqual([]); + const result = unwrapInvocation( + yield* executor.tools.invoke("f.things.listThings", {}, autoApprove), + ); + expect(result.data).toEqual([]); }), ); }); diff --git a/packages/plugins/openapi/src/testing/index.ts b/packages/plugins/openapi/src/testing/index.ts index fbae84dc4..fb3a45037 100644 --- a/packages/plugins/openapi/src/testing/index.ts +++ b/packages/plugins/openapi/src/testing/index.ts @@ -1,4 +1,4 @@ -import { Context, Data, Effect, Layer, Predicate, Ref, Schema, Scope } from "effect"; +import { Context, Data, Effect, Layer, Option, Predicate, Ref, Schema, Scope } from "effect"; import { HttpClient, HttpRouter, @@ -13,7 +13,7 @@ import { OpenApi, } from "effect/unstable/httpapi"; import { OAuthTestServer, serveTestHttpServerLayer } from "@executor-js/sdk/testing"; -import type { ScopeId } from "@executor-js/sdk/core"; +import { isToolResult, type ScopeId } from "@executor-js/sdk/core"; import type { OpenApiPluginExtension, OpenApiSpecConfig } from "../sdk/plugin"; export class OpenApiTestServerAddressError extends Data.TaggedError( @@ -618,3 +618,55 @@ export const TestLayers = { echo: OpenApiEchoTestServer.layer, echoWithOAuth: OpenApiEchoTestServer.layerWithOAuth, }; + +const OpenApiTransportEnvelope = Schema.Struct({ + status: Schema.Number, + headers: Schema.Record(Schema.String, Schema.String), + data: Schema.Unknown, +}); + +const decodeOpenApiTransportEnvelope = Schema.decodeUnknownOption(OpenApiTransportEnvelope); + +export interface OpenApiInvocationResult | unknown[] | null> { + readonly status: number | null; + readonly headers: Record | null; + readonly data: TData; + readonly error: unknown; +} + +export const unwrapInvocation = | null>( + raw: unknown, +): OpenApiInvocationResult => { + if (!isToolResult(raw)) { + return { + status: null, + headers: null, + data: raw as TData, + error: null, + }; + } + if (raw.ok) { + const inner = raw.data; + const wrapped = Option.getOrUndefined(decodeOpenApiTransportEnvelope(inner)); + if (wrapped !== undefined) { + return { + status: wrapped.status, + headers: wrapped.headers, + data: wrapped.data as TData, + error: null, + }; + } + return { + status: null, + headers: null, + data: inner as TData, + error: null, + }; + } + return { + status: raw.error.status ?? null, + headers: null, + data: null as TData, + error: raw.error.details ?? raw.error, + }; +}; diff --git a/packages/plugins/workos-vault/src/sdk/secret-store.test.ts b/packages/plugins/workos-vault/src/sdk/secret-store.test.ts index c612398f1..2d6cb3637 100644 --- a/packages/plugins/workos-vault/src/sdk/secret-store.test.ts +++ b/packages/plugins/workos-vault/src/sdk/secret-store.test.ts @@ -256,7 +256,7 @@ describe("WorkOS Vault secret provider", () => { const client = makeFakeClient({ rejectReadNamesLongerThan: 80 }); const executor = yield* makeExecutor(client); const longSecretId = SecretId.make( - "openapi-oauth-dealcloud-api-oauth2-user-org-user-01kp6xm1zpvqvtpj77f0yv4eax.access_token", + "openapi-oauth-example-api-oauth2-user-org-user-01kp6xm1zpvqvtpj77f0yv4eax.access_token", ); yield* executor.secrets.set(