mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-09 16:35:27 +02:00
## Thinking Path > - Paperclip lets operators assign tasks to AI agents and review their work. > - The task composer controls the next message and its assigned agent. > - Operators needed a way to choose that agent's model and effort without leaving the composer. > - The old mode selector, upload button, and input cards made the mobile composer crowded and hid normal messaging during a pending decision. > - Harnesses publish different model and effort capabilities, so the picker must follow the selected agent. > - This pull request adds one responsive composer flow, keeps pending cards visible above it, and protects Codex ACP authentication in the local test path. > - Operators can choose run settings, send a message, and answer a pending card as separate actions. ## Linked Issues or Issue Description **Subsystem affected** Task composer UI, issue thread interactions, Codex ACP credential handling, and Storybook. **Problem or motivation** The composer did not expose model or effort for the selected agent. Mobile actions wrapped poorly. Pending questions and confirmations replaced the composer. A local Codex ACP test could also reuse host authentication after the managed key was removed. **Proposed solution** Put assignee search, model search, exact model IDs, effort, and fast mode in one picker. Use a mobile dialog. Replace the direct-upload plus action and separate mode selector with an Add menu and removable Plan or Ask chips. Place pending interaction cards above the usable composer. Keep these cards pending after an ordinary message unless their creator asks for comment superseding. Replace managed ACP auth files atomically and isolate the test key from host credentials. **Roadmap alignment** ROADMAP.md does not list an overlapping composer milestone. This change improves the existing task and review flows. ## What Changed - Added the combined assignee, model, and effort picker to both task composers. Search matches agent name, role, and harness. The server uses a curated Codex list by default and honors instance-declared models. Manual IDs remain available. - Added an effort slider for known model capabilities, a conditional Codex fast control, and reset. The picker opens in a modal on mobile. - Added the Add menu for files, supported goals, Plan mode, and Ask mode. Plan and Ask are exclusive removable chips. Keyboard mode cycling remains available. - Adjusted mobile spacing, avatars, wrapping, and Send placement. Removed the composer divider. - Moved pending question, confirmation, review, and related cards above the composer. Ordinary comments now leave question and confirmation cards pending by default. The onboarding prompt retains explicit comment superseding. - Updated the Storybook composer group with responsive states and the production picker. Added UI, service, route, and browser regression coverage. - Isolated Codex ACP API-key authentication, skipped subscription auth merge and shared-home copy-back for remote API-key runs, and replaced the managed auth file atomically. ## Verification - `pnpm -r typecheck` — passed on the final local head. - `pnpm check:token-gates` — passed on the final local head. - `pnpm exec vitest run server/src/__tests__/adapter-models.test.ts ui/src/components/task-chat/ComposerRunSettingsPicker.test.tsx` — 31 tests passed, including role and harness search, declared Codex models, and filtering general OpenAI models. - `pnpm exec vitest run server/src/__tests__/issue-thread-interactions-service.test.ts` — 74 tests passed. - `pnpm exec vitest run packages/adapters/codex-local/src/server/acp.test.ts` — 42 tests passed, including remote API-key copy-back isolation. - `pnpm test:run` — attempted locally; the embedded PostgreSQL test database could not initialize on macOS. The isolated `heartbeat-run-event-sequencing` suite reproduced that environment failure. GitHub CI runs the full test matrix for this head. - `pnpm build` — passed on the final head. `pnpm build-storybook` passed after the last UI change; only server code, tests, and docs changed afterward. - Live local test drive — Codex ACP ran a task with a managed API key. The test agent was restored to its default ACP configuration afterward. - Review the interactive stories under the top-level Composer group with `pnpm storybook`. Check a narrow desktop width and mobile Plan, Ask, picker, and pending-question states. ## Risks - A pending card stays open when an ordinary comment changes the discussion. Its creator can set `supersedeOnUserComment: true` when a new comment should replace it. - Model and effort overrides persist on the task until reset or changed. An unlisted manual model ID may fail when the provider runs it. - Some harness catalogs do not report effort support. The picker hides effort for those models. - No database migration is required. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI GPT-6 via Codex. This runtime does not expose the exact model ID or context window to the task. The model used code execution and browser tools. ## 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 - [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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: OpenAI Codex <codex@openai.com>
453 lines
16 KiB
TypeScript
453 lines
16 KiB
TypeScript
import { randomUUID } from "node:crypto";
|
|
import {
|
|
expect,
|
|
test,
|
|
type APIRequestContext,
|
|
type Locator,
|
|
type Page,
|
|
} from "@playwright/test";
|
|
|
|
// Real local Board UI, upload storage, comment HTTP routes, and disposable DB.
|
|
// Only the interface flag and explicit transport/upload rejection faults are mocked.
|
|
// Tasks belong to the Board user: no agent, runner, or provider is contacted.
|
|
type Attachment = {
|
|
id: string;
|
|
issueCommentId: string | null;
|
|
contentPath: string;
|
|
};
|
|
type Comment = { id: string; body: string; clientRequestId?: string | null };
|
|
|
|
async function body<T>(
|
|
response: Awaited<ReturnType<APIRequestContext["get"]>>,
|
|
): Promise<T> {
|
|
expect(response.ok(), `${response.status()}: ${await response.text()}`).toBe(
|
|
true,
|
|
);
|
|
return response.json() as Promise<T>;
|
|
}
|
|
|
|
async function setup(page: Page, request: APIRequestContext, classic: boolean) {
|
|
const company = await body<{ id: string; issuePrefix: string }>(
|
|
await request.post("/api/companies", {
|
|
data: { name: `Board receipt browser ${randomUUID()}` },
|
|
}),
|
|
);
|
|
const issue = await body<{ id: string; identifier: string }>(
|
|
await request.post(`/api/companies/${company.id}/issues`, {
|
|
data: {
|
|
title: "Inspect the newly uploaded Board files",
|
|
status: "todo",
|
|
assigneeUserId: "local-board",
|
|
},
|
|
}),
|
|
);
|
|
await page.route("**/api/instance/settings/experimental", (route) =>
|
|
route.fulfill({
|
|
contentType: "application/json",
|
|
body: JSON.stringify({ enableClassicTaskInterface: classic }),
|
|
}),
|
|
);
|
|
await page.goto(`/${company.issuePrefix}/issues/${issue.identifier}`);
|
|
await expect(
|
|
page.getByRole("heading", {
|
|
name: "Inspect the newly uploaded Board files",
|
|
exact: true,
|
|
}),
|
|
).toBeVisible();
|
|
const composer = classic
|
|
? page.getByTestId("issue-chat-composer")
|
|
: page.locator(".paperclip-task-chat-composer");
|
|
const editor = composer.getByRole("textbox", {
|
|
name: "editable markdown",
|
|
exact: true,
|
|
});
|
|
const send = classic
|
|
? composer.getByRole("button", { name: /Send|Reply/ }).last()
|
|
: page.getByTestId("task-chat-composer-send");
|
|
await expect(editor).toBeVisible();
|
|
return {
|
|
company,
|
|
issue,
|
|
composer,
|
|
editor,
|
|
send,
|
|
attachments: () =>
|
|
request
|
|
.get(`/api/issues/${issue.id}/attachments`)
|
|
.then(body<Attachment[]>),
|
|
comments: () =>
|
|
request.get(`/api/issues/${issue.id}/comments`).then(body<Comment[]>),
|
|
};
|
|
}
|
|
|
|
const files = [
|
|
{
|
|
name: "board-fresh.txt",
|
|
mimeType: "text/plain",
|
|
buffer: Buffer.from("Object: cat\nAccent: teal\nCount: 3\n"),
|
|
},
|
|
{
|
|
name: "board-fresh.png",
|
|
mimeType: "image/png",
|
|
buffer: Buffer.from(
|
|
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+jfFoAAAAASUVORK5CYII=",
|
|
"base64",
|
|
),
|
|
},
|
|
];
|
|
|
|
async function openAttachmentChooser(page: Page, composer: Locator) {
|
|
await composer.getByRole("button", { name: "Add to composer" }).click();
|
|
const chooser = page.waitForEvent("filechooser");
|
|
await page.getByRole("menuitem", { name: "Files and images", exact: true }).click();
|
|
return chooser;
|
|
}
|
|
|
|
async function upload(
|
|
page: Page,
|
|
fixture: Awaited<ReturnType<typeof setup>>,
|
|
file: (typeof files)[number],
|
|
) {
|
|
const chooser = await openAttachmentChooser(page, fixture.composer);
|
|
const response = page.waitForResponse(
|
|
(res) =>
|
|
res.request().method() === "POST" &&
|
|
new URL(res.url()).pathname.endsWith(
|
|
`/issues/${fixture.issue.id}/attachments`,
|
|
),
|
|
);
|
|
await chooser.setFiles(file);
|
|
const receipt = await body<Attachment>(await response);
|
|
await expect
|
|
.poll(async () =>
|
|
(await fixture.attachments()).some((item) => item.id === receipt.id),
|
|
)
|
|
.toBe(true);
|
|
return receipt;
|
|
}
|
|
|
|
for (const classic of [false, true]) {
|
|
test(`uploaded file and image receipts survive reload and bind exactly once (classic=${classic})`, async ({
|
|
page,
|
|
request,
|
|
}, testInfo) => {
|
|
const fixture = await setup(page, request, classic);
|
|
await fixture.editor.fill(
|
|
"Inspect only these fresh Board attachments. Keep this internal.",
|
|
);
|
|
const receipts = [];
|
|
for (const file of files) receipts.push(await upload(page, fixture, file));
|
|
const draftKey = `paperclip:issue-comment-draft:${fixture.issue.id}`;
|
|
await expect
|
|
.poll(() =>
|
|
page.evaluate(
|
|
(key) =>
|
|
JSON.parse(localStorage.getItem(`${key}:attachments:v1`) ?? "null")
|
|
?.attachments?.length,
|
|
draftKey,
|
|
),
|
|
)
|
|
.toBe(2);
|
|
await expect
|
|
.poll(() => page.evaluate((key) => localStorage.getItem(key), draftKey))
|
|
.toContain("Inspect only these fresh Board attachments.");
|
|
await testInfo.attach("before-reload-draft", {
|
|
body: JSON.stringify(
|
|
await page.evaluate(
|
|
(key) => ({
|
|
text: localStorage.getItem(key),
|
|
receipts: localStorage.getItem(`${key}:attachments:v1`),
|
|
}),
|
|
draftKey,
|
|
),
|
|
),
|
|
contentType: "application/json",
|
|
});
|
|
await page.reload();
|
|
await expect(fixture.editor).toContainText(
|
|
"Inspect only these fresh Board attachments.",
|
|
);
|
|
await expect(
|
|
fixture.composer.getByText("board-fresh.txt", { exact: true }),
|
|
).toBeVisible();
|
|
await expect(fixture.composer.locator("img")).toHaveCount(1);
|
|
const outbound = page.waitForRequest(
|
|
(req) =>
|
|
req.method() === "POST" &&
|
|
new URL(req.url()).pathname.endsWith("/comments"),
|
|
);
|
|
await fixture.send.click();
|
|
expect((await outbound).postDataJSON().attachmentIds.sort()).toEqual(
|
|
receipts.map((row) => row.id).sort(),
|
|
);
|
|
await expect.poll(async () => (await fixture.comments()).length).toBe(1);
|
|
const [comment] = await fixture.comments();
|
|
const savedBubble = classic
|
|
? page.locator(`[id="comment-${comment!.id}"]`)
|
|
: page.getByTestId("task-chat-human-bubble").filter({
|
|
hasText:
|
|
"Inspect only these fresh Board attachments. Keep this internal.",
|
|
});
|
|
await expect(savedBubble).toHaveCount(1);
|
|
expect(comment.body).toContain(receipts[0]!.contentPath);
|
|
expect(comment.body).toContain(receipts[1]!.contentPath);
|
|
expect(
|
|
(await fixture.attachments()).map((item) => item.issueCommentId),
|
|
).toEqual([comment.id, comment.id]);
|
|
for (let index = 0; index < receipts.length; index++) {
|
|
const downloaded = await request.get(receipts[index]!.contentPath);
|
|
expect(downloaded.ok()).toBe(true);
|
|
expect(await downloaded.body()).toEqual(files[index]!.buffer);
|
|
}
|
|
await expect
|
|
.poll(() =>
|
|
page.evaluate(
|
|
(key) => localStorage.getItem(`${key}:attachments:v1`),
|
|
draftKey,
|
|
),
|
|
)
|
|
.toBeNull();
|
|
await page.reload();
|
|
await expect(fixture.editor).toBeEmpty();
|
|
expect(await fixture.comments()).toHaveLength(1);
|
|
await expect(savedBubble).toHaveCount(1);
|
|
await page.screenshot({
|
|
path: testInfo.outputPath("bound-board-attachments.png"),
|
|
fullPage: true,
|
|
});
|
|
});
|
|
|
|
test(`lost accepted response settles from its exact receipt without replay (classic=${classic})`, async ({
|
|
page,
|
|
request,
|
|
}, testInfo) => {
|
|
const fixture = await setup(page, request, classic);
|
|
await fixture.editor.fill("Accepted once: inspect this exact file.");
|
|
const receipt = await upload(page, fixture, files[0]!);
|
|
let accepted = false;
|
|
let attempts = 0;
|
|
let acceptedRequestId: string | undefined;
|
|
await page.route("**/api/issues/*/comments", async (route) => {
|
|
if (route.request().method() !== "POST") return route.continue();
|
|
attempts++;
|
|
if (accepted) return route.continue();
|
|
acceptedRequestId = route.request().postDataJSON().clientRequestId;
|
|
const response = await route.fetch();
|
|
expect(response.status()).toBe(201);
|
|
accepted = true;
|
|
await route.abort("connectionreset");
|
|
});
|
|
await fixture.send.click();
|
|
await expect.poll(() => accepted).toBe(true);
|
|
expect(await fixture.comments()).toHaveLength(1);
|
|
expect(acceptedRequestId).toEqual(expect.any(String));
|
|
expect((await fixture.comments())[0]!.clientRequestId).toBe(acceptedRequestId);
|
|
expect(
|
|
(await fixture.attachments()).find((row) => row.id === receipt.id)
|
|
?.issueCommentId,
|
|
).toBeTruthy();
|
|
await page.reload();
|
|
// The durable request receipt proves delivery even though the POST reply
|
|
// was lost. No manual Review/Discard bookkeeping or blind replay remains.
|
|
await expect(fixture.editor).toBeEmpty();
|
|
await expect(fixture.composer.getByRole("alert")).toHaveCount(0);
|
|
await expect(fixture.composer.getByText("board-fresh.txt", { exact: true })).toHaveCount(0);
|
|
await expect(fixture.send).toBeDisabled();
|
|
await fixture.editor.press("Control+Enter");
|
|
expect(attempts).toBe(1);
|
|
expect(await fixture.comments()).toHaveLength(1);
|
|
await page.screenshot({
|
|
path: testInfo.outputPath("accepted-response-lost.png"),
|
|
fullPage: true,
|
|
});
|
|
expect(
|
|
(await fixture.attachments()).find((row) => row.id === receipt.id)
|
|
?.issueCommentId,
|
|
).toBeTruthy();
|
|
await page.reload();
|
|
await expect(fixture.composer.getByRole("alert")).toHaveCount(0);
|
|
await fixture.editor.fill(
|
|
"A deliberately new comment after the original receipt settled.",
|
|
);
|
|
await fixture.send.click();
|
|
await expect.poll(async () => (await fixture.comments()).length).toBe(2);
|
|
});
|
|
|
|
test(`known rejection retains the same uploaded receipt for explicit retry (classic=${classic})`, async ({
|
|
page,
|
|
request,
|
|
}) => {
|
|
const fixture = await setup(page, request, classic);
|
|
await fixture.editor.fill("Known rejection, then retry the same file.");
|
|
const receipt = await upload(page, fixture, files[0]!);
|
|
let rejected = false;
|
|
await page.route("**/api/issues/*/comments", async (route) => {
|
|
if (route.request().method() !== "POST" || rejected)
|
|
return route.continue();
|
|
rejected = true;
|
|
await route.fulfill({
|
|
status: 422,
|
|
contentType: "application/json",
|
|
body: JSON.stringify({ error: "Fixture policy rejected this attempt" }),
|
|
});
|
|
});
|
|
await fixture.send.click();
|
|
await expect(fixture.editor).toContainText("Known rejection,");
|
|
await expect(fixture.send).toBeEnabled();
|
|
expect(await fixture.comments()).toHaveLength(0);
|
|
await page.reload();
|
|
await expect(fixture.editor).toContainText("Known rejection,");
|
|
await expect(fixture.composer.getByRole("alert")).toHaveCount(0);
|
|
await fixture.send.click();
|
|
await expect.poll(async () => (await fixture.comments()).length).toBe(1);
|
|
expect(
|
|
(await fixture.attachments()).find((row) => row.id === receipt.id)
|
|
?.issueCommentId,
|
|
).toBeTruthy();
|
|
});
|
|
|
|
test(`reload during a pending save settles its receipt and preserves a newer draft (classic=${classic})`, async ({
|
|
page,
|
|
request,
|
|
}) => {
|
|
const fixture = await setup(page, request, classic);
|
|
await fixture.editor.fill("One text-only save interrupted by reload.");
|
|
let release!: () => void;
|
|
const held = new Promise<void>((resolve) => {
|
|
release = resolve;
|
|
});
|
|
let accepted = false;
|
|
let attempts = 0;
|
|
let acceptedRequestId: string | undefined;
|
|
await page.route("**/api/issues/*/comments", async (route) => {
|
|
if (route.request().method() !== "POST") return route.continue();
|
|
attempts++;
|
|
acceptedRequestId ??= route.request().postDataJSON().clientRequestId;
|
|
const response = await route.fetch();
|
|
expect(response.status()).toBe(201);
|
|
accepted = true;
|
|
await held;
|
|
// Reload cancels the original browser request; the owned server response
|
|
// has already been observed and must never be replayed by the fixture.
|
|
await route.fulfill({ response }).catch(() => {});
|
|
});
|
|
try {
|
|
await fixture.send.click();
|
|
await expect.poll(() => accepted).toBe(true);
|
|
expect(await fixture.comments()).toHaveLength(1);
|
|
await fixture.editor.fill("A newer draft written while delivery was pending.");
|
|
await page.reload();
|
|
release();
|
|
await expect(fixture.editor).toHaveText(
|
|
"A newer draft written while delivery was pending.",
|
|
);
|
|
await expect(fixture.composer.getByRole("alert")).toHaveCount(0);
|
|
await expect(fixture.send).toBeEnabled();
|
|
expect(attempts).toBe(1);
|
|
expect(await fixture.comments()).toHaveLength(1);
|
|
expect(acceptedRequestId).toEqual(expect.any(String));
|
|
expect((await fixture.comments())[0]!.clientRequestId).toBe(acceptedRequestId);
|
|
await fixture.send.click();
|
|
await expect.poll(async () => (await fixture.comments()).length).toBe(2);
|
|
expect(attempts).toBe(2);
|
|
expect((await fixture.comments()).map((comment) => comment.body).sort()).toEqual([
|
|
"A newer draft written while delivery was pending.",
|
|
"One text-only save interrupted by reload.",
|
|
]);
|
|
} finally {
|
|
release();
|
|
}
|
|
});
|
|
}
|
|
|
|
for (const classic of [false, true])
|
|
test(`removed pending inline upload cannot reappear or submit after its HTTP response (classic=${classic})`, async ({
|
|
page,
|
|
request,
|
|
}) => {
|
|
const fixture = await setup(page, request, classic);
|
|
await fixture.editor.fill("Send without the removed image.");
|
|
let release!: () => void;
|
|
const held = new Promise<void>((resolve) => {
|
|
release = resolve;
|
|
});
|
|
let arrived = false;
|
|
await page.route(
|
|
`**/api/companies/${fixture.company.id}/issues/${fixture.issue.id}/attachments`,
|
|
async (route) => {
|
|
const response = await route.fetch();
|
|
arrived = true;
|
|
await held;
|
|
await route.fulfill({ response });
|
|
},
|
|
);
|
|
const chooser = await openAttachmentChooser(page, fixture.composer);
|
|
await chooser.setFiles(files[1]!);
|
|
await expect.poll(() => arrived).toBe(true);
|
|
await expect(fixture.send).toBeDisabled();
|
|
try {
|
|
await expect(
|
|
fixture.composer.getByText(
|
|
classic ? "Uploading to task" : "Uploading…",
|
|
{ exact: true },
|
|
),
|
|
).toBeVisible();
|
|
const remove = fixture.composer.getByRole("button", {
|
|
name: "Remove board-fresh.png",
|
|
});
|
|
await expect(remove).toBeVisible();
|
|
await remove.click();
|
|
release();
|
|
await expect(fixture.send).toBeEnabled();
|
|
await expect(fixture.composer.locator("img")).toHaveCount(0);
|
|
const outbound = page.waitForRequest(
|
|
(req) =>
|
|
req.method() === "POST" &&
|
|
new URL(req.url()).pathname.endsWith("/comments"),
|
|
);
|
|
await fixture.send.click();
|
|
expect((await outbound).postDataJSON().attachmentIds).toBeUndefined();
|
|
await expect.poll(async () => (await fixture.comments()).length).toBe(1);
|
|
expect((await fixture.comments())[0]!.body).not.toContain(
|
|
"/api/attachments/",
|
|
);
|
|
expect((await fixture.attachments())[0]!.issueCommentId).toBeNull();
|
|
} finally {
|
|
release();
|
|
}
|
|
});
|
|
|
|
test("legacy failed upload can be removed before sending the retained text", async ({
|
|
page,
|
|
request,
|
|
}) => {
|
|
const fixture = await setup(page, request, true);
|
|
await fixture.editor.fill("Keep this text after removing the failed file.");
|
|
await page.route(
|
|
`**/api/companies/${fixture.company.id}/issues/${fixture.issue.id}/attachments`,
|
|
(route) =>
|
|
route.fulfill({
|
|
status: 500,
|
|
contentType: "application/json",
|
|
body: JSON.stringify({ error: "Fixture upload rejected" }),
|
|
}),
|
|
);
|
|
const chooser = await openAttachmentChooser(page, fixture.composer);
|
|
await chooser.setFiles(files[0]!);
|
|
await expect(
|
|
fixture.composer.getByText("Fixture upload rejected", { exact: true }),
|
|
).toBeVisible();
|
|
await expect(fixture.send).toBeDisabled();
|
|
const remove = fixture.composer.getByRole("button", {
|
|
name: "Remove board-fresh.txt",
|
|
});
|
|
await expect(remove).toBeVisible();
|
|
await remove.click();
|
|
await fixture.send.click();
|
|
await expect.poll(async () => (await fixture.comments()).length).toBe(1);
|
|
expect((await fixture.comments())[0]!.body).toBe(
|
|
"Keep this text after removing the failed file.",
|
|
);
|
|
expect(await fixture.attachments()).toHaveLength(0);
|
|
});
|