From fbaf587214e55786c6529f2d3d46178c5acac6d7 Mon Sep 17 00:00:00 2001 From: Dotta Date: Sun, 16 Aug 2026 20:31:25 +0000 Subject: [PATCH] fix(server): require explicit feedback disposition Co-Authored-By: Paperclip --- .../adapter-utils/src/server-utils.test.ts | 1 + packages/adapter-utils/src/server-utils.ts | 1 + packages/shared/src/types/issue.ts | 5 ++ packages/shared/src/validators/issue.test.ts | 13 ++++ packages/shared/src/validators/issue.ts | 8 ++ .../heartbeat-process-recovery.test.ts | 23 +++++- server/src/routes/issues.ts | 74 ++++++++++++++++++- server/src/services/heartbeat.ts | 33 +++++++-- server/src/services/issues.ts | 3 + 9 files changed, 151 insertions(+), 10 deletions(-) diff --git a/packages/adapter-utils/src/server-utils.test.ts b/packages/adapter-utils/src/server-utils.test.ts index 526fe367892..efe54515dca 100644 --- a/packages/adapter-utils/src/server-utils.test.ts +++ b/packages/adapter-utils/src/server-utils.test.ts @@ -1115,6 +1115,7 @@ describe("renderPaperclipWakePrompt", () => { expect(commentPrompt).toContain("Only fetch the API thread"); expect(commentPrompt).toContain("- pending comments: 1/1"); expect(commentPrompt).toContain("- latest comment id: comment-1"); + expect(commentPrompt).toContain("`feedbackDisposition.handledCommentIds`"); const fallbackPrompt = renderPaperclipWakePrompt({ ...base, fallbackFetchNeeded: true }); expect(fallbackPrompt).toContain("Only fetch the API thread"); diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index 23a73e09b9f..0695d85873f 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -1557,6 +1557,7 @@ export function renderPaperclipWakePrompt( ? [ `- pending comments: ${normalized.includedCount}/${normalized.requestedCount}`, `- latest comment id: ${normalized.latestCommentId ?? "unknown"}`, + "- feedback disposition: when your issue comment handles this feedback batch, include `feedbackDisposition.handledCommentIds` with every handled comment id; an ordinary comment does not acknowledge delivery", ] : []), `- fallback fetch needed: ${normalized.fallbackFetchNeeded ? "yes" : "no"}`, diff --git a/packages/shared/src/types/issue.ts b/packages/shared/src/types/issue.ts index e263822ce79..3d3c271fc79 100644 --- a/packages/shared/src/types/issue.ts +++ b/packages/shared/src/types/issue.ts @@ -1098,6 +1098,11 @@ export interface IssueCommentMetadata { version: 1; sourceRunId?: string | null; authorizationReason?: string | null; + feedbackDisposition?: { + kind: "feedback_delivery"; + rootWakeupRequestId: string; + handledCommentIds: string[]; + } | null; sections: IssueCommentMetadataSection[]; } diff --git a/packages/shared/src/validators/issue.test.ts b/packages/shared/src/validators/issue.test.ts index 20161a2094d..b9f5fc84e6d 100644 --- a/packages/shared/src/validators/issue.test.ts +++ b/packages/shared/src/validators/issue.test.ts @@ -258,6 +258,19 @@ describe("issue validators", () => { expect(parsed.body).toBe("Progress update\n\nNext action."); }); + it("accepts an explicit feedback disposition comment batch", () => { + const handledCommentIds = [ + "11111111-1111-4111-8111-111111111111", + "22222222-2222-4222-8222-222222222222", + ]; + const parsed = addIssueCommentSchema.parse({ + body: "Applied both requested changes.", + feedbackDisposition: { handledCommentIds }, + }); + + expect(parsed.feedbackDisposition?.handledCommentIds).toEqual(handledCommentIds); + }); + it("accepts structured issue comment presentation and metadata", () => { const parsed = addIssueCommentSchema.parse({ body: "Paperclip needs a disposition before this issue can continue.", diff --git a/packages/shared/src/validators/issue.ts b/packages/shared/src/validators/issue.ts index 03b4cb264a8..781a4a5548b 100644 --- a/packages/shared/src/validators/issue.ts +++ b/packages/shared/src/validators/issue.ts @@ -675,6 +675,11 @@ export const issueCommentMetadataSchema = z.object({ version: z.literal(1), sourceRunId: z.string().uuid().nullable().optional(), authorizationReason: z.string().trim().min(1).max(160).nullable().optional(), + feedbackDisposition: z.object({ + kind: z.literal("feedback_delivery"), + rootWakeupRequestId: z.string().uuid(), + handledCommentIds: z.array(z.string().uuid()).min(1).max(50), + }).strict().nullable().optional(), sections: z.array(issueCommentMetadataSectionSchema).min(1).max(20), }).strict(); @@ -686,6 +691,9 @@ export const addIssueCommentSchema = z.object({ authorType: issueCommentAuthorTypeSchema.optional(), presentation: issueCommentPresentationSchema.nullable().optional(), metadata: issueCommentMetadataSchema.nullable().optional(), + feedbackDisposition: z.object({ + handledCommentIds: z.array(z.string().uuid()).min(1).max(50), + }).strict().optional(), reopen: z.boolean().optional(), resume: z.boolean().optional(), interrupt: z.boolean().optional(), diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index 7fed6a2a82f..e44826b2f34 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -1798,6 +1798,18 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { authorAgentId: fixture.agentId, createdByRunId: retryRun.id, body: "Applied the requested feedback.", + metadata: { + version: 1, + feedbackDisposition: { + kind: "feedback_delivery", + rootWakeupRequestId: fixture.wakeupRequestId, + handledCommentIds: [commentId], + }, + sections: [{ + title: "Feedback disposition", + rows: [{ type: "key_value", label: "Handled comment", value: commentId }], + }], + }, }); await db.update(issues).set({ status: "done" }).where(eq(issues.id, fixture.issueId)); releaseAdapter(); @@ -1837,7 +1849,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { }); }); - it("keeps feedback exhausted when a successful replay records no handling evidence", async () => { + it("keeps feedback exhausted when a successful replay posts an ordinary run comment", async () => { let releaseAdapter: (() => void) | null = null; const adapterStarted = new Promise((resolve) => { mockAdapterExecute.mockImplementationOnce(async () => { @@ -1880,6 +1892,15 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { .then((rows) => rows[0] ?? null); if (!retryRun || !releaseAdapter) throw new Error("feedback retry did not reach the adapter"); + await db.insert(issueComments).values({ + companyId: fixture.companyId, + issueId: fixture.issueId, + authorType: "agent", + authorAgentId: fixture.agentId, + createdByRunId: retryRun.id, + body: "Unrelated status update.", + }); + releaseAdapter(); expect((await waitForRunToSettle(heartbeat, retryRun.id, 5_000))?.status).toBe("succeeded"); await heartbeat.waitForRunExecutionDrain(retryRun.id); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 2ac2be7040e..ad7a8be92cf 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -89,6 +89,7 @@ import { type IssueWakeDiagnosticWakeRequest, type IssueWakeDiagnosticsResponse, type IssueFeedbackDeliveryRetryResponse, + type IssueCommentMetadata, type IssueRelationIssueSummary, type IssueReviewPolicy, type IssueThreadInteractionCanonicalResolverPolicy, @@ -112,6 +113,7 @@ import { FEEDBACK_DELIVERY_RECOVERY_CAUSE, FEEDBACK_DELIVERY_RETRY_WAKE_REASON, FEEDBACK_DELIVERY_WAKE_COMMENT_IDS_KEY, + readFeedbackDeliveryRunContext, } from "../services/recovery/feedback-delivery.js"; import { evaluateAgentInvokabilityFromDb } from "../services/agent-invokability.js"; import { getTelemetryClient } from "../telemetry.js"; @@ -4979,6 +4981,72 @@ export function issueRoutes( return false; } + async function resolveFeedbackDispositionMetadata( + req: Request, + res: Response, + issue: { id: string; companyId: string }, + ): Promise<{ ok: true; metadata: IssueCommentMetadata | null } | { ok: false }> { + const requested = req.body.feedbackDisposition as { handledCommentIds: string[] } | undefined; + if (!requested) return { ok: true, metadata: null }; + if (req.actor.type !== "agent" || !req.actor.agentId || !req.actor.runId) { + res.status(403).json({ error: "Feedback dispositions require an authenticated agent run" }); + return { ok: false }; + } + const actorAgentId = req.actor.agentId; + const actorRunId = req.actor.runId; + + const run = await db + .select({ + id: heartbeatRuns.id, + companyId: heartbeatRuns.companyId, + agentId: heartbeatRuns.agentId, + wakeupRequestId: heartbeatRuns.wakeupRequestId, + contextSnapshot: heartbeatRuns.contextSnapshot, + scheduledRetryReason: heartbeatRuns.scheduledRetryReason, + scheduledRetryAttempt: heartbeatRuns.scheduledRetryAttempt, + }) + .from(heartbeatRuns) + .where(and( + eq(heartbeatRuns.id, actorRunId), + eq(heartbeatRuns.companyId, issue.companyId), + eq(heartbeatRuns.agentId, actorAgentId), + )) + .then((rows) => rows[0] ?? null); + const delivery = run ? readFeedbackDeliveryRunContext({ companyId: issue.companyId, run }) : null; + if (!delivery || delivery.issueId !== issue.id) { + res.status(422).json({ error: "The current run is not a feedback-delivery run for this issue" }); + return { ok: false }; + } + + const handledCommentIds = [...new Set(requested.handledCommentIds)]; + const deliveredCommentIds = new Set(delivery.commentIds); + if (handledCommentIds.some((commentId) => !deliveredCommentIds.has(commentId))) { + res.status(422).json({ + error: "Feedback disposition comment ids must belong to the current run's feedback batch", + }); + return { ok: false }; + } + + return { + ok: true, + metadata: { + version: 1, + feedbackDisposition: { + kind: "feedback_delivery", + rootWakeupRequestId: delivery.rootWakeupRequestId, + handledCommentIds, + }, + sections: [{ + title: "Feedback disposition", + rows: [ + { type: "key_value", label: "Root wake", value: delivery.rootWakeupRequestId }, + { type: "key_value", label: "Handled comments", value: handledCommentIds.join(", ") }, + ], + }], + }, + }; + } + async function assertExplicitResumeIntentAllowed( req: Request, res: Response, @@ -11836,6 +11904,8 @@ export function issueRoutes( presentation: req.body.presentation, metadata: req.body.metadata, })) return; + const feedbackDisposition = await resolveFeedbackDispositionMetadata(req, res, issue); + if (!feedbackDisposition.ok) return; const closedExecutionWorkspace = await getClosedIssueExecutionWorkspace(issue); const actor = getActorInfo(req); @@ -12099,7 +12169,7 @@ export function issueRoutes( const commentOptions = { authorType: req.body.authorType ?? (actor.actorType === "agent" ? "agent" : "user"), presentation: commentPresentation, - metadata: req.body.metadata ?? null, + metadata: req.body.metadata ?? feedbackDisposition.metadata, sourceTrust, }; let txResult: { comment: Awaited>; issue: NonNullable>> }; @@ -12190,7 +12260,7 @@ export function issueRoutes( }, { authorType: req.body.authorType ?? (actor.actorType === "agent" ? "agent" : "user"), presentation: commentPresentation, - metadata: req.body.metadata ?? null, + metadata: req.body.metadata ?? feedbackDisposition.metadata, authorizationReason: commentAuthorizationReason, sourceTrust: await sourceTrustForActorWrite(currentIssue, actor), }); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 8ff0159fe07..7bb5adb5b02 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -10210,17 +10210,36 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) run: typeof heartbeatRuns.$inferSelect, delivery: FeedbackDeliveryContext, ) { - const comment = await findRunIssueComment(run.id, run.companyId, delivery.issueId); + const comment = await db + .select({ id: issueComments.id, metadata: issueComments.metadata }) + .from(issueComments) + .where(and( + eq(issueComments.companyId, run.companyId), + eq(issueComments.issueId, delivery.issueId), + eq(issueComments.createdByRunId, run.id), + isNull(issueComments.deletedAt), + )) + .orderBy(desc(issueComments.createdAt), desc(issueComments.id)) + .then((rows) => rows.find((row) => { + const disposition = row.metadata?.feedbackDisposition; + if ( + disposition?.kind !== "feedback_delivery" + || disposition.rootWakeupRequestId !== delivery.rootWakeupRequestId + ) return false; + const handledIds = new Set(disposition.handledCommentIds); + return delivery.commentIds.length > 0 + && delivery.commentIds.every((commentId) => handledIds.has(commentId)); + }) ?? null); if (!comment) return null; - // A feedback-delivery run's own persisted context is the durable association - // between its disposition comment and the exact feedback batch it received. - // Other same-run issue activity is intentionally insufficient: it can be - // unrelated to one or more outstanding human comments. + const disposition = comment.metadata?.feedbackDisposition; + if (!disposition) return null; + // Only an explicit, persisted disposition can acknowledge feedback. Ordinary + // run comments and other same-run issue activity can be unrelated. return { kind: "comment", id: comment.id, - rootWakeupRequestId: delivery.rootWakeupRequestId, - sourceCommentIds: delivery.commentIds, + rootWakeupRequestId: disposition.rootWakeupRequestId, + sourceCommentIds: disposition.handledCommentIds, } as const; } diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index e7500973b01..78c56fd1ca4 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -425,6 +425,9 @@ function withAgentCommentAuthorizationMetadata( return { version: 1, ...(metadata?.sourceRunId !== undefined ? { sourceRunId: metadata.sourceRunId } : {}), + ...(metadata?.feedbackDisposition !== undefined + ? { feedbackDisposition: metadata.feedbackDisposition } + : {}), authorizationReason: reason, sections: metadata?.sections.length ? metadata.sections