diff --git a/doc/plans/2026-10-02-stock-harness-paperclip-checklist.md b/doc/plans/2026-10-02-stock-harness-paperclip-checklist.md index a06a34870fa..5d9c2299a73 100644 --- a/doc/plans/2026-10-02-stock-harness-paperclip-checklist.md +++ b/doc/plans/2026-10-02-stock-harness-paperclip-checklist.md @@ -665,3 +665,8 @@ review hold is resolved for this summary. The original comparison remains unchan - [x] Provider-free verification: 67 focused scheduler/native-conformance/authority tests and repository typecheck pass. The scheduler regression holds the parent open, dispatches/replays the child result, then verifies exactly one sequential continuation with the latest summary. Pending, consumed and current-run delivery identities have native finalization coverage. - [ ] Freeze and measure the corrected source in the same two original live cases, one attempt each. Do not reroll the original baseline or regrade the failed candidate. - [ ] Verify the latest source head in CI/review and inspect final parent artifact delivery before claiming readiness. Keep draft until these gates pass. + +- [x] Preserve intermediate campaign `37518652522` as two cancelled, ungraded attempts on `3c1cf68`; provider activity/charges and cleanup are unverified because only invocation policies survived. These are separate from the original 17 actual runs. +- [x] Audit the concurrency review finding with a real database barrier and distinct agents. Existing implicit locking serializes this tested ordering; make the parent lock explicit before child status writes. Do not claim a newly reproduced live defect. +- [ ] Finish fresh source checks/review, then qualify the corrected source against the frozen baseline and inspect latest-artifact delivery. +- [x] Explicit-lock revision: 68 focused tests and full repository typecheck pass. Fresh build, CI/review and live checks remain pending. diff --git a/doc/plans/2026-10-06-native-delegation-handoff.md b/doc/plans/2026-10-06-native-delegation-handoff.md index 53204654ddb..9388b8a66c8 100644 --- a/doc/plans/2026-10-06-native-delegation-handoff.md +++ b/doc/plans/2026-10-06-native-delegation-handoff.md @@ -95,3 +95,30 @@ The original baseline remains frozen. Measure only the corrected candidate in the same two cases, one attempt each, under the existing bounds and oracle. Keep all previous runs and inspect final artifact delivery separately from machine pass totals. Live qualification and fresh CI/review remain pending; keep draft. + + +### Concurrency review and cancelled intermediate campaign + +Fresh review of `3c1cf6830ce9db4123b834ffb99c594c383a8986` raised an +outbox-insertion versus parent-completion race. A controlled PostgreSQL barrier +uses distinct parent and child agents and pauses the real child transaction at +wake insertion. The parent must wait, then preserve the pending result. The +previous source also passes that ordering because child writes acquire an +implicit parent foreign-key lock. This is not a reproduced additional failure. +The correction makes parent serialization explicit before child writes, avoiding +shared-lock upgrades between siblings. The original four failing regressions +remain the before/after evidence for the active-parent fix. + +Intermediate campaign `37518652522` was cancelled once that review required +source changes. Both paid-cell steps started and both tasks were created before +cancellation completed. Each retained artifact contains only its invocation +policy. No original grade, API run inventory, final ledger or cleanup receipt +survived. Provider-run count and charges are unknown; record two incomplete +attempts, not passes or zero-provider setup failures. Preserve these separately +from the original 17 actual runs. The next candidate must pass fresh review +before its bounded live attempts. Original baseline, prompts and oracle stay +frozen. + +The final lock revision passes all 68 focused scheduler, conformance and arbiter +tests plus repository typecheck. Build, fresh CI/review and live qualification +remain pending. diff --git a/server/src/services/native-runtime/paperclip-control-plane-port.test.ts b/server/src/services/native-runtime/paperclip-control-plane-port.test.ts index 6dd46e01ff3..b509a6f7feb 100644 --- a/server/src/services/native-runtime/paperclip-control-plane-port.test.ts +++ b/server/src/services/native-runtime/paperclip-control-plane-port.test.ts @@ -1542,6 +1542,108 @@ describe("PaperclipControlPlanePort conformance", () => { }); }); + it("serializes a concurrent child notification with the parent's completion check", async () => { + const identity = CONTROL_PLANE_CONFORMANCE_OPEN.identity; + async function preparedRun(parentId: string | null) { + const agentId = randomUUID(); + await db.insert(agents).values({ id: agentId, companyId: identity.companyId, + name: parentId ? "Child worker" : "Parent lead", adapterType: "codex_local", status: "running" }); + const issueId = randomUUID(); + const runId = randomUUID(); + const localContractId = randomUUID(); + const runnerInstanceId = randomUUID(); + await db.insert(issues).values({ id: issueId, companyId: identity.companyId, parentId, + title: parentId ? "Revised child" : "Deliver revised child result", status: "in_progress", + assigneeAgentId: agentId, workMode: "standard" }); + await db.insert(completionContracts).values({ id: localContractId, companyId: identity.companyId, + issueId, revision: 1, schemaVersion: "paperclip.completion-contract.v1", policyVersion: "phase6-v3", + risk: "low", completionAuthority: "agent_claim_policy", incompleteCriteriaPolicy: "preserve_non_terminal", + contractJson: { revision: "handoff-race-v1", objective: "Deliver the requested result", + criteria: [{ id: "objective", requirement: "Complete the requested work" }] }, + canonicalSha256: localContractId, createdByActorType: "system", createdByActorId: "test" }); + await db.insert(heartbeatRuns).values({ id: runId, companyId: identity.companyId, agentId, + status: "succeeded", runtimeMode: "native", nativeIssueId: issueId, nativeSessionId: identity.sessionId, + runnerInstanceId, completionContractId: localContractId, completionContractSha256: localContractId, + contextSnapshot: { issueId } }); + const port = new PaperclipControlPlanePort(db, { ...identity, agentId, issueId, runId, + completionContractId: localContractId, completionContractSha256: localContractId, + sourceInstanceId: runnerInstanceId, controlPlaneSourceInstanceId: "handoff-race" }); + await port.openRun({ identity: { ...identity, agentId, issueId, runId }, backendKind: "mock", sourceInstanceId: runnerInstanceId }); + await port.completeRun({ result: { ...structuredClone(CONTROL_PLANE_CONFORMANCE_RESULT), + summary: parentId ? "Revised result is ready." : "The result is delivered.", + completionClaim: { contractRevision: "handoff-race-v1", objectiveSatisfied: true, + criteria: [{ criterionId: "objective", status: "satisfied", evidenceRefs: [] }], remainingWork: [] }, + evidence: [], verification: [{ commandOrCheck: "node --test", status: "passed" }], attentionRequests: [], + }, terminal: CONTROL_PLANE_CONFORMANCE_TERMINAL, callerResultId: "handoff-race-result" }); + return { issueId, runId }; + } + const parent = await preparedRun(null); + const child = await preparedRun(parent.issueId); + const suffix = randomUUID().replace(/-/g, ""); + const trigger = `handoff_barrier_${suffix}`; + const lockKey = Number.parseInt(suffix.slice(0, 7), 16); + // A test-only database barrier pauses the real child transaction after it + // reads the parent and immediately before it inserts the completion wake. + await db.execute(sql.raw(`create function ${trigger}() returns trigger language plpgsql as $$ + begin + if NEW.requested_by_actor_id = 'native-status-committer' + and NEW.payload->>'issueId' = '${parent.issueId}' then + perform pg_advisory_xact_lock(${lockKey}, 1); + end if; + return NEW; + end $$`)); + await db.execute(sql.raw(`create trigger ${trigger} before insert on agent_wakeup_requests + for each row execute function ${trigger}()`)); + let childCompletion: Promise | undefined; + let parentCompletion: Promise | undefined; + let parentSettled = false; + async function waitUntil(check: () => Promise) { + for (let attempt = 0; attempt < 500; attempt++) { + if (await check()) return true; + await new Promise(resolve => setTimeout(resolve, 10)); + } + return false; + } + try { + await db.transaction(async (tx) => { + await tx.execute(sql`select pg_advisory_xact_lock(${lockKey}::integer, 1)`); + const [controller] = await tx.execute(sql`select pg_backend_pid() as pid`) as unknown as Array<{ pid: number }>; + childCompletion = finalizeNativeRun({ db, runId: child.runId, workspaceFinalizeStatus: "succeeded" }) + .then(() => null, (error: unknown) => error); + let childPid: number | null = null; + expect(await waitUntil(async () => { + const [blocked] = await db.execute(sql`select pid from pg_stat_activity + where ${controller.pid} = any(pg_blocking_pids(pid)) limit 1`) as unknown as Array<{ pid: number }>; + childPid = blocked?.pid ?? null; + return childPid !== null; + })).toBe(true); + parentCompletion = finalizeNativeRun({ db, runId: parent.runId, workspaceFinalizeStatus: "succeeded" }) + .then(() => { parentSettled = true; return null; }, (error: unknown) => { parentSettled = true; return error; }); + let parentWaiting = false; + expect(await waitUntil(async () => { + const [state] = await db.execute(sql`select exists (select 1 from pg_stat_activity + where ${childPid}::integer = any(pg_blocking_pids(pid))) as waiting`) as unknown as Array<{ waiting: boolean }>; + parentWaiting = state.waiting; + return parentWaiting || parentSettled; + })).toBe(true); + expect(parentSettled).toBe(false); + expect(parentWaiting).toBe(true); + }); + expect(await childCompletion).toBeNull(); + expect(await parentCompletion).toBeNull(); + expect((await db.select().from(issues).where(eq(issues.id, child.issueId)))[0]!.status).toBe("done"); + expect((await db.select().from(issues).where(eq(issues.id, parent.issueId)))[0]!.status).toBe("in_progress"); + expect(await db.select().from(statusDecisions).where(eq(statusDecisions.issueId, parent.issueId))) + .toEqual([expect.objectContaining({ reasonCode: "native_child_completion_pending" })]); + const wakes = await db.select().from(agentWakeupRequests).where(eq(agentWakeupRequests.companyId, identity.companyId)); + expect(wakes.filter(wake => wake.payload?.issueId === parent.issueId)).toHaveLength(1); + } finally { + await Promise.all([childCompletion, parentCompletion]); + await db.execute(sql.raw(`drop trigger ${trigger} on agent_wakeup_requests`)); + await db.execute(sql.raw(`drop function ${trigger}()`)); + } + }, 30_000); + it.each(["child_dependency", "child"].flatMap((relationship) => ["manual", "task_watchdog", "task_watchdog_product_bug"].map((originKind) => ({ relationship, originKind })), ))("preserves completion wake identity across reopen ($relationship, $originKind)", async ({ relationship, originKind }) => { diff --git a/server/src/services/native-runtime/status-decision-committer.ts b/server/src/services/native-runtime/status-decision-committer.ts index 629ff5f7a67..9f4b117cc19 100644 --- a/server/src/services/native-runtime/status-decision-committer.ts +++ b/server/src/services/native-runtime/status-decision-committer.ts @@ -1910,6 +1910,16 @@ export async function commitNativeStatusDecision(input: { ); } + if (input.decision.statusAction === "done" && issue.status !== "done" && + issue.parentId && issue.originKind !== "task_watchdog") { + // Serialize child notification creation with the parent's final pending- + // result check. Lock before child writes can acquire an implicit parent + // foreign-key lock, so concurrent siblings never upgrade shared locks. + await tx.select({ id: issues.id }).from(issues).where(and( + eq(issues.id, issue.parentId), eq(issues.companyId, input.companyId), + )).for("update"); + } + const materialized: NativeMaterializedStatusEffect[] = []; const preMaterializedEffects = new Map( (input.preMaterializedEffects ?? []).map((effect) => [