Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/connection-null-default.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eve": patch
---

A connection tool call no longer fails with `Instance type "null" is invalid` when the model omits an argument whose schema declares a `null` default it does not allow, such as Pydantic's `{ "type": "string", "default": null }` for `x: str = None`. eve fills schema defaults once, when the call runs, instead of filling them before approval and rejecting them on the second check.
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ export default defineEval({
count: 1,
output: { petId: 4217, name: "Biscuit", kennel: "B3" },
});
// A text result without an output schema is parsed as JSON, and the
// omitted `day` reaches the server as its schema default.
// A text result without an output schema is parsed as JSON. The call keeps
// the model's input, and the server echoes the schema default it received
// for the omitted `day`.
turn.calledTool("kennel__list_feedings", {
count: 1,
input: { day: "today" },
output: (value) =>
Array.isArray(value) &&
value.length === 2 &&
Expand Down
5 changes: 2 additions & 3 deletions packages/eve/src/execution/catalog/connection-entry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,7 @@ describe("connection approval context", () => {
});

describe("connection tool calls", () => {
// Expected failure until #5509 is fixed: the call rejects the `null` default its own check filled in.
it.fails("runs a call whose omitted argument defaults to null", async () => {
it("runs a call whose omitted argument defaults to null", async () => {
// Pydantic publishes `query: str = None` as a string property that defaults to null.
const search: ConnectionToolMetadata = {
description: "Search issues.",
Expand Down Expand Up @@ -142,7 +141,7 @@ describe("connection tool calls", () => {
// The harness validates the model's input, then runs the tool on the validated value.
await contextStorage.run(new ContextContainer(), async () => {
const checked = await (entry.inputSchema as ToolSchema)["~standard"].validate({});
expect(checked).toEqual({ value: { query: null } });
expect(checked).toEqual({ value: {} });
await entry.execute!((checked as { value: unknown }).value, {
messages: [],
toolCallId: "call-search",
Expand Down
40 changes: 23 additions & 17 deletions packages/eve/src/execution/catalog/connection-entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,8 @@ async function callConnectionTool(
}
// Validation may have run before a sign-in made the tools listable.
const isEventAction = client.eventActions?.ownsName(toolName) === true;
const checked = await checkCall(connection, listing.tools, toolName, rawInput, !isEventAction);
const modelInput = withoutProvidedArguments(connection, rawInput, !isEventAction);
const checked = await checkCall(connection, listing.tools, toolName, modelInput);
if ("issues" in checked) throw new Error(issuesMessage(checked.issues));
const { input, tool } = checked;

Expand Down Expand Up @@ -224,39 +225,44 @@ async function checkInput(
if (isConnectionAuthorizationRequiredError(error)) return { value: input };
return { issues: [{ message: listingFailureMessage(connection.connectionName, error) }] };
}
const checked = await checkCall(
connection,
tools,
toolName,
input,
client.eventActions?.ownsName(toolName) !== true,
);
return "issues" in checked ? { issues: checked.issues } : { value: checked.input };
const remote = client.eventActions?.ownsName(toolName) !== true;
const modelInput = withoutProvidedArguments(connection, input, remote);
const checked = await checkCall(connection, tools, toolName, modelInput);
// The call validates this value again when it runs. Defaults are filled only there,
// because a filled default its own schema rejects, such as Pydantic's `null` for
// `x: str = None`, would fail that second check.
return "issues" in checked ? { issues: checked.issues } : { value: modelInput };
}

type CallCheck =
| { readonly input: JsonObject; readonly tool: ConnectionToolMetadata }
| { readonly issues: readonly StandardSchemaV1.Issue[] };

/** Provided values replace these keys at execution, so a model value for one is dropped, not rejected. */
function withoutProvidedArguments(
connection: ResolvedConnectionDefinition,
input: unknown,
remote: boolean,
): Record<string, unknown> {
const provided = remote ? (connection.toolCall?.providedArguments ?? {}) : {};
return Object.fromEntries(
Object.entries(isObject(input) ? input : {}).filter(([key]) => !Object.hasOwn(provided, key)),
);
}

/** The tool a call runs and its input with schema defaults filled in, or why it cannot run. */
async function checkCall(
connection: ResolvedConnectionDefinition,
tools: readonly ConnectionToolMetadata[],
toolName: string,
input: unknown,
remote = true,
input: Record<string, unknown>,
): Promise<CallCheck> {
const tool = tools.find((entry) => entry.name === toolName);
if (tool === undefined) {
return { issues: [{ message: unknownToolMessage(connection, toolName, tools) }] };
}
// Provided values replace these keys at execution, so a model value for one is dropped, not rejected.
const provided = remote ? (connection.toolCall?.providedArguments ?? {}) : {};
const modelInput = Object.fromEntries(
Object.entries(isObject(input) ? input : {}).filter(([key]) => !Object.hasOwn(provided, key)),
);
const result = await defineJsonSchema(tool.inputSchema as JsonObject)["~standard"].validate(
modelInput,
input,
);
if (result.issues === undefined) return { input: result.value as JsonObject, tool };
return {
Expand Down
4 changes: 2 additions & 2 deletions packages/eve/src/execution/catalog/step-catalog.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -510,9 +510,9 @@ describe("buildStepCatalog", () => {
return { linear, ...catalogContext({ connections: [linear], tools: [inlineTool("add")] }) };
}

it("resolves a name under a connection's prefix to that connection's tool, filling schema defaults", async () => {
it("resolves a name under a connection's prefix to that connection's tool, which fills schema defaults when it runs", async () => {
const { catalog, linear, run } = linearCatalog();
const validated = { input: { limit: 20, team: "core" }, name: "linear__list_issues" };
const validated = { input: { team: "core" }, name: "linear__list_issues" };

expect(
await validateTool(catalog, { input: { team: "core" }, name: "linear__list_issues" }),
Expand Down
Loading