diff --git a/src/plugins/workflow/checkpoints.test.ts b/src/plugins/workflow/checkpoints.test.ts index 938d33e..1782e25 100644 --- a/src/plugins/workflow/checkpoints.test.ts +++ b/src/plugins/workflow/checkpoints.test.ts @@ -387,7 +387,7 @@ describe("verifyDelegatedCheckpoint", () => { }); test("records reviewer outcomes with call binding and rejects stale callbacks", async () => { - const { runId } = await startedGeneration("binding"); + const { runId, reviewWorkItemId } = await startedGeneration("binding"); recordCheckpointReviewerLaunch(store, { runId, checkpointId: "CHECKPOINT-R-001", @@ -404,6 +404,15 @@ describe("verifyDelegatedCheckpoint", () => { expect(mismatch.ok).toBe(false); if (!mismatch.ok) expect(mismatch.errorCode).toBe("CALLBACK_MISMATCH"); + // A live launch keeps the reviewer in flight on the linked review item, + // so a relaunch attempt is still rejected while the reviewer works. + const tracked = store.beginTrackedLaunch({ + sessionId: SESSION, + workItemId: reviewWorkItemId, + agent: "vv-code-reviewer", + }); + expect(tracked.ok).toBe(true); + const duplicate = recordCheckpointReviewerLaunch(store, { runId, checkpointId: "CHECKPOINT-R-001", @@ -414,6 +423,61 @@ describe("verifyDelegatedCheckpoint", () => { if (!duplicate.ok) expect(duplicate.errorCode).toBe("ALREADY_LAUNCHED"); }); + test("releases a provably dead reviewer binding after a transport-failed launch (issue #43)", async () => { + const { runId, reviewWorkItemId } = await startedGeneration("dead-binding"); + recordCheckpointReviewerLaunch(store, { + runId, + checkpointId: "CHECKPOINT-R-001", + reviewer: "code", + callId: "rv-dead", + }); + const tracked = store.beginTrackedLaunch({ + sessionId: SESSION, + workItemId: reviewWorkItemId, + agent: "vv-code-reviewer", + }); + expect(tracked.ok).toBe(true); + // The launch dies at transport: the tracked launch reverts the round and + // the reviewer leaves inFlightReviewers, but the checkpoint binding must + // not stay dead forever. + const reverted = store.revertReviewerLaunch({ + sessionId: SESSION, + workItemId: reviewWorkItemId, + agent: "vv-code-reviewer", + }); + expect(reverted.ok).toBe(true); + + const relaunched = recordCheckpointReviewerLaunch(store, { + runId, + checkpointId: "CHECKPOINT-R-001", + reviewer: "code", + callId: "rv-fresh", + }); + expect(relaunched.ok).toBe(true); + + const recorded = recordCheckpointReviewerResult(store, { + runId, + checkpointId: "CHECKPOINT-R-001", + reviewer: "code", + callId: "rv-fresh", + status: "PASS", + }); + expect(recorded.ok).toBe(true); + if (recorded.ok) expect(recorded.roundComplete).toBe(true); + + // A completed review still rejects a relaunch of the same role (the + // original guard order — ALREADY_LAUNCHED before ALREADY_RECORDED — is + // preserved when a result is already recorded). + const afterResult = recordCheckpointReviewerLaunch(store, { + runId, + checkpointId: "CHECKPOINT-R-001", + reviewer: "code", + callId: "rv-3", + }); + expect(afterResult.ok).toBe(false); + if (!afterResult.ok) expect(afterResult.errorCode).toBe("ALREADY_LAUNCHED"); + }); + test("FAIL settles the checkpoint as failed even after the review item closes", async () => { const { runId, reviewWorkItemId } = await startedGeneration("fail"); const launched = store.beginTrackedLaunch({ diff --git a/src/plugins/workflow/checkpoints.ts b/src/plugins/workflow/checkpoints.ts index 0c7654b..fb0ec6f 100644 --- a/src/plugins/workflow/checkpoints.ts +++ b/src/plugins/workflow/checkpoints.ts @@ -927,11 +927,37 @@ export function recordCheckpointReviewerLaunchInStore( }; } if (active.review.reviewerCallIds[input.reviewer]) { - return { - ok: false, - errorCode: "ALREADY_LAUNCHED", - message: `ALREADY_LAUNCHED: ${input.reviewer} already launched for generation ${active.review.generation}`, - }; + // A reviewer launch that dies at transport (a host-terminal error before + // any protocol report) used to leave its callId bound here forever: the + // tracked launch reverts the linked review item's round (the role leaves + // inFlightReviewers) but nothing released this binding, so the slot could + // never be relaunched — verify reports "incomplete" forever, recover + // refuses while the generation counts as live, and start refuses + // ALREADY_IN_REVIEW (issue #43). Release a provably dead binding: no + // result recorded for the role AND the linked review item's active round + // has the role out of flight. A live relaunch (role still in flight) and + // a completed review (result recorded) still reject exactly as before. + const linkedRecord = [...data.records.values()].find( + (entry) => + entry.sessionId === active.run.sessionId && + entry.workItemId === active.review.reviewWorkItemId, + ); + const roleInFlight = Boolean( + linkedRecord?.currentRound && + linkedRecord.currentRound.status === "active" && + Array.isArray(linkedRecord.currentRound.inFlightReviewers) && + linkedRecord.currentRound.inFlightReviewers.includes(input.reviewer), + ); + const resultRecorded = Boolean(active.review.results[input.reviewer]); + if (!roleInFlight && !resultRecorded) { + delete active.review.reviewerCallIds[input.reviewer]; + } else { + return { + ok: false, + errorCode: "ALREADY_LAUNCHED", + message: `ALREADY_LAUNCHED: ${input.reviewer} already launched for generation ${active.review.generation}`, + }; + } } if (active.review.results[input.reviewer]) { return {