Skip to content

Commit 83a168f

Browse files
committed
Fix tool result contracts
1 parent 3030c8e commit 83a168f

17 files changed

Lines changed: 336 additions & 54 deletions

File tree

apps/cloud/src/services/sources-api.node.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ describe("sources api (HTTP)", () => {
398398
status: "completed",
399399
result: {
400400
ok: true,
401-
data: [{ type: "text", text: "cloud-mcp-ok" }],
401+
data: { content: [{ type: "text", text: "cloud-mcp-ok" }] },
402402
},
403403
});
404404
expect((yield* server.requests).length).toBeGreaterThanOrEqual(2);

packages/core/execution/src/description.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ const formatDescription = (sources: readonly Source[]): string => {
4848
"- `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`.",
4949
'- When you already know the namespace, narrow with `tools.search({ namespace: "github", query: "issues" })`.',
5050
"- `tools.executor.sources.list()` returns the same paged shape: `{ items: [{ id, toolCount, ... }], total, hasMore, nextOffset }`.",
51+
"- 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`.",
5152
"- 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 })`.",
5253
"- Always use the namespace prefix when calling tools: `tools.<namespace>.<tool>(args)`. Example: `tools.home_assistant_rest_api.states.getState(...)` — not `tools.states.getState(...)`.",
5354
"- The `tools` object is a lazy proxy — `Object.keys(tools)` won't work. Use `tools.search()` or `tools.executor.sources.list()` instead.",

packages/core/execution/src/tool-invoker.test.ts

Lines changed: 154 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, expect, it } from "@effect/vitest";
22
import { Effect, Fiber, Schema } from "effect";
3+
import * as ts from "typescript";
34

45
import {
56
ElicitationResponse,
@@ -19,6 +20,10 @@ const RepoInputSchema = Schema.toStandardSchemaV1(
1920
Schema.toStandardJSONSchemaV1(Schema.Struct({ owner: Schema.String, repo: Schema.String })),
2021
);
2122

23+
const RepoDetailsOutputSchema = Schema.toStandardSchemaV1(
24+
Schema.toStandardJSONSchemaV1(Schema.Struct({ defaultBranch: Schema.String })),
25+
);
26+
2227
const ContactInputSchema = Schema.toStandardSchemaV1(
2328
Schema.toStandardJSONSchemaV1(Schema.Struct({ email: Schema.String })),
2429
);
@@ -29,6 +34,58 @@ const EmptyInputSchema = Schema.toStandardSchemaV1(
2934

3035
const acceptAll = () => Effect.succeed(ElicitationResponse.make({ action: "accept" }));
3136

37+
type DescribedToolContract = {
38+
readonly outputTypeScript: string;
39+
readonly typeScriptDefinitions: Record<string, string>;
40+
};
41+
42+
const typeCheckDescribedInvocation = (
43+
described: DescribedToolContract,
44+
runtimeResult: unknown,
45+
consumerSource: string,
46+
): readonly string[] => {
47+
const fileName = "described-tool-contract.ts";
48+
const source = [
49+
...Object.entries(described.typeScriptDefinitions).map(([name, definition]) => {
50+
return `type ${name} = ${definition};`;
51+
}),
52+
`type ToolOutput = ${described.outputTypeScript};`,
53+
`const invokedResult: ToolOutput = ${JSON.stringify(runtimeResult)};`,
54+
consumerSource,
55+
].join("\n");
56+
57+
const options: ts.CompilerOptions = {
58+
module: ts.ModuleKind.ESNext,
59+
noEmit: true,
60+
skipLibCheck: true,
61+
strict: true,
62+
target: ts.ScriptTarget.ES2022,
63+
};
64+
const host = ts.createCompilerHost(options);
65+
const originalGetSourceFile = host.getSourceFile.bind(host);
66+
const originalReadFile = host.readFile.bind(host);
67+
const originalFileExists = host.fileExists.bind(host);
68+
69+
host.getSourceFile = (candidate, languageVersion, onError, shouldCreateNewSourceFile) => {
70+
if (candidate === fileName) {
71+
return ts.createSourceFile(candidate, source, languageVersion, true);
72+
}
73+
return originalGetSourceFile(candidate, languageVersion, onError, shouldCreateNewSourceFile);
74+
};
75+
host.readFile = (candidate) => (candidate === fileName ? source : originalReadFile(candidate));
76+
host.fileExists = (candidate) => candidate === fileName || originalFileExists(candidate);
77+
78+
const program = ts.createProgram([fileName], options, host);
79+
return ts.getPreEmitDiagnostics(program).map((diagnostic) => {
80+
const message = ts.flattenDiagnosticMessageText(diagnostic.messageText, "\n");
81+
if (!diagnostic.file || diagnostic.start === undefined) {
82+
return message;
83+
}
84+
const position = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start);
85+
return `${diagnostic.file.fileName}:${position.line + 1}:${position.character + 1} ${message}`;
86+
});
87+
};
88+
3289
// ---------------------------------------------------------------------------
3390
// Test plugins — each one declares a namespace as a static source with N
3491
// tools. Handlers return static data; the suite only cares about discovery
@@ -54,6 +111,7 @@ const githubPlugin = definePlugin(() => ({
54111
name: "getRepositoryDetails",
55112
description: "Get repository details including the default branch",
56113
inputSchema: RepoInputSchema,
114+
outputSchema: RepoDetailsOutputSchema,
57115
handler: () => Effect.succeed({ defaultBranch: "main" }),
58116
},
59117
{
@@ -342,8 +400,102 @@ describe("tool discovery", () => {
342400
expect(described.name).toBe("listRepositoryIssues");
343401
expect(described.description).toBe("List issues for a repository");
344402
expect(described.inputTypeScript).toBe("{ owner: string; repo: string; }");
345-
expect(described.outputTypeScript).toBeUndefined();
346-
expect(described.typeScriptDefinitions).toBeUndefined();
403+
expect(described.outputTypeScript).toBe(
404+
"{ ok: true; data: unknown } | { ok: false; error: ToolError }",
405+
);
406+
expect(described.typeScriptDefinitions).toEqual({
407+
ToolError:
408+
"{ code: string; message: string; status?: number; details?: unknown; retryable?: boolean }",
409+
});
410+
}),
411+
);
412+
413+
it.effect("describes a return type that accepts the sandbox invocation result", () =>
414+
Effect.gen(function* () {
415+
const executor = yield* makeSearchExecutor();
416+
const engine = createExecutionEngine({ executor, codeExecutor });
417+
418+
const execution = yield* engine.execute(
419+
[
420+
'const details = await tools.describe.tool({ path: "github.getRepositoryDetails" });',
421+
"const result = await tools.github.getRepositoryDetails({ owner: 'executor', repo: 'executor' });",
422+
"return {",
423+
" outputTypeScript: details.outputTypeScript,",
424+
" typeScriptDefinitions: details.typeScriptDefinitions,",
425+
" result,",
426+
"};",
427+
].join("\n"),
428+
{ onElicitation: acceptAll },
429+
);
430+
431+
expect(execution.error).toBeUndefined();
432+
const observed = execution.result as DescribedToolContract & { readonly result: unknown };
433+
const diagnostics = typeCheckDescribedInvocation(
434+
observed,
435+
observed.result,
436+
[
437+
"function readDefaultBranch(result: ToolOutput): string {",
438+
" if (!result.ok) return result.error.message;",
439+
" return result.data.defaultBranch;",
440+
"}",
441+
"readDefaultBranch(invokedResult);",
442+
].join("\n"),
443+
);
444+
expect(diagnostics).toEqual([]);
445+
}),
446+
);
447+
448+
it.effect(
449+
"describes an error-as-value return type that accepts sandbox invocation failures",
450+
() =>
451+
Effect.gen(function* () {
452+
const executor = yield* createExecutor(
453+
makeTestConfig({ plugins: [errorPlugin()] as const }),
454+
);
455+
const engine = createExecutionEngine({ executor, codeExecutor });
456+
457+
const execution = yield* engine.execute(
458+
[
459+
'const details = await tools.describe.tool({ path: "records.queryRows" });',
460+
"const result = await tools.records.queryRows({});",
461+
"return {",
462+
" outputTypeScript: details.outputTypeScript,",
463+
" typeScriptDefinitions: details.typeScriptDefinitions,",
464+
" result,",
465+
"};",
466+
].join("\n"),
467+
{ onElicitation: acceptAll },
468+
);
469+
470+
expect(execution.error).toBeUndefined();
471+
const observed = execution.result as DescribedToolContract & { readonly result: unknown };
472+
const diagnostics = typeCheckDescribedInvocation(
473+
observed,
474+
observed.result,
475+
[
476+
"function readToolResult(result: ToolOutput): unknown {",
477+
" if (!result.ok) return result.error.message;",
478+
" return result.data;",
479+
"}",
480+
"readToolResult(invokedResult);",
481+
].join("\n"),
482+
);
483+
expect(diagnostics).toEqual([]);
484+
}),
485+
);
486+
487+
it.effect("describes the ToolResult wrapper through the direct describe helper", () =>
488+
Effect.gen(function* () {
489+
const executor = yield* makeSearchExecutor();
490+
const described = yield* describeTool(executor, "github.getRepositoryDetails");
491+
492+
expect(described.outputTypeScript).toBe(
493+
"{ ok: true; data: { defaultBranch: string; } } | { ok: false; error: ToolError }",
494+
);
495+
expect(described.typeScriptDefinitions).toEqual({
496+
ToolError:
497+
"{ code: string; message: string; status?: number; details?: unknown; retryable?: boolean }",
498+
});
347499
}),
348500
);
349501

packages/core/execution/src/tool-invoker.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,18 @@ import type { SandboxToolInvoker } from "@executor-js/codemode-core";
1313
import { ExecutionToolError } from "./errors";
1414

1515
const OPAQUE_DEFECT_MESSAGE = "Internal tool error";
16+
const TOOL_ERROR_TYPESCRIPT =
17+
"{ code: string; message: string; status?: number; details?: unknown; retryable?: boolean }";
18+
19+
const wrapOutputTypeScript = (outputTypeScript?: string): string =>
20+
`{ ok: true; data: ${outputTypeScript ?? "unknown"} } | { ok: false; error: ToolError }`;
21+
22+
const withToolResultDefinitions = (
23+
definitions?: Record<string, string>,
24+
): Record<string, string> => ({
25+
...(definitions ?? {}),
26+
ToolError: TOOL_ERROR_TYPESCRIPT,
27+
});
1628

1729
const newCorrelationId = (): string => {
1830
// 8-hex-char correlation id; enough entropy to disambiguate within a
@@ -497,7 +509,7 @@ export const describeTool = Effect.fn("executor.tools.describe")(function* (
497509
name: schema.name ?? path,
498510
description: schema.description,
499511
inputTypeScript: schema.inputTypeScript,
500-
outputTypeScript: schema.outputTypeScript,
501-
typeScriptDefinitions: schema.typeScriptDefinitions,
512+
outputTypeScript: wrapOutputTypeScript(schema.outputTypeScript),
513+
typeScriptDefinitions: withToolResultDefinitions(schema.typeScriptDefinitions),
502514
};
503515
});

packages/kernel/runtime-dynamic-worker/src/invocation.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import * as Cause from "effect/Cause";
44
import * as Data from "effect/Data";
55
import * as Effect from "effect/Effect";
66
import type { SandboxToolInvoker } from "@executor-js/codemode-core";
7+
import { ExecutionToolError } from "@executor-js/execution";
78
import {
89
ToolDispatcher,
910
makeDynamicWorkerExecutor,
@@ -257,6 +258,27 @@ describe("makeDynamicWorkerExecutor", () => {
257258
expect(result.error).toBe("Internal tool error");
258259
});
259260

261+
it("preserves public ExecutionToolError messages across the worker bridge", async () => {
262+
const executor = makeDynamicWorkerExecutor({ loader });
263+
const invoker = {
264+
invoke: () =>
265+
Effect.fail(
266+
new ExecutionToolError({
267+
message:
268+
"tools.search expects an object: { query?: string; namespace?: string; limit?: number; offset?: number }",
269+
}),
270+
),
271+
} satisfies SandboxToolInvoker;
272+
273+
const result = await Effect.runPromise(
274+
executor.execute("async () => await tools.search('github')", invoker),
275+
);
276+
277+
expect(result.error).toBe(
278+
"tools.search expects an object: { query?: string; namespace?: string; limit?: number; offset?: number }",
279+
);
280+
});
281+
260282
it("does not expose host error stack details to sandbox error handlers", async () => {
261283
const executor = makeDynamicWorkerExecutor({ loader });
262284
const invoker = {

packages/kernel/runtime-dynamic-worker/src/module-template.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,14 @@ export const buildExecutorModule = (body: string, timeoutMs: number): string =>
127127
" for (const [k, v] of Object.entries(value)) out[k] = __decodeBinary(v, seen);",
128128
" return out;",
129129
" };",
130+
" const __publicToolErrorMessage = (error) => {",
131+
" const values = [error && error.primary, ...(Array.isArray(error && error.failures) ? error.failures : [])];",
132+
" for (const value of values) {",
133+
" if (value && value.__type === 'Error' && value.name === 'ExecutionToolError' && typeof value.message === 'string') return value.message;",
134+
" }",
135+
" if (error && typeof error.message === 'string' && error.message.startsWith('Internal tool error')) return error.message;",
136+
" return null;",
137+
" };",
130138
" const __makeToolsProxy = (path = []) => new Proxy(() => undefined, {",
131139
" get(_target, prop) {",
132140
" if (prop === 'then' || typeof prop === 'symbol') return undefined;",
@@ -138,7 +146,7 @@ export const buildExecutorModule = (body: string, timeoutMs: number): string =>
138146
" return (async () => {",
139147
" const encoded = await __encodeBinary(args[0]);",
140148
" const data = await __dispatcher.call(toolPath, encoded);",
141-
" if (!data.ok) throw new Error(data.error && typeof data.error.message === 'string' && data.error.message.startsWith('Internal tool error') ? data.error.message : 'Internal tool error');",
149+
" if (!data.ok) throw new Error(__publicToolErrorMessage(data.error) || 'Internal tool error');",
142150
" return __decodeBinary(data.result);",
143151
" })();",
144152
" },",

packages/plugins/graphql/src/sdk/plugin.test.ts

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, it, expect } from "@effect/vitest";
22
import { Effect, Predicate } from "effect";
3+
import { HttpServerRequest, HttpServerResponse } from "effect/unstable/http";
34

45
import {
56
ConnectionId,
@@ -13,7 +14,7 @@ import {
1314
SecretId,
1415
TokenMaterial,
1516
} from "@executor-js/sdk";
16-
import { makeTestConfig } from "@executor-js/sdk/testing";
17+
import { makeTestConfig, serveTestHttpApp } from "@executor-js/sdk/testing";
1718
import { memorySecretsPlugin } from "@executor-js/sdk/testing";
1819

1920
import { graphqlPlugin } from "./plugin";
@@ -208,6 +209,46 @@ describe("graphqlPlugin real protocol server", () => {
208209
}),
209210
);
210211

212+
it.effect("surfaces non-2xx invocation responses as ToolResult.fail", () =>
213+
Effect.gen(function* () {
214+
const server = yield* serveTestHttpApp((request) =>
215+
Effect.gen(function* () {
216+
const webRequest = yield* HttpServerRequest.toWeb(request);
217+
const body = yield* Effect.promise(() => webRequest.text());
218+
if (body.includes("__schema")) {
219+
return HttpServerResponse.jsonUnsafe({ data: introspectionResult });
220+
}
221+
return HttpServerResponse.text("temporary upstream outage", {
222+
status: 503,
223+
contentType: "text/plain",
224+
});
225+
}),
226+
);
227+
const executor = yield* createExecutor(
228+
makeTestConfig({ plugins: [graphqlPlugin()] as const }),
229+
);
230+
231+
yield* executor.graphql.addSource({
232+
endpoint: server.url("/graphql"),
233+
scope: TEST_SCOPE,
234+
namespace: "http_error_graph",
235+
});
236+
237+
const result = yield* executor.tools.invoke("http_error_graph.query.hello", {
238+
name: "Ada",
239+
});
240+
241+
expect(result).toMatchObject({
242+
ok: false,
243+
error: {
244+
code: "graphql_http_error",
245+
status: 503,
246+
message: "GraphQL request failed with HTTP 503",
247+
},
248+
});
249+
}),
250+
);
251+
211252
it.effect("invokes OAuth-backed sources with a bearer token", () =>
212253
Effect.gen(function* () {
213254
const server = yield* serveGreetingServer;

packages/plugins/graphql/src/sdk/plugin.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,6 +1074,18 @@ export const graphqlPlugin = definePlugin((options?: GraphqlPluginOptions) => {
10741074
details: { errors },
10751075
});
10761076
}
1077+
if (result.status < 200 || result.status >= 300) {
1078+
return ToolResult.fail({
1079+
code: "graphql_http_error",
1080+
status: result.status,
1081+
message: `GraphQL request failed with HTTP ${result.status}`,
1082+
details: {
1083+
status: result.status,
1084+
data: result.data,
1085+
errors: result.errors,
1086+
},
1087+
});
1088+
}
10771089
return ToolResult.ok(result.data);
10781090
}),
10791091

0 commit comments

Comments
 (0)