From 4e519bd4286ef8daa42c92fca715101e760f696c Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 12 Nov 2026 02:09:14 -0600 Subject: [PATCH] fix(chat): send writes with an empty responseText and point card declines to resolve-from-comment (#21050) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Chat agents call the Paperclip API through `call_api` and find routes with `search_api` > - #21035 added a pre-send refusal for writes that carry `responseText`, and new summaries for the question cancel and withdraw routes > - Full chat cohort 5 (OpenCode with DeepSeek v4.1 flash) showed that both changes went too far > - The refusal blocked a valid write that had an empty `responseText`, and the summaries hid the route that records a person's decline > - This pull request narrows the refusal and corrects the guidance text > - The benefit is that valid writes go through on the first try, and agents settle a declined card instead of leaving it pending ## Linked Issues or Issue Description Refs #21035, #21002 **What happened?** - inbox-search.restore-archive: DeepSeek sent `responseText: {}` with a valid body on `POST /api/issues/{id}/inbox-archive`. The refusal from #21035 rejected 24 POSTs in 3 minutes. The model did not drop the key, and the run was cancelled at the 240s window. In cohort 4 the same POSTs went through on the first try. - questions-and-approvals.reject-and-revise: the person said "Do not publish it; just make a draft I can review." The CEO quoted the cancel summary ("an agent withdraws its own pending card with POST .../withdraw"), saw that the person created the publish card, and left the card pending. The plan decision did not settle. In cohorts 3 and 4 the CEO declined the card with `resolve-from-comment` (resolver policy `anyone`). **Expected behavior** - A write with an empty or default `responseText` is sent once. A write whose `responseText` sets a window is still refused before it is sent. - The guidance says that withdraw is only for the agent's own obsolete card, and that a person's explicit decline or answer goes through `resolve-from-comment` when the card's resolver policy allows. **Steps to reproduce** Run the two chat cases above with OpenCode and DeepSeek v4.1 flash on master. ## What Changed - `call_api` (`assertRunnerApiDispatchable`): the check reads the raw `responseText` value. `{}` or `{ offsetBytes: 0 }` on a write is dropped and the write is sent. A `responseText` with `limitBytes` or a later offset is still refused. `saveResponse: false` on a write was already accepted; a test now covers it. - OpenAPI: the cancel and withdraw summaries say that withdraw is only for the agent's own obsolete card, and that a decline or answer on the person's explicit instruction uses `POST .../resolve-from-comment` when the resolver policy allows. The `resolve-from-comment` summary says it also records a decline of a card the person created. - `AGENT_CHAT_DIRECTIVE`: a person's explicit decline in chat answers a pending confirmation with reject, whoever created the card. Withdraw never records the person's decision. - `doc/runner-api-tools.md`: describes the narrowed refusal. ## Verification - `server/src/services/native-runtime/runner-api.test.ts`: new test sends `POST /api/issues/{id}/inbox-archive` with a valid body and `responseText: {}`, `{ offsetBytes: 0 }`, `saveResponse: false`, or both. The request is sent exactly once. Red before the fix (3 of 4 cases refused), green after. The refusal test for a `responseText` with `limitBytes` stays. - `server/src/__tests__/openapi-routes.test.ts`: new test for the cancel, withdraw and resolve-from-comment summaries. Red before, green after. - `server/src/__tests__/agent-conversations.test.ts`: new directive test for the decline rule. Red before, green after. - Also passed: `runner-api.integration.test.ts`, `native-case-tools.integration.test.ts`, server `tsc --noEmit`. SDK public input pins: `--check` reports no change. - Paid run (OpenCode + DeepSeek v4.1 flash) on this branch for inbox-search.restore-archive, questions-and-approvals.reject-and-revise, gateways-profiles.rotate-token and budgets-costs.repair-cost-recording: 4 of 4 cells passed on 3f20fb2bcc (run 43177772183); details in a PR comment. ## Risks - A write with a populated `responseText` is still refused, as in #21035. Only the empty and default shapes change. A dropped empty `responseText` changes nothing, because the server already removed it from writes before sending. - The directive grows by two sentences, which adds a small number of prompt tokens per chat turn. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`) in Claude Code, with tool use, shell execution and code editing. ## 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 - [x] All Paperclip CI gates are green - [x] 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 --- doc/runner-api-tools.md | 11 ++++++---- .../src/__tests__/agent-conversations.test.ts | 4 ++++ server/src/__tests__/openapi-routes.test.ts | 15 +++++++++++++- server/src/routes/openapi.ts | 8 ++++---- server/src/services/agent-conversations.ts | 2 +- .../paperclip-runner-tool-authority.ts | 2 +- .../native-runtime/runner-api-client.ts | 15 ++++++++------ .../native-runtime/runner-api.test.ts | 20 ++++++++++++++++--- 8 files changed, 57 insertions(+), 20 deletions(-) diff --git a/doc/runner-api-tools.md b/doc/runner-api-tools.md index 319e4cd5d0b..a4dc4081fc2 100644 --- a/doc/runner-api-tools.md +++ b/doc/runner-api-tools.md @@ -149,10 +149,13 @@ The result contains `data` as text (including JSON), plus `responseText` with until it is null. Windows end at UTF-8 boundaries. The limit defaults to 24 KiB and accepts 4–24,576 bytes. The server rejects binary content, invalid UTF-8, and offsets inside a code point or beyond the response. This option only works -with GET; never repeat a mutation to retrieve another part of its response. A -new write call that carries `responseText` is refused before it is sent, like -`saveResponse`: it never carries request fields, so resend the write once -without it and with its fields in `body`. Older saved calls stay valid for recovery. +with GET; never repeat a mutation to retrieve another part of its response. On +a write, an empty or default `responseText` (`{}` or `offsetBytes: 0`) and +`saveResponse: false` change nothing, so they are dropped and the write is sent +once. A new write whose `responseText` sets a window (`limitBytes` or a later +offset) is refused before it is sent: it never carries request fields, so resend +the write once without it and with its fields in `body`. Older saved calls stay +valid for recovery. Read the saved artifact for a stable snapshot instead of paging a changing live response. A text-window call against a live response above 24 KiB also returns that snapshot's artifact reference; continue on its content operation. A saved diff --git a/server/src/__tests__/agent-conversations.test.ts b/server/src/__tests__/agent-conversations.test.ts index f233015711e..c230042f65b 100644 --- a/server/src/__tests__/agent-conversations.test.ts +++ b/server/src/__tests__/agent-conversations.test.ts @@ -987,6 +987,10 @@ describe("conversation execution wake policy", () => { }); describe("chat prompt policy", () => { + it("tells chat to record a person's decline with resolve-from-comment whoever created the card, and keeps withdraw for its own obsolete card", () => { + expect(AGENT_CHAT_DIRECTIVE).toContain("A person's explicit decline in chat (for example \"do not publish it\") answers a pending confirmation with reject, whoever created the card"); + expect(AGENT_CHAT_DIRECTIVE).toContain("Withdraw is only for your own card that later evidence made obsolete; it never records the person's decision"); + }); it("tells chat to save a requested start order as a blocker on the waiting task, not only a promise to hold it", () => { expect(AGENT_CHAT_DIRECTIVE).toContain("must not start until another task is done or approved"); expect(AGENT_CHAT_DIRECTIVE).toContain("add the prerequisite to the waiting task's blockedByIssueIds with PATCH /api/issues/{id}, keeping its existing blockers"); diff --git a/server/src/__tests__/openapi-routes.test.ts b/server/src/__tests__/openapi-routes.test.ts index ab15de12539..c4e7fc9c54a 100644 --- a/server/src/__tests__/openapi-routes.test.ts +++ b/server/src/__tests__/openapi-routes.test.ts @@ -420,6 +420,19 @@ describe("openapi routes", () => { expect(spec.paths["/api/issues/{id}/interactions/{interactionId}/withdraw"].post["x-paperclip-authorization"]).toEqual({ actor: "board_or_agent" }); }); + it("points agents to resolve-from-comment to record a person's decline and keeps withdraw for their own obsolete card", () => { + const spec = buildOpenApiSpec() as any; + const interaction = "/api/issues/{id}/interactions/{interactionId}"; + for (const summary of [spec.paths[`${interaction}/cancel`].post.summary, spec.paths[`${interaction}/withdraw`].post.summary]) { + expect(summary).toContain("only for the agent's own obsolete card"); + expect(summary).toContain(`POST ${interaction}/resolve-from-comment`); + expect(summary).toContain("when the card's resolver policy allows"); + expect(summary).toContain("to accept or decline a confirmation card on the person's explicit instruction"); + expect(summary).toContain("not for question forms or governed approvals"); + } + expect(spec.paths[`${interaction}/resolve-from-comment`].post.summary).toContain("including a decline of a card the person created"); + }); + it("documents the owned pending-question read as canonical attention items without client scope overrides", () => { const spec = buildOpenApiSpec() as any; const operation = spec.paths["/api/issues/{id}/pending-questions"].get; @@ -816,7 +829,7 @@ describe("openapi routes", () => { expect( res.body.paths["/api/issues/{id}/interactions/{interactionId}/withdraw"] .post.summary, - ).toBe("Withdraw a pending issue thread interaction"); + ).toMatch(/^Withdraw a pending issue thread interaction\. /); const createInteraction = res.body.paths["/api/issues/{id}/interactions"].post; expect(createInteraction.description).toContain( diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index f6bac90e8d2..843d85134c5 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -8678,7 +8678,7 @@ registry.registerPath({ method: "post", path: "/api/issues/{id}/interactions/{interactionId}/resolve-from-comment", tags: ["issues"], - summary: "Record a user's conversational confirmation answer", + summary: "Record a user's conversational confirmation answer, including a decline of a card the person created", description: "An eligible active agent run resolves a confirmation on its own task using the latest user comment. Resolver permissions, target staleness and conversation reset boundaries remain enforced. Matching retries are idempotent. No new wake is scheduled.", request: { params: z.object({ id: z.string(), interactionId: z.string() }), @@ -8760,7 +8760,7 @@ registry.registerPath({ method: "post", path: "/api/issues/{id}/interactions/{interactionId}/withdraw", tags: ["issues"], - summary: "Withdraw a pending issue thread interaction", + summary: "Withdraw a pending issue thread interaction. For agents, withdraw is only for the agent's own obsolete card; to accept or decline a confirmation card on the person's explicit instruction, an agent records the person's comment with POST /api/issues/{id}/interactions/{interactionId}/resolve-from-comment when the card's resolver policy allows (not for question forms or governed approvals)", request: { params: z.object({ id: z.string(), interactionId: z.string() }), body: jsonBody(withdrawIssueThreadInteractionSchema), @@ -11491,7 +11491,7 @@ registerCurrentRoute({ method: "post", path: "/api/issues/{id}/interactions/{interactionId}/cancel", tags: ["issues"], - summary: "Cancel an issue question interaction (board users only; an agent withdraws its own pending card with POST /api/issues/{id}/interactions/{interactionId}/withdraw)", + summary: "Cancel an issue question interaction (board users only). Agents: POST /api/issues/{id}/interactions/{interactionId}/withdraw is only for the agent's own obsolete card; to accept or decline a confirmation card on the person's explicit instruction, an agent records the person's comment with POST /api/issues/{id}/interactions/{interactionId}/resolve-from-comment when the card's resolver policy allows (not for question forms or governed approvals)", body: cancelIssueThreadInteractionSchema, }); @@ -11514,7 +11514,7 @@ registerCurrentRoute({ method: "post", path: "/api/issues/{id}/interactions/{interactionId}/withdraw", tags: ["issues"], - summary: "Withdraw a pending issue thread interaction", + summary: "Withdraw a pending issue thread interaction. For agents, withdraw is only for the agent's own obsolete card; to accept or decline a confirmation card on the person's explicit instruction, an agent records the person's comment with POST /api/issues/{id}/interactions/{interactionId}/resolve-from-comment when the card's resolver policy allows (not for question forms or governed approvals)", body: withdrawIssueThreadInteractionSchema, }); diff --git a/server/src/services/agent-conversations.ts b/server/src/services/agent-conversations.ts index 3e43d8b821d..6994ad7cc61 100644 --- a/server/src/services/agent-conversations.ts +++ b/server/src/services/agent-conversations.ts @@ -112,7 +112,7 @@ When a tool or save receipt rejects an action, tell the user which action failed When the user asks to approve a plan before handoff, publish the plan and create a revision-bound approval card before ending the turn. With native tools, call request_human_input using interactionKind: "confirmation", targetRevisionId from the saved document's latestRevisionId, a revision-specific idempotencyKey, and continuationPolicy: "wake_assignee". Set payload.target to { type: "issue_document", key: "plan", revisionId: latestRevisionId }. Through the HTTP API, POST the equivalent request_confirmation interaction to /api/issues/{issueId}/interactions. A written request to approve in your reply does not create an approval card. After requested revisions, create a fresh card for the newly saved revision. This applies to explicitly requested plan approval; ordinary conversation replies and draft planning do not need confirmation. In Ask mode, discuss the plan without creating or revising documents or approval cards. -When a user answers a pending confirmation in chat, read the latest cards and user comments, identify the specific proposal and decision, and persist it before acting: POST /api/issues/{issueId}/interactions/{interactionId}/resolve-from-comment with { commentId: the user message ID, decision: "accept" or "reject" }. Use call_api on native runners. For a checkbox confirmation, also send selectedOptionIds for the choices the user actually approved; defaults alone are not consent. An ambiguous "yes" with multiple pending proposals requires clarification, not approving all of them. A requested revision is not acceptance. Do not use this endpoint for question forms or governed tool/secret approvals. Respect resolver-policy denials. If saving the decision fails, re-read the card and retry the same decision when appropriate before doing the approved work; never leave it pending and proceed anyway. If a card is already resolved, use its recorded outcome. Do not ask the user to clear a card after recording their answer. +When a user answers a pending confirmation in chat, read the latest cards and user comments, identify the specific proposal and decision, and persist it before acting: POST /api/issues/{issueId}/interactions/{interactionId}/resolve-from-comment with { commentId: the user message ID, decision: "accept" or "reject" }. Use call_api on native runners. For a checkbox confirmation, also send selectedOptionIds for the choices the user actually approved; defaults alone are not consent. An ambiguous "yes" with multiple pending proposals requires clarification, not approving all of them. A requested revision is not acceptance. Do not use this endpoint for question forms or governed tool/secret approvals. Respect resolver-policy denials. A person's explicit decline in chat (for example "do not publish it") answers a pending confirmation with reject, whoever created the card: record it with resolve-from-comment when the card's resolver policy allows, then do the revised work. Withdraw is only for your own card that later evidence made obsolete; it never records the person's decision. If saving the decision fails, re-read the card and retry the same decision when appropriate before doing the approved work; never leave it pending and proceed anyway. If a card is already resolved, use its recorded outcome. Do not ask the user to clear a card after recording their answer. Before handing off work, inspect available projects and repositories. Every task you create from this chat must belong to a suitable project. Reuse an appropriate existing project; otherwise use create_project. Consider all relevant available repositories and pass repositoryIds for one or multiple repositories when the work spans them. For existing GitHub repositories you can access that are absent from the catalog, pass their HTTPS repositoryUrls; this registers them with the project without creating remote GitHub repositories. You may combine known IDs and URLs and attach multiple repositories. The direct HTTP equivalent is POST /api/companies/{companyId}/projects with name, repositoryIds and/or repositoryUrls arrays, and an idempotencyKey. Include all selected repositories in that creation; do not combine these arrays with workspace. Never invent repository IDs or substitute inaccessible repositories. Ask when the choice is materially ambiguous or required access is missing. Non-code projects may need no repository. 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 b8edb3356fa..82f3e3f3550 100644 --- a/server/src/services/native-runtime/paperclip-runner-tool-authority.ts +++ b/server/src/services/native-runtime/paperclip-runner-tool-authority.ts @@ -1221,7 +1221,7 @@ export class PaperclipRunnerToolAuthority { } // 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))); + assertRunnerApiDispatchable(operation, input, 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 9f41df7c50a..0d4ae28abe2 100644 --- a/server/src/services/native-runtime/runner-api-client.ts +++ b/server/src/services/native-runtime/runner-api-client.ts @@ -88,15 +88,18 @@ export function validateRunnerApiCall(value: unknown, context: RunnerApiContext) * 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 { - assertBodyPresent(operation, input, receivedKeys); - // Like saveResponse, responseText is a GET read option. On a write it changes nothing, and models that send it - // there tend to put request fields beside it instead of in body; refuse before sending and say where they go. - if (operation.method !== "GET" && receivedKeys.includes("responseText")) { +export function assertRunnerApiDispatchable(operation: RunnerApiOperation, input: RunnerApiCall, received: Readonly>): void { + assertBodyPresent(operation, input, Object.keys(received)); + if (operation.method !== "GET" && setsResponseWindow(received.responseText)) { throw badRequest(`This request was not sent: responseText applies only to GET reads and never carries request fields. Resend ${operation.operationId} once without responseText, with its request fields in body${requestFieldHint(operation)}.`); } } +function setsResponseWindow(value: unknown): boolean { + if (!value || typeof value !== "object") return false; + return Object.entries(value).some(([key, setting]) => setting !== undefined && !(key === "offsetBytes" && setting === 0)); +} + function assertBodyPresent(operation: RunnerApiOperation, input: RunnerApiCall, receivedKeys: readonly string[]): void { if (input.body !== undefined || input.files?.length || !operation.requestBody?.required) return; // A PATCH without a body succeeds as a no-op that reads like a completed @@ -200,7 +203,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)); + assertRunnerApiDispatchable(operation, validated, 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.test.ts b/server/src/services/native-runtime/runner-api.test.ts index 5d9a3055ce6..2d9cebb96a4 100644 --- a/server/src/services/native-runtime/runner-api.test.ts +++ b/server/src/services/native-runtime/runner-api.test.ts @@ -811,9 +811,7 @@ describe("runner API request boundary", () => { 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(); }); - // Full chat cohort 4: write calls carried responseText beside (or instead of) their body. Like saveResponse, a new - // write with responseText is refused before sending; an older saved call still validates for recovery. - it.each([{}, { limitBytes: 8192 }, { offsetBytes: 0, limitBytes: 4096 }])("refuses a new write that carries responseText before sending: %j", async responseText => { + it.each([{ limitBytes: 8192 }, { offsetBytes: 0, limitBytes: 4096 }])("refuses a new write whose responseText sets a window before sending: %j", async responseText => { const request = vi.fn(async () => Response.json({ id: "company-a", name: "Acorn Studio" })); const call = { operationId: "PATCH /api/companies/{companyId}", pathParams: { companyId: "company-a" }, body: { name: "Acorn Studio" }, responseText }; const error = await executeRunnerApi(call, context, io(request)).then(() => null, (caught: Error) => caught); @@ -822,6 +820,22 @@ describe("runner API request boundary", () => { expect(error?.message).toContain("Resend PATCH /api/companies/{companyId} once without responseText, with its request fields in body"); expect(validateRunnerApiCall(call, context).input).not.toHaveProperty("responseText"); }); + it.each([ + { responseText: {} }, + { responseText: { offsetBytes: 0 } }, + { saveResponse: false }, + { responseText: {}, saveResponse: false }, + ])("sends a write once when its read options are empty or default: %j", async options => { + const request = vi.fn(async () => Response.json({ ok: true })); + const result = await executeRunnerApi({ operationId: "POST /api/issues/{id}/inbox-archive", pathParams: { id: "issue-a" }, body: {}, ...options }, context, io(request)); + expect(request).toHaveBeenCalledTimes(1); + const [url, init] = request.mock.calls[0]!; + expect(String(url)).toMatch(/\/api\/issues\/issue-a\/inbox-archive$/); + expect(init?.method).toBe("POST"); + expect(JSON.parse(String(init?.body))).toEqual({}); + expect(result).toMatchObject({ ok: true, status: 200 }); + expect(result).not.toHaveProperty("responseText"); + }); it("still sends the same write once without responseText", async () => { 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" } }, context, io(request));