From 8b85fdfa3cc855cda7620b56eaa4f85be0e26cd3 Mon Sep 17 00:00:00 2001 From: Aron Prins Date: Sun, 7 Jun 2026 00:55:55 +0200 Subject: [PATCH] fix(cli): send X-Paperclip-Run-Id so agents can mutate their issues via the CLI (#7642) 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. > - Agents act on issues through the `paperclipai` CLI as well as the HTTP API; the server gates agent-authenticated **mutations** of an in-progress issue (checkout, release, interactions, PATCH, attachment upload) behind the `X-Paperclip-Run-Id` header (`requireAgentRunId` / `assertAgentIssueMutationAllowed`). > - The CLI's HTTP client (`client/http.ts`) already supports sending that header, but `resolveCommandContext` never populated `runId`, so there was no way to provide it — every agent-authenticated mutation via the CLI failed with `401 Agent run id required`. > - Separately, `issue attachment:upload` hand-rolls its own multipart `fetch` (bypassing the JSON client), so it never forwarded the run-id at all, and its `inferContentTypeFromPath` couldn't produce `text/html` and appended `; charset=utf-8` to `md`/`txt` — which fails the server's exact-match content-type allowlist (`422 Unsupported attachment content type`). > - This PR lets the CLI send `X-Paperclip-Run-Id` from a new global `--run-id` flag (falling back to `$PAPERCLIP_RUN_ID`), and fixes `attachment:upload` to forward the run-id and emit server-allowed bare MIME types. > - The benefit is that an embodied agent can drive the full issue lifecycle (checkout → work → disposition → upload deliverable) entirely through the official CLI, instead of dropping to raw HTTP. ## Linked Issues or Issue Description No issue exactly covers the CLI **send** side, so describing it here (bug path). Related: - `Refs #2063` — "Sub-agents cannot post comments on subtickets — Agent run id required" (same error string; that report focuses on the server gate, this PR fixes the CLI not sending the header for agent mutations). - `Refs #1199` — injects `X-Paperclip-Run-Id` on the **http adapter's** outbound request (server side). This PR is the complementary **CLI client** side. **Bug (per `bug_report.yml`):** - **What happened:** Running agent-authenticated CLI mutations (`issue checkout` / `issue update` on an in-progress issue / `issue attachment:upload`) returns `401 Agent run id required`, even with `--run-id`/`$PAPERCLIP_RUN_ID` set; `attachment:upload` of an HTML/markdown deliverable additionally returns `422 Unsupported attachment content type`. - **Expected:** The CLI forwards the agent run-id so the server authorizes the mutation, and uploads use a content-type the server accepts. - **Steps to reproduce:** As an agent token, `paperclipai issue checkout --agent-id ` then `paperclipai issue update --status done` (→ 401); `paperclipai issue attachment:upload ./report.html` (→ 401, then 422 once run-id is wired). - **Deployment mode:** local_trusted (applies to all modes — server-side gate is mode-independent). ## What Changed - `cli/src/commands/client/common.ts`: resolve `runId` in `resolveCommandContext` from a new global `--run-id` flag, falling back to `$PAPERCLIP_RUN_ID`, so the existing HTTP client sends `X-Paperclip-Run-Id`; thread `runId` into the attachment-upload path; align `inferContentTypeFromPath` with the server's `DEFAULT_ALLOWED_TYPES` (add `html`/`htm`/`csv`/`zip`/`mp4`/`m4v`/`webm`/`mov`/`qt`, drop the `; charset` suffix). - `cli/src/commands/client/issue.ts`: pass `ctx.api.runId` into `uploadAttachment` and send the `X-Paperclip-Run-Id` header on the hand-rolled multipart request (matching what the JSON client injects automatically). - Tests: CLI asserts `attachment:upload` forwards `x-paperclip-run-id` + the inferred bare MIME type, and that `inferContentTypeFromPath` covers the allowed types; a server test locks the contract that an in-progress checkout owner without a run-id is rejected `401` on attachment upload. ## Verification ```bash # CLI tests (no DB) node_modules/.bin/vitest run \ cli/src/__tests__/common.test.ts \ cli/src/__tests__/issue-subresources.test.ts # → 2 files, 13 tests passed # Server contract test (embedded postgres) node_modules/.bin/vitest run \ server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts # → 37 tests passed ``` Manual: with an agent token and a valid `$PAPERCLIP_RUN_ID`, `issue checkout` / `issue update --status done` / `issue attachment:upload ./report.html` now succeed where they previously returned 401/422. ## Risks Low. Additive only: - `--run-id` is a new optional flag; behavior is unchanged when it (and `$PAPERCLIP_RUN_ID`) are unset — the header is simply omitted as before. - The content-type map only **widens** the allowed set to match the server's existing allowlist and removes a suffix the server already rejected, so no previously-accepted upload changes type. - No schema/migration changes; no server behavior changes (the server test only documents the existing gate). ## Model Used Claude Opus 4.8 (1M context window), via Claude Code (tool use / agentic file edits + local test execution). Extended reasoning enabled. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots — N/A (CLI-only) - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green — pending CI run on this PR - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups — pending review - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Claude Opus 4.8 (1M context) --- cli/src/__tests__/common.test.ts | 29 +++++++++++++++- cli/src/__tests__/issue-subresources.test.ts | 34 +++++++++++++++++++ cli/src/commands/client/common.ts | 27 +++++++++++++-- cli/src/commands/client/issue.ts | 12 +++++-- ...ue-agent-mutation-ownership-routes.test.ts | 21 ++++++++++++ 5 files changed, 118 insertions(+), 5 deletions(-) diff --git a/cli/src/__tests__/common.test.ts b/cli/src/__tests__/common.test.ts index 9562e47f..f436bee0 100644 --- a/cli/src/__tests__/common.test.ts +++ b/cli/src/__tests__/common.test.ts @@ -4,7 +4,7 @@ import path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { writeContext } from "../client/context.js"; import { setStoredBoardCredential } from "../client/board-auth.js"; -import { resolveApiBase, resolveCommandContext } from "../commands/client/common.js"; +import { inferContentTypeFromPath, resolveApiBase, resolveCommandContext } from "../commands/client/common.js"; const ORIGINAL_ENV = { ...process.env }; @@ -158,3 +158,30 @@ describe("resolveCommandContext", () => { expect(explicitResolved.authSource).toBe("explicit"); }); }); + +describe("inferContentTypeFromPath", () => { + it("maps the issue-attachment file types the server allows", () => { + // Must match server/src/attachment-types.ts DEFAULT_ALLOWED_TYPES exactly. + expect(inferContentTypeFromPath("newsletter.html")).toBe("text/html"); + expect(inferContentTypeFromPath("page.htm")).toBe("text/html"); + expect(inferContentTypeFromPath("data.csv")).toBe("text/csv"); + expect(inferContentTypeFromPath("bundle.zip")).toBe("application/zip"); + expect(inferContentTypeFromPath("demo.mp4")).toBe("video/mp4"); + expect(inferContentTypeFromPath("clip.webm")).toBe("video/webm"); + expect(inferContentTypeFromPath("teaser.m4v")).toBe("video/x-m4v"); + expect(inferContentTypeFromPath("walkthrough.mov")).toBe("video/quicktime"); + expect(inferContentTypeFromPath("report.pdf")).toBe("application/pdf"); + expect(inferContentTypeFromPath("chart.png")).toBe("image/png"); + }); + + it("emits text types with no charset parameter so they match the exact allowlist", () => { + expect(inferContentTypeFromPath("notes.md")).toBe("text/markdown"); + expect(inferContentTypeFromPath("log.txt")).toBe("text/plain"); + }); + + it("is case-insensitive and returns undefined for unknown extensions", () => { + expect(inferContentTypeFromPath("/abs/Path/IMAGE.PNG")).toBe("image/png"); + expect(inferContentTypeFromPath("archive.unknownext")).toBeUndefined(); + expect(inferContentTypeFromPath("noextension")).toBeUndefined(); + }); +}); diff --git a/cli/src/__tests__/issue-subresources.test.ts b/cli/src/__tests__/issue-subresources.test.ts index fafd5de6..ce1c60b9 100644 --- a/cli/src/__tests__/issue-subresources.test.ts +++ b/cli/src/__tests__/issue-subresources.test.ts @@ -221,6 +221,40 @@ describe("issue subresource commands", () => { selectedOptionIds: ["file-a", "file-b"], }); }); + + it("forwards the agent run-id header and inferred content-type on attachment:upload", async () => { + // Regression: the multipart upload uses a hand-rolled fetch (not the JSON + // client), so it must forward X-Paperclip-Run-Id itself — otherwise an + // agent-authenticated upload is rejected with "401 Agent run id required". + const RUN_ID = "cccccccc-cccc-4ccc-8ccc-cccccccccccc"; + const tmp = await mkdtemp(join(tmpdir(), "paperclip-cli-test-")); + const filePath = join(tmp, "deliverable.html"); + await writeFile(filePath, "hi", "utf8"); + const fetchMock = vi.fn().mockImplementation(() => Promise.resolve(jsonResponse())); + vi.stubGlobal("fetch", fetchMock); + vi.spyOn(process.stdout, "write").mockImplementation(() => true); + + try { + await run([ + "issue", "attachment:upload", ISSUE_ID, + "--company-id", COMPANY_ID, + "--file", filePath, + "--run-id", RUN_ID, + ]); + } finally { + await rm(tmp, { recursive: true, force: true }); + } + + expect(fetchMock).toHaveBeenCalledTimes(1); + const [url, init] = fetchMock.mock.calls[0] as [string, RequestInit]; + expect(url).toBe(`http://localhost:3100/api/companies/${COMPANY_ID}/issues/${ISSUE_ID}/attachments`); + expect(init.method).toBe("POST"); + const headers = init.headers as Record; + expect(headers["x-paperclip-run-id"]).toBe(RUN_ID); + expect(headers.authorization).toBe("Bearer board-token"); + const file = (init.body as FormData).get("file") as File; + expect(file.type).toBe("text/html"); + }); }); function jsonResponse(body: unknown = { ok: true }, init: ResponseInit = { status: 200 }): Response { diff --git a/cli/src/commands/client/common.ts b/cli/src/commands/client/common.ts index 6a48b4ab..ad5095e6 100644 --- a/cli/src/commands/client/common.ts +++ b/cli/src/commands/client/common.ts @@ -13,6 +13,7 @@ export interface BaseClientOptions { profile?: string; apiBase?: string; apiKey?: string; + runId?: string; companyId?: string; json?: boolean; } @@ -34,6 +35,7 @@ export function addCommonClientOptions(command: Command, opts?: { includeCompany .option("--profile ", "CLI context profile name") .option("--api-base ", "Base URL for the Paperclip API") .option("--api-key ", "Bearer token for agent-authenticated calls") + .option("--run-id ", "Heartbeat run id for agent-authenticated mutations (checkout/release/interactions/in-progress update); falls back to $PAPERCLIP_RUN_ID") .option("--json", "Output raw JSON"); if (opts?.includeCompany) { @@ -68,9 +70,16 @@ export function resolveCommandContext( ); } + // Agent-authenticated mutations (checkout, release, interactions, PATCH of an + // in-progress issue) require the X-Paperclip-Run-Id header (the server returns + // "401 Agent run id required" without it). Source it from --run-id, else the + // PAPERCLIP_RUN_ID env the adapter/embodiment context already exports. + const runId = options.runId?.trim() || process.env.PAPERCLIP_RUN_ID?.trim() || undefined; + const api = new PaperclipApiClient({ apiBase, apiKey, + runId, recoverAuth: explicitApiKey || !canAttemptInteractiveBoardAuth() ? undefined : async ({ error }) => { @@ -126,18 +135,32 @@ export function apiPath(strings: TemplateStringsArray, ...values: Array { const bytes = await readFile(input.filePath); const form = new FormData(); form.set("file", new Blob([bytes], { type: inferContentTypeFromPath(input.filePath) }), input.filePath.split(/[\\/]/).pop() ?? "attachment"); if (input.commentId) form.set("issueCommentId", input.commentId); + // This multipart upload uses a hand-rolled fetch rather than PaperclipApiClient, + // so it must forward the agent run-id header itself — otherwise an + // agent-authenticated upload is rejected with "401 Agent run id required" + // (the client injects x-paperclip-run-id automatically for JSON requests). + const headers: Record = {}; + if (apiKey) headers.authorization = `Bearer ${apiKey}`; + if (input.runId) headers["x-paperclip-run-id"] = input.runId; const response = await fetch(buildApiUrl(apiBase, apiPath`/api/companies/${input.companyId}/issues/${input.issueId}/attachments`), { method: "POST", - headers: apiKey ? { authorization: `Bearer ${apiKey}` } : undefined, + headers, body: form, }); return parseFetchResponse(response); diff --git a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts index 1fed2d47..31bf37ff 100644 --- a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts +++ b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts @@ -608,6 +608,27 @@ describe("agent issue mutation checkout ownership", () => { expect(mockStorageService.deleteObject).not.toHaveBeenCalled(); }); + it("rejects the checked-out owner without a run id on attachment upload (401)", async () => { + // Regression: an agent-authenticated client (e.g. the CLI's attachment:upload) + // that fails to send X-Paperclip-Run-Id must be rejected — mutating your own + // in-progress checkout requires proving run ownership. + const app = await createApp({ + type: "agent", + agentId: ownerAgentId, + companyId, + source: "agent_key", + // intentionally no runId + }); + + const res = await request(app) + .post(`/api/companies/${companyId}/issues/${issueId}/attachments`) + .attach("file", Buffer.from("report"), { filename: "report.html", contentType: "text/html" }); + + expect(res.status, JSON.stringify(res.body)).toBe(401); + expect(res.body.error).toBe("Agent run id required"); + expect(mockStorageService.putFile).not.toHaveBeenCalled(); + }); + it("allows the checked-out owner with the matching run id to patch and update documents", async () => { const app = await createApp(ownerActor());