diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 6af830b5ce9..8ff0159fe07 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -10208,53 +10208,20 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) async function findFeedbackHandlingEvidence( run: typeof heartbeatRuns.$inferSelect, - issueId: string, + delivery: FeedbackDeliveryContext, ) { - const [comment, documentRevision, workProduct, issueActivity] = await Promise.all([ - findRunIssueComment(run.id, run.companyId, issueId), - db - .select({ id: documentRevisions.id }) - .from(documentRevisions) - .innerJoin(issueDocuments, eq(issueDocuments.documentId, documentRevisions.documentId)) - .where(and( - eq(documentRevisions.companyId, run.companyId), - eq(documentRevisions.createdByRunId, run.id), - eq(issueDocuments.companyId, run.companyId), - eq(issueDocuments.issueId, issueId), - // This system-maintained document is refreshed automatically during - // run finalization; it does not prove the agent handled the feedback. - sql`${issueDocuments.key} != ${ISSUE_CONTINUATION_SUMMARY_DOCUMENT_KEY}`, - )) - .limit(1) - .then((rows) => rows[0] ?? null), - db - .select({ id: issueWorkProducts.id }) - .from(issueWorkProducts) - .where(and( - eq(issueWorkProducts.companyId, run.companyId), - eq(issueWorkProducts.issueId, issueId), - eq(issueWorkProducts.createdByRunId, run.id), - )) - .limit(1) - .then((rows) => rows[0] ?? null), - db - .select({ id: activityLog.id, action: activityLog.action }) - .from(activityLog) - .where(and( - eq(activityLog.companyId, run.companyId), - eq(activityLog.entityType, "issue"), - eq(activityLog.entityId, issueId), - eq(activityLog.runId, run.id), - inArray(activityLog.action, ISSUE_PROGRESS_ACTIVITY_ACTIONS), - )) - .limit(1) - .then((rows) => rows[0] ?? null), - ]); - if (comment) return { kind: "comment", id: comment.id } as const; - if (documentRevision) return { kind: "document_revision", id: documentRevision.id } as const; - if (workProduct) return { kind: "work_product", id: workProduct.id } as const; - if (issueActivity) return { kind: "issue_activity", id: issueActivity.id, action: issueActivity.action } as const; - return null; + const comment = await findRunIssueComment(run.id, run.companyId, delivery.issueId); + 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. + return { + kind: "comment", + id: comment.id, + rootWakeupRequestId: delivery.rootWakeupRequestId, + sourceCommentIds: delivery.commentIds, + } as const; } async function logFeedbackRetryQueuedOnce(input: { @@ -10684,7 +10651,7 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) const delivery = await readFeedbackDeliveryContext(input.run); if (!delivery) return { kind: "not_applicable" as const }; - const handlingEvidence = await findFeedbackHandlingEvidence(input.run, delivery.issueId); + const handlingEvidence = await findFeedbackHandlingEvidence(input.run, delivery); if (input.run.status === "succeeded" && handlingEvidence) { const activeAction = await recoveryActions.getActiveForIssue( @@ -10694,7 +10661,7 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) if ( activeAction && isSuccessfulFeedbackDeliveryRecoveryMatch({ - hasHandlingEvidence: true, + handlingEvidence, run: input.run, action: activeAction, }) diff --git a/server/src/services/recovery/feedback-delivery.test.ts b/server/src/services/recovery/feedback-delivery.test.ts index f2af8527984..2d90b00c431 100644 --- a/server/src/services/recovery/feedback-delivery.test.ts +++ b/server/src/services/recovery/feedback-delivery.test.ts @@ -105,12 +105,16 @@ describe("successful feedback delivery recovery matching", () => { kind: "feedback_delivery", status: "active", cause: "feedback_delivery_exhausted", - evidence: { rootWakeupRequestId: ROOT_WAKE_ID }, + evidence: { rootWakeupRequestId: ROOT_WAKE_ID, outstandingCommentIds: ["comment-1", "comment-2"] }, + }; + const matchingHandlingEvidence = { + rootWakeupRequestId: ROOT_WAKE_ID, + sourceCommentIds: ["comment-1", "comment-2"], }; it("matches only a successful run for the same company, issue, and root wake", () => { expect(isSuccessfulFeedbackDeliveryRecoveryMatch({ - hasHandlingEvidence: true, + handlingEvidence: matchingHandlingEvidence, run: matchingRun, action: matchingAction, })).toBe(true); @@ -118,7 +122,18 @@ describe("successful feedback delivery recovery matching", () => { it("rejects a successful run without same-run handling evidence", () => { expect(isSuccessfulFeedbackDeliveryRecoveryMatch({ - hasHandlingEvidence: false, + handlingEvidence: null, + run: matchingRun, + action: matchingAction, + })).toBe(false); + }); + + it("rejects evidence that does not cover every outstanding source comment", () => { + expect(isSuccessfulFeedbackDeliveryRecoveryMatch({ + handlingEvidence: { + ...matchingHandlingEvidence, + sourceCommentIds: ["comment-1"], + }, run: matchingRun, action: matchingAction, })).toBe(false); @@ -146,7 +161,7 @@ describe("successful feedback delivery recovery matching", () => { ["resolved action", { action: { ...matchingAction, status: "resolved" } }], ])("rejects a %s", (_label, overrides) => { expect(isSuccessfulFeedbackDeliveryRecoveryMatch({ - hasHandlingEvidence: true, + handlingEvidence: matchingHandlingEvidence, run: "run" in overrides ? overrides.run : matchingRun, action: "action" in overrides ? overrides.action : matchingAction, })).toBe(false); diff --git a/server/src/services/recovery/feedback-delivery.ts b/server/src/services/recovery/feedback-delivery.ts index 72ab4434f09..93e0040101b 100644 --- a/server/src/services/recovery/feedback-delivery.ts +++ b/server/src/services/recovery/feedback-delivery.ts @@ -78,7 +78,10 @@ export function buildFeedbackDeliveryManualRetryIdempotencyKey(input: { } export function isSuccessfulFeedbackDeliveryRecoveryMatch(input: { - hasHandlingEvidence: boolean; + handlingEvidence: { + rootWakeupRequestId: string; + sourceCommentIds: string[]; + } | null; run: { companyId: string; status: string; @@ -93,7 +96,7 @@ export function isSuccessfulFeedbackDeliveryRecoveryMatch(input: { evidence: unknown; }; }) { - if (input.run.status !== "succeeded" || !input.hasHandlingEvidence) return false; + if (input.run.status !== "succeeded" || !input.handlingEvidence) return false; if ( input.action.kind !== FEEDBACK_DELIVERY_RECOVERY_ACTION_KIND || input.action.cause !== FEEDBACK_DELIVERY_RECOVERY_CAUSE @@ -111,11 +114,22 @@ export function isSuccessfulFeedbackDeliveryRecoveryMatch(input: { : {}; const issueId = readNonEmptyString(context.issueId) ?? readNonEmptyString(context.taskId); const rootWakeupRequestId = readNonEmptyString(context.feedbackDeliveryRootWakeupRequestId); + const outstandingCommentIds = Array.isArray(evidence.outstandingCommentIds) + ? evidence.outstandingCommentIds.map(readNonEmptyString).filter((value): value is string => Boolean(value)) + : []; + const handledCommentIds = new Set( + input.handlingEvidence.sourceCommentIds + .map(readNonEmptyString) + .filter((value): value is string => Boolean(value)), + ); return Boolean( issueId && rootWakeupRequestId && input.action.sourceIssueId === issueId && readNonEmptyString(evidence.rootWakeupRequestId) === rootWakeupRequestId + && input.handlingEvidence.rootWakeupRequestId === rootWakeupRequestId + && outstandingCommentIds.length > 0 + && outstandingCommentIds.every((commentId) => handledCommentIds.has(commentId)) ); }