From 64f01e2cd9e8ef06ff44d84fe037a7abdee55c1b Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 16 May 2026 21:30:19 -0500 Subject: [PATCH] Address graph liveness review followups Co-Authored-By: Paperclip --- ...eartbeat-issue-liveness-escalation.test.ts | 149 ++++++++++++------ server/src/routes/issues.ts | 22 ++- server/src/services/recovery/service.ts | 2 +- ui/src/pages/IssueDetail.tsx | 1 + 4 files changed, 109 insertions(+), 65 deletions(-) diff --git a/server/src/__tests__/heartbeat-issue-liveness-escalation.test.ts b/server/src/__tests__/heartbeat-issue-liveness-escalation.test.ts index 545a8215c0d..365b71e37e7 100644 --- a/server/src/__tests__/heartbeat-issue-liveness-escalation.test.ts +++ b/server/src/__tests__/heartbeat-issue-liveness-escalation.test.ts @@ -1019,64 +1019,109 @@ describeEmbeddedPostgres("heartbeat issue graph liveness escalation", () => { expect(actions).toHaveLength(1); }); - it.each(["resolved", "cancelled"] as const)( - "does not create a fresh recovery object when the same-fingerprint action was already %s", - async (status) => { - await enableAutoRecovery(); - const { companyId, managerId, blockedIssueId, blockerIssueId } = await seedBlockedChain(); - const heartbeat = heartbeatService(db); - const incidentKey = [ - "harness_liveness", + it("does not create a fresh recovery object when the same-fingerprint action was already resolved", async () => { + await enableAutoRecovery(); + const { companyId, managerId, blockedIssueId, blockerIssueId } = await seedBlockedChain(); + const heartbeat = heartbeatService(db); + const incidentKey = [ + "harness_liveness", + companyId, + blockedIssueId, + "blocked_by_unassigned_issue", + blockerIssueId, + ].join(":"); + const resolvedAt = new Date(); + + await db.insert(issueRecoveryActions).values({ + companyId, + sourceIssueId: blockerIssueId, + kind: "issue_graph_liveness", + status: "resolved", + ownerType: "agent", + ownerAgentId: managerId, + cause: "issue_graph_liveness", + fingerprint: [ + "harness_liveness_leaf", companyId, - blockedIssueId, "blocked_by_unassigned_issue", blockerIssueId, - ].join(":"); - const resolvedAt = new Date(); + ].join(":"), + evidence: { incidentKey }, + nextAction: "Already handled", + outcome: "restored", + resolvedAt, + }); - await db.insert(issueRecoveryActions).values({ + const result = await heartbeat.reconcileIssueGraphLiveness(); + + expect(result.escalationsCreated).toBe(0); + expect(result.existingEscalations).toBe(0); + expect(result.resolvedUnchangedSourceRecoveryActions).toBe(1); + + const openEscalations = await db + .select() + .from(issues) + .where( + and( + eq(issues.companyId, companyId), + eq(issues.originKind, "harness_liveness_escalation"), + eq(issues.originId, incidentKey), + ), + ); + expect(openEscalations).toHaveLength(0); + const actions = await db + .select() + .from(issueRecoveryActions) + .where(and(eq(issueRecoveryActions.companyId, companyId), eq(issueRecoveryActions.sourceIssueId, blockerIssueId))); + expect(actions).toHaveLength(1); + expect(actions[0]?.status).toBe("resolved"); + }); + + it("creates a fresh recovery object when the same-fingerprint action was cancelled", async () => { + await enableAutoRecovery(); + const { companyId, managerId, blockedIssueId, blockerIssueId } = await seedBlockedChain(); + const heartbeat = heartbeatService(db); + const incidentKey = [ + "harness_liveness", + companyId, + blockedIssueId, + "blocked_by_unassigned_issue", + blockerIssueId, + ].join(":"); + const resolvedAt = new Date(); + + await db.insert(issueRecoveryActions).values({ + companyId, + sourceIssueId: blockerIssueId, + kind: "issue_graph_liveness", + status: "cancelled", + ownerType: "agent", + ownerAgentId: managerId, + cause: "issue_graph_liveness", + fingerprint: [ + "harness_liveness_leaf", companyId, - sourceIssueId: blockerIssueId, - kind: "issue_graph_liveness", - status, - ownerType: "agent", - ownerAgentId: managerId, - cause: "issue_graph_liveness", - fingerprint: [ - "harness_liveness_leaf", - companyId, - "blocked_by_unassigned_issue", - blockerIssueId, - ].join(":"), - evidence: { incidentKey }, - nextAction: "Already handled", - outcome: "restored", - resolvedAt, - }); + "blocked_by_unassigned_issue", + blockerIssueId, + ].join(":"), + evidence: { incidentKey }, + nextAction: "Cancelled by operator", + outcome: "cancelled", + resolvedAt, + }); - const result = await heartbeat.reconcileIssueGraphLiveness(); + const result = await heartbeat.reconcileIssueGraphLiveness(); - expect(result.escalationsCreated).toBe(0); - expect(result.existingEscalations).toBe(0); - expect(result.resolvedUnchangedSourceRecoveryActions).toBe(1); + expect(result.escalationsCreated).toBe(1); + expect(result.existingEscalations).toBe(0); + expect(result.sourceRecoveryActionsOpened).toBe(1); + expect(result.resolvedUnchangedSourceRecoveryActions).toBe(0); - const openEscalations = await db - .select() - .from(issues) - .where( - and( - eq(issues.companyId, companyId), - eq(issues.originKind, "harness_liveness_escalation"), - eq(issues.originId, incidentKey), - ), - ); - expect(openEscalations).toHaveLength(0); - const actions = await db - .select() - .from(issueRecoveryActions) - .where(and(eq(issueRecoveryActions.companyId, companyId), eq(issueRecoveryActions.sourceIssueId, blockerIssueId))); - expect(actions).toHaveLength(1); - expect(actions[0]?.status).toBe(status); - }, - ); + const actions = await db + .select() + .from(issueRecoveryActions) + .where(and(eq(issueRecoveryActions.companyId, companyId), eq(issueRecoveryActions.sourceIssueId, blockerIssueId))); + expect(actions).toHaveLength(2); + expect(actions.map((action) => action.status).sort()).toEqual(["active", "cancelled"]); + }); }); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index d44fec20cc4..179ca94e756 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -1557,18 +1557,16 @@ export function issueRoutes( blocks?: IssueRelationIssueSummary[]; }>; const issueIds = issuesWithRelations.map((issue) => issue.id); - const relationRecoveryCandidateIds = [ - ...new Set(issuesWithRelations.flatMap((issue) => [ - ...((issue.blockedBy ?? []).flatMap((summary) => [ - summary.id, - ...((summary.terminalBlockers ?? []).map((terminal) => terminal.id)), - ])), - ...((issue.blocks ?? []).flatMap((summary) => [ - summary.id, - ...((summary.terminalBlockers ?? []).map((terminal) => terminal.id)), - ])), - ])), - ]; + const relationRecoveryCandidateIdSet = new Set(); + const collectRelationRecoveryCandidateIds = (summary: IssueRelationIssueSummary) => { + relationRecoveryCandidateIdSet.add(summary.id); + summary.terminalBlockers?.forEach(collectRelationRecoveryCandidateIds); + }; + issuesWithRelations.forEach((issue) => { + issue.blockedBy?.forEach(collectRelationRecoveryCandidateIds); + issue.blocks?.forEach(collectRelationRecoveryCandidateIds); + }); + const relationRecoveryCandidateIds = [...relationRecoveryCandidateIdSet]; const [handoffStates, recoveryActionByIssue] = await Promise.all([ listSuccessfulRunHandoffStates(db, companyId, issueIds), recoveryActionsSvc.listActiveForIssues(companyId, [...issueIds, ...relationRecoveryCandidateIds]), diff --git a/server/src/services/recovery/service.ts b/server/src/services/recovery/service.ts index 0962de4ed88..ec70208866c 100644 --- a/server/src/services/recovery/service.ts +++ b/server/src/services/recovery/service.ts @@ -2511,7 +2511,7 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) eq(issueRecoveryActions.kind, "issue_graph_liveness"), eq(issueRecoveryActions.cause, "issue_graph_liveness"), eq(issueRecoveryActions.fingerprint, livenessRecoveryLeafFingerprint(finding)), - inArray(issueRecoveryActions.status, ["resolved", "cancelled"]), + eq(issueRecoveryActions.status, "resolved"), ), ) .orderBy(desc(issueRecoveryActions.resolvedAt), desc(issueRecoveryActions.updatedAt)) diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index ce165a3d0a4..2e79e047e82 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -3010,6 +3010,7 @@ export function IssueDetail() { void resolveRecoveryAction.mutateAsync({ actionId, outcome: "blocked", sourceIssueStatus: "blocked" }); return; case "delegated": + // The API also permits "blocked" for delegated recovery when a first-class blocker should keep the source parked. void resolveRecoveryAction.mutateAsync({ actionId, outcome: "delegated",