From 89ea418e76a70d808653c98033f1c3e9f3ebbafc Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 11 Nov 2026 20:55:24 -0600 Subject: [PATCH] fix(native): start the configured review stage when a worker reports finished work as needs_review (#21005) 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 > - Tasks can have an execution policy with a review stage that names an independent reviewer > - #20882 made a native worker's done report start that configured stage > - A worker can also report finished work as needs_review and list the review itself as the only blocking remaining work > - That report skipped the configured-stage check. The generic "needs_review requires a named reviewer" error made the worker file a separate review card, and the configured stage never started > - This pull request treats that report as finished work and gives the configured-stage guidance first > - The benefit is that the configured reviewer gates Done whichever way the worker phrases its finished report ## Linked Issues or Issue Description Refs #20882 **What happened** A task had a review stage for a human reviewer. The native worker built and verified the deliverable. It reported `needs_review` with `objectiveSatisfied: true`, all criteria satisfied, and one blocking remaining item: "Independent human review of the prepared website.html deliverable." The completion feedback skipped the configured-stage check because that item blocked completion. The generic needs_review error then told the worker to name a reviewer. The worker added a review attention request, and Paperclip made a separate "Review requested" card. `executionState` stayed null, and the configured reviewer was not assigned. **Expected behavior** On a task with a configured review stage, a worker that reports finished, verified work is told to report done. Paperclip then starts the configured stage with its reviewer. **Steps to reproduce** 1. Create a task with `executionPolicy.stages = [{ type: "review", participants: [{ type: "user", userId }] }]` for a native agent. 2. Have the agent report `needs_review` with a satisfied objective and the review as its only blocking remaining work. 3. Before this fix, the feedback asks for a named reviewer. The task gets a separate review card and no stage. ## What Changed - `native-completion-feedback.ts`: on a task whose configured review stage would start, a `needs_review` report with a satisfied objective and satisfied criteria counts as finished work. It gets the configured-stage guidance before the generic named-reviewer error. A review needed before the work can continue (objective not satisfied) keeps its own request path. Approval requests keep their own path. - `native-completion-feedback.ts`: a done report on such a task that still lists the review as outstanding gets the same guidance, not the generic "done report includes unfinished work" error. So the worker is not sent back to needs_review. - The guidance now says to report done without listing the review as remaining work or as an attention request. - Tests: two new cases in `native-execution-policy-review-stage.test.ts` use the recorded report shape. ## Verification - `cd server && npx vitest run src/services/native-runtime/native-execution-policy-review-stage.test.ts`: 15 passed. The new recorded-shape test fails on master (red) and passes with this change (green). - `npx vitest run` for `native-completion-feedback`, `native-current-wake-completion-feedback` and `native-execution-policy-review-stage-race`: 42 passed. - Paid chat eval run on a combined throwaway branch: results are in a PR comment. ## Risks - On a configured-review task, a worker that reports a satisfied objective but has real non-review blocking work now gets the configured-stage guidance. That guidance says to report done only when the work is complete and verified, so the task does not close early. ## 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 --- .../native-completion-feedback.ts | 14 ++++++++++-- ...tive-execution-policy-review-stage.test.ts | 22 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/server/src/services/native-runtime/native-completion-feedback.ts b/server/src/services/native-runtime/native-completion-feedback.ts index e091aa4db8a..cd1e111f50b 100644 --- a/server/src/services/native-runtime/native-completion-feedback.ts +++ b/server/src/services/native-runtime/native-completion-feedback.ts @@ -40,6 +40,8 @@ const repositoryYieldCorrection = 'This task has a saved repository preparation // control plane is reachable now. Ask once per run to re-check with fresh // Paperclip calls instead of resting the task blocked on a past stall. const paperclipServiceRecheckCorrection = "Paperclip answered this report, so its control plane is reachable now. Paperclip tool calls that did not return earlier were most likely delayed by a Paperclip restart or pause, and their results may have arrived since. Do not report this as a blocker. Re-read the current Paperclip state (for example read_document) before writing again; when a write is still needed, repeat it with the same idempotencyKey so it cannot apply twice. Then continue the task and report its real outcome. Report blocked only if a fresh Paperclip call fails now."; +const configuredReviewStageCorrection = + "This task has a configured review stage with its own reviewer. Do not request a separate review. Report done once the work is complete and verified, without listing that review as remaining work or an attention request; Paperclip then starts the configured review, and the task stays in review until that reviewer decides."; const PAPERCLIP_SERVICE_RECHECK_LIMIT = 1_000; const paperclipServiceRechecked = new Set(); @@ -184,6 +186,14 @@ export async function nativeCompletionFeedback( signals.verification.some((entry) => entry.status === "failed") && "verification.failed", signals.actionableAttentionRequests.length > 0 && "attentionRequests.actionable", ].filter(Boolean).join(", ") : ""; + if (result.completionClaim.objectiveSatisfied && + result.completionClaim.criteria.every(entry => entry.status === "satisfied") && + !signals.verification.some(entry => entry.status === "failed") && + signals.actionableAttentionRequests.every(request => request.kind === "review") && + issue.executionState?.status !== "pending" && !isConversation(issue) && + completionReviewStageEntry(issue, run.agentId).kind === "starts") { + throw new Error(configuredReviewStageCorrection); + } throw new Error( "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." + (failedActionChat ? ` Rejection fields: ${rejectionReasons}.` + " This chat turn may finish with a truthful report of the failed approved action; that does not mean the external action succeeded. Keep the failure, observed state, and unresolved work explicit in the summary. Verify the observed state factually. Use not_run with a truthful reason only for a check that could not run or complete because its dependency was unavailable. Do not change a failed check to passed or hide incomplete work. Do not automatically replay the action or request approval again to force success; report the outcome and let the user decide whether to retry." : ""), @@ -389,12 +399,12 @@ export async function nativeCompletionFeedback( // Only a review of finished work belongs to the configured stage; a review // needed before the work can continue keeps its own request path. const workFinished = result.completionClaim.objectiveSatisfied && - !result.completionClaim.remainingWork.some(work => work.blocksCompletion); + result.completionClaim.criteria.every(entry => entry.status === "satisfied"); if (reviewStage && workFinished && result.reportedWorkDisposition === "needs_review" && (signals.actionableAttentionRequests.length === 0 || signals.actionableAttentionRequests.some(request => request.kind === "review"))) { throw new Error( - "This task has a configured review stage with its own reviewer. Do not request a separate review. Report done once the work is complete and verified; Paperclip then starts the configured review, and the task stays in review until that reviewer decides.", + configuredReviewStageCorrection, ); } if ( diff --git a/server/src/services/native-runtime/native-execution-policy-review-stage.test.ts b/server/src/services/native-runtime/native-execution-policy-review-stage.test.ts index a518ec9119a..fee42a153d7 100644 --- a/server/src/services/native-runtime/native-execution-policy-review-stage.test.ts +++ b/server/src/services/native-runtime/native-execution-policy-review-stage.test.ts @@ -146,6 +146,28 @@ describe.skipIf(!support.supported)("native completion with a configured review } }); }); + it("routes a finished needs_review report that lists only the review as blocking work to the configured stage", async () => { + await finishVerifiedDone({ stage: "user", unprovenEvidence: true, beforeFinish: async (runId, result) => { + const reviewOnly = { ...result.completionClaim, + remainingWork: [{ description: "Independent human review of the prepared website.html deliverable.", blocksCompletion: true }] }; + const review = { kind: "review" as const, summary: "Independent human review of website.html by Riley.", ownerClass: "human" as const }; + await expect(nativeCompletionFeedback(db, runId, { ...result, reportedWorkDisposition: "needs_review", + completionClaim: reviewOnly, attentionRequests: [review] })).rejects.toThrow("configured review stage with its own reviewer"); + await expect(nativeCompletionFeedback(db, runId, { ...result, reportedWorkDisposition: "needs_review", completionClaim: reviewOnly })) + .rejects.toThrow("without listing that review as remaining work"); + await expect(nativeCompletionFeedback(db, runId, { ...result, completionClaim: reviewOnly })) + .rejects.toThrow("configured review stage with its own reviewer"); + } }); + }); + + it("keeps the generic done rejection for unfinished work without a configured review stage", async () => { + await finishVerifiedDone({ beforeFinish: async (runId, result) => { + await expect(nativeCompletionFeedback(db, runId, { ...result, completionClaim: { ...result.completionClaim, + remainingWork: [{ description: "Independent review.", blocksCompletion: true }] } })) + .rejects.toThrow("The done report includes unfinished work"); + } }); + }); + it("keeps the review request path for a review needed before the work can continue", async () => { await finishVerifiedDone({ stage: "user", beforeFinish: async (runId, result) => { const early = { ...result, reportedWorkDisposition: "needs_review" as const,