mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
fix(server): let a native reviewer approve its review stage (#20373)
## 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
302ada90df
commit
09241442ea
8 files changed
+232
-10
No files matched your search
@@ -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.",
|
||||
"",
|
||||
);
|
||||
|
||||
@@ -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<string, unknown>) => ({ ...issue, ...patch }));
|
||||
const db = createRunContextDb();
|
||||
const inserted: Array<{ table: string; values: Record<string, unknown> }> = [];
|
||||
db.insert = vi.fn((table: Parameters<typeof getTableName>[0]) => ({ values: vi.fn(async (values: Record<string, unknown>) => {
|
||||
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<string, any>;
|
||||
// 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<string, unknown>), 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" });
|
||||
|
||||
|
||||
@@ -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 &&
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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":"<your decision and the evidence you checked>"}, 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.`;
|
||||
}
|
||||
|
||||
@@ -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<boolean> {
|
||||
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<typeof resolveNativeFinalizerStatus>[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,
|
||||
|
||||
@@ -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" });
|
||||
|
||||
@@ -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" }],
|
||||
};
|
||||
|
||||
Reference in new issue
Block a user