Fix Cloud tenant issue identifier routes (#5196)
## Summary - Allow Cloud tenant issue identifiers with alphanumeric prefixes, such as `PC1897-1`, to normalize as issue references. - Resolve those identifiers through issue detail/update routes, active run/live run polling, activity, costs, and `issueService.getById`. - Keep UI issue-link parsing aligned so tenant links normalize back to `/issues/<IDENTIFIER>`. ## Root Cause Cloud tenant issue prefixes include digits from the stack-id hash. The app-side route normalization still accepted only all-letter prefixes, so `/api/issues/PC1897-1` skipped identifier lookup and fell through as a non-UUID id. ## Verification - `pnpm exec vitest run packages/shared/src/issue-references.test.ts ui/src/lib/issue-reference.test.ts server/src/__tests__/issue-identifier-routes.test.ts server/src/__tests__/activity-routes.test.ts server/src/__tests__/costs-service.test.ts server/src/__tests__/agent-live-run-routes.test.ts server/src/__tests__/issues-service.test.ts` - `pnpm --filter @paperclipai/shared typecheck && pnpm --filter @paperclipai/server typecheck` - `git diff --check` Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -10,6 +10,7 @@ import {
|
||||
describe("issue references", () => {
|
||||
it("normalizes identifiers to uppercase", () => {
|
||||
expect(normalizeIssueIdentifier("pap-123")).toBe("PAP-123");
|
||||
expect(normalizeIssueIdentifier("pc1a2-7")).toBe("PC1A2-7");
|
||||
expect(normalizeIssueIdentifier("not-an-issue")).toBeNull();
|
||||
});
|
||||
|
||||
@@ -27,14 +28,14 @@ describe("issue references", () => {
|
||||
});
|
||||
|
||||
it("finds identifiers and issue paths in plain text", () => {
|
||||
expect(findIssueReferenceMatches("See PAP-1, /issues/PAP-2, and https://x.test/PAP/issues/pap-3.")).toEqual([
|
||||
expect(findIssueReferenceMatches("See PAP-1, /issues/PC1A2-2, and https://x.test/PAP/issues/pc1a2-3.")).toEqual([
|
||||
{ index: 4, length: 5, identifier: "PAP-1", matchedText: "PAP-1" },
|
||||
{ index: 11, length: 13, identifier: "PAP-2", matchedText: "/issues/PAP-2" },
|
||||
{ index: 11, length: 15, identifier: "PC1A2-2", matchedText: "/issues/PC1A2-2" },
|
||||
{
|
||||
index: 30,
|
||||
length: 31,
|
||||
identifier: "PAP-3",
|
||||
matchedText: "https://x.test/PAP/issues/pap-3",
|
||||
index: 32,
|
||||
length: 33,
|
||||
identifier: "PC1A2-3",
|
||||
matchedText: "https://x.test/PAP/issues/pc1a2-3",
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
export const ISSUE_REFERENCE_IDENTIFIER_RE = /^[A-Z]+-\d+$/;
|
||||
export const ISSUE_REFERENCE_IDENTIFIER_RE = /^[A-Z][A-Z0-9]*-\d+$/;
|
||||
|
||||
export interface IssueReferenceMatch {
|
||||
index: number;
|
||||
@@ -7,7 +7,7 @@ export interface IssueReferenceMatch {
|
||||
matchedText: string;
|
||||
}
|
||||
|
||||
const ISSUE_REFERENCE_TOKEN_RE = /https?:\/\/[^\s<>()]+|\/[^\s<>()]+|[A-Z]+-\d+/gi;
|
||||
const ISSUE_REFERENCE_TOKEN_RE = /https?:\/\/[^\s<>()]+|\/[^\s<>()]+|[A-Z][A-Z0-9]*-\d+/gi;
|
||||
|
||||
function preserveNewlinesAsWhitespace(value: string) {
|
||||
return value.replace(/[^\n]/g, " ");
|
||||
|
||||
@@ -128,7 +128,7 @@ describe.sequential("activity routes", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("resolves issue identifiers before loading runs", async () => {
|
||||
it("resolves alphanumeric issue identifiers before loading runs", async () => {
|
||||
mockIssueService.getByIdentifier.mockResolvedValue({
|
||||
id: "issue-uuid-1",
|
||||
companyId: "company-1",
|
||||
@@ -141,10 +141,10 @@ describe.sequential("activity routes", () => {
|
||||
]);
|
||||
|
||||
const app = await createApp();
|
||||
const res = await requestApp(app, (baseUrl) => request(baseUrl).get("/api/issues/PAP-475/runs"));
|
||||
const res = await requestApp(app, (baseUrl) => request(baseUrl).get("/api/issues/pc1a2-475/runs"));
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockIssueService.getByIdentifier).toHaveBeenCalledWith("PAP-475");
|
||||
expect(mockIssueService.getByIdentifier).toHaveBeenCalledWith("PC1A2-475");
|
||||
expect(mockIssueService.getById).not.toHaveBeenCalled();
|
||||
expect(mockActivityService.runsForIssue).toHaveBeenCalledWith("company-1", "issue-uuid-1");
|
||||
expect(res.body).toEqual([{ runId: "run-1", adapterType: "codex_local" }]);
|
||||
|
||||
@@ -215,11 +215,11 @@ describe("agent live run routes", () => {
|
||||
it("returns a compact active run payload for issue polling", async () => {
|
||||
const res = await requestApp(
|
||||
await createApp(),
|
||||
(baseUrl) => request(baseUrl).get("/api/issues/PAP-1295/active-run"),
|
||||
(baseUrl) => request(baseUrl).get("/api/issues/pc1a2-1295/active-run"),
|
||||
);
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||
expect(mockIssueService.getByIdentifier).toHaveBeenCalledWith("PAP-1295");
|
||||
expect(mockIssueService.getByIdentifier).toHaveBeenCalledWith("PC1A2-1295");
|
||||
expect(mockHeartbeatService.getRunIssueSummary).toHaveBeenCalledWith("run-1");
|
||||
expect(res.body).toMatchObject({
|
||||
id: "run-1",
|
||||
@@ -268,7 +268,7 @@ describe("agent live run routes", () => {
|
||||
|
||||
const res = await requestApp(
|
||||
await createApp(),
|
||||
(baseUrl) => request(baseUrl).get("/api/issues/PAP-1295/active-run"),
|
||||
(baseUrl) => request(baseUrl).get("/api/issues/PC1A2-1295/active-run"),
|
||||
);
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||
|
||||
@@ -178,12 +178,12 @@ beforeEach(() => {
|
||||
mockIssueService.getById.mockResolvedValue({
|
||||
id: "issue-1",
|
||||
companyId: "company-1",
|
||||
identifier: "PAP-1",
|
||||
identifier: "PC1A2-1",
|
||||
});
|
||||
mockIssueService.getByIdentifier.mockResolvedValue({
|
||||
id: "issue-1",
|
||||
companyId: "company-1",
|
||||
identifier: "PAP-1",
|
||||
identifier: "PC1A2-1",
|
||||
});
|
||||
mockBudgetService.upsertPolicy.mockResolvedValue(undefined);
|
||||
});
|
||||
@@ -227,10 +227,10 @@ describe("cost routes", () => {
|
||||
|
||||
it("returns issue subtree cost summaries for issue refs", async () => {
|
||||
const app = await createApp();
|
||||
const res = await request(app).get("/api/issues/PAP-1/cost-summary");
|
||||
const res = await request(app).get("/api/issues/pc1a2-1/cost-summary");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockIssueService.getByIdentifier).toHaveBeenCalledWith("PAP-1");
|
||||
expect(mockIssueService.getByIdentifier).toHaveBeenCalledWith("PC1A2-1");
|
||||
expect(mockCostService.issueTreeSummary).toHaveBeenCalledWith("company-1", "issue-1");
|
||||
expect(res.body).toEqual({
|
||||
issueId: "issue-1",
|
||||
|
||||
@@ -0,0 +1,105 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { companies, createDb, issues } from "@paperclipai/db";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
import { errorHandler } from "../middleware/index.js";
|
||||
import { issueRoutes } from "../routes/issues.js";
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
||||
if (!embeddedPostgresSupport.supported) {
|
||||
console.warn(
|
||||
`Skipping embedded Postgres issue identifier route tests on this host: ${embeddedPostgresSupport.reason ?? "unsupported environment"}`,
|
||||
);
|
||||
}
|
||||
|
||||
describeEmbeddedPostgres("issue identifier routes", () => {
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
|
||||
beforeAll(async () => {
|
||||
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-issue-identifier-routes-");
|
||||
db = createDb(tempDb.connectionString);
|
||||
}, 20_000);
|
||||
|
||||
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: true,
|
||||
};
|
||||
next();
|
||||
});
|
||||
app.use("/api", issueRoutes(db, {} as any));
|
||||
app.use(errorHandler);
|
||||
return app;
|
||||
}
|
||||
|
||||
it("resolves alphanumeric Cloud tenant issue identifiers for detail reads and updates", async () => {
|
||||
const companyId = randomUUID();
|
||||
const issueId = randomUUID();
|
||||
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Cloud tenant",
|
||||
issuePrefix: "PC1A2",
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
await db.insert(issues).values({
|
||||
id: issueId,
|
||||
companyId,
|
||||
issueNumber: 7,
|
||||
identifier: "PC1A2-7",
|
||||
title: "Tenant identifier route",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
createdByUserId: "cloud-user-1",
|
||||
});
|
||||
|
||||
const app = createApp(companyId);
|
||||
const read = await request(app).get("/api/issues/pc1a2-7");
|
||||
|
||||
expect(read.status, JSON.stringify(read.body)).toBe(200);
|
||||
expect(read.body).toMatchObject({
|
||||
id: issueId,
|
||||
companyId,
|
||||
identifier: "PC1A2-7",
|
||||
});
|
||||
|
||||
const updated = await request(app)
|
||||
.patch("/api/issues/PC1A2-7")
|
||||
.send({ priority: "high" });
|
||||
|
||||
expect(updated.status, JSON.stringify(updated.body)).toBe(200);
|
||||
expect(updated.body).toMatchObject({
|
||||
id: issueId,
|
||||
companyId,
|
||||
identifier: "PC1A2-7",
|
||||
priority: "high",
|
||||
});
|
||||
|
||||
const stored = await db
|
||||
.select({ priority: issues.priority })
|
||||
.from(issues)
|
||||
.where(eq(issues.id, issueId))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
expect(stored?.priority).toBe("high");
|
||||
});
|
||||
});
|
||||
@@ -460,14 +460,14 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => {
|
||||
expect(result.map((issue) => issue.id)).toEqual([grandchildId]);
|
||||
});
|
||||
|
||||
it("accepts issue identifiers through getById", async () => {
|
||||
it("accepts issue identifiers with alphanumeric prefixes through getById", async () => {
|
||||
const companyId = randomUUID();
|
||||
const issueId = randomUUID();
|
||||
|
||||
await db.insert(companies).values({
|
||||
id: companyId,
|
||||
name: "Paperclip",
|
||||
issuePrefix: "PAP",
|
||||
issuePrefix: "PC1A2",
|
||||
requireBoardApprovalForNewAgents: false,
|
||||
});
|
||||
|
||||
@@ -475,19 +475,19 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => {
|
||||
id: issueId,
|
||||
companyId,
|
||||
issueNumber: 1064,
|
||||
identifier: "PAP-1064",
|
||||
identifier: "PC1A2-1064",
|
||||
title: "Feedback votes error",
|
||||
status: "todo",
|
||||
priority: "medium",
|
||||
createdByUserId: "user-1",
|
||||
});
|
||||
|
||||
const issue = await svc.getById("PAP-1064");
|
||||
const issue = await svc.getById("pc1a2-1064");
|
||||
|
||||
expect(issue).toEqual(
|
||||
expect.objectContaining({
|
||||
id: issueId,
|
||||
identifier: "PAP-1064",
|
||||
identifier: "PC1A2-1064",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { Router } from "express";
|
||||
import { z } from "zod";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import { normalizeIssueIdentifier } from "@paperclipai/shared";
|
||||
import { validate } from "../middleware/validate.js";
|
||||
import { activityService, normalizeActivityLimit } from "../services/activity.js";
|
||||
import { assertAuthenticated, assertBoard, assertCompanyAccess } from "./authz.js";
|
||||
@@ -24,8 +25,9 @@ export function activityRoutes(db: Db) {
|
||||
const issueSvc = issueService(db);
|
||||
|
||||
async function resolveIssueByRef(rawId: string) {
|
||||
if (/^[A-Z]+-\d+$/i.test(rawId)) {
|
||||
return issueSvc.getByIdentifier(rawId);
|
||||
const identifier = normalizeIssueIdentifier(rawId);
|
||||
if (identifier) {
|
||||
return issueSvc.getByIdentifier(identifier);
|
||||
}
|
||||
return issueSvc.getById(rawId);
|
||||
}
|
||||
|
||||
@@ -13,6 +13,7 @@ import {
|
||||
createAgentSchema,
|
||||
deriveAgentUrlKey,
|
||||
isUuidLike,
|
||||
normalizeIssueIdentifier,
|
||||
resetAgentSessionSchema,
|
||||
testAdapterEnvironmentSchema,
|
||||
type AgentSkillSnapshot,
|
||||
@@ -3088,8 +3089,8 @@ export function agentRoutes(
|
||||
router.get("/issues/:issueId/live-runs", async (req, res) => {
|
||||
const rawId = req.params.issueId as string;
|
||||
const issueSvc = issueService(db);
|
||||
const isIdentifier = /^[A-Z]+-\d+$/i.test(rawId);
|
||||
const issue = isIdentifier ? await issueSvc.getByIdentifier(rawId) : await issueSvc.getById(rawId);
|
||||
const identifier = normalizeIssueIdentifier(rawId);
|
||||
const issue = identifier ? await issueSvc.getByIdentifier(identifier) : await issueSvc.getById(rawId);
|
||||
if (!issue) {
|
||||
res.status(404).json({ error: "Issue not found" });
|
||||
return;
|
||||
@@ -3142,8 +3143,8 @@ export function agentRoutes(
|
||||
router.get("/issues/:issueId/active-run", async (req, res) => {
|
||||
const rawId = req.params.issueId as string;
|
||||
const issueSvc = issueService(db);
|
||||
const isIdentifier = /^[A-Z]+-\d+$/i.test(rawId);
|
||||
const issue = isIdentifier ? await issueSvc.getByIdentifier(rawId) : await issueSvc.getById(rawId);
|
||||
const identifier = normalizeIssueIdentifier(rawId);
|
||||
const issue = identifier ? await issueSvc.getByIdentifier(identifier) : await issueSvc.getById(rawId);
|
||||
if (!issue) {
|
||||
res.status(404).json({ error: "Issue not found" });
|
||||
return;
|
||||
|
||||
@@ -3,6 +3,7 @@ import type { Db } from "@paperclipai/db";
|
||||
import {
|
||||
createCostEventSchema,
|
||||
createFinanceEventSchema,
|
||||
normalizeIssueIdentifier,
|
||||
resolveBudgetIncidentSchema,
|
||||
updateBudgetSchema,
|
||||
upsertBudgetPolicySchema,
|
||||
@@ -62,8 +63,9 @@ export function costRoutes(
|
||||
const issues = issueService(db);
|
||||
|
||||
async function resolveIssueByRef(rawId: string) {
|
||||
if (/^[A-Z]+-\d+$/i.test(rawId)) {
|
||||
return issues.getByIdentifier(rawId);
|
||||
const identifier = normalizeIssueIdentifier(rawId);
|
||||
if (identifier) {
|
||||
return issues.getByIdentifier(identifier);
|
||||
}
|
||||
return issues.getById(rawId);
|
||||
}
|
||||
|
||||
@@ -30,6 +30,7 @@ import {
|
||||
updateIssueSchema,
|
||||
getClosedIsolatedExecutionWorkspaceMessage,
|
||||
isClosedIsolatedExecutionWorkspace,
|
||||
normalizeIssueIdentifier as normalizeIssueReferenceIdentifier,
|
||||
type ExecutionWorkspace,
|
||||
} from "@paperclipai/shared";
|
||||
import { trackAgentTaskCompleted } from "@paperclipai/shared/telemetry";
|
||||
@@ -880,9 +881,10 @@ export function issueRoutes(
|
||||
});
|
||||
}
|
||||
|
||||
async function normalizeIssueIdentifier(rawId: string): Promise<string> {
|
||||
if (/^[A-Z]+-\d+$/i.test(rawId)) {
|
||||
const issue = await svc.getByIdentifier(rawId);
|
||||
async function resolveIssueRouteId(rawId: string): Promise<string> {
|
||||
const identifier = normalizeIssueReferenceIdentifier(rawId);
|
||||
if (identifier) {
|
||||
const issue = await svc.getByIdentifier(identifier);
|
||||
if (issue) {
|
||||
return issue.id;
|
||||
}
|
||||
@@ -920,7 +922,7 @@ export function issueRoutes(
|
||||
// Resolve issue identifiers (e.g. "PAP-39") to UUIDs for all /issues/:id routes
|
||||
router.param("id", async (req, res, next, rawId) => {
|
||||
try {
|
||||
req.params.id = await normalizeIssueIdentifier(rawId);
|
||||
req.params.id = await resolveIssueRouteId(rawId);
|
||||
next();
|
||||
} catch (err) {
|
||||
next(err);
|
||||
@@ -930,7 +932,7 @@ export function issueRoutes(
|
||||
// Resolve issue identifiers (e.g. "PAP-39") to UUIDs for company-scoped attachment routes.
|
||||
router.param("issueId", async (req, res, next, rawId) => {
|
||||
try {
|
||||
req.params.issueId = await normalizeIssueIdentifier(rawId);
|
||||
req.params.issueId = await resolveIssueRouteId(rawId);
|
||||
next();
|
||||
} catch (err) {
|
||||
next(err);
|
||||
|
||||
@@ -33,7 +33,13 @@ import type {
|
||||
IssueProductivityReviewTrigger,
|
||||
IssueRelationIssueSummary,
|
||||
} from "@paperclipai/shared";
|
||||
import { clampIssueRequestDepth, extractAgentMentionIds, extractProjectMentionIds, isUuidLike } from "@paperclipai/shared";
|
||||
import {
|
||||
clampIssueRequestDepth,
|
||||
extractAgentMentionIds,
|
||||
extractProjectMentionIds,
|
||||
isUuidLike,
|
||||
normalizeIssueIdentifier as normalizeIssueReferenceIdentifier,
|
||||
} from "@paperclipai/shared";
|
||||
import { conflict, notFound, unprocessable } from "../errors.js";
|
||||
import { parseObject } from "../adapters/utils.js";
|
||||
import {
|
||||
@@ -2418,8 +2424,9 @@ export function issueService(db: Db) {
|
||||
|
||||
getById: async (raw: string) => {
|
||||
const id = raw.trim();
|
||||
if (/^[A-Z]+-\d+$/i.test(id)) {
|
||||
return getIssueByIdentifier(id);
|
||||
const identifier = normalizeIssueReferenceIdentifier(id);
|
||||
if (identifier) {
|
||||
return getIssueByIdentifier(identifier);
|
||||
}
|
||||
if (!isUuidLike(id)) {
|
||||
return null;
|
||||
|
||||
@@ -5,6 +5,7 @@ describe("issue-reference", () => {
|
||||
it("extracts issue ids from company-scoped issue paths", () => {
|
||||
expect(parseIssuePathIdFromPath("/PAP/issues/PAP-1271")).toBe("PAP-1271");
|
||||
expect(parseIssuePathIdFromPath("/PAP/issues/pap-1272")).toBe("PAP-1272");
|
||||
expect(parseIssuePathIdFromPath("/PC1A2/issues/pc1a2-7")).toBe("PC1A2-7");
|
||||
expect(parseIssuePathIdFromPath("/issues/PAP-1179")).toBe("PAP-1179");
|
||||
expect(parseIssuePathIdFromPath("/issues/:id")).toBeNull();
|
||||
});
|
||||
@@ -30,6 +31,10 @@ describe("issue-reference", () => {
|
||||
issuePathId: "PAP-1271",
|
||||
href: "/issues/PAP-1271",
|
||||
});
|
||||
expect(parseIssueReferenceFromHref("pc1a2-7")).toEqual({
|
||||
issuePathId: "PC1A2-7",
|
||||
href: "/issues/PC1A2-7",
|
||||
});
|
||||
expect(parseIssueReferenceFromHref("/PAP/issues/pap-1180")).toEqual({
|
||||
issuePathId: "PAP-1180",
|
||||
href: "/issues/PAP-1180",
|
||||
|
||||
@@ -5,9 +5,9 @@ type MarkdownNode = {
|
||||
children?: MarkdownNode[];
|
||||
};
|
||||
|
||||
const BARE_ISSUE_IDENTIFIER_RE = /^[A-Z][A-Z0-9]+-\d+$/i;
|
||||
const BARE_ISSUE_IDENTIFIER_RE = /^[A-Z][A-Z0-9]*-\d+$/i;
|
||||
const ISSUE_SCHEME_RE = /^issue:\/\/:?([^?#\s]+)(?:[?#].*)?$/i;
|
||||
const ISSUE_REFERENCE_TOKEN_RE = /issue:\/\/:?[^\s<>()]+|https?:\/\/[^\s<>()]+|\/(?:[^\s<>()/]+\/)*issues\/[A-Z][A-Z0-9]+-\d+(?=$|[\s<>)\],.;!?:])|\b[A-Z][A-Z0-9]+-\d+\b/gi;
|
||||
const ISSUE_REFERENCE_TOKEN_RE = /issue:\/\/:?[^\s<>()]+|https?:\/\/[^\s<>()]+|\/(?:[^\s<>()/]+\/)*issues\/[A-Z][A-Z0-9]*-\d+(?=$|[\s<>)\],.;!?:])|\b[A-Z][A-Z0-9]*-\d+\b/gi;
|
||||
|
||||
export function parseIssuePathIdFromPath(pathOrUrl: string | null | undefined): string | null {
|
||||
if (!pathOrUrl) return null;
|
||||
|
||||
Reference in New Issue
Block a user