From 7c6d9779290dc473d3d18fbaf6a16ab67a771ec7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=A6=A8=E5=86=89?= Date: Fri, 16 Oct 2026 07:04:57 +0800 Subject: [PATCH] fix(wake-queue): skip malformed queued comment ids so one bad id cannot block an issue's wakes (#13912) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The wake queue decides whether a finishing run's `@`-mention actually wakes the mentioned agent, by checking the liveness of the comment ids queued in the wake payload > - `getQueuedCommentLiveness` passes every queued comment id straight into an SQL `inArray`. Postgres rejects the whole statement on the first malformed uuid, so one bad id (for example an 8-char short form written into a wake payload) silently blocks every queued wake for that issue > - The run that triggers the check still records `succeeded`, so from the board the agent finished its work and the mentioned agent simply never responds; the reporter hit this twice on the same long-lived issue > - This pull request filters the queued ids with the existing `isUuidLike` helper before the query, and skips the query entirely when nothing valid remains > - The benefit is that a malformed id is skipped instead of poisoning the batch — it can never match a comment row anyway, so the downstream liveness semantics do not change Fixes: #13908 Related issue class (malformed ids reaching database queries): #887, #7639, #8093, #11018, #13504 — this is the first report for the wake-queue site. ## What Changed - `server/src/modules/wake-queue/adapters/postgres.ts`: in `getQueuedCommentLiveness`, filter `queuedCommentIds` through `isUuidLike` before building the `inArray` predicate; when no valid uuid remains, skip the query and return an empty row set. - The post-processing loops now iterate the filtered id list, which is equivalent because a malformed id can never match a comment row. ## Verification - New embedded-Postgres regression tests in `server/src/modules/wake-queue/adapters/postgres.test.ts`: - a batch mixing one live comment id, one malformed short-form id, and one valid-but-missing uuid returns only the live comment instead of throwing; - an all-malformed batch returns nothing live instead of generating `inArray ()`. - `./node_modules/.bin/vitest run server/src/modules/wake-queue/adapters/postgres.test.ts` — 58/58 pass. ## Risks Low risk. The only behavioral change is that a wake batch that would previously fail the whole statement now dispatches the valid entries. A malformed id previously matched no rows (the statement errored before any filtering), so treating it as not-live is exactly what the old downstream filters would have concluded had the query run. ## Model Used Codex (OpenAI GPT-5, coding agent, tool use, repo-level context) — assisted with locating the call site, authoring the guard, and the regression tests; human review before submission. ## 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 (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] 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: 馨冉 --- .../wake-queue/adapters/postgres.test.ts | 46 +++++++++++++++++++ .../modules/wake-queue/adapters/postgres.ts | 16 +++++-- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/server/src/modules/wake-queue/adapters/postgres.test.ts b/server/src/modules/wake-queue/adapters/postgres.test.ts index 9bc674d5fac..1e13681d345 100644 --- a/server/src/modules/wake-queue/adapters/postgres.test.ts +++ b/server/src/modules/wake-queue/adapters/postgres.test.ts @@ -245,6 +245,52 @@ describeEmbeddedPostgres("wake-queue postgres adapter", () => { expect(checked).toBe(true); }); + it("skips malformed queued comment ids instead of failing the whole liveness batch", async () => { + const companyId = await seedCompany(); + const agentId = await seedAgent({ companyId }); + const issueId = await seedIssue({ companyId, assigneeAgentId: agentId }); + const runId = await seedRun({ companyId, agentId, status: "succeeded", contextSnapshot: { issueId } }); + const [comment] = await db.insert(issueComments).values({ + companyId, issueId, authorAgentId: agentId, createdByRunId: null, body: "Mentioned follow-up", + }).returning(); + const missingCommentId = randomUUID(); + // An 8-char short form, like the id observed in a live wake payload. It is + // not a uuid, so it can never match a comment row, but it used to abort the + // whole `inArray` statement and block every queued wake for the issue. + const malformedCommentId = "0422d095"; + const adapter = createPostgresWakeQueueAdapter(db, stubDeps); + let result: { liveNonSelfCommentIds: string[]; containedSelfAuthoredComment: boolean } | null = null; + await adapter.withIssueExecutionLock({ companyId, runId, now: new Date() }, async (_locked, ports) => { + result = await ports.transaction.getQueuedCommentLiveness({ + companyId, issueId, wakeAgentId: agentId, finishingRunId: runId, finishingRunAgentId: agentId, + queuedCommentIds: [comment.id, malformedCommentId, missingCommentId], + }); + return { outcome: { kind: "released" as const }, postCommitEffects: [] }; + }); + expect(result).not.toBeNull(); + expect(result!.liveNonSelfCommentIds).toEqual([comment.id]); + expect(result!.containedSelfAuthoredComment).toBe(false); + }); + + it("treats an all-malformed queued comment id list as nothing live", async () => { + const companyId = await seedCompany(); + const agentId = await seedAgent({ companyId }); + const issueId = await seedIssue({ companyId, assigneeAgentId: agentId }); + const runId = await seedRun({ companyId, agentId, status: "succeeded", contextSnapshot: { issueId } }); + const adapter = createPostgresWakeQueueAdapter(db, stubDeps); + let result: { liveNonSelfCommentIds: string[]; containedSelfAuthoredComment: boolean } | null = null; + await adapter.withIssueExecutionLock({ companyId, runId, now: new Date() }, async (_locked, ports) => { + result = await ports.transaction.getQueuedCommentLiveness({ + companyId, issueId, wakeAgentId: agentId, finishingRunId: runId, finishingRunAgentId: agentId, + queuedCommentIds: ["0422d095"], + }); + return { outcome: { kind: "released" as const }, postCommitEffects: [] }; + }); + expect(result).not.toBeNull(); + expect(result!.liveNonSelfCommentIds).toEqual([]); + expect(result!.containedSelfAuthoredComment).toBe(false); + }); + it.each([false, true])("rechecks disabled chat mode before interrupted queue promotion (conversation=%s)", async (conversation) => { const settings = instanceSettingsService(db); const original = (await settings.getExperimental()).enableAgentChat; diff --git a/server/src/modules/wake-queue/adapters/postgres.ts b/server/src/modules/wake-queue/adapters/postgres.ts index 480d020088d..5d7a7b9eade 100644 --- a/server/src/modules/wake-queue/adapters/postgres.ts +++ b/server/src/modules/wake-queue/adapters/postgres.ts @@ -6,7 +6,7 @@ import { currentConversationCommentCondition } from "../../../services/agent-con import { getExecutionBlocker } from "../../../services/execution-blocker.js"; import { and, asc, eq, inArray, isNull, notInArray, or, sql } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; -import { extractIssueReferenceIdentifiers } from "@paperclipai/shared"; +import { extractIssueReferenceIdentifiers, isUuidLike } from "@paperclipai/shared"; import { activityLog, agentWakeupRequests, @@ -285,12 +285,20 @@ function buildTransaction(tx: Db, deps: WakeQueuePostgresAdapterDeps, db: Db, ru }, async getQueuedCommentLiveness({ companyId, issueId, wakeAgentId, finishingRunId, finishingRunAgentId, queuedCommentIds }) { - const rows = await tx + // Malformed ids (for example an 8-char short form written into a wake + // payload) can never match a comment row, but Postgres rejects the whole + // `inArray` statement on the first invalid uuid and blocks every queued + // wake for the issue. Skip them up front so one bad id cannot take down + // the batch; the liveness filters below already treat them as not live. + const queryableCommentIds = queuedCommentIds + .map((commentId) => commentId.trim()) + .filter((commentId) => isUuidLike(commentId)); + const rows = queryableCommentIds.length === 0 ? [] : await tx .select({ id: issueComments.id, deletedAt: issueComments.deletedAt, createdByRunId: issueComments.createdByRunId }) .from(issueComments) - .where(and(eq(issueComments.companyId, companyId), eq(issueComments.issueId, issueId), inArray(issueComments.id, queuedCommentIds), currentConversationCommentCondition())); + .where(and(eq(issueComments.companyId, companyId), eq(issueComments.issueId, issueId), inArray(issueComments.id, queryableCommentIds), currentConversationCommentCondition())); const targetsFinishingRunAgent = wakeAgentId === finishingRunAgentId; - const liveNonSelfCommentIds = queuedCommentIds.filter((commentId) => { + const liveNonSelfCommentIds = queryableCommentIds.filter((commentId) => { const row = rows.find((candidate) => candidate.id === commentId); return Boolean(row && !row.deletedAt && (!targetsFinishingRunAgent || row.createdByRunId !== finishingRunId)); });