diff --git a/server/src/__tests__/issue-liveness.test.ts b/server/src/__tests__/issue-liveness.test.ts index bda50dfc761..592244fa109 100644 --- a/server/src/__tests__/issue-liveness.test.ts +++ b/server/src/__tests__/issue-liveness.test.ts @@ -234,4 +234,159 @@ describe("issue graph liveness classifier", () => { incidentKey: `harness_liveness:${companyId}:${blockedId}:invalid_review_participant:missing-agent`, }); }); + + it("detects the PAP-2239-style blocked chain at the first stalled in_review leaf without duplicate findings", () => { + const phaseIssueId = "phase-issue-1"; + const reviewLeafId = "review-leaf-1"; + + const findings = classifyIssueGraphLiveness({ + issues: [ + issue({ + id: "pap-2239", + identifier: "PAP-2239", + title: "External object reference project", + status: "blocked", + }), + issue({ + id: phaseIssueId, + identifier: "PAP-2276", + title: "UX acceptance review phase", + status: "blocked", + assigneeAgentId: coderId, + }), + issue({ + id: reviewLeafId, + identifier: "PAP-2279", + title: "Screenshot acceptance review", + status: "in_review", + assigneeAgentId: coderId, + executionState: null, + }), + ], + relations: [ + { companyId, blockerIssueId: phaseIssueId, blockedIssueId: "pap-2239" }, + { companyId, blockerIssueId: reviewLeafId, blockedIssueId: phaseIssueId }, + ], + agents: [agent(), manager], + }); + + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + issueId: "pap-2239", + identifier: "PAP-2239", + state: "in_review_without_action_path", + recoveryIssueId: reviewLeafId, + recommendedOwnerAgentId: coderId, + dependencyPath: [ + expect.objectContaining({ issueId: "pap-2239" }), + expect.objectContaining({ issueId: phaseIssueId }), + expect.objectContaining({ issueId: reviewLeafId }), + ], + incidentKey: `harness_liveness:${companyId}:pap-2239:in_review_without_action_path:${reviewLeafId}`, + }); + }); + + it("does not flag healthy in_review issues with an explicit action path", () => { + const reviewIssueId = "review-1"; + const baseReviewIssue = issue({ + id: reviewIssueId, + identifier: "PAP-2279", + title: "Screenshot acceptance review", + status: "in_review", + assigneeAgentId: coderId, + executionState: null, + }); + + const cases = [ + { + name: "typed agent participant", + issue: { + ...baseReviewIssue, + executionState: { + currentParticipant: { type: "agent", agentId: coderId }, + }, + }, + }, + { + name: "typed user participant", + issue: { + ...baseReviewIssue, + executionState: { + currentParticipant: { type: "user", userId: "board-user-1" }, + }, + }, + }, + { + name: "user owner", + issue: { ...baseReviewIssue, assigneeAgentId: null, assigneeUserId: "board-user-1" }, + }, + { + name: "active run", + issue: baseReviewIssue, + activeRuns: [{ companyId, issueId: reviewIssueId, agentId: coderId, status: "running" }], + }, + { + name: "queued wake", + issue: baseReviewIssue, + queuedWakeRequests: [{ companyId, issueId: reviewIssueId, agentId: coderId, status: "queued" }], + }, + { + name: "pending interaction", + issue: baseReviewIssue, + pendingInteractions: [{ companyId, issueId: reviewIssueId, status: "pending" }], + }, + { + name: "pending approval", + issue: baseReviewIssue, + pendingApprovals: [{ companyId, issueId: reviewIssueId, status: "pending" }], + }, + { + name: "open recovery issue", + issue: baseReviewIssue, + openRecoveryIssues: [{ companyId, issueId: reviewIssueId, status: "todo" }], + }, + ]; + + for (const testCase of cases) { + const findings = classifyIssueGraphLiveness({ + issues: [testCase.issue], + relations: [], + agents: [agent(), manager], + activeRuns: testCase.activeRuns, + queuedWakeRequests: testCase.queuedWakeRequests, + pendingInteractions: testCase.pendingInteractions, + pendingApprovals: testCase.pendingApprovals, + openRecoveryIssues: testCase.openRecoveryIssues, + }); + + expect(findings, testCase.name).toEqual([]); + } + }); + + it("ignores cross-company waiting paths for stalled in_review issues", () => { + const reviewIssueId = "review-1"; + + const findings = classifyIssueGraphLiveness({ + issues: [ + issue({ + id: reviewIssueId, + identifier: "PAP-2279", + title: "Screenshot acceptance review", + status: "in_review", + assigneeAgentId: coderId, + executionState: null, + }), + ], + relations: [], + agents: [agent(), manager], + pendingInteractions: [{ companyId: "other-company", issueId: reviewIssueId, status: "pending" }], + openRecoveryIssues: [{ companyId: "other-company", issueId: reviewIssueId, status: "todo" }], + }); + + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + state: "in_review_without_action_path", + recoveryIssueId: reviewIssueId, + }); + }); }); diff --git a/server/src/services/recovery/issue-graph-liveness.ts b/server/src/services/recovery/issue-graph-liveness.ts index 598d9ca3f84..68c64183e76 100644 --- a/server/src/services/recovery/issue-graph-liveness.ts +++ b/server/src/services/recovery/issue-graph-liveness.ts @@ -6,7 +6,8 @@ export type IssueLivenessState = | "blocked_by_unassigned_issue" | "blocked_by_uninvokable_assignee" | "blocked_by_cancelled_issue" - | "invalid_review_participant"; + | "invalid_review_participant" + | "in_review_without_action_path"; export interface IssueLivenessIssueInput { id: string; @@ -47,6 +48,12 @@ export interface IssueLivenessExecutionPathInput { status: string; } +export interface IssueLivenessWaitingPathInput { + companyId: string; + issueId: string; + status: string; +} + export interface IssueLivenessDependencyPathEntry { issueId: string; identifier: string | null; @@ -89,6 +96,9 @@ export interface IssueGraphLivenessInput { agents: IssueLivenessAgentInput[]; activeRuns?: IssueLivenessExecutionPathInput[]; queuedWakeRequests?: IssueLivenessExecutionPathInput[]; + pendingInteractions?: IssueLivenessWaitingPathInput[]; + pendingApprovals?: IssueLivenessWaitingPathInput[]; + openRecoveryIssues?: IssueLivenessWaitingPathInput[]; } const INVOKABLE_AGENT_STATUSES = new Set(["active", "idle", "running", "error"]); @@ -122,6 +132,14 @@ function hasActiveExecutionPath( ); } +function hasWaitingPath( + companyId: string, + issueId: string, + waitingPaths: IssueLivenessWaitingPathInput[], +) { + return waitingPaths.some((entry) => entry.companyId === companyId && entry.issueId === issueId); +} + function readPrincipalAgentId(principal: unknown): string | null { if (!principal || typeof principal !== "object") return null; const value = principal as Record; @@ -293,120 +311,225 @@ export function classifyIssueGraphLiveness(input: IssueGraphLivenessInput): Issu const issuesById = new Map(input.issues.map((issue) => [issue.id, issue])); const agentsById = new Map(input.agents.map((agent) => [agent.id, agent])); const blockersByBlockedIssueId = new Map(); + const unresolvedBlockers = new Set(); const findings: IssueLivenessFinding[] = []; const activeRuns = input.activeRuns ?? []; const queuedWakeRequests = input.queuedWakeRequests ?? []; + const pendingInteractions = input.pendingInteractions ?? []; + const pendingApprovals = input.pendingApprovals ?? []; + const openRecoveryIssues = input.openRecoveryIssues ?? []; for (const relation of input.relations) { const list = blockersByBlockedIssueId.get(relation.blockedIssueId) ?? []; list.push(relation); blockersByBlockedIssueId.set(relation.blockedIssueId, list); + + const blocker = issuesById.get(relation.blockerIssueId); + const blocked = issuesById.get(relation.blockedIssueId); + if ( + blocker && + blocked && + blocker.companyId === relation.companyId && + blocked.companyId === relation.companyId && + blocker.status !== "done" && + blocker.status !== "cancelled" && + blocked.status === "blocked" + ) { + unresolvedBlockers.add(blocker.id); + } + } + + for (const relations of blockersByBlockedIssueId.values()) { + relations.sort((left, right) => { + const leftIssue = issuesById.get(left.blockerIssueId); + const rightIssue = issuesById.get(right.blockerIssueId); + const leftLabel = leftIssue ? issueLabel(leftIssue) : left.blockerIssueId; + const rightLabel = rightIssue ? issueLabel(rightIssue) : right.blockerIssueId; + return leftLabel.localeCompare(rightLabel); + }); + } + + function hasExplicitWaitingPath(issue: IssueLivenessIssueInput) { + return Boolean(issue.assigneeUserId) || + hasActiveExecutionPath(issue.companyId, issue.id, activeRuns, queuedWakeRequests) || + hasWaitingPath(issue.companyId, issue.id, pendingInteractions) || + hasWaitingPath(issue.companyId, issue.id, pendingApprovals) || + hasWaitingPath(issue.companyId, issue.id, openRecoveryIssues); + } + + function reviewFinding( + source: IssueLivenessIssueInput, + reviewIssue: IssueLivenessIssueInput, + dependencyPath: IssueLivenessIssueInput[], + ): IssueLivenessFinding | null { + if (reviewIssue.status !== "in_review") return null; + if (hasExplicitWaitingPath(reviewIssue)) return null; + + const ownerCandidates = ownerCandidatesForRecoveryIssue(reviewIssue, input.agents, agentsById, { + includeStalledAssignee: true, + }); + + const participant = reviewIssue.executionState?.currentParticipant; + const participantAgentId = readPrincipalAgentId(participant); + if (participantAgentId) { + const participantAgent = agentsById.get(participantAgentId); + if (isInvokableAgent(participantAgent) && participantAgent?.companyId === reviewIssue.companyId) return null; + + return finding({ + issue: source, + state: "invalid_review_participant", + reason: participantAgent + ? `${issueLabel(reviewIssue)} is in review, but current participant agent is ${participantAgent.status}.` + : `${issueLabel(reviewIssue)} is in review, but current participant agent cannot be resolved.`, + dependencyPath, + recoveryIssue: reviewIssue, + recommendedOwnerCandidateAgentIds: ownerCandidates.map((candidate) => candidate.agentId), + recommendedOwnerCandidates: ownerCandidates, + recommendedAction: + `Repair ${issueLabel(reviewIssue)}'s review participant or return the issue to an active assignee with a clear change request.`, + participantAgentId, + }); + } + + if (principalIsResolvableUser(participant)) return null; + + if (reviewIssue.executionState) { + return finding({ + issue: source, + state: "invalid_review_participant", + reason: `${issueLabel(reviewIssue)} is in review, but its current participant cannot be resolved.`, + dependencyPath, + recoveryIssue: reviewIssue, + recommendedOwnerCandidateAgentIds: ownerCandidates.map((candidate) => candidate.agentId), + recommendedOwnerCandidates: ownerCandidates, + recommendedAction: + `Repair ${issueLabel(reviewIssue)}'s review participant or return the issue to an active assignee with a clear change request.`, + }); + } + + if (!reviewIssue.assigneeAgentId || reviewIssue.assigneeUserId) return null; + + return finding({ + issue: source, + state: "in_review_without_action_path", + reason: `${issueLabel(reviewIssue)} is in review with an agent assignee but no participant, interaction, approval, user owner, wake, active run, or recovery issue owning the next action.`, + dependencyPath, + recoveryIssue: reviewIssue, + recommendedOwnerCandidateAgentIds: ownerCandidates.map((candidate) => candidate.agentId), + recommendedOwnerCandidates: ownerCandidates, + recommendedAction: + `Review ${issueLabel(reviewIssue)} and make the next action explicit: add a reviewer/interaction, return it to active work with a change request, mark it done if accepted, or open a bounded recovery issue.`, + blockerIssueId: reviewIssue.id, + }); + } + + function blockedFindingForLeaf( + source: IssueLivenessIssueInput, + blocker: IssueLivenessIssueInput, + dependencyPath: IssueLivenessIssueInput[], + ): IssueLivenessFinding | null { + const ownerCandidates = ownerCandidatesForRecoveryIssue(blocker, input.agents, agentsById, { + includeStalledAssignee: true, + }); + + if (blocker.status === "cancelled") { + return finding({ + issue: source, + state: "blocked_by_cancelled_issue", + reason: `${issueLabel(source)} is still blocked by cancelled issue ${issueLabel(blocker)}.`, + dependencyPath, + recoveryIssue: blocker, + recommendedOwnerCandidateAgentIds: ownerCandidates.map((candidate) => candidate.agentId), + recommendedOwnerCandidates: ownerCandidates, + recommendedAction: + `Inspect ${issueLabel(blocker)} and either remove it from ${issueLabel(source)}'s blockers or replace it with an actionable unblock issue.`, + blockerIssueId: blocker.id, + }); + } + + if (hasExplicitWaitingPath(blocker)) return null; + + if (blocker.status === "in_review") { + return reviewFinding(source, blocker, dependencyPath); + } + + if (!blocker.assigneeAgentId && !blocker.assigneeUserId) { + return finding({ + issue: source, + state: "blocked_by_unassigned_issue", + reason: `${issueLabel(source)} is blocked by unassigned issue ${issueLabel(blocker)} with no user owner.`, + dependencyPath, + recoveryIssue: blocker, + recommendedOwnerCandidateAgentIds: ownerCandidates.map((candidate) => candidate.agentId), + recommendedOwnerCandidates: ownerCandidates, + recommendedAction: + `Assign ${issueLabel(blocker)} to an owner who can complete it, or remove it from ${issueLabel(source)}'s blockers if it is no longer required.`, + blockerIssueId: blocker.id, + }); + } + + if (!blocker.assigneeAgentId) return null; + + const blockerAgent = agentsById.get(blocker.assigneeAgentId); + if (!blockerAgent || blockerAgent.companyId !== source.companyId || BLOCKING_AGENT_STATUSES.has(blockerAgent.status)) { + return finding({ + issue: source, + state: "blocked_by_uninvokable_assignee", + reason: blockerAgent + ? `${issueLabel(source)} is blocked by ${issueLabel(blocker)}, but its assignee is ${blockerAgent.status}.` + : `${issueLabel(source)} is blocked by ${issueLabel(blocker)}, but its assignee no longer exists.`, + dependencyPath, + recoveryIssue: blocker, + recommendedOwnerCandidateAgentIds: ownerCandidates.map((candidate) => candidate.agentId), + recommendedOwnerCandidates: ownerCandidates, + recommendedAction: + `Review ${issueLabel(blocker)} and assign it to an active owner or replace the blocker with an actionable issue.`, + blockerIssueId: blocker.id, + }); + } + + return null; + } + + function firstBlockedChainFinding( + source: IssueLivenessIssueInput, + current: IssueLivenessIssueInput, + dependencyPath: IssueLivenessIssueInput[], + seen: Set, + ): IssueLivenessFinding | null { + if (seen.has(current.id)) return null; + seen.add(current.id); + + const relations = blockersByBlockedIssueId.get(current.id) ?? []; + for (const relation of relations) { + if (relation.companyId !== current.companyId || relation.companyId !== source.companyId) continue; + const blocker = issuesById.get(relation.blockerIssueId); + if (!blocker || blocker.companyId !== source.companyId || blocker.status === "done") continue; + const path = [...dependencyPath, blocker]; + + if (blocker.status === "blocked") { + const nested = firstBlockedChainFinding(source, blocker, path, new Set(seen)); + if (nested) return nested; + if (hasExplicitWaitingPath(blocker)) continue; + } + + const leafFinding = blockedFindingForLeaf(source, blocker, path); + if (leafFinding) return leafFinding; + } + + return null; } for (const issue of input.issues) { if (issue.status === "blocked") { - const relations = blockersByBlockedIssueId.get(issue.id) ?? []; - for (const relation of relations) { - if (relation.companyId !== issue.companyId) continue; - const blocker = issuesById.get(relation.blockerIssueId); - if (!blocker || blocker.companyId !== issue.companyId || blocker.status === "done") continue; - const ownerCandidates = ownerCandidatesForRecoveryIssue(blocker, input.agents, agentsById, { - includeStalledAssignee: true, - }); - - if (blocker.status === "cancelled") { - findings.push(finding({ - issue, - state: "blocked_by_cancelled_issue", - reason: `${issueLabel(issue)} is still blocked by cancelled issue ${issueLabel(blocker)}.`, - dependencyPath: [issue, blocker], - recoveryIssue: blocker, - recommendedOwnerCandidateAgentIds: ownerCandidates.map((candidate) => candidate.agentId), - recommendedOwnerCandidates: ownerCandidates, - recommendedAction: - `Inspect ${issueLabel(blocker)} and either remove it from ${issueLabel(issue)}'s blockers or replace it with an actionable unblock issue.`, - blockerIssueId: blocker.id, - })); - continue; - } - - if (!blocker.assigneeAgentId && !blocker.assigneeUserId) { - if (hasActiveExecutionPath(issue.companyId, blocker.id, activeRuns, queuedWakeRequests)) continue; - findings.push(finding({ - issue, - state: "blocked_by_unassigned_issue", - reason: `${issueLabel(issue)} is blocked by unassigned issue ${issueLabel(blocker)} with no user owner.`, - dependencyPath: [issue, blocker], - recoveryIssue: blocker, - recommendedOwnerCandidateAgentIds: ownerCandidates.map((candidate) => candidate.agentId), - recommendedOwnerCandidates: ownerCandidates, - recommendedAction: - `Assign ${issueLabel(blocker)} to an owner who can complete it, or remove it from ${issueLabel(issue)}'s blockers if it is no longer required.`, - blockerIssueId: blocker.id, - })); - continue; - } - - if (!blocker.assigneeAgentId) continue; - if (hasActiveExecutionPath(issue.companyId, blocker.id, activeRuns, queuedWakeRequests)) continue; - - const blockerAgent = agentsById.get(blocker.assigneeAgentId); - if (!blockerAgent || blockerAgent.companyId !== issue.companyId || BLOCKING_AGENT_STATUSES.has(blockerAgent.status)) { - findings.push(finding({ - issue, - state: "blocked_by_uninvokable_assignee", - reason: blockerAgent - ? `${issueLabel(issue)} is blocked by ${issueLabel(blocker)}, but its assignee is ${blockerAgent.status}.` - : `${issueLabel(issue)} is blocked by ${issueLabel(blocker)}, but its assignee no longer exists.`, - dependencyPath: [issue, blocker], - recoveryIssue: blocker, - recommendedOwnerCandidateAgentIds: ownerCandidates.map((candidate) => candidate.agentId), - recommendedOwnerCandidates: ownerCandidates, - recommendedAction: - `Review ${issueLabel(blocker)} and assign it to an active owner or replace the blocker with an actionable issue.`, - blockerIssueId: blocker.id, - })); - } - } + if (unresolvedBlockers.has(issue.id)) continue; + const chainFinding = firstBlockedChainFinding(issue, issue, [issue], new Set()); + if (chainFinding) findings.push(chainFinding); } - if (issue.status !== "in_review" || !issue.executionState) continue; - const ownerCandidates = ownerCandidatesForRecoveryIssue(issue, input.agents, agentsById); - const participant = issue.executionState.currentParticipant; - const participantAgentId = readPrincipalAgentId(participant); - if (participantAgentId) { - const participantAgent = agentsById.get(participantAgentId); - if (!isInvokableAgent(participantAgent) || participantAgent?.companyId !== issue.companyId) { - findings.push(finding({ - issue, - state: "invalid_review_participant", - reason: participantAgent - ? `${issueLabel(issue)} is in review, but current participant agent is ${participantAgent.status}.` - : `${issueLabel(issue)} is in review, but current participant agent cannot be resolved.`, - dependencyPath: [issue], - recoveryIssue: issue, - recommendedOwnerCandidateAgentIds: ownerCandidates.map((candidate) => candidate.agentId), - recommendedOwnerCandidates: ownerCandidates, - recommendedAction: - `Repair ${issueLabel(issue)}'s review participant or return the issue to an active assignee with a clear change request.`, - participantAgentId, - })); - } - continue; - } - - if (!principalIsResolvableUser(participant)) { - findings.push(finding({ - issue, - state: "invalid_review_participant", - reason: `${issueLabel(issue)} is in review, but its current participant cannot be resolved.`, - dependencyPath: [issue], - recoveryIssue: issue, - recommendedOwnerCandidateAgentIds: ownerCandidates.map((candidate) => candidate.agentId), - recommendedOwnerCandidates: ownerCandidates, - recommendedAction: - `Repair ${issueLabel(issue)}'s review participant or return the issue to an active assignee with a clear change request.`, - })); + if (issue.status === "in_review" && !unresolvedBlockers.has(issue.id)) { + const review = reviewFinding(issue, issue, [issue]); + if (review) findings.push(review); } } diff --git a/server/src/services/recovery/service.ts b/server/src/services/recovery/service.ts index f20dd05915f..30cc552eb85 100644 --- a/server/src/services/recovery/service.ts +++ b/server/src/services/recovery/service.ts @@ -3,11 +3,14 @@ import type { Db } from "@paperclipai/db"; import { agents, agentWakeupRequests, + approvals, companies, heartbeatRunEvents, heartbeatRunWatchdogDecisions, heartbeatRuns, + issueApprovals, issueRelations, + issueThreadInteractions, issues, } from "@paperclipai/db"; import { parseObject, asBoolean, asNumber } from "../../adapters/utils.js"; @@ -1540,7 +1543,17 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) } async function collectIssueGraphLivenessFindings() { - const [issueRows, relationRows, agentRows, activeRunRows, activeIssueRunRows, wakeRows] = await Promise.all([ + const [ + issueRows, + relationRows, + agentRows, + activeRunRows, + activeIssueRunRows, + wakeRows, + interactionRows, + approvalRows, + recoveryIssueRows, + ] = await Promise.all([ db .select({ id: issues.id, @@ -1617,8 +1630,50 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) }) .from(agentWakeupRequests) .where(inArray(agentWakeupRequests.status, ["queued", "deferred_issue_execution"])), + db + .select({ + companyId: issueThreadInteractions.companyId, + issueId: issueThreadInteractions.issueId, + status: issueThreadInteractions.status, + }) + .from(issueThreadInteractions) + .where(eq(issueThreadInteractions.status, "pending")), + db + .select({ + companyId: issueApprovals.companyId, + issueId: issueApprovals.issueId, + status: approvals.status, + }) + .from(issueApprovals) + .innerJoin(approvals, eq(issueApprovals.approvalId, approvals.id)) + .where(inArray(approvals.status, ["pending", "revision_requested"])), + db + .select({ + companyId: issues.companyId, + id: issues.id, + status: issues.status, + originId: issues.originId, + }) + .from(issues) + .where( + and( + isNull(issues.hiddenAt), + eq(issues.originKind, STRANDED_ISSUE_RECOVERY_ORIGIN_KIND), + notInArray(issues.status, ["done", "cancelled"]), + ), + ), ]); + const openRecoveryIssues = recoveryIssueRows.flatMap((row) => { + const issueId = readNonEmptyString(row.originId); + if (!issueId) return []; + return [{ + companyId: row.companyId, + issueId, + status: row.status, + }]; + }); + return classifyIssueGraphLiveness({ issues: issueRows, relations: relationRows, @@ -1640,6 +1695,9 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) status: row.status, issueId: issueIdFromWakePayload(row.payload), })), + pendingInteractions: interactionRows, + pendingApprovals: approvalRows, + openRecoveryIssues, }); }