diff --git a/docs/api/issues.md b/docs/api/issues.md index 7478e5480bc..f1e0ab0d6eb 100644 --- a/docs/api/issues.md +++ b/docs/api/issues.md @@ -68,6 +68,8 @@ Updatable fields: `title`, `description`, `status`, `priority`, `assigneeAgentId For `PATCH /api/issues/{issueId}`, `assigneeAgentId` may be either the agent UUID or the agent shortname/urlKey within the same company. +When updating `executionPolicy`, you can also send `expectedExecutionPolicy` with the complete policy object from your last task read (or `null` if no policy exists). The server compares this snapshot under the task row lock. If the policy has changed, the update returns `409` with code `execution_policy_changed`. Read the task again before retrying. Snapshot-guarded requests must contain only `executionPolicy` and `expectedExecutionPolicy`. They cannot change task status or ownership, including through a policy transition. Mixed requests return `422` before execution-control side effects. Omit this field to retain the existing update behavior. + ### Update Response Without a `Prefer` header, a successful update returns the full, updated issue row with two additive fields: diff --git a/packages/shared/src/validators/issue.test.ts b/packages/shared/src/validators/issue.test.ts index ab714359114..b0d99661ef9 100644 --- a/packages/shared/src/validators/issue.test.ts +++ b/packages/shared/src/validators/issue.test.ts @@ -16,6 +16,13 @@ import { import { createAgentSchema } from "./agent.js"; describe("issue validators", () => { + it("preserves the exact expected execution policy snapshot without defaults", () => { + const snapshot = { mode: "normal", stages: [], monitor: { nextCheckAt: "2026-10-10T12:00:00Z" } }; + expect(updateIssueSchema.parse({ expectedExecutionPolicy: snapshot }).expectedExecutionPolicy).toEqual(snapshot); + expect(updateIssueSchema.parse({ expectedExecutionPolicy: null }).expectedExecutionPolicy).toBeNull(); + expect(updateIssueSchema.parse({}).expectedExecutionPolicy).toBeUndefined(); + expect(updateIssueSchema.safeParse({ expectedExecutionPolicy: [] }).success).toBe(false); + }); it("accepts private visibility without adding defaults to updates", () => { expect(createIssueSchema.parse({ title: "Private", visibility: "private" }).visibility).toBe("private"); expect(createIssueSchema.parse({ title: "Open" }).visibility).toBe("open"); diff --git a/packages/shared/src/validators/issue.ts b/packages/shared/src/validators/issue.ts index 8d56cc0c496..29f6f07376c 100644 --- a/packages/shared/src/validators/issue.ts +++ b/packages/shared/src/validators/issue.ts @@ -891,6 +891,8 @@ export const updateIssueSchema = objectWithoutDefaults( ) .partial() .extend({ + /** Reject a policy write if the policy changed after the caller read it. */ + expectedExecutionPolicy: z.record(z.string(), z.unknown()).optional().nullable(), requestDepth: issueRequestDepthInputSchema.optional(), assigneeAgentId: z.string().trim().min(1).optional().nullable(), comment: multilineTextSchema.pipe(z.string().min(1)).optional(), diff --git a/server/src/__tests__/issue-execution-policy-routes.test.ts b/server/src/__tests__/issue-execution-policy-routes.test.ts index be3f8c36cb0..4ead3acee65 100644 --- a/server/src/__tests__/issue-execution-policy-routes.test.ts +++ b/server/src/__tests__/issue-execution-policy-routes.test.ts @@ -272,6 +272,34 @@ describe("issue execution policy routes", () => { await loadAppModules(); }); + it.each([ + { status: "done" }, + { status: "cancelled" }, + { assigneeAgentId: "33333333-3333-4333-8333-333333333333" }, + { assigneeUserId: "operator" }, + { interrupt: true }, + { comment: "Continue now" }, + ])("rejects mixed snapshot-guarded updates before execution side effects: %j", async (controlFields) => { + const res = await request(await createApp({ type: "board", source: "local_implicit", userId: "local-board" })) + .patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa") + .send({ expectedExecutionPolicy: null, executionPolicy: null, ...controlFields }); + expect(res.status).toBe(422); + expect(res.body.error).toContain("policy-only"); + expect(mockIssueService.getById).not.toHaveBeenCalled(); + expect(mockIssueService.update).not.toHaveBeenCalled(); + expect(mockRunnerGoalService.act).not.toHaveBeenCalled(); + expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled(); + }); + + it("rejects a snapshot without a policy update", async () => { + const res = await request(await createApp({ type: "board", source: "local_implicit", userId: "local-board" })) + .patch("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa") + .send({ expectedExecutionPolicy: null }); + expect(res.status).toBe(422); + expect(mockIssueService.getById).not.toHaveBeenCalled(); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + it("reauthorizes a terminal verdict against the review policy held under the update lock", async () => { const issue = { id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index a71cc5b3b2d..c40bc18031d 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -540,6 +540,34 @@ describeEmbeddedPostgres("issueService.list participantAgentId", () => { await ensureIssueRelationsTable(db); }, 20_000); + it("rejects stale monitor cancellation under the task lock and preserves saved policy settings", async () => { + const companyId = randomUUID(); + const issueId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Monitor cancellation", issuePrefix: "MCL", requireBoardApprovalForNewAgents: false }); + const oldPolicy = { + mode: "normal" as const, commentRequired: true, stages: [], + monitor: { nextCheckAt: "2026-10-10T12:00:00.000Z", notes: "Check status", scheduledBy: "board" as const }, + }; + const savedPolicy = { + ...oldPolicy, + stages: [{ type: "review" as const, approvalsNeeded: 1 as const, participants: [{ type: "user" as const, userId: "reviewer" }] }], + maxReviewRounds: 4, + authorizationPolicy: { assignmentPolicy: { mode: "protected" as const } }, + }; + await db.insert(issues).values({ id: issueId, companyId, title: "Monitored task", executionPolicy: oldPolicy }); + await svc.update(issueId, { executionPolicy: savedPolicy }); + const { monitor: _oldMonitor, ...staleCancellation } = oldPolicy; + await expect(svc.update(issueId, { executionPolicy: staleCancellation, expectedExecutionPolicy: oldPolicy })) + .rejects.toMatchObject({ status: 409 }); + expect((await svc.getById(issueId))?.executionPolicy).toEqual(savedPolicy); + const { monitor: _savedMonitor, ...cancellation } = savedPolicy; + await svc.update(issueId, { executionPolicy: cancellation, expectedExecutionPolicy: savedPolicy }); + expect((await svc.getById(issueId))?.executionPolicy).toEqual(cancellation); + await expect(svc.update(issueId, { executionPolicy: null, expectedExecutionPolicy: null })) + .rejects.toMatchObject({ status: 409 }); + expect((await svc.getById(issueId))?.executionPolicy).toEqual(cancellation); + }); + afterEach(async () => { await db.delete(issueComments); await db.delete(issueThreadInteractions); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 4ce53bce298..f738e91abac 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -13373,6 +13373,14 @@ export function issueRoutes( "/issues/:id", validateIssueMutationBody(updateIssueRouteSchema), async (req, res) => { + // Snapshot-guarded writes are policy-only. Do not admit task-control + // actions that can stop execution before the service takes its row lock. + if (req.body.expectedExecutionPolicy !== undefined && ( + req.body.executionPolicy === undefined || + Object.keys(req.body).some((key) => key !== "executionPolicy" && key !== "expectedExecutionPolicy") + )) { + throw unprocessable("expectedExecutionPolicy requires a policy-only update"); + } const id = req.params.id as string; const existing = await getAccessibleResource( req, @@ -13459,6 +13467,7 @@ export function issueRoutes( interrupt: interruptRequested, deferWakeForGoal, hiddenAt: hiddenAtRaw, + expectedExecutionPolicy, onBehalfOfUserId: _requestedOnBehalfOfUserId, ...updateFields } = req.body; @@ -13856,6 +13865,13 @@ export function issueRoutes( lastDecisionId: decisionId, }; } + if (expectedExecutionPolicy !== undefined && ( + (transition.patch.status !== undefined && transition.patch.status !== existing.status) || + (transition.patch.assigneeAgentId !== undefined && transition.patch.assigneeAgentId !== existing.assigneeAgentId) || + (transition.patch.assigneeUserId !== undefined && transition.patch.assigneeUserId !== existing.assigneeUserId) + )) { + throw unprocessable("Snapshot-guarded policy updates cannot change task status or ownership"); + } Object.assign(updateFields, transition.patch); const nextStatus = updateFields.status ?? existing.status; @@ -14178,6 +14194,7 @@ export function issueRoutes( const postCommitIssueActions: IssuePostCommitAction[] = []; const issueUpdateData = { ...updateFields, + expectedExecutionPolicy, actorAgentId: actor.agentId ?? null, actorRunId: actor.agentId ? actor.runId : null, actorRunStopId: actor.agentId && interruptedRunId === actor.runId ? issueMutationStopId : null, diff --git a/server/src/services/issues.ts b/server/src/services/issues.ts index a4ef06d713d..400593dace3 100644 --- a/server/src/services/issues.ts +++ b/server/src/services/issues.ts @@ -9,6 +9,7 @@ import { createdFromIssueCondition } from "./issue-creation-origin.js"; import { executionProjectionsForRuns } from "./execution-projection.js"; import type { ExecutionProjection } from "@paperclipai/shared"; import { Buffer } from "node:buffer"; +import { isDeepStrictEqual } from "node:util"; import { createHash, randomUUID } from "node:crypto"; import { isExplicitContinuationRetryClaim } from "./explicit-continuation-retry-claim.js"; import { markdownToPlainText, parseMarkdown } from "chat"; @@ -10833,6 +10834,7 @@ export function issueService(db: Db) { actorRunStopId?: string | null; actorUserId?: string | null; companyGuard?: string; + expectedExecutionPolicy?: typeof issues.$inferInsert.executionPolicy; }, dbOrTx: any = db, postCommitActivityPublications?: ActivityPublication[], @@ -10880,6 +10882,7 @@ export function issueService(db: Db) { actorRunStopId, actorUserId, companyGuard, + expectedExecutionPolicy, ...issueData } = data; // An explicit edit claims the title, even if it keeps the same text. @@ -11172,6 +11175,14 @@ export function issueService(db: Db) { .for("update") .then((rows: Array) => rows[0] ?? null); if (!receiptExisting) return null; + if (expectedExecutionPolicy !== undefined && !isDeepStrictEqual( + receiptExisting.executionPolicy ?? null, + expectedExecutionPolicy, + )) { + throw conflict("The task execution settings changed. Try cancelling the monitor again.", { + code: "execution_policy_changed", + }); + } if (changesPrivacy) { const nextProjectId = issueData.projectId !== undefined ? issueData.projectId : receiptExisting.projectId; const [privacyProject] = nextProjectId ? await tx.select().from(projects) diff --git a/ui/src/components/IssueMonitorBanner.test.tsx b/ui/src/components/IssueMonitorBanner.test.tsx index 07598d50a2f..5680dc963af 100644 --- a/ui/src/components/IssueMonitorBanner.test.tsx +++ b/ui/src/components/IssueMonitorBanner.test.tsx @@ -1,5 +1,6 @@ // @vitest-environment jsdom +import { act } from "react"; import { flushSync } from "react-dom"; import { createRoot } from "react-dom/client"; import type { Issue, IssueWorkProduct } from "@paperclipai/shared"; @@ -145,6 +146,35 @@ describe("IssueMonitorBanner / IssueMonitorComposerStrip rendering", () => { } as unknown as Issue; } + it("requires confirmation, keeps the monitor on dismissal, and reports cancellation errors", async () => { + const issue = issueWithMonitor(new Date(NOW.getTime() + 60_000).toISOString()); + const onCancel = vi.fn().mockRejectedValueOnce(new Error("Unable to save monitor")).mockResolvedValueOnce(undefined); + const root = createRoot(container); + flushSync(() => root.render()); + const open = () => flushSync(() => (container.querySelector('[aria-label="Cancel monitor"]') as HTMLButtonElement).click()); + open(); + expect(onCancel).not.toHaveBeenCalled(); + expect(document.querySelector('[role="alertdialog"]')?.textContent).toContain("no longer resume"); + flushSync(() => (Array.from(document.querySelectorAll("button")).find(b => b.textContent === "Keep monitor")!).click()); + expect(onCancel).not.toHaveBeenCalled(); + open(); + await act(async () => { Array.from(document.querySelectorAll("button")).find(b => b.textContent === "Cancel monitor")!.click(); }); + expect(document.querySelector('[role="alert"]')?.textContent).toBe("Unable to save monitor"); + expect(document.querySelector('[role="alertdialog"]')).not.toBeNull(); + await act(async () => { Array.from(document.querySelectorAll("button")).find(b => b.textContent === "Cancel monitor")!.click(); }); + expect(onCancel).toHaveBeenCalledTimes(2); + expect(document.querySelector('[role="alertdialog"]')).toBeNull(); + flushSync(() => root.unmount()); + }); + + it("does not offer monitor cancellation for automatic retries", () => { + const issue = { scheduledRetry: { status: "scheduled_retry", scheduledRetryAt: NOW.toISOString() } } as Issue; + const root = createRoot(container); + flushSync(() => root.render()); + expect(container.querySelector('[aria-label="Cancel monitor"]')).toBeNull(); + flushSync(() => root.unmount()); + }); + it("shows the saved PR and a status check above the composer during a GitHub review wait", () => { const issue = issueWithMonitor(new Date(NOW.getTime() + 6 * 60 * 60_000).toISOString()); issue.executionState!.monitor!.serviceName = "github"; diff --git a/ui/src/components/IssueMonitorBanner.tsx b/ui/src/components/IssueMonitorBanner.tsx index 035eca29b84..78f36b8ca11 100644 --- a/ui/src/components/IssueMonitorBanner.tsx +++ b/ui/src/components/IssueMonitorBanner.tsx @@ -1,8 +1,18 @@ -import { useMemo } from "react"; -import { Clock } from "lucide-react"; +import { useMemo, useState } from "react"; +import { Clock, X } from "lucide-react"; import type { Issue, IssueWorkProduct } from "@paperclipai/shared"; import { Button } from "@/components/ui/button"; +import { + AlertDialog, + AlertDialogTrigger, + AlertDialogContent, + AlertDialogHeader, + AlertDialogTitle, + AlertDialogDescription, + AlertDialogFooter, + AlertDialogCancel, +} from "@/components/ui/alert-dialog"; import { InlineBanner } from "@/components/InlineBanner"; import { IssuePullRequestLinks } from "@/components/IssuePullRequestLinks"; import { getIssuePullRequests, pullRequestNeedsReview } from "@/lib/issue-pull-requests"; @@ -180,12 +190,51 @@ function CheckNowButton({ ); } +function CancelMonitorButton({ onCancel }: { onCancel: () => Promise }) { + const [open, setOpen] = useState(false); + const [pending, setPending] = useState(false); + const [error, setError] = useState(null); + const cancel = async () => { + setPending(true); + setError(null); + try { + await onCancel(); + setOpen(false); + } catch (err) { + setError(err instanceof Error ? err.message : "Unable to cancel the monitor. Try again."); + } finally { + setPending(false); + } + }; + return ( + { if (!pending) { setOpen(next); setError(null); } }}> + + + + + + Cancel monitor? + This removes the scheduled monitor check. The agent will no longer resume from this monitor. You can send a message to continue the task. + + {error ?

{error}

: null} + + Keep monitor + + +
+
+ ); +} + export interface IssueMonitorSurfaceProps { issue: Issue; onCheckNow?: (() => void) | null; checkingNow?: boolean; workProducts?: IssueWorkProduct[] | null; checkError?: string | null; + onCancelMonitor?: (() => Promise) | null; } /** @@ -199,19 +248,23 @@ export function IssueMonitorBanner({ checkingNow = false, workProducts, checkError, + onCancelMonitor, }: IssueMonitorSurfaceProps) { const reviews = getIssuePullRequests(workProducts).filter(pullRequestNeedsReview); const copy = useMonitorSurfaceCopy(issue, reviews.length); if (!copy) return null; + const canCancel = onCancelMonitor && deriveMonitorState(issue).source === "monitor"; + return ( : null} > + {canCancel ? : null}
{copy.pullRequestReview ? : null} {copy.bannerMeta.join(" · ")} diff --git a/ui/src/pages/IssueDetail.test.tsx b/ui/src/pages/IssueDetail.test.tsx index af4a0d6cde1..69d2043dc49 100644 --- a/ui/src/pages/IssueDetail.test.tsx +++ b/ui/src/pages/IssueDetail.test.tsx @@ -594,6 +594,7 @@ vi.mock("../components/Identity", () => ({ })); vi.mock("@/components/ui/button", () => ({ + buttonVariants: () => "", Button: ({ children, disabled, @@ -1422,6 +1423,26 @@ describe("IssueDetail", () => { mockRouteParams.companyPrefix = "PAP"; }); + it("clears only the monitor after confirmation and refreshes the task", async () => { + const nextCheckAt = new Date(Date.now() + 60_000).toISOString(); + const preservedPolicy = { mode: "normal", commentRequired: false, stages: [{ type: "review", approvalsNeeded: 1, participants: [{ type: "user", userId: "reviewer-1" }] }], maxReviewRounds: 4, authorizationPolicy: { assignmentPolicy: { mode: "protected" } } }; + const policy = { ...preservedPolicy, monitor: { nextCheckAt, scheduledBy: "board", notes: "Check deployment" } } as Issue["executionPolicy"]; + const monitored = createIssue({ status: "in_progress", executionPolicy: policy, executionState: { monitor: { status: "scheduled", nextCheckAt, attemptCount: 1 } } as Issue["executionState"] }); + const cleared = createIssue({ status: "in_progress", executionPolicy: preservedPolicy as Issue["executionPolicy"] }); + mockIssuesApi.get.mockResolvedValue(monitored); + mockIssuesApi.update.mockImplementation(async () => { + mockIssuesApi.get.mockResolvedValue(cleared); + return cleared; + }); + await act(async () => root.render()); + await waitForAssertion(() => expect(container.querySelector('[aria-label="Cancel monitor"]')).not.toBeNull()); + await act(async () => (container.querySelector('[aria-label="Cancel monitor"]') as HTMLButtonElement).click()); + expect(mockIssuesApi.update).not.toHaveBeenCalled(); + await act(async () => Array.from(document.querySelectorAll("button")).find(b => b.textContent === "Cancel monitor")!.click()); + await waitForAssertion(() => expect(mockIssuesApi.update).toHaveBeenCalledWith(monitored.id, { expectedExecutionPolicy: policy, executionPolicy: preservedPolicy })); + await waitForAssertion(() => expect(container.querySelector('[aria-label="Cancel monitor"]')).toBeNull()); + }); + it.each([false, true])("keeps monitor errors on the checked task (late response: %s)", async (lateResponse) => { const monitor = { monitor: { status: "scheduled", nextCheckAt: new Date(Date.now() + 60_000).toISOString(), attemptCount: 1, serviceName: "github" } } as Issue["executionState"]; mockIssuesApi.get.mockResolvedValue(createIssue({ status: "in_progress", executionState: monitor })); diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index e42768c867d..7b5dbcd5413 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -4568,6 +4568,28 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS ], ); + const cancelIssueMonitor = useMutation({ + mutationKey: ["cancel-issue-monitor", issueId], + mutationFn: async () => { + const current = await issuesApi.get(issueId!); + const { monitor: _monitor, ...policy } = current.executionPolicy ?? { mode: "normal" as const, commentRequired: true, stages: [] }; + return issuesApi.update(current.id, { + expectedExecutionPolicy: current.executionPolicy ?? null, + executionPolicy: { + ...policy, + mode: policy.mode ?? "normal", + commentRequired: policy.commentRequired ?? true, + stages: policy.stages ?? [], + }, + }); + }, + onSuccess: () => { + invalidateIssueDetail(); + invalidateIssueRunState(); + invalidateIssueCollections(); + }, + }); + const checkIssueMonitorNow = useMutation({ mutationKey: ["check-issue-monitor-now", issueId], mutationFn: () => issuesApi.checkMonitorNow(issueId!), @@ -7487,6 +7509,8 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS : null} cancelIssueMonitor.mutateAsync()} issue={issue} workProducts={workProducts} checkError={checkIssueMonitorNow.error?.message} diff --git a/ui/storybook/stories/monitor-surfaces.stories.tsx b/ui/storybook/stories/monitor-surfaces.stories.tsx index 7fbfc66c988..92bc8b219d1 100644 --- a/ui/storybook/stories/monitor-surfaces.stories.tsx +++ b/ui/storybook/stories/monitor-surfaces.stories.tsx @@ -207,6 +207,7 @@ function MonitorSurfaceStories() {
undefined} onCheckNow={() => undefined} checkingNow={false} />