diff --git a/server/src/services/native-runtime/paperclip-runner-tool-authority.ts b/server/src/services/native-runtime/paperclip-runner-tool-authority.ts index 832581b6030..ecf9cfdcf07 100644 --- a/server/src/services/native-runtime/paperclip-runner-tool-authority.ts +++ b/server/src/services/native-runtime/paperclip-runner-tool-authority.ts @@ -72,7 +72,7 @@ import { runnerApiSensitiveResponse } from "./runner-api-sensitive-response.js"; import { workspaceFileResourceService } from "../workspace-file-resources.js"; import { badRequest, forbidden, notFound, HttpError } from "../../errors.js"; import { runnerApiCatalog, searchRunnerApi } from "./runner-api-catalog.js"; -import { executeRunnerApi, validateRunnerApiCall, RUNNER_API_MAX_BYTES, type RunnerApiCall, type RunnerApiFile, type RunnerApiIo } from "./runner-api-client.js"; +import { assertRunnerApiDispatchable, executeRunnerApi, validateRunnerApiCall, RUNNER_API_MAX_BYTES, type RunnerApiCall, type RunnerApiFile, type RunnerApiIo } from "./runner-api-client.js"; import { acquireRunnerApiResponseSlot, runnerApiCompanyCaptureMaxBytes, RUNNER_API_RESPONSE_MAX_BYTES, RUNNER_API_RESPONSE_RUN_MAX_BYTES, RunnerApiResponseLimitError } from "./runner-api-response-limits.js"; import { and, desc, eq, inArray, isNull, notInArray, sql } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; @@ -1171,6 +1171,9 @@ export class PaperclipRunnerToolAuthority { const result = await this.#apiReceiptResult(existing); return { result: existing.encryptedResult ? result : result ?? { ok: false, status: null, error: "api_outcome_unknown", outcome: "unknown", guidance: "Inspect state before issuing another mutation." } }; } + // A new call that would be rejected before dispatch must not reserve a + // pending receipt; that would make its callId report an unknown outcome. + assertRunnerApiDispatchable(operation, input, Object.keys(record(value))); if (Object.keys(receipts).length >= 512) throw badRequest("Run API mutation limit reached"); receipts[key] = { digest, operationId: operation.operationId, state: "pending" }; await tx.update(heartbeatRuns).set({ resultJson: { ...resultJson, apiToolReceipts: receipts } }).where(eq(heartbeatRuns.id, this.binding.runId)); diff --git a/server/src/services/native-runtime/runner-api-client.ts b/server/src/services/native-runtime/runner-api-client.ts index ecce7755c8f..256831b8f3f 100644 --- a/server/src/services/native-runtime/runner-api-client.ts +++ b/server/src/services/native-runtime/runner-api-client.ts @@ -52,10 +52,17 @@ export interface RunnerApiIo { export function validateRunnerApiCall(value: unknown, context: RunnerApiContext): { input: RunnerApiCall; operation: RunnerApiOperation } { const parsed = runnerApiCallSchema.safeParse(value); if (!parsed.success) throw badRequest("Invalid call_api arguments", { issues: parsed.error.issues }); - const input = parsed.data; + let input = parsed.data; const operation = runnerApiOperation(input.operationId); if (operation.transport !== "rest") throw unprocessable(`This endpoint requires its existing protocol client; call_api supports REST only and no other API route substitutes for it.${operation.description.trim() ? " Read its catalog description for details." : ""}`); - if (input.responseText && operation.method !== "GET") throw badRequest("responseText requires a GET operation; read the saved artifact instead of repeating a mutation"); + if (input.responseText && operation.method !== "GET") { + // A later page of a mutation response would need the mutation repeated. + // A first-window request changes only presentation, so the mutation runs + // once and its response is returned through the normal inline/artifact path. + if ((input.responseText.offsetBytes ?? 0) > 0) throw badRequest("This request was not sent: responseText.offsetBytes applies only to GET reads, and a later page of a mutation response would repeat the mutation. Remove responseText and send the mutation once; read any saved response artifact with GET /api/assets/{assetId}/content."); + const { responseText: _ignored, ...rest } = input; + input = rest; + } if (!operation.allowedModes.includes(context.workMode)) throw forbidden("call_api permits only reads in Ask and Plan modes; use the permitted dedicated tools"); if (input.pathParams?.companyId && input.pathParams.companyId !== context.companyId) throw forbidden("API call belongs to another company"); const contentType = (input.contentType ?? "application/json").split(";", 1)[0].trim().toLowerCase(); @@ -71,6 +78,26 @@ export function validateRunnerApiCall(value: unknown, context: RunnerApiContext) return { input, operation }; } +/** + * Rules for a new dispatch only. Recovery and saved receipts still validate an + * older accepted call with validateRunnerApiCall, so these rules stay out of it. + * Callers that reserve a mutation receipt must run this before the reservation. + */ +export function assertRunnerApiDispatchable(operation: RunnerApiOperation, input: RunnerApiCall, receivedKeys: readonly string[]): void { + if (operation.method === "PATCH" && input.body === undefined && !input.files?.length && operation.requestBody?.required) { + // A PATCH without a body changes nothing but still returns success, which + // reads as a completed update. Name the missing argument instead. + throw badRequest(`This request was not sent: this PATCH requires body, a JSON object with the fields to change${patchFieldHint(operation)}. Received arguments: ${[...receivedKeys].sort().join(", ")}.`); + } +} + +function patchFieldHint(operation: RunnerApiOperation): string { + const schema = operation.requestBody?.content?.["application/json"]?.schema as { properties?: Record } | undefined; + const fields = Object.keys(schema?.properties ?? {}); + if (!fields.length) return ""; + return ` (allowed fields: ${fields.slice(0, 12).join(", ")}${fields.length > 12 ? ", ..." : ""})`; +} + export function runnerApiUrl(operation: RunnerApiOperation, input: RunnerApiCall, context: RunnerApiContext, apiUrl: string): URL { const origin = new URL(apiUrl); if (!["http:", "https:"].includes(origin.protocol) || origin.username || origin.password || origin.search || origin.hash) throw new Error("Invalid configured Paperclip API origin"); @@ -152,6 +179,7 @@ export function runnerApiPagination(headers: Headers) { export async function executeRunnerApi(input: RunnerApiCall, context: RunnerApiContext, io: RunnerApiIo) { const inlineBytes = RUNNER_API_INLINE_BYTES; const { operation, input: validated } = validateRunnerApiCall(input, context); + assertRunnerApiDispatchable(operation, validated, Object.keys(input)); input = validated; const sensitiveResponse = runnerApiSensitiveResponse(operation.method, operation.path); const url = runnerApiUrl(operation, input, context, io.apiUrl); diff --git a/server/src/services/native-runtime/runner-api.integration.test.ts b/server/src/services/native-runtime/runner-api.integration.test.ts index 3093f5f8be2..22d4c31b504 100644 --- a/server/src/services/native-runtime/runner-api.integration.test.ts +++ b/server/src/services/native-runtime/runner-api.integration.test.ts @@ -554,6 +554,38 @@ else if(m.id!==undefined) send({id:m.id,result:{}}); expect(await effects()).toEqual(before); }); + it("recovers a body-less PATCH saved before the body rule while rejecting a new one before dispatch", async () => { + const fixture = await server.fixture(); + const canonical = (value: unknown): string => Array.isArray(value) ? `[${value.map(canonical).join(",")}]` + : value && typeof value === "object" ? `{${Object.keys(value).sort().map(key => `${JSON.stringify(key)}:${canonical((value as Record)[key])}`).join(",")}}` + : JSON.stringify(value) ?? "null"; + const sha = (value: string) => createHash("sha256").update(value).digest("hex"); + const bodyless = { operationId: "PATCH /api/projects/{id}", pathParams: { id: fixture.projectId } }; + const call = { tool: "call_api", callId: "saved-bodyless-patch", arguments: bodyless }; + // An older server accepted and completed this call; seed its receipt from a real completed PATCH. + const result = await fixture.authority.execute({ ...call, arguments: { ...bodyless, body: { name: "Saved before upgrade" } } }); + expect(result).toMatchObject({ ok: true, status: 200 }); + const [run] = await server.db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, fixture.runId)); + const receipts = structuredClone((run.resultJson as Record).apiToolReceipts); + receipts[sha(call.callId)].digest = sha(canonical(bodyless)); + await server.db.update(heartbeatRuns).set({ resultJson: { ...run.resultJson, apiToolReceipts: receipts } }).where(eq(heartbeatRuns.id, fixture.runId)); + const reopened = new PaperclipRunnerToolAuthority(server.db, fixture.authority.binding); + const network = vi.spyOn(globalThis, "fetch"); + try { + expect(await reopened.recover(call)).toEqual({ result }); + expect(await reopened.execute(call)).toEqual(result); + const rejected = { ...call, callId: "new-bodyless-patch" }; + await expect(reopened.execute(rejected)).rejects.toThrow(/not sent.*requires body/); + await expect(reopened.execute(rejected)).rejects.toThrow(/not sent.*requires body/); + expect(network).not.toHaveBeenCalled(); + } finally { network.mockRestore(); } + // The rejected call reserved no receipt, so its callId stays usable for the corrected call. + const [after] = await server.db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, fixture.runId)); + expect(Object.keys((after.resultJson as Record).apiToolReceipts)).not.toContain(sha("new-bodyless-patch")); + expect(await reopened.execute({ tool: "call_api", callId: "new-bodyless-patch", arguments: { ...bodyless, body: { name: "Corrected" } } })) + .toMatchObject({ ok: true, status: 200, data: { name: "Corrected" } }); + }); + it("returns and replays scoped secret values without plaintext receipts or response assets", async () => { const fixture = await server.fixture(); const value = "disposable-private-credential-".repeat(1500); diff --git a/server/src/services/native-runtime/runner-api.test.ts b/server/src/services/native-runtime/runner-api.test.ts index a3401d5c953..b08b92c611d 100644 --- a/server/src/services/native-runtime/runner-api.test.ts +++ b/server/src/services/native-runtime/runner-api.test.ts @@ -275,7 +275,8 @@ describe("runner API request boundary", () => { [name, name === "companyId" ? context.companyId : "fixture"]), ); const result = await executeRunnerApi( - { operationId: operation.operationId, pathParams }, context, io(request), + // A body-less PATCH is rejected before dispatch; send an empty update. + { operationId: operation.operationId, pathParams, ...(operation.method === "PATCH" && operation.requestBody?.required ? { body: {} } : {}) }, context, io(request), ); expect(operation.callPolicy).toBe("rest"); expect(result).toMatchObject({ ok: false, status: 403, data: { error: "Route permission required" } }); @@ -326,7 +327,7 @@ describe("runner API request boundary", () => { ); await expect( executeRunnerApi( - { operationId: operation.operationId, pathParams }, + { operationId: operation.operationId, pathParams, ...(operation.method === "PATCH" ? { body: { status: "resolved" } } : {}) }, context, io(request), ), @@ -759,7 +760,26 @@ describe("runner API request boundary", () => { }); it("does not page mutation responses by repeating the mutation", async () => { const request = vi.fn(); - await expect(executeRunnerApi({ operationId: createProject, body: { name: "Once" }, responseText: {} }, context, io(request))).rejects.toThrow("GET"); + await expect(executeRunnerApi({ operationId: createProject, body: { name: "Once" }, responseText: { offsetBytes: 4 } }, context, io(request))).rejects.toThrow("This request was not sent"); + expect(request).not.toHaveBeenCalled(); + }); + it.each([{}, { limitBytes: 8192 }, { offsetBytes: 0, limitBytes: 4096 }])("sends a mutation once when responseText asks only for the first window: %j", async responseText => { + const request = vi.fn(async () => Response.json({ id: "company-a", name: "Acorn Studio" })); + const result = await executeRunnerApi({ operationId: "PATCH /api/companies/{companyId}", pathParams: { companyId: "company-a" }, body: { name: "Acorn Studio" }, responseText }, context, io(request)); + expect(request).toHaveBeenCalledTimes(1); + const [, init] = request.mock.calls[0]!; + expect(init?.method).toBe("PATCH"); + expect(JSON.parse(String(init?.body))).toEqual({ name: "Acorn Studio" }); + expect(result).toMatchObject({ ok: true, status: 200, data: { name: "Acorn Studio" } }); + expect(result).not.toHaveProperty("responseText"); + }); + it("rejects a PATCH with no body instead of sending a no-op update", async () => { + const request = vi.fn(); + const call = { operationId: "PATCH /api/companies/{companyId}", pathParams: { companyId: "company-a" }, query: {}, responseText: { limitBytes: 8192 } }; + await expect(executeRunnerApi(call, context, io(request))).rejects.toThrow(/not sent.*requires body.*name.*Received arguments: operationId, pathParams, query, responseText/); + expect(() => validateRunnerApiCall({ operationId: "PATCH /api/companies/{companyId}", pathParams: { companyId: "company-a" }, body: {} }, context)).not.toThrow(); + // Recovery validates saved calls without dispatch: an older accepted body-less PATCH stays readable. + expect(() => validateRunnerApiCall({ operationId: "PATCH /api/companies/{companyId}", pathParams: { companyId: "company-a" } }, context)).not.toThrow(); expect(request).not.toHaveBeenCalled(); }); it("rejects binary, invalid UTF-8 and offsets that split text", async () => {