From 09241442ea1d700b7e9dcb8cb1b627c29d81584e Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 9 Nov 2026 17:11:42 -0600 Subject: [PATCH] fix(server): let a native reviewer approve its review stage (#20373) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Execution policies can require an independent review stage before a task is done > - A native (Runner) agent that is the active reviewer had no way to record its decision > - `PATCH status: "done"` is refused for a native run that owns the task (#18411), and `paperclip_finish` accepts the report but leaves the stage pending > - The reviewer then searches the API until a deadline, and the task stays in review with nobody able to decide it > - This pull request records the final-stage approval on `PATCH` without releasing the run's lock, and lets the run's own finish report set the final status > - The benefit is that an agent reviewer can actually approve its review stage, in one short turn ## Linked Issues or Issue Description Refs #18411 (added the native status guard). Related: #20264 (explicit stage-decision operation, still open; this change is narrower and compatible with it). **What happened?** A native reviewer agent was woken as the active `review` participant. It read the deliverable and wrote its review in about 100 seconds. It then called `paperclip_finish` with `done`. The tool answered "Completion report accepted; the task's configured review stage is still pending". It then tried `PATCH /api/issues/{id}` with `status: "done"` and a comment. The route refused it with `native_run_status_reported_by_finish`. The agent searched the API for 2 more minutes and found no other path. The stage stayed pending. When the run ends with `done`, the status arbiter sees the pending stage and keeps the task in review (`governed_gate_pending`), so the stage can never resolve. **Expected behavior** The active review/approval participant can approve its stage from a native run. The task reaches `done` after the approval. The implementer still cannot approve. A run that is not the active participant still cannot change the task status. **Steps to reproduce** 1. Create a task with an execution policy that has one `review` stage with a native agent participant. 2. Move the task to `in_review` so the stage is pending and the reviewer agent wakes. 3. In the reviewer run, call `PATCH /api/issues/{id}` with `{ "status": "done", "comment": "Approved: ..." }`. 4. Observe `409 native_run_status_reported_by_finish`. Call `paperclip_finish` with `done` and observe that the stage stays pending. ## What Changed - `PATCH /api/issues/:id`: when the requester is the native run that holds the task lock and is the active participant, a `status: "done"` request on the final stage now records the approval decision (`executionState` completed, decision row, activity). The route removes `status` from the write, so the task stays `in_review` and the run keeps its execution lock. The run's `paperclip_finish` report then sets `done` through the normal status arbiter. - An intermediate-stage approval (which would hand the task to the next participant) and every other status change from such a run keep the existing `native_run_status_reported_by_finish` refusal. - `paperclip_finish` from the active participant (`done` or `needs_review`) while its stage is still pending is now rejected with the exact decision path, instead of being accepted with "stage still pending". - The native finalizer now treats the final-stage approval that the run itself recorded (read from the `issue_execution_decisions` row, bound to this run and agent) as the completion authority for its clean `done` report. Before this, a reviewer that cited free-text evidence had its report graded as incomplete, and the task was reopened as `in_progress` with a new wake for the reviewer. Failed verification, blocking remaining work, attention requests, other dispositions, and a still-pending stage keep the old outcome. - The reviewer/approver wake text names the decision path (`PATCH` with `status` and a required decision comment) and says a completion report does not record the decision. A Runner reviewer that needs changes is told to report them with `paperclip_block`, because a changes request from the run that owns the task is a reassignment that stops that run. - The locked review-policy recheck treats the deferred approval as a verdict, so a policy changed to `human_only` before the locked write still refuses it. ## Verification - `pnpm exec vitest run server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts server/src/__tests__/issue-execution-policy-routes.test.ts server/src/__tests__/issue-comment-reopen-routes.test.ts server/src/__tests__/issue-execution-policy.test.ts packages/adapter-utils/src/server-utils.test.ts`: 5 files, 595 tests pass. - `pnpm exec vitest run server/src/services/native-runtime/native-completion-feedback.test.ts`: 29 tests pass (embedded PostgreSQL), including the decision-row binding for `executionStageApprovedByRun`. - `pnpm exec vitest run server/src/services/native-runtime/status-arbiter.test.ts server/src/services/native-runtime/native-run-finalizer-telemetry.test.ts server/src/services/native-runtime/native-stale-owner-completion.test.ts`: 72 tests pass. - New route tests fail without the route change (409 instead of 200/422) and pass with it. The new feedback test fails without the feedback change ("Completion report accepted" instead of a rejection). - `npx tsc --noEmit -p server` and `-p packages/adapter-utils`: pass. - A route test changes the review policy to `human_only` between the first check and the locked write; the deferred approval is refused (403). - Live full-stack chat eval `task-coordination.independent-review` (Runner, OpenCode) with this change: passed in GitHub Actions run 42705964209 (eval branch with the first two commits cherry-picked). The reviewer run recorded its approval 24 s after its review brief was released, reported `done` 14 s later, and the finalizer set the task to `done` with reason `execution_stage_approved`; total reviewer run 73 s. The earlier run 42702224713 exposed the finalizer evidence issue that the second commit fixes. ## Risks - Low to medium. The change is limited to one request shape: a native run, holding the lock, that is the active participant of a pending final stage, sending `status: "done"`. All other requests keep the old guard. - Between the approval and the finish report, the task shows `in_review` with a completed `executionState`. If the run fails before it reports, the existing native recovery owns the task, as for any other run. - Previous-release story: no persisted shape changes. A reviewer run that started before the deploy could not have recorded a deferred approval (the old route refused it), so the new finalizer input is false for it and the old outcome applies. - Intermediate-stage approvals and changes requests from native runs are still refused by the route. A Runner reviewer reports required changes with `paperclip_block`, which keeps the stage pending for a person. A self-service changes request needs the deferred handoff semantics in #20264. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- packages/adapter-utils/src/server-utils.ts | 3 + ...ue-agent-mutation-ownership-routes.test.ts | 86 +++++++++++++++++++ server/src/routes/issues.ts | 42 +++++++-- .../native-completion-feedback.test.ts | 34 +++++++- .../native-completion-feedback.ts | 11 +++ .../native-runtime/native-run-finalizer.ts | 25 ++++++ .../native-runtime/status-arbiter.test.ts | 19 ++++ .../services/native-runtime/status-arbiter.ts | 22 ++++- 8 files changed, 232 insertions(+), 10 deletions(-) diff --git a/packages/adapter-utils/src/server-utils.ts b/packages/adapter-utils/src/server-utils.ts index 4fbd4ae8938..0e0da0d06c5 100644 --- a/packages/adapter-utils/src/server-utils.ts +++ b/packages/adapter-utils/src/server-utils.ts @@ -2983,6 +2983,9 @@ function renderPaperclipWakePromptBody( `You are waking as the active ${executionStage.wakeRole} for this issue.`, "Do not execute the task itself or continue executor work.", "Review the issue and choose one of the allowed actions above.", + "Record your decision on this issue with PATCH /api/issues/{issueId} and a required decision comment: approve with {\"status\":\"done\",\"comment\":\"...\"}, or request changes with {\"status\":\"in_progress\",\"comment\":\"...\"}.", + "A Paperclip Runner run cannot hand the task back itself; to request changes from one, report the required changes as a blocker with paperclip_block.", + "A run completion report does not record the decision; record it before you report.", "If you request changes, the workflow routes back to the stored return assignee.", "", ); diff --git a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts index 804847e4429..3aa93293e45 100644 --- a/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts +++ b/server/src/__tests__/issue-agent-mutation-ownership-routes.test.ts @@ -3135,6 +3135,92 @@ describe("agent issue mutation checkout ownership", () => { expect(mockHeartbeatService.cancelRun).not.toHaveBeenCalled(); }); + describe("as the active review participant", () => { + const reviewStage = (id: string) => ({ id, type: "review", approvalsNeeded: 1, + participants: [{ id: `${id.slice(0, -1)}f`, type: "agent", agentId: ownerAgentId, userId: null }] }); + const reviewIssue = (stageIds: string[]) => makeIssue({ + status: "in_review", executionRunId: ownerRunId, checkoutRunId: ownerRunId, + executionPolicy: { mode: "normal", commentRequired: true, stages: stageIds.map(reviewStage) }, + executionState: { + status: "pending", currentStageId: stageIds[0], currentStageIndex: 0, currentStageType: "review", + currentParticipant: { type: "agent", agentId: ownerAgentId }, + returnAssignee: { type: "agent", agentId: peerAgentId }, + completedStageIds: [], lastDecisionId: null, lastDecisionOutcome: null, + }, + }); + const finalStage = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", laterStage = "bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb"; + + it("records its final-stage approval and leaves the final status to its finish report", async () => { + const issue = reviewIssue([finalStage]); + mockIssueService.getById.mockResolvedValue(issue); + mockIssueService.update.mockImplementation(async (_id: string, patch: Record) => ({ ...issue, ...patch })); + const db = createRunContextDb(); + const inserted: Array<{ table: string; values: Record }> = []; + db.insert = vi.fn((table: Parameters[0]) => ({ values: vi.fn(async (values: Record) => { + inserted.push({ table: getTableName(table), values }); + }) })) as typeof db.insert; + + const res = await request(createApp(runActor(), db)).patch(`/api/issues/${issueId}`) + .send({ status: "done", comment: "Approved: verified the heading and signup action." }); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + const patch = mockIssueService.update.mock.calls[0]?.[1] as Record; + // No status in the write: the service would otherwise release this + // run's execution lock before paperclip_finish reports the outcome. + expect(patch).not.toHaveProperty("status"); + expect(patch.executionState).toMatchObject({ status: "completed", lastDecisionOutcome: "approved", lastDecisionId: expect.any(String) }); + // The finalizer's completion authority reads exactly this row (executionStageApprovedByRun). + const decision = inserted.find(row => row.table === "issue_execution_decisions")?.values; + expect(decision).toMatchObject({ id: patch.executionState.lastDecisionId, companyId, issueId, stageId: finalStage, outcome: "approved", + actorAgentId: ownerAgentId, createdByRunId: ownerRunId, body: "Approved: verified the heading and signup action." }); + }); + + it("rechecks the review policy under the issue lock for the deferred approval", async () => { + const issue = reviewIssue([finalStage]); + mockIssueService.getById.mockResolvedValue(issue); + // The policy becomes human-only after the first check, before the locked write. + mockIssueService.getByIdForUpdate.mockResolvedValue({ ...issue, reviewPolicy: "human_only" }); + + const res = await request(createApp(runActor())).patch(`/api/issues/${issueId}`) + .send({ status: "done", comment: "Approved: verified." }); + + expect(res.status, JSON.stringify(res.body)).toBe(403); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + + it("still requires the decision comment", async () => { + mockIssueService.getById.mockResolvedValue(reviewIssue([finalStage])); + + const res = await request(createApp(runActor())).patch(`/api/issues/${issueId}`).send({ status: "done" }); + + expect(res.status, JSON.stringify(res.body)).toBe(422); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + + it("keeps refusing an intermediate approval that would hand the task to the next participant", async () => { + mockIssueService.getById.mockResolvedValue(reviewIssue([finalStage, laterStage])); + + const res = await request(createApp(runActor())).patch(`/api/issues/${issueId}`) + .send({ status: "done", comment: "Approved." }); + + expect(res.status, JSON.stringify(res.body)).toBe(409); + expect(res.body.details).toMatchObject({ code: "native_run_status_reported_by_finish", requestedStatus: "done" }); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + + it("keeps refusing the approval when this run is not the active participant", async () => { + const issue = reviewIssue([finalStage]); + mockIssueService.getById.mockResolvedValue({ ...issue, + executionState: { ...(issue.executionState as unknown as Record), currentParticipant: { type: "agent", agentId: peerAgentId } } }); + + const res = await request(createApp(runActor())).patch(`/api/issues/${issueId}`) + .send({ status: "done", comment: "Approved." }); + + expect(res.status, JSON.stringify(res.body)).toBe(409); + expect(mockIssueService.update).not.toHaveBeenCalled(); + }); + }); + it("still lets the run edit other fields of its task", async () => { const res = await request(createApp(runActor())).patch(`/api/issues/${issueId}`).send({ title: "Renamed by its run" }); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index d377db61f49..88f2e545dc3 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -14540,12 +14540,21 @@ export function issueRoutes( * Conversation tasks are excluded: a conversation turn settles from its final * reply (nativeConversationReplyResult), not from a runner tool, so a reply * that moves the conversation to review still settles without the lock. + * + * The one status-shaped request a run must be able to send is the active + * review/approval participant's approval (`status: "done"` with its decision + * comment): no runner tool records a stage decision, so refusing it leaves + * the stage pending forever. With `allowStageApproval` the guard returns + * "defer_final_status" for that request instead of refusing it. The caller + * records the decision and keeps the task (and the run's lock) in review; + * the run's own paperclip_finish report then sets the final status. */ async function assertNativeRunKeepsOwnExecutionLock( req: Request, - issue: { companyId: string; status: string; parentId: string | null; assigneeAgentId: string | null; assigneeUserId: string | null; checkoutRunId: string | null; executionRunId: string | null; conversationAgentId?: string | null; conversationUserId?: string | null }, + issue: { companyId: string; status: string; parentId: string | null; assigneeAgentId: string | null; assigneeUserId: string | null; checkoutRunId: string | null; executionRunId: string | null; conversationAgentId?: string | null; conversationUserId?: string | null; executionState?: unknown }, requestedStatus: unknown = req.body.status, - ) { + options: { allowStageApproval?: boolean } = {}, + ): Promise<"defer_final_status" | undefined> { const runId = req.actor.type === "agent" ? req.actor.runId : null; if (!runId || !req.actor.agentId || req.actor.agentId !== issue.assigneeAgentId) return; if (runId !== issue.executionRunId && runId !== issue.checkoutRunId) return; @@ -14558,6 +14567,10 @@ export function issueRoutes( const run = await heartbeat.getRun(runId); if (!run || run.runtimeMode !== "native" || run.status !== "running" || run.agentId !== req.actor.agentId || run.companyId !== issue.companyId) return; + if (options.allowStageApproval && requestedStatus === "done" && issue.status === "in_review" && + activeExecutionParticipantAgentId(issue) === req.actor.agentId) { + return "defer_final_status"; + } // A blocker that the dependency rules reject anyway keeps its specific // error; otherwise the run reports the block with paperclip_block. if (Array.isArray(req.body.blockedByIssueIds) && req.body.blockedByIssueIds.length > 0) { @@ -14566,7 +14579,11 @@ export function issueRoutes( blockedByIssueIds: req.body.blockedByIssueIds, parentIssueId: issue.parentId, }); } - throw conflict( + throw nativeRunStatusReportedByFinish(requestedStatus); + } + + function nativeRunStatusReportedByFinish(requestedStatus: unknown) { + return conflict( "This run owns the task's execution, so it cannot change the task status directly; doing so would end its access to the task before it reports. " + "Report the outcome with paperclip_finish instead: reportedWorkDisposition \"needs_review\" for review, \"done\" when complete, or \"yielded\" with continuation.kind \"monitor\" to wait on a scheduled monitor. " + "Use paperclip_block for a blocker. Paperclip sets the task status from that report.", @@ -14682,7 +14699,7 @@ export function issueRoutes( ); if (!existing) return; if (!(await assertIssueReadAllowed(req, res, existing))) return; - await assertNativeRunKeepsOwnExecutionLock(req, existing); + const nativeStageApproval = await assertNativeRunKeepsOwnExecutionLock(req, existing, req.body.status, { allowStageApproval: true }); if (req.body.expectedStatusVersion !== undefined && ( existing.statusVersion !== req.body.expectedStatusVersion || existing.status !== "backlog" )) { @@ -15233,6 +15250,18 @@ export function issueRoutes( throw unprocessable("Snapshot-guarded policy updates cannot change task status or ownership"); } Object.assign(updateFields, transition.patch); + let deferredNativeStageApproval = false; + if (nativeStageApproval === "defer_final_status") { + // Only the final stage's approval is deferred. Anything else this + // request would do (an intermediate approval that hands the task to the + // next participant, or no decision at all) keeps the ordinary refusal. + const finalApproval = transition.decision?.outcome === "approved" && updateFields.status === "done" && + (transition.patch.assigneeAgentId === undefined || transition.patch.assigneeAgentId === existing.assigneeAgentId) && + transition.patch.assigneeUserId === undefined; + if (!finalApproval) throw nativeRunStatusReportedByFinish(req.body.status); + delete updateFields.status; + deferredNativeStageApproval = true; + } const nextStatus = updateFields.status ?? existing.status; if (updateFields.unblockDescriptor && nextStatus !== "blocked") { @@ -15659,10 +15688,13 @@ export function issueRoutes( const lockedPolicyChangeRequested = req.body.reviewPolicy !== undefined && req.body.reviewPolicy !== lockedExisting.reviewPolicy; + // A deferred native approval is still a review verdict even though + // its status write is left to the run's finish report. const lockedReviewVerdictRequested = lockedExisting.status === "in_review" && (updateFields.status === "done" || - updateFields.status === "cancelled"); + updateFields.status === "cancelled" || + deferredNativeStageApproval); if ( (lockedReviewVerdictRequested || lockedPolicyChangeRequested) && lockedExisting.reviewPolicy != null && diff --git a/server/src/services/native-runtime/native-completion-feedback.test.ts b/server/src/services/native-runtime/native-completion-feedback.test.ts index ad9ba109e86..e5b3132393c 100644 --- a/server/src/services/native-runtime/native-completion-feedback.test.ts +++ b/server/src/services/native-runtime/native-completion-feedback.test.ts @@ -1,11 +1,12 @@ import { createHash, randomUUID } from "node:crypto"; import { eq } from "drizzle-orm"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; -import { agents, companies, createDb, documentRevisions, executionWorkspaces, executionWorkspaceRepositories, heartbeatRuns, issues, issueThreadInteractions, issueTreeHolds, issueDocuments } from "@paperclipai/db"; +import { agents, companies, createDb, documentRevisions, executionWorkspaces, executionWorkspaceRepositories, heartbeatRuns, issueExecutionDecisions, issues, issueThreadInteractions, issueTreeHolds, issueDocuments } from "@paperclipai/db"; import { startRecoveryPostgres } from "../../__tests__/helpers/recovery-postgres.js"; import type { PrpStructuredRunResult } from "../../vendor/paperclip-runner/index.js"; import { documentService } from "../documents.js"; import { nativeCompletionFeedback } from "./native-completion-feedback.js"; +import { executionStageApprovedByRun } from "./native-run-finalizer.js"; import { PaperclipRunnerToolAuthority } from "./paperclip-runner-tool-authority.js"; const done: PrpStructuredRunResult = { @@ -43,6 +44,37 @@ describe("native final-response feedback", () => { } }) as { downloadPath: string }; return { ...done, evidence: [{ ref: `${receipt.downloadPath}#download` }] }; } + it("sends the active review participant to record its stage decision before reporting", async () => { + const value = await fixture(), stageId = randomUUID(), otherAgentId = randomUUID(); + await db.insert(agents).values({ id: otherAgentId, companyId: value.companyId, name: "Implementer", adapterType: "paperclip_runner", status: "active" }); + const pending = { status: "pending", currentStageId: stageId, currentStageIndex: 0, currentStageType: "review", + currentParticipant: { type: "agent", agentId: value.agentId }, returnAssignee: { type: "agent", agentId: otherAgentId }, + completedStageIds: [], lastDecisionId: null, lastDecisionOutcome: null }; + await db.update(issues).set({ status: "in_review", executionState: pending }).where(eq(issues.id, value.issueId)); + // The retained failure: the reviewer's finish was accepted while its own + // stage stayed pending, and nothing it could call afterwards decided it. + await expect(nativeCompletionFeedback(db, value.runId, done)).rejects.toThrow(/active review participant[\s\S]*PATCH \/api\/issues\/\{id\}/); + await expect(nativeCompletionFeedback(db, value.runId, { ...done, reportedWorkDisposition: "needs_review" })).rejects.toThrow("does not record your review decision"); + // Someone else's pending stage is still explained, not attributed to this run. + await db.update(issues).set({ executionState: { ...pending, currentParticipant: { type: "agent", agentId: otherAgentId } } }).where(eq(issues.id, value.issueId)); + await expect(nativeCompletionFeedback(db, value.runId, done)).resolves.toContain("configured review stage is still pending"); + // Once the participant's approval is recorded, its report is accepted. + await db.update(issues).set({ executionState: { ...pending, status: "completed", completedStageIds: [stageId], lastDecisionId: randomUUID(), lastDecisionOutcome: "approved" } }).where(eq(issues.id, value.issueId)); + await expect(nativeCompletionFeedback(db, value.runId, done)).resolves.not.toContain("review stage"); + }); + it("binds final-stage approval authority to the decision row this run wrote", async () => { + const value = await fixture(), stageId = randomUUID(), decisionId = randomUUID(); + const run = { id: value.runId, companyId: value.companyId, agentId: value.agentId }; + const completed = { status: "completed", currentStageId: null, completedStageIds: [stageId], lastDecisionId: decisionId, lastDecisionOutcome: "approved" }; + await db.insert(issueExecutionDecisions).values({ id: decisionId, companyId: value.companyId, issueId: value.issueId, stageId, stageType: "review", + actorAgentId: value.agentId, outcome: "approved", body: "Approved: verified.", createdByRunId: value.runId }); + expect(await executionStageApprovedByRun(db, run, { id: value.issueId, executionState: completed })).toBe(true); + // Another run's decision, a pending stage, or a changes request grants nothing. + expect(await executionStageApprovedByRun(db, { ...run, id: randomUUID() }, { id: value.issueId, executionState: completed })).toBe(false); + expect(await executionStageApprovedByRun(db, run, { id: value.issueId, executionState: { ...completed, status: "pending" } })).toBe(false); + expect(await executionStageApprovedByRun(db, run, { id: value.issueId, executionState: { ...completed, lastDecisionOutcome: "changes_requested" } })).toBe(false); + expect(await executionStageApprovedByRun(db, run, { id: value.issueId, executionState: { ...completed, lastDecisionId: randomUUID() } })).toBe(false); + }); it("rejects a duplicate approval report for the exact pending tool-action card", async () => { const value = await fixture(), interactionId = randomUUID(); await db.insert(issueThreadInteractions).values({ id: interactionId, companyId: value.companyId, issueId: value.issueId, diff --git a/server/src/services/native-runtime/native-completion-feedback.ts b/server/src/services/native-runtime/native-completion-feedback.ts index 941edb16daf..4b9f4c2c87e 100644 --- a/server/src/services/native-runtime/native-completion-feedback.ts +++ b/server/src/services/native-runtime/native-completion-feedback.ts @@ -151,6 +151,17 @@ export async function nativeCompletionFeedback( "The done report includes unfinished work, failed verification, or an outstanding decision. Finish the work or report the concrete blocker/reviewer request. No human completion approval was created.", ); } + const stage = record(issue.executionState), participant = record(stage.currentParticipant); + if (stage.status === "pending" && participant.type === "agent" && participant.agentId === run.agentId && + issue.executionRunId === run.id && ["done", "needs_review"].includes(result.reportedWorkDisposition)) { + // The active review/approval participant decides the stage on the task + // itself. A finish report cannot record that decision, so accepting it + // would leave the stage pending with nobody left to decide it. + const stageType = typeof stage.currentStageType === "string" ? stage.currentStageType : "review"; + throw new Error(`You are the active ${stageType} participant on this task. A finish report does not record your ${stageType} decision, so this report was not accepted. ` + + `To approve, record the decision first with paperclip_call_api operationId "PATCH /api/issues/{id}" for this task and {"status":"done","comment":""}, then report this run with paperclip_finish. ` + + "To request changes, or if you cannot decide, report the required changes or the concrete blocker with paperclip_block."); + } if (["done", "cancelled"].includes(issue.status)) { return `Report accepted; task is already ${issue.status}. This report will not reopen it.`; } diff --git a/server/src/services/native-runtime/native-run-finalizer.ts b/server/src/services/native-runtime/native-run-finalizer.ts index 52d3aa178fc..7937e7dceb5 100644 --- a/server/src/services/native-runtime/native-run-finalizer.ts +++ b/server/src/services/native-runtime/native-run-finalizer.ts @@ -45,6 +45,7 @@ import { heartbeatRuns, heartbeatRunEvents, issueComments, + issueExecutionDecisions, issueThreadInteractions, issues, nativeRunFinalizations, @@ -162,6 +163,29 @@ export function resolveNativeFinalizerStatus( export { pendingNativeGovernance } from "./native-governance.js"; +/** + * Whether this run recorded the approval that completed the issue's final + * execution-policy stage. Read from the saved decision row, never from the + * model's report. + */ +export async function executionStageApprovedByRun( + db: Db, + run: { id: string; companyId: string; agentId: string }, + issue: { id: string; executionState: unknown }, +): Promise { + const state = record(issue.executionState); + if (state.status !== "completed" || state.lastDecisionOutcome !== "approved" || typeof state.lastDecisionId !== "string") return false; + const [decision] = await db.select({ id: issueExecutionDecisions.id }).from(issueExecutionDecisions).where(and( + eq(issueExecutionDecisions.id, state.lastDecisionId), + eq(issueExecutionDecisions.companyId, run.companyId), + eq(issueExecutionDecisions.issueId, issue.id), + eq(issueExecutionDecisions.outcome, "approved"), + eq(issueExecutionDecisions.actorAgentId, run.agentId), + eq(issueExecutionDecisions.createdByRunId, run.id), + )).limit(1); + return Boolean(decision); +} + async function acceptedInteractionFromRun(input: { db: Db; companyId: string; @@ -1548,6 +1572,7 @@ export async function finalizeNativeRun(input: { const qualityReview = await readRequiredQualityReview(input.db, authoritativeIssue); const statusInput: Parameters[0] = { requiredQualityReviewSatisfied: qualityReview.satisfied, + executionStageApprovedByRun: await executionStageApprovedByRun(input.db, run, authoritativeIssue), caseHandoffAuthorized: Boolean(await assertNativeCaseHandoffResult(input.db, run.id, authoritativeIssue.id, result)), monitorWaitDispatched: monitorDelivery !== null, monitorWaitPreserved: preservedMonitorWait !== null, diff --git a/server/src/services/native-runtime/status-arbiter.test.ts b/server/src/services/native-runtime/status-arbiter.test.ts index a90d98eadc3..0eb079e508d 100644 --- a/server/src/services/native-runtime/status-arbiter.test.ts +++ b/server/src/services/native-runtime/status-arbiter.test.ts @@ -44,6 +44,25 @@ function arbitrate( } describe("native status authority", () => { + it("completes a reviewer's done report from its own recorded final-stage approval", () => { + // Retained live failure: the reviewer approved its stage, then cited free-text + // evidence; the report was graded as incomplete and the task reopened. + const freeText = assessment({ objectiveSatisfied: false, allCriteriaSatisfied: false, verificationPassed: false }); + expect(arbitrate({ assessment: freeText, priorIssueStatus: "in_review" })) + .toMatchObject({ toStatus: "in_progress", reasonCode: "completion_evidence_incomplete" }); + expect(arbitrate({ assessment: freeText, priorIssueStatus: "in_review", executionStageApprovedByRun: true })) + .toMatchObject({ toStatus: "done", reasonCode: "execution_stage_approved" }); + // The decision authorizes completion only for a clean done report. + expect(arbitrate({ assessment: { ...freeText, hasBlockingRemainingWork: true }, priorIssueStatus: "in_review", executionStageApprovedByRun: true })) + .not.toMatchObject({ toStatus: "done" }); + expect(arbitrate({ assessment: { ...freeText, hasFailedVerification: true }, priorIssueStatus: "in_review", executionStageApprovedByRun: true })) + .not.toMatchObject({ toStatus: "done" }); + expect(arbitrate({ assessment: { ...freeText, reportedDisposition: "needs_review" }, priorIssueStatus: "in_review", executionStageApprovedByRun: true })) + .not.toMatchObject({ toStatus: "done" }); + // A still-pending stage keeps governing the task. + expect(arbitrate({ assessment: freeText, priorIssueStatus: "in_review", executionStageApprovedByRun: true, governanceGate: { kind: "execution_stage", id: "run" } })) + .toMatchObject({ toStatus: "in_review", reasonCode: "governed_gate_pending" }); + }); it("cannot complete an explicitly required quality review from claims or file evidence", () => { expect(arbitrate({ completionClaimPolicyAccepted: true, requiredQualityReviewSatisfied: false })) .toMatchObject({ toStatus: "in_progress", reasonCode: "required_quality_review_incomplete" }); diff --git a/server/src/services/native-runtime/status-arbiter.ts b/server/src/services/native-runtime/status-arbiter.ts index ba5bf640d92..89b62d3028c 100644 --- a/server/src/services/native-runtime/status-arbiter.ts +++ b/server/src/services/native-runtime/status-arbiter.ts @@ -168,6 +168,12 @@ export function arbitrateNativeStatus(input: { reviewOwnerUserId?: string | null; /** Review decisions own task state; a reviewer's finish report cannot override them. */ nativeReviewOutcome?: "resolved" | "pending" | "stale"; + /** + * This run, as the configured participant, recorded the approval that + * completed the task's final execution-policy stage. That durable decision is + * the completion authority for its done report, as the policy intends. + */ + executionStageApprovedByRun?: boolean; /** Re-derived from the committed runner terminal and pinned execution identity. */ providerModelRejected?: boolean; providerOverloaded?: boolean; @@ -439,7 +445,13 @@ export function arbitrateNativeStatus(input: { !input.assessment.hasFailedVerification && input.assessment.attentionRequests.length === 0 && !input.assessment.hasBlockingRemainingWork; - const complete = evidenceComplete || policyClaimComplete; + const stageApprovalComplete = + input.executionStageApprovedByRun === true && + input.assessment.reportedDisposition === "done" && + !input.assessment.hasFailedVerification && + input.assessment.attentionRequests.length === 0 && + !input.assessment.hasBlockingRemainingWork; + const complete = evidenceComplete || policyClaimComplete || stageApprovalComplete; if (complete && input.hasPendingChildCompletion) { return { policyVersion: NATIVE_STATUS_ARBITER_POLICY_VERSION, @@ -456,9 +468,11 @@ export function arbitrateNativeStatus(input: { policyVersion: NATIVE_STATUS_ARBITER_POLICY_VERSION, statusAction: "done", toStatus: "done", - reasonCode: input.completionClaimPolicyAccepted - ? "completion_claim_policy_accepted" - : "completion_contract_satisfied", + reasonCode: !evidenceComplete && !policyClaimComplete + ? "execution_stage_approved" + : input.completionClaimPolicyAccepted + ? "completion_claim_policy_accepted" + : "completion_contract_satisfied", unblockDescriptor: null, effects: [{ kind: "release_checkout" }], };