fix(api): send call_api mutations once when responseText asks only for the first window (#20344)

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Native agents use the `call_api` fallback tool for API operations
that have no dedicated tool.
> - `responseText` is an optional `call_api` argument. It returns a
bounded text window of a GET response.
> - The server rejects every `responseText` on a mutation, also a
first-window request that cannot repeat the mutation.
> - A native agent attached `responseText` to a company rename PATCH. It
got the same 400 on each retry, and no call succeeded.
> - The same call also had no `body`. A PATCH without a body changes
nothing, but the route returns 200. With the first rejection removed,
the agent could read that 200 as a completed rename.
> - This pull request sends a mutation once when `responseText` asks
only for the first window, and it rejects a PATCH without a body before
dispatch.
> - The benefit is that a valid mutation goes through on the first call,
a later-page request still cannot repeat a mutation, and an empty PATCH
cannot look like a successful update.

## Linked Issues or Issue Description

Refs #18105. That open pull request keeps the 400 for all mutation
`responseText` values and corrects its message. This pull request
changes the same check. It accepts a first-window request and keeps a
corrected 400 for a later page. Whichever pull request merges second
must resolve this line.

**What happened**

A chat agent (OpenCode, `deepseek-v4.1-flash`) tried to rename its
company with `call_api`. We recovered the exact arguments from the
payload digest in the run events: `operationId: "PATCH
/api/companies/{companyId}"`, `pathParams.companyId`, `query: {}` and
`responseText: { limitBytes: 8192 }`. There was no `body`. The OpenCode
MCP bridge validates the arguments with Ajv without `useDefaults`, and
`8192` is not a default in any schema. Thus the model, not the bridge,
sent `responseText`. The server returned "responseText requires a GET
operation". The agent sent the identical call 10 more times and one
identical branding PATCH. No call succeeded. A second chat case in the
same cohort sent `responseText` on `POST /api/issues/{id}/inbox-archive`
three times before it recovered.

**Expected behavior**

`responseText` with no offset or offset 0 changes only how the response
is shown. The mutation must run once and return its response on the
normal inline or saved-artifact path. A later page (`offsetBytes > 0`)
must still fail before dispatch, because it would repeat the mutation. A
PATCH without a body must fail before dispatch with a message that names
`body`, the allowed fields and the received argument keys.

**Steps to reproduce**

1. Call `executeRunnerApi` with `PATCH /api/companies/{companyId}`, a
body, and `responseText: { limitBytes: 8192 }`.
2. Before this change: a 400, and no HTTP request. After: one PATCH with
the body, and the normal response.
3. Call it with no body. Before: the request is sent and the route
returns 200 with no change. After: a 400 that names `body`, and no HTTP
request.

## What Changed

- `validateRunnerApiCall`: for a non-GET operation, drop a first-window
`responseText` (no offset or offset 0) and continue. A later page still
returns 400 with "This request was not sent" and the reason.
- New `assertRunnerApiDispatchable`: reject a new PATCH with a required
request body when `body` and `files` are both absent. The message lists
up to 12 allowed fields and the received argument keys. `call_api` runs
it inside its receipt reservation, after saved and inherited receipts
are read and before it writes a pending receipt, so a rejected call
leaves no receipt. `executeRunnerApi` also runs it for direct callers.
Recovery and receipt replay use only `validateRunnerApiCall`, so a
body-less PATCH that an older server completed still returns its saved
result.
- Tests: first-window `responseText` on a PATCH dispatches once with the
body (three window shapes). A later page is rejected with no dispatch. A
body-less PATCH is rejected with no dispatch, and `body: {}` is still
accepted. The catalog-wide dispatch tests now send `body: {}` for PATCH
operations with a required body.

## Verification

- Red: the 5 new or changed cases failed on `origin/master`
(`runner-api.test.ts`).
- Green: `npx vitest run src/services/native-runtime/runner-api.test.ts
src/services/native-runtime/runner-tool-results.test.ts` passed (2,087
tests). `tests/runner-e2e/api-response-reading.test.ts` passed (14
tests).
- `tsc --noEmit -p server/tsconfig.json`: no new errors. The error count
is the same before and after this change; the remaining errors come from
packages that are not built in this worktree.
- Upgrade safety and receipts: a new integration test seeds a completed
receipt for a body-less PATCH. `recover()` and a same-callId `execute()`
return the saved result. A new body-less PATCH is rejected twice with
the same error, writes no receipt and sends no HTTP request, and the
corrected PATCH with that callId then succeeds. The test failed with the
rule in `validateRunnerApiCall`, and again with the rule after the
reservation; it passes now. `runner-api.integration.test.ts`,
`runner-api.test.ts` and `native-case-tools.integration.test.ts` pass
(2,164 passed, 2 skipped).
- One paid chat eval run (GitHub Actions run 42680925312) with the first
commit cherry-picked onto the eval branch: `companies.rename-company`
passed. The agent sent one correct PATCH with a body, attributed to the
agent. That run did not send `responseText`, so it does not exercise the
changed path; the unit tests cover it.

## Risks

- Behavior change: a mutation with a first-window `responseText` now
runs. Before, it failed. The response comes back on the normal path
(inline up to 24 KiB, else a saved artifact), not as a text window.
- Behavior change: a caller that sent a PATCH with no body to a route
with a required body now gets a 400. Such a call changed nothing before.
POST, PUT and DELETE are not changed, because some of them have useful
empty bodies.
- No route, permission or company check changes.

## Model Used

- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code
execution in Claude Code.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
DottaandClaude Opus 5.5 authored and GitHub committed 2026-11-09 14:03:08 -06:00
1 parent 97b89981d3
commit 8a6b082274
4 files changed
+89 -6

No files matched your search

@@ -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));
@@ -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<string, unknown> } | 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);
@@ -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<string, unknown>)[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<string, any>).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<string, any>).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);
@@ -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<typeof fetch>();
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<typeof fetch>(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<typeof fetch>();
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 () => {