fix(cli): send X-Paperclip-Run-Id so agents can mutate their issues via the CLI (#7642)
## 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 <id> --agent-id <id>` then `paperclipai issue update <id> --status done` (→ 401); `paperclipai issue attachment:upload <id> ./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) <noreply@anthropic.com>
This commit is contained in:
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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, "<html><body>hi</body></html>", "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<string, string>;
|
||||
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 {
|
||||
|
||||
@@ -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 <name>", "CLI context profile name")
|
||||
.option("--api-base <url>", "Base URL for the Paperclip API")
|
||||
.option("--api-key <token>", "Bearer token for agent-authenticated calls")
|
||||
.option("--run-id <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<string |
|
||||
export function inferContentTypeFromPath(filePath: string): string | undefined {
|
||||
const ext = filePath.split(/[\\/]/).pop()?.split(".").pop()?.toLowerCase();
|
||||
if (!ext) return undefined;
|
||||
// These MIME strings are matched against the server's issue-attachment
|
||||
// allowlist (server/src/attachment-types.ts DEFAULT_ALLOWED_TYPES) by EXACT
|
||||
// string, so text types must carry no "; charset=..." parameter or the upload
|
||||
// is rejected with "422 Unsupported attachment content type". Keep this set in
|
||||
// sync with that allowlist (plus svg/avif, accepted by the asset routes).
|
||||
return {
|
||||
avif: "image/avif",
|
||||
csv: "text/csv",
|
||||
gif: "image/gif",
|
||||
htm: "text/html",
|
||||
html: "text/html",
|
||||
jpeg: "image/jpeg",
|
||||
jpg: "image/jpeg",
|
||||
json: "application/json",
|
||||
md: "text/markdown; charset=utf-8",
|
||||
m4v: "video/x-m4v",
|
||||
md: "text/markdown",
|
||||
mov: "video/quicktime",
|
||||
mp4: "video/mp4",
|
||||
pdf: "application/pdf",
|
||||
png: "image/png",
|
||||
qt: "video/quicktime",
|
||||
svg: "image/svg+xml",
|
||||
txt: "text/plain; charset=utf-8",
|
||||
txt: "text/plain",
|
||||
webm: "video/webm",
|
||||
webp: "image/webp",
|
||||
zip: "application/zip",
|
||||
}[ext];
|
||||
}
|
||||
|
||||
|
||||
@@ -953,6 +953,7 @@ export function registerIssueCommands(program: Command): void {
|
||||
issueId,
|
||||
filePath: opts.file,
|
||||
commentId: opts.commentId,
|
||||
runId: ctx.api.runId,
|
||||
});
|
||||
printOutput(attachment, { json: ctx.json });
|
||||
} catch (err) {
|
||||
@@ -1392,15 +1393,22 @@ function buildApiUrl(apiBase: string, path: string): string {
|
||||
async function uploadAttachment(
|
||||
apiBase: string,
|
||||
apiKey: string | undefined,
|
||||
input: { companyId: string; issueId: string; filePath: string; commentId?: string },
|
||||
input: { companyId: string; issueId: string; filePath: string; commentId?: string; runId?: string },
|
||||
): Promise<unknown> {
|
||||
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<string, string> = {};
|
||||
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);
|
||||
|
||||
Reference in New Issue
Block a user