diff --git a/maps/agent-access-map/task-coordination.md b/maps/agent-access-map/task-coordination.md index a9bd1ddcc56..dda7994a89a 100644 --- a/maps/agent-access-map/task-coordination.md +++ b/maps/agent-access-map/task-coordination.md @@ -27,7 +27,11 @@ API: `present` — representative operation IDs: Agent access: `available` — Native create_task/reassign_task; REST / CLI / MCP. Policy: Task visibility/mutation, task-assignment and ownership policy; review audience -independent; tree controls board-only. +independent; tree controls board-only. Resume preview counts only current agent +assignees on todo, in-progress, and in-review tasks as wake targets. Release keeps +blocked and backlog statuses intact; existing execution gates still apply. +Reopened tasks use current eligibility rather than the pause snapshot's skipped +flag. Other active pause holds and execution reconciliation gates remain effective. Evidence: `source-only` — [implementation](../../server/src/routes/issue-tree-control.ts#L95) at the reviewed diff --git a/maps/feature-map/task-coordination.md b/maps/feature-map/task-coordination.md index 86a60dc5611..67f58c06244 100644 --- a/maps/feature-map/task-coordination.md +++ b/maps/feature-map/task-coordination.md @@ -49,6 +49,13 @@ Automated: [execution policy](../../server/src/__tests__/issue-execution-policy. Manual: Complete implementation under a review-required policy and confirm it waits for the required review outcome. Exercise rejection and later approval. Pause the parent while child work is pending, then verify resumption respects budget and ownership gates. +Resume removes the pause without changing task status. Its wake count includes only +current agent assignees on todo, in-progress, or in-review tasks. The dialog explains +that blocked, backlog, and unassigned tasks remain idle. Verify this in both task +interfaces with [the paused composer browser suite](../../tests/e2e/paused-composer.spec.ts). +Release rechecks tasks reopened after the pause; another active pause still prevents +their execution, and uncertain prior executions must be reconciled before waking. + ## Gotchas - Review completion and task completion are different events under some policies. diff --git a/server/src/__tests__/issue-tree-control-routes.test.ts b/server/src/__tests__/issue-tree-control-routes.test.ts index 7931bb768ec..682650b7c5d 100644 --- a/server/src/__tests__/issue-tree-control-routes.test.ts +++ b/server/src/__tests__/issue-tree-control-routes.test.ts @@ -14,6 +14,7 @@ const mockTreeControlService = vi.hoisted(() => ({ cancelIssueStatusesForHold: vi.fn(), restoreIssueStatusesForHold: vi.fn(), getHold: vi.fn(), + getActivePauseHoldGate: vi.fn(), releaseHold: vi.fn(), cancelUnclaimedWakeupsForTree: vi.fn(), })); @@ -72,6 +73,7 @@ describe("issue tree control routes", () => { mockReplayBlocks.mockResolvedValue([]); mockExecutionBlocker.mockResolvedValue(null); mockTreeControlService.getHold.mockResolvedValue(null); + mockTreeControlService.getActivePauseHoldGate.mockResolvedValue(null); mockIssueService.getById.mockResolvedValue({ id: "11111111-1111-4111-8111-111111111111", companyId: "company-2", @@ -213,6 +215,36 @@ describe("issue tree control routes", () => { }, ); + it.each([ + { skipReason: "terminal_status", stillPaused: false }, + { skipReason: "already_held", stillPaused: false }, + { skipReason: "already_held", stillPaused: true }, + ])("rechecks skipped pause members with $skipReason and remaining pause=$stillPaused", async ({ skipReason, stillPaused }) => { + const rootId = "11111111-1111-4111-8111-111111111111"; + const holdId = "33333333-3333-4333-8333-333333333333"; + const childId = "44444444-4444-4444-8444-444444444444"; + const root = { id: rootId, companyId: "company-2", status: "todo", assigneeAgentId: null }; + const child = { id: childId, companyId: "company-2", status: "todo", assigneeAgentId: "current-agent" }; + mockIssueService.getById.mockImplementation(async (id: string) => id === rootId ? root : child); + const hold = { id: holdId, rootIssueId: rootId, mode: "pause", members: [ + { issueId: childId, skipped: true, skipReason, issueStatus: "done" }, + ] }; + mockTreeControlService.getHold.mockResolvedValue(hold); + mockTreeControlService.releaseHold.mockResolvedValue({ ...hold, status: "released" }); + mockTreeControlService.getActivePauseHoldGate.mockResolvedValue(stillPaused ? { holdId: "remaining-hold" } : null); + mockReplayBlocks.mockResolvedValue([{ id: childId, identifier: "TEST-2" }]); + const app = await createApp({ type: "board", userId: "user-1", companyIds: ["company-2"], source: "session" }); + const response = await request(app) + .post(`/api/issues/${rootId}/tree-holds/${holdId}/release`) + .send({ metadata: { wakeAgents: true } }); + expect(response.status).toBe(200); + expect(mockExecutionBlocker).toHaveBeenCalledWith(expect.anything(), "company-2", childId); + expect(mockHeartbeatService.wakeup).toHaveBeenCalledTimes(stillPaused ? 0 : 1); + if (!stillPaused) expect(mockHeartbeatService.wakeup).toHaveBeenCalledWith("current-agent", expect.objectContaining({ + payload: expect.objectContaining({ issueId: childId }), + })); + }); + it("resumes sandbox-backed work through the shared plugin worker manager", async () => { const rootId = "11111111-1111-4111-8111-111111111111"; const holdId = "33333333-3333-4333-8333-333333333333"; @@ -284,14 +316,14 @@ describe("issue tree control routes", () => { ).toEqual([rootId, "child"]); }); - it("keeps the hold active when resume requests an unsafe execution replay", async () => { + it.each([false, true])("keeps the hold active when resume requests an unsafe execution replay (previously skipped=%s)", async (skipped) => { const rootId = "11111111-1111-4111-8111-111111111111"; const holdId = "33333333-3333-4333-8333-333333333333"; mockTreeControlService.getHold.mockResolvedValue({ id: holdId, rootIssueId: rootId, mode: "pause", - members: [{ issueId: rootId }], + members: [{ issueId: rootId, skipped, skipReason: skipped ? "terminal_status" : null }], }); mockReplayBlocks.mockResolvedValue([{ id: rootId, identifier: "TEST-1" }]); mockExecutionBlocker.mockResolvedValue({ nextAction: "Resume without waking agents until the previous execution stops.", diff --git a/server/src/__tests__/issue-tree-control-service.test.ts b/server/src/__tests__/issue-tree-control-service.test.ts index 43c620053a5..923f399d3bd 100644 --- a/server/src/__tests__/issue-tree-control-service.test.ts +++ b/server/src/__tests__/issue-tree-control-service.test.ts @@ -6,6 +6,8 @@ import { agents, agentWakeupRequests, companies, + applyPendingMigrations, + closeRegisteredClients, createDb, heartbeatRuns, issueComments, @@ -20,8 +22,10 @@ import { } from "./helpers/embedded-postgres.js"; import { issueTreeControlService } from "../services/issue-tree-control.js"; import { issueService } from "../services/issues.js"; +import { resolveExternalTestDatabaseUrl } from "./helpers/external-test-database.js"; -const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const externalUrl = resolveExternalTestDatabaseUrl("PAPERCLIP_ISSUE_TREE_TEST_DATABASE_URL"); +const embeddedPostgresSupport = externalUrl ? { supported: true } : await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; if (!embeddedPostgresSupport.supported) { @@ -35,6 +39,11 @@ describeEmbeddedPostgres("issueTreeControlService", () => { let tempDb: Awaited> | null = null; beforeAll(async () => { + if (externalUrl) { + await applyPendingMigrations(externalUrl); + db = createDb(externalUrl); + return; + } tempDb = await startEmbeddedPostgresTestDatabase("paperclip-issue-tree-control-"); db = createDb(tempDb.connectionString); }, 20_000); @@ -53,6 +62,7 @@ describeEmbeddedPostgres("issueTreeControlService", () => { }); afterAll(async () => { + if (externalUrl) await closeRegisteredClients(externalUrl); await tempDb?.cleanup(); }); @@ -623,6 +633,58 @@ describeEmbeddedPostgres("issueTreeControlService", () => { expect(checkedOutLegacyFullPauseRoot.checkoutRunId).toBe(rootRunId); }); + it("previews only executable current assignees as resume wake targets", async () => { + const companyId = randomUUID(); + const rootId = randomUUID(); + const readyAgentId = randomUUID(); + const parkedAgentId = randomUUID(); + const previousAgentId = randomUUID(); + const runId = randomUUID(); + await db.insert(companies).values({ + id: companyId, name: "Resume preview", issuePrefix: "RESUME", + }); + await db.insert(agents).values([readyAgentId, parkedAgentId, previousAgentId].map((id) => ({ + id, companyId, name: id, role: "engineer", status: "idle", adapterType: "process", + }))); + const children = ["todo", "in_progress", "in_review", "blocked", "backlog", "done"].map((status) => ({ + id: randomUUID(), companyId, parentId: rootId, title: status, status, + assigneeAgentId: status === "blocked" || status === "backlog" ? parkedAgentId : readyAgentId, + })); + await db.insert(issues).values([ + { id: rootId, companyId, title: "Unassigned root", status: "todo" }, + ...children, + ]); + const svc = issueTreeControlService(db); + const pause = await svc.createHold(companyId, rootId, { + mode: "pause", + actor: { actorType: "user", actorId: "board-user", userId: "board-user" }, + }); + expect(pause.hold.members?.find((member) => member.issueId === children[5].id)).toMatchObject({ + skipped: true, skipReason: "terminal_status", + }); + await db.update(issues).set({ status: "todo" }).where(eq(issues.id, children[5].id)); + // A previous executor is observable while stopping, but is not the current assignee to wake. + await db.insert(heartbeatRuns).values({ + id: runId, companyId, agentId: previousAgentId, invocationSource: "assignment", + status: "running", contextSnapshot: { issueId: children[0].id }, + }); + await db.update(issues).set({ executionRunId: runId }).where(eq(issues.id, children[0].id)); + + const preview = await svc.preview(companyId, rootId, { mode: "resume" }); + expect(preview.totals.affectedIssues).toBe(7); + expect(preview.affectedAgents).toEqual([{ agentId: readyAgentId, issueCount: 4, activeRunCount: 0 }]); + expect(preview.totals.affectedAgents).toBe(1); + expect(preview.warnings).toContainEqual({ + code: "resume_tasks_remain_idle", + message: "Blocked, backlog, and unassigned tasks will be unpaused but will not start.", + issueIds: [rootId, children[3].id, children[4].id].sort(), + }); + const rows = await db.select().from(issues).where(eq(issues.companyId, companyId)); + expect(rows.find((row) => row.id === children[3].id)?.status).toBe("blocked"); + expect(rows.find((row) => row.id === children[4].id)?.status).toBe("backlog"); + expect((await svc.getHold(companyId, pause.hold.id))?.status).toBe("active"); + }); + it("resumes subtree pauses by releasing matching pause holds", async () => { const companyId = randomUUID(); const rootIssueId = randomUUID(); diff --git a/server/src/routes/issue-tree-control.ts b/server/src/routes/issue-tree-control.ts index c9a00491ba3..05c208756e9 100644 --- a/server/src/routes/issue-tree-control.ts +++ b/server/src/routes/issue-tree-control.ts @@ -6,6 +6,7 @@ import { } from "@paperclipai/db"; import { and, eq, inArray, isNotNull } from "drizzle-orm"; import { getExecutionBlocker } from "../services/execution-blocker.js"; +import { ISSUE_TREE_RESUME_EXECUTABLE_STATUSES } from "../services/issue-tree-control.js"; import { conflict } from "../errors.js"; import { createIssueTreeHoldSchema, @@ -26,7 +27,6 @@ import { assertBoard, getAccessibleResource, getActorInfo } from "./authz.js"; import type { PluginWorkerManager } from "../services/plugin-worker-manager.js"; const TREE_RUN_CANCELLATION_RESPONSE_WAIT_MS = 1_000; -const RESUME_EXECUTABLE_STATUSES = ["todo", "in_progress", "in_review"]; function errorToMessage(error: unknown) { return error instanceof Error ? error.message : String(error); @@ -436,15 +436,13 @@ export function issueTreeControlRoutes( const activeHold = await treeControlSvc.getHold(root.companyId, holdId); const issueIds = activeHold?.mode === "pause" && activeHold.rootIssueId === root.id - ? (activeHold.members ?? []) - .filter((member) => !member.skipped) - .map((member) => member.issueId) + ? (activeHold.members ?? []).map((member) => member.issueId) : []; if (issueIds.length > 0) { const candidates = await db.select({ id: issueRows.id, identifier: issueRows.identifier }) .from(issueRows).where(and( eq(issueRows.companyId, root.companyId), inArray(issueRows.id, issueIds), - inArray(issueRows.status, RESUME_EXECUTABLE_STATUSES), isNotNull(issueRows.assigneeAgentId), + inArray(issueRows.status, ISSUE_TREE_RESUME_EXECUTABLE_STATUSES), isNotNull(issueRows.assigneeAgentId), )); for (const task of candidates) { const blocked = await getExecutionBlocker(db, root.companyId, task.id); @@ -489,16 +487,18 @@ export function issueTreeControlRoutes( const wakeFailures: Array<{ issueId: string; message: string }> = []; if (hold.mode === "pause" && req.body.metadata?.wakeAgents === true) { for (const member of hold.members ?? []) { - if (member.skipped) continue; try { const issue = await issuesSvc.getById(member.issueId); if ( !issue || issue.companyId !== root.companyId || !issue.assigneeAgentId || - !RESUME_EXECUTABLE_STATUSES.includes(issue.status) + !ISSUE_TREE_RESUME_EXECUTABLE_STATUSES.includes(issue.status) ) continue; + // The snapshot may predate reopening a task or releasing another + // pause. Current task state and remaining holds govern execution. + if (await treeControlSvc.getActivePauseHoldGate(root.companyId, issue.id)) continue; await heartbeat.wakeup(issue.assigneeAgentId, { source: "assignment", triggerDetail: "system", diff --git a/server/src/services/issue-tree-control.ts b/server/src/services/issue-tree-control.ts index ecf8a07c6fd..1d8af92fd5b 100644 --- a/server/src/services/issue-tree-control.ts +++ b/server/src/services/issue-tree-control.ts @@ -71,6 +71,7 @@ type RestoreTreeStatusResult = TreeStatusUpdateResult & { }; const TERMINAL_ISSUE_STATUSES = new Set(["done", "cancelled"]); +export const ISSUE_TREE_RESUME_EXECUTABLE_STATUSES = ["todo", "in_progress", "in_review"]; const ACTIVE_RUN_STATUSES = ["queued", "running"] as const; const DEFAULT_RELEASE_POLICY: IssueTreeHoldReleasePolicy = { strategy: "manual" }; const MAX_PAUSE_HOLD_ANCESTOR_DEPTH = 100; @@ -325,13 +326,21 @@ function issueSkipReason(input: { return null; } -function buildAffectedAgents(issuesToPreview: IssueTreePreviewIssue[]): IssueTreePreviewAgent[] { +function canWakeOnResume(issue: IssueTreePreviewIssue): boolean { + return !!issue.assigneeAgentId && ISSUE_TREE_RESUME_EXECUTABLE_STATUSES.includes(issue.status); +} + +function buildAffectedAgents( + issuesToPreview: IssueTreePreviewIssue[], + mode: IssueTreeControlMode, +): IssueTreePreviewAgent[] { const byAgentId = new Map(); for (const issue of issuesToPreview) { if (issue.skipped) continue; + if (mode === "resume" && !canWakeOnResume(issue)) continue; const agentIds = new Set(); if (issue.assigneeAgentId) agentIds.add(issue.assigneeAgentId); - if (issue.activeRun) agentIds.add(issue.activeRun.agentId); + if (mode !== "resume" && issue.activeRun) agentIds.add(issue.activeRun.agentId); for (const agentId of agentIds) { const current = byAgentId.get(agentId) ?? { agentId, issueCount: 0, activeRunCount: 0 }; current.issueCount += 1; @@ -388,6 +397,17 @@ function buildWarnings(input: { }); } + if (input.mode === "resume") { + const idleIssueIds = affectedIssues.filter((issue) => !canWakeOnResume(issue)).map((issue) => issue.id); + if (idleIssueIds.length > 0) { + warnings.push({ + code: "resume_tasks_remain_idle", + message: "Blocked, backlog, and unassigned tasks will be unpaused but will not start.", + issueIds: idleIssueIds.sort(), + }); + } + } + if (input.mode === "restore") { const changedIssueIds = input.issuesToPreview .filter((issue) => issue.skipReason === "changed_after_cancel") @@ -676,7 +696,7 @@ export function issueTreeControlService(db: Db) { const activeRuns = activeRunRows .map(toPreviewRun) .sort((a, b) => a.issueId.localeCompare(b.issueId) || a.id.localeCompare(b.id)); - const affectedAgents = buildAffectedAgents(issuesToPreview); + const affectedAgents = buildAffectedAgents(issuesToPreview, input.mode); return { companyId, diff --git a/tests/e2e/composer-stop.spec.ts b/tests/e2e/composer-stop.spec.ts index a9c747f9bf5..30c31a33a8b 100644 --- a/tests/e2e/composer-stop.spec.ts +++ b/tests/e2e/composer-stop.spec.ts @@ -380,7 +380,16 @@ for (const adapter of ["process", "paperclip_runner"] as const) { ).toEqual([]); } await menu(page, "Resume subtree"); - await page.getByRole("dialog").getByRole("checkbox").check(); + const resumeWake = page.getByRole("dialog").getByRole("checkbox"); + if (adapter === "process") { + // Stopped legacy work is blocked pending reconciliation. Removing the + // pause must not promise or request a wake through that recovery gate. + await expect(resumeWake).toHaveAccessibleName("Wake affected agents (0)"); + await expect(resumeWake).toBeDisabled(); + await expect(resumeWake).not.toBeChecked(); + } else { + await resumeWake.check(); + } await page .getByRole("dialog") .getByRole("button", { name: "Resume subtree", exact: true }) diff --git a/tests/e2e/paused-composer.spec.ts b/tests/e2e/paused-composer.spec.ts index c01d083632a..f3c41525648 100644 --- a/tests/e2e/paused-composer.spec.ts +++ b/tests/e2e/paused-composer.spec.ts @@ -7,6 +7,51 @@ async function json(response: APIResponse) { } for (const classic of [false, true]) { + test(`resume preview: ${classic ? "classic" : "task chat"} does not promise to wake blocked work`, async ({ page, request }) => { + test.setTimeout(120_000); + const company = await json(await request.post("/api/companies", { data: { name: `Resume preview ${Date.now()}` } })); + const settings = await json(await request.get("/api/instance/settings/experimental")); + try { + await json(await request.patch("/api/instance/settings/experimental", { data: { enableClassicTaskInterface: classic } })); + const agent = await json(await request.post(`/api/companies/${company.id}/agents`, { data: { + name: "Resume preview fixture", role: "engineer", adapterType: "process", + adapterConfig: { command: "/usr/bin/true" }, + runtimeConfig: { heartbeat: { enabled: false, wakeOnDemand: false } }, + } })); + const task = await json(await request.post(`/api/companies/${company.id}/issues`, { data: { + title: "Blocked work stays blocked", status: "blocked", assigneeAgentId: agent.id, + } })); + await page.goto(`/${company.issuePrefix}/issues/${task.identifier}`); + await page.getByRole("button", { name: "More task actions", exact: true }).click(); + await page.getByRole("button", { name: "Pause work", exact: true }).click(); + const takeover = page.getByTestId("paused-composer-takeover"); + await expect(takeover).toBeVisible(); + // Enable dispatch only after the hold exists; assignment itself otherwise starts a run. + expect((await json(await request.get(`/api/issues/${task.id}`))).status).toBe("blocked"); + await json(await request.patch(`/api/agents/${agent.id}`, { data: { + runtimeConfig: { heartbeat: { enabled: false, wakeOnDemand: true } }, + } })); + await takeover.getByRole("button", { name: "Resume task" }).click(); + const dialog = page.getByRole("dialog"); + await expect(dialog).toContainText("The pause will be removed from 1 task."); + const wake = dialog.getByRole("checkbox", { name: "Wake affected agents (0)" }); + await expect(wake).toBeDisabled(); + await expect(wake).not.toBeChecked(); + await expect(dialog).toContainText("Blocked, backlog, and unassigned tasks will be unpaused but will not start."); + await dialog.getByRole("button", { name: "Resume work", exact: true }).click(); + await expect(dialog).toHaveCount(0); + await expect(takeover).toHaveCount(0); + await page.reload(); + const editor = (classic ? page.getByTestId("issue-chat-composer") : page).getByRole("textbox", { name: "editable markdown" }); + await expect(editor).toBeVisible(); + expect((await json(await request.get(`/api/issues/${task.id}`))).status).toBe("blocked"); + expect(await json(await request.get(`/api/companies/${company.id}/heartbeat-runs?agentId=${agent.id}`))).toEqual([]); + } finally { + await request.patch(`/api/companies/${company.id}`, { data: { status: "archived" } }); + await request.patch("/api/instance/settings/experimental", { data: { enableClassicTaskInterface: settings.enableClassicTaskInterface } }); + } + }); + test(`paused composer: ${classic ? "classic" : "task chat"} preserves drafts and requires resume`, async ({ page, request }) => { test.setTimeout(120_000); const company = await json(await request.post("/api/companies", { data: { name: `Paused composer ${Date.now()}` } })); @@ -63,8 +108,11 @@ for (const classic of [false, true]) { await takeover.getByRole("link", { name: "Resume subtree" }).click(); await expect(page).toHaveURL(new RegExp(task.identifier)); await takeover.getByRole("button", { name: "Resume subtree" }).click(); - await page.getByRole("dialog").getByRole("checkbox").uncheck(); + const subtreeWake = page.getByRole("dialog").getByRole("checkbox"); + if (await subtreeWake.isEnabled()) await subtreeWake.uncheck(); + else await expect(subtreeWake).not.toBeChecked(); await page.getByRole("dialog").getByRole("button", { name: "Resume subtree", exact: true }).click(); + await expect(page.getByRole("dialog")).toHaveCount(0); await page.goto(`/${company.issuePrefix}/issues/${child.identifier}`); await expect(takeover).toHaveCount(0); await expect(editor).toBeVisible(); diff --git a/ui/src/components/TaskTreeControls.tsx b/ui/src/components/TaskTreeControls.tsx index d2fd9137547..dfd0d1aaf1f 100644 --- a/ui/src/components/TaskTreeControls.tsx +++ b/ui/src/components/TaskTreeControls.tsx @@ -78,6 +78,7 @@ export function TaskTreeControlDialog({ scope, affectedCount, affectedAgentCount, + warnings = [], loading, error, pending, @@ -93,6 +94,7 @@ export function TaskTreeControlDialog({ scope: "leaf" | "subtree"; affectedCount: number; affectedAgentCount: number; + warnings?: string[]; loading: boolean; error?: string | null; pending: boolean; @@ -129,7 +131,9 @@ export function TaskTreeControlDialog({ ? "Loading…" : cancel ? `${tasks} will be cancelled.` - : `${tasks} will ${mode === "restore" ? "be restored" : "resume"}.`} + : mode === "restore" + ? `${tasks} will be restored.` + : `The pause will be removed from ${tasks}.`} {error ? ( @@ -151,13 +155,16 @@ export function TaskTreeControlDialog({ ) : null} + {!loading ? warnings.map((warning) => ( +

{warning}

+ )) : null}