mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 02:44:50 +01:00
fix(server): require explicit feedback disposition
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
53c075fbb3
commit
fbaf587214
9 files changed
+151
-10
No files matched your search
@@ -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");
|
||||
|
||||
@@ -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"}`,
|
||||
|
||||
@@ -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[];
|
||||
}
|
||||
|
||||
|
||||
@@ -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.",
|
||||
|
||||
@@ -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(),
|
||||
|
||||
@@ -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<void>((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);
|
||||
|
||||
@@ -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<ReturnType<typeof svc.addComment>>; issue: NonNullable<Awaited<ReturnType<typeof svc.update>>> };
|
||||
@@ -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),
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user