From 0dfbbdeb317b7e729d4a28d08f46cdbcd6f30240 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 12 Nov 2026 00:27:35 -0600 Subject: [PATCH] fix(chat): route company creation, decline retries and question cancellation correctly (#21035) 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 follow the chat directive and discover API routes with search_api and call_api > - Full chat cohort 4 (OpenCode with DeepSeek v4.1 flash) showed four guidance gaps > - The missing-name rule overrode the board-only company-creation boundary; a repeated request after a decline did not post a new approval card; search_api showed the board-only question cancel route as open to agents (and withdraw as board-only); write calls carried `responseText` beside or instead of `body` > - This pull request fixes the guidance, the route authorization metadata and the pre-send refusal > - The benefit is that agents take the supported path the first time instead of asking for the wrong input, refusing a valid retry, or calling a route that returns 403 ## Linked Issues or Issue Description **What happened?** - onboarding.new-company: asked to "set up a new company for my website business", the CEO cited the missing-name rule (#21007) and asked for a business name with a question card. Creating a company is a board action, so the agent should send the person to the creation flow. - app-permissions.ask-before-action: the person declined a write card and then repeated the same request. The CEO asked the person to tell it to resubmit instead of posting a new approval card. The gateway already allows a fresh request after a new human message (`isNewHumanIntentAfterToolRejection`), but the decline text said "Do not retry the same call". - questions-and-approvals.multi-part-question: search_api listed `POST /api/issues/{id}/interactions/{interactionId}/cancel` as `board_or_agent`. The route rejects agents. The agent route, `/withdraw`, was listed as board-only although its route admits the card's creator agent and the task's assignee. - Several cases: write calls through call_api carried `responseText` with no body (for example "Received arguments: operationId, pathParams, responseText"). **Expected behavior** The agent links the company-creation flow; it posts a fresh approval card for a repeated request; search_api shows the true authorization of cancel and withdraw; a write with `responseText` is refused before it is sent, with the same guidance #21002 gave `saveResponse`. **Steps to reproduce** Run the four chat cases above with OpenCode and DeepSeek v4.1 flash on master. Refs #21002, #21007 ## What Changed - `AGENT_CHAT_DIRECTIVE`: the missing-name rule applies to work the agent creates itself and never comes before a board-only boundary. For a new company the agent links [Create organization](/onboarding) (no company prefix) and repeats the business details. A repeated request after a declined approval card is an explicit retry: make the same call so a fresh card is posted. A clarification question card puts candidate links in its description, because labels show as plain text. - Decline instructions in `tool-action-delivery.ts` and `tool-gateway.ts` say that a later request from the person for the same action is an explicit retry. - OpenAPI: `POST /api/issues/{id}/interactions/{interactionId}/cancel` is board-only and its summary names the `/withdraw` route for agents. `/withdraw` is no longer marked board-only, which matches its route. - `call_api`: a new write that carries `responseText` is refused before it is sent; the message says `responseText` never carries request fields and names the allowed body fields. Older saved calls still validate for recovery. `doc/runner-api-tools.md` is updated. ## Verification - `server/src/__tests__/agent-conversations.test.ts`: new directive tests for the board-only boundary order, the decline retry and card links. - `server/src/__tests__/openapi-routes.test.ts`: cancel is `board`, withdraw is `board_or_agent`, the cancel summary names withdraw. - `server/src/services/native-runtime/runner-api.test.ts`: a write with `responseText` is refused before sending (three window shapes), the same write without it is sent once, and saved calls still validate. - `tool-gateway.test.ts`, `tool-gateway-service.test.ts`, `issue-thread-interaction-routes.test.ts`, `tool-action-replay-intent.test.ts`: passed. `server` `tsc --noEmit`: passed. - Paid run (8 cells, OpenCode + DeepSeek v4.1 flash) together with the eval PR: see the PR comment for results. ## Risks - A model that used to send a write with `responseText` and a valid body now gets one refusal and must resend without it. Nothing is sent before the refusal, so there is no duplicate side effect. - The directive grows by a few sentences, which adds a small number of prompt tokens per chat turn. - Agents that read `/withdraw` as board-only before now see that they can withdraw their own card; the route already enforced who may. ## 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] 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 | 5 ++++- .../src/__tests__/agent-conversations.test.ts | 21 +++++++++++++++++++ server/src/__tests__/openapi-routes.test.ts | 10 +++++++++ server/src/routes/openapi.ts | 6 ++++-- server/src/services/agent-conversations.ts | 4 +++- .../native-runtime/runner-api-client.ts | 9 ++++++++ .../native-runtime/runner-api.test.ts | 20 ++++++++++++------ server/src/services/tool-action-delivery.ts | 2 +- server/src/services/tool-gateway.ts | 2 +- 9 files changed, 67 insertions(+), 12 deletions(-) diff --git a/doc/runner-api-tools.md b/doc/runner-api-tools.md index e35df345626..319e4cd5d0b 100644 --- a/doc/runner-api-tools.md +++ b/doc/runner-api-tools.md @@ -149,7 +149,10 @@ 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. +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. 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 e51d83ed205..f233015711e 100644 --- a/server/src/__tests__/agent-conversations.test.ts +++ b/server/src/__tests__/agent-conversations.test.ts @@ -1001,6 +1001,27 @@ describe("chat prompt policy", () => { expect(prompt).toContain("//agents/"); }); + // Full chat cohort 4 (onboarding.new-company): the CEO applied the missing-name rule and asked for a business name + // instead of sending the person to the board-only company-creation flow. + it("keeps the missing-name rule behind the board-only company-creation boundary", () => { + expect(AGENT_CHAT_DIRECTIVE).toContain("This name rule is for work you create yourself. It never comes before a board-only boundary"); + expect(AGENT_CHAT_DIRECTIVE).toContain("Otherwise link [Create organization](/onboarding), which opens the new-organization wizard"); + // Paperclip Cloud creates organizations in the Cloud portfolio; a bare /onboarding there opens the existing company's agent step. + expect(AGENT_CHAT_DIRECTIVE).toContain("when its response has a cloud field, this is a Paperclip Cloud stack, organizations are created in the person's Paperclip Cloud portfolio rather than in this app, so say that and do not link /onboarding"); + expect(AGENT_CHAT_DIRECTIVE).toContain("/onboarding takes no company prefix; //onboarding only adds an agent to this company"); + const nameRule = AGENT_CHAT_DIRECTIVE.indexOf("Only when none of these gives a name, ask for it before starting"); + expect(nameRule).toBeGreaterThan(-1); + expect(AGENT_CHAT_DIRECTIVE.indexOf("It never comes before a board-only boundary")).toBeGreaterThan(nameRule); + }); + // Full chat cohort 4 (app-permissions.ask-before-action): after a decline, the repeated request got "tell me to resubmit". + it("treats a repeated request after a declined approval card as an explicit retry that posts a fresh card", () => { + expect(AGENT_CHAT_DIRECTIVE).toContain("the new message is an explicit retry, not an ambiguous request: make the same call again so Paperclip posts a fresh approval card"); + expect(AGENT_CHAT_DIRECTIVE).toContain("Do not ask them to confirm the retry, tell them to resubmit, or treat the earlier decline as final."); + }); + // Full chat cohort 4 (tasks.ambiguous-task): the clarification card named both candidates in plain option labels. + it("puts candidate links in a clarification card's description, which renders links", () => { + expect(AGENT_CHAT_DIRECTIVE).toContain("If you ask with a question card instead, put the candidate links in the card's description"); + }); it("tells chat that instruction saves keep no history, so an undo with no listed revision asks instead of reconstructing", () => { expect(AGENT_CHAT_DIRECTIVE).toContain("Saving instructions does not record earlier versions"); expect(AGENT_CHAT_DIRECTIVE).toContain("an empty history means no earlier text is retained"); diff --git a/server/src/__tests__/openapi-routes.test.ts b/server/src/__tests__/openapi-routes.test.ts index 70f02e50c45..ab15de12539 100644 --- a/server/src/__tests__/openapi-routes.test.ts +++ b/server/src/__tests__/openapi-routes.test.ts @@ -410,6 +410,16 @@ describe("openapi routes", () => { for (const code of ["200", "401", "403"]) expect(operation.responses[code]).toBeDefined(); }); + // Full chat cohort 4: search_api listed the cancel route as open to agents, so an agent tried it and got 403. + it("documents question cancellation as board-only and points agents to withdraw", () => { + const spec = buildOpenApiSpec() as any; + const cancel = spec.paths["/api/issues/{id}/interactions/{interactionId}/cancel"].post; + expect(cancel["x-paperclip-authorization"]).toEqual({ actor: "board" }); + expect(cancel.security).toEqual([{ BoardSessionAuth: [] }, { BoardApiKeyAuth: [] }]); + expect(cancel.summary).toContain("POST /api/issues/{id}/interactions/{interactionId}/withdraw"); + expect(spec.paths["/api/issues/{id}/interactions/{interactionId}/withdraw"].post["x-paperclip-authorization"]).toEqual({ actor: "board_or_agent" }); + }); + 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; diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index 8bb2fe0cdee..f6bac90e8d2 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -1462,6 +1462,9 @@ const BOARD_ONLY_OPERATIONS = new Set([ "POST /api/companies/{companyId}/attention/{sourceKind}/{sourceId}/transfer", "POST /api/approvals/{id}/cancel", "POST /api/issues/{id}/interactions/{interactionId}/dismiss", + // Cancelling a question is board-only (routes/issues.ts rejects agent actors). Withdraw is not listed here: its + // route admits the card's creator agent and the task's assignee, so agents withdraw their own pending card there. + "POST /api/issues/{id}/interactions/{interactionId}/cancel", "POST /api/issues/{id}/retry", "GET /api/companies/{companyId}/prompt-injection-prevention", "PUT /api/companies/{companyId}/prompt-injection-prevention", @@ -1557,7 +1560,6 @@ const BOARD_ONLY_OPERATIONS = new Set([ "POST /api/issues/{id}/interactions/{interactionId}/reject", "POST /api/issues/{id}/interactions/{interactionId}/respond", "POST /api/issues/{id}/interactions/{interactionId}/skip", - "POST /api/issues/{id}/interactions/{interactionId}/withdraw", "POST /api/companies/{companyId}/tools/examples/{id}/smoke", "POST /api/agents/me/connections/{connectionId}/token", "GET /api/tools/oauth/callback", @@ -11489,7 +11491,7 @@ registerCurrentRoute({ method: "post", path: "/api/issues/{id}/interactions/{interactionId}/cancel", tags: ["issues"], - summary: "Cancel an issue question interaction", + summary: "Cancel an issue question interaction (board users only; an agent withdraws its own pending card with POST /api/issues/{id}/interactions/{interactionId}/withdraw)", body: cancelIssueThreadInteractionSchema, }); diff --git a/server/src/services/agent-conversations.ts b/server/src/services/agent-conversations.ts index 5ad146fe31b..3e43d8b821d 100644 --- a/server/src/services/agent-conversations.ts +++ b/server/src/services/agent-conversations.ts @@ -90,7 +90,9 @@ export function isConversationReset(body: string): boolean { return body.trim() === "/new"; } -export const AGENT_CHAT_DIRECTIVE = `You are in an ongoing conversation with the user. Help them clarify the outcome they want. Ask focused questions when missing information materially affects the task; when the request is already clear, do not require a ritual confirmation. When a request to change one record matches several records you can access and nothing in the conversation selects one, ask which one before changing any of them: link each candidate, name what distinguishes them (such as project or status), and restate the requested change. Do not pick the first match, change every match, or keep searching for a hidden answer; asking in your reply is enough, and you finish the change when the person answers. When the person asks you to build or hand off something that must carry a name, such as the product or brand name shown on a landing page, use the name their request, this conversation or the work's saved brief gives; a project the person names in the request counts, so "Start a Launch project and build its landing page" carries the name Launch and needs no question. Only when none of these gives a name, ask for it before starting: use request_human_input with interactionKind: "questions" and one required free-text question, then end your turn. Do not take the name from unrelated records such as other existing projects or the company name. Do not build, delegate or brief the work with an invented or placeholder name; when the answer arrives, put it in the brief of the work you create. +export const AGENT_CHAT_DIRECTIVE = `You are in an ongoing conversation with the user. Help them clarify the outcome they want. Ask focused questions when missing information materially affects the task; when the request is already clear, do not require a ritual confirmation. When a request to change one record matches several records you can access and nothing in the conversation selects one, ask which one before changing any of them: link each candidate, name what distinguishes them (such as project or status), and restate the requested change. Do not pick the first match, change every match, or keep searching for a hidden answer; asking in your reply is enough, and you finish the change when the person answers. If you ask with a question card instead, put the candidate links in the card's description: its option labels show as plain text, without links. When the person asks you to build or hand off something that must carry a name, such as the product or brand name shown on a landing page, use the name their request, this conversation or the work's saved brief gives; a project the person names in the request counts, so "Start a Launch project and build its landing page" carries the name Launch and needs no question. Only when none of these gives a name, ask for it before starting: use request_human_input with interactionKind: "questions" and one required free-text question, then end your turn. Do not take the name from unrelated records such as other existing projects or the company name. Do not build, delegate or brief the work with an invented or placeholder name; when the answer arrives, put it in the brief of the work you create. This name rule is for work you create yourself. It never comes before a board-only boundary: creating a new company (organization) is a person's action that no agent credential can take, so do not ask for its name or other details first. Reply now: tell the person where they create it and repeat the business details they gave so they can enter them there. Where depends on the deployment, which GET /api/health shows: when its response has a cloud field, this is a Paperclip Cloud stack, organizations are created in the person's Paperclip Cloud portfolio rather than in this app, so say that and do not link /onboarding. Otherwise link [Create organization](/onboarding), which opens the new-organization wizard (also Create organization in the company switcher). /onboarding takes no company prefix; //onboarding only adds an agent to this company. + +When the person declined an approval card for an action and then asks for that same action again in a later message, the new message is an explicit retry, not an ambiguous request: make the same call again so Paperclip posts a fresh approval card for them to decide. Do not ask them to confirm the retry, tell them to resubmit, or treat the earlier decline as final. When changing defaults or policy for new or future work, update the resource that owns those defaults. Leave existing items and active work unchanged unless the user explicitly includes them in the requested change. This boundary includes metadata and association changes, even when they seem harmless or helpful. Do not broaden the request to align related existing items. Explain which subsequent work inherits the saved defaults and whether existing work changed. diff --git a/server/src/services/native-runtime/runner-api-client.ts b/server/src/services/native-runtime/runner-api-client.ts index 22d0dabbd46..9f41df7c50a 100644 --- a/server/src/services/native-runtime/runner-api-client.ts +++ b/server/src/services/native-runtime/runner-api-client.ts @@ -89,6 +89,15 @@ export function validateRunnerApiCall(value: unknown, context: RunnerApiContext) * 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")) { + 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 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 // update. A POST or PUT without a body whose schema names required fields diff --git a/server/src/services/native-runtime/runner-api.test.ts b/server/src/services/native-runtime/runner-api.test.ts index 53b8de560fd..5d9a3055ce6 100644 --- a/server/src/services/native-runtime/runner-api.test.ts +++ b/server/src/services/native-runtime/runner-api.test.ts @@ -811,15 +811,23 @@ 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(); }); - it.each([{}, { limitBytes: 8192 }, { offsetBytes: 0, limitBytes: 4096 }])("sends a mutation once when responseText asks only for the first window: %j", async responseText => { + // 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 => { 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)); + 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); + expect(request).not.toHaveBeenCalled(); + expect(error?.message).toContain("This request was not sent: responseText applies only to GET reads and never carries request fields."); + 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("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)); 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(JSON.parse(String(request.mock.calls[0]![1]?.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(); diff --git a/server/src/services/tool-action-delivery.ts b/server/src/services/tool-action-delivery.ts index 92db1431888..62365177ae3 100644 --- a/server/src/services/tool-action-delivery.ts +++ b/server/src/services/tool-action-delivery.ts @@ -241,7 +241,7 @@ export function toolActionDeliveryService( request.status === "executed" ? "The approved action already ran. Do not call it again; continue with the recorded result. Process the result and answer the user in your own words. Do not paste the raw tool or transport JSON unless the user asks for it." : request.status === "rejected" - ? "The human declined this action. Do not retry the same call. Adjust your approach or explain what is blocked." + ? "The human declined this action. Do not retry the same call on your own. Adjust your approach or explain what is blocked. If the person asks for the same action again in a later message, that is an explicit retry: make the call again so a fresh approval card is posted." : request.status === "expired" || request.status === "cancelled" ? "The review is no longer available. Do not execute the stored request. Explain the recorded outcome before requesting another review." diff --git a/server/src/services/tool-gateway.ts b/server/src/services/tool-gateway.ts index 54ebbfb0974..80e88955423 100644 --- a/server/src/services/tool-gateway.ts +++ b/server/src/services/tool-gateway.ts @@ -8970,7 +8970,7 @@ export function createToolGatewayService( invocationId: invocation.id, actionRequestId: actionRequest.id, instructions: - "The action was declined. Do not retry the same call; adjust your approach or report the decline on the task.", + "The action was declined. Do not retry the same call on your own; adjust your approach or report the decline on the task. If the person asks for the same action again in a new message, the call made for that message posts a fresh approval card.", }, ); }