Skip to content

Commit 44a02ad

Browse files
RhysSullivanRhysSullivan
authored
[codex] fix MCP output schema contracts (#851)
* add MCP output schema regression tests * extract tool output type contract helper * fix MCP tool output schema wrapper * derive MCP output schema from SDK * format MCP output schema changes * Project typed MCP results for sandbox agents * Align MCP invoke results with tool schema * Keep MCP code mode result envelope consistent
1 parent 76a49ed commit 44a02ad

8 files changed

Lines changed: 305 additions & 49 deletions

File tree

‎bun.lock‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

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

54
import {
65
ElicitationResponse,
@@ -10,7 +9,7 @@ import {
109
definePlugin,
1110
tool,
1211
} from "@executor-js/sdk";
13-
import { makeTestConfig } from "@executor-js/sdk/testing";
12+
import { makeTestConfig, typeCheckOutputTypeScript } from "@executor-js/sdk/testing";
1413
import { makeQuickJsExecutor } from "@executor-js/runtime-quickjs";
1514
import { createExecutionEngine } from "./engine";
1615
import { describeTool, makeExecutorToolInvoker, searchTools } from "./tool-invoker";
@@ -44,48 +43,13 @@ const typeCheckDescribedInvocation = (
4443
described: DescribedToolContract,
4544
runtimeResult: unknown,
4645
consumerSource: string,
47-
): readonly string[] => {
48-
const fileName = "described-tool-contract.ts";
49-
const source = [
50-
...Object.entries(described.typeScriptDefinitions).map(([name, definition]) => {
51-
return `type ${name} = ${definition};`;
52-
}),
53-
`type ToolOutput = ${described.outputTypeScript};`,
54-
`const invokedResult: ToolOutput = ${JSON.stringify(runtimeResult)};`,
46+
): readonly string[] =>
47+
typeCheckOutputTypeScript(described, runtimeResult, {
5548
consumerSource,
56-
].join("\n");
57-
58-
const options: ts.CompilerOptions = {
59-
module: ts.ModuleKind.ESNext,
60-
noEmit: true,
61-
skipLibCheck: true,
62-
strict: true,
63-
target: ts.ScriptTarget.ES2022,
64-
};
65-
const host = ts.createCompilerHost(options);
66-
const originalGetSourceFile = host.getSourceFile.bind(host);
67-
const originalReadFile = host.readFile.bind(host);
68-
const originalFileExists = host.fileExists.bind(host);
69-
70-
host.getSourceFile = (candidate, languageVersion, onError, shouldCreateNewSourceFile) => {
71-
if (candidate === fileName) {
72-
return ts.createSourceFile(candidate, source, languageVersion, true);
73-
}
74-
return originalGetSourceFile(candidate, languageVersion, onError, shouldCreateNewSourceFile);
75-
};
76-
host.readFile = (candidate) => (candidate === fileName ? source : originalReadFile(candidate));
77-
host.fileExists = (candidate) => candidate === fileName || originalFileExists(candidate);
78-
79-
const program = ts.createProgram([fileName], options, host);
80-
return ts.getPreEmitDiagnostics(program).map((diagnostic) => {
81-
const message = ts.flattenDiagnosticMessageText(diagnostic.messageText, "\n");
82-
if (!diagnostic.file || diagnostic.start === undefined) {
83-
return message;
84-
}
85-
const position = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start);
86-
return `${diagnostic.file.fileName}:${position.line + 1}:${position.character + 1} ${message}`;
49+
fileName: "described-tool-contract.ts",
50+
typeName: "ToolOutput",
51+
valueName: "invokedResult",
8752
});
88-
};
8953

9054
// ---------------------------------------------------------------------------
9155
// Test plugins — each one declares a namespace as a static source with N

‎packages/core/sdk/src/testing.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ export {
3232
type OAuthTestServerShape,
3333
} from "./testing/oauth-test-server";
3434
export { createSqliteTestFumaDb, type SqliteTestFumaDb } from "./sqlite-test-db";
35+
export {
36+
typeCheckOutputTypeScript,
37+
type OutputTypeScriptContract,
38+
type TypeCheckOutputTypeScriptOptions,
39+
} from "./testing/tool-output-contract";
3540

3641
export class TestHttpServerAddressError extends Data.TaggedError("TestHttpServerAddressError")<{
3742
readonly address: unknown;

‎packages/core/sdk/src/testing/tool-output-contract.test.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import { describe, expect, it } from "@effect/vitest";
2+
3+
import { typeCheckOutputTypeScript } from "./tool-output-contract";
4+
5+
describe("typeCheckOutputTypeScript", () => {
6+
it("accepts runtime output that matches the described TypeScript contract", () => {
7+
const diagnostics = typeCheckOutputTypeScript(
8+
{
9+
outputTypeScript: "{ ok: true; data: ResultData }",
10+
typeScriptDefinitions: {
11+
Payload: "{ answer: string }",
12+
ResultData:
13+
'{ content: readonly { type: "text"; text: string }[]; structuredContent: Payload }',
14+
},
15+
},
16+
{
17+
ok: true,
18+
data: {
19+
content: [{ type: "text", text: "done" }],
20+
structuredContent: { answer: "done" },
21+
},
22+
},
23+
{
24+
consumerSource:
25+
"const answer: string = invokedOutput.data.structuredContent.answer; answer;",
26+
},
27+
);
28+
29+
expect(diagnostics).toEqual([]);
30+
});
31+
32+
it("reports when the described contract omits the runtime output wrapper", () => {
33+
const diagnostics = typeCheckOutputTypeScript(
34+
{
35+
outputTypeScript: "{ ok: true; data: { answer: string } }",
36+
},
37+
{
38+
ok: true,
39+
data: {
40+
content: [{ type: "text", text: "done" }],
41+
structuredContent: { answer: "done" },
42+
},
43+
},
44+
);
45+
46+
expect(diagnostics.join("\n")).toContain("answer");
47+
});
48+
49+
it("reports missing output TypeScript contracts", () => {
50+
expect(typeCheckOutputTypeScript({}, { ok: true })).toEqual(["missing outputTypeScript"]);
51+
});
52+
});

‎packages/core/sdk/src/testing/tool-output-contract.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import * as ts from "typescript";
2+
3+
export type OutputTypeScriptContract = {
4+
readonly outputTypeScript?: string;
5+
readonly typeScriptDefinitions?: Record<string, string>;
6+
};
7+
8+
export type TypeCheckOutputTypeScriptOptions = {
9+
readonly consumerSource?: string;
10+
readonly fileName?: string;
11+
readonly typeName?: string;
12+
readonly valueName?: string;
13+
};
14+
15+
export const typeCheckOutputTypeScript = (
16+
contract: OutputTypeScriptContract | null | undefined,
17+
runtimeOutput: unknown,
18+
options: TypeCheckOutputTypeScriptOptions = {},
19+
): readonly string[] => {
20+
if (!contract?.outputTypeScript) {
21+
return ["missing outputTypeScript"];
22+
}
23+
24+
const fileName = options.fileName ?? "tool-output-contract.ts";
25+
const typeName = options.typeName ?? "ToolOutput";
26+
const valueName = options.valueName ?? "invokedOutput";
27+
const source = [
28+
...Object.entries(contract.typeScriptDefinitions ?? {}).map(
29+
([name, definition]) => `type ${name} = ${definition};`,
30+
),
31+
`type ${typeName} = ${contract.outputTypeScript};`,
32+
`const ${valueName}: ${typeName} = ${JSON.stringify(runtimeOutput)};`,
33+
options.consumerSource ?? `${valueName};`,
34+
].join("\n");
35+
36+
const compilerOptions: ts.CompilerOptions = {
37+
module: ts.ModuleKind.ESNext,
38+
noEmit: true,
39+
skipLibCheck: true,
40+
strict: true,
41+
target: ts.ScriptTarget.ES2022,
42+
};
43+
const host = ts.createCompilerHost(compilerOptions);
44+
const originalGetSourceFile = host.getSourceFile.bind(host);
45+
const originalReadFile = host.readFile.bind(host);
46+
const originalFileExists = host.fileExists.bind(host);
47+
48+
host.getSourceFile = (candidate, languageVersion, onError, shouldCreateNewSourceFile) => {
49+
if (candidate === fileName) {
50+
return ts.createSourceFile(candidate, source, languageVersion, true);
51+
}
52+
return originalGetSourceFile(candidate, languageVersion, onError, shouldCreateNewSourceFile);
53+
};
54+
host.readFile = (candidate) => (candidate === fileName ? source : originalReadFile(candidate));
55+
host.fileExists = (candidate) => candidate === fileName || originalFileExists(candidate);
56+
57+
const program = ts.createProgram([fileName], compilerOptions, host);
58+
return ts.getPreEmitDiagnostics(program).map((diagnostic) => {
59+
const message = ts.flattenDiagnosticMessageText(diagnostic.messageText, "\n");
60+
if (!diagnostic.file || diagnostic.start === undefined) {
61+
return message;
62+
}
63+
const position = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start);
64+
return `${diagnostic.file.fileName}:${position.line + 1}:${position.character + 1} ${message}`;
65+
});
66+
};

‎packages/plugins/mcp/package.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@
6666
"@executor-js/config": "workspace:*",
6767
"@executor-js/sdk": "workspace:*",
6868
"@modelcontextprotocol/sdk": "^1.29.0",
69-
"effect": "catalog:"
69+
"effect": "catalog:",
70+
"zod": "^4.3.6"
7071
},
7172
"devDependencies": {
7273
"@effect/atom-react": "catalog:",
@@ -78,8 +79,7 @@
7879
"bun-types": "catalog:",
7980
"react": "catalog:",
8081
"tsup": "catalog:",
81-
"vitest": "catalog:",
82-
"zod": "^4.3.6"
82+
"vitest": "catalog:"
8383
},
8484
"peerDependencies": {
8585
"@effect/atom-react": "catalog:",

0 commit comments

Comments
 (0)