Files
PaperClipAI/server/src/__tests__/helpers/route-test-harness.ts
2ebc236b08 fix(server): accept parentIssueId alias in GET /issues (#4032)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Agents coordinate through the server API. They find sub-tasks by
filtering the company issues list by parent.
> - `GET /api/companies/:companyId/issues` accepts `?parentId=`. Many
callers send `?parentIssueId=` instead, which the handler never read.
> - The mismatch is silent. The filter is dropped and the full company
list comes back, so agents fetch everything and filter client-side.
Issue #3846 reports this.
> - `parentIssueId` is not an arbitrary spelling. It is the field name
the wakeup payloads in this same route file already use, so callers
expect it.
> - This pull request accepts `parentIssueId` as an alias for `parentId`
at the route boundary, on both the issues list and `issues/count`.
> - The benefit is that parent filtering works for both spellings, and
the list and its count cannot disagree.

## Linked Issues or Issue Description

Fixes #3846

Related: #3870 proposes the same alias for the list route.

## What Changed

- `server/src/routes/issues.ts`: `listFilters.parentId` in `GET
/companies/:companyId/issues` now reads `req.query.parentId ??
req.query.parentIssueId`.
- `server/src/routes/issues.ts`: `blockedCountFilters.parentId` in `GET
/companies/:companyId/issues/count` reads the same alias, so the list
and its count agree.
- `server/src/__tests__/issues-parent-id-alias.test.ts`: new regression
test for alias resolution, precedence, and absence.

## Verification

- Run `pnpm run test:run --
server/src/__tests__/issues-parent-id-alias.test.ts`.
- The test covers four query shapes: `?parentId=`, `?parentIssueId=`,
both present (short form wins), and neither present (filter unset).
- Existing callers are unaffected. The UI client `ui/src/api/issues.ts`
only sets `parentId`. Nullish coalescing falls back only when the
primary key is absent.
- The service layer applies the filter with `if (filters?.parentId)` in
`server/src/services/issues.ts`. This pull request does not change it.

## Risks

- Low risk. The change only widens accepted query input. Both spellings
resolve, and the short form still wins.
- `?parentId=` with an empty value stays falsy and unfiltered, exactly
as before.
- This route has no validation middleware, and these list filters are
not in the published OpenAPI surface. No contract needs an update.

## Model Used

- Claude Opus 5 (`claude-opus-5`), extended thinking with tool use, run
by the maintainer's triage agent. It rebased the original commit onto
current `master`, extended the alias to `issues/count`, and wrote the
regression test. @scokeepa authored the original one-line route change.

## 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] My branch name describes the change and contains no internal
Paperclip ticket id or instance-derived details
- [ ] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [ ] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: josangmun <cmeia.ai02@cmeia.co.kr>
Co-authored-by: Andrew Aymeloglu <aaymeloglu@gmail.com>
2026-08-04 20:45:42 -05:00

142 lines
4.1 KiB
TypeScript

import { randomUUID } from "node:crypto";
import express, { type Router } from "express";
import { isNotNull } from "drizzle-orm";
import {
companies,
companyMemberships,
createDb,
issues,
principalPermissionGrants,
} from "@paperclipai/db";
import { afterAll, afterEach, beforeAll, describe } from "vitest";
import { errorHandler } from "../../middleware/index.js";
import { ensureHumanRoleDefaultGrants } from "../../services/principal-access-compatibility.js";
import {
getEmbeddedPostgresTestSupport,
startEmbeddedPostgresTestDatabase,
} from "./embedded-postgres.js";
type Db = ReturnType<typeof createDb>;
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
/** `describe` on hosts that can run embedded Postgres, `describe.skip` elsewhere. */
export const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
export type EmbeddedPostgresContext = { readonly db: Db };
/**
* Registers the embedded Postgres lifecycle for the enclosing `describe`. Read
* `.db` from inside a test; it is unavailable until `beforeAll` has run.
*/
export function useEmbeddedPostgres(
prefix: string,
opts: { resetEach?: (db: Db) => Promise<void> } = {},
): EmbeddedPostgresContext {
let db: Db | null = null;
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
beforeAll(async () => {
tempDb = await startEmbeddedPostgresTestDatabase(prefix);
db = createDb(tempDb.connectionString);
}, 20_000);
if (opts.resetEach) {
const resetEach = opts.resetEach;
afterEach(async () => {
if (db) await resetEach(db);
});
}
afterAll(async () => {
await tempDb?.cleanup();
tempDb = null;
db = null;
});
return {
get db() {
if (!db) throw new Error("embedded Postgres is not started yet — read .db inside a test");
return db;
},
};
}
export type BoardActor = {
type: "board";
source: string;
userId: string;
companyIds: string[];
memberships: { companyId: string; membershipRole: string; status: string }[];
isInstanceAdmin: boolean;
};
export type SeededCompany = { companyId: string; userId: string; actor: BoardActor };
/** Creates a company whose board user holds company-scoped read and write grants. */
export async function seedCompanyWithBoardAccess(db: Db, name: string): Promise<SeededCompany> {
const companyId = randomUUID();
const userId = `user-${randomUUID()}`;
await db.insert(companies).values({
id: companyId,
name: `${name} ${companyId}`,
issuePrefix: `T${companyId.replaceAll("-", "").slice(0, 6).toUpperCase()}`,
requireBoardApprovalForNewAgents: false,
});
await db.insert(companyMemberships).values({
companyId,
principalType: "user",
principalId: userId,
status: "active",
membershipRole: "owner",
updatedAt: new Date(),
});
await ensureHumanRoleDefaultGrants(db, {
companyId,
principalId: userId,
membershipRole: "owner",
grantedByUserId: null,
});
return {
companyId,
userId,
actor: {
type: "board",
source: "session",
userId,
companyIds: [companyId],
memberships: [{ companyId, membershipRole: "owner", status: "active" }],
isInstanceAdmin: false,
},
};
}
type RouterFactory = (db: Db, storage: never) => Router;
/** Mounts route modules under `/api` behind a fixed actor, with the real error handler. */
export function routeApp(db: Db, actor: BoardActor, ...routerFactories: RouterFactory[]) {
const app = express();
app.use(express.json());
app.use((req, _res, next) => {
(req as any).actor = actor;
next();
});
for (const factory of routerFactories) {
app.use("/api", factory(db, {} as never));
}
app.use(errorHandler);
return app;
}
/**
* Clears the company and issue fixtures seeded by `seedCompanyWithBoardAccess`.
* Suites that seed other tables need their own reset.
*/
export async function resetCompanyIssueFixtures(db: Db) {
await db.delete(issues).where(isNotNull(issues.parentId));
await db.delete(issues);
await db.delete(principalPermissionGrants);
await db.delete(companyMemberships);
await db.delete(companies);
}