From d354ecadfa3b0dfd5d63f43dcc54d827e045a955 Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 29 Apr 2026 10:26:27 -0500 Subject: [PATCH] PAP-2820 Enforce issue transition dispositions --- .../__tests__/issue-transition-guard.test.ts | 166 +++++++++++ server/src/services/issue-transition-guard.ts | 277 ++++++++++++++++++ server/src/services/issues.ts | 19 ++ 3 files changed, 462 insertions(+) create mode 100644 server/src/__tests__/issue-transition-guard.test.ts create mode 100644 server/src/services/issue-transition-guard.ts diff --git a/server/src/__tests__/issue-transition-guard.test.ts b/server/src/__tests__/issue-transition-guard.test.ts new file mode 100644 index 00000000000..b31cbf1c21d --- /dev/null +++ b/server/src/__tests__/issue-transition-guard.test.ts @@ -0,0 +1,166 @@ +import { randomUUID } from "node:crypto"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { + agents, + companies, + createDb, + issueRelations, + issues, +} from "@paperclipai/db"; +import { + getEmbeddedPostgresTestSupport, + startEmbeddedPostgresTestDatabase, +} from "./helpers/embedded-postgres.js"; +import { issueService } from "../services/issues.js"; + +const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; + +if (!embeddedPostgresSupport.supported) { + console.warn( + `Skipping embedded Postgres issue transition guard tests on this host: ${ + embeddedPostgresSupport.reason ?? "unsupported environment" + }`, + ); +} + +describeEmbeddedPostgres("issue transition guard", () => { + let db!: ReturnType; + let tempDb: Awaited> | null = null; + + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-issue-transition-guard-"); + db = createDb(tempDb.connectionString); + }, 20_000); + + afterEach(async () => { + await db.delete(issueRelations); + await db.delete(issues); + await db.delete(agents); + await db.delete(companies); + }); + + afterAll(async () => { + await tempDb?.cleanup(); + }); + + async function seedCompanyAndAgents() { + const companyId = randomUUID(); + const coderAgentId = randomUUID(); + const reviewerAgentId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + await db.insert(agents).values([ + { + id: coderAgentId, + companyId, + name: "CodexCoder", + role: "engineer", + status: "active", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: {}, + permissions: {}, + }, + { + id: reviewerAgentId, + companyId, + name: "Reviewer", + role: "qa", + status: "active", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: {}, + permissions: {}, + }, + ]); + return { companyId, coderAgentId, reviewerAgentId }; + } + + it("rejects bare agent-owned in_review creates", async () => { + const { companyId, coderAgentId } = await seedCompanyAndAgents(); + const svc = issueService(db); + + await expect( + svc.create(companyId, { + title: "Bare review", + status: "in_review", + assigneeAgentId: coderAgentId, + }), + ).rejects.toMatchObject({ + status: 422, + message: "Invalid issue transition: in_review_without_action_path", + }); + }); + + it("rejects blocked updates without blockers or a wait path", async () => { + const { companyId, coderAgentId } = await seedCompanyAndAgents(); + const svc = issueService(db); + const issue = await svc.create(companyId, { + title: "Needs a real blocker", + status: "todo", + assigneeAgentId: coderAgentId, + }); + + await expect(svc.update(issue.id, { status: "blocked" })).rejects.toMatchObject({ + status: 422, + message: "Invalid issue transition: blocked_without_action_path", + }); + }); + + it("accepts blocked writes with a healthy first-class blocker chain", async () => { + const { companyId, coderAgentId } = await seedCompanyAndAgents(); + const svc = issueService(db); + const blocker = await svc.create(companyId, { + title: "Do this first", + status: "todo", + assigneeAgentId: coderAgentId, + }); + + const issue = await svc.create(companyId, { + title: "Wait on blocker", + status: "blocked", + assigneeAgentId: coderAgentId, + blockedByIssueIds: [blocker.id], + }); + + expect(issue.status).toBe("blocked"); + }); + + it("accepts workflow-normalized review state with a typed participant", async () => { + const { companyId, coderAgentId, reviewerAgentId } = await seedCompanyAndAgents(); + const svc = issueService(db); + const issue = await svc.create(companyId, { + title: "Review me", + status: "in_progress", + assigneeAgentId: coderAgentId, + }); + + const updated = await svc.update(issue.id, { + status: "in_review", + assigneeAgentId: reviewerAgentId, + executionState: { + status: "pending", + currentStageId: randomUUID(), + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: reviewerAgentId, userId: null }, + returnAssignee: { type: "agent", agentId: coderAgentId, userId: null }, + reviewRequest: null, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + }); + + expect(updated).toMatchObject({ + id: issue.id, + status: "in_review", + assigneeAgentId: reviewerAgentId, + }); + }); +}); diff --git a/server/src/services/issue-transition-guard.ts b/server/src/services/issue-transition-guard.ts new file mode 100644 index 00000000000..2f4fe747884 --- /dev/null +++ b/server/src/services/issue-transition-guard.ts @@ -0,0 +1,277 @@ +import { and, desc, eq, inArray, isNull, notInArray, sql } from "drizzle-orm"; +import type { Db } from "@paperclipai/db"; +import { + agentWakeupRequests, + agents, + approvals, + heartbeatRuns, + issueApprovals, + issueRelations, + issueThreadInteractions, + issues, +} from "@paperclipai/db"; +import type { IssueExecutionDisposition } from "@paperclipai/shared"; +import { unprocessable } from "../errors.js"; +import { + classifyIssueExecutionDisposition, + type IssueExecutionParticipantState, + type IssueExecutionStateVector, +} from "./issue-execution-disposition.js"; +import { parseIssueExecutionState } from "./issue-execution-policy.js"; +import { + isLiveExplicitApprovalWaitingPath, + isLiveExplicitInteractionWaitingPath, +} from "./recovery/explicit-waiting-paths.js"; + +const ACTIVE_RUN_STATUSES = ["queued", "running"] as const; +const SCHEDULED_RETRY_RUN_STATUSES = ["scheduled_retry"] as const; +const ACTIVE_WAKE_STATUSES = ["queued", "deferred_issue_execution"] as const; +const PENDING_INTERACTION_STATUSES = ["pending"] as const; +const WAITING_INTERACTION_KINDS = ["request_confirmation", "ask_user_questions"] as const; +const PENDING_APPROVAL_STATUSES = ["pending", "revision_requested"] as const; +const OPEN_RECOVERY_ORIGIN_KINDS = ["harness_liveness_escalation", "stranded_issue_recovery"] as const; +const OPEN_RECOVERY_TERMINAL_STATUSES = ["done", "cancelled"] as const; + +type DbReader = Pick; +type IssueRow = typeof issues.$inferSelect; + +export type IssueTransitionGuardSource = "create" | "update"; + +function isValidParticipant(participant: unknown) { + if (!participant || typeof participant !== "object") return false; + const value = participant as Record; + if (value.type === "agent") return typeof value.agentId === "string" && value.agentId.length > 0; + if (value.type === "user") return typeof value.userId === "string" && value.userId.length > 0; + return false; +} + +function isGuardedStatus(status: string | null | undefined) { + return status === "in_progress" || status === "in_review" || status === "blocked"; +} + +function invalidMessage(disposition: Extract) { + return `Invalid issue transition: ${disposition.reason}`; +} + +async function buildIssueExecutionStateVector( + dbOrTx: DbReader, + issue: IssueRow, +): Promise { + const [agent] = issue.assigneeAgentId + ? await dbOrTx + .select({ + id: agents.id, + status: agents.status, + }) + .from(agents) + .where(and(eq(agents.companyId, issue.companyId), eq(agents.id, issue.assigneeAgentId))) + : []; + + const [executionRun] = issue.executionRunId + ? await dbOrTx + .select({ status: heartbeatRuns.status }) + .from(heartbeatRuns) + .where(and(eq(heartbeatRuns.companyId, issue.companyId), eq(heartbeatRuns.id, issue.executionRunId))) + : []; + + const [queuedWake] = await dbOrTx + .select({ + issueId: sql`${agentWakeupRequests.payload} ->> 'issueId'`, + status: agentWakeupRequests.status, + }) + .from(agentWakeupRequests) + .where( + and( + eq(agentWakeupRequests.companyId, issue.companyId), + inArray(agentWakeupRequests.status, [...ACTIVE_WAKE_STATUSES]), + sql`${agentWakeupRequests.runId} is null`, + eq(sql`${agentWakeupRequests.payload} ->> 'issueId'`, issue.id), + ), + ) + .orderBy(desc(agentWakeupRequests.createdAt), desc(agentWakeupRequests.id)) + .limit(1); + + const executionState = parseIssueExecutionState(issue.executionState); + let participant: IssueExecutionParticipantState = "none"; + if (executionState?.status === "pending") { + participant = isValidParticipant(executionState.currentParticipant) ? "valid" : "invalid"; + } + + let pendingInteraction: "none" | "live" = "none"; + const interactionRows: Array<{ + issueId: string; + companyId: string; + status: string; + kind: string; + createdByUserId: string | null; + createdAt: Date; + }> = await dbOrTx + .select({ + issueId: issueThreadInteractions.issueId, + companyId: issueThreadInteractions.companyId, + status: issueThreadInteractions.status, + kind: issueThreadInteractions.kind, + createdByUserId: issueThreadInteractions.createdByUserId, + createdAt: issueThreadInteractions.createdAt, + }) + .from(issueThreadInteractions) + .where( + and( + eq(issueThreadInteractions.companyId, issue.companyId), + eq(issueThreadInteractions.issueId, issue.id), + inArray(issueThreadInteractions.status, [...PENDING_INTERACTION_STATUSES]), + inArray(issueThreadInteractions.kind, [...WAITING_INTERACTION_KINDS]), + ), + ); + for (const row of interactionRows) { + if (isLiveExplicitInteractionWaitingPath(issue, row)) { + pendingInteraction = "live"; + break; + } + } + + let pendingApproval: "none" | "live" = "none"; + const approvalRows: Array<{ + issueId: string; + companyId: string; + status: string; + requestedByUserId: string | null; + linkedByUserId: string | null; + createdAt: Date; + updatedAt: Date; + linkedAt: Date; + }> = await dbOrTx + .select({ + issueId: issueApprovals.issueId, + companyId: issueApprovals.companyId, + status: approvals.status, + requestedByUserId: approvals.requestedByUserId, + linkedByUserId: issueApprovals.linkedByUserId, + createdAt: approvals.createdAt, + updatedAt: approvals.updatedAt, + linkedAt: issueApprovals.createdAt, + }) + .from(issueApprovals) + .innerJoin(approvals, eq(issueApprovals.approvalId, approvals.id)) + .where( + and( + eq(issueApprovals.companyId, issue.companyId), + eq(issueApprovals.issueId, issue.id), + inArray(approvals.status, [...PENDING_APPROVAL_STATUSES]), + ), + ); + for (const row of approvalRows) { + if (isLiveExplicitApprovalWaitingPath(issue, row)) { + pendingApproval = "live"; + break; + } + } + + const recoveryRows: Array<{ id: string }> = await dbOrTx + .select({ id: issues.id }) + .from(issues) + .where( + and( + eq(issues.companyId, issue.companyId), + inArray(issues.originKind, [...OPEN_RECOVERY_ORIGIN_KINDS]), + eq(issues.originId, issue.id), + isNull(issues.hiddenAt), + notInArray(issues.status, [...OPEN_RECOVERY_TERMINAL_STATUSES]), + ), + ) + .limit(1); + + const blockerRows: Array<{ + id: string; + status: string; + assigneeAgentId: string | null; + assigneeUserId: string | null; + originKind: string | null; + }> = await dbOrTx + .select({ + id: issues.id, + status: issues.status, + assigneeAgentId: issues.assigneeAgentId, + assigneeUserId: issues.assigneeUserId, + originKind: issues.originKind, + }) + .from(issueRelations) + .innerJoin(issues, eq(issueRelations.issueId, issues.id)) + .where( + and( + eq(issueRelations.companyId, issue.companyId), + eq(issueRelations.relatedIssueId, issue.id), + eq(issueRelations.type, "blocks"), + eq(issues.companyId, issue.companyId), + ), + ); + + return { + issue: { + id: issue.id, + status: issue.status as IssueExecutionStateVector["issue"]["status"], + assigneeAgentId: issue.assigneeAgentId, + assigneeUserId: issue.assigneeUserId, + originKind: issue.originKind, + }, + agent: agent + ? { + id: agent.id, + status: agent.status, + } + : null, + execution: { + activeRun: executionRun ? (ACTIVE_RUN_STATUSES as readonly string[]).includes(executionRun.status) : false, + scheduledRetry: executionRun + ? (SCHEDULED_RETRY_RUN_STATUSES as readonly string[]).includes(executionRun.status) + : false, + queuedWake: queuedWake?.status === "queued", + deferredExecution: queuedWake?.status === "deferred_issue_execution", + }, + waits: { + participant, + pendingInteraction, + pendingApproval, + openRecoveryIssue: recoveryRows.length > 0 || + blockerRows.some((blocker) => + (OPEN_RECOVERY_ORIGIN_KINDS as readonly string[]).includes(blocker.originKind ?? "") + ), + }, + blockers: blockerRows.map((blocker) => ({ + issue: { + id: blocker.id, + status: blocker.status as IssueExecutionStateVector["issue"]["status"], + assigneeAgentId: blocker.assigneeAgentId, + assigneeUserId: blocker.assigneeUserId, + }, + })), + }; +} + +export async function assertIssueTransitionAllowed(input: { + db: DbReader; + issue: IssueRow; + source: IssueTransitionGuardSource; + statusTouched?: boolean; + assigneeTouched?: boolean; + blockerTouched?: boolean; + executionStateTouched?: boolean; +}) { + const shouldGuard = + isGuardedStatus(input.issue.status) && + (input.source === "create" || + input.statusTouched || + input.assigneeTouched || + input.blockerTouched || + input.executionStateTouched); + if (!shouldGuard) return; + + const stateVector = await buildIssueExecutionStateVector(input.db, input.issue); + const disposition = classifyIssueExecutionDisposition(stateVector); + if (disposition.kind !== "invalid") return; + + throw unprocessable(invalidMessage(disposition), { + disposition, + suggestedCorrection: disposition.suggestedCorrection, + }); +} diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index b6e57fd2120..256e13d1118 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -59,6 +59,7 @@ import { isLiveExplicitApprovalWaitingPath, isLiveExplicitInteractionWaitingPath, } from "./recovery/explicit-waiting-paths.js"; +import { assertIssueTransitionAllowed } from "./issue-transition-guard.js"; const ALL_ISSUE_STATUSES = ["backlog", "todo", "in_progress", "in_review", "blocked", "done", "cancelled"]; const MAX_ISSUE_COMMENT_PAGE_LIMIT = 500; @@ -3230,6 +3231,15 @@ export function issueService(db: Db) { tx, ); } + await assertIssueTransitionAllowed({ + db: tx, + issue, + source: "create", + statusTouched: true, + assigneeTouched: issueData.assigneeAgentId !== undefined || issueData.assigneeUserId !== undefined, + blockerTouched: blockedByIssueIds !== undefined, + executionStateTouched: issueData.executionState !== undefined, + }); const [enriched] = await withIssueLabels(tx, [issue]); return enriched; }); @@ -3383,6 +3393,15 @@ export function issueService(db: Db) { tx, ); } + await assertIssueTransitionAllowed({ + db: tx, + issue: updated, + source: "update", + statusTouched: issueData.status !== undefined, + assigneeTouched: issueData.assigneeAgentId !== undefined || issueData.assigneeUserId !== undefined, + blockerTouched: blockedByIssueIds !== undefined, + executionStateTouched: issueData.executionState !== undefined, + }); const [enriched] = await withIssueLabels(tx, [updated]); return enriched; };