diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index 0d6675133e..a60da3e442 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -6063,3 +6063,91 @@ describeEmbeddedPostgres("issueService.assertCheckoutOwner stale checkout adopti }); }); + +describeEmbeddedPostgres("issueService.addComment createdByRunId", () => { + let db!: ReturnType; + let svc!: ReturnType; + let tempDb: Awaited> | null = null; + let companyId!: string; + let agentId!: string; + let issueId!: string; + + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-issues-comment-runid-"); + db = createDb(tempDb.connectionString); + svc = issueService(db); + + companyId = randomUUID(); + agentId = randomUUID(); + issueId = randomUUID(); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + await db.insert(agents).values({ + id: agentId, + companyId, + name: "TestAgent", + role: "engineer", + status: "active", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: {}, + permissions: {}, + }); + await db.insert(issues).values({ + id: issueId, + companyId, + title: "Test issue", + status: "todo", + priority: "medium", + }); + }, 20_000); + + afterAll(async () => { + await tempDb?.cleanup(); + }); + + async function createdByRunIdFor(commentId: string) { + return db + .select({ createdByRunId: issueComments.createdByRunId }) + .from(issueComments) + .where(eq(issueComments.id, commentId)) + .then((rows) => rows[0]?.createdByRunId ?? null); + } + + it("nulls out a non-UUID x-paperclip-run-id instead of 500-ing", async () => { + const comment = await svc.addComment(issueId, "hello from a synthetic run id", { + runId: "client-request-abc123", + }); + + expect(comment.id).toBeTruthy(); + expect(await createdByRunIdFor(comment.id)).toBeNull(); + }); + + it("nulls out a UUID runId absent from heartbeat_runs instead of 500-ing", async () => { + const comment = await svc.addComment(issueId, "hello from a stale run", { + runId: randomUUID(), + }); + + expect(comment.id).toBeTruthy(); + expect(await createdByRunIdFor(comment.id)).toBeNull(); + }); + + it("preserves a valid runId that exists in heartbeat_runs for the company", async () => { + const runId = randomUUID(); + await db.insert(heartbeatRuns).values({ + id: runId, + companyId, + agentId, + status: "running", + }); + + const comment = await svc.addComment(issueId, "hello from a live run", { runId }); + + expect(await createdByRunIdFor(comment.id)).toBe(runId); + }); +}); diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index 64df23809b..66e974649a 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -355,6 +355,25 @@ type DerivedIssueCommentAttribution = { derivedAuthorSource: IssueCommentDerivedAuthorSource; }; +/** + * Resolve a `created_by_run_id` safe for the heartbeat_runs FK; returns null for + * missing/invalid ids so an unknown run id never 500s a comment insert. + */ +async function resolveCommentCreatedByRunId( + dbOrTx: any, + companyId: string, + runId: string | null | undefined, +): Promise { + const normalized = typeof runId === "string" ? runId.trim() : ""; + if (!normalized || !isUuidLike(normalized)) return null; + const existing = await dbOrTx + .select({ id: heartbeatRuns.id }) + .from(heartbeatRuns) + .where(and(eq(heartbeatRuns.id, normalized), eq(heartbeatRuns.companyId, companyId))) + .then((rows: Array<{ id: string }>) => rows[0] ?? null); + return existing?.id ?? null; +} + /** * Best-effort agent attribution for comments whose stored author is a non-human * sentinel (e.g. `local-board`). Callers MUST pre-filter `comments` to drop any @@ -7428,6 +7447,14 @@ export function issueService(db: Db) { const presentation = issueCommentPresentationSchema.nullable().parse(options?.presentation ?? null); const metadata = issueCommentMetadataSchema.nullable().parse(options?.metadata ?? null); const createdAt = options?.createdAt ? new Date(options.createdAt) : null; + // Invalid/stale run ids must not 500 the insert — null out unknowns. + const createdByRunId = await resolveCommentCreatedByRunId(dbOrTx, issue.companyId, actor.runId); + if (actor.runId && !createdByRunId) { + logger.warn( + { issueId, companyId: issue.companyId, runId: actor.runId }, + "dropping invalid createdByRunId for issue comment insert", + ); + } const [comment] = await dbOrTx .insert(issueComments) .values({ @@ -7436,7 +7463,7 @@ export function issueService(db: Db) { authorAgentId: actor.agentId ?? null, authorUserId: actor.userId ?? null, authorType, - createdByRunId: actor.runId ?? null, + createdByRunId, body: redactedBody, presentation, metadata,