fix(SAG-2595): land updatedSince issues-list filter on master (#9050)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The issues-list REST endpoint (`GET /api/companies/:companyId/issues`) backs the digester and other pollers that ask "what changed since last time". > - The service layer supports rich filters, but there was no `updatedSince` filter — so every routine fire re-read the full backlog instead of just the delta. > - A prior commit added this filter, but it was never merged to `master`; it only ran in production because a feature branch happened to be the live checkout, and the behavior vanished when that directory was repurposed. > - This pull request re-lands just the `updatedSince` filter (route param parse + validation, service `IssueFilters` field, and the `updatedAt` predicate) as a single-purpose change. > - The benefit is that pollers can request only issues updated after a timestamp, and the fix now lives durably on `master` instead of a transient checkout. ## Linked Issues or Issue Description No public GitHub issue exists; describing inline per the bug report template. **What happened** `GET /api/companies/:companyId/issues` ignores an `updatedSince` query parameter, so consumers (e.g. the digester and other pollers) cannot request only the delta since a prior poll and must re-read the whole backlog on every fire. **Expected behavior** Passing `updatedSince=<ISO 8601 timestamp>` returns only issues whose `updatedAt` is strictly after that timestamp; a malformed value returns `400`. **Steps to reproduce** 1. Call `GET /api/companies/:companyId/issues?updatedSince=<a future ISO 8601 timestamp>`. 2. Observe the endpoint returns the full backlog instead of an empty list (the parameter is silently ignored). ## What Changed - `server/src/routes/issues.ts`: parse the `updatedSince` query param, return `400` for a non-parseable timestamp, and pass it into `svc.list()`. - `server/src/services/issues.ts`: add `updatedSince?: string` to `IssueFilters` and, when present and valid, add a `gt(issues.updatedAt, since)` condition to the list query. - `server/src/__tests__/issue-list-updatedsince-filter-routes.test.ts`: new route+service coverage — future timestamp returns 0 issues, a past timestamp returns only the delta, and a malformed timestamp returns 400. ## Verification - `pnpm vitest run src/__tests__/issue-list-updatedsince-filter-routes.test.ts` — 3/3 pass. - `pnpm vitest run src/__tests__/issue-list-assignee-filter-routes.test.ts` — 5/5 pass (regression check on the sibling filter path). - `tsc --noEmit` on `server/` — no new errors introduced (pre-existing unrelated `plugin-sdk` build errors on `master` are untouched). ## Risks Low risk. Purely additive: the new filter only takes effect when `updatedSince` is supplied, so existing callers that omit it are unaffected. Invalid timestamps fail fast with `400` rather than silently returning all rows. ## Model Used Claude — `claude-sonnet-4-6` (implementation) with `claude-opus-4-8` review/merge-gate; tool use + code execution enabled. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots (N/A — no UI change) - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green (in progress) - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups (in progress) - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Coder (Claude) <coder-claude@paperclip.ing>
This commit is contained in:
parent
c5574599b1
commit
0db8480b19
|
|
@ -0,0 +1,176 @@
|
|||
import { randomUUID } from "node:crypto";
|
||||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import { companies, companyMemberships, createDb, issues, principalPermissionGrants } from "@paperclipai/db";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
import { errorHandler } from "../middleware/index.js";
|
||||
import { issueRoutes } from "../routes/issues.js";
|
||||
import { ensureHumanRoleDefaultGrants } from "../services/principal-access-compatibility.js";
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
||||
if (!embeddedPostgresSupport.supported) {
|
||||
console.warn(
|
||||
`Skipping embedded Postgres issue list route tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`,
|
||||
);
|
||||
}
|
||||
|
||||
describeEmbeddedPostgres("issue list routes updatedSince filter", () => {
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
|
||||
beforeAll(async () => {
|
||||
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-issue-list-routes-");
|
||||
db = createDb(tempDb.connectionString);
|
||||
}, 20_000);
|
||||
|
||||
afterEach(async () => {
|
||||
await db.delete(issues);
|
||||
await db.delete(principalPermissionGrants);
|
||||
await db.delete(companyMemberships);
|
||||
await db.delete(companies);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await tempDb?.cleanup();
|
||||
});
|
||||
|
||||
function createApp(companyId: string) {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use((req, _res, next) => {
|
||||
(req as any).actor = {
|
||||
type: "board",
|
||||
userId: "cloud-user-1",
|
||||
companyIds: [companyId],
|
||||
memberships: [{ companyId, membershipRole: "owner", status: "active" }],
|
||||
source: "cloud_tenant",
|
||||
isInstanceAdmin: false,
|
||||
};
|
||||
next();
|
||||
});
|
||||
app.use("/api", issueRoutes(db, {} as any));
|
||||
app.use(errorHandler);
|
||||
return app;
|
||||
}
|
||||
|
||||
function uniqueIssuePrefix() {
|
||||
return `P${randomUUID().replace(/-/g, "").slice(0, 4).toUpperCase()}`;
|
||||
}
|
||||
|
||||
async function seedCloudTenantMember(companyId: string) {
|
||||
await db.insert(companyMemberships).values({
|
||||
companyId,
|
||||
principalType: "user",
|
||||
principalId: "cloud-user-1",
|
||||
status: "active",
|
||||
membershipRole: "owner",
|
||||
updatedAt: new Date(),
|
||||
});
|
||||
await ensureHumanRoleDefaultGrants(db, {
|
||||
companyId,
|
||||
principalId: "cloud-user-1",
|
||||
membershipRole: "owner",
|
||||
grantedByUserId: null,
|
||||
});
|
||||
}
|
||||
|
||||
it("returns 0 issues when updatedSince is in the future", async () => {
|
||||
const companyId = randomUUID();
|
||||
const issueId = randomUUID();
|
||||
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: uniqueIssuePrefix(),
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
await seedCloudTenantMember(companyId);
|
||||
await db.insert(issues).values({
|
||||
id: issueId,
|
||||
companyId,
|
||||
title: "Existing issue",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
});
|
||||
|
||||
const futureDate = new Date(Date.now() + 24 * 60 * 60 * 1000).toISOString();
|
||||
|
||||
const app = createApp(companyId);
|
||||
const res = await request(app)
|
||||
.get(`/api/companies/${companyId}/issues`)
|
||||
.query({ updatedSince: futureDate, limit: "20" });
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||
expect(res.body).toEqual([]);
|
||||
});
|
||||
|
||||
it("returns only issues updated after a past updatedSince timestamp", async () => {
|
||||
const companyId = randomUUID();
|
||||
const staleIssueId = randomUUID();
|
||||
const freshIssueId = randomUUID();
|
||||
const since = new Date("2026-07-01T00:00:00.000Z");
|
||||
const staleUpdatedAt = new Date("2026-06-30T00:00:00.000Z");
|
||||
const freshUpdatedAt = new Date("2026-07-02T00:00:00.000Z");
|
||||
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: uniqueIssuePrefix(),
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
await seedCloudTenantMember(companyId);
|
||||
await db.insert(issues).values([
|
||||
{
|
||||
id: staleIssueId,
|
||||
companyId,
|
||||
title: "Stale issue",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
updatedAt: staleUpdatedAt,
|
||||
},
|
||||
{
|
||||
id: freshIssueId,
|
||||
companyId,
|
||||
title: "Fresh issue",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
updatedAt: freshUpdatedAt,
|
||||
},
|
||||
]);
|
||||
|
||||
const app = createApp(companyId);
|
||||
const res = await request(app)
|
||||
.get(`/api/companies/${companyId}/issues`)
|
||||
.query({ updatedSince: since.toISOString(), limit: "20" });
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||
expect(res.body.map((issue: { id: string }) => issue.id)).toEqual([freshIssueId]);
|
||||
});
|
||||
|
||||
it("returns 400 for a malformed updatedSince timestamp", async () => {
|
||||
const companyId = randomUUID();
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: uniqueIssuePrefix(),
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
await seedCloudTenantMember(companyId);
|
||||
|
||||
const app = createApp(companyId);
|
||||
const res = await request(app)
|
||||
.get(`/api/companies/${companyId}/issues`)
|
||||
.query({ updatedSince: "not-a-date", limit: "20" });
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body).toMatchObject({
|
||||
error: "updatedSince must be a valid ISO 8601 timestamp when provided",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -5364,6 +5364,7 @@ export function issueRoutes(
|
|||
const includeLiveDescendantSummary = parseOptionalBooleanQuery(req.query.includeLiveDescendantSummary);
|
||||
const assigneeAgentFilterRaw = req.query.assigneeAgentId;
|
||||
let assigneeAgentId: string | null | undefined;
|
||||
const rawUpdatedSince = req.query.updatedSince as string | undefined;
|
||||
|
||||
if (assigneeUserFilterRaw === "me" && (!assigneeUserId || req.actor.type !== "board")) {
|
||||
res.status(403).json({ error: "assigneeUserId=me requires board authentication" });
|
||||
|
|
@ -5430,6 +5431,10 @@ export function issueRoutes(
|
|||
return;
|
||||
}
|
||||
}
|
||||
if (rawUpdatedSince !== undefined && !Number.isFinite(new Date(rawUpdatedSince).getTime())) {
|
||||
res.status(400).json({ error: "updatedSince must be a valid ISO 8601 timestamp when provided" });
|
||||
return;
|
||||
}
|
||||
const offset = parsedOffset ?? 0;
|
||||
|
||||
const listFilters: IssueFilters = {
|
||||
|
|
@ -5466,6 +5471,7 @@ export function issueRoutes(
|
|||
offset,
|
||||
sortField: sortField === "updated" ? "updated" : undefined,
|
||||
sortDir: sortDir === "asc" || sortDir === "desc" ? sortDir : undefined,
|
||||
updatedSince: rawUpdatedSince,
|
||||
};
|
||||
const requestKey = issueListRequestKey({
|
||||
req,
|
||||
|
|
|
|||
|
|
@ -572,6 +572,8 @@ export interface IssueFilters {
|
|||
offset?: number;
|
||||
sortField?: "updated";
|
||||
sortDir?: "asc" | "desc";
|
||||
/** ISO 8601 timestamp — only return issues with updatedAt strictly after this value. */
|
||||
updatedSince?: string;
|
||||
}
|
||||
|
||||
type IssueRow = typeof issues.$inferSelect;
|
||||
|
|
@ -5531,6 +5533,12 @@ export function issueService(db: Db) {
|
|||
)!,
|
||||
);
|
||||
}
|
||||
if (filters?.updatedSince) {
|
||||
const since = new Date(filters.updatedSince);
|
||||
if (Number.isFinite(since.getTime())) {
|
||||
conditions.push(gt(issues.updatedAt, since));
|
||||
}
|
||||
}
|
||||
if (filters?.excludeRoutineExecutions && !filters?.originKind && !filters?.originId) {
|
||||
conditions.push(ne(issues.originKind, "routine_execution"));
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue