mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-26 18:31:34 +01:00
Aron PrinsandClaude Opus 4.8
8b85fdfa3c
## 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>
263 lines
13 KiB
TypeScript
263 lines
13 KiB
TypeScript
import { Command } from "commander";
|
|
import { mkdtemp, rm, writeFile } from "node:fs/promises";
|
|
import { join } from "node:path";
|
|
import { tmpdir } from "node:os";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { registerIssueCommands } from "../commands/client/issue.js";
|
|
|
|
const COMPANY_ID = "22222222-2222-4222-8222-222222222222";
|
|
const ISSUE_ID = "44444444-4444-4444-8444-444444444444";
|
|
const COMMENT_ID = "55555555-5555-4555-8555-555555555555";
|
|
const APPROVAL_ID = "66666666-6666-4666-8666-666666666666";
|
|
const PRODUCT_ID = "77777777-7777-4777-8777-777777777777";
|
|
const INTERACTION_ID = "88888888-8888-4888-8888-888888888888";
|
|
const HOLD_ID = "99999999-9999-4999-8999-999999999999";
|
|
const ATTACHMENT_ID = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa";
|
|
const LABEL_ID = "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb";
|
|
|
|
function createProgram(): Command {
|
|
const program = new Command();
|
|
program.exitOverride();
|
|
program.configureOutput({
|
|
writeOut: () => {},
|
|
writeErr: () => {},
|
|
});
|
|
registerIssueCommands(program);
|
|
return program;
|
|
}
|
|
|
|
async function run(args: string[]): Promise<void> {
|
|
await createProgram().parseAsync([
|
|
...args,
|
|
"--api-base", "http://localhost:3100",
|
|
"--api-key", "board-token",
|
|
], { from: "user" });
|
|
}
|
|
|
|
describe("issue subresource commands", () => {
|
|
beforeEach(() => {
|
|
vi.restoreAllMocks();
|
|
delete process.env.PAPERCLIP_API_KEY;
|
|
delete process.env.PAPERCLIP_API_URL;
|
|
vi.spyOn(console, "log").mockImplementation(() => {});
|
|
});
|
|
|
|
afterEach(() => {
|
|
vi.restoreAllMocks();
|
|
});
|
|
|
|
it("wraps core issue get, update, and delete endpoints", async () => {
|
|
const fetchMock = vi.fn().mockImplementation(() => Promise.resolve(jsonResponse()));
|
|
vi.stubGlobal("fetch", fetchMock);
|
|
|
|
await run(["issue", "get", ISSUE_ID]);
|
|
await run(["issue", "update", ISSUE_ID, "--title", "New title"]);
|
|
await run(["issue", "delete", ISSUE_ID, "--yes"]);
|
|
|
|
expect(fetchMock.mock.calls.map((call) => [call[1]?.method ?? "GET", call[0]])).toEqual([
|
|
["GET", `http://localhost:3100/api/issues/${ISSUE_ID}`],
|
|
["PATCH", `http://localhost:3100/api/issues/${ISSUE_ID}`],
|
|
["DELETE", `http://localhost:3100/api/issues/${ISSUE_ID}`],
|
|
]);
|
|
});
|
|
|
|
it("wraps comments, approvals, markers, and recovery action endpoints", async () => {
|
|
const fetchMock = vi
|
|
.fn()
|
|
.mockImplementation(() => Promise.resolve(jsonResponse()));
|
|
vi.stubGlobal("fetch", fetchMock);
|
|
|
|
await run(["issue", "comments", ISSUE_ID, "--limit", "10"]);
|
|
await run(["issue", "comment:get", ISSUE_ID, COMMENT_ID]);
|
|
await run(["issue", "comment:delete", ISSUE_ID, COMMENT_ID]);
|
|
await run(["issue", "approvals", ISSUE_ID]);
|
|
await run(["issue", "approval:link", ISSUE_ID, APPROVAL_ID]);
|
|
await run(["issue", "approval:unlink", ISSUE_ID, APPROVAL_ID]);
|
|
await run(["issue", "read", ISSUE_ID]);
|
|
await run(["issue", "unread", ISSUE_ID]);
|
|
await run(["issue", "archive", ISSUE_ID]);
|
|
await run(["issue", "unarchive", ISSUE_ID]);
|
|
await run(["issue", "recovery-actions", ISSUE_ID]);
|
|
await run([
|
|
"issue", "recovery:resolve", ISSUE_ID,
|
|
"--outcome", "restored",
|
|
"--source-issue-status", "todo",
|
|
"--action-id", APPROVAL_ID,
|
|
]);
|
|
|
|
expect(fetchMock.mock.calls.map((call) => [call[1]?.method ?? "GET", call[0]])).toEqual([
|
|
["GET", `http://localhost:3100/api/issues/${ISSUE_ID}/comments?limit=10`],
|
|
["GET", `http://localhost:3100/api/issues/${ISSUE_ID}/comments/${COMMENT_ID}`],
|
|
["DELETE", `http://localhost:3100/api/issues/${ISSUE_ID}/comments/${COMMENT_ID}`],
|
|
["GET", `http://localhost:3100/api/issues/${ISSUE_ID}/approvals`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/approvals`],
|
|
["DELETE", `http://localhost:3100/api/issues/${ISSUE_ID}/approvals/${APPROVAL_ID}`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/read`],
|
|
["DELETE", `http://localhost:3100/api/issues/${ISSUE_ID}/read`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/inbox-archive`],
|
|
["DELETE", `http://localhost:3100/api/issues/${ISSUE_ID}/inbox-archive`],
|
|
["GET", `http://localhost:3100/api/issues/${ISSUE_ID}/recovery-actions`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/recovery-actions/resolve`],
|
|
]);
|
|
});
|
|
|
|
it("wraps document and work product endpoints", async () => {
|
|
const fetchMock = vi
|
|
.fn()
|
|
.mockImplementation(() => Promise.resolve(jsonResponse()));
|
|
vi.stubGlobal("fetch", fetchMock);
|
|
|
|
await run(["issue", "documents", ISSUE_ID, "--include-system"]);
|
|
await run(["issue", "document:get", ISSUE_ID, "plan"]);
|
|
await run(["issue", "document:put", ISSUE_ID, "plan", "--body", "# Plan", "--title", "Plan"]);
|
|
await run(["issue", "document:lock", ISSUE_ID, "plan"]);
|
|
await run(["issue", "document:unlock", ISSUE_ID, "plan"]);
|
|
await run(["issue", "document:revisions", ISSUE_ID, "plan"]);
|
|
await run(["issue", "document:restore", ISSUE_ID, "plan", APPROVAL_ID]);
|
|
await run(["issue", "document:delete", ISSUE_ID, "plan"]);
|
|
await run(["issue", "work-products", ISSUE_ID]);
|
|
await run([
|
|
"issue", "work-product:create", ISSUE_ID,
|
|
"--payload-json", JSON.stringify({ type: "pull_request", provider: "github", title: "PR", url: "https://example.com/pr/1" }),
|
|
]);
|
|
await run([
|
|
"issue", "work-product:update", PRODUCT_ID,
|
|
"--payload-json", JSON.stringify({ title: "Updated PR" }),
|
|
]);
|
|
await run(["issue", "work-product:delete", PRODUCT_ID]);
|
|
|
|
expect(fetchMock.mock.calls.map((call) => [call[1]?.method ?? "GET", call[0]])).toEqual([
|
|
["GET", `http://localhost:3100/api/issues/${ISSUE_ID}/documents?includeSystem=true`],
|
|
["GET", `http://localhost:3100/api/issues/${ISSUE_ID}/documents/plan`],
|
|
["PUT", `http://localhost:3100/api/issues/${ISSUE_ID}/documents/plan`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/documents/plan/lock`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/documents/plan/unlock`],
|
|
["GET", `http://localhost:3100/api/issues/${ISSUE_ID}/documents/plan/revisions`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/documents/plan/revisions/${APPROVAL_ID}/restore`],
|
|
["DELETE", `http://localhost:3100/api/issues/${ISSUE_ID}/documents/plan`],
|
|
["GET", `http://localhost:3100/api/issues/${ISSUE_ID}/work-products`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/work-products`],
|
|
["PATCH", `http://localhost:3100/api/work-products/${PRODUCT_ID}`],
|
|
["DELETE", `http://localhost:3100/api/work-products/${PRODUCT_ID}`],
|
|
]);
|
|
});
|
|
|
|
it("wraps interactions, tree holds, labels, feedback votes, and attachments", async () => {
|
|
const tmp = await mkdtemp(join(tmpdir(), "paperclip-cli-test-"));
|
|
const filePath = join(tmp, "attachment.txt");
|
|
await writeFile(filePath, "hello", "utf8");
|
|
const fetchMock = vi
|
|
.fn()
|
|
.mockImplementation(() => Promise.resolve(jsonResponse()));
|
|
vi.stubGlobal("fetch", fetchMock);
|
|
vi.spyOn(process.stdout, "write").mockImplementation(() => true);
|
|
|
|
try {
|
|
await run(["issue", "interactions", ISSUE_ID]);
|
|
await run([
|
|
"issue", "interaction:create", ISSUE_ID,
|
|
"--payload-json", JSON.stringify({
|
|
kind: "request_confirmation",
|
|
payload: { version: 1, prompt: "Continue?" },
|
|
}),
|
|
]);
|
|
await run(["issue", "interaction:accept", ISSUE_ID, INTERACTION_ID]);
|
|
await run(["issue", "interaction:accept", ISSUE_ID, INTERACTION_ID, "--selected-client-keys", "yes"]);
|
|
await run(["issue", "interaction:accept", ISSUE_ID, INTERACTION_ID, "--selected-option-ids", "file-a,file-b"]);
|
|
await run(["issue", "interaction:reject", ISSUE_ID, INTERACTION_ID, "--reason", "no"]);
|
|
await run(["issue", "interaction:cancel", ISSUE_ID, INTERACTION_ID, "--reason", "stale"]);
|
|
await run([
|
|
"issue", "interaction:respond", ISSUE_ID, INTERACTION_ID,
|
|
"--answers-json", JSON.stringify([{ questionId: "q1", optionIds: ["a1"] }]),
|
|
]);
|
|
await run(["issue", "tree-state", ISSUE_ID]);
|
|
await run(["issue", "tree-preview", ISSUE_ID, "--payload-json", JSON.stringify({ mode: "pause" })]);
|
|
await run(["issue", "tree-holds", ISSUE_ID, "--status", "active", "--include-members"]);
|
|
await run(["issue", "tree-hold:create", ISSUE_ID, "--payload-json", JSON.stringify({ mode: "pause", reason: "test" })]);
|
|
await run(["issue", "tree-hold:get", ISSUE_ID, HOLD_ID]);
|
|
await run(["issue", "tree-hold:release", ISSUE_ID, HOLD_ID]);
|
|
await run(["issue", "attachments", ISSUE_ID]);
|
|
await run(["issue", "attachment:upload", ISSUE_ID, "--company-id", COMPANY_ID, "--file", filePath]);
|
|
await run(["issue", "attachment:download", ATTACHMENT_ID]);
|
|
await run(["issue", "attachment:delete", ATTACHMENT_ID]);
|
|
await run(["issue", "label:list", "--company-id", COMPANY_ID]);
|
|
await run(["issue", "label:create", "--company-id", COMPANY_ID, "--name", "bug", "--color", "#ff0000"]);
|
|
await run(["issue", "label:delete", LABEL_ID]);
|
|
await run(["issue", "feedback:votes", ISSUE_ID]);
|
|
await run([
|
|
"issue", "feedback:vote", ISSUE_ID,
|
|
"--payload-json", JSON.stringify({ targetType: "issue_comment", targetId: COMMENT_ID, vote: "up" }),
|
|
]);
|
|
} finally {
|
|
await rm(tmp, { recursive: true, force: true });
|
|
}
|
|
|
|
expect(fetchMock.mock.calls.map((call) => [call[1]?.method ?? "GET", call[0]])).toEqual([
|
|
["GET", `http://localhost:3100/api/issues/${ISSUE_ID}/interactions`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/interactions`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/interactions/${INTERACTION_ID}/accept`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/interactions/${INTERACTION_ID}/accept`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/interactions/${INTERACTION_ID}/accept`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/interactions/${INTERACTION_ID}/reject`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/interactions/${INTERACTION_ID}/cancel`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/interactions/${INTERACTION_ID}/respond`],
|
|
["GET", `http://localhost:3100/api/issues/${ISSUE_ID}/tree-control/state`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/tree-control/preview`],
|
|
["GET", `http://localhost:3100/api/issues/${ISSUE_ID}/tree-holds?status=active&includeMembers=true`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/tree-holds`],
|
|
["GET", `http://localhost:3100/api/issues/${ISSUE_ID}/tree-holds/${HOLD_ID}`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/tree-holds/${HOLD_ID}/release`],
|
|
["GET", `http://localhost:3100/api/issues/${ISSUE_ID}/attachments`],
|
|
["POST", `http://localhost:3100/api/companies/${COMPANY_ID}/issues/${ISSUE_ID}/attachments`],
|
|
["GET", `http://localhost:3100/api/attachments/${ATTACHMENT_ID}/content`],
|
|
["DELETE", `http://localhost:3100/api/attachments/${ATTACHMENT_ID}`],
|
|
["GET", `http://localhost:3100/api/companies/${COMPANY_ID}/labels`],
|
|
["POST", `http://localhost:3100/api/companies/${COMPANY_ID}/labels`],
|
|
["DELETE", `http://localhost:3100/api/labels/${LABEL_ID}`],
|
|
["GET", `http://localhost:3100/api/issues/${ISSUE_ID}/feedback-votes`],
|
|
["POST", `http://localhost:3100/api/issues/${ISSUE_ID}/feedback-votes`],
|
|
]);
|
|
expect(JSON.parse(String(fetchMock.mock.calls[4]?.[1]?.body))).toEqual({
|
|
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 {
|
|
return new Response(JSON.stringify(body), init);
|
|
}
|