diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index ad7a8be92cf..ecf1e570d83 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -113,6 +113,7 @@ import { FEEDBACK_DELIVERY_RECOVERY_CAUSE, FEEDBACK_DELIVERY_RETRY_WAKE_REASON, FEEDBACK_DELIVERY_WAKE_COMMENT_IDS_KEY, + hasCompleteFeedbackDispositionCoverage, readFeedbackDeliveryRunContext, } from "../services/recovery/feedback-delivery.js"; import { evaluateAgentInvokabilityFromDb } from "../services/agent-invokability.js"; @@ -5019,10 +5020,12 @@ export function issueRoutes( } const handledCommentIds = [...new Set(requested.handledCommentIds)]; - const deliveredCommentIds = new Set(delivery.commentIds); - if (handledCommentIds.some((commentId) => !deliveredCommentIds.has(commentId))) { + if (!hasCompleteFeedbackDispositionCoverage({ + deliveryCommentIds: delivery.commentIds, + handledCommentIds, + })) { res.status(422).json({ - error: "Feedback disposition comment ids must belong to the current run's feedback batch", + error: "Feedback disposition must cover every comment in the current run's feedback batch and no others", }); return { ok: false }; } diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 7bb5adb5b02..415ef3a8aad 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -238,6 +238,7 @@ import { buildFeedbackDeliveryFingerprint, buildFeedbackDeliveryRetryIdempotencyKey, carriesExplicitFeedbackResume, + hasCompleteFeedbackDispositionCoverage, isEligibleFeedbackDeliveryWake, isSuccessfulFeedbackDeliveryRecoveryMatch, readFeedbackDeliveryRetryGeneration, @@ -10226,9 +10227,10 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) 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)); + return hasCompleteFeedbackDispositionCoverage({ + deliveryCommentIds: delivery.commentIds, + handledCommentIds: disposition.handledCommentIds, + }); }) ?? null); if (!comment) return null; const disposition = comment.metadata?.feedbackDisposition; diff --git a/server/src/services/recovery/feedback-delivery.test.ts b/server/src/services/recovery/feedback-delivery.test.ts index 2d90b00c431..36d9fdaff28 100644 --- a/server/src/services/recovery/feedback-delivery.test.ts +++ b/server/src/services/recovery/feedback-delivery.test.ts @@ -6,6 +6,7 @@ import { buildFeedbackDeliveryFingerprint, buildFeedbackDeliveryRetryIdempotencyKey, decideStrandedFeedbackDeliveryBackstop, + hasCompleteFeedbackDispositionCoverage, isSuccessfulFeedbackDeliveryRecoveryMatch, readFeedbackDeliveryRunContext, type StrandedFeedbackDeliveryBackstopProbe, @@ -168,6 +169,23 @@ describe("successful feedback delivery recovery matching", () => { }); }); +describe("feedback disposition coverage", () => { + it("requires the handled ids to exactly cover the delivered batch", () => { + expect(hasCompleteFeedbackDispositionCoverage({ + deliveryCommentIds: ["comment-1", "comment-2"], + handledCommentIds: ["comment-2", "comment-1"], + })).toBe(true); + expect(hasCompleteFeedbackDispositionCoverage({ + deliveryCommentIds: ["comment-1", "comment-2"], + handledCommentIds: ["comment-1"], + })).toBe(false); + expect(hasCompleteFeedbackDispositionCoverage({ + deliveryCommentIds: ["comment-1"], + handledCommentIds: ["comment-1", "comment-2"], + })).toBe(false); + }); +}); + describe("stranded feedback delivery backstop", () => { it("requeues exactly one normal-model recovery for the original assignee", () => { const decision = decide(); diff --git a/server/src/services/recovery/feedback-delivery.ts b/server/src/services/recovery/feedback-delivery.ts index 93e0040101b..27a8fdaf1b7 100644 --- a/server/src/services/recovery/feedback-delivery.ts +++ b/server/src/services/recovery/feedback-delivery.ts @@ -77,6 +77,21 @@ export function buildFeedbackDeliveryManualRetryIdempotencyKey(input: { return `${input.fingerprint}:manual:${input.attempt}`; } +export function hasCompleteFeedbackDispositionCoverage(input: { + deliveryCommentIds: string[]; + handledCommentIds: string[]; +}) { + const deliveryIds = new Set( + input.deliveryCommentIds.map(readNonEmptyString).filter((value): value is string => Boolean(value)), + ); + const handledIds = new Set( + input.handledCommentIds.map(readNonEmptyString).filter((value): value is string => Boolean(value)), + ); + return deliveryIds.size > 0 + && handledIds.size === deliveryIds.size + && [...deliveryIds].every((commentId) => handledIds.has(commentId)); +} + export function isSuccessfulFeedbackDeliveryRecoveryMatch(input: { handlingEvidence: { rootWakeupRequestId: string; diff --git a/server/src/services/recovery/index.ts b/server/src/services/recovery/index.ts index 7e9a0173758..fe02abc4568 100644 --- a/server/src/services/recovery/index.ts +++ b/server/src/services/recovery/index.ts @@ -41,6 +41,7 @@ export { buildFeedbackDeliveryRetryIdempotencyKey, carriesExplicitFeedbackResume, decideStrandedFeedbackDeliveryBackstop, + hasCompleteFeedbackDispositionCoverage, isEligibleFeedbackDeliveryWake, isSuccessfulFeedbackDeliveryRecoveryMatch, readFeedbackDeliveryRetryGeneration,