Arnike/paperclip
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #7553 from paperclipai/codex/pap-10343-operator-qol-pr

Browse Source 
[codex] Group operator QoL fixes
This commit is contained in:
Dotta
2026-06-05 05:29:56 -10:00
committed by GitHub
parent e78c06464e af0c43b205
commit 487361a5cc
25 changed files with 896 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/__tests__/issue-attachment-routes.test.ts
+12 -1
View File
@@ -1,4 +1,5 @@
import { Readable } from "node:stream";
import type { IncomingMessage } from "node:http";
import express from "express";
import request from "supertest";
import { beforeEach, describe, expect, it, vi } from "vitest";
@@ -190,6 +191,13 @@ function makeAttachment(contentType: string, originalFilename: string) {
};
}
function parseBinaryResponse(res: IncomingMessage, callback: (error: Error | null, body?: Buffer) => void) {
const chunks: Buffer[] = [];
res.on("data", (chunk) => chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)));
res.on("end", () => callback(null, Buffer.concat(chunks)));
res.on("error", callback);
}
describe("normalizeIssueAttachmentMaxBytes", () => {
it("keeps the process-level attachment cap as the final cap", async () => {
const previous = process.env.PAPERCLIP_ATTACHMENT_MAX_BYTES;
@@ -362,7 +370,10 @@ describe("issue attachment routes", () => {
mockIssueService.getAttachmentById.mockResolvedValue(makeAttachment("text/html", "report.html"));
const app = await createApp(storage);
const res = await request(app).get("/api/attachments/attachment-1/content");
const res = await request(app)
.get("/api/attachments/attachment-1/content")
.buffer(true)
.parse(parseBinaryResponse);
expect(res.status).toBe(200);
expect([
server/src/__tests__/issue-thread-interactions-service.test.ts
+51 -9
View File
@@ -453,8 +453,12 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
companyId,
}, created.id, {
answers: [
{ questionId: "scope", optionIds: ["phase-1"] },
{ questionId: "extras", optionIds: ["docs", "tests", "docs"] },
{ questionId: "scope", optionIds: [], otherText: "Custom Phase 1" },
{
questionId: "extras",
optionIds: ["docs", "tests", "docs"],
otherText: " Pair with release notes ",
},
],
summaryMarkdown: "Ship Phase 1 with tests and docs.",
}, {
@@ -465,8 +469,8 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
expect(answered.result).toEqual({
version: 1,
answers: [
{ questionId: "scope", optionIds: ["phase-1"] },
{ questionId: "extras", optionIds: ["docs", "tests"] },
{ questionId: "scope", optionIds: [], otherText: "Custom Phase 1" },
{ questionId: "extras", optionIds: ["docs", "tests"], otherText: "Pair with release notes" },
],
summaryMarkdown: "Ship Phase 1 with tests and docs.",
});
@@ -824,7 +828,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
});
});
it("expires request confirmations opted into user-comment supersede after creation", async () => {
it("expires request confirmations by default when a user comments after creation", async () => {
const { companyId, issueId } = await seedConfirmationIssue();
const commentId = randomUUID();
@@ -836,12 +840,17 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
payload: {
version: 1,
prompt: "Proceed with the current draft?",
supersedeOnUserComment: true,
},
}, {
userId: "local-board",
});
expect(created).toMatchObject({
payload: {
supersedeOnUserComment: true,
},
});
const expired = await interactionsSvc.expireRequestConfirmationsSupersededByComment({
id: issueId,
companyId,
@@ -866,7 +875,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
});
});
it("keeps request confirmations pending unless user-comment supersede is explicitly enabled", async () => {
it("keeps request confirmations pending when user-comment supersede is explicitly disabled", async () => {
const { companyId, issueId } = await seedConfirmationIssue("Comment supersede opt-out");
await interactionsSvc.create({
@@ -877,6 +886,7 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
payload: {
version: 1,
prompt: "Proceed with the current draft?",
supersedeOnUserComment: false,
},
}, {
userId: "local-board",
@@ -899,6 +909,40 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
expect(rows[0]?.status).toBe("pending");
});
it("keeps legacy request confirmations pending when comment supersede was not stored", async () => {
const { companyId, issueId } = await seedConfirmationIssue("Legacy confirmation without comment supersede flag");
await db.insert(issueThreadInteractions).values({
id: randomUUID(),
companyId,
issueId,
kind: "request_confirmation",
status: "pending",
continuationPolicy: { kind: "none" },
payload: {
version: 1,
prompt: "Proceed with the current draft?",
},
createdByUserId: "local-board",
});
const expired = await interactionsSvc.expireRequestConfirmationsSupersededByComment({
id: issueId,
companyId,
}, {
id: randomUUID(),
createdAt: new Date(Date.now() + 1_000),
authorUserId: "local-board",
}, {
userId: "local-board",
});
expect(expired).toHaveLength(0);
const rows = await db.select().from(issueThreadInteractions);
expect(rows).toHaveLength(1);
expect(rows[0]?.status).toBe("pending");
});
it("does not supersede request confirmations for agent, system, or older user comments", async () => {
const { companyId, issueId } = await seedConfirmationIssue("Comment supersede exclusions");
@@ -910,7 +954,6 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
payload: {
version: 1,
prompt: "Proceed with the current draft?",
supersedeOnUserComment: true,
},
}, {
userId: "local-board",
@@ -966,7 +1009,6 @@ describeEmbeddedPostgres("issueThreadInteractionService", () => {
payload: {
version: 1,
prompt: "Proceed with the current draft?",
supersedeOnUserComment: true,
},
}, {
userId: "local-board",
server/src/__tests__/issues-service.test.ts
+72
View File
@@ -149,9 +149,11 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => {
afterEach(async () => {
await db.delete(issueComments);
await db.delete(issueRelations);
await db.delete(issueDocuments);
await db.delete(issueInboxArchives);
await db.delete(activityLog);
await db.delete(issues);
await db.delete(documents);
await db.delete(executionWorkspaces);
await db.delete(projectWorkspaces);
await db.delete(projects);
@@ -386,6 +388,76 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => {
expect(result.map((issue) => issue.id)).toEqual([titleMatchId, descriptionMatchId]);
});
it("filters issues by whether they have a plan document", async () => {
const companyId = randomUUID();
const withPlanId = randomUUID();
const withoutPlanId = randomUUID();
const otherDocumentId = randomUUID();
const planDocumentId = randomUUID();
await db.insert(companies).values({
id: companyId,
name: "Paperclip",
issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`,
requireBoardApprovalForNewAgents: false,
});
await db.insert(issues).values([
{
id: withPlanId,
companyId,
title: "Issue with plan",
status: "todo",
priority: "medium",
},
{
id: withoutPlanId,
companyId,
title: "Issue without plan",
status: "todo",
priority: "medium",
},
]);
await db.insert(documents).values([
{
id: planDocumentId,
companyId,
title: null,
format: "markdown",
latestBody: "# Plan",
},
{
id: otherDocumentId,
companyId,
title: "Notes",
format: "markdown",
latestBody: "# Notes",
},
]);
await db.insert(issueDocuments).values([
{
companyId,
issueId: withPlanId,
documentId: planDocumentId,
key: "plan",
},
{
companyId,
issueId: withoutPlanId,
documentId: otherDocumentId,
key: "notes",
},
]);
const withPlan = await svc.list(companyId, { hasPlanDocument: true });
const withoutPlan = await svc.list(companyId, { hasPlanDocument: false });
expect(withPlan.map((issue) => issue.id)).toEqual([withPlanId]);
expect(withoutPlan.map((issue) => issue.id)).toEqual([withoutPlanId]);
});
it("can page issues by most recently updated before priority", async () => {
const companyId = randomUUID();
const oldCriticalIssueId = randomUUID();
server/src/attachment-types.ts
+2
View File
@@ -35,6 +35,7 @@ export const DEFAULT_ALLOWED_TYPES: readonly string[] = [
"video/mp4",
"video/webm",
"video/quicktime",
"video/x-m4v",
];
export const DEFAULT_ATTACHMENT_CONTENT_TYPE = "application/octet-stream";
@@ -49,6 +50,7 @@ export const INLINE_ATTACHMENT_TYPES: readonly string[] = [
"video/mp4",
"video/webm",
"video/quicktime",
"video/x-m4v",
];
/**
server/src/routes/issues.ts
+19
View File
@@ -1193,6 +1193,13 @@ export function issueRoutes(
return value === true || value === "true" || value === "1";
}
function parseOptionalBooleanQuery(value: unknown) {
if (value === undefined) return undefined;
if (value === true || value === "true" || value === "1") return true;
if (value === false || value === "false" || value === "0") return false;
return null;
}
function shouldIncludeDocumentAnnotations(req: Request) {
if (req.query.includeAnnotations === "false" || req.query.includeAnnotations === "0") return false;
return req.actor.type === "agent" || parseBooleanQuery(req.query.includeAnnotations);
@@ -1993,6 +2000,7 @@ export function issueRoutes(
const attention = req.query.attention as string | undefined;
const sortField = req.query.sortField as string | undefined;
const sortDir = req.query.sortDir as string | undefined;
const hasPlanDocument = parseOptionalBooleanQuery(req.query.hasPlanDocument);
if (assigneeUserFilterRaw === "me" && (!assigneeUserId || req.actor.type !== "board")) {
res.status(403).json({ error: "assigneeUserId=me requires board authentication" });
@@ -2030,6 +2038,10 @@ export function issueRoutes(
res.status(400).json({ error: "sortDir must be 'asc' or 'desc' when provided" });
return;
}
if (hasPlanDocument === null) {
res.status(400).json({ error: "hasPlanDocument must be true or false when provided" });
return;
}
const offset = parsedOffset ?? 0;
const result = await svc.list(companyId, {
@@ -2059,6 +2071,7 @@ export function issueRoutes(
includeBlockedBy: req.query.includeBlockedBy === "true" || req.query.includeBlockedBy === "1",
includeBlockedInboxAttention:
req.query.includeBlockedInboxAttention === "true" || req.query.includeBlockedInboxAttention === "1",
hasPlanDocument,
q: req.query.q as string | undefined,
limit,
offset,
@@ -2094,6 +2107,7 @@ export function issueRoutes(
const companyId = req.params.companyId as string;
assertCompanyAccess(req, companyId);
const attention = req.query.attention as string | undefined;
const hasPlanDocument = parseOptionalBooleanQuery(req.query.hasPlanDocument);
if (attention !== "blocked") {
res.status(400).json({ error: "issues/count currently requires attention=blocked" });
return;
@@ -2102,6 +2116,10 @@ export function issueRoutes(
res.status(400).json({ error: "issues/count does not accept limit or offset" });
return;
}
if (hasPlanDocument === null) {
res.status(400).json({ error: "hasPlanDocument must be true or false when provided" });
return;
}
const count = await svc.count(companyId, {
attention: "blocked",
@@ -2126,6 +2144,7 @@ export function issueRoutes(
req.query.includePluginOperations === "true" || req.query.includePluginOperations === "1",
includeBlockedBy: true,
includeBlockedInboxAttention: true,
hasPlanDocument,
q: req.query.q as string | undefined,
});
res.json({ count });
server/src/services/issue-thread-interactions.ts
+18 -2
View File
@@ -162,6 +162,17 @@ function shouldSupersedeRequestConfirmationOnUserComment(interaction: RequestCon
return interaction.payload.supersedeOnUserComment === true;
}
function normalizeCreateInteractionInput(input: CreateIssueThreadInteraction): CreateIssueThreadInteraction {
if (input.kind !== "request_confirmation") return input;
return {
...input,
payload: {
...input.payload,
supersedeOnUserComment: input.payload.supersedeOnUserComment ?? true,
},
};
}
function isCommentAtOrAfterInteraction(args: {
commentCreatedAt: Date | string;
interactionCreatedAt: Date | string;
@@ -272,15 +283,20 @@ function normalizeQuestionAnswers(args: {
throw unprocessable(`Question ${answer.questionId} only allows one answer`);
}
const otherText = answer.otherText?.trim() ?? "";
answerByQuestionId.set(answer.questionId, {
questionId: answer.questionId,
optionIds: uniqueOptionIds,
...(otherText ? { otherText } : {}),
});
}
for (const question of args.questions) {
const answer = answerByQuestionId.get(question.id);
if (question.required && (!answer || answer.optionIds.length === 0)) {
if (
question.required
&& (!answer || (answer.optionIds.length === 0 && !answer.otherText))
) {
throw unprocessable(`Question ${question.id} requires an answer`);
}
}
@@ -668,7 +684,7 @@ export function issueThreadInteractionService(db: Db) {
input: CreateIssueThreadInteraction,
actor: InteractionActor,
) => {
const data = createIssueThreadInteractionSchema.parse(input);
const data = normalizeCreateInteractionInput(createIssueThreadInteractionSchema.parse(input));
if (data.idempotencyKey) {
const existing = await getIdempotentInteraction({
server/src/services/issues.ts
+23
View File
@@ -241,6 +241,7 @@ export interface IssueFilters {
includePluginOperations?: boolean;
includeBlockedBy?: boolean;
includeBlockedInboxAttention?: boolean;
hasPlanDocument?: boolean;
q?: string;
limit?: number;
offset?: number;
@@ -2169,6 +2170,19 @@ function issueRef(row: Pick<IssueRow, "id" | "identifier" | "title" | "status" |
};
}
function hasPlanDocumentCondition(companyId: string, hasPlanDocument: boolean): SQL {
const existsPlanDocument = sql<boolean>`
EXISTS (
SELECT 1
FROM ${issueDocuments}
WHERE ${issueDocuments.companyId} = ${companyId}
AND ${issueDocuments.issueId} = ${issues.id}
AND ${issueDocuments.key} = 'plan'
)
`;
return hasPlanDocument ? existsPlanDocument : sql<boolean>`NOT ${existsPlanDocument}`;
}
function isoDate(value: Date | string | null | undefined): string | null {
if (!value) return null;
const date = value instanceof Date ? value : new Date(value);
@@ -2881,6 +2895,9 @@ async function blockedInboxIssueConditions(
if (filters?.originKind) conditions.push(eq(issues.originKind, filters.originKind));
if (filters?.originKindPrefix) conditions.push(like(issues.originKind, `${filters.originKindPrefix}%`));
if (filters?.originId) conditions.push(eq(issues.originId, filters.originId));
if (filters?.hasPlanDocument !== undefined) {
conditions.push(hasPlanDocumentCondition(companyId, filters.hasPlanDocument));
}
if (!shouldIncludePluginOperationIssues(filters)) conditions.push(nonPluginOperationIssueCondition());
if (filters?.labelId) {
const labeledIssueIds = await dbOrTx
@@ -3826,6 +3843,9 @@ export function issueService(db: Db) {
if (filters?.originKind) conditions.push(eq(issues.originKind, filters.originKind));
if (filters?.originKindPrefix) conditions.push(like(issues.originKind, `${filters.originKindPrefix}%`));
if (filters?.originId) conditions.push(eq(issues.originId, filters.originId));
if (filters?.hasPlanDocument !== undefined) {
conditions.push(hasPlanDocumentCondition(companyId, filters.hasPlanDocument));
}
if (!shouldIncludePluginOperationIssues(filters)) {
conditions.push(nonPluginOperationIssueCondition());
}
@@ -3989,6 +4009,9 @@ export function issueService(db: Db) {
if (filters?.originKind) conditions.push(eq(issues.originKind, filters.originKind));
if (filters?.originKindPrefix) conditions.push(like(issues.originKind, `${filters.originKindPrefix}%`));
if (filters?.originId) conditions.push(eq(issues.originId, filters.originId));
if (filters?.hasPlanDocument !== undefined) {
conditions.push(hasPlanDocumentCondition(companyId, filters.hasPlanDocument));
}
if (!shouldIncludePluginOperationIssues(filters)) conditions.push(nonPluginOperationIssueCondition());
const [row] = await db
.select({ count: sql<number>`count(*)` })