mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
fix(recovery): let a new task wait for its manually paused assignee (#20281)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A person can pause an agent. Tasks assigned to a paused agent wait until the person resumes it. > - Stranded-task recovery looks for assigned work that cannot continue and moves it to `blocked` for the board. > - A task created for a manually paused agent has no run yet. Recovery saw an assignee that is not invokable and moved the new task to `blocked` a few seconds after creation. > - The person paused the agent on purpose. The blocked task and its "the board must choose the next action" comment fight that decision, and the person must unblock the task by hand after resume. > - This pull request lets new `todo` work with no run wait while a manual pause lasts. The existing assigned-todo dispatch starts it when the agent is resumed. > - The benefit is that work queued for a paused agent starts on resume, as the pause promises. ## Linked Issues or Issue Description **What happened** A chat evaluation pauses an agent (Morgan) and then runs `paperclipai issue create ... --assignee-agent-id <Morgan> --status todo`. The CLI returns the task with status `todo`. Within about five seconds the task changes to `blocked`, with the system comment "Paperclip cannot safely continue automatic recovery because the original assignee is not invokable. The source assignment is unchanged and the board must choose the next action." The server log shows only the expected 409 "Agent is not invokable" for the assignment wake. **Expected behavior** The task stays `todo` while the person keeps the agent paused. When the person resumes the agent, the task starts once. A comment and a `blocked` status are not added. **Steps to reproduce** 1. Pause an agent from its page. 2. Create a `todo` task assigned to that agent. 3. Wait for the stranded-task recovery sweep. See that the task is `blocked` with the comment above. ## What Changed - `server/src/services/recovery/service.ts`: in `reconcileStrandedAssignedIssues`, skip a `todo` task with no run when its assignee is paused with a `manual` lifecycle hold. The existing rule above it already skips work whose run the pause cancelled. Other pause causes, such as a budget hold, keep the existing escalation. - `server/src/__tests__/heartbeat-paused-assignee-new-task.test.ts`: a PostgreSQL test. The task stays `todo` with no comment and no run during two recovery sweeps. After resume it starts exactly once and finishes. A budget-held assignee still escalates. ## Verification - `cd server && npx vitest run src/__tests__/heartbeat-paused-assignee-new-task.test.ts`: 2 passed. Without the fix, the first test fails with `expected 'blocked' to be 'todo'`. - `npx vitest run src/__tests__/issue-recovery-actions.test.ts src/__tests__/heartbeat-refused-assignment-wake.test.ts`: passed. - `src/__tests__/issue-tree-pause-resume-run.test.ts` had timing failures on this low-memory machine both with and without this change (different tests each time). Those tests do not pause agents, so this change does not reach them. CI runs them on this branch. ## Risks - Low risk. The skip needs all of these: the agent is paused, it has a `manual` hold, the task is `todo`, and the task has no run. Work that already ran keeps the current rules. - No persisted state changes shape. State saved by the previous release carries forward: a task that the previous release already moved to `blocked` stays `blocked`. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), with tool use and code execution in Claude Code. ## 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Related: #20143 (a pause's stop never blocks the task behind an incident), #9998 (open: reject paused agents as assignment targets). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
6db0bb9776
commit
3ef45fdd92
2 files changed
+102
No files matched your search
@@ -0,0 +1,92 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { mkdtemp, rm } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import path from "node:path";
|
||||
import { and, eq } from "drizzle-orm";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { agents, authUsers, companies, companyMemberships, createDb, heartbeatRuns, issueComments, issues } from "@paperclipai/db";
|
||||
import { registerServerAdapter, unregisterServerAdapter } from "../adapters/index.js";
|
||||
import { heartbeatService } from "../services/heartbeat.js";
|
||||
import { recoveryPostgresSupport, startRecoveryPostgres } from "./helpers/recovery-postgres.js";
|
||||
|
||||
const describePostgres = recoveryPostgresSupport.supported ? describe : describe.skip;
|
||||
if (!recoveryPostgresSupport.supported) console.warn(`Skipping paused assignee new task tests: ${recoveryPostgresSupport.reason}`);
|
||||
|
||||
describePostgres("new task for a manually paused assignee", () => {
|
||||
let temporary: Awaited<ReturnType<typeof startRecoveryPostgres>>;
|
||||
let db: ReturnType<typeof createDb>;
|
||||
let heartbeat: ReturnType<typeof heartbeatService>;
|
||||
const adapterType = "codex_local";
|
||||
const dispatched: string[] = [];
|
||||
const oldHome = process.env.PAPERCLIP_HOME;
|
||||
let home: string;
|
||||
|
||||
beforeAll(async () => {
|
||||
temporary = await startRecoveryPostgres("heartbeat-paused-assignee-");
|
||||
db = createDb(temporary.connectionString);
|
||||
home = await mkdtemp(path.join(tmpdir(), "paperclip-paused-assignee-"));
|
||||
process.env.PAPERCLIP_HOME = home;
|
||||
heartbeat = heartbeatService(db);
|
||||
registerServerAdapter({
|
||||
type: adapterType, supportsLocalAgentJwt: false,
|
||||
execute: async input => {
|
||||
dispatched.push(input.runId);
|
||||
await db.update(issues).set({ status: "done" }).where(eq(issues.id, String(input.context.issueId)));
|
||||
return { exitCode: 0, signal: null, timedOut: false, resultJson: {} };
|
||||
},
|
||||
testEnvironment: async () => ({ adapterType, status: "pass", checks: [], testedAt: new Date().toISOString() }),
|
||||
});
|
||||
}, 240_000);
|
||||
|
||||
afterAll(async () => {
|
||||
await heartbeat?.drainActiveRunExecutions();
|
||||
unregisterServerAdapter(adapterType);
|
||||
await db?.$client.end({ timeout: 0 });
|
||||
await temporary?.cleanup();
|
||||
if (oldHome === undefined) delete process.env.PAPERCLIP_HOME; else process.env.PAPERCLIP_HOME = oldHome;
|
||||
if (home) await rm(home, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
async function seed(holds: string[]) {
|
||||
const companyId = randomUUID(), agentId = randomUUID(), issueId = randomUUID();
|
||||
const userId = `paused-user-${companyId}`;
|
||||
await db.insert(authUsers).values({ id: userId, name: "Board user", email: `${userId}@example.test`, createdAt: new Date(), updatedAt: new Date() });
|
||||
await db.insert(companies).values({ id: companyId, name: "Paused assignee fixture", issuePrefix: `P${companyId.slice(0, 6)}` });
|
||||
await db.insert(companyMemberships).values({ companyId, principalId: userId, principalType: "user", status: "active", membershipRole: "member" });
|
||||
await db.insert(agents).values({ id: agentId, companyId, name: "Morgan", adapterType, adapterConfig: {},
|
||||
status: "paused", lifecycleState: "paused", lifecycleHolds: holds, pauseReason: holds[0] ?? null, pausedAt: new Date() });
|
||||
await db.insert(issues).values({ id: issueId, companyId, title: "Task created while Morgan is paused", status: "todo",
|
||||
assigneeAgentId: agentId, responsibleUserId: userId, issueNumber: 1 });
|
||||
return { companyId, agentId, issueId };
|
||||
}
|
||||
|
||||
const runs = (f: { companyId: string; agentId: string }) => db.select().from(heartbeatRuns).where(and(
|
||||
eq(heartbeatRuns.companyId, f.companyId), eq(heartbeatRuns.agentId, f.agentId)));
|
||||
|
||||
it("keeps the task waiting while the pause lasts and starts it once on resume", async () => {
|
||||
const f = await seed(["manual"]);
|
||||
await heartbeat.reconcileStrandedAssignedIssues({ companyId: f.companyId });
|
||||
await heartbeat.reconcileStrandedAssignedIssues({ companyId: f.companyId });
|
||||
const [waiting] = await db.select().from(issues).where(eq(issues.id, f.issueId));
|
||||
expect(waiting.status).toBe("todo");
|
||||
expect(await db.select().from(issueComments).where(eq(issueComments.issueId, f.issueId))).toHaveLength(0);
|
||||
expect(await runs(f)).toHaveLength(0);
|
||||
|
||||
await db.update(agents).set({ status: "idle", lifecycleState: "ready", lifecycleHolds: [], pauseReason: null, pausedAt: null })
|
||||
.where(eq(agents.id, f.agentId));
|
||||
await heartbeat.reconcileStrandedAssignedIssues({ companyId: f.companyId });
|
||||
await heartbeat.drainActiveRunExecutions();
|
||||
const ids = new Set((await runs(f)).map(run => run.id));
|
||||
expect(dispatched.filter(id => ids.has(id))).toHaveLength(1);
|
||||
const [finished] = await db.select().from(issues).where(eq(issues.id, f.issueId));
|
||||
expect(finished.status).toBe("done");
|
||||
}, 180_000);
|
||||
|
||||
it("still escalates a new task whose assignee is paused by something other than a person", async () => {
|
||||
const f = await seed(["budget"]);
|
||||
await heartbeat.reconcileStrandedAssignedIssues({ companyId: f.companyId });
|
||||
const [saved] = await db.select().from(issues).where(eq(issues.id, f.issueId));
|
||||
expect(saved.status).not.toBe("todo");
|
||||
expect(await runs(f)).toHaveLength(0);
|
||||
}, 180_000);
|
||||
});
|
||||
@@ -4725,6 +4725,16 @@ export function recoveryService(
|
||||
result.skipped += 1;
|
||||
continue;
|
||||
}
|
||||
if (
|
||||
agent?.status === "paused" &&
|
||||
agent.companyId === issue.companyId &&
|
||||
agent.lifecycleHolds.includes("manual") &&
|
||||
issue.status === "todo" &&
|
||||
!latestRun
|
||||
) {
|
||||
result.skipped += 1;
|
||||
continue;
|
||||
}
|
||||
if (issue.status !== "in_review" && !agentInvokable) {
|
||||
const classification = classifyContinuationFailure(latestRun);
|
||||
if (
|
||||
|
||||
Reference in new issue
Block a user