From 5e3b2284841aea3f2fe80328117cdb88251af9f8 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 26 Oct 2026 02:59:03 -0500 Subject: [PATCH] fix(server): wake native reassessment from approval evidence Co-Authored-By: Paperclip --- ...-native-status-reassessment-sleep-audit.md | 43 +++++++++++++++ .../native-status-reassessment-work.test.ts | 53 +++++++++++++++++-- .../services/approval-work-notifications.ts | 6 ++- 3 files changed, 98 insertions(+), 4 deletions(-) diff --git a/doc/plans/2026-10-26-native-status-reassessment-sleep-audit.md b/doc/plans/2026-10-26-native-status-reassessment-sleep-audit.md index 5ee9b751220..80b7140a2ce 100644 --- a/doc/plans/2026-10-26-native-status-reassessment-sleep-audit.md +++ b/doc/plans/2026-10-26-native-status-reassessment-sleep-audit.md @@ -258,3 +258,46 @@ changes and workspace-finalization barriers. Workspace recovery writes a separat workspace_finalize success receipt; snapshot-only rows are not that barrier. Continue the concurrent authority audit, independent-current-history/backlog cost, lifecycle/failure evidence, startup/periodic wiring and full PR acceptance. + + +## Approval evidence producer checkpoint + +Approval references are classified from the current approval status and its +company/task link. A product can have changed after the selected assessment +while that approval is still pending or unlinked. The worker correctly goes +quiet with insufficient evidence, but must inspect that same candidate when +an approval is resolved or linked later. This does not add a new reassessment +condition: the existing changed-product predicate must still be satisfied. + +The existing `notifyApprovalWork` boundary now sends the same task scopes to +native reassessment as to governed review responses, before the approval/link +write. It already resolves linked task IDs for status changes and accepts +explicit task scopes for link/unlink and bulk link. No new transaction wrapper, +company scan, table, or provider policy was introduced. Unlinked approval creation +has no task to reassess; the subsequent link owns activation. The production +writer audit also found budget approval resolution and gateway formal-approval +linking use this boundary with the appropriate scope. Company deletion removes +the records rather than creating a surviving reassessment obligation. + +Four real PostgreSQL regressions failed before the notification: approval +resolution, single link, bulk link, and an approval inside a caller-owned outer +transaction. With the notification they pass, including no premature decision +before outer commit, no decision after rollback, and driver-observed SQL quiet +before activation and after settlement. The full lifecycle file now passes 17 +cases. Existing approval service and route-idempotency consumers pass another +19 cases (36 total in the combined run). Server typecheck passes. + +Evidence: `/tmp/sleep50-approval-before.log` and `.status` (four failures), +`/tmp/sleep50-approval-after.log` and `.status` (36 passes), and +`/tmp/sleep50-approval-types.log` and `.status` (pass). The before and after runs +used separate freshly migrated PostgreSQL test databases. This is local producer +acceptance, not full slice or remote PR acceptance. + +Interaction evidence remains next: confirmation/question paths signal through +chat milestones, while other interaction outcomes signal governed review response +directly. Their common activation boundary must cover reassessment without teaching +the scheduler interaction policy or coupling it to a review-specific worker. +Several interaction mutations also lack either signal and require fact-level +audit before the old polling path can be removed. Run evidence, ownership, +completion contracts, workspace barriers, concurrent authority, discovery cost, +and startup registration remain outstanding as listed above. diff --git a/server/src/__tests__/native-status-reassessment-work.test.ts b/server/src/__tests__/native-status-reassessment-work.test.ts index e409a87bc54..e5d63b17b4c 100644 --- a/server/src/__tests__/native-status-reassessment-work.test.ts +++ b/server/src/__tests__/native-status-reassessment-work.test.ts @@ -5,6 +5,8 @@ import { issueRoutes } from "../routes/issues.js"; import { errorHandler } from "../middleware/index.js"; import { workProductService } from "../services/work-products.js"; import { issueService } from "../services/issues.js"; +import { approvalService } from "../services/approvals.js"; +import { issueApprovalService } from "../services/issue-approvals.js"; import { randomUUID } from "node:crypto"; import { setTimeout as delay } from "node:timers/promises"; import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; @@ -69,7 +71,7 @@ describe("native reassessment worker lifecycle", () => { async function status(issueId: string) { return (await db.select({ status: issues.status }).from(issues).where(eq(issues.id, issueId)))[0].status; } - async function seedReview() { + async function seedReview(evidenceRef?: string) { const issueId = randomUUID(), runId = randomUUID(), resultId = randomUUID(), contractId = randomUUID(); const assessmentId = randomUUID(), workProductId = randomUUID(); await db.insert(issues).values({ id: issueId, companyId, title: "Persisted native result", status: "in_progress", assigneeAgentId: agentId }); @@ -84,8 +86,8 @@ describe("native reassessment worker lifecycle", () => { serverFingerprint: resultId, schemaStatus: "accepted", canonicalSha256: resultId, resultJson: { result: { reportedWorkDisposition: "done", summary: "Completed", attentionRequests: [], completionClaim: { contractRevision: "test-v1", objectiveSatisfied: true, remainingWork: [], - criteria: [{ criterionId: "objective", status: "satisfied", evidenceRefs: [`work_product:${workProductId}`] }] }, - verification: [{ commandOrCheck: "artifact", status: "passed", artifactRef: `work_product:${workProductId}` }], + criteria: [{ criterionId: "objective", status: "satisfied", evidenceRefs: [evidenceRef ?? `work_product:${workProductId}`] }] }, + verification: [{ commandOrCheck: "artifact", status: "passed", artifactRef: evidenceRef ?? `work_product:${workProductId}` }], }, terminal: { runTerminalState: "succeeded" } } }); await db.insert(issueWorkProducts).values({ id: workProductId, companyId, issueId, type: "artifact", provider: "paperclip", title: "Verified artifact", status: "ready_for_review", reviewState: "approved", @@ -106,6 +108,51 @@ describe("native reassessment worker lifecycle", () => { return { issueId, runId, workProductId, decisionId: committed.decision.id }; } + it.each(["resolution", "link", "bulk link"] as const)("wakes dormant evidence from real approval %s", async mutation => { + const approvalId = randomUUID(); + const seeded = await seedReview(`approval:${approvalId}`); + await approvalService(db).create(companyId, { id: approvalId, type: "approve_ceo_strategy", payload: {}, + status: mutation === "resolution" ? "pending" : "approved" }); + if (mutation === "resolution") await issueApprovalService(db).link(seeded.issueId, approvalId); + await vi.advanceTimersByTimeAsync(1000); + await db.update(issueWorkProducts).set({ updatedAt: new Date() }).where(eq(issueWorkProducts.id, seeded.workProductId)); + await start(); await flush(); await quiet(); + expect(await status(seeded.issueId)).toBe("in_review"); + if (mutation === "resolution") await approvalService(db).approve(approvalId, "board-user"); + if (mutation === "link") await issueApprovalService(db).link(seeded.issueId, approvalId); + if (mutation === "bulk link") await issueApprovalService(db).linkManyForApproval(approvalId, [seeded.issueId]); + await flush(); + expect(await status(seeded.issueId)).toBe("done"); + expect(await db.select().from(statusDecisions).where(eq(statusDecisions.issueId, seeded.issueId))).toHaveLength(2); + await quiet(); + }); + + it("defers approval evidence until outer commit and ignores rolled-back approval", async () => { + const approvalId = randomUUID(); + const seeded = await seedReview(`approval:${approvalId}`); + await approvalService(db).create(companyId, { id: approvalId, type: "approve_ceo_strategy", payload: {} }); + await issueApprovalService(db).link(seeded.issueId, approvalId); + await vi.advanceTimersByTimeAsync(1000); + await db.update(issueWorkProducts).set({ updatedAt: new Date() }).where(eq(issueWorkProducts.id, seeded.workProductId)); + await start(); await flush(); await quiet(); + await expect(db.transaction(async tx => { + await approvalService(tx as unknown as typeof db).approve(approvalId, "board-user"); + await vi.advanceTimersByTimeAsync(1000); + expect(await status(seeded.issueId)).toBe("in_review"); + throw new Error("rollback approval evidence"); + })).rejects.toThrow("rollback approval evidence"); + await flush(); await quiet(); + expect(await status(seeded.issueId)).toBe("in_review"); + await db.transaction(async tx => { + await approvalService(tx as unknown as typeof db).approve(approvalId, "board-user"); + await vi.advanceTimersByTimeAsync(1000); + expect(await status(seeded.issueId)).toBe("in_review"); + }); + await flush(); + expect(await status(seeded.issueId)).toBe("done"); + await quiet(); + }); + it.each(["update", "create", "primary replacement", "import", "attachment", "source promotion"] as const)("wakes from the real work-product %s writer", async writer => { const seeded = await seedReview(); if (writer === "update") await db.update(issueWorkProducts).set({ reviewState: "none" }).where(eq(issueWorkProducts.id, seeded.workProductId)); diff --git a/server/src/services/approval-work-notifications.ts b/server/src/services/approval-work-notifications.ts index c7b7c349da2..3f8c4dbc98e 100644 --- a/server/src/services/approval-work-notifications.ts +++ b/server/src/services/approval-work-notifications.ts @@ -1,5 +1,6 @@ import { and, eq } from "drizzle-orm"; import { issueApprovals, type Db } from "@paperclipai/db"; +import { notifyNativeStatusReassessment } from "./native-status-reassessment-notifications.js"; import { notifyNativeReviewResponse } from "./native-review-response-notifications.js"; import { DELIVERY_QUEUES, notifyDeliveryWork } from "./delivery-work-notifications.js"; @@ -16,7 +17,10 @@ export async function notifyApprovalWork(transaction: object, companyId: string, .where(and(eq(issueApprovals.companyId, companyId), eq(issueApprovals.approvalId, scope.approvalId))); for (const link of links) issueIds.add(link.issueId); } - for (const issueId of issueIds) await notifyNativeReviewResponse(transaction, { companyId, issueId }); + for (const issueId of issueIds) { + await notifyNativeReviewResponse(transaction, { companyId, issueId }); + await notifyNativeStatusReassessment(transaction, { companyId, issueId }); + } } export function writeApprovalChange(db: Db, companyId: string, write: (transaction: Db) => Promise, scope?: ApprovalChangeScope): Promise {