From 4f9894df4459fde964de91aee9ae09c73bf57efc Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 16 Jul 2026 02:34:20 -0500 Subject: [PATCH] fix(server): bound accepted-interaction continuation recovery (#9656) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Heartbeat recovery keeps assigned issues moving when a run or continuation path disappears > - Accepted issue-thread interactions can create a continuation wake after an agent previously parked for review > - The recovery sweep could requeue that accepted-interaction wake while the queued-run gate cancelled it using the older pre-acceptance park summary > - That cancellation path had no bound, so recovery could repeat the same wake and cancellation indefinitely > - This pull request makes accepted-interaction evidence supersede the older park and caps repeated recovery cancellations at three attempts > - The benefit is that accepted work resumes normally, while genuine repeated failures become a visible dependency wait or escalation instead of a cancel loop ## Linked Issues or Issue Description Refs #9331 The accepted-interaction continuation recovery added by #9331 can encounter a stale continuation summary written before approval. The sweep requeues a continuation carrying the accepted interaction timestamp, but queued-run invalidation cancels it because the older summary says to wait for review. Recovery then sees the accepted interaction without a successful run and requeues again. This PR prevents that stale-summary cancellation and adds a bounded fallback if three equivalent cancellations have already occurred. ## What Changed - Let queued continuation wakes with a parseable `interactionResolvedAt` bypass a pre-acceptance waiting-for-review park summary. - Count consecutive unsuccessful continuation runs for the same issue and agent since interaction acceptance; after three review-park cancellations, convert a real dependency wait or use the existing visible escalation path. - Add focused regression coverage for the park bypass, unchanged non-interaction park behavior, below-cap requeue, cap escalation, and successful-run skip. - Document the accepted-interaction precedence and bounded requeue contract in execution semantics §9.2. ## Verification - `pnpm vitest run server/src/__tests__/heartbeat-process-recovery.test.ts -t "accepted interaction continuation recovery|accepted interaction recovery after its continuation succeeds|requeues accepted interaction continuations stranded"` - `pnpm vitest run server/src/__tests__/heartbeat-stale-queue-invalidation.test.ts -t "pre-acceptance review park|continuation summary parks executor work"` - `pnpm --filter @paperclipai/server typecheck` ## Risks - Low risk: the park bypass only applies when the queued context contains a parseable interaction resolution timestamp. - The retry bound is scoped to unsuccessful `issue_continuation_needed` runs for the same company, issue, agent, error code, and post-acceptance time window. - No schema, migration, API, or UI changes. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex, exact model ID `gpt-5.5`, high-reasoning coding mode with repository tool use and command execution; context-window size was not exposed by the runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- doc/execution-semantics.md | 2 + .../heartbeat-process-recovery.test.ts | 193 +++++++++++++++++- ...heartbeat-stale-queue-invalidation.test.ts | 59 ++++++ server/src/services/heartbeat.ts | 3 + server/src/services/recovery/service.ts | 39 ++++ 5 files changed, 295 insertions(+), 1 deletion(-) diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index ecabd67698..e2ee66faf2 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -516,6 +516,8 @@ Recovery rule for a parked-for-review continuation: - if the issue has a real waiting target — open (non-terminal) sub-tasks or existing unresolved blockers — Paperclip converts the deliberate wait into a first-class dependency wait: it sets the issue `blocked` by those issues, keeps the original assignee, and posts a plain-language comment explaining that the task will resume automatically when its dependencies finish. The issue then self-resumes through the normal `issue_blockers_resolved` path; no recovery action or escalation owner is involved - if the issue has no waiting target, the park is indistinguishable from a genuine strand and falls through to the standard §9.2 escalation, preserving stranded detection +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. + This keeps the post-decomposition umbrella (§7) on a real waiting path instead of relying on `parentId` rollup, which §6 does not treat as a dependency. ### 9.3 Recovery model-profile lane diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index 015a9c1196..2464f82fbb 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -1,6 +1,6 @@ import { randomUUID } from "node:crypto"; import { spawn, type ChildProcess } from "node:child_process"; -import { and, eq, or, inArray } from "drizzle-orm"; +import { and, eq, or, inArray, sql } from "drizzle-orm"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { activityLog, @@ -4097,6 +4097,197 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { }); }); + it("escalates accepted interaction continuation recovery after three review-park cancellations", async () => { + const companyId = randomUUID(); + const agentId = randomUUID(); + const issueId = randomUUID(); + const interactionId = randomUUID(); + const resolvedAt = new Date("2026-03-19T00:05:00.000Z"); + const issuePrefix = `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`; + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix, + defaultResponsibleUserId: "responsible-user", + requireBoardApprovalForNewAgents: false, + }); + await db.insert(agents).values({ + id: agentId, + companyId, + name: "CodexCoder", + role: "engineer", + status: "idle", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: { heartbeat: { wakeOnDemand: true, maxConcurrentRuns: 1 } }, + permissions: {}, + }); + await db.insert(issues).values({ + id: issueId, + companyId, + title: "Accepted plan cancellation loop", + status: "in_review", + priority: "medium", + assigneeAgentId: agentId, + responsibleUserId: "responsible-user", + issueNumber: 1, + identifier: `${issuePrefix}-1`, + }); + await db.insert(issueThreadInteractions).values({ + id: interactionId, + companyId, + issueId, + kind: "request_confirmation", + status: "accepted", + continuationPolicy: "wake_assignee_on_accept", + createdByAgentId: agentId, + resolvedByUserId: "responsible-user", + resolvedAt, + updatedAt: resolvedAt, + payload: { version: 1, prompt: "Approve the plan?" }, + result: { outcome: "accepted" }, + }); + for (let attempt = 1; attempt <= 3; attempt += 1) { + const finishedAt = new Date(resolvedAt.getTime() + attempt * 60_000); + await db.insert(heartbeatRuns).values({ + id: randomUUID(), + companyId, + agentId, + invocationSource: "automation", + triggerDetail: "system", + status: "cancelled", + errorCode: "issue_continuation_waiting_on_review", + error: "Continuation summary still says to wait for review", + contextSnapshot: { + issueId, + taskId: issueId, + wakeReason: "issue_continuation_needed", + retryReason: "issue_continuation_needed", + mutation: "interaction", + interactionId, + interactionResolvedAt: resolvedAt.toISOString(), + }, + createdAt: finishedAt, + startedAt: finishedAt, + finishedAt, + updatedAt: finishedAt, + }); + } + + const heartbeat = heartbeatService(db); + const result = await heartbeat.reconcileStrandedAssignedIssues(); + + expect(result.continuationRequeued).toBe(0); + expect(result.waitingOnReviewResolved).toBe(0); + expect(result.escalated).toBe(1); + expect(result.issueIds).toContain(issueId); + + const [issue, continuationRuns, comments] = await Promise.all([ + db.select().from(issues).where(eq(issues.id, issueId)).then((rows) => rows[0] ?? null), + db + .select({ id: heartbeatRuns.id }) + .from(heartbeatRuns) + .where(and( + eq(heartbeatRuns.companyId, companyId), + eq(heartbeatRuns.agentId, agentId), + sql`${heartbeatRuns.contextSnapshot} ->> 'issueId' = ${issueId}`, + sql`${heartbeatRuns.contextSnapshot} ->> 'retryReason' = 'issue_continuation_needed'`, + )), + db.select({ body: issueComments.body }).from(issueComments).where(eq(issueComments.issueId, issueId)), + ]); + expect(issue?.status).toBe("blocked"); + expect(continuationRuns).toHaveLength(3); + expect(comments.some((comment) => comment.body.includes(interactionId))).toBe(true); + }); + + it("skips accepted interaction recovery after its continuation succeeds", async () => { + const companyId = randomUUID(); + const agentId = randomUUID(); + const issueId = randomUUID(); + const interactionId = randomUUID(); + const resolvedAt = new Date("2026-03-19T00:05:00.000Z"); + const succeededAt = new Date("2026-03-19T00:06:00.000Z"); + const issuePrefix = `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`; + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix, + defaultResponsibleUserId: "responsible-user", + requireBoardApprovalForNewAgents: false, + }); + await db.insert(agents).values({ + id: agentId, + companyId, + name: "CodexCoder", + role: "engineer", + status: "idle", + adapterType: "codex_local", + adapterConfig: {}, + runtimeConfig: { heartbeat: { wakeOnDemand: true, maxConcurrentRuns: 1 } }, + permissions: {}, + }); + await db.insert(issues).values({ + id: issueId, + companyId, + title: "Accepted plan already resumed", + status: "in_review", + priority: "medium", + assigneeAgentId: agentId, + responsibleUserId: "responsible-user", + issueNumber: 1, + identifier: `${issuePrefix}-1`, + }); + await db.insert(issueThreadInteractions).values({ + id: interactionId, + companyId, + issueId, + kind: "request_confirmation", + status: "accepted", + continuationPolicy: "wake_assignee_on_accept", + createdByAgentId: agentId, + resolvedByUserId: "responsible-user", + resolvedAt, + updatedAt: resolvedAt, + payload: { version: 1, prompt: "Approve the plan?" }, + result: { outcome: "accepted" }, + }); + await db.insert(heartbeatRuns).values({ + id: randomUUID(), + companyId, + agentId, + invocationSource: "automation", + triggerDetail: "system", + status: "succeeded", + contextSnapshot: { + issueId, + taskId: issueId, + wakeReason: "issue_continuation_needed", + retryReason: "issue_continuation_needed", + mutation: "interaction", + interactionId, + interactionResolvedAt: resolvedAt.toISOString(), + }, + createdAt: succeededAt, + startedAt: succeededAt, + finishedAt: succeededAt, + updatedAt: succeededAt, + }); + + const heartbeat = heartbeatService(db); + const result = await heartbeat.reconcileStrandedAssignedIssues(); + + expect(result.continuationRequeued).toBe(0); + expect(result.escalated).toBe(0); + expect(result.skipped).toBeGreaterThanOrEqual(1); + const runs = await db + .select({ id: heartbeatRuns.id }) + .from(heartbeatRuns) + .where(eq(heartbeatRuns.agentId, agentId)); + expect(runs).toHaveLength(1); + }); + it("requeues accepted interaction continuations even when a later successful run is unrelated", async () => { const companyId = randomUUID(); const agentId = randomUUID(); diff --git a/server/src/__tests__/heartbeat-stale-queue-invalidation.test.ts b/server/src/__tests__/heartbeat-stale-queue-invalidation.test.ts index 32611ab4fa..35f33c9f46 100644 --- a/server/src/__tests__/heartbeat-stale-queue-invalidation.test.ts +++ b/server/src/__tests__/heartbeat-stale-queue-invalidation.test.ts @@ -1543,4 +1543,63 @@ describeEmbeddedPostgres("heartbeat stale queued-run invalidation", () => { expect(wakeup?.error).toContain("continuation summary says the executor should wait"); expect(countExecuteCallsForRun(runId)).toBe(0); }); + + it("runs accepted-interaction continuation recovery despite a pre-acceptance review park", async () => { + const { companyId, agentId } = await seedCompanyAndAgent(); + const issueId = randomUUID(); + await db.insert(issues).values({ + id: issueId, + companyId, + title: "Approved implementation resumes", + status: "in_progress", + priority: "medium", + assigneeAgentId: agentId, + }); + await seedContinuationSummary({ + companyId, + issueId, + agentId, + body: [ + "# Continuation Summary", + "", + "## Next Action", + "", + "- Wait for reviewer feedback or approval before continuing executor work.", + ].join("\n"), + }); + + const { runId } = await seedQueuedRun({ + companyId, + agentId, + issueId, + wakeReason: "issue_continuation_needed", + invocationSource: "automation", + contextExtras: { + retryReason: "issue_continuation_needed", + mutation: "interaction", + interactionId: randomUUID(), + interactionResolvedAt: "2026-03-19T00:05:00.000Z", + }, + }); + + await heartbeat.resumeQueuedRuns(); + + await waitForCondition(async () => { + const run = await db + .select({ status: heartbeatRuns.status }) + .from(heartbeatRuns) + .where(eq(heartbeatRuns.id, runId)) + .then((rows) => rows[0] ?? null); + return run?.status === "succeeded"; + }); + + const run = await db + .select({ status: heartbeatRuns.status, errorCode: heartbeatRuns.errorCode }) + .from(heartbeatRuns) + .where(eq(heartbeatRuns.id, runId)) + .then((rows) => rows[0] ?? null); + expect(run?.status).toBe("succeeded"); + expect(run?.errorCode).toBeNull(); + expect(countExecuteCallsForRun(runId)).toBe(1); + }); }); diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 58941ca95f..a487f8a914 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -10505,10 +10505,13 @@ export function heartbeatService(db: Db, options: HeartbeatServiceOptions = {}) const resumeIntent = context.resumeIntent === true || context.followUpRequested === true; const wakeReason = readNonEmptyString(context.wakeReason); const retryReason = readNonEmptyString(context.retryReason) ?? run.scheduledRetryReason ?? null; + const interactionResolvedAt = readNonEmptyString(context.interactionResolvedAt); + const hasResolvedInteractionEvidence = interactionResolvedAt !== null && !Number.isNaN(Date.parse(interactionResolvedAt)); if ( issue.status === "in_progress" && !wakeCommentId && + !hasResolvedInteractionEvidence && (wakeReason === "issue_continuation_needed" || retryReason === "issue_continuation_needed") ) { const queuedWake = parseObject(context.paperclipWake); diff --git a/server/src/services/recovery/service.ts b/server/src/services/recovery/service.ts index 90b24a6a2f..b6a335ccc2 100644 --- a/server/src/services/recovery/service.ts +++ b/server/src/services/recovery/service.ts @@ -289,6 +289,7 @@ const NON_RETRYABLE_CONTINUATION_ERROR_CODES = new Set([ // issue has a real waiting target we convert it into a normal dependency wait rather // than escalating it as stranded. const CONTINUATION_WAITING_ON_REVIEW_ERROR_CODE = "issue_continuation_waiting_on_review"; +const INTERACTION_CONTINUATION_REQUEUE_MAX_ATTEMPTS = 3; const CONTINUATION_RECOVERY_TRANSIENT_MAX_ATTEMPTS = 3; const CONTINUATION_RECOVERY_DEFAULT_MAX_ATTEMPTS = 1; @@ -635,7 +636,9 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) async function summarizeRecentContinuationRetries( companyId: string, issueId: string, + agentId: string, errorCodeToMatch: string | null, + since: Date | null = null, ) { const rows = await db .select({ @@ -649,7 +652,9 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) .where( and( eq(heartbeatRuns.companyId, companyId), + eq(heartbeatRuns.agentId, agentId), sql`${heartbeatRuns.contextSnapshot} ->> 'issueId' = ${issueId}`, + ...(since ? [or(gte(heartbeatRuns.createdAt, since), gte(heartbeatRuns.finishedAt, since))] : []), ), ) .orderBy(desc(heartbeatRuns.createdAt), desc(heartbeatRuns.id)) @@ -3336,6 +3341,39 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) agentId, acceptedInteractionResolvedAt, ); + const { consecutive } = await summarizeRecentContinuationRetries( + issue.companyId, + issue.id, + agentId, + CONTINUATION_WAITING_ON_REVIEW_ERROR_CODE, + acceptedInteractionResolvedAt, + ); + if (consecutive >= INTERACTION_CONTINUATION_REQUEUE_MAX_ATTEMPTS && latestPostResolutionRun) { + const resolved = await resolveContinuationWaitingOnReview(issue); + if (resolved) { + result.waitingOnReviewResolved += 1; + result.issueIds.push(issue.id); + continue; + } + + const updated = await escalateStrandedAssignedIssue({ + issue, + previousStatus: issue.status as StrandedPreviousStatus, + latestRun: latestPostResolutionRun, + comment: + `Paperclip stopped requeueing accepted interaction \`${acceptedContinuationInteraction.id}\` after ` + + `${consecutive} consecutive continuation wakes were cancelled while waiting on review. ` + + "Moving the issue to `blocked` so the missing execution path is visible for intervention.", + }); + if (updated) { + result.escalated += 1; + result.issueIds.push(issue.id); + } else { + result.skipped += 1; + } + continue; + } + const queued = await enqueueStrandedIssueRecovery({ issueId: issue.id, agentId, @@ -3656,6 +3694,7 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup }) const { consecutive, latestFinishedAt } = await summarizeRecentContinuationRetries( issue.companyId, issue.id, + agentId, classification.errorCode, ); if (consecutive >= classification.maxAttempts) {