From 4d2c91869445c08ff6e81278bbbb734f4d710a5e Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 19 Sep 2026 10:07:44 -0500 Subject: [PATCH] test(runner-e2e): require durable review ordering evidence Co-Authored-By: Paperclip --- tests/runner-e2e/README.md | 8 ++-- tests/runner-e2e/everyday-flow.ts | 2 +- tests/runner-e2e/everyday-observations.ts | 25 ++++++++++++- tests/runner-e2e/everyday.test.ts | 45 +++++++++++++++++++---- 4 files changed, 66 insertions(+), 14 deletions(-) diff --git a/tests/runner-e2e/README.md b/tests/runner-e2e/README.md index ecd5953029..ef33a335a5 100644 --- a/tests/runner-e2e/README.md +++ b/tests/runner-e2e/README.md @@ -793,6 +793,8 @@ on that same run still fails immediately and fails the lifecycle grader. The run ID alone is not an exemption from recovery failures. The review-handoff case also requires proof that the parent was blocked before -the review wake. If all tasks finish without that ordering, the harness fails -promptly with an unexercised-boundary diagnostic. Successful work alone does not -prove that this recovery path was tested. +the review wake. When all tasks finish and persisted timestamps prove that the +accepted review started before any parent run finished, the harness fails +promptly with an unexercised-boundary diagnostic. Missing evidence in separately +fetched snapshots does not trigger this rejection. Successful work alone does +not prove that this recovery path was tested. diff --git a/tests/runner-e2e/everyday-flow.ts b/tests/runner-e2e/everyday-flow.ts index 9b11c328e3..a0c4fc5d2c 100644 --- a/tests/runner-e2e/everyday-flow.ts +++ b/tests/runner-e2e/everyday-flow.ts @@ -694,7 +694,7 @@ export async function runEverydayFlow(input: Input) { ); return failed ? `Review handoff prerequisite failed: ${failed.errorCode}: ${failed.error}` - : storyUnexercisedReviewBoundary(state.issues, state.runs); + : storyUnexercisedReviewBoundary(state.issues, state.runs, parent!.id, fixtures.agent.id); }, }); const child = boundary.issues.find((issue) => issue.parentId === parent!.id)!; diff --git a/tests/runner-e2e/everyday-observations.ts b/tests/runner-e2e/everyday-observations.ts index 5fb35cee38..adbaaeed81 100644 --- a/tests/runner-e2e/everyday-observations.ts +++ b/tests/runner-e2e/everyday-observations.ts @@ -229,17 +229,38 @@ export function storyUnexpectedRunFailure(runs: StoryRun[], allowedRunIds: reado ); } -/** Called only after the required review boundary did not match. */ +/** Called after the boundary matcher; reject only with durable proof of the wrong ordering. */ export function storyUnexercisedReviewBoundary( issues: StoryIssue[], runs: StoryRun[], + parentId: string, + leadId: string, ): string | undefined { if (issues.length === 0 || runs.length === 0) return; if (!issues.every((issue) => issue.status === "done" && !issue.scheduledRetry && !issue.activeRecoveryAction, )) return; if (!runs.every((run) => run.status === "succeeded")) return; - return "Review handoff boundary not exercised: all tasks finished without evidence of a blocked parent before the review wake."; + const child = issues.find((issue) => issue.parentId === parentId); + const accepted = child?.interactions?.flatMap((interaction) => { + const review = storyAcceptedAgentReview(child, interaction.id, leadId, runs); + return review ? [review] : []; + })[0]; + const reviewRun = accepted && runs.find((run) => run.id === accepted.resolvedByRunId); + const reviewStartedAt = Date.parse(reviewRun?.startedAt ?? ""); + const parentRuns = runs.filter((run) => + run.nativeIssueId === parentId || + run.contextSnapshot?.issueId === parentId || + run.contextSnapshot?.taskId === parentId, + ); + // Snapshots are fetched separately. Missing review evidence is not proof that it + // never happened; wait unless persisted timestamps rule out the required order. + if (!Number.isFinite(reviewStartedAt) || parentRuns.length === 0) return; + if (!parentRuns.every((run) => { + const finishedAt = Date.parse(run.finishedAt ?? ""); + return Number.isFinite(finishedAt) && finishedAt > reviewStartedAt; + })) return; + return "Review handoff boundary not exercised: the accepted review started before any parent run finished, so the blocked-parent-before-review ordering was not tested."; } export function storyLifecycleChecks(input: { diff --git a/tests/runner-e2e/everyday.test.ts b/tests/runner-e2e/everyday.test.ts index 6f08592011..b4bbdfba42 100644 --- a/tests/runner-e2e/everyday.test.ts +++ b/tests/runner-e2e/everyday.test.ts @@ -24,6 +24,7 @@ import { storyParentCompletionPrecedesReview, storyAcceptedAgentReview, type StoryRun, + type StoryIssue, } from "./everyday-observations.js"; describe("everyday workflow grader and review timing", () => { @@ -57,16 +58,44 @@ describe("everyday workflow grader and review timing", () => { describe("unexercised review boundary diagnostics", () => { const done = { id: "parent", companyId: "company", title: "task", status: "done" }; - const completed = { id: "run", companyId: "company", agentId: "lead", status: "succeeded" }; - it("fails promptly when all work finishes without the required review ordering", () => { - expect(storyUnexercisedReviewBoundary([done], [completed])).toContain("not exercised"); + const completed = { + id: "run", companyId: "company", agentId: "lead", status: "succeeded", + nativeIssueId: "parent", finishedAt: "2026-09-19T14:47:48.134Z", + }; + const child = { + ...done, id: "child", parentId: "parent", interactions: [{ + id: "card", issueId: "child", kind: "request_confirmation", status: "accepted", + addresseeAgentId: "lead", resolvedByAgentId: "lead", resolvedByRunId: "review", + createdAt: "2026-09-19T14:47:04.000Z", resolvedAt: "2026-09-19T14:47:21.876Z", + payload: { target: { type: "custom", key: "native_completion_review", revisionId: "decision" } }, + result: { version: 1, outcome: "accepted" }, + }], + }; + const review = { + ...completed, id: "review", nativeIssueId: "child", startedAt: "2026-09-19T14:47:05.802Z", + contextSnapshot: { nativeReviewInteractionId: "card", nativeReviewDecisionId: "decision" }, + }; + const diagnose = (issues: StoryIssue[] = [done, child], runs: StoryRun[] = [completed, review]) => + storyUnexercisedReviewBoundary(issues, runs, "parent", "lead"); + it("fails promptly with persisted proof that review preceded parent completion", () => { + expect(diagnose()).toContain("not exercised"); }); it("does not preempt in-flight finalization, recovery, or unfinished work", () => { - expect(storyUnexercisedReviewBoundary([done], [{ ...completed, status: "running" }])).toBeUndefined(); - expect(storyUnexercisedReviewBoundary([{ ...done, scheduledRetry: {} }], [completed])).toBeUndefined(); - expect(storyUnexercisedReviewBoundary([{ ...done, activeRecoveryAction: {} }], [completed])).toBeUndefined(); - expect(storyUnexercisedReviewBoundary([{ ...done, status: "blocked" }], [completed])).toBeUndefined(); - expect(storyUnexercisedReviewBoundary([], [])).toBeUndefined(); + expect(diagnose([done, child], [{ ...completed, status: "running" }, review])).toBeUndefined(); + expect(storyUnexercisedReviewBoundary([{ ...done, scheduledRetry: {} }, child], [completed, review], "parent", "lead")).toBeUndefined(); + expect(storyUnexercisedReviewBoundary([{ ...done, activeRecoveryAction: {} }, child], [completed, review], "parent", "lead")).toBeUndefined(); + expect(diagnose([{ ...done, status: "blocked" }, child])).toBeUndefined(); + expect(diagnose([], [])).toBeUndefined(); + }); + it("waits when separately fetched snapshots lack review evidence or timestamps", () => { + expect(diagnose([done, { ...child, interactions: [] }])).toBeUndefined(); + expect(diagnose([done, child], [completed])).toBeUndefined(); + expect(diagnose([done, child], [{ ...completed, finishedAt: "" }, review])).toBeUndefined(); + expect(diagnose([done, child], [completed, { ...review, startedAt: "" }])).toBeUndefined(); + expect(diagnose([done, child], [review])).toBeUndefined(); + }); + it("does not reject a completed workflow with the required timing", () => { + expect(diagnose([done, child], [{ ...completed, finishedAt: review.startedAt }, review])).toBeUndefined(); }); });