diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 606e23c0e43..7820c255b28 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -512,6 +512,7 @@ V1 non-terminal liveness rule: - heartbeat finalization evaluates liveness from persisted Paperclip state; an issue cannot remain healthy `in_progress` solely because the exiting heartbeat started a local/background watcher - a continuation cancelled as `issue_continuation_waiting_on_review` first converts open children or blockers into a first-class dependency wait; without a real target it is classified as `deliberate_wait_without_target` and gives the invokable original owner at most three normal-model disposition-repair attempts (immediate, 60 seconds, and 120 seconds, with bounded jitter) - disposition repair revalidates blockers, children, interactions, approvals, monitors, execution stages, queued wakes, active runs, work products, owner invokability, budgets, and governance before every attempt; the attempt bound is keyed by durable source state, so comments or equivalent parked prose do not reset it while durable source-state changes may establish a new fingerprint +- backwards-compatible upgrades count consecutive historical `issue_continuation_waiting_on_review` cancellations for the unchanged accepted-interaction source state against the same three-attempt disposition-repair ceiling; missing pre-upgrade recovery-action rows do not reset the budget - when Paperclip cannot safely infer the next action, it surfaces the problem through visible blocked/recovery work instead of silently completing or reassigning work - explicit recovery actions are the liveness primitive; source-scoped actions are the default form, issue-backed recovery is a fallback for independent repair work or safety boundaries, and comments alone are evidence rather than a healthy liveness path - recovery-action ownership is separate from source-task ownership: automatic repair and manager escalation preserve the source `assigneeAgentId`; reassignment requires an explicit decision or a policy-defined serious failure diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 6fae278098f..c40431ac1ce 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -554,6 +554,7 @@ Recovery rule for a parked-for-review continuation: - if the issue has no waiting target and the original owner is invokable, Paperclip classifies it as `deliberate_wait_without_target` and gives that owner at most three normal-model disposition-repair attempts: immediate, after 60 seconds, and after 120 seconds, with bounded jitter on the delayed attempts - before every attempt, Paperclip revalidates unresolved blockers and children, interactions, linked approvals, monitors, execution stages, queued wakes, active runs, work products, owner invokability, and budget or governance gates. Any real live or waiting path suppresses the retry - the retry bound is keyed by an idempotent durable source-state fingerprint. Comments, repeated parked summaries, and equivalent prose do not reset it. Durable changes such as source status or assignee changes, dependency or interaction changes, approval or execution-policy changes, monitor changes, or work-product changes may create a new fingerprint +- on upgrade, consecutive historical `issue_continuation_waiting_on_review` cancellations for the same accepted interaction and still-unchanged durable source state seed this same counter. Three applicable pre-upgrade parks therefore exhaust the ceiling immediately; the absence of a historical `deliberate_wait_without_target` recovery-action row does not grant three new attempts - after three attempts with the same fingerprint, Paperclip escalates a separate source-scoped recovery action to the manager ladder or board. The source assignee remains unchanged. A non-invokable or budget-blocked owner routes directly to the explicit recovery action without consuming futile source attempts An accepted interaction supersedes a continuation park recorded before that acceptance. A queued continuation carrying a parseable `interactionResolvedAt` must not be cancelled solely because an older continuation summary says to wait for review or approval. Interaction-continuation recovery is bounded: after three consecutive continuation wakes are cancelled without a run starting, recovery converts a real dependency wait when one exists or escalates the missing execution path visibly instead of requeueing forever. diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index bddc5a034d5..32eb78055c1 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -3980,7 +3980,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { }); it("escalates an exhausted failed successful-run handoff without using generic continuation recovery first", async () => { - const { companyId, agentId, issueId } = await seedStrandedIssueFixture({ + const { companyId, agentId, runId, issueId } = await seedStrandedIssueFixture({ status: "in_progress", runStatus: "failed", runErrorCode: "adapter_failed", @@ -4228,7 +4228,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { }); it("converts a continuation parked for review into a dependency wait on its existing blockers", async () => { - const { companyId, agentId, runId, issueId } = await seedStrandedIssueFixture({ + const { companyId, agentId, issueId } = await seedStrandedIssueFixture({ status: "in_progress", runStatus: "cancelled", retryReason: "issue_continuation_needed", @@ -5398,7 +5398,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { }); expect(sourceIssue).toMatchObject({ status: "blocked", - assigneeAgentId: agentId, + assigneeAgentId: sourceAssigneeAgentId, }); const recoveryAction = await expectSourceScopedStrandedRecoveryAction({ @@ -5591,7 +5591,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { }); }); - it("escalates accepted interaction continuation recovery after three review-park cancellations", async () => { + it("counts three historical review-park cancellations against the upgraded disposition-repair ceiling", async () => { const companyId = randomUUID(); const agentId = randomUUID(); const issueId = randomUUID(); @@ -5692,7 +5692,24 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { ]); expect(issue?.status).toBe("blocked"); expect(continuationRuns).toHaveLength(3); - expect(comments.some((comment) => comment.body.includes(interactionId))).toBe(true); + expect(comments.some((comment) => comment.body.includes("Attempts: 3/3"))).toBe(true); + const action = await db + .select() + .from(issueRecoveryActions) + .where(and( + eq(issueRecoveryActions.companyId, companyId), + eq(issueRecoveryActions.sourceIssueId, issueId), + )) + .then((rows) => rows[0] ?? null); + expect(action).toMatchObject({ + kind: "deliberate_wait_without_target", + status: "escalated", + previousOwnerAgentId: agentId, + returnOwnerAgentId: agentId, + attemptCount: 3, + maxAttempts: 3, + resolutionNote: "unchanged_source_state_exhausted", + }); }); it("skips accepted interaction recovery after its continuation succeeds", async () => { diff --git a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts index 950c155e65e..4103989346e 100644 --- a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts +++ b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts @@ -51,6 +51,14 @@ const mockCompanyService = vi.hoisted(() => ({ getById: vi.fn(), })); +const mockBudgetService = vi.hoisted(() => ({ + getInvocationBlock: vi.fn(async () => null), +})); + +const mockProjectService = vi.hoisted(() => ({ + getById: vi.fn(async () => null), +})); + const mockDocumentService = vi.hoisted(() => ({ upsertIssueDocument: vi.fn(), })); @@ -193,6 +201,7 @@ function registerRouteMocks() { ISSUE_LIST_MAX_LIMIT: 500, accessService: () => mockAccessService, agentService: () => mockAgentService, + budgetService: () => mockBudgetService, clampIssueListLimit: (value: number) => Math.min(Math.max(value, 1), 500), companySkillService: () => ({ completeTestRunForIssue: vi.fn(async () => null), @@ -236,7 +245,7 @@ function registerRouteMocks() { issueThreadInteractionService: () => mockIssueThreadInteractionService, taskWatchdogService: () => mockTaskWatchdogService, logActivity: mockLogActivity, - projectService: () => ({}), + projectService: () => mockProjectService, routineService: () => ({ syncRunStatusForIssue: vi.fn(async () => undefined), }), @@ -260,6 +269,8 @@ function makeIssue(overrides: Record = {}) { title: "Owned active issue", executionPolicy: null, executionState: null, + checkoutRunId: null, + executionRunId: null, hiddenAt: null, ...overrides, }; @@ -293,21 +304,27 @@ function createRunContextDb( const firstRun = runRows[0] ?? {}; const runAgentId = typeof firstRun.agentId === "string" ? firstRun.agentId : ownerAgentId; const runAgentCompanyId = typeof firstRun.agentCompanyId === "string" ? firstRun.agentCompanyId : companyId; - const rowsForSelection = (selection: Record) => { + const rowsForSelection = async (selection: Record) => { const keys = Object.keys(selection); if (keys.includes("entityId")) return []; if (keys.includes("contextSnapshot")) return runRows; if (keys.includes("agentCompanyId")) return runRows; + if (keys.length === 0) { + const issue = await mockIssueService.getById(issueId); + return issue ? [issue] : []; + } return [{ id: runAgentId, companyId: runAgentCompanyId, permissions: {}, role: "engineer", reportsTo: null }]; }; const buildQuery = (selection: Record) => { - const rows = rowsForSelection(selection); const whereResult = { orderBy: vi.fn(async () => []), limit: vi.fn(() => ({ - then: async (resolve: (limitedRows: unknown[]) => unknown) => resolve(rows), + then: async (resolve: (limitedRows: unknown[]) => unknown) => resolve(await rowsForSelection(selection)), })), - then: async (resolve: (selectedRows: unknown[]) => unknown) => resolve(rows), + for: vi.fn(() => ({ + then: async (resolve: (selectedRows: unknown[]) => unknown) => resolve(await rowsForSelection(selection)), + })), + then: async (resolve: (selectedRows: unknown[]) => unknown) => resolve(await rowsForSelection(selection)), }; const query = { innerJoin: vi.fn(() => query), @@ -315,12 +332,14 @@ function createRunContextDb( }; return query; }; - return { - transaction: async (callback: (tx: Record) => Promise) => callback({}), + const dbStub = { + transaction: async (callback: (tx: typeof dbStub) => Promise) => callback(dbStub), select: vi.fn((selection: Record = {}) => ({ from: vi.fn(() => buildQuery(selection)), })), + insert: vi.fn(() => ({ values: vi.fn(async () => undefined) })), }; + return dbStub; } async function createApp(actor: Record, db?: unknown) { @@ -426,6 +445,10 @@ describe("agent issue mutation checkout ownership", () => { mockAgentService.list.mockReset(); mockAgentService.resolveByReference.mockReset(); mockCompanyService.getById.mockReset(); + mockBudgetService.getInvocationBlock.mockReset(); + mockBudgetService.getInvocationBlock.mockResolvedValue(null); + mockProjectService.getById.mockReset(); + mockProjectService.getById.mockResolvedValue(null); mockIssueService.addComment.mockReset(); mockIssueService.assertCheckoutOwner.mockReset(); mockIssueService.create.mockReset(); @@ -1705,14 +1728,10 @@ describe("agent issue mutation checkout ownership", () => { expect(mockIssueRecoveryActionService.resolveActiveForIssue).not.toHaveBeenCalled(); }); - it("allows the named recovery owner to resolve a board-owned source issue", async () => { + it("rejects the named recovery owner completing a board-owned source issue", async () => { mockIssueService.getById.mockResolvedValue( makeIssue({ status: "blocked", assigneeAgentId: null, assigneeUserId: "board-user" }), ); - mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ - ...makeIssue({ status: "blocked", assigneeAgentId: null, assigneeUserId: "board-user" }), - ...patch, - })); mockIssueRecoveryActionService.getActiveForIssue.mockResolvedValue({ id: recoveryActionId, ownerAgentId, @@ -1726,8 +1745,144 @@ describe("agent issue mutation checkout ownership", () => { sourceIssueStatus: "done", }); + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.details?.code).toBe("recovery_source_authority_required"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + expect(mockIssueRecoveryActionService.resolveActiveForIssue).not.toHaveBeenCalled(); + }); + + it("rejects a recovery owner completing an independently agent-owned source issue", async () => { + mockIssueService.getById.mockResolvedValue( + makeIssue({ status: "blocked", assigneeAgentId: ownerAgentId }), + ); + mockIssueRecoveryActionService.getActiveForIssue.mockResolvedValue({ + id: recoveryActionId, + ownerAgentId: peerAgentId, + }); + + const res = await request(await createApp(peerActor())) + .post(`/api/issues/${issueId}/recovery-actions/resolve`) + .send({ + actionId: recoveryActionId, + outcome: "restored", + sourceIssueStatus: "done", + }); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.details?.code).toBe("recovery_source_authority_required"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + + it.each(["done", "cancelled"])( + "rejects recovery-owner PATCH of an agent-owned source to %s", + async (status) => { + mockIssueService.getById.mockResolvedValue( + makeIssue({ status: "blocked", assigneeAgentId: ownerAgentId }), + ); + mockIssueRecoveryActionService.getActiveForIssue.mockResolvedValue({ + id: recoveryActionId, + ownerAgentId: peerAgentId, + }); + + const res = await request(await createApp(peerActor())) + .patch(`/api/issues/${issueId}`) + .send({ status }); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.details?.code).toBe("recovery_source_authority_required"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }, + ); + + it("rejects recovery-owner reassignment of an independently agent-owned source", async () => { + mockIssueService.getById.mockResolvedValue( + makeIssue({ status: "blocked", assigneeAgentId: ownerAgentId }), + ); + mockIssueRecoveryActionService.getActiveForIssue.mockResolvedValue({ + id: recoveryActionId, + ownerAgentId: peerAgentId, + }); + mockAgentService.resolveByReference.mockResolvedValue({ + ambiguous: false, + agent: makeAgent(peerAgentId), + }); + + const res = await request(await createApp(peerActor())) + .patch(`/api/issues/${issueId}`) + .send({ assigneeAgentId: peerAgentId }); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.details?.code).toBe("recovery_source_authority_required"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + + it("rejects a recovery owner who is not the current governed review participant", async () => { + mockIssueService.getById.mockResolvedValue(makeIssue({ + status: "in_review", + assigneeAgentId: ownerAgentId, + executionState: { + status: "pending", + currentStageId: "88888888-8888-4888-8888-888888888888", + currentStageIndex: 0, + currentStageType: "review", + currentParticipant: { type: "agent", agentId: ownerAgentId }, + returnAssignee: { type: "agent", agentId: ownerAgentId }, + completedStageIds: [], + lastDecisionId: null, + lastDecisionOutcome: null, + }, + })); + mockIssueRecoveryActionService.getActiveForIssue.mockResolvedValue({ + id: recoveryActionId, + ownerAgentId: peerAgentId, + }); + + const res = await request(await createApp(peerActor())) + .post(`/api/issues/${issueId}/recovery-actions/resolve`) + .send({ actionId: recoveryActionId, outcome: "restored", sourceIssueStatus: "done" }); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.details?.code).toBe("recovery_source_authority_required"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + + it("keeps configured review policy authoritative during recovery resolution", async () => { + mockIssueService.getById.mockResolvedValue(makeIssue({ + status: "in_review", + assigneeAgentId: ownerAgentId, + reviewPolicy: "human_only", + })); + mockIssueRecoveryActionService.getActiveForIssue.mockResolvedValue({ + id: recoveryActionId, + ownerAgentId, + }); + + const res = await request(await createApp(ownerActor())) + .post(`/api/issues/${issueId}/recovery-actions/resolve`) + .send({ actionId: recoveryActionId, outcome: "restored", sourceIssueStatus: "done" }); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(res.body.details?.code).toBe("review_policy_denied"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + + it("allows a recovery owner to record a receipt without mutating a board-owned source", async () => { + mockIssueService.getById.mockResolvedValue(makeIssue({ + status: "in_review", + assigneeAgentId: null, + assigneeUserId: "board-user", + })); + mockIssueRecoveryActionService.getActiveForIssue.mockResolvedValue({ + id: recoveryActionId, + ownerAgentId, + }); + + const res = await request(await createApp(ownerActor())) + .post(`/api/issues/${issueId}/recovery-actions/resolve`) + .send({ actionId: recoveryActionId, outcome: "restored", sourceIssueStatus: "in_review" }); + expect(res.status, JSON.stringify(res.body)).toBe(200); - expect(mockIssueService.update).toHaveBeenCalled(); + expect(mockIssueService.update).not.toHaveBeenCalled(); expect(mockIssueRecoveryActionService.resolveActiveForIssue).toHaveBeenCalled(); }); @@ -1741,10 +1896,11 @@ describe("agent issue mutation checkout ownership", () => { })); mockIssueRecoveryActionService.getActiveForIssue.mockResolvedValue({ id: recoveryActionId, - ownerAgentId, + ownerAgentId: peerAgentId, + returnOwnerAgentId: ownerAgentId, }); - const res = await request(await createApp(ownerActor())) + const res = await request(await createApp(peerActor())) .post(`/api/issues/${issueId}/recovery-actions/resolve`) .send({ actionId: recoveryActionId, @@ -1753,6 +1909,12 @@ describe("agent issue mutation checkout ownership", () => { }); expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(mockIssueService.update).toHaveBeenCalledWith( + issueId, + expect.not.objectContaining({ assigneeAgentId: expect.anything() }), + expect.anything(), + expect.any(Array), + ); expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith( ownerAgentId, expect.objectContaining({ @@ -1766,6 +1928,73 @@ describe("agent issue mutation checkout ownership", () => { ); }); + it.each([ + ["checkoutRunId", ownerRunId], + ["executionRunId", ownerRunId], + ])("blocks safe hand-back while the source has an active %s", async (lockField, lockRunId) => { + mockIssueService.getById.mockResolvedValue(makeIssue({ + status: "blocked", + assigneeAgentId: ownerAgentId, + [lockField]: lockRunId, + })); + mockIssueRecoveryActionService.getActiveForIssue.mockResolvedValue({ + id: recoveryActionId, + ownerAgentId: peerAgentId, + returnOwnerAgentId: ownerAgentId, + }); + + const res = await request(await createApp(peerActor())) + .post(`/api/issues/${issueId}/recovery-actions/resolve`) + .send({ actionId: recoveryActionId, outcome: "restored", sourceIssueStatus: "todo" }); + + expect(res.status, JSON.stringify(res.body)).toBe(409); + expect(res.body.details?.code).toBe("recovery_source_run_lock"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + + it("blocks safe hand-back while the original owner's budget is paused", async () => { + mockIssueService.getById.mockResolvedValue( + makeIssue({ status: "blocked", assigneeAgentId: ownerAgentId }), + ); + mockIssueRecoveryActionService.getActiveForIssue.mockResolvedValue({ + id: recoveryActionId, + ownerAgentId: peerAgentId, + returnOwnerAgentId: ownerAgentId, + }); + mockBudgetService.getInvocationBlock.mockResolvedValue({ + scope: "agent", + reason: "hard_limit_reached", + }); + + const res = await request(await createApp(peerActor())) + .post(`/api/issues/${issueId}/recovery-actions/resolve`) + .send({ actionId: recoveryActionId, outcome: "restored", sourceIssueStatus: "todo" }); + + expect(res.status, JSON.stringify(res.body)).toBe(409); + expect(res.body.details?.code).toBe("recovery_safe_hand_back_budget_blocked"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + + it("blocks safe hand-back while a governed approval remains pending", async () => { + mockIssueService.getById.mockResolvedValue( + makeIssue({ status: "blocked", assigneeAgentId: ownerAgentId }), + ); + mockIssueRecoveryActionService.getActiveForIssue.mockResolvedValue({ + id: recoveryActionId, + ownerAgentId: peerAgentId, + returnOwnerAgentId: ownerAgentId, + }); + mockIssueApprovalService.listApprovalsForIssue.mockResolvedValue([{ status: "pending" }]); + + const res = await request(await createApp(peerActor())) + .post(`/api/issues/${issueId}/recovery-actions/resolve`) + .send({ actionId: recoveryActionId, outcome: "restored", sourceIssueStatus: "todo" }); + + expect(res.status, JSON.stringify(res.body)).toBe(409); + expect(res.body.details?.code).toBe("recovery_governed_approval_pending"); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + it("uses the authorization decision path for assignment changes", async () => { const decide = vi.fn(async () => ({ allowed: false, diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 50ae5ccd194..2a7c527e1f7 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -110,6 +110,7 @@ import * as serviceIndex from "../services/index.js"; import { accessService, agentService, + budgetService, companySkillService, companyService, companySearchService, @@ -2727,6 +2728,7 @@ export function issueRoutes( const svc = issueService(db); const runRedactions = createRunSecretRedactionRegistry(db); const access = accessService(db); + const budgetsSvc = budgetService(db); const heartbeat = heartbeatService(db, { pluginWorkerManager: opts.pluginWorkerManager, }); @@ -4842,9 +4844,8 @@ export function issueRoutes( return false; } - async function assertRecoveryActionAuthority( + async function requireRecoveryActionAuthority( req: Request, - res: Response, issue: { id: string; companyId: string; assigneeAgentId: string | null }, activeRecoveryAction: Awaited>, input: { source: "issue_update" | "recovery_action_resolution" }, @@ -4854,8 +4855,7 @@ export function issueRoutes( const actorAgentId = req.actor.agentId; if (!actorAgentId) { - res.status(403).json({ error: "Agent authentication required" }); - return false; + throw forbidden("Agent authentication required"); } if (issue.assigneeAgentId === actorAgentId) return true; if ( @@ -4872,9 +4872,9 @@ export function issueRoutes( return true; } - res.status(403).json({ - error: "Agent cannot resolve another owner's recovery action", - details: { + throw forbidden( + "Agent cannot resolve another owner's recovery action", + { issueId: issue.id, recoveryActionId: activeRecoveryAction.id, actorAgentId, @@ -4883,8 +4883,157 @@ export function issueRoutes( source: input.source, securityPrinciples: ["Least Privilege", "Complete Mediation", "Secure Defaults"], }, - }); - return false; + ); + } + + function activeExecutionParticipantAgentId(issue: { executionState?: unknown }) { + const state = parseIssueExecutionState(issue.executionState); + return state?.status === "pending" && state.currentParticipant?.type === "agent" + ? state.currentParticipant.agentId + : null; + } + + async function requireRecoverySourceMutationAuthority( + req: Request, + issue: { + id: string; + companyId: string; + status: string; + assigneeAgentId: string | null; + checkoutRunId?: string | null; + executionRunId?: string | null; + executionState?: unknown; + }, + ) { + if (req.actor.type !== "agent") return; + const actorAgentId = req.actor.agentId; + if (!actorAgentId) throw forbidden("Agent authentication required"); + + const isSourceOwner = issue.assigneeAgentId === actorAgentId; + const isExecutionParticipant = activeExecutionParticipantAgentId(issue) === actorAgentId; + const hasPolicyGrant = Boolean( + issue.assigneeAgentId && + await hasActiveCheckoutManagementOverride(actorAgentId, issue.companyId, issue.assigneeAgentId) + ); + if (!isSourceOwner && !isExecutionParticipant && !hasPolicyGrant) { + throw forbidden( + "Recovery ownership does not authorize this source issue mutation", + { + code: "recovery_source_authority_required", + issueId: issue.id, + actorAgentId, + assigneeAgentId: issue.assigneeAgentId, + currentExecutionParticipantAgentId: activeExecutionParticipantAgentId(issue), + remediation: + "Have the source owner, current execution participant, board, or a policy-authorized agent perform the source mutation.", + securityPrinciples: ["Least Privilege", "Complete Mediation", "Secure Defaults"], + }, + ); + } + + const actorRunId = req.actor.runId?.trim() || null; + const conflictingRunId = [issue.checkoutRunId, issue.executionRunId] + .find((runId) => runId && runId !== actorRunId); + if (conflictingRunId && !hasPolicyGrant) { + throw conflict("Source issue mutation is locked by another active checkout or run", { + code: "recovery_source_run_lock", + issueId: issue.id, + actorAgentId, + actorRunId, + checkoutRunId: issue.checkoutRunId ?? null, + executionRunId: issue.executionRunId ?? null, + }); + } + if (isSourceOwner && issue.status === "in_progress" && !actorRunId && !hasPolicyGrant) { + throw unauthorized("Agent run id required"); + } + } + + async function assertSafeRecoveryHandBackGates(input: { + req: Request; + issue: { + id: string; + companyId: string; + projectId: string | null; + assigneeAgentId: string | null; + checkoutRunId?: string | null; + executionRunId?: string | null; + executionState?: unknown; + }; + recoveryAction: NonNullable>>; + }) { + const returnOwnerAgentId = input.recoveryAction.returnOwnerAgentId; + if (!returnOwnerAgentId || input.issue.assigneeAgentId !== returnOwnerAgentId) { + throw forbidden( + "Safe recovery hand-back requires the recorded original owner to remain assigned", + { + code: "recovery_safe_hand_back_owner_mismatch", + issueId: input.issue.id, + assigneeAgentId: input.issue.assigneeAgentId, + returnOwnerAgentId, + }, + ); + } + const actorRunId = input.req.actor.type === "agent" + ? input.req.actor.runId?.trim() || null + : null; + const conflictingRunId = [input.issue.checkoutRunId, input.issue.executionRunId] + .find((runId) => runId && runId !== actorRunId); + if (conflictingRunId) { + throw conflict("Safe recovery hand-back is locked by another active checkout or run", { + code: "recovery_source_run_lock", + issueId: input.issue.id, + actorRunId, + checkoutRunId: input.issue.checkoutRunId ?? null, + executionRunId: input.issue.executionRunId ?? null, + }); + } + if (parseIssueExecutionState(input.issue.executionState)?.status === "pending") { + throw conflict("Safe recovery hand-back cannot bypass a pending execution review or approval stage", { + code: "recovery_governed_stage_pending", + issueId: input.issue.id, + }); + } + + const activePauseHold = await treeControlSvc.getActivePauseHoldGate(input.issue.companyId, input.issue.id); + if (activePauseHold) { + throw conflict("Safe recovery hand-back blocked by active subtree pause hold", { + issueId: input.issue.id, + holdId: activePauseHold.holdId, + rootIssueId: activePauseHold.rootIssueId, + mode: activePauseHold.mode, + }); + } + if (input.issue.projectId) { + const project = await projectsSvc.getById(input.issue.projectId); + if (project?.pausedAt) { + throw conflict( + project.pauseReason === "budget" + ? "Project is paused because its budget hard-stop was reached" + : "Project is paused", + ); + } + } + const approvals = await issueApprovalsSvc.listApprovalsForIssue(input.issue.id); + if (approvals.some((approval) => ACTIVE_REVIEW_APPROVAL_STATUSES.has(String(approval.status)))) { + throw conflict("Safe recovery hand-back cannot bypass a pending governed approval", { + code: "recovery_governed_approval_pending", + issueId: input.issue.id, + }); + } + const budgetBlock = await budgetsSvc.getInvocationBlock( + input.issue.companyId, + returnOwnerAgentId, + { issueId: input.issue.id, projectId: input.issue.projectId }, + ); + if (budgetBlock) { + throw conflict("Safe recovery hand-back is blocked by the source owner's budget or pause gate", { + code: "recovery_safe_hand_back_budget_blocked", + issueId: input.issue.id, + returnOwnerAgentId, + budgetBlock, + }); + } } async function resolveActiveIssueRun(issue: { @@ -6404,18 +6553,16 @@ export function issueRoutes( const id = req.params.id as string; const existing = await getAccessibleResource(req, res, svc.getById(id), "Issue not found"); if (!existing) return; - if (!(await assertAgentIssueMutationAllowed(req, res, existing))) return; - const activeRecoveryAction = await recoveryActionsSvc.getActiveForIssue(existing.companyId, existing.id); - if ( - !(await assertRecoveryActionAuthority( - req, - res, - existing, - activeRecoveryAction, - { source: "recovery_action_resolution" }, - )) - ) { - return; + if (!(await assertIssueReadAllowed(req, res, existing))) return; + if (await assertLowTrustControlPlaneDenied(req, res, existing.companyId, existing)) return; + if (req.actor.type === "agent") { + const boundaryDecision = await decideIssueAccess(req, existing, "issue:mutate"); + if (!boundaryDecision.allowed) { + await denyIssueWrite(req, res, existing, issueWriteDenialCodeForDecision(boundaryDecision)); + return; + } + if (!requireAgentRunId(req, res)) return; + if (!(await assertCrossIssueInfluenceWithinRunCap(req, res, existing, "update"))) return; } const { actionId, outcome, sourceIssueStatus, resolutionNote } = req.body; @@ -6424,29 +6571,35 @@ export function issueRoutes( } const actor = getActorInfo(req); - const handBackAgentId = outcome === "restored" && sourceIssueStatus === "todo" - ? activeRecoveryAction?.returnOwnerAgentId ?? null - : null; - const recordedOutcome = handBackAgentId - ? "handed_back" - : outcome === "restored" && sourceIssueStatus === "done" - ? "owner_completed" - : outcome; - const updateFields = sourceIssueStatus ? { status: sourceIssueStatus } : {}; - await assertInReviewReviewPath({ - existing, - updateFields, - actorType: actor.actorType, - actorId: actor.actorId, - actorAgentId: actor.agentId, - actorRunId: actor.runId, - }); - const actionStatus = outcome === "cancelled" ? "cancelled" : "resolved"; const postCommitActivityPublications: ActivityPublication[] = []; const result = await db.transaction(async (tx) => { - let issue = existing; - if (outcome === "blocked") { + const lockedIssue = await tx + .select() + .from(issueRows) + .where(and(eq(issueRows.companyId, existing.companyId), eq(issueRows.id, existing.id))) + .for("update") + .then((rows) => rows[0] ?? null); + if (!lockedIssue) throw notFound("Issue not found"); + + const activeRecoveryAction = await recoveryActionsSvc.getActiveForIssue( + lockedIssue.companyId, + lockedIssue.id, + tx, + ); + if (!activeRecoveryAction || (actionId && activeRecoveryAction.id !== actionId)) { + throw notFound("Active recovery action not found"); + } + await requireRecoveryActionAuthority( + req, + lockedIssue, + activeRecoveryAction, + { source: "recovery_action_resolution" }, + ); + + let issue = lockedIssue; + const sourceStatusChanged = sourceIssueStatus !== lockedIssue.status; + if (outcome === "blocked" && sourceStatusChanged) { const unresolvedBlockers = await tx .select({ id: issueRows.id }) .from(issueRelations) @@ -6465,12 +6618,84 @@ export function issueRoutes( } } - if (sourceIssueStatus) { + if (sourceStatusChanged) { + const safeHandBack = + outcome === "restored" && + sourceIssueStatus === "todo" && + activeRecoveryAction.returnOwnerAgentId != null && + lockedIssue.assigneeAgentId === activeRecoveryAction.returnOwnerAgentId; + if (safeHandBack) { + await assertSafeRecoveryHandBackGates({ + req, + issue: lockedIssue, + recoveryAction: activeRecoveryAction, + }); + } else { + await requireRecoverySourceMutationAuthority(req, lockedIssue); + } + + if ( + lockedIssue.status === "in_review" && + (sourceIssueStatus === "done" || sourceIssueStatus === "cancelled") && + lockedIssue.reviewPolicy != null && + lockedIssue.reviewPolicy !== "anyone" + ) { + await assertIssueReviewVerdictActorAllowed(tx as unknown as Db, { + issue: lockedIssue, + actor: { type: actor.actorType, id: actor.actorId }, + }); + } + + const updateFields: Record = { status: sourceIssueStatus }; + if (!safeHandBack) { + await assertAgentInReviewReviewPath({ + existing: lockedIssue, + updateFields, + actorType: req.actor.type, + actorAgentId: actor.agentId, + actorRunId: actor.runId, + }); + const executionPolicy = normalizeIssueExecutionPolicy(lockedIssue.executionPolicy ?? null); + const transition = applyIssueExecutionPolicyTransition({ + issue: lockedIssue, + policy: executionPolicy, + previousPolicy: executionPolicy, + requestedStatus: sourceIssueStatus, + requestedAssigneePatch: {}, + actor: { + agentId: actor.agentId ?? null, + userId: actor.actorType === "user" ? actor.actorId : null, + }, + allowBoardOverride: req.actor.type === "board", + commentBody: resolutionNote ?? null, + }); + Object.assign(updateFields, transition.patch); + if (transition.decision) { + const decisionId = randomUUID(); + const nextExecutionState = updateFields.executionState; + if (!nextExecutionState || typeof nextExecutionState !== "object") { + throw new Error("Execution policy decision patch is missing executionState"); + } + updateFields.executionState = { ...nextExecutionState, lastDecisionId: decisionId }; + await tx.insert(issueExecutionDecisions).values({ + id: decisionId, + companyId: lockedIssue.companyId, + issueId: lockedIssue.id, + stageId: transition.decision.stageId, + stageType: transition.decision.stageType, + actorAgentId: actor.agentId ?? null, + actorUserId: actor.actorType === "user" ? actor.actorId : null, + outcome: transition.decision.outcome, + body: transition.decision.body, + createdByRunId: actor.runId ?? null, + }); + } + } + const updatedIssue = await svc.update( id, { - status: sourceIssueStatus, - ...(handBackAgentId ? { assigneeAgentId: handBackAgentId } : {}), + ...updateFields, actorAgentId: actor.agentId ?? null, actorUserId: actor.actorType === "user" ? actor.actorId : null, }, @@ -6481,11 +6706,20 @@ export function issueRoutes( issue = updatedIssue; } + const recordedOutcome = + outcome === "restored" && issue.status === "todo" && + activeRecoveryAction.returnOwnerAgentId != null && + issue.assigneeAgentId === activeRecoveryAction.returnOwnerAgentId + ? "handed_back" + : outcome === "restored" && issue.status === "done" + ? "owner_completed" + : outcome; + const recoveryAction = await recoveryActionsSvc.resolveActiveForIssue( { companyId: existing.companyId, sourceIssueId: existing.id, - actionId: actionId ?? null, + actionId: activeRecoveryAction.id, status: actionStatus, outcome: recordedOutcome, resolutionNote: resolutionNote ?? null, @@ -8844,17 +9078,29 @@ export function issueRoutes( const activeRecoveryActionBeforeUpdate = recoveryRelevantSourceMutationRequested ? await recoveryActionsSvc.getActiveForIssue(existing.companyId, existing.id) : null; - if ( - recoveryRelevantSourceMutationRequested && - !(await assertRecoveryActionAuthority( + if (recoveryRelevantSourceMutationRequested) { + await requireRecoveryActionAuthority( req, - res, existing, activeRecoveryActionBeforeUpdate, { source: "issue_update" }, - )) - ) { - return; + ); + const recoveryRestrictedSourceMutationRequested = + activeRecoveryActionBeforeUpdate != null && + ( + updateFields.status === "done" || + updateFields.status === "cancelled" || + normalizedAssigneeAgentId !== undefined || + req.body.assigneeUserId !== undefined || + ( + activeExecutionParticipantAgentId(existing) != null && + typeof updateFields.status === "string" && + updateFields.status !== existing.status + ) + ); + if (recoveryRestrictedSourceMutationRequested) { + await requireRecoverySourceMutationAuthority(req, existing); + } } if ( resumeRequested !== true && diff --git a/server/src/services/issue-recovery-actions.ts b/server/src/services/issue-recovery-actions.ts index 513eccf33c9..f9755274bdc 100644 --- a/server/src/services/issue-recovery-actions.ts +++ b/server/src/services/issue-recovery-actions.ts @@ -124,8 +124,12 @@ export function issueRecoveryActionService(db: Db) { } } - async function getActiveForIssue(companyId: string, sourceIssueId: string): Promise { - const row = await db + async function getActiveForIssue( + companyId: string, + sourceIssueId: string, + dbOrTx: DbOrTransaction = db, + ): Promise { + const row = await dbOrTx .select() .from(issueRecoveryActions) .where( diff --git a/server/src/services/recovery/service.ts b/server/src/services/recovery/service.ts index bb62fd34ba5..852233f53d8 100644 --- a/server/src/services/recovery/service.ts +++ b/server/src/services/recovery/service.ts @@ -3735,6 +3735,7 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) async function reconcileDispositionRepair( issue: typeof issues.$inferSelect, latestRun: LatestIssueRun, + options: { historicalAttemptCount?: number } = {}, ): Promise<"queued" | "escalated" | "covered" | "skipped"> { const current = await db .select() @@ -3775,7 +3776,15 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) activeRepairAction.fingerprint === state.fingerprint ? activeRepairAction.attemptCount : 0; - const sameFingerprintAttempt = Math.max(runAttempt, persistedAttempt); + // Upgrade compatibility: pre-fingerprint continuation parks already spent + // attempts against this unchanged source state. Seed the durable counter + // from that consecutive legacy history instead of granting three fresh + // attempts merely because the recovery-action row did not exist yet. + const historicalAttempt = Math.min( + DISPOSITION_REPAIR_MAX_ATTEMPTS, + Math.max(0, Math.floor(options.historicalAttemptCount ?? 0)), + ); + const sameFingerprintAttempt = Math.max(runAttempt, persistedAttempt, historicalAttempt); if (!ownerInvokable || budgetBlocked) { const escalated = await escalateDispositionRepair({ issue: current, @@ -4338,6 +4347,13 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) ? acceptedContinuationInteraction.resolvedAt ?? acceptedContinuationInteraction.updatedAt : null; if (acceptedContinuationInteraction && acceptedInteractionResolvedAt && !pendingExecutionState) { + const legacyReviewParkAttempts = await summarizeRecentContinuationRetries( + issue.companyId, + issue.id, + agentId, + CONTINUATION_WAITING_ON_REVIEW_ERROR_CODE, + acceptedInteractionResolvedAt, + ); const successfulRunSinceResolution = await hasSuccessfulIssueRunSince( issue.companyId, issue.id, @@ -4378,7 +4394,9 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) result.issueIds.push(issue.id); continue; } - const outcome = await reconcileDispositionRepair(issue, latestPostResolutionRun); + const outcome = await reconcileDispositionRepair(issue, latestPostResolutionRun, { + historicalAttemptCount: legacyReviewParkAttempts.consecutive, + }); if (outcome === "queued") { result.continuationRequeued += 1; result.dispositionRepairRequeued += 1; @@ -4391,13 +4409,7 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) } continue; } - const { consecutive } = await summarizeRecentContinuationRetries( - issue.companyId, - issue.id, - agentId, - CONTINUATION_WAITING_ON_REVIEW_ERROR_CODE, - acceptedInteractionResolvedAt, - ); + const { consecutive } = legacyReviewParkAttempts; if (consecutive >= INTERACTION_CONTINUATION_REQUEUE_MAX_ATTEMPTS && latestPostResolutionRun) { const resolved = await resolveContinuationWaitingOnReview(issue); if (resolved) {