diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 4c4601dabc8..9e190669123 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -2062,7 +2062,7 @@ results, intentional stops, and historical unmarked failures keep their existing terminal behavior. This continuation uses the already charged restart attempt and does not reset the provider-attempt budget. -Provider execution and control-plane finalization have different clocks. A healthy provider can think or execute a long tool without output. Once execution settles, recovery and finalization control steps have a 60-second deadline, checked on startup and every 15 seconds. With a healthy database and scheduler, an abandoned transition must be repaired or surfaced within 90 seconds. Terminal persistence must not wait on provider cleanup or publication; a late finalizer cannot change a reassigned or closed task or release another run's locks. Historical ambiguous runs are never automatically replayed after an upgrade. +Provider execution and control-plane finalization have different clocks. A healthy provider can think or execute a long tool without output. Once execution settles, recovery and finalization control steps have a 60-second deadline, checked on startup and every 15 seconds. With a healthy database and scheduler, an abandoned transition must be repaired or surfaced within 90 seconds. Terminal persistence must not wait on provider cleanup or publication; a late finalizer cannot change a reassigned or closed task or release another run's locks. In particular, a done report from a run whose task now belongs to another agent, a user or nobody is kept as an accepted result but records `completion_owner_superseded` and preserves the task status for the current owner. Historical ambiguous runs are never automatically replayed after an upgrade. When that deadline fails a native run with a prepared workspace, the same transaction records the exact source's file-preservation obligation. Existing diff --git a/server/src/services/native-runtime/native-run-finalizer.ts b/server/src/services/native-runtime/native-run-finalizer.ts index d5f850982ac..740e1908026 100644 --- a/server/src/services/native-runtime/native-run-finalizer.ts +++ b/server/src/services/native-runtime/native-run-finalizer.ts @@ -57,6 +57,7 @@ import type { PrpIgnoredAttentionRequest, PrpTerminalState } from "@paperclipai/ import { arbitrateNativeStatus, NATIVE_STATUS_ARBITER_POLICY_VERSION, + supersedeStaleOwnerCompletion, type NativeAuthoritativeIssueStatus, } from "./status-arbiter.js"; import { recordNativeWorkAssessment } from "./work-assessments.js"; @@ -1539,6 +1540,11 @@ export async function finalizeNativeRun(input: { proposedDecision = { ...proposedDecision, statusAction: "in_review", toStatus: "in_review", reasonCode: "onboarding_first_task_decision_required", unblockDescriptor: null, effects: [{ kind: "release_checkout" }] }; } + // A reassigned-away run keeps its accepted result, but cannot close the + // new owner's task. The status committer rechecks this under the issue lock. + if (!isConversation(authoritativeIssue)) { + proposedDecision = supersedeStaleOwnerCompletion(proposedDecision, authoritativeIssue, run.agentId); + } if (proposedDecision.statusAction === "done") { const continuation = record(run.contextSnapshot?.executionContinuation); const objective = typeof continuation.objective === "string" ? continuation.objective diff --git a/server/src/services/native-runtime/native-stale-owner-completion.test.ts b/server/src/services/native-runtime/native-stale-owner-completion.test.ts new file mode 100644 index 00000000000..3541caa00b3 --- /dev/null +++ b/server/src/services/native-runtime/native-stale-owner-completion.test.ts @@ -0,0 +1,183 @@ +import { randomUUID } from "node:crypto"; +import { eq } from "drizzle-orm"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { agents, companies, completionContracts, createDb, heartbeatRuns, issueWorkProducts, issues, nativeRunFinalizations, + nativeRunResults, statusDecisions, type Db } from "@paperclipai/db"; +import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../../__tests__/helpers/embedded-postgres.js"; +import { resolveExternalTestDatabaseUrl } from "../../__tests__/helpers/external-test-database.js"; +import { NativeRunCoordinatorStore } from "./native-run-coordinator-store.js"; +import { finalizeNativeRun } from "./native-run-finalizer.js"; +import { reconcileNativeFinalizations } from "./native-finalization-reconciler.js"; +import { NATIVE_STATUS_ARBITER_POLICY_VERSION, supersedeStaleOwnerCompletion, type NativeStatusDecision } from "./status-arbiter.js"; + +const external = resolveExternalTestDatabaseUrl("PAPERCLIP_NATIVE_STALE_OWNER_TEST_DATABASE_URL"); +describe("stale owner completion policy", () => { + const done: NativeStatusDecision = { policyVersion: NATIVE_STATUS_ARBITER_POLICY_VERSION, statusAction: "done", toStatus: "done", + reasonCode: "completion_claim_policy_accepted", unblockDescriptor: null, effects: [{ kind: "release_checkout" }] }; + const owner = { status: "in_progress", assigneeAgentId: "agent-a", assigneeUserId: null }; + + it("keeps the current owner's done decision", () => { + expect(supersedeStaleOwnerCompletion(done, owner, "agent-a")).toBe(done); + }); + it.each([ + ["another agent", { ...owner, assigneeAgentId: "agent-b" }], + ["a board user", { ...owner, assigneeAgentId: null, assigneeUserId: "user-1" }], + ["nobody", { ...owner, assigneeAgentId: null }], + ])("preserves status when the task now belongs to %s", (_label, issue) => { + expect(supersedeStaleOwnerCompletion(done, issue, "agent-a")).toEqual({ ...done, statusAction: "preserve", + toStatus: "in_progress", reasonCode: "completion_owner_superseded", effects: [] }); + }); + it("leaves non-terminal decisions to their existing owner checks", () => { + const blocked: NativeStatusDecision = { ...done, statusAction: "blocked", toStatus: "blocked", reasonCode: "x" }; + expect(supersedeStaleOwnerCompletion(blocked, { ...owner, assigneeAgentId: "agent-b" }, "agent-a")).toBe(blocked); + }); +}); + +const support = external ? { supported: true } : await getEmbeddedPostgresTestSupport(); + +describe.skipIf(!support.supported)("native completion after reassignment", () => { + let database: Awaited> | undefined; + let db: Db; + beforeAll(async () => { + if (!external) database = await startEmbeddedPostgresTestDatabase("native-stale-owner-"); + db = createDb(external ?? database!.connectionString); + }, 60_000); + afterAll(async () => { await db?.$client.end(); await database?.cleanup(); }); + + /** + * A task whose original owner reported done in an accepted result. With + * pendingEvidence, the server arbiter waits for review of a work product. + */ + async function acceptedDoneResult(options: { pendingEvidence?: boolean } = {}) { + const companyId = randomUUID(), agentId = randomUUID(), successorId = randomUUID(), issueId = randomUUID(); + const runId = randomUUID(), sessionId = randomUUID(), runnerId = randomUUID(); + await db.insert(companies).values({ id: companyId, name: "Stale owner fixture", issuePrefix: companyId.slice(0, 8) }); + await db.insert(agents).values([ + { id: agentId, companyId, name: "Original owner", adapterType: "paperclip_runner" }, + { id: successorId, companyId, name: "New owner", adapterType: "paperclip_runner" }, + ]); + await db.insert(issues).values({ id: issueId, companyId, title: "FizzBuzz variants", status: "in_progress", + assigneeAgentId: agentId, description: "Write fizzbuzz with tests and mark done." }); + const [contract] = await db.insert(completionContracts).values({ companyId, issueId, revision: 1, + schemaVersion: "paperclip.completion-contract.v1", policyVersion: "test", risk: options.pendingEvidence ? "standard" : "low", + completionAuthority: options.pendingEvidence ? "server_arbiter" : "agent_claim_policy", + incompleteCriteriaPolicy: "preserve_non_terminal", contractJson: { revision: "test", objective: "Write fizzbuzz", criteria: [{ id: "objective", requirement: "Complete the task" }] }, + canonicalSha256: `stale-owner-${issueId}`, createdByActorType: "system", createdByActorId: "test" }).returning(); + await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, nativeIssueId: issueId, nativeSessionId: sessionId, + runnerInstanceId: runnerId, completionContractId: contract.id, completionContractSha256: contract.canonicalSha256, + status: "running", runtimeMode: "native", contextSnapshot: { issueId, source: "issue.assigned" } }); + await db.update(issues).set({ executionRunId: runId }).where(eq(issues.id, issueId)); + const workProductId = randomUUID(); + if (options.pendingEvidence) await db.insert(issueWorkProducts).values({ id: workProductId, companyId, issueId, + type: "artifact", provider: "paperclip", title: "fizzbuzz.py", status: "ready_for_review", reviewState: "none" }); + const evidenceRef = options.pendingEvidence ? `work_product:${workProductId}` : `task:${issueId}`; + const port = new NativeRunCoordinatorStore(db, { companyId, issueId, runId, agentId, normalizedSessionId: sessionId, + runnerSourceInstanceId: runnerId, completionContractId: contract.id, completionContractSha256: contract.canonicalSha256, + completionContractRevision: "test", completionContractCriterionIds: ["objective"] }); + await port.completeRun({ result: { schema: "paperclip.run_result.v1", reportedWorkDisposition: "done", + summary: "Implemented fizzbuzz; 6 passed.", completionClaim: { contractRevision: "test", objectiveSatisfied: true, + criteria: [{ criterionId: "objective", status: "satisfied", evidenceRefs: [evidenceRef] }], remainingWork: [] }, + evidence: [{ ref: evidenceRef }], + verification: [{ commandOrCheck: "pytest -q", status: "passed", detail: "6 passed", artifactRef: evidenceRef }], attentionRequests: [], artifacts: [] }, + terminal: { schema: "paperclip.prp.terminal.v1", runTerminalState: "succeeded", turnTerminalState: "completed", + reportedWorkDisposition: "done" }, turnId: "fizzbuzz-turn" }); + return { companyId, agentId, successorId, issueId, runId, workProductId }; + } + + /** The board's reassignment: new owner, and the old run loses execution. */ + async function reassign(f: Awaited>) { + await db.update(issues).set({ assigneeAgentId: f.successorId, executionRunId: null, checkoutRunId: null }) + .where(eq(issues.id, f.issueId)); + await db.update(heartbeatRuns).set({ status: "cancelled", errorCode: "issue_reassigned", finishedAt: new Date() }) + .where(eq(heartbeatRuns.id, f.runId)); + } + + const issue = async (issueId: string) => (await db.select().from(issues).where(eq(issues.id, issueId)))[0]; + const decision = async (runId: string) => { + const [coordinator] = await db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, runId)); + const [row] = await db.select().from(statusDecisions).where(eq(statusDecisions.id, coordinator.decisionId!)); + return { coordinator, row }; + }; + + it("closes the task when the reporting run still owns it", async () => { + const f = await acceptedDoneResult(); + await finalizeNativeRun({ db, runId: f.runId, workspaceFinalizeStatus: "succeeded", projectRunStatus: false }); + expect(await issue(f.issueId)).toMatchObject({ status: "done", assigneeAgentId: f.agentId }); + expect((await decision(f.runId)).row.reasonCode).toBe("completion_claim_policy_accepted"); + }); + + it("does not let a reassigned-away run's done result close the new owner's task", async () => { + const f = await acceptedDoneResult(); + const [acceptedBefore] = await db.select().from(nativeRunResults).where(eq(nativeRunResults.runId, f.runId)); + await reassign(f); + + await finalizeNativeRun({ db, runId: f.runId, workspaceFinalizeStatus: "succeeded", projectRunStatus: false }); + + const current = await issue(f.issueId); + expect(current).toMatchObject({ status: "in_progress", assigneeAgentId: f.successorId }); + const { coordinator, row } = await decision(f.runId); + expect(coordinator.phase).toBe("committed"); + expect(row).toMatchObject({ toStatus: "in_progress", reasonCode: "completion_owner_superseded" }); + // The original owner's accepted result is kept as history, unchanged. + expect((await db.select().from(nativeRunResults).where(eq(nativeRunResults.runId, f.runId)))[0]).toEqual(acceptedBefore); + }); + + it("does not close the task when reassignment lands after arbitration but before the status commit", async () => { + const f = await acceptedDoneResult(); + let reassigned = false; + // Reassign at the last finalizer write before the status transaction. + const racing = new Proxy(db, { + get(target, property, receiver) { + const value = Reflect.get(target, property, receiver); + if (property !== "update") return typeof value === "function" ? value.bind(target) : value; + return (table: unknown) => { + const builder = target.update(table as typeof nativeRunFinalizations); + if (table !== nativeRunFinalizations || reassigned) return builder; + const set = builder.set.bind(builder); + return Object.assign(builder, { set: (values: Record) => { + const next = set(values as never); + if (values.phase !== "arbitrating" || !("assessmentId" in values)) return next; + const where = next.where.bind(next); + return Object.assign(next, { where: (...args: Parameters) => { + const query = where(...args); + return { then: (resolve: (value: unknown) => unknown, reject: (error: unknown) => unknown) => + query.then(async (result) => { reassigned = true; await reassign(f); return result; }).then(resolve, reject) }; + } }); + } }); + }; + }, + }) as Db; + + await finalizeNativeRun({ db: racing, runId: f.runId, workspaceFinalizeStatus: "succeeded", projectRunStatus: false }); + + expect(reassigned).toBe(true); + expect(await issue(f.issueId)).toMatchObject({ status: "in_progress", assigneeAgentId: f.successorId }); + expect((await decision(f.runId)).row).toMatchObject({ reasonCode: "completion_owner_superseded" }); + }); + async function approveEvidenceLater(f: Awaited>) { + await db.update(issueWorkProducts).set({ reviewState: "approved", updatedAt: new Date(Date.now() + 1_000) }) + .where(eq(issueWorkProducts.id, f.workProductId)); + } + + it("closes the owner's task when later evidence satisfies the contract", async () => { + const f = await acceptedDoneResult({ pendingEvidence: true }); + await finalizeNativeRun({ db, runId: f.runId, workspaceFinalizeStatus: "succeeded", projectRunStatus: false }); + expect((await issue(f.issueId)).status).not.toBe("done"); + await approveEvidenceLater(f); + await reconcileNativeFinalizations(db, [f.runId]); + expect(await issue(f.issueId)).toMatchObject({ status: "done", assigneeAgentId: f.agentId }); + expect((await decision(f.runId)).row.reasonCode).toBe("decision_superseded_by_new_evidence"); + }); + + it("does not let later evidence from a reassigned-away run close the new owner's task", async () => { + const f = await acceptedDoneResult({ pendingEvidence: true }); + await reassign(f); + await finalizeNativeRun({ db, runId: f.runId, workspaceFinalizeStatus: "succeeded", projectRunStatus: false }); + const [acceptedBefore] = await db.select().from(nativeRunResults).where(eq(nativeRunResults.runId, f.runId)); + await approveEvidenceLater(f); + await reconcileNativeFinalizations(db, [f.runId]); + expect(await issue(f.issueId)).toMatchObject({ status: "in_progress", assigneeAgentId: f.successorId }); + expect((await decision(f.runId)).row).toMatchObject({ toStatus: "in_progress", reasonCode: "completion_owner_superseded" }); + expect((await db.select().from(nativeRunResults).where(eq(nativeRunResults.runId, f.runId)))[0]).toEqual(acceptedBefore); + }); +}); diff --git a/server/src/services/native-runtime/native-status-reassessment.ts b/server/src/services/native-runtime/native-status-reassessment.ts index 4da42937557..1bc389852a5 100644 --- a/server/src/services/native-runtime/native-status-reassessment.ts +++ b/server/src/services/native-runtime/native-status-reassessment.ts @@ -9,7 +9,7 @@ import { classifyNativeEvidence } from "./evidence-classifier.js"; import { hasPendingNativeChildCompletion } from "./native-child-completion-delivery.js"; import { resolveNativeReconciliationStatus, type NativeReconciliationFacts } from "./native-reconciliation-status.js"; import { pendingNativeGovernance, resolveNativeFinalizerStatus } from "./native-run-finalizer.js"; -import { NATIVE_STATUS_ARBITER_POLICY_VERSION, type NativeAuthoritativeIssueStatus, type NativeStatusDecision } from "./status-arbiter.js"; +import { NATIVE_STATUS_ARBITER_POLICY_VERSION, supersedeStaleOwnerCompletion, type NativeAuthoritativeIssueStatus, type NativeStatusDecision } from "./status-arbiter.js"; import { commitNativeStatusDecision, NativeStatusRaceError } from "./status-decision-committer.js"; import { recordNativeWorkAssessment } from "./work-assessments.js"; import { readPreservedNativeResultFinalization } from "./preserved-native-result-finalization.js"; @@ -234,6 +234,10 @@ export async function reassessNativeStatusFromSource( decision = resolveNativeFinalizerStatus({ assessment: reassessment, terminalState: "succeeded", workspaceFinalizeStatus: "succeeded", requiredQualityReviewSatisfied: false, priorIssueStatus: row.issueStatus as NativeAuthoritativeIssueStatus, agentId: row.agentId }); } + // New evidence from a reassigned-away run cannot close the new owner's task. + if (currentIssue && !currentIssue.conversationAgentId) { + decision = supersedeStaleOwnerCompletion(decision, { ...currentIssue, status: row.issueStatus }, row.agentId); + } let committed: Awaited>; try { committed = await commitNativeStatusDecision({ diff --git a/server/src/services/native-runtime/status-arbiter.ts b/server/src/services/native-runtime/status-arbiter.ts index 008ca97b44b..e50ff7b66ed 100644 --- a/server/src/services/native-runtime/status-arbiter.ts +++ b/server/src/services/native-runtime/status-arbiter.ts @@ -91,6 +91,34 @@ export interface NativeStatusDecision { effects: NativeStatusEffect[]; } +/** + * Completion is the current owner's decision. A run whose task was reassigned + * away (to another agent or to a user) keeps its accepted result as history, + * but its done report cannot close the new owner's task. + */ +export function ownsNativeTaskCompletion( + issue: { assigneeAgentId: string | null; assigneeUserId: string | null }, + agentId: string, +) { + return issue.assigneeAgentId === agentId && !issue.assigneeUserId; +} + +export function supersedeStaleOwnerCompletion( + decision: NativeStatusDecision, + issue: { status: string; assigneeAgentId: string | null; assigneeUserId: string | null }, + agentId: string, +): NativeStatusDecision { + if (decision.statusAction !== "done" || ownsNativeTaskCompletion(issue, agentId)) return decision; + return { + policyVersion: NATIVE_STATUS_ARBITER_POLICY_VERSION, + statusAction: "preserve", + toStatus: issue.status as NativeAuthoritativeIssueStatus, + reasonCode: "completion_owner_superseded", + unblockDescriptor: null, + effects: [], + }; +} + /** Pure authority boundary: model prose is evidence, never a status command. */ export function arbitrateNativeStatus(input: { assessment: NativeEvidenceAssessment;