Revert "Explain schema expectations for rejected native tool calls"

This reverts commit 65f108fbfb.
This commit is contained in:
Dotta committed 2026-10-30 14:18:46 -05:00
1 parent 65f108fbfb
commit 87ebd4951d
7 files changed
+9 -212

No files matched your search

@@ -7,7 +7,6 @@ import {
} from "../../contracts/completion-result.js";
import { canonicalOpenCodeMcpToolName, startOpenCodeMcpBridge, type OpenCodeMcpBridge } from "./mcp-bridge.js";
import { requestHumanInputAction } from "../../protocol-actions/request-human-input.js";
const bridges: OpenCodeMcpBridge[] = [];
@@ -32,20 +31,6 @@ async function rpc(
}
describe("OpenCode MCP bridge", () => {
it("tells a malformed question caller the expected payload version before any side effect", async () => {
const handler = vi.fn(async () => ({ disposition: "applied" }));
const bridge = await startOpenCodeMcpBridge({ tools: [{ name: "request_human_input",
inputSchema: requestHumanInputAction.live.descriptor.inputSchema }], handler });
bridges.push(bridge);
const result = await (await rpc(bridge, { id: "invalid-question", method: "tools/call", params: {
name: "paperclip_request_human_input", arguments: { idempotencyKey: "question", interactionKind: "questions",
title: "Choose a time", prompt: "Morning or Afternoon?", continuationPolicy: "wake_assignee", payload: { version: 2 } },
} })).json() as { result: { isError: boolean; content: Array<{ text: string }> } };
expect(result.result.isError).toBe(true);
expect(result.result.content[0]!.text).toContain("questionSet");
expect(result.result.content[0]!.text).toContain("/payload/version must be equal to constant (expected 1)");
expect(handler).not.toHaveBeenCalled();
});
it("binds to loopback, requires authentication, and exposes only its closed catalog", async () => {
const bridge = await startOpenCodeMcpBridge({
secret: "session-secret",
@@ -1,7 +1,6 @@
import { randomBytes, timingSafeEqual } from "node:crypto";
import { createServer, type IncomingMessage, type Server, type ServerResponse } from "node:http";
import { Ajv2020, type ValidateFunction } from "ajv/dist/2020.js";
import { toolInputValidationMessage } from "../tool-input-errors.js";
import {
PRP_BLOCK_RESULT_PROVIDER_INPUT_SCHEMA,
@@ -236,9 +235,10 @@ async function handleRequest(
const args = params.arguments ?? {};
const validator = context.validators.get(tool);
if (validator === undefined || !validator(args)) {
const detail = validator?.errors?.map((error) => `${error.instancePath || "/"} ${error.message ?? "is invalid"}`).join("; ");
writeRpc(response, id, {
isError: true,
content: [{ type: "text", text: toolInputValidationMessage(validator?.errors) }],
content: [{ type: "text", text: `Invalid tool input${detail ? `: ${detail}` : "."}` }],
});
return;
}
@@ -1,104 +0,0 @@
import assert from "node:assert/strict";
import { spawn, type ChildProcess } from "node:child_process";
import { once } from "node:events";
import { createServer } from "node:http";
import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import path from "node:path";
import { setTimeout as delay } from "node:timers/promises";
import { it } from "vitest";
import { requestHumanInputAction } from "../../protocol-actions/request-human-input.js";
import { resolvePinnedOpenCodeCommand } from "./opencode-server-driver.js";
import { startOpenCodeMcpBridge } from "./mcp-bridge.js";
type Row = Record<string, any>;
it("lets the pinned client correct a rejected question from schema-derived feedback without API fallback", async () => {
const root = await mkdtemp(path.join(tmpdir(), "opencode-question-schema-"));
const requests: Row[] = [], calls: unknown[] = [];
const input = { idempotencyKey: "choice-1", interactionKind: "questions", title: "Choose a time", prompt: "Morning or Afternoon?",
resolverPolicy: "human_only", continuationPolicy: "wake_assignee", payload: { version: 1,
questionSet: { schema: "paperclip.question_set.v1", questions: [{ id: "time", prompt: "Morning or Afternoon?", required: true,
answerMode: "single_select", options: [{ id: "am", label: "Morning" }, { id: "pm", label: "Afternoon" }] }] } } };
let rawOutput = "", child: ChildProcess | undefined, exited: Promise<unknown> | undefined;
const bridge = await startOpenCodeMcpBridge({ tools: [{ name: "request_human_input",
description: requestHumanInputAction.live.descriptor.description, inputSchema: requestHumanInputAction.live.descriptor.inputSchema }],
handler: async call => { calls.push(call.arguments); return { disposition: "applied", interaction: { id: "card", ...call.arguments as object } }; } });
const provider = createServer(async (request, response) => {
let text = "";
for await (const chunk of request) text += chunk;
const body = JSON.parse(text || "{}"); requests.push({ url: request.url, body });
if (request.url !== "/v1/chat/completions") { response.writeHead(404).end(); return; }
response.writeHead(200, { "content-type": "text/event-stream" });
const question = body.tools?.find((tool: Row) => tool.function?.name === "paperclip_request_human_input");
const feedback = body.messages?.filter((message: Row) => message.role === "tool").at(-1)?.content;
const corrected = typeof feedback === "string" && feedback.includes("questionSet") && feedback.includes("expected 1");
const complete = calls.length > 0 || requests.length >= 3;
const delta = complete ? { role: "assistant", content: "Question fixture done." }
: { role: "assistant", tool_calls: [{ index: 0, id: "question-call", type: "function", function: {
name: question?.function.name ?? "paperclip_request_human_input",
arguments: JSON.stringify(corrected ? input : { ...input, payload: { version: 2 } }),
} }] };
for (const [part, finishReason] of [[delta, null], [{}, complete ? "stop" : "tool_calls"]]) {
response.write(`data: ${JSON.stringify({ id: "chatcmpl-local", object: "chat.completion.chunk", created: 1, model: "qualification",
choices: [{ index: 0, delta: part, finish_reason: finishReason }] })}\n\n`);
}
response.end("data: [DONE]\n\n");
});
await new Promise<void>(resolve => provider.listen(0, "127.0.0.1", resolve));
try {
const providerUrl = `http://127.0.0.1:${(provider.address() as { port: number }).port}/v1`;
const config = path.join(root, "opencode.json"), workspace = path.join(root, "workspace");
await mkdir(workspace);
await writeFile(config, JSON.stringify({ model: "openrouter/qualification", small_model: "openrouter/qualification",
share: "disabled", autoupdate: false, plugin: [], enabled_providers: ["openrouter"],
provider: { openrouter: { options: { baseURL: providerUrl, apiKey: "local-fixture-only" },
models: { qualification: { name: "qualification", limit: { context: 100000, output: 1000 } } } } },
permission: { "*": "deny", "paperclip_*": "allow" },
mcp: { paperclip: { type: "remote", url: bridge.url, enabled: true, oauth: false,
headers: { Authorization: `Bearer ${bridge.secret}` }, timeout: 5000 } },
}));
child = spawn(resolvePinnedOpenCodeCommand(), ["serve", "--hostname", "127.0.0.1", "--port", "0"], {
cwd: workspace, detached: true, stdio: ["ignore", "pipe", "pipe"], env: { PATH: process.env.PATH, HOME: root,
XDG_CONFIG_HOME: path.join(root, "config"), XDG_DATA_HOME: path.join(root, "data"), XDG_CACHE_HOME: path.join(root, "cache"),
OPENCODE_CONFIG: config, OPENCODE_DISABLE_PROJECT_CONFIG: "true", OPENCODE_DISABLE_MODELS_FETCH: "true",
OPENCODE_DISABLE_DEFAULT_PLUGINS: "true", OPENCODE_SERVER_USERNAME: "paperclip", OPENCODE_SERVER_PASSWORD: "local-fixture-only" },
});
exited = once(child, "exit");
for (const stream of [child.stdout!, child.stderr!]) stream.on("data", chunk => { rawOutput = `${rawOutput}${chunk}`.slice(-16384); });
let baseUrl: string | undefined;
for (let index = 0; index < 150; index++) {
baseUrl = rawOutput.match(/http:\/\/127\.0\.0\.1:\d+/)?.[0];
if (baseUrl) break;
assert.equal(child.exitCode, null, rawOutput); await delay(100);
}
assert.ok(baseUrl, rawOutput);
const headers = { Authorization: `Basic ${Buffer.from("paperclip:local-fixture-only").toString("base64")}`, "content-type": "application/json" };
const api = async (route: string, options: RequestInit = {}) => {
const response = await fetch(baseUrl + route, { headers, signal: AbortSignal.timeout(15_000), ...options });
assert.ok(response.ok, `${route}:${response.status} ${await response.clone().text()}`); return response;
};
const session = await (await api("/session", { method: "POST", body: JSON.stringify({ title: "Local question schema" }) })).json() as Row;
await api(`/session/${session.id}/prompt_async`, { method: "POST", body: JSON.stringify({
model: { providerID: "openrouter", modelID: "qualification" }, parts: [{ type: "text", text: "Ask the time choice." }],
}) });
for (let index = 0; index < 150; index++) {
const messages = await (await api(`/session/${session.id}/message`)).json() as Row[];
if (messages.some(message => message.info.role === "assistant" && message.parts.some((part: Row) => part.type === "text" && part.text === "Question fixture done."))) break;
await delay(100);
}
const offered = requests.find(request => request.body.tools)?.body.tools.find((tool: Row) => tool.function?.name === "paperclip_request_human_input");
assert.equal(requests.length, 3);
assert.ok(requests[1]!.body.messages.some((message: Row) => message.role === "tool" && String(message.content).includes("Invalid tool input")));
assert.ok(offered, "Human input tool not offered");
assert.deepEqual(calls, [input]);
} finally {
if (child?.pid) {
const processId = child.pid;
try { process.kill(-processId, "SIGTERM"); } catch {}
const timer = setTimeout(() => { try { process.kill(-processId, "SIGKILL"); } catch {} }, 2000);
try { await exited; } finally { clearTimeout(timer); }
}
provider.closeAllConnections(); await new Promise<void>(resolve => provider.close(() => resolve()));
await bridge.close(); await rm(root, { recursive: true, force: true });
}
}, 30_000);
@@ -1,6 +1,5 @@
import { readNativeSemanticReceipt, semanticInputSha256, type SemanticToolReceipt, type SemanticToolResult } from "./semantic-tool-receipt.js";
import { validatePrpStructuredRunResult } from "../protocol/replay-contract.js";
import { requestHumanInputAction } from "../protocol-actions/request-human-input.js";
import { connect } from "node:net";
import { afterEach, describe, expect, it, vi } from "vitest";
@@ -39,21 +38,6 @@ async function rpc(
}
describe("runner semantic MCP bridge", () => {
it("returns bounded schema-derived question guidance before dispatch", async () => {
const handler = vi.fn(async () => ({ disposition: "applied" }));
const bridge = await startRunnerToolBridge({ tools: [{ name: "request_human_input",
inputSchema: requestHumanInputAction.live.descriptor.inputSchema }], handler });
bridges.push(bridge);
const result = await (await rpc(bridge, { id: "invalid-question", method: "tools/call", params: {
name: "paperclip_request_human_input", arguments: { idempotencyKey: "question", interactionKind: "questions",
title: "Choose a time", prompt: "Morning or Afternoon?", continuationPolicy: "wake_assignee", payload: { version: 2 } },
} })).json() as { result: { isError: boolean; content: Array<{ text: string }> } };
expect(result.result.isError).toBe(true);
expect(result.result.content[0]!.text).toContain("questionSet");
expect(result.result.content[0]!.text).toContain("/payload/version must be equal to constant (expected 1)");
expect(Buffer.byteLength(result.result.content[0]!.text)).toBeLessThanOrEqual(2048);
expect(handler).not.toHaveBeenCalled();
});
it.each([false, true])("preloads only essential public Claude task tools when opted in (%s)", async preloadClaudeTaskTools => {
const bridge = await startRunnerToolBridge({
preloadClaudeTaskTools,
@@ -8,7 +8,6 @@ import {
} from "node:http";
import { Ajv2020, type ValidateFunction } from "ajv/dist/2020.js";
import { toolInputValidationMessage } from "./tool-input-errors.js";
import {
PRP_BLOCK_RESULT_PROVIDER_INPUT_SCHEMA,
@@ -314,10 +313,16 @@ async function handleRequest(
const args = params.arguments ?? {};
const validator = context.validators.get(tool);
if (validator === undefined || !validator(args)) {
const detail = validator?.errors
?.map(
(error) =>
`${error.instancePath || "/"} ${error.message ?? "is invalid"}`,
)
.join("; ");
writeToolError(
response,
id,
toolInputValidationMessage(validator?.errors),
`Invalid tool input${detail ? `: ${detail}` : "."}`,
);
return;
}
@@ -1,38 +0,0 @@
import { describe, expect, it } from "vitest";
import { toolInputValidationMessage } from "./tool-input-errors.js";
describe("schema-derived tool validation feedback", () => {
it("includes expected constants, enum choices, types and missing fields", () => {
const message = toolInputValidationMessage([
{ keyword: "const", schemaPath: "#/properties/payload/properties/version/const", params: { allowedValue: 1 }, message: "must be equal to constant" },
{ keyword: "enum", schemaPath: "#/properties/mode/enum", params: { allowedValues: ["single", "multi"] }, message: "must be equal to one of the allowed values" },
{ keyword: "type", schemaPath: "#/properties/payload/type", params: { type: "object" }, message: "must be object" },
{ keyword: "required", schemaPath: "#/then/properties/payload/anyOf/0/required", params: { missingProperty: "questionSet" }, message: "must have required property 'questionSet'" },
]);
expect(message).toContain("/payload/version must be equal to constant (expected 1)");
expect(message).toContain('/mode must be equal to one of the allowed values (expected one of "single", "multi")');
expect(message).toContain("/payload must be object");
expect(message).toContain("/payload must have required property 'questionSet'");
});
it("derives locations from the public schema instead of model-controlled property names or values", () => {
const error = { keyword: "type", instancePath: "/secret-input-property", data: "secret-value",
schemaPath: "#/properties/payload/additionalProperties/type", params: { type: "object" }, message: "must be object" };
const message = toolInputValidationMessage([error]);
expect(message).toBe("Invalid tool input: /payload must be object");
expect(message).not.toContain("secret");
});
it("bounds repeated errors and large schema values without serializing nested objects", () => {
const message = toolInputValidationMessage(Array.from({ length: 1000 }, () => ({ keyword: "enum", schemaPath: "#/properties/value/enum",
params: { allowedValues: [...Array.from({ length: 100 }, (_, index) => `value-${index}-${"😀".repeat(200)}`), { private: "not serialized" }] },
message: "must be equal to one of the allowed values" })));
expect(Buffer.byteLength(message)).toBeLessThanOrEqual(2048);
expect(message).not.toContain("value-99");
expect(message).not.toContain("not serialized");
expect(message).not.toContain("�");
expect(toolInputValidationMessage([{ keyword: "const", schemaPath: "#/const", params: { allowedValue: { nested: "not serialized" } } }]))
.toBe("Invalid tool input: / is invalid (expected object from the tool schema)");
});
it("retains a bounded failure when the validator has no error details", () => {
expect(toolInputValidationMessage(null)).toBe("Invalid tool input.");
});
});
@@ -1,35 +0,0 @@
import type { ErrorObject } from "ajv";
type SchemaError = Pick<ErrorObject, "keyword" | "message" | "schemaPath" | "params">;
const maxErrors = 8, maxBytes = 2048;
function schemaLocation(schemaPath: string): string {
const parts = schemaPath.split("/"), location: string[] = [];
for (let index = 0; index < parts.length; index++) {
if (parts[index] === "properties" && parts[index + 1]) location.push(parts[++index]!);
else if (parts[index] === "items") location.push("[]");
}
return location.length ? `/${location.join("/")}`.slice(0, 200) : "/";
}
function expected(value: unknown): string {
if (typeof value === "string") return JSON.stringify(value.slice(0, 80).replace(/[\uD800-\uDBFF]$/, "")) + (value.length > 80 ? "…" : "");
if (value === null || typeof value === "boolean" || typeof value === "number") return String(value);
return Array.isArray(value) ? "array from the tool schema" : "object from the tool schema";
}
export function toolInputValidationMessage(errors: readonly SchemaError[] | null | undefined): string {
const details = (errors ?? []).slice(0, maxErrors).map(error => {
let requirement = error.message ?? "is invalid";
if (error.keyword === "const") requirement += ` (expected ${expected(error.params.allowedValue)})`;
if (error.keyword === "enum" && Array.isArray(error.params.allowedValues)) {
requirement += ` (expected one of ${error.params.allowedValues.slice(0, 8).map(expected).join(", ")}${error.params.allowedValues.length > 8 ? ", …" : ""})`;
}
return `${schemaLocation(error.schemaPath)} ${requirement.slice(0, 400).replace(/[\uD800-\uDBFF]$/, "")}`;
});
let message = `Invalid tool input${details.length ? `: ${details.join("; ")}` : "."}`;
if ((errors?.length ?? 0) > maxErrors) message += "; additional validation errors omitted";
const bytes = Buffer.from(message);
if (bytes.length <= maxBytes) return message;
let end = maxBytes - 3;
while (end > 0 && (bytes[end]! & 0xc0) === 0x80) end--;
return `${bytes.subarray(0, end).toString("utf8")}…`;
}