mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-31 14:16:39 +01:00
fix(server): wake native reassessment from approval evidence
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
2d1b61c75f
commit
5e3b228484
3 files changed
+98
-4
No files matched your search
@@ -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.
|
||||
@@ -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));
|
||||
|
||||
@@ -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<T>(db: Db, companyId: string, write: (transaction: Db) => Promise<T>, scope?: ApprovalChangeScope): Promise<T> {
|
||||
|
||||
Reference in new issue
Block a user