diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 1c2ebb86dec..dd0378688f5 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -239,6 +239,16 @@ The work is complete and terminal. The work will not continue and is terminal. +When a board user marks an active native task `done` or `cancelled` through +the task update API, Paperclip must confirm the run's cancellation before +clearing its task execution lock. This also applies to providers without +session goals. An unconfirmed stop leaves the task open. Native agents still +report their own completion through `paperclip_finish` or `paperclip_block`, +so normal result and workspace finalization retain their existing ordering. +The cancellation records the board user's identity. Its acknowledged operator +Stop receipt keeps recovery from promoting old queued input or creating a retry +while the task closure commits. + ## 4. Agent-Owned vs User-Owned Execution The execution model differs depending on assignee type. diff --git a/maps/agent-access-map/tasks.md b/maps/agent-access-map/tasks.md index 681376b7a38..7f80a1314c0 100644 --- a/maps/agent-access-map/tasks.md +++ b/maps/agent-access-map/tasks.md @@ -349,6 +349,12 @@ Agent access: `partial` — authorized REST/call_api and CLI status updates; nat Policy: Task mutation/run/assignment/recovery/resolver gates. Blocked requires unresolved blocker/pending input/approval or unblockDescriptor; agent can name only SELF unblock owner. In_review requires actionable path. Native bound completion contract/criterion revision, deliverable evidence and verification must agree; done rejects unfinished work; needs_review names valid different reviewer. Pending input/approval/review/blocker can keep task open. Final state commits after provider turn/workspace finalization; report never overwrites successor/already terminal state. +Board PATCH to Done or Cancelled stops an active native run before releasing +execution ownership, even when it has no session goal. An unconfirmed stop +returns 409 without updating the task. Native self-completion still uses its +semantic finish tools. Route regression: +[issue-update-comment-wakeup-routes.test.ts](../../server/src/__tests__/issue-update-comment-wakeup-routes.test.ts). + Evidence: `source-only` — reviewed at `3bbc2104b65caf9f7763938a01260a5673e12f7a`; [issues.ts](../../server/src/routes/issues.ts), [paperclip-runner-tool-authority.ts](../../server/src/services/native-runtime/paperclip-runner-tool-authority.ts), [native-completion-feedback.ts](../../server/src/services/native-runtime/native-completion-feedback.ts), [native-completion-feedback.test.ts](../../server/src/services/native-runtime/native-completion-feedback.test.ts), [native-deliverable-feedback.test.ts](../../server/src/services/native-runtime/native-deliverable-feedback.test.ts), [paperclip-runner-tool-authority.test.ts](../../server/src/services/native-runtime/paperclip-runner-tool-authority.test.ts), [issue-stalled-review-decision-routes.test.ts](../../server/src/__tests__/issue-stalled-review-decision-routes.test.ts). No live execution claimed. Verification: `not-run` — Actual native accepted report→provider stop→workspace finalization→persisted status/output; stale/failed report correction without repeating work. Test blocked/review/done, pending question/blocker, invalid/self/foreign reviewer, missing output and duplicate report. diff --git a/maps/feature-map/tasks.md b/maps/feature-map/tasks.md index 28dfc336689..a9afa296295 100644 --- a/maps/feature-map/tasks.md +++ b/maps/feature-map/tasks.md @@ -20,6 +20,9 @@ Agent access: [companion](../agent-access-map/tasks.md). - `discussion`: `tasks/discussion` — persist comments, references, and files on the task. - `completion`: `tasks/completion` — distinguish backlog, active work, review, blocked, done, and cancelled outcomes. + A board user closing an active native task must stop its run before the task + releases execution ownership, including providers without session goals. + Verify Done and Cancelled, an unconfirmed stop, and ordinary agent completion. - `privacy`: `tasks/privacy` — create private tasks, share or revoke access, and inspect parent/project inheritance before changing visibility. The [Product task-properties journey](../../tests/runner-e2e/PRODUCT-JOURNEYS.md) diff --git a/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts b/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts index 81108a04abf..5ff825d9c02 100644 --- a/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts +++ b/server/src/__tests__/issue-update-comment-wakeup-routes.test.ts @@ -1,5 +1,6 @@ import express from "express"; import request from "supertest"; +import { isAcknowledgedNativeStop } from "../services/acknowledged-native-stop.js"; import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; vi.mock("../services/activity-log.js", () => ({ @@ -335,6 +336,93 @@ describe("issue update comment wakeups", () => { }, ); + it.each(["done", "cancelled"])("stops a goal-less native run before a board marks its task %s", async (status) => { + const existing = makeIssue({ status: "in_progress", assigneeAgentId: ASSIGNEE_AGENT_ID, + assigneeUserId: null, executionRunId: SOURCE_RUN_ID }); + mockIssueService.getById.mockResolvedValue(existing); + mockHeartbeatService.getRun.mockResolvedValue({ id: SOURCE_RUN_ID, companyId: existing.companyId, + agentId: ASSIGNEE_AGENT_ID, runtimeMode: "native", status: "running", + nativeIssueId: existing.id, contextSnapshot: { issueId: existing.id } }); + const effects: string[] = []; + mockHeartbeatService.cancelRun.mockImplementation(async () => { + effects.push("stop"); + return { id: SOURCE_RUN_ID, companyId: existing.companyId, status: "cancelled" }; + }); + mockIssueService.update.mockImplementation(async () => { + effects.push("update"); + return { ...existing, status, executionRunId: null }; + }); + + const res = await request(await createApp()).patch(`/api/issues/${existing.id}`).send({ status }); + + expect(res.status).toBe(200); + expect(effects).toEqual(["stop", "update"]); + expect(mockHeartbeatService.cancelRun).toHaveBeenCalledOnce(); + expect(mockHeartbeatService.cancelRun).toHaveBeenCalledWith(SOURCE_RUN_ID, + "Cancelled before issue terminalization", expect.objectContaining({ + errorCode: "issue_terminalized", + resultJson: expect.objectContaining({ terminalizationStopConfirmed: true }), + eventPayload: { issueId: existing.id, status }, + })); + const stopMetadata = mockHeartbeatService.cancelRun.mock.calls[0]![2].resultJson; + // Exercise the recovery consumer with the route's actual producer fields. + // A Stop receipt alone must not authorize this intentional-stop exception. + expect(isAcknowledgedNativeStop({ id: SOURCE_RUN_ID, companyId: existing.companyId, + status: "cancelled", nativeIssueId: existing.id, resultJson: { ...stopMetadata, + nativeCancellation: { schema: "paperclip.native-cancellation.v1", runId: SOURCE_RUN_ID, + companyId: existing.companyId, issueId: existing.id, scope: "run", + reasonCode: "cancellation_run_only", dispatchState: "acknowledged", dispatched: true, + intentAuditId: "stop-intent", acknowledgementAuditId: "stop-acknowledgement" }, + } })).toBe(true); + expect(res.body).toMatchObject({ status, executionRunId: null }); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); + }); + + it("keeps the task open when a goal-less native run cannot confirm its stop", async () => { + const existing = makeIssue({ status: "in_progress", assigneeAgentId: ASSIGNEE_AGENT_ID, + assigneeUserId: null, executionRunId: SOURCE_RUN_ID }); + mockIssueService.getById.mockResolvedValue(existing); + mockHeartbeatService.getRun.mockResolvedValue({ id: SOURCE_RUN_ID, companyId: existing.companyId, + agentId: ASSIGNEE_AGENT_ID, runtimeMode: "native", status: "running", + nativeIssueId: existing.id, contextSnapshot: { issueId: existing.id } }); + mockHeartbeatService.cancelRun.mockResolvedValue(null); + mockIssueService.update.mockResolvedValue({ ...existing, status: "done", executionRunId: null }); + + const res = await request(await createApp()).patch(`/api/issues/${existing.id}`).send({ status: "done" }); + + expect(res.status).toBe(409); + expect(mockIssueService.update).not.toHaveBeenCalled(); + expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); + }); + + it("does not inspect agent runs when closing a task without an execution owner", async () => { + const existing = makeIssue({ status: "in_progress", assigneeAgentId: ASSIGNEE_AGENT_ID, + assigneeUserId: null, executionRunId: null }); + mockIssueService.getById.mockResolvedValue(existing); + mockIssueService.update.mockResolvedValue({ ...existing, status: "done" }); + + const res = await request(await createApp()).patch(`/api/issues/${existing.id}`).send({ status: "done" }); + + expect(res.status).toBe(200); + expect(mockHeartbeatService.getRun).not.toHaveBeenCalled(); + expect(mockHeartbeatService.getActiveRunForAgent).not.toHaveBeenCalled(); + expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled(); + }); + + it("preserves legacy completion without a native goal", async () => { + const existing = makeIssue({ status: "in_progress", assigneeAgentId: ASSIGNEE_AGENT_ID, + assigneeUserId: null, executionRunId: SOURCE_RUN_ID }); + mockIssueService.getById.mockResolvedValue(existing); + mockHeartbeatService.getRun.mockResolvedValue({ id: SOURCE_RUN_ID, companyId: existing.companyId, + agentId: ASSIGNEE_AGENT_ID, runtimeMode: "legacy", status: "running", contextSnapshot: { issueId: existing.id } }); + mockIssueService.update.mockResolvedValue({ ...existing, status: "done", executionRunId: null }); + + const res = await request(await createApp()).patch(`/api/issues/${existing.id}`).send({ status: "done" }); + + expect(res.status).toBe(200); + expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled(); + }); + it.each([false, true])("still wakes explicitly reopened work (reassigned: %s)", async (reassigned) => { const existing = makeIssue({ status: "done", assigneeAgentId: PREVIOUS_AGENT_ID, assigneeUserId: null }); const agentId = reassigned ? ASSIGNEE_AGENT_ID : PREVIOUS_AGENT_ID; diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 1326833bd9a..3d6af2376f9 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -14708,16 +14708,28 @@ export function issueRoutes( issueId: existing.id, agentId: existing.assigneeAgentId, }); - const runToStopForTerminalization = goalStopAction + const runToStopForTerminalization = goalStopAction || + (req.actor.type === "board" && existing.executionRunId) ? await resolveActiveIssueRun(existing) : null; - if (goalStopAction && runToStopForTerminalization) { + // Closing a native task revokes its tool binding. Retire the board's + // active executor before releasing that binding, even without a goal. + const mustStopNativeRun = req.actor.type === "board" && + runToStopForTerminalization?.runtimeMode === "native"; + if (runToStopForTerminalization && (goalStopAction || mustStopNativeRun)) { const cancelled = await heartbeat.cancelRun( runToStopForTerminalization.id, "Cancelled before issue terminalization", { errorCode: "issue_terminalized", - resultJson: { terminalizationStopConfirmed: true, issueMutationStopId }, + resultJson: { + terminalizationStopConfirmed: true, + issueMutationStopId, + ...(actor.actorType === "user" ? { + cancelledByActorType: "user", + cancelledByUserId: actor.actorId, + } : {}), + }, eventMessage: "run cancelled before issue terminalization", eventPayload: { issueId: existing.id,