fix(native): start the configured review stage when a native worker reports done

A native worker that reported verified work as done closed its task even when
the task's execution policy required an independent human review stage. The
native finalizer committed status through the issue service, which does not run
the execution-policy transition that the issue route runs. A worker that
reported needs_review instead moved the task to in_review without starting the
configured stage or binding its reviewer.

The finalizer now routes a done decision on a task with an unstarted human
review stage to in_review, and the status committer starts that stage from the
locked task (pending state, reviewer assignment, return assignee). Completion
feedback tells the worker to report done so the configured stage requests the
review, and rejects a separate review request on such a task.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 08e32deee144dfcfb7a35269027e0921b4c5e203)
This commit is contained in:
Dotta committed 2026-11-11 12:02:16 -06:00
1 parent 76da2fec4f
commit b8462a0433
5 files changed
+147

No files matched your search

@@ -1237,6 +1237,32 @@ export function applyIssueExecutionPolicyTransition(input: TransitionInput): Tra
return stageResult;
}
/** Status decision reason when verified native work enters a configured human review stage. */
export const COMPLETION_REVIEW_STAGE_REASON = "execution_stage_review_required";
/**
* The review stage a worker's completion must enter before the task can be
* Done, as the issue route would start it for `status: "done"`. Returns the
* stage patch (in_review, pending state, reviewer assignment) only when the
* next pending stage goes to a person; null when no stage would start.
*/
export function completionHumanReviewStageEntry(issue: IssueLike, actorAgentId: string): Record<string, unknown> | null {
const policy = normalizeIssueExecutionPolicy(issue.executionPolicy ?? null);
if (!policy) return null;
let patch: Record<string, unknown>;
try {
({ patch } = applyIssueExecutionStageTransition({
issue, policy, previousPolicy: policy, requestedStatus: "done",
requestedAssigneePatch: {}, actor: { agentId: actorAgentId, userId: null },
}));
} catch {
return null;
}
const state = patch.executionState as Partial<IssueExecutionState> | null | undefined;
if (patch.status !== "in_review" || state?.status !== "pending" || state.currentParticipant?.type !== "user") return null;
return patch;
}
export function applyIssueMonitorPolicyTransition(input: TransitionInput): TransitionResult {
return { patch: applyMonitorTransition(input, {}) };
}
@@ -10,6 +10,7 @@ import { findSupersededExternalReviewRequests } from "./superseded-external-revi
import { evaluateAgentInvokabilityFromDb } from "../agent-invokability.js";
import { issueService } from "../issues.js";
import { isConversation } from "../agent-conversations.js";
import { completionHumanReviewStageEntry } from "../issue-execution-policy.js";
import { issueTreeControlService } from "../issue-tree-control.js";
import { isPublishedGitHubReviewCompletionContinuation, readPublishedGitHubReviewCompletionContinuation, resolveExternalChatResponseWaitAuthorization } from "./chat-attachment-reuse.js";
import { getNativeReviewAssignment, readNativeReviewAssignmentContext } from "./native-review-participant.js";
@@ -381,6 +382,14 @@ export async function nativeCompletionFeedback(
);
}
}
const reviewStage = issue.executionState?.status !== "pending" && !isConversation(issue) &&
completionHumanReviewStageEntry(issue, run.agentId) !== null;
if (reviewStage && result.reportedWorkDisposition === "needs_review" &&
signals.actionableAttentionRequests.every(request => request.kind === "review" || request.kind === "approval")) {
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.",
);
}
if (
result.reportedWorkDisposition === "needs_review" &&
signals.actionableAttentionRequests.length === 0
@@ -407,6 +416,9 @@ export async function nativeCompletionFeedback(
if (result.reportedWorkDisposition === "blocked") {
return "Blocker report accepted. Explain why work cannot continue, name the blocker owner and give the unblock action in your final response; do not describe the task as completed.";
}
if (reviewStage && result.reportedWorkDisposition === "done") {
return `Completion report accepted. This task's configured review stage starts after this turn, and the task stays in review until its reviewer decides. Describe the delivered work and say it awaits that review on [this task](/issues/${issue.identifier ?? issue.id}); do not say the task is done.`;
}
const links = await savedDocumentLinks(db, run, issue);
const documentGuidance = links.length
? ` When compatible with the requested response format, include these clickable links to this run's saved documents in your final response: ${links.join(" ")}` : "";
@@ -0,0 +1,94 @@
import { randomUUID } from "node:crypto";
import { eq } from "drizzle-orm";
import { afterAll, beforeAll, describe, expect, it } from "vitest";
import { agents, companies, completionContracts, createDb, heartbeatRuns, companyMemberships, issueWorkProducts, issues, statusDecisions } from "@paperclipai/db";
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../../__tests__/helpers/embedded-postgres.js";
import { nativeCompletionFeedback } from "./native-completion-feedback.js";
import { finalizeNativeRun } from "./native-run-finalizer.js";
import { PaperclipControlPlanePort } from "./paperclip-control-plane-port.js";
import { CONTROL_PLANE_CONFORMANCE_RESULT, CONTROL_PLANE_CONFORMANCE_TERMINAL } from "../../vendor/paperclip-runner/testing.js";
import type { PrpStructuredRunResult } from "../../vendor/paperclip-runner/index.js";
const support = await getEmbeddedPostgresTestSupport();
describe.skipIf(!support.supported)("native completion with a configured review stage", () => {
let temporary: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
let db: ReturnType<typeof createDb>;
beforeAll(async () => {
temporary = await startEmbeddedPostgresTestDatabase("native-review-stage-");
db = createDb(temporary.connectionString);
}, 90_000);
afterAll(async () => { await temporary?.cleanup(); });
async function finishVerifiedDone(withStage: boolean, beforeFinish?: (runId: string, result: PrpStructuredRunResult) => Promise<void>) {
const companyId = randomUUID(), agentId = randomUUID(), issueId = randomUUID(), runId = randomUUID();
const reviewerId = `reviewer-${randomUUID()}`, stageId = randomUUID();
await db.insert(companies).values({ id: companyId, name: "Review stage", issuePrefix: `R${companyId.slice(0, 7)}` });
await db.insert(agents).values({ id: agentId, companyId, name: "Worker", adapterType: "paperclip_runner", status: "active" });
await db.insert(companyMemberships).values({ companyId, principalType: "user", principalId: reviewerId, status: "active", membershipRole: "operator" });
await db.insert(issues).values({ id: issueId, companyId, title: "Build the site", status: "in_progress", assigneeAgentId: agentId,
...(withStage ? { executionPolicy: { mode: "normal", commentRequired: true,
stages: [{ id: stageId, type: "review", approvalsNeeded: 1, participants: [{ id: randomUUID(), type: "user", userId: reviewerId }] }] } } : {}) });
await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, nativeIssueId: issueId, status: "running",
runtimeMode: "native", contextSnapshot: { issueId } });
await db.update(issues).set({ executionRunId: runId }).where(eq(issues.id, issueId));
const contractId = randomUUID(), sessionId = randomUUID(), runnerId = randomUUID(), contractSha256 = `contract-${runId}`;
await db.insert(completionContracts).values({ id: contractId, companyId, issueId, revision: 1,
schemaVersion: "paperclip.completion-contract.v1", policyVersion: "test", risk: "standard",
completionAuthority: "server_arbiter", incompleteCriteriaPolicy: "preserve_non_terminal",
contractJson: { revision: "standalone-v1", objective: "Build the site", criteria: [{ id: "objective", requirement: "Build the site" }] },
canonicalSha256: contractSha256, createdByActorType: "system", createdByActorId: "test" });
await db.update(heartbeatRuns).set({ completionContractId: contractId, completionContractSha256: contractSha256,
nativeSessionId: sessionId, runnerInstanceId: runnerId }).where(eq(heartbeatRuns.id, runId));
const [verified] = await db.insert(issueWorkProducts).values({ companyId, issueId,
type: "artifact", provider: "test", title: "Site check", status: "completed", reviewState: "approved" }).returning();
const evidenceRef = `work_product:${verified!.id}`;
const result: PrpStructuredRunResult = { ...CONTROL_PLANE_CONFORMANCE_RESULT,
completionClaim: { ...CONTROL_PLANE_CONFORMANCE_RESULT.completionClaim,
criteria: [{ criterionId: "objective", status: "satisfied", evidenceRefs: [evidenceRef] }] },
evidence: [{ kind: "work_product", ref: evidenceRef }],
verification: [{ commandOrCheck: "Site check", status: "passed", artifactRef: evidenceRef }] };
if (beforeFinish) await beforeFinish(runId, result);
const identity = { companyId, agentId, issueId, runId };
const port = new PaperclipControlPlanePort(db, { ...identity, sessionId, completionContractId: contractId,
completionContractSha256: contractSha256, sourceInstanceId: runnerId, controlPlaneSourceInstanceId: randomUUID() });
await port.openRun({ identity: { ...identity, sessionId }, backendKind: "mock", sourceInstanceId: runnerId });
await port.completeRun({ result, terminal: CONTROL_PLANE_CONFORMANCE_TERMINAL, callerResultId: randomUUID() });
await finalizeNativeRun({ db, runId, workspaceFinalizeStatus: "succeeded" });
const [task] = await db.select().from(issues).where(eq(issues.id, issueId));
const decisions = await db.select().from(statusDecisions).where(eq(statusDecisions.issueId, issueId));
return { task: task!, decisions, reviewerId, stageId, agentId };
}
it("closes verified work on a task without a configured review stage", async () => {
const r = await finishVerifiedDone(false);
expect(r.task.status).toBe("done");
expect(r.decisions.map(row => row.reasonCode)).toEqual(["completion_contract_satisfied"]);
});
it("starts the configured human review stage instead of closing verified work", async () => {
const r = await finishVerifiedDone(true);
expect(r.task).toMatchObject({ status: "in_review", assigneeUserId: r.reviewerId, assigneeAgentId: null });
expect(r.task.executionState).toMatchObject({ status: "pending", currentStageId: r.stageId, currentStageType: "review",
currentParticipant: { type: "user", userId: r.reviewerId }, returnAssignee: { type: "agent", agentId: r.agentId } });
expect(r.decisions.map(row => [row.toStatus, row.reasonCode])).toEqual([["in_review", "execution_stage_review_required"]]);
});
it("tells the worker the configured stage requests the review", async () => {
await finishVerifiedDone(true, async (runId, result) => {
const accepted = await nativeCompletionFeedback(db, runId, result);
expect(accepted).toContain("configured review stage starts after this turn");
expect(accepted).toContain("do not say the task is done");
await expect(nativeCompletionFeedback(db, runId, { ...result, reportedWorkDisposition: "needs_review",
attentionRequests: [{ kind: "review", summary: "Riley: please review the site.", ownerClass: "human" }] }))
.rejects.toThrow("Report done once the work is complete and verified");
await expect(nativeCompletionFeedback(db, runId, { ...result, reportedWorkDisposition: "needs_review" }))
.rejects.toThrow("configured review stage with its own reviewer");
});
});
it("keeps ordinary completion feedback when no review stage is configured", async () => {
await finishVerifiedDone(false, async (runId, result) => {
expect(await nativeCompletionFeedback(db, runId, result)).not.toContain("configured review stage");
});
});
});
@@ -37,6 +37,7 @@ import { findSupersededExternalReviewRequests } from "./superseded-external-revi
import { getNativeReviewAssignment, readNativeReviewAssignmentContext } from "./native-review-participant.js";
import { conversationNativeDecision, isConversation, settleConversationTurnForRun } from "../agent-conversations.js";
import { issueTreeControlService } from "../issue-tree-control.js";
import { COMPLETION_REVIEW_STAGE_REASON, completionHumanReviewStageEntry } from "../issue-execution-policy.js";
import { randomUUID } from "node:crypto";
import { preserveNativeWorkspaceExportLease } from "./native-workspace-export-resume.js";
import { and, desc, eq, inArray, isNotNull, isNull, or, sql } from "drizzle-orm";
@@ -1723,6 +1724,13 @@ export async function finalizeNativeRun(input: {
: effect.kind === "record_finalization_error" ? { ...effect, nextAction: error.message } : effect) };
}
}
// A configured human review stage gates Done exactly as the issue route
// does: verified work enters that stage instead of closing the task.
if (proposedDecision.statusAction === "done" && !isConversation(authoritativeIssue) &&
completionHumanReviewStageEntry(authoritativeIssue, run.agentId)) {
proposedDecision = { ...proposedDecision, statusAction: "in_review", toStatus: "in_review",
reasonCode: COMPLETION_REVIEW_STAGE_REASON, unblockDescriptor: null };
}
const decision = conversationNativeDecision({
conversation: isConversation(authoritativeIssue), terminalState,
workspaceFinalizeStatus: input.workspaceFinalizeStatus, hasGovernanceGate: !!governanceGate,
@@ -73,6 +73,7 @@ import {
resolveExternalChatResponseWaitAuthorizationInTransaction,
} from "./chat-attachment-reuse.js";
import { issueService } from "../issues.js";
import { COMPLETION_REVIEW_STAGE_REASON, completionHumanReviewStageEntry } from "../issue-execution-policy.js";
import { companySkillService } from "../company-skills.js";
import { syncRoutineRunStatusForIssue } from "../routine-run-status.js";
import { issueThreadInteractionService } from "../issue-thread-interactions.js";
@@ -2337,9 +2338,15 @@ export async function commitNativeStatusDecision(input: {
if (!updated) throw new NativeStatusRaceError();
} else {
failAt("status_projection", input.failpoint);
// Start the configured review stage from the locked task, as the issue
// route does; a changed policy makes the saved decision stale.
const reviewStage = input.decision.reasonCode === COMPLETION_REVIEW_STAGE_REASON
? completionHumanReviewStageEntry(issue, originRun?.agentId ?? "") : null;
if (input.decision.reasonCode === COMPLETION_REVIEW_STAGE_REASON && !reviewStage) throw new NativeStatusRaceError();
const projected = await issueService(tx as unknown as Db).update(
input.issueId,
{
...reviewStage,
status: input.decision.toStatus,
statusVersion: input.priorStatusVersion + 1,
lastStatusDecisionId: decisionRow.id,