From be911754c5e2e7230708026c617c8d4726bb16ff Mon Sep 17 00:00:00 2001 From: dotta Date: Fri, 20 Mar 2026 15:46:01 -0500 Subject: [PATCH] Default comment reopen to checked Co-Authored-By: Paperclip --- packages/shared/src/validators/issue.ts | 1 + .../issue-comment-reopen-routes.test.ts | 143 ++++++++++++++++++ server/src/routes/issues.ts | 15 +- ui/src/components/CommentThread.tsx | 28 ++-- 4 files changed, 169 insertions(+), 18 deletions(-) create mode 100644 server/src/__tests__/issue-comment-reopen-routes.test.ts diff --git a/packages/shared/src/validators/issue.ts b/packages/shared/src/validators/issue.ts index 5b1b37d9..3715e4e6 100644 --- a/packages/shared/src/validators/issue.ts +++ b/packages/shared/src/validators/issue.ts @@ -65,6 +65,7 @@ export type CreateIssueLabel = z.infer; export const updateIssueSchema = createIssueSchema.partial().extend({ comment: z.string().min(1).optional(), + reopen: z.boolean().optional(), hiddenAt: z.string().datetime().nullable().optional(), }); diff --git a/server/src/__tests__/issue-comment-reopen-routes.test.ts b/server/src/__tests__/issue-comment-reopen-routes.test.ts new file mode 100644 index 00000000..f7f54c23 --- /dev/null +++ b/server/src/__tests__/issue-comment-reopen-routes.test.ts @@ -0,0 +1,143 @@ +import express from "express"; +import request from "supertest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { issueRoutes } from "../routes/issues.js"; +import { errorHandler } from "../middleware/index.js"; + +const mockIssueService = vi.hoisted(() => ({ + getById: vi.fn(), + update: vi.fn(), + addComment: vi.fn(), + findMentionedAgents: vi.fn(), +})); + +const mockAccessService = vi.hoisted(() => ({ + canUser: vi.fn(), + hasPermission: vi.fn(), +})); + +const mockHeartbeatService = vi.hoisted(() => ({ + wakeup: vi.fn(async () => undefined), + reportRunActivity: vi.fn(async () => undefined), +})); + +const mockAgentService = vi.hoisted(() => ({ + getById: vi.fn(), +})); + +const mockLogActivity = vi.hoisted(() => vi.fn(async () => undefined)); + +vi.mock("../services/index.js", () => ({ + accessService: () => mockAccessService, + agentService: () => mockAgentService, + documentService: () => ({}), + executionWorkspaceService: () => ({}), + goalService: () => ({}), + heartbeatService: () => mockHeartbeatService, + issueApprovalService: () => ({}), + issueService: () => mockIssueService, + logActivity: mockLogActivity, + projectService: () => ({}), + workProductService: () => ({}), +})); + +function createApp() { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + (req as any).actor = { + type: "board", + userId: "local-board", + companyIds: ["company-1"], + source: "local_implicit", + isInstanceAdmin: false, + }; + next(); + }); + app.use("/api", issueRoutes({} as any, {} as any)); + app.use(errorHandler); + return app; +} + +function makeIssue(status: "todo" | "done") { + return { + id: "11111111-1111-4111-8111-111111111111", + companyId: "company-1", + status, + assigneeAgentId: "22222222-2222-4222-8222-222222222222", + assigneeUserId: null, + createdByUserId: "local-board", + identifier: "PAP-580", + title: "Comment reopen default", + }; +} + +describe("issue comment reopen routes", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockIssueService.addComment.mockResolvedValue({ + id: "comment-1", + issueId: "11111111-1111-4111-8111-111111111111", + companyId: "company-1", + body: "hello", + createdAt: new Date(), + updatedAt: new Date(), + authorAgentId: null, + authorUserId: "local-board", + }); + mockIssueService.findMentionedAgents.mockResolvedValue([]); + }); + + it("treats reopen=true as a no-op when the issue is already open", async () => { + mockIssueService.getById.mockResolvedValue(makeIssue("todo")); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...makeIssue("todo"), + ...patch, + })); + + const res = await request(createApp()) + .patch("/api/issues/11111111-1111-4111-8111-111111111111") + .send({ comment: "hello", reopen: true, assigneeAgentId: "33333333-3333-4333-8333-333333333333" }); + + expect(res.status).toBe(200); + expect(mockIssueService.update).toHaveBeenCalledWith("11111111-1111-4111-8111-111111111111", { + assigneeAgentId: "33333333-3333-4333-8333-333333333333", + }); + expect(mockLogActivity).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + action: "issue.updated", + details: expect.not.objectContaining({ reopened: true }), + }), + ); + }); + + it("reopens closed issues via the PATCH comment path", async () => { + mockIssueService.getById.mockResolvedValue(makeIssue("done")); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ + ...makeIssue("done"), + ...patch, + })); + + const res = await request(createApp()) + .patch("/api/issues/11111111-1111-4111-8111-111111111111") + .send({ comment: "hello", reopen: true, assigneeAgentId: "33333333-3333-4333-8333-333333333333" }); + + expect(res.status).toBe(200); + expect(mockIssueService.update).toHaveBeenCalledWith("11111111-1111-4111-8111-111111111111", { + assigneeAgentId: "33333333-3333-4333-8333-333333333333", + status: "todo", + }); + expect(mockLogActivity).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + action: "issue.updated", + details: expect.objectContaining({ + reopened: true, + reopenedFrom: "done", + status: "todo", + }), + }), + ); + }); +}); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 0e19a871..43eebe66 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -824,10 +824,14 @@ export function issueRoutes(db: Db, storage: StorageService) { if (!(await assertAgentRunCheckoutOwnership(req, res, existing))) return; const actor = getActorInfo(req); - const { comment: commentBody, hiddenAt: hiddenAtRaw, ...updateFields } = req.body; + const isClosed = existing.status === "done" || existing.status === "cancelled"; + const { comment: commentBody, reopen: reopenRequested, hiddenAt: hiddenAtRaw, ...updateFields } = req.body; if (hiddenAtRaw !== undefined) { updateFields.hiddenAt = hiddenAtRaw ? new Date(hiddenAtRaw) : null; } + if (commentBody && reopenRequested === true && isClosed && updateFields.status === undefined) { + updateFields.status = "todo"; + } let issue; try { issue = await svc.update(id, updateFields); @@ -875,6 +879,13 @@ export function issueRoutes(db: Db, storage: StorageService) { } const hasFieldChanges = Object.keys(previous).length > 0; + const reopened = + commentBody && + reopenRequested === true && + isClosed && + previous.status !== undefined && + issue.status === "todo"; + const reopenFromStatus = reopened ? existing.status : null; await logActivity(db, { companyId: issue.companyId, actorType: actor.actorType, @@ -888,6 +899,7 @@ export function issueRoutes(db: Db, storage: StorageService) { ...updateFields, identifier: issue.identifier, ...(commentBody ? { source: "comment" } : {}), + ...(reopened ? { reopened: true, reopenedFrom: reopenFromStatus } : {}), _previous: hasFieldChanges ? previous : undefined, }, }); @@ -913,6 +925,7 @@ export function issueRoutes(db: Db, storage: StorageService) { bodySnippet: comment.body.slice(0, 120), identifier: issue.identifier, issueTitle: issue.title, + ...(reopened ? { reopened: true, reopenedFrom: reopenFromStatus, source: "comment" } : {}), ...(hasFieldChanges ? { updated: true } : {}), }, }); diff --git a/ui/src/components/CommentThread.tsx b/ui/src/components/CommentThread.tsx index 0e97f31a..151e1064 100644 --- a/ui/src/components/CommentThread.tsx +++ b/ui/src/components/CommentThread.tsx @@ -50,7 +50,6 @@ interface CommentThreadProps { mentions?: MentionOption[]; } -const CLOSED_STATUSES = new Set(["done", "cancelled"]); const DRAFT_DEBOUNCE_MS = 800; function loadDraft(draftKey: string): string { @@ -261,7 +260,6 @@ export function CommentThread({ companyId, projectId, onAdd, - issueStatus, agentMap, imageUploadHandler, onAttachImage, @@ -286,8 +284,6 @@ export function CommentThread({ const location = useLocation(); const hasScrolledRef = useRef(false); - const isClosed = issueStatus ? CLOSED_STATUSES.has(issueStatus) : false; - const timeline = useMemo(() => { const commentItems: TimelineItem[] = comments.map((comment) => ({ kind: "comment", @@ -369,10 +365,10 @@ export function CommentThread({ setSubmitting(true); try { - await onAdd(trimmed, isClosed && reopen ? true : undefined, reassignment ?? undefined); + await onAdd(trimmed, reopen ? true : undefined, reassignment ?? undefined); setBody(""); if (draftKey) clearDraft(draftKey); - setReopen(false); + setReopen(true); setReassignTarget(effectiveSuggestedAssigneeValue); } finally { setSubmitting(false); @@ -439,17 +435,15 @@ export function CommentThread({ )} - {isClosed && ( - - )} + {enableReassign && reassignOptions.length > 0 && (