From 87ebd4951d86493cb4769e93f3cbc5ad08e7b07e Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 30 Oct 2026 14:18:46 -0500 Subject: [PATCH] Revert "Explain schema expectations for rejected native tool calls" This reverts commit 65f108fbfbafc08ffa93af03604f77eb9d1620b6. --- .../src/drivers/opencode/mcp-bridge.test.ts | 15 --- .../src/drivers/opencode/mcp-bridge.ts | 4 +- .../drivers/opencode/question-schema.test.ts | 104 ------------------ .../src/drivers/runner-tool-bridge.test.ts | 16 --- .../src/drivers/runner-tool-bridge.ts | 9 +- .../src/drivers/tool-input-errors.test.ts | 38 ------- .../src/drivers/tool-input-errors.ts | 35 ------ 7 files changed, 9 insertions(+), 212 deletions(-) delete mode 100644 packages/paperclip-runner/src/drivers/opencode/question-schema.test.ts delete mode 100644 packages/paperclip-runner/src/drivers/tool-input-errors.test.ts delete mode 100644 packages/paperclip-runner/src/drivers/tool-input-errors.ts diff --git a/packages/paperclip-runner/src/drivers/opencode/mcp-bridge.test.ts b/packages/paperclip-runner/src/drivers/opencode/mcp-bridge.test.ts index fec65a014e9..40c2fc68fd7 100644 --- a/packages/paperclip-runner/src/drivers/opencode/mcp-bridge.test.ts +++ b/packages/paperclip-runner/src/drivers/opencode/mcp-bridge.test.ts @@ -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", diff --git a/packages/paperclip-runner/src/drivers/opencode/mcp-bridge.ts b/packages/paperclip-runner/src/drivers/opencode/mcp-bridge.ts index ac5c229c24e..af847094c40 100644 --- a/packages/paperclip-runner/src/drivers/opencode/mcp-bridge.ts +++ b/packages/paperclip-runner/src/drivers/opencode/mcp-bridge.ts @@ -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; } diff --git a/packages/paperclip-runner/src/drivers/opencode/question-schema.test.ts b/packages/paperclip-runner/src/drivers/opencode/question-schema.test.ts deleted file mode 100644 index 9afa2efd2ee..00000000000 --- a/packages/paperclip-runner/src/drivers/opencode/question-schema.test.ts +++ /dev/null @@ -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; -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 | 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(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(resolve => provider.close(() => resolve())); - await bridge.close(); await rm(root, { recursive: true, force: true }); - } -}, 30_000); diff --git a/packages/paperclip-runner/src/drivers/runner-tool-bridge.test.ts b/packages/paperclip-runner/src/drivers/runner-tool-bridge.test.ts index d28d000e6ff..4b212476c49 100644 --- a/packages/paperclip-runner/src/drivers/runner-tool-bridge.test.ts +++ b/packages/paperclip-runner/src/drivers/runner-tool-bridge.test.ts @@ -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, diff --git a/packages/paperclip-runner/src/drivers/runner-tool-bridge.ts b/packages/paperclip-runner/src/drivers/runner-tool-bridge.ts index cfdc5f246f9..624569fa195 100644 --- a/packages/paperclip-runner/src/drivers/runner-tool-bridge.ts +++ b/packages/paperclip-runner/src/drivers/runner-tool-bridge.ts @@ -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; } diff --git a/packages/paperclip-runner/src/drivers/tool-input-errors.test.ts b/packages/paperclip-runner/src/drivers/tool-input-errors.test.ts deleted file mode 100644 index 02644d0fe50..00000000000 --- a/packages/paperclip-runner/src/drivers/tool-input-errors.test.ts +++ /dev/null @@ -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."); - }); -}); diff --git a/packages/paperclip-runner/src/drivers/tool-input-errors.ts b/packages/paperclip-runner/src/drivers/tool-input-errors.ts deleted file mode 100644 index 26bc6781533..00000000000 --- a/packages/paperclip-runner/src/drivers/tool-input-errors.ts +++ /dev/null @@ -1,35 +0,0 @@ -import type { ErrorObject } from "ajv"; - -type SchemaError = Pick; -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")}…`; -}