fix(recovery): exempt stranded escalation when assignee shows recent visible progress (#5213)
## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - The recovery service watches `in_progress` agent-assigned issues every 30s and creates "Recover stalled issue …" child issues when execution looks stranded > - The `isRepeatedProductiveContinuationRecovery` branch escalates after just **two consecutive productive continuation runs** — fine for genuinely stuck agents that loop without doing anything, but a false positive for batch workflows that legitimately advance every heartbeat (e.g. multi-frame image generation that produces 1–2 frames + an attachment per heartbeat) > - In production this fired ~95 times for a 19-character batch run, burning a recovery owner heartbeat each time > - This pull request adds a "recent visible progress" exemption: if the assignee posted a comment or any attachment within the exemption window (default 30 min, env-tunable, 60s floor), skip the escalation and let the normal continuation-retry path enqueue the next wake > - The benefit is one platform tweak unblocks all current and future batch workflows without weakening the genuinely-stuck case — agents that go silent still escalate after the window elapses ## What Changed - `server/src/services/recovery/service.ts` - new `STRANDED_RECENT_PROGRESS_EXEMPTION_MS` constant (default 30 min, override via env, floored at 60s) - new `hasRecentVisibleProgress(companyId, issueId, assigneeAgentId, windowMs)` helper — single parallel query against `issue_comments` (filtered by `authorAgentId`) + `issue_attachments`, both using existing indexes - in `reconcileStrandedAssignedIssues`, the `isRepeatedProductiveContinuationRecovery` branch now consults the helper before escalating; on exemption it falls through to the existing continuation-retry enqueue path - new `recentProgressExempted` counter on the reconcile result, surfaced in the periodic recovery log via the existing `...reconciled` spread - `server/src/__tests__/heartbeat-process-recovery.test.ts` - new test: recent agent comment → no escalation, continuation re-queued, `recentProgressExempted: 1` - new test: stale (24h-old) agent comment → escalation still fires as before ## Verification - `pnpm typecheck` — green across the workspace - `pnpm exec vitest run server/src/__tests__/heartbeat-process-recovery.test.ts` — 39/39 pass (37 pre-existing + 2 new) - Smoke after deploy: confirm Image Spec multi-frame generation no longer creates `Recover stalled issue …` child issues per heartbeat ## Risks - **Behavioral shift, low blast radius.** A genuinely-stuck agent that posts cosmetic comments every <30 min would now escalate later instead of immediately. Mitigated by: - Window is configurable via `STRANDED_RECENT_PROGRESS_EXEMPTION_MS` - Other escalation paths are untouched (failed/cancelled/timed_out runs still escalate immediately, paused-tree handling unchanged, recovery-issue-on-recovery guard unchanged) - Periodic recovery log now reports `recentProgressExempted` so a runaway exemption is visible in operations - No DB migration required — both `issue_comments` and `issue_attachments` queries use existing indexes - Backward compatible: pre-existing test "blocks stranded in-progress work after a productive continuation retry was already used" still passes unchanged because no comment is seeded → no exemption → escalates ## Model Used - Claude Opus 4.7 (`claude-opus-4-7`), extended thinking, tool use enabled. Investigation, change, tests, and PR body all human-supervised through a Paperclip agent heartbeat. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots — N/A, server-only - [ ] I have updated relevant documentation to reflect my changes — no docs touched the previous behavior; the env knob is self-documenting via the comment in `service.ts` - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge ## Cross-references and status (maintainer) Rebased onto current `master`. No duplicate PRs absorbed. Refs #6072 — related open report in the same `reconcileStrandedAssignedIssues` / `isRepeatedProductiveContinuationRecovery` family (stale productive-continuation evidence). This PR does not fix #6072, but the recent-visible-progress exemption added here shrinks the false-positive surface in that branch and the new `recentProgressExempted` counter gives operators visibility into the broader escalation path. Co-authored-by: sunghere <sunghere@users.noreply.github.com> Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
sunghere
@@ -3418,6 +3418,78 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
|
||||
expect(retryRun?.contextSnapshot as Record<string, unknown>).not.toHaveProperty("modelProfile");
|
||||
});
|
||||
|
||||
it("exempts stranded-recovery escalation when assignee posted a recent comment (GGU-809)", async () => {
|
||||
const { companyId, agentId, issueId, runId } = await seedStrandedIssueFixture({
|
||||
status: "in_progress",
|
||||
runStatus: "succeeded",
|
||||
retryReason: "issue_continuation_needed",
|
||||
runSource: "issue.productive_terminal_continuation_recovery",
|
||||
livenessState: "advanced",
|
||||
});
|
||||
// Recent agent-authored comment should suppress the repeat-productive
|
||||
// escalation and let the normal continuation-retry path proceed.
|
||||
await db.insert(issueComments).values({
|
||||
companyId,
|
||||
issueId,
|
||||
authorAgentId: agentId,
|
||||
body: "frame 02/08 generated, attaching shortly",
|
||||
});
|
||||
const heartbeat = heartbeatService(db);
|
||||
|
||||
const result = await heartbeat.reconcileStrandedAssignedIssues();
|
||||
expect(result.escalated).toBe(0);
|
||||
expect(result.recentProgressExempted).toBe(1);
|
||||
expect(result.continuationRequeued).toBe(1);
|
||||
expect(result.issueIds).toEqual([issueId]);
|
||||
|
||||
const issue = await db.select().from(issues).where(eq(issues.id, issueId)).then((rows) => rows[0] ?? null);
|
||||
expect(issue?.status).toBe("in_progress");
|
||||
|
||||
const recoveryIssues = await db
|
||||
.select()
|
||||
.from(issues)
|
||||
.where(and(eq(issues.companyId, companyId), eq(issues.originKind, "stranded_issue_recovery")));
|
||||
expect(recoveryIssues).toHaveLength(0);
|
||||
|
||||
const runs = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.agentId, agentId));
|
||||
expect(runs).toHaveLength(2);
|
||||
const retryRun = runs.find((row) => row.id !== runId);
|
||||
expect(retryRun?.contextSnapshot as Record<string, unknown> | undefined).toMatchObject({
|
||||
issueId,
|
||||
retryReason: "issue_continuation_needed",
|
||||
source: "issue.productive_terminal_continuation_recovery",
|
||||
});
|
||||
});
|
||||
|
||||
it("still escalates stranded-recovery work when the recent comment is older than the exemption window (GGU-809)", async () => {
|
||||
const { companyId, agentId, issueId } = await seedStrandedIssueFixture({
|
||||
status: "in_progress",
|
||||
runStatus: "succeeded",
|
||||
retryReason: "issue_continuation_needed",
|
||||
runSource: "issue.productive_terminal_continuation_recovery",
|
||||
livenessState: "advanced",
|
||||
});
|
||||
// Comment older than the exemption window must NOT suppress escalation.
|
||||
const stale = new Date(Date.now() - 24 * 60 * 60 * 1000);
|
||||
await db.insert(issueComments).values({
|
||||
companyId,
|
||||
issueId,
|
||||
authorAgentId: agentId,
|
||||
body: "old progress note",
|
||||
createdAt: stale,
|
||||
updatedAt: stale,
|
||||
});
|
||||
const heartbeat = heartbeatService(db);
|
||||
|
||||
const result = await heartbeat.reconcileStrandedAssignedIssues();
|
||||
expect(result.escalated).toBe(1);
|
||||
expect(result.recentProgressExempted).toBe(0);
|
||||
expect(result.continuationRequeued).toBe(0);
|
||||
|
||||
const issue = await db.select().from(issues).where(eq(issues.id, issueId)).then((rows) => rows[0] ?? null);
|
||||
expect(issue?.status).toBe("blocked");
|
||||
});
|
||||
|
||||
it("does not reconcile user-assigned work through the agent stranded-work recovery path", async () => {
|
||||
const { issueId, runId } = await seedStrandedIssueFixture({
|
||||
status: "todo",
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
heartbeatRunEvents,
|
||||
heartbeatRunWatchdogDecisions,
|
||||
heartbeatRuns,
|
||||
issueAttachments,
|
||||
issueComments,
|
||||
issueApprovals,
|
||||
issueRecoveryActions,
|
||||
@@ -81,6 +82,18 @@ const SESSIONED_LOCAL_ADAPTERS = new Set([
|
||||
"pi_local",
|
||||
]);
|
||||
|
||||
// GGU-809: when a stranded `in_progress` issue would otherwise hit the
|
||||
// `isRepeatedProductiveContinuationRecovery` escalation path, exempt the
|
||||
// escalation if the assignee posted a comment or attachment within this window.
|
||||
// Batch workflows (e.g. Image Spec multi-frame generation) make real progress
|
||||
// every heartbeat and would otherwise trigger a recovery issue after just two
|
||||
// productive heartbeats. Floor the override at 60s to keep the exemption from
|
||||
// being effectively disabled by misconfiguration.
|
||||
export const STRANDED_RECENT_PROGRESS_EXEMPTION_MS = Math.max(
|
||||
60_000,
|
||||
Number(process.env.STRANDED_RECENT_PROGRESS_EXEMPTION_MS) || 30 * 60 * 1000,
|
||||
);
|
||||
|
||||
type RecoveryWakeupOptions = {
|
||||
source?: "timer" | "assignment" | "on_demand" | "automation";
|
||||
triggerDetail?: "manual" | "ping" | "callback" | "system";
|
||||
@@ -584,6 +597,47 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
|
||||
.then((rows) => Boolean(rows[0]));
|
||||
}
|
||||
|
||||
// GGU-809: visible-progress signal for stranded-recovery escalation guard.
|
||||
// Returns true if the assignee posted a comment, OR any attachment was added
|
||||
// to the issue, within `windowMs`. Used to suppress false-positive recovery
|
||||
// issues for batch workflows that genuinely advance every heartbeat.
|
||||
async function hasRecentVisibleProgress(
|
||||
companyId: string,
|
||||
issueId: string,
|
||||
assigneeAgentId: string,
|
||||
windowMs: number,
|
||||
) {
|
||||
const since = new Date(Date.now() - windowMs);
|
||||
const [comment, attachment] = await Promise.all([
|
||||
db
|
||||
.select({ id: issueComments.id })
|
||||
.from(issueComments)
|
||||
.where(
|
||||
and(
|
||||
eq(issueComments.companyId, companyId),
|
||||
eq(issueComments.issueId, issueId),
|
||||
eq(issueComments.authorAgentId, assigneeAgentId),
|
||||
gt(issueComments.createdAt, since),
|
||||
),
|
||||
)
|
||||
.limit(1)
|
||||
.then((rows) => rows[0] ?? null),
|
||||
db
|
||||
.select({ id: issueAttachments.id })
|
||||
.from(issueAttachments)
|
||||
.where(
|
||||
and(
|
||||
eq(issueAttachments.companyId, companyId),
|
||||
eq(issueAttachments.issueId, issueId),
|
||||
gt(issueAttachments.createdAt, since),
|
||||
),
|
||||
)
|
||||
.limit(1)
|
||||
.then((rows) => rows[0] ?? null),
|
||||
]);
|
||||
return Boolean(comment || attachment);
|
||||
}
|
||||
|
||||
async function enqueueStrandedIssueRecovery(input: {
|
||||
issueId: string;
|
||||
agentId: string;
|
||||
@@ -2475,6 +2529,7 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
|
||||
orphanBlockersAssigned: 0,
|
||||
successfulRunHandoffEscalated: 0,
|
||||
escalated: 0,
|
||||
recentProgressExempted: 0,
|
||||
skipped: 0,
|
||||
issueIds: [] as string[],
|
||||
};
|
||||
@@ -2623,21 +2678,34 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
|
||||
}
|
||||
|
||||
if (isRepeatedProductiveContinuationRecovery(successfulRun)) {
|
||||
const updated = await escalateStrandedAssignedIssue({
|
||||
issue,
|
||||
previousStatus: "in_progress",
|
||||
latestRun: successfulRun,
|
||||
comment:
|
||||
"Paperclip automatically retried continuation for this assigned `in_progress` issue and the retry " +
|
||||
"made progress, but it still has no live execution path. Moving it to `blocked` so it is visible for intervention.",
|
||||
});
|
||||
if (updated) {
|
||||
result.escalated += 1;
|
||||
result.issueIds.push(issue.id);
|
||||
} else {
|
||||
result.skipped += 1;
|
||||
// GGU-809: skip escalation if the assignee has shown visible progress
|
||||
// (comment or attachment) within the exemption window. Falling
|
||||
// through here lets the normal continuation-retry path enqueue the
|
||||
// next wake, which is the correct behaviour for batch workflows.
|
||||
const exempted = await hasRecentVisibleProgress(
|
||||
issue.companyId,
|
||||
issue.id,
|
||||
agentId,
|
||||
STRANDED_RECENT_PROGRESS_EXEMPTION_MS,
|
||||
);
|
||||
if (!exempted) {
|
||||
const updated = await escalateStrandedAssignedIssue({
|
||||
issue,
|
||||
previousStatus: "in_progress",
|
||||
latestRun: successfulRun,
|
||||
comment:
|
||||
"Paperclip automatically retried continuation for this assigned `in_progress` issue and the retry " +
|
||||
"made progress, but it still has no live execution path. Moving it to `blocked` so it is visible for intervention.",
|
||||
});
|
||||
if (updated) {
|
||||
result.escalated += 1;
|
||||
result.issueIds.push(issue.id);
|
||||
} else {
|
||||
result.skipped += 1;
|
||||
}
|
||||
continue;
|
||||
}
|
||||
continue;
|
||||
result.recentProgressExempted += 1;
|
||||
}
|
||||
|
||||
if (await isInvocationBudgetBlocked(issue, agentId)) {
|
||||
|
||||
Reference in New Issue
Block a user