From 3b4bf0bd8753a5528e123ec398cad184582475e7 Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 11 Sep 2026 12:16:48 -0500 Subject: [PATCH] fix: use stable cursors for related task pagination Co-Authored-By: Paperclip --- doc/SPEC-implementation.md | 1 + .../issue-created-from-routes.test.ts | 24 +++++++++++++++++++ server/src/routes/issues.ts | 7 +++--- server/src/services/issues.ts | 14 ++++++++++- ui/src/api/issues.test.ts | 4 ++-- ui/src/api/issues.ts | 12 ++++++---- .../originating-tasks/OriginatingTasks.tsx | 6 ++++- .../prototypes/originating-tasks/README.md | 3 ++- 8 files changed, 59 insertions(+), 12 deletions(-) diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 5caa6747b7..d8c652c173 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -248,6 +248,7 @@ See `doc/project-repositories.md` for the API and UI contract. - identifier fields: `issue_number`, `identifier` - origin fields: `origin_kind`, `origin_id`, `origin_run_id`, `origin_fingerprint` - Creation stores the actor run in `origin_run_id` unless an explicit origin run is supplied. `GET /api/companies/:companyId/issues?createdFromIssueId=` selects tasks created by runs bound to that source task, using native run issue identity or persisted legacy task context. Historical rows without an origin run may use their recorded creation activity; comments and shared creators do not establish provenance. Source, run, activity and result are company-scoped. +- Relation lists can use `sortField=id&sortDir=asc&afterId=` for stable pagination. The cursor excludes earlier IDs and cannot be combined with an offset or activity-based order. - The streamlined task page's Tasks tab keeps two independent memberships: the existing subtask tree, and created tasks grouped by their current project (or No project). A created subtask appears in both. Only Subtasks has completion progress; groups collapse independently and unfinished tasks sort above finished tasks. - `request_depth` int not null default 0 - `work_mode` text not null default `standard`; supported values: diff --git a/server/src/__tests__/issue-created-from-routes.test.ts b/server/src/__tests__/issue-created-from-routes.test.ts index 1c09c1b1ba..c694460a3c 100644 --- a/server/src/__tests__/issue-created-from-routes.test.ts +++ b/server/src/__tests__/issue-created-from-routes.test.ts @@ -1,5 +1,6 @@ import { randomUUID } from "node:crypto"; import express from "express"; +import { eq } from "drizzle-orm"; import request from "supertest"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { activityLog, agents, companies, createDb, heartbeatRuns, issues, projects } from "@paperclipai/db"; @@ -105,6 +106,29 @@ describePostgres("tasks created from an issue", () => { } }); + it("keeps cursor pages complete when activity changes or an earlier task disappears", async () => { + const first = await request(app()).get(`/api/companies/${companyId}/issues`).query({ createdFromIssueId: sourceId, sortField: "id", sortDir: "asc", limit: 2 }); + expect(first.status).toBe(200); + const sortedIds = [...expected].sort(); + expect(first.body.map((row: { id: string }) => row.id)).toEqual(sortedIds.slice(0, 2)); + await db.update(issues).set({ updatedAt: new Date("2099-01-01"), priority: "critical" }).where(eq(issues.id, sortedIds.at(-1)!)); + await db.update(issues).set({ hiddenAt: new Date() }).where(eq(issues.id, sortedIds[0]!)); + try { + const remaining = await request(app()).get(`/api/companies/${companyId}/issues`).query({ createdFromIssueId: sourceId, sortField: "id", sortDir: "asc", afterId: first.body.at(-1).id, limit: 500 }); + expect(remaining.status).toBe(200); + expect(remaining.body.map((row: { id: string }) => row.id)).toEqual(sortedIds.slice(2)); + } finally { + await db.update(issues).set({ hiddenAt: null }).where(eq(issues.id, sortedIds[0]!)); + } + }); + + it("rejects cursors without matching ID order", async () => { + for (const query of [{ afterId: sourceId }, { afterId: "bad", sortField: "id", sortDir: "asc" }, { afterId: sourceId, sortField: "id", sortDir: "desc" }, { afterId: sourceId, sortField: "id", sortDir: "asc", offset: 2 }]) { + const result = await request(app()).get(`/api/companies/${companyId}/issues`).query(query); + expect(result.status).toBe(422); + } + }); + it("does not include manual children in creation provenance", async () => { const children = await issueService(db).list(companyId, { parentId: sourceId }); expect(children.map((row) => row.title)).toContain("Manual child"); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index baa4d74320..e587b51a2e 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -7847,10 +7847,10 @@ export function issueRoutes( res.status(400).json({ error: "offset must be a non-negative integer" }); return; } - if (sortField !== undefined && sortField !== "updated") { + if (sortField !== undefined && sortField !== "updated" && sortField !== "id") { res .status(400) - .json({ error: "sortField must be 'updated' when provided" }); + .json({ error: "sortField must be 'updated' or 'id' when provided" }); return; } if (sortDir !== undefined && sortDir !== "asc" && sortDir !== "desc") { @@ -7945,7 +7945,8 @@ export function issueRoutes( q: req.query.q as string | undefined, limit, offset, - sortField: sortField === "updated" ? "updated" : undefined, + sortField: sortField === "updated" || sortField === "id" ? sortField : undefined, + afterId: req.query.afterId as string | undefined, sortDir: sortDir === "asc" || sortDir === "desc" ? sortDir : undefined, updatedSince: rawUpdatedSince, }; diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index 81c9bea29f..1197e3fbae 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -1765,7 +1765,8 @@ export interface IssueFilters { q?: string; limit?: number; offset?: number; - sortField?: "updated"; + sortField?: "updated" | "id"; + afterId?: string; sortDir?: "asc" | "desc"; /** ISO 8601 timestamp — only return issues with updatedAt strictly after this value. */ updatedSince?: string; @@ -3114,6 +3115,7 @@ function issueListOrderBy( sortDir?: IssueFilters["sortDir"]; }, ) { + if (sortField === "id") return [sortDir === "desc" ? desc(issues.id) : asc(issues.id)]; const canonicalLastActivityAt = issueCanonicalLastActivityAtExpr(companyId); if (sortField === "updated") { const activityOrder = @@ -7878,6 +7880,15 @@ export function issueService(db: Db) { addStopRelayCommentIfNeeded, list: async (companyId: string, filters?: IssueFilters) => { + if (filters?.sortField === "id" && filters.attention) { + throw unprocessable("ID ordering is not supported for blocked attention lists"); + } + if (filters?.afterId !== undefined && ( + !isUuidLike(filters.afterId) || filters.sortField !== "id" || + filters.sortDir !== "asc" || (filters.offset ?? 0) !== 0 + )) { + throw unprocessable("afterId requires a UUID, ascending ID order and no offset"); + } if (filters?.attention === "blocked") { return listBlockedInboxIssues(db, companyId, { ...filters, @@ -7890,6 +7901,7 @@ export function issueService(db: Db) { eq(issues.companyId, companyId), visibleIssueCondition(), ]; + if (filters?.afterId) conditions.push(gt(issues.id, filters.afterId)); const assigneeAgentFilter = parseIssueAssigneeAgentFilter( filters?.assigneeAgentId, ); diff --git a/ui/src/api/issues.test.ts b/ui/src/api/issues.test.ts index 38b928aa17..3d78406b57 100644 --- a/ui/src/api/issues.test.ts +++ b/ui/src/api/issues.test.ts @@ -34,8 +34,8 @@ describe("issuesApi.list", () => { mockApi.get.mockResolvedValueOnce(firstPage).mockResolvedValueOnce([{ id: "last-task" }]); const result = await issuesApi.listAll("company-1", { createdFromIssueId: "source-1" }); expect(result).toHaveLength(501); - expect(mockApi.get).toHaveBeenNthCalledWith(1, "/companies/company-1/issues?createdFromIssueId=source-1&limit=500&offset=0"); - expect(mockApi.get).toHaveBeenNthCalledWith(2, "/companies/company-1/issues?createdFromIssueId=source-1&limit=500&offset=500"); + expect(mockApi.get).toHaveBeenNthCalledWith(1, "/companies/company-1/issues?createdFromIssueId=source-1&limit=500&sortField=id&sortDir=asc"); + expect(mockApi.get).toHaveBeenNthCalledWith(2, "/companies/company-1/issues?createdFromIssueId=source-1&limit=500&sortField=id&sortDir=asc&afterId=task-499"); }); it("passes parentId through to the company issues endpoint", async () => { diff --git a/ui/src/api/issues.ts b/ui/src/api/issues.ts index b8f6f6edde..26110545d8 100644 --- a/ui/src/api/issues.ts +++ b/ui/src/api/issues.ts @@ -105,7 +105,8 @@ export type IssueListFilters = { q?: string; limit?: number; offset?: number; - sortField?: "updated"; + sortField?: "updated" | "id"; + afterId?: string; sortDir?: "asc" | "desc"; }; @@ -153,18 +154,21 @@ function issueListSearchParams(filters?: IssueListFilters) { params.set("offset", String(filters.offset)); if (filters?.sortField) params.set("sortField", filters.sortField); if (filters?.sortDir) params.set("sortDir", filters.sortDir); + if (filters?.afterId) params.set("afterId", filters.afterId); return params; } export const issuesApi = { /** Fetch every page for bounded task-detail relations, not just the default first page. */ - listAll: async (companyId: string, filters: Omit, options?: RequestOptions): Promise => { + listAll: async (companyId: string, filters: Omit, options?: RequestOptions): Promise => { const pageSize = 500; const tasks = new Map(); - for (let offset = 0; ; offset += pageSize) { - const page = await issuesApi.list(companyId, { ...filters, limit: pageSize, offset }, options); + let afterId: string | undefined; + for (;;) { + const page = await issuesApi.list(companyId, { ...filters, limit: pageSize, sortField: "id", sortDir: "asc", afterId }, options); for (const task of page) tasks.set(task.id, task); if (page.length < pageSize) return [...tasks.values()]; + afterId = page[page.length - 1]!.id; } }, list: ( diff --git a/ui/storybook/prototypes/originating-tasks/OriginatingTasks.tsx b/ui/storybook/prototypes/originating-tasks/OriginatingTasks.tsx index 34d97caf5b..9319598ab1 100644 --- a/ui/storybook/prototypes/originating-tasks/OriginatingTasks.tsx +++ b/ui/storybook/prototypes/originating-tasks/OriginatingTasks.tsx @@ -193,9 +193,13 @@ function TaskPageData({ children, scenario }: { children: React.ReactNode; scena const projectId = url.searchParams.get("projectId"); const createdFrom = url.searchParams.get("createdFromIssueId"); const filtered = rows.filter((row) => (!parent || row.parentId === parent) && (!projectId || row.projectId === projectId) && (!createdFrom || (row.originRunId && runSources.get(row.originRunId) === createdFrom))); + const afterId = url.searchParams.get("afterId"); + const ordered = url.searchParams.get("sortField") === "id" + ? filtered.sort((a, b) => a.id.localeCompare(b.id)).filter((row) => !afterId || row.id > afterId) + : filtered; const offset = Number(url.searchParams.get("offset") ?? 0); const limit = Number(url.searchParams.get("limit") ?? filtered.length); - return Response.json(filtered.slice(offset, offset + limit)); + return Response.json(ordered.slice(offset, offset + limit)); } return originalFetch(input, init); }; diff --git a/ui/storybook/prototypes/originating-tasks/README.md b/ui/storybook/prototypes/originating-tasks/README.md index 0c812507ad..8922801c0f 100644 --- a/ui/storybook/prototypes/originating-tasks/README.md +++ b/ui/storybook/prototypes/originating-tasks/README.md @@ -36,6 +36,7 @@ run was provided, including the legacy child helper. Native creation already provides its run. No schema migration or runner-mode distinction is needed. The real issue page queries this projection alongside its existing subtask query. -Existing company issue-list invalidation refreshes both on task activity. Errors +Both queries page by immutable task ID with an afterId cursor so task activity +and removal of earlier rows do not shift later pages. Existing company issue-list invalidation refreshes both on task activity. Errors remain visible with a Retry action. Historical rows without either an origin run or attributed creation activity cannot be attributed and are not included in project groups.