From 7ad225a19830db2bc515e2049aa9ba38f2cdf87a Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 24 Apr 2026 08:02:45 -0500 Subject: [PATCH] [codex] Improve issue thread review flow (#4381) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - Issue detail is where operators coordinate review, approvals, and follow-up work with active runs > - That thread UI needs to surface blockers, descendants, review handoffs, and reply ergonomics clearly enough for humans to guide agent work > - Several small gaps in the issue-thread flow were making review and navigation clunkier than necessary > - This pull request improves the reply composer, descendant/blocker presentation, interaction folding, and review-request handoff plumbing together as one cohesive issue-thread workflow slice > - The benefit is a cleaner operator review loop without changing the broader task model ## What Changed - restored and refined the floating reply composer behavior in the issue thread - folded expired confirmation interactions and improved post-submit thread scrolling behavior - surfaced descendant issue context and inline blocker/paused-assignee notices on the issue detail view - tightened large-board first paint behavior in `IssuesList` - added loose review-request handoffs through the issue execution-policy/update path and covered them with tests ## Verification - `pnpm vitest run ui/src/pages/IssueDetail.test.tsx` - `pnpm vitest run server/src/__tests__/issues-service.test.ts server/src/__tests__/issue-execution-policy.test.ts` - `pnpm exec vitest run --project @paperclipai/ui ui/src/components/IssueChatThread.test.tsx ui/src/components/IssueProperties.test.tsx ui/src/components/IssuesList.test.tsx ui/src/lib/issue-tree.test.ts ui/src/api/issues.test.ts` - `pnpm exec vitest run --project @paperclipai/adapter-utils packages/adapter-utils/src/server-utils.test.ts` - `pnpm exec vitest run --project @paperclipai/server server/src/__tests__/issue-comment-reopen-routes.test.ts -t "coerces executor handoff patches into workflow-controlled review wakes|wakes the return assignee with execution_changes_requested"` - `pnpm exec vitest run --project @paperclipai/server server/src/__tests__/issue-execution-policy.test.ts server/src/__tests__/issues-service.test.ts` ## Visual Evidence - UI layout changes are covered by the focused issue-thread component and issue-detail tests listed above. Browser screenshots were not attachable from this automated greploop environment, so reviewers should use the running preview for final visual confirmation. ## Risks - Moderate UI-flow risk: these changes touch the issue detail experience in multiple spots, so regressions would most likely show up as thread-layout quirks or incorrect review-handoff behavior > 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 Codex GPT-5-based coding agent with tool use and code execution in the Codex CLI environment ## 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 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 or documented the visual verification path - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .../adapter-utils/src/server-utils.test.ts | 28 ++ packages/adapter-utils/src/server-utils.ts | 16 +- packages/shared/src/index.ts | 1 + packages/shared/src/types/index.ts | 1 + packages/shared/src/types/issue.ts | 5 + packages/shared/src/validators/index.ts | 1 + packages/shared/src/validators/issue.ts | 6 + .../issue-comment-reopen-routes.test.ts | 9 + .../__tests__/issue-execution-policy.test.ts | 107 +++++++ server/src/__tests__/issues-service.test.ts | 104 +++++++ server/src/routes/issues.ts | 22 +- server/src/services/issue-execution-policy.ts | 16 + server/src/services/issues.ts | 19 ++ ui/src/api/issues.test.ts | 8 + ui/src/api/issues.ts | 2 + ui/src/components/IssueChatThread.test.tsx | 156 +++++++++- ui/src/components/IssueChatThread.tsx | 284 +++++++++++++++++- ui/src/components/IssueProperties.test.tsx | 1 + ui/src/components/IssuesList.test.tsx | 121 +++++++- ui/src/components/IssuesList.tsx | 82 ++++- ui/src/lib/issue-tree.test.ts | 32 +- ui/src/lib/issue-tree.ts | 36 +++ ui/src/lib/queryKeys.ts | 2 + ui/src/pages/IssueDetail.test.tsx | 16 +- ui/src/pages/IssueDetail.tsx | 15 +- 25 files changed, 1046 insertions(+), 44 deletions(-) diff --git a/packages/adapter-utils/src/server-utils.test.ts b/packages/adapter-utils/src/server-utils.test.ts index 4faceaae..4fd22814 100644 --- a/packages/adapter-utils/src/server-utils.test.ts +++ b/packages/adapter-utils/src/server-utils.test.ts @@ -326,6 +326,34 @@ describe("renderPaperclipWakePrompt", () => { expect(prompt).toContain("PAP-1723 Finish blocker (todo)"); }); + it("renders loose review request instructions for execution handoffs", () => { + const prompt = renderPaperclipWakePrompt({ + reason: "execution_review_requested", + issue: { + id: "issue-1", + identifier: "PAP-2011", + title: "Review request handoff", + status: "in_review", + }, + executionStage: { + wakeRole: "reviewer", + stageId: "stage-1", + stageType: "review", + currentParticipant: { type: "agent", agentId: "agent-1" }, + returnAssignee: { type: "agent", agentId: "agent-2" }, + reviewRequest: { + instructions: "Please focus on edge cases and leave a short risk summary.", + }, + allowedActions: ["approve", "request_changes"], + }, + fallbackFetchNeeded: false, + }); + + expect(prompt).toContain("Review request instructions:"); + expect(prompt).toContain("Please focus on edge cases and leave a short risk summary."); + expect(prompt).toContain("You are waking as the active reviewer for this issue."); + }); + it("includes continuation and child issue summaries in structured wake context", () => { const payload = { reason: "issue_children_completed", diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index 9e40d4ae..20ce8f2f 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -282,6 +282,9 @@ type PaperclipWakeExecutionStage = { stageType: string | null; currentParticipant: PaperclipWakeExecutionPrincipal | null; returnAssignee: PaperclipWakeExecutionPrincipal | null; + reviewRequest: { + instructions: string; + } | null; lastDecisionOutcome: string | null; allowedActions: string[]; }; @@ -484,11 +487,14 @@ function normalizePaperclipWakeExecutionStage(value: unknown): PaperclipWakeExec : []; const currentParticipant = normalizePaperclipWakeExecutionPrincipal(stage.currentParticipant); const returnAssignee = normalizePaperclipWakeExecutionPrincipal(stage.returnAssignee); + const reviewRequestRaw = parseObject(stage.reviewRequest); + const reviewInstructions = asString(reviewRequestRaw.instructions, "").trim(); + const reviewRequest = reviewInstructions ? { instructions: reviewInstructions } : null; const stageId = asString(stage.stageId, "").trim() || null; const stageType = asString(stage.stageType, "").trim() || null; const lastDecisionOutcome = asString(stage.lastDecisionOutcome, "").trim() || null; - if (!wakeRole && !stageId && !stageType && !currentParticipant && !returnAssignee && !lastDecisionOutcome && allowedActions.length === 0) { + if (!wakeRole && !stageId && !stageType && !currentParticipant && !returnAssignee && !reviewRequest && !lastDecisionOutcome && allowedActions.length === 0) { return null; } @@ -498,6 +504,7 @@ function normalizePaperclipWakeExecutionStage(value: unknown): PaperclipWakeExec stageType, currentParticipant, returnAssignee, + reviewRequest, lastDecisionOutcome, allowedActions, }; @@ -664,6 +671,13 @@ export function renderPaperclipWakePrompt( if (executionStage.allowedActions.length > 0) { lines.push(`- allowed actions: ${executionStage.allowedActions.join(", ")}`); } + if (executionStage.reviewRequest) { + lines.push( + "", + "Review request instructions:", + executionStage.reviewRequest.instructions, + ); + } lines.push(""); if (executionStage.wakeRole === "reviewer" || executionStage.wakeRole === "approver") { lines.push( diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index cad6d81c..b68d2d14 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -631,6 +631,7 @@ export { updateIssueSchema, issueExecutionPolicySchema, issueExecutionStateSchema, + issueReviewRequestSchema, issueExecutionWorkspaceSettingsSchema, checkoutIssueSchema, addIssueCommentSchema, diff --git a/packages/shared/src/types/index.ts b/packages/shared/src/types/index.ts index f23acff3..400721ce 100644 --- a/packages/shared/src/types/index.ts +++ b/packages/shared/src/types/index.ts @@ -119,6 +119,7 @@ export type { IssueExecutionStage, IssueExecutionStageParticipant, IssueExecutionStagePrincipal, + IssueReviewRequest, IssueExecutionDecision, IssueComment, IssueThreadInteractionActorFields, diff --git a/packages/shared/src/types/issue.ts b/packages/shared/src/types/issue.ts index f105ab5c..b49094d0 100644 --- a/packages/shared/src/types/issue.ts +++ b/packages/shared/src/types/issue.ts @@ -168,6 +168,10 @@ export interface IssueExecutionPolicy { stages: IssueExecutionStage[]; } +export interface IssueReviewRequest { + instructions: string; +} + export interface IssueExecutionState { status: IssueExecutionStateStatus; currentStageId: string | null; @@ -175,6 +179,7 @@ export interface IssueExecutionState { currentStageType: IssueExecutionStageType | null; currentParticipant: IssueExecutionStagePrincipal | null; returnAssignee: IssueExecutionStagePrincipal | null; + reviewRequest: IssueReviewRequest | null; completedStageIds: string[]; lastDecisionId: string | null; lastDecisionOutcome: IssueExecutionDecisionOutcome | null; diff --git a/packages/shared/src/validators/index.ts b/packages/shared/src/validators/index.ts index 5e8b22c5..5db0a16d 100644 --- a/packages/shared/src/validators/index.ts +++ b/packages/shared/src/validators/index.ts @@ -151,6 +151,7 @@ export { updateIssueSchema, issueExecutionPolicySchema, issueExecutionStateSchema, + issueReviewRequestSchema, issueExecutionWorkspaceSettingsSchema, checkoutIssueSchema, addIssueCommentSchema, diff --git a/packages/shared/src/validators/issue.ts b/packages/shared/src/validators/issue.ts index 6a0f2df9..0db2e1bc 100644 --- a/packages/shared/src/validators/issue.ts +++ b/packages/shared/src/validators/issue.ts @@ -105,6 +105,10 @@ export const issueExecutionPolicySchema = z.object({ stages: z.array(issueExecutionStageSchema).default([]), }); +export const issueReviewRequestSchema = z.object({ + instructions: z.string().trim().min(1).max(20000), +}).strict(); + export const issueExecutionStateSchema = z.object({ status: z.enum(ISSUE_EXECUTION_STATE_STATUSES), currentStageId: z.string().uuid().nullable(), @@ -112,6 +116,7 @@ export const issueExecutionStateSchema = z.object({ currentStageType: z.enum(ISSUE_EXECUTION_STAGE_TYPES).nullable(), currentParticipant: issueExecutionStagePrincipalSchema.nullable(), returnAssignee: issueExecutionStagePrincipalSchema.nullable(), + reviewRequest: issueReviewRequestSchema.nullable().optional().default(null), completedStageIds: z.array(z.string().uuid()).default([]), lastDecisionId: z.string().uuid().nullable(), lastDecisionOutcome: z.enum(ISSUE_EXECUTION_DECISION_OUTCOMES).nullable(), @@ -164,6 +169,7 @@ export type CreateIssueLabel = z.infer; export const updateIssueSchema = createIssueSchema.partial().extend({ assigneeAgentId: z.string().trim().min(1).optional().nullable(), comment: z.string().min(1).optional(), + reviewRequest: issueReviewRequestSchema.optional().nullable(), reopen: z.boolean().optional(), interrupt: 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 index a821d533..1ff5233c 100644 --- a/server/src/__tests__/issue-comment-reopen-routes.test.ts +++ b/server/src/__tests__/issue-comment-reopen-routes.test.ts @@ -795,6 +795,9 @@ describe("issue comment reopen routes", () => { status: "in_review", assigneeAgentId: null, assigneeUserId: "local-board", + reviewRequest: { + instructions: "Please verify the fix against the reproduction steps and note any residual risk.", + }, }); expect(res.status).toBe(200); @@ -811,6 +814,9 @@ describe("issue comment reopen routes", () => { type: "agent", agentId: "22222222-2222-4222-8222-222222222222", }, + reviewRequest: { + instructions: "Please verify the fix against the reproduction steps and note any residual risk.", + }, }); expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith( "33333333-3333-4333-8333-333333333333", @@ -821,6 +827,9 @@ describe("issue comment reopen routes", () => { executionStage: expect.objectContaining({ wakeRole: "reviewer", stageType: "review", + reviewRequest: { + instructions: "Please verify the fix against the reproduction steps and note any residual risk.", + }, allowedActions: ["approve", "request_changes"], }), }), diff --git a/server/src/__tests__/issue-execution-policy.test.ts b/server/src/__tests__/issue-execution-policy.test.ts index 6aeac554..37a64dc2 100644 --- a/server/src/__tests__/issue-execution-policy.test.ts +++ b/server/src/__tests__/issue-execution-policy.test.ts @@ -171,6 +171,75 @@ describe("issue execution policy transitions", () => { expect(result.decision).toBeUndefined(); }); + it("carries loose review instructions on the pending handoff", () => { + const reviewInstructions = [ + "Please focus on whether the migration path is reversible.", + "", + "- Check failure handling", + "- Call out any unclear operator instructions", + ].join("\n"); + + const result = applyIssueExecutionPolicyTransition({ + issue: { + status: "in_progress", + assigneeAgentId: coderAgentId, + assigneeUserId: null, + executionPolicy: policy, + executionState: null, + }, + policy, + requestedStatus: "done", + requestedAssigneePatch: {}, + actor: { agentId: coderAgentId }, + commentBody: "Implemented the migration", + reviewRequest: { instructions: reviewInstructions }, + }); + + expect(result.patch.executionState).toMatchObject({ + status: "pending", + currentStageType: "review", + currentParticipant: { type: "agent", agentId: qaAgentId }, + reviewRequest: { instructions: reviewInstructions }, + }); + }); + + it("clears loose review instructions with explicit null during a stage transition", () => { + const reviewStageId = policy.stages[0].id; + const result = applyIssueExecutionPolicyTransition({ + issue: { + status: "in_progress", + assigneeAgentId: coderAgentId, + assigneeUserId: null, + executionPolicy: policy, + executionState: { + status: "pending", + currentStageId: reviewStageId, + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: qaAgentId }, + returnAssignee: { type: "agent", agentId: coderAgentId }, + reviewRequest: { instructions: "Old review request" }, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + }, + policy, + requestedStatus: "in_review", + requestedAssigneePatch: {}, + actor: { agentId: coderAgentId }, + commentBody: "Ready for review", + reviewRequest: null, + }); + + expect(result.patch.executionState).toMatchObject({ + status: "pending", + currentStageType: "review", + currentParticipant: { type: "agent", agentId: qaAgentId }, + reviewRequest: null, + }); + }); + it("reviewer approves → advances to approval stage", () => { const reviewStageId = policy.stages[0].id; const result = applyIssueExecutionPolicyTransition({ @@ -214,6 +283,44 @@ describe("issue execution policy transitions", () => { }); }); + it("lets a reviewer provide loose instructions for the next approval stage", () => { + const reviewStageId = policy.stages[0].id; + const approvalInstructions = "Please decide whether this is ready to ship, with any launch caveats."; + const result = applyIssueExecutionPolicyTransition({ + issue: { + status: "in_review", + assigneeAgentId: qaAgentId, + assigneeUserId: null, + executionPolicy: policy, + executionState: { + status: "pending", + currentStageId: reviewStageId, + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: qaAgentId }, + returnAssignee: { type: "agent", agentId: coderAgentId }, + reviewRequest: { instructions: "Review the implementation details." }, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + }, + policy, + requestedStatus: "done", + requestedAssigneePatch: {}, + actor: { agentId: qaAgentId }, + commentBody: "QA signoff complete", + reviewRequest: { instructions: approvalInstructions }, + }); + + expect(result.patch.executionState).toMatchObject({ + status: "pending", + currentStageType: "approval", + currentParticipant: { type: "user", userId: ctoUserId }, + reviewRequest: { instructions: approvalInstructions }, + }); + }); + it("approver approves → marks completed (allows done)", () => { const reviewStageId = policy.stages[0].id; const approvalStageId = policy.stages[1].id; diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index b21c603e..05ea80c4 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -355,6 +355,110 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => { expect(result.map((issue) => issue.id)).toEqual([commentMatchId, descriptionMatchId]); }); + it("filters issue lists to the full descendant tree for a root issue", async () => { + const companyId = randomUUID(); + const rootId = randomUUID(); + const childId = randomUUID(); + const grandchildId = randomUUID(); + const siblingId = 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: rootId, + companyId, + title: "Root", + status: "todo", + priority: "medium", + }, + { + id: childId, + companyId, + parentId: rootId, + title: "Child", + status: "todo", + priority: "medium", + }, + { + id: grandchildId, + companyId, + parentId: childId, + title: "Grandchild", + status: "todo", + priority: "medium", + }, + { + id: siblingId, + companyId, + title: "Sibling", + status: "todo", + priority: "medium", + }, + ]); + + const result = await svc.list(companyId, { descendantOf: rootId }); + + expect(new Set(result.map((issue) => issue.id))).toEqual(new Set([childId, grandchildId])); + }); + + it("combines descendant filtering with search", async () => { + const companyId = randomUUID(); + const rootId = randomUUID(); + const childId = randomUUID(); + const grandchildId = randomUUID(); + const outsideMatchId = 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: rootId, + companyId, + title: "Root", + status: "todo", + priority: "medium", + }, + { + id: childId, + companyId, + parentId: rootId, + title: "Relevant parent", + status: "todo", + priority: "medium", + }, + { + id: grandchildId, + companyId, + parentId: childId, + title: "Needle grandchild", + status: "todo", + priority: "medium", + }, + { + id: outsideMatchId, + companyId, + title: "Needle outside", + status: "todo", + priority: "medium", + }, + ]); + + const result = await svc.list(companyId, { descendantOf: rootId, q: "needle" }); + + expect(result.map((issue) => issue.id)).toEqual([grandchildId]); + }); + it("accepts issue identifiers through getById", async () => { const companyId = randomUUID(); const issueId = randomUUID(); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index aef722aa..0df9172c 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -101,6 +101,7 @@ type ExecutionStageWakeContext = { stageType: ParsedExecutionState["currentStageType"]; currentParticipant: ParsedExecutionState["currentParticipant"]; returnAssignee: ParsedExecutionState["returnAssignee"]; + reviewRequest: ParsedExecutionState["reviewRequest"]; lastDecisionOutcome: ParsedExecutionState["lastDecisionOutcome"]; allowedActions: string[]; }; @@ -124,6 +125,7 @@ function buildExecutionStageWakeContext(input: { stageType: input.state.currentStageType, currentParticipant: input.state.currentParticipant, returnAssignee: input.state.returnAssignee, + reviewRequest: input.state.reviewRequest ?? null, lastDecisionOutcome: input.state.lastDecisionOutcome, allowedActions: input.allowedActions, }; @@ -831,6 +833,7 @@ export function issueRoutes( workspaceId: req.query.workspaceId as string | undefined, executionWorkspaceId: req.query.executionWorkspaceId as string | undefined, parentId: req.query.parentId as string | undefined, + descendantOf: req.query.descendantOf as string | undefined, labelId: req.query.labelId as string | undefined, originKind: req.query.originKind as string | undefined, originId: req.query.originId as string | undefined, @@ -1787,6 +1790,7 @@ export function issueRoutes( : null; const { comment: commentBody, + reviewRequest, reopen: reopenRequested, interrupt: interruptRequested, hiddenAt: hiddenAtRaw, @@ -1813,7 +1817,8 @@ export function issueRoutes( : false; let interruptedRunId: string | null = null; const closedExecutionWorkspace = await getClosedIssueExecutionWorkspace(existing); - const isAgentWorkUpdate = req.actor.type === "agent" && Object.keys(updateFields).length > 0; + const isAgentWorkUpdate = + req.actor.type === "agent" && (Object.keys(updateFields).length > 0 || reviewRequest !== undefined); if (closedExecutionWorkspace && (commentBody || isAgentWorkUpdate)) { respondClosedIssueExecutionWorkspace(res, closedExecutionWorkspace); @@ -1887,6 +1892,7 @@ export function issueRoutes( userId: actor.actorType === "user" ? actor.actorId : null, }, commentBody, + reviewRequest: reviewRequest === undefined ? undefined : reviewRequest, }); const decisionId = transition.decision ? randomUUID() : null; if (decisionId) { @@ -1900,6 +1906,20 @@ export function issueRoutes( }; } Object.assign(updateFields, transition.patch); + if (reviewRequest !== undefined && transition.patch.executionState === undefined) { + const existingExecutionState = parseIssueExecutionState(existing.executionState); + if (!existingExecutionState || existingExecutionState.status !== "pending") { + if (reviewRequest !== null) { + res.status(422).json({ error: "reviewRequest requires an active review or approval stage" }); + return; + } + } else { + updateFields.executionState = { + ...existingExecutionState, + reviewRequest, + }; + } + } const nextAssigneeAgentId = updateFields.assigneeAgentId === undefined ? existing.assigneeAgentId : (updateFields.assigneeAgentId as string | null); diff --git a/server/src/services/issue-execution-policy.ts b/server/src/services/issue-execution-policy.ts index 428a1101..9a78512d 100644 --- a/server/src/services/issue-execution-policy.ts +++ b/server/src/services/issue-execution-policy.ts @@ -31,6 +31,7 @@ type TransitionInput = { requestedAssigneePatch: RequestedAssigneePatch; actor: ActorLike; commentBody?: string | null; + reviewRequest?: IssueExecutionState["reviewRequest"] | null; }; type TransitionResult = { @@ -168,6 +169,7 @@ function buildCompletedState(previous: IssueExecutionState | null, currentStage: currentStageType: null, currentParticipant: null, returnAssignee: previous?.returnAssignee ?? null, + reviewRequest: null, completedStageIds, lastDecisionId: previous?.lastDecisionId ?? null, lastDecisionOutcome: "approved", @@ -186,6 +188,7 @@ function buildStateWithCompletedStages(input: { currentStageType: input.previous?.currentStageType ?? null, currentParticipant: input.previous?.currentParticipant ?? null, returnAssignee: input.previous?.returnAssignee ?? input.returnAssignee, + reviewRequest: input.previous?.reviewRequest ?? null, completedStageIds: input.completedStageIds, lastDecisionId: input.previous?.lastDecisionId ?? null, lastDecisionOutcome: input.previous?.lastDecisionOutcome ?? null, @@ -204,6 +207,7 @@ function buildSkippedStageCompletedState(input: { currentStageType: null, currentParticipant: null, returnAssignee: input.previous?.returnAssignee ?? input.returnAssignee, + reviewRequest: null, completedStageIds: input.completedStageIds, lastDecisionId: input.previous?.lastDecisionId ?? null, lastDecisionOutcome: input.previous?.lastDecisionOutcome ?? null, @@ -216,6 +220,7 @@ function buildPendingState(input: { stageIndex: number; participant: IssueExecutionStagePrincipal; returnAssignee: IssueExecutionStagePrincipal | null; + reviewRequest?: IssueExecutionState["reviewRequest"] | null; }): IssueExecutionState { return { status: PENDING_STATUS, @@ -224,6 +229,7 @@ function buildPendingState(input: { currentStageType: input.stage.type, currentParticipant: input.participant, returnAssignee: input.returnAssignee, + reviewRequest: input.reviewRequest ?? null, completedStageIds: input.previous?.completedStageIds ?? [], lastDecisionId: input.previous?.lastDecisionId ?? null, lastDecisionOutcome: input.previous?.lastDecisionOutcome ?? null, @@ -236,6 +242,7 @@ function buildChangesRequestedState(previous: IssueExecutionState, currentStage: status: CHANGES_REQUESTED_STATUS, currentStageId: currentStage.id, currentStageType: currentStage.type, + reviewRequest: null, lastDecisionOutcome: "changes_requested", }; } @@ -247,6 +254,7 @@ function buildPendingStagePatch(input: { stage: IssueExecutionStage; participant: IssueExecutionStagePrincipal; returnAssignee: IssueExecutionStagePrincipal | null; + reviewRequest?: IssueExecutionState["reviewRequest"] | null; }) { input.patch.status = "in_review"; Object.assign(input.patch, patchForPrincipal(input.participant)); @@ -256,6 +264,7 @@ function buildPendingStagePatch(input: { stageIndex: input.policy.stages.findIndex((candidate) => candidate.id === input.stage.id), participant: input.participant, returnAssignee: input.returnAssignee, + reviewRequest: input.reviewRequest, }); } @@ -295,6 +304,9 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra const currentStage = input.policy ? findStageById(input.policy, existingState?.currentStageId) : null; const requestedStatus = input.requestedStatus; const activeStage = currentStage && existingState?.status === PENDING_STATUS ? currentStage : null; + const effectiveReviewRequest = input.reviewRequest === undefined + ? existingState?.reviewRequest ?? null + : input.reviewRequest; if (!input.policy) { if (existingState) { @@ -359,6 +371,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra stage: activeStage, participant, returnAssignee: existingState?.returnAssignee ?? currentAssignee ?? actor, + reviewRequest: effectiveReviewRequest, }); return { patch, @@ -405,6 +418,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra stage: nextStage, participant, returnAssignee: existingState?.returnAssignee ?? currentAssignee ?? actor, + reviewRequest: input.reviewRequest ?? null, }); return { patch, @@ -461,6 +475,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra stage: activeStage, participant: currentParticipant, returnAssignee: existingState?.returnAssignee ?? currentAssignee ?? actor, + reviewRequest: effectiveReviewRequest, }); return { patch, @@ -538,6 +553,7 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra stage: pendingStage, participant, returnAssignee, + reviewRequest: input.reviewRequest ?? null, }); return { patch, diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index 013d0e9b..36e9ac97 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -106,6 +106,7 @@ export interface IssueFilters { workspaceId?: string; executionWorkspaceId?: string; parentId?: string; + descendantOf?: string; labelId?: string; originKind?: string; originId?: string; @@ -1396,6 +1397,24 @@ export function issueService(db: Db) { AND ${issueComments.body} ILIKE ${containsPattern} ESCAPE '\\' ) `; + if (filters?.descendantOf) { + conditions.push(sql` + ${issues.id} IN ( + WITH RECURSIVE descendants(id) AS ( + SELECT ${issues.id} + FROM ${issues} + WHERE ${issues.companyId} = ${companyId} + AND ${issues.parentId} = ${filters.descendantOf} + UNION + SELECT ${issues.id} + FROM ${issues} + JOIN descendants ON ${issues.parentId} = descendants.id + WHERE ${issues.companyId} = ${companyId} + ) + SELECT id FROM descendants + ) + `); + } if (filters?.status) { const statuses = filters.status.split(",").map((s) => s.trim()); conditions.push(statuses.length === 1 ? eq(issues.status, statuses[0]) : inArray(issues.status, statuses)); diff --git a/ui/src/api/issues.test.ts b/ui/src/api/issues.test.ts index a32add42..dedbfa9c 100644 --- a/ui/src/api/issues.test.ts +++ b/ui/src/api/issues.test.ts @@ -24,6 +24,14 @@ describe("issuesApi.list", () => { ); }); + it("passes descendantOf through to the company issues endpoint", async () => { + await issuesApi.list("company-1", { descendantOf: "issue-root-1", limit: 25 }); + + expect(mockApi.get).toHaveBeenCalledWith( + "/companies/company-1/issues?descendantOf=issue-root-1&limit=25", + ); + }); + it("passes generic workspaceId filters through to the company issues endpoint", async () => { await issuesApi.list("company-1", { workspaceId: "workspace-1", limit: 1000 }); diff --git a/ui/src/api/issues.ts b/ui/src/api/issues.ts index b2856513..d7e244b2 100644 --- a/ui/src/api/issues.ts +++ b/ui/src/api/issues.ts @@ -43,6 +43,7 @@ export const issuesApi = { executionWorkspaceId?: string; originKind?: string; originId?: string; + descendantOf?: string; includeRoutineExecutions?: boolean; q?: string; limit?: number; @@ -63,6 +64,7 @@ export const issuesApi = { if (filters?.executionWorkspaceId) params.set("executionWorkspaceId", filters.executionWorkspaceId); if (filters?.originKind) params.set("originKind", filters.originKind); if (filters?.originId) params.set("originId", filters.originId); + if (filters?.descendantOf) params.set("descendantOf", filters.descendantOf); if (filters?.includeRoutineExecutions) params.set("includeRoutineExecutions", "true"); if (filters?.q) params.set("q", filters.q); if (filters?.limit) params.set("limit", String(filters.limit)); diff --git a/ui/src/components/IssueChatThread.test.tsx b/ui/src/components/IssueChatThread.test.tsx index 9a85a862..920eba15 100644 --- a/ui/src/components/IssueChatThread.test.tsx +++ b/ui/src/components/IssueChatThread.test.tsx @@ -14,6 +14,7 @@ import { } from "./IssueChatThread"; import type { AskUserQuestionsInteraction, + RequestConfirmationInteraction, SuggestTasksInteraction, } from "../lib/issue-thread-interactions"; @@ -215,6 +216,39 @@ function createQuestionInteraction( }; } +function createExpiredRequestConfirmationInteraction( + overrides: Partial = {}, +): RequestConfirmationInteraction { + return { + id: "interaction-confirmation-expired", + companyId: "company-1", + issueId: "issue-1", + kind: "request_confirmation", + title: "Approve the plan", + status: "expired", + continuationPolicy: "wake_assignee_on_accept", + createdByAgentId: "agent-1", + createdByUserId: null, + resolvedByAgentId: null, + resolvedByUserId: "user-1", + createdAt: new Date("2026-04-06T12:04:00.000Z"), + updatedAt: new Date("2026-04-06T12:05:00.000Z"), + resolvedAt: new Date("2026-04-06T12:05:00.000Z"), + payload: { + version: 1, + prompt: "Approve the plan and let the assignee start implementation?", + acceptLabel: "Approve plan", + rejectLabel: "Request revisions", + }, + result: { + version: 1, + outcome: "superseded_by_comment", + commentId: "comment-1", + }, + ...overrides, + }; +} + describe("IssueChatThread", () => { let container: HTMLDivElement; @@ -535,6 +569,50 @@ describe("IssueChatThread", () => { }); }); + it("folds expired request confirmations into an activity row by default", async () => { + const root = createRoot(container); + + await act(async () => { + root.render( + + {}} + currentUserId="user-1" + userLabelMap={new Map([["user-1", "Dotta"]])} + showComposer={false} + enableLiveTranscriptPolling={false} + /> + , + ); + }); + + expect(container.textContent).toContain("Dotta"); + expect(container.textContent).toContain("updated this task"); + expect(container.textContent).toContain("Expired confirmation"); + expect(container.textContent).not.toContain("Approve the plan"); + + const toggleButton = Array.from(container.querySelectorAll("button")).find((button) => + button.textContent?.includes("Expired confirmation"), + ); + expect(toggleButton).toBeTruthy(); + + await act(async () => { + toggleButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + expect(container.textContent).toContain("Approve the plan"); + expect(container.textContent).toContain("Confirmation expired after comment"); + + act(() => { + root.unmount(); + }); + }); + it("renders the transcript directly from stable Paperclip messages", () => { const root = createRoot(container); @@ -706,7 +784,7 @@ describe("IssueChatThread", () => { }); }); - it("keeps the composer inline with bottom breathing room and a capped editor height", () => { + it("keeps the composer floating with a capped editor height", () => { const root = createRoot(container); act(() => { @@ -724,15 +802,85 @@ describe("IssueChatThread", () => { ); }); + const dock = container.querySelector('[data-testid="issue-chat-composer-dock"]') as HTMLDivElement | null; + expect(dock).not.toBeNull(); + expect(dock?.className).toContain("sticky"); + expect(dock?.className).toContain("bottom-[calc(env(safe-area-inset-bottom)+20px)]"); + expect(dock?.className).toContain("z-20"); + const composer = container.querySelector('[data-testid="issue-chat-composer"]') as HTMLDivElement | null; expect(composer).not.toBeNull(); - expect(composer?.className).not.toContain("sticky"); - expect(composer?.className).not.toContain("bottom-0"); - expect(composer?.className).toContain("pb-[calc(env(safe-area-inset-bottom)+1.5rem)]"); + expect(composer?.className).toContain("rounded-md"); + expect(composer?.className).not.toContain("rounded-lg"); + expect(composer?.className).toContain("p-[15px]"); const editor = container.querySelector('textarea[aria-label="Issue chat editor"]') as HTMLTextAreaElement | null; expect(editor?.dataset.contentClassName).toContain("max-h-[28dvh]"); expect(editor?.dataset.contentClassName).toContain("overflow-y-auto"); + expect(editor?.dataset.contentClassName).not.toContain("min-h-[72px]"); + + act(() => { + root.unmount(); + }); + }); + + it("renders the bottom spacer with zero height until the user has submitted", () => { + const root = createRoot(container); + + act(() => { + root.render( + + {}} + enableLiveTranscriptPolling={false} + /> + , + ); + }); + + const spacer = container.querySelector('[data-testid="issue-chat-bottom-spacer"]') as HTMLDivElement | null; + expect(spacer).not.toBeNull(); + expect(spacer?.style.height).toBe("0px"); + + act(() => { + root.unmount(); + }); + }); + + it("omits the bottom spacer when the composer is hidden", () => { + const root = createRoot(container); + + act(() => { + root.render( + + {}} + showComposer={false} + enableLiveTranscriptPolling={false} + /> + , + ); + }); + + const spacer = container.querySelector('[data-testid="issue-chat-bottom-spacer"]'); + expect(spacer).toBeNull(); act(() => { root.unmount(); diff --git a/ui/src/components/IssueChatThread.tsx b/ui/src/components/IssueChatThread.tsx index 6c5de990..ea1d4d3a 100644 --- a/ui/src/components/IssueChatThread.tsx +++ b/ui/src/components/IssueChatThread.tsx @@ -20,6 +20,7 @@ import { useRef, useState, type ChangeEvent, + type DragEvent as ReactDragEvent, type ErrorInfo, type Ref, type ReactNode, @@ -52,7 +53,7 @@ import type { RequestConfirmationInteraction, SuggestTasksInteraction, } from "../lib/issue-thread-interactions"; -import { isIssueThreadInteraction } from "../lib/issue-thread-interactions"; +import { buildIssueThreadInteractionSummary, isIssueThreadInteraction } from "../lib/issue-thread-interactions"; import { resolveIssueChatTranscriptRuns } from "../lib/issueChatTranscriptRuns"; import type { IssueTimelineAssignee, IssueTimelineEvent } from "../lib/issue-timeline-events"; import { Button } from "@/components/ui/button"; @@ -505,6 +506,11 @@ function IssueChatFallbackThread({ const DRAFT_DEBOUNCE_MS = 800; const COMPOSER_FOCUS_SCROLL_PADDING_PX = 96; +const SUBMIT_SCROLL_RESERVE_VH = 0.4; + +function hasFilePayload(evt: ReactDragEvent) { + return Array.from(evt.dataTransfer?.types ?? []).includes("Files"); +} function toIsoString(value: string | Date | null | undefined): string | null { if (!value) return null; @@ -610,6 +616,23 @@ function initialsForName(name: string) { return name.slice(0, 2).toUpperCase(); } +function formatInteractionActorLabel(args: { + agentId?: string | null; + userId?: string | null; + agentMap?: Map; + currentUserId?: string | null; + userLabelMap?: ReadonlyMap | null; +}) { + const { agentId, userId, agentMap, currentUserId, userLabelMap } = args; + if (agentId) return agentMap?.get(agentId)?.name ?? agentId.slice(0, 8); + if (userId) { + return userLabelMap?.get(userId) + ?? formatAssigneeUserLabel(userId, currentUserId, userLabelMap) + ?? "Board"; + } + return "System"; +} + export function resolveIssueChatHumanAuthor(args: { authorName?: string | null; authorUserId?: string | null; @@ -1735,6 +1758,106 @@ function IssueChatFeedbackButtons({ ); } +function ExpiredRequestConfirmationActivity({ + message, + anchorId, + interaction, +}: { + message: ThreadMessage; + anchorId?: string; + interaction: RequestConfirmationInteraction; +}) { + const { + agentMap, + currentUserId, + userLabelMap, + onAcceptInteraction, + onRejectInteraction, + } = useContext(IssueChatCtx); + const [expanded, setExpanded] = useState(false); + const hasResolvedActor = Boolean(interaction.resolvedByAgentId || interaction.resolvedByUserId); + const actorAgentId = hasResolvedActor + ? interaction.resolvedByAgentId ?? null + : interaction.createdByAgentId ?? null; + const actorUserId = hasResolvedActor + ? interaction.resolvedByUserId ?? null + : interaction.createdByUserId ?? null; + const actorName = formatInteractionActorLabel({ + agentId: actorAgentId, + userId: actorUserId, + agentMap, + currentUserId, + userLabelMap, + }); + const actorIcon = actorAgentId ? agentMap?.get(actorAgentId)?.icon : undefined; + const isCurrentUser = Boolean(actorUserId && currentUserId && actorUserId === currentUserId); + const detailsId = anchorId ? `${anchorId}-details` : `${interaction.id}-details`; + const summary = buildIssueThreadInteractionSummary(interaction); + + const rowContent = ( +
+
+ {actorName} + updated this task + + {timeAgo(message.createdAt)} + + +
+ {expanded ? ( +

+ {summary} +

+ ) : null} +
+ ); + + return ( +
+ {isCurrentUser ? ( +
+ {rowContent} +
+ ) : ( +
+ + {actorIcon ? ( + + ) : ( + {initialsForName(actorName)} + )} + + {rowContent} +
+ )} + {expanded ? ( +
+ +
+ ) : null} +
+ ); +} + function IssueChatSystemMessage({ message }: { message: ThreadMessage }) { const { agentMap, @@ -1767,6 +1890,16 @@ function IssueChatSystemMessage({ message }: { message: ThreadMessage }) { : null; if (custom.kind === "interaction" && interaction) { + if (interaction.kind === "request_confirmation" && interaction.status === "expired") { + return ( + + ); + } + return (
@@ -1921,12 +2054,15 @@ const IssueChatComposer = forwardRef(null); const editorRef = useRef(null); const composerContainerRef = useRef(null); const draftTimer = useRef | null>(null); + const canAcceptFiles = Boolean(onImageUpload || onAttachImage); function queueViewportRestore(snapshot: ReturnType) { if (!snapshot) return; @@ -2026,25 +2162,46 @@ const IssueChatComposer = forwardRef prev ? `${prev}\n\n${markdown}` : markdown); + } else if (onAttachImage) { + await onAttachImage(file); + } + } + async function handleAttachFile(evt: ChangeEvent) { const file = evt.target.files?.[0]; if (!file) return; setAttaching(true); try { - if (onImageUpload) { - const url = await onImageUpload(file); - const safeName = file.name.replace(/[[\]]/g, "\\$&"); - const markdown = `![${safeName}](${url})`; - setBody((prev) => prev ? `${prev}\n\n${markdown}` : markdown); - } else if (onAttachImage) { - await onAttachImage(file); - } + await attachFile(file); } finally { setAttaching(false); if (attachInputRef.current) attachInputRef.current.value = ""; } } + async function handleDroppedFiles(files: FileList | null | undefined) { + if (!files || files.length === 0) return; + setAttaching(true); + try { + for (const file of Array.from(files)) { + await attachFile(file); + } + } finally { + setAttaching(false); + } + } + + function resetDragState() { + dragDepthRef.current = 0; + setIsDragOver(false); + } + const canSubmit = !submitting && !!body.trim(); if (composerDisabledReason) { @@ -2059,7 +2216,35 @@ const IssueChatComposer = forwardRef { + if (!canAcceptFiles || !hasFilePayload(evt)) return; + dragDepthRef.current += 1; + setIsDragOver(true); + }} + onDragOver={(evt) => { + if (!canAcceptFiles || !hasFilePayload(evt)) return; + evt.preventDefault(); + evt.dataTransfer.dropEffect = "copy"; + }} + onDragLeave={() => { + if (!canAcceptFiles) return; + dragDepthRef.current = Math.max(0, dragDepthRef.current - 1); + if (dragDepthRef.current === 0) setIsDragOver(false); + }} + onDrop={(evt) => { + if (!canAcceptFiles) return; + if (evt.defaultPrevented) { + resetDragState(); + return; + } + evt.preventDefault(); + resetDragState(); + void handleDroppedFiles(evt.dataTransfer?.files); + }} > {composerHint ? ( @@ -2204,6 +2389,11 @@ export function IssueChatThread({ const composerViewportAnchorRef = useRef(null); const composerViewportSnapshotRef = useRef>(null); const preserveComposerViewportRef = useRef(false); + const pendingSubmitScrollRef = useRef(false); + const lastUserMessageIdRef = useRef(null); + const spacerBaselineAnchorRef = useRef(null); + const spacerInitialReserveRef = useRef(0); + const [bottomSpacerHeight, setBottomSpacerHeight] = useState(0); const displayLiveRuns = useMemo(() => { const deduped = new Map(); for (const run of liveRuns) { @@ -2317,10 +2507,57 @@ export function IssueChatThread({ const runtime = usePaperclipIssueRuntime({ messages, isRunning, - onSend: ({ body, reopen, reassignment }) => onAdd(body, reopen, reassignment), + onSend: ({ body, reopen, reassignment }) => { + pendingSubmitScrollRef.current = true; + return onAdd(body, reopen, reassignment); + }, onCancel: onCancelRun, }); + useEffect(() => { + const lastUserMessage = [...messages].reverse().find((m) => m.role === "user"); + const lastUserId = lastUserMessage?.id ?? null; + + if ( + pendingSubmitScrollRef.current + && lastUserId + && lastUserId !== lastUserMessageIdRef.current + ) { + pendingSubmitScrollRef.current = false; + const custom = lastUserMessage?.metadata.custom as { anchorId?: unknown } | undefined; + const anchorId = typeof custom?.anchorId === "string" ? custom.anchorId : null; + if (anchorId) { + const reserve = Math.round(window.innerHeight * SUBMIT_SCROLL_RESERVE_VH); + spacerBaselineAnchorRef.current = anchorId; + spacerInitialReserveRef.current = reserve; + setBottomSpacerHeight(reserve); + requestAnimationFrame(() => { + const el = document.getElementById(anchorId); + el?.scrollIntoView({ behavior: "smooth", block: "start" }); + }); + } + } + + lastUserMessageIdRef.current = lastUserId; + }, [messages]); + + useLayoutEffect(() => { + const anchorId = spacerBaselineAnchorRef.current; + if (!anchorId || spacerInitialReserveRef.current <= 0) return; + const userEl = document.getElementById(anchorId); + const bottomEl = bottomAnchorRef.current; + if (!userEl || !bottomEl) return; + const contentBelow = Math.max( + 0, + bottomEl.getBoundingClientRect().top - userEl.getBoundingClientRect().bottom, + ); + const next = Math.max(0, spacerInitialReserveRef.current - contentBelow); + setBottomSpacerHeight((prev) => (prev === next ? prev : next)); + if (next === 0) { + spacerBaselineAnchorRef.current = null; + spacerInitialReserveRef.current = 0; + } + }, [messages]); useLayoutEffect(() => { const composerElement = composerViewportAnchorRef.current; if (preserveComposerViewportRef.current) { @@ -2459,15 +2696,30 @@ export function IssueChatThread({ return ; }) )} + {showComposer ? ( +
+ + +
+ ) : null}
+ {showComposer ? ( +
+ ) : null}
{showComposer ? ( -
- - +
= {}): Iss currentStageType: "review", currentParticipant: { type: "agent", agentId: "agent-1", userId: null }, returnAssignee: { type: "agent", agentId: "agent-2", userId: null }, + reviewRequest: null, completedStageIds: [], lastDecisionId: null, lastDecisionOutcome: "changes_requested", diff --git a/ui/src/components/IssuesList.test.tsx b/ui/src/components/IssuesList.test.tsx index 52f6f42f..d6219038 100644 --- a/ui/src/components/IssuesList.test.tsx +++ b/ui/src/components/IssuesList.test.tsx @@ -22,6 +22,8 @@ const mockIssuesApi = vi.hoisted(() => ({ listLabels: vi.fn(), })); +const mockKanbanBoard = vi.hoisted(() => vi.fn()); + const mockAuthApi = vi.hoisted(() => ({ getSession: vi.fn(), })); @@ -87,7 +89,16 @@ vi.mock("./IssueRow", () => ({ })); vi.mock("./KanbanBoard", () => ({ - KanbanBoard: () => null, + KanbanBoard: (props: { issues: Issue[] }) => { + mockKanbanBoard(props); + return ( +
+ {props.issues.map((issue) => ( + {issue.title} + ))} +
+ ); + }, })); // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -189,6 +200,7 @@ describe("IssuesList", () => { container = document.createElement("div"); document.body.appendChild(container); dialogState.openNewIssue.mockReset(); + mockKanbanBoard.mockReset(); mockIssuesApi.list.mockReset(); mockIssuesApi.listLabels.mockReset(); mockAuthApi.getSession.mockReset(); @@ -404,6 +416,113 @@ describe("IssuesList", () => { }); }); + it("loads board issues with a separate result limit for each status column", async () => { + localStorage.setItem( + "paperclip:test-issues:company-1", + JSON.stringify({ viewMode: "board" }), + ); + + const parentIssue = createIssue({ + id: "issue-parent-total-limit", + title: "Parent total-limited issue", + status: "todo", + }); + const backlogIssue = createIssue({ + id: "issue-backlog", + title: "Backlog column issue", + status: "backlog", + }); + const doneIssue = createIssue({ + id: "issue-done", + title: "Done column issue", + status: "done", + }); + + mockIssuesApi.list.mockImplementation((_companyId, filters) => { + if (filters?.status === "backlog") return Promise.resolve([backlogIssue]); + if (filters?.status === "done") return Promise.resolve([doneIssue]); + return Promise.resolve([]); + }); + + const { root } = renderWithQueryClient( + undefined} + />, + container, + ); + + await waitForAssertion(() => { + expect(mockIssuesApi.list).toHaveBeenCalledWith("company-1", expect.objectContaining({ + status: "backlog", + limit: 200, + includeRoutineExecutions: true, + })); + expect(mockIssuesApi.list).toHaveBeenCalledWith("company-1", expect.objectContaining({ + status: "done", + limit: 200, + includeRoutineExecutions: true, + })); + expect(mockKanbanBoard).toHaveBeenLastCalledWith(expect.objectContaining({ + issues: expect.arrayContaining([ + expect.objectContaining({ id: "issue-backlog" }), + expect.objectContaining({ id: "issue-done" }), + ]), + })); + expect(container.textContent).toContain("Backlog column issue"); + expect(container.textContent).toContain("Done column issue"); + expect(container.textContent).not.toContain("Parent total-limited issue"); + }); + + act(() => { + root.unmount(); + }); + }); + + it("shows a refinement hint when a board column hits its server cap", async () => { + localStorage.setItem( + "paperclip:test-issues:company-1", + JSON.stringify({ viewMode: "board" }), + ); + + const cappedBacklogIssues = Array.from({ length: 200 }, (_, index) => + createIssue({ + id: `issue-backlog-${index + 1}`, + identifier: `PAP-${index + 1}`, + title: `Backlog issue ${index + 1}`, + status: "backlog", + }), + ); + + mockIssuesApi.list.mockImplementation((_companyId, filters) => { + if (filters?.status === "backlog") return Promise.resolve(cappedBacklogIssues); + return Promise.resolve([]); + }); + + const { root } = renderWithQueryClient( + undefined} + />, + container, + ); + + await waitForAssertion(() => { + expect(container.textContent).toContain("Some board columns are showing up to 200 issues. Refine filters or search to reveal the rest."); + }); + + act(() => { + root.unmount(); + }); + }); + it("caps the first paint for large issue lists", async () => { const manyIssues = Array.from({ length: 220 }, (_, index) => createIssue({ diff --git a/ui/src/components/IssuesList.tsx b/ui/src/components/IssuesList.tsx index a9bc2382..3a63a92a 100644 --- a/ui/src/components/IssuesList.tsx +++ b/ui/src/components/IssuesList.tsx @@ -1,5 +1,5 @@ import { startTransition, useDeferredValue, useEffect, useMemo, useState, useCallback, useRef } from "react"; -import { useQuery } from "@tanstack/react-query"; +import { useQueries, useQuery } from "@tanstack/react-query"; import { accessApi } from "../api/access"; import { useDialog } from "../context/DialogContext"; import { useCompany } from "../context/CompanyContext"; @@ -57,12 +57,14 @@ import { KanbanBoard } from "./KanbanBoard"; import { buildIssueTree, countDescendants } from "../lib/issue-tree"; import { buildSubIssueDefaultsForViewer } from "../lib/subIssueDefaults"; import { statusBadge } from "../lib/status-colors"; -import type { Issue, Project } from "@paperclipai/shared"; +import { ISSUE_STATUSES, type Issue, type Project } from "@paperclipai/shared"; const ISSUE_SEARCH_DEBOUNCE_MS = 250; const ISSUE_SEARCH_RESULT_LIMIT = 200; +const ISSUE_BOARD_COLUMN_RESULT_LIMIT = 200; const INITIAL_ISSUE_ROW_RENDER_LIMIT = 150; const ISSUE_ROW_RENDER_BATCH_SIZE = 150; const ISSUE_ROW_RENDER_BATCH_DELAY_MS = 0; +const boardIssueStatuses = ISSUE_STATUSES; /* ── View state ── */ @@ -175,6 +177,15 @@ function sortIssues(issues: Issue[], state: IssueViewState): Issue[] { return sorted; } +function issueMatchesLocalSearch(issue: Issue, normalizedSearch: string): boolean { + if (!normalizedSearch) return true; + return [ + issue.identifier, + issue.title, + issue.description, + ].some((value) => value?.toLowerCase().includes(normalizedSearch)); +} + /* ── Component ── */ interface Agent { @@ -206,6 +217,7 @@ interface IssuesListProps { initialWorkspaces?: string[]; initialSearch?: string; searchFilters?: Omit; + searchWithinLoadedIssues?: boolean; baseCreateIssueDefaults?: Record; createIssueLabel?: string; enableRoutineVisibilityFilter?: boolean; @@ -291,6 +303,7 @@ export function IssuesList({ initialWorkspaces, initialSearch, searchFilters, + searchWithinLoadedIssues = false, baseCreateIssueDefaults, createIssueLabel, enableRoutineVisibilityFilter = false, @@ -391,9 +404,34 @@ export function IssuesList({ ...searchFilters, ...(enableRoutineVisibilityFilter ? { includeRoutineExecutions: true } : {}), }), - enabled: !!selectedCompanyId && normalizedIssueSearch.length > 0, + enabled: !!selectedCompanyId && normalizedIssueSearch.length > 0 && !searchWithinLoadedIssues, placeholderData: (previousData) => previousData, }); + const boardIssueQueries = useQueries({ + queries: boardIssueStatuses.map((status) => ({ + queryKey: [ + ...queryKeys.issues.list(selectedCompanyId ?? "__no-company__"), + "board-column", + status, + normalizedIssueSearch, + projectId ?? "__all-projects__", + searchFilters ?? {}, + ISSUE_BOARD_COLUMN_RESULT_LIMIT, + enableRoutineVisibilityFilter ? "with-routine-executions" : "without-routine-executions", + ], + queryFn: () => + issuesApi.list(selectedCompanyId!, { + ...searchFilters, + ...(normalizedIssueSearch.length > 0 ? { q: normalizedIssueSearch } : {}), + projectId, + status, + limit: ISSUE_BOARD_COLUMN_RESULT_LIMIT, + ...(enableRoutineVisibilityFilter ? { includeRoutineExecutions: true } : {}), + }), + enabled: !!selectedCompanyId && viewState.viewMode === "board" && !searchWithinLoadedIssues, + placeholderData: (previousData: Issue[] | undefined) => previousData, + })), + }); const { data: executionWorkspaces = [] } = useQuery({ queryKey: selectedCompanyId ? queryKeys.executionWorkspaces.summaryList(selectedCompanyId) @@ -570,11 +608,36 @@ export function IssuesList({ return map; }, [issues]); + const boardIssues = useMemo(() => { + if (viewState.viewMode !== "board" || searchWithinLoadedIssues) return null; + const merged = new Map(); + let isPending = false; + for (const query of boardIssueQueries) { + isPending ||= query.isPending; + for (const issue of query.data ?? []) { + merged.set(issue.id, issue); + } + } + if (merged.size > 0) return [...merged.values()]; + return isPending ? issues : []; + }, [boardIssueQueries, issues, searchWithinLoadedIssues, viewState.viewMode]); + const boardColumnLimitReached = useMemo( + () => + viewState.viewMode === "board" && + !searchWithinLoadedIssues && + boardIssueQueries.some((query) => (query.data?.length ?? 0) === ISSUE_BOARD_COLUMN_RESULT_LIMIT), + [boardIssueQueries, searchWithinLoadedIssues, viewState.viewMode], + ); + const filtered = useMemo(() => { - const sourceIssues = normalizedIssueSearch.length > 0 ? searchedIssues : issues; - const filteredByControls = applyIssueFilters(sourceIssues, viewState, currentUserId, enableRoutineVisibilityFilter); + const useRemoteSearch = normalizedIssueSearch.length > 0 && !searchWithinLoadedIssues; + const sourceIssues = boardIssues ?? (useRemoteSearch ? searchedIssues : issues); + const searchScopedIssues = normalizedIssueSearch.length > 0 && searchWithinLoadedIssues + ? sourceIssues.filter((issue) => issueMatchesLocalSearch(issue, normalizedIssueSearch)) + : sourceIssues; + const filteredByControls = applyIssueFilters(searchScopedIssues, viewState, currentUserId, enableRoutineVisibilityFilter); return sortIssues(filteredByControls, viewState); - }, [issues, searchedIssues, viewState, normalizedIssueSearch, currentUserId, enableRoutineVisibilityFilter]); + }, [boardIssues, issues, searchedIssues, searchWithinLoadedIssues, viewState, normalizedIssueSearch, currentUserId, enableRoutineVisibilityFilter]); const { data: labels } = useQuery({ queryKey: queryKeys.issues.labels(selectedCompanyId!), @@ -872,11 +935,16 @@ export function IssuesList({ {isLoading && } {error &&

{error.message}

} - {normalizedIssueSearch.length > 0 && searchedIssues.length === ISSUE_SEARCH_RESULT_LIMIT && ( + {!searchWithinLoadedIssues && normalizedIssueSearch.length > 0 && searchedIssues.length === ISSUE_SEARCH_RESULT_LIMIT && (

Showing up to {ISSUE_SEARCH_RESULT_LIMIT} matches. Refine the search to narrow further.

)} + {boardColumnLimitReached && ( +

+ Some board columns are showing up to {ISSUE_BOARD_COLUMN_RESULT_LIMIT} issues. Refine filters or search to reveal the rest. +

+ )} {!isLoading && filtered.length === 0 && viewState.viewMode === "list" && ( { expect(countDescendants("nonexistent", childMap)).toBe(0); }); }); + +describe("filterIssueDescendants", () => { + it("returns only children and deeper descendants of the requested root", () => { + const root = makeIssue("root"); + const child = makeIssue("child", "root"); + const grandchild = makeIssue("grandchild", "child"); + const unrelatedParent = makeIssue("other"); + const unrelatedChild = makeIssue("other-child", "other"); + + expect(filterIssueDescendants("root", [ + root, + child, + grandchild, + unrelatedParent, + unrelatedChild, + ]).map((issue) => issue.id)).toEqual(["child", "grandchild"]); + }); + + it("handles stale broad issue-list responses without requiring the root in the list", () => { + const child = makeIssue("child", "root"); + const grandchild = makeIssue("grandchild", "child"); + const globalIssue = makeIssue("global"); + + expect(filterIssueDescendants("root", [ + globalIssue, + child, + grandchild, + ]).map((issue) => issue.id)).toEqual(["child", "grandchild"]); + }); +}); diff --git a/ui/src/lib/issue-tree.ts b/ui/src/lib/issue-tree.ts index e88709ed..25707053 100644 --- a/ui/src/lib/issue-tree.ts +++ b/ui/src/lib/issue-tree.ts @@ -34,3 +34,39 @@ export function countDescendants(id: string, childMap: Map): nu const children = childMap.get(id) ?? []; return children.reduce((sum, c) => sum + 1 + countDescendants(c.id, childMap), 0); } + +/** + * Filters a flat issue list to only descendants of `rootId`. + * + * This is intentionally useful even when the list contains unrelated issues: + * stale servers may ignore newer descendant-scoped query params, and the UI + * must still avoid rendering global issue data in a sub-issue panel. + */ +export function filterIssueDescendants(rootId: string, items: Issue[]): Issue[] { + const childrenByParentId = new Map(); + for (const item of items) { + if (!item.parentId) continue; + const siblings = childrenByParentId.get(item.parentId) ?? []; + siblings.push(item); + childrenByParentId.set(item.parentId, siblings); + } + + const descendants: Issue[] = []; + const seen = new Set([rootId]); + let frontier = [rootId]; + + while (frontier.length > 0) { + const nextFrontier: string[] = []; + for (const parentId of frontier) { + for (const child of childrenByParentId.get(parentId) ?? []) { + if (seen.has(child.id)) continue; + seen.add(child.id); + descendants.push(child); + nextFrontier.push(child.id); + } + } + frontier = nextFrontier; + } + + return descendants; +} diff --git a/ui/src/lib/queryKeys.ts b/ui/src/lib/queryKeys.ts index c345b87a..c61b216a 100644 --- a/ui/src/lib/queryKeys.ts +++ b/ui/src/lib/queryKeys.ts @@ -41,6 +41,8 @@ export const queryKeys = { ["issues", companyId, "project", projectId] as const, listByParent: (companyId: string, parentId: string) => ["issues", companyId, "parent", parentId] as const, + listByDescendantRoot: (companyId: string, rootIssueId: string) => + ["issues", companyId, "descendants", rootIssueId] as const, listByExecutionWorkspace: (companyId: string, executionWorkspaceId: string) => ["issues", companyId, "execution-workspace", executionWorkspaceId] as const, detail: (id: string) => ["issues", "detail", id] as const, diff --git a/ui/src/pages/IssueDetail.test.tsx b/ui/src/pages/IssueDetail.test.tsx index 7fbd28e2..bd47590b 100644 --- a/ui/src/pages/IssueDetail.test.tsx +++ b/ui/src/pages/IssueDetail.test.tsx @@ -850,8 +850,8 @@ describe("IssueDetail", () => { }; mockIssuesApi.get.mockResolvedValue(createIssue()); - mockIssuesApi.list.mockImplementation((_companyId, filters?: { parentId?: string }) => - Promise.resolve(filters?.parentId === "issue-1" ? [childIssue] : []), + mockIssuesApi.list.mockImplementation((_companyId, filters?: { descendantOf?: string }) => + Promise.resolve(filters?.descendantOf === "issue-1" ? [childIssue] : []), ); mockIssuesApi.getTreeControlState.mockImplementation(() => Promise.resolve({ activePauseHold: activePauseHoldState }), @@ -937,8 +937,8 @@ describe("IssueDetail", () => { }); mockIssuesApi.get.mockResolvedValue(createIssue()); - mockIssuesApi.list.mockImplementation((_companyId, filters?: { parentId?: string }) => - Promise.resolve(filters?.parentId === "issue-1" ? [childIssue] : []), + mockIssuesApi.list.mockImplementation((_companyId, filters?: { descendantOf?: string }) => + Promise.resolve(filters?.descendantOf === "issue-1" ? [childIssue] : []), ); mockIssuesApi.previewTreeControl.mockResolvedValue(pausePreview); mockIssuesApi.createTreeHold.mockResolvedValue({ hold: pauseHold, preview: pausePreview }); @@ -1031,8 +1031,8 @@ describe("IssueDetail", () => { }); mockIssuesApi.get.mockResolvedValue(createIssue()); - mockIssuesApi.list.mockImplementation((_companyId, filters?: { parentId?: string }) => - Promise.resolve(filters?.parentId === "issue-1" ? [childIssue] : []), + mockIssuesApi.list.mockImplementation((_companyId, filters?: { descendantOf?: string }) => + Promise.resolve(filters?.descendantOf === "issue-1" ? [childIssue] : []), ); mockIssuesApi.listTreeHolds.mockImplementation((_issueId, filters?: { mode?: string }) => Promise.resolve(filters?.mode === "cancel" ? [cancelHold] : []), @@ -1106,8 +1106,8 @@ describe("IssueDetail", () => { }); mockIssuesApi.get.mockResolvedValue(createIssue()); - mockIssuesApi.list.mockImplementation((_companyId, filters?: { parentId?: string }) => - Promise.resolve(filters?.parentId === "issue-1" ? [childIssue] : []), + mockIssuesApi.list.mockImplementation((_companyId, filters?: { descendantOf?: string }) => + Promise.resolve(filters?.descendantOf === "issue-1" ? [childIssue] : []), ); mockIssuesApi.previewTreeControl.mockResolvedValue(createCancelPreview(24)); mockAuthApi.getSession.mockResolvedValue({ diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index a6e18300..a4345e0a 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -98,6 +98,7 @@ import { Textarea } from "@/components/ui/textarea"; import { formatIssueActivityAction } from "@/lib/activity-format"; import { buildIssuePropertiesPanelKey } from "../lib/issue-properties-panel-key"; import { shouldRenderRichSubIssuesSection } from "../lib/issue-detail-subissues"; +import { filterIssueDescendants } from "../lib/issue-tree"; import { buildSubIssueDefaultsForViewer } from "../lib/subIssueDefaults"; import { Activity as ActivityIcon, @@ -1132,9 +1133,9 @@ export function IssueDetail() { const { data: rawChildIssues = [], isLoading: childIssuesLoading } = useQuery({ queryKey: issue?.id && resolvedCompanyId - ? queryKeys.issues.listByParent(resolvedCompanyId, issue.id) + ? queryKeys.issues.listByDescendantRoot(resolvedCompanyId, issue.id) : ["issues", "parent", "pending"], - queryFn: () => issuesApi.list(resolvedCompanyId!, { parentId: issue!.id }), + queryFn: () => issuesApi.list(resolvedCompanyId!, { descendantOf: issue!.id }), enabled: !!resolvedCompanyId && !!issue?.id, placeholderData: keepPreviousDataForSameQueryTail(issue?.id ?? "pending"), }); @@ -1286,8 +1287,11 @@ export function IssueDetail() { [issue?.project, issue?.projectId, orderedProjects], ); const childIssues = useMemo( - () => [...rawChildIssues].sort((a, b) => new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime()), - [rawChildIssues], + () => { + const descendants = issue?.id ? filterIssueDescendants(issue.id, rawChildIssues) : rawChildIssues; + return [...descendants].sort((a, b) => new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime()); + }, + [issue?.id, rawChildIssues], ); const liveIssueIds = useMemo(() => collectLiveIssueIds(companyLiveRuns), [companyLiveRuns]); const issuePanelKey = useMemo( @@ -3133,7 +3137,8 @@ export function IssueDetail() { projectId={issue.projectId ?? undefined} viewStateKey={`paperclip:issue-detail:${issue.id}:subissues-view`} issueLinkState={resolvedIssueDetailState ?? location.state} - searchFilters={{ parentId: issue.id }} + searchFilters={{ descendantOf: issue.id }} + searchWithinLoadedIssues baseCreateIssueDefaults={buildSubIssueDefaultsForViewer(issue, currentUserId)} createIssueLabel="Sub-issue" onUpdateIssue={handleChildIssueUpdate}