mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-26 18:31:34 +01:00
## 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>
142 lines
4.1 KiB
TypeScript
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);
|
|
}
|