Repository navigation
Conversation
A reviewer launch that dies at transport (a host-terminal error before any protocol report) leaves its callId bound to the checkpoint generation forever: the tracked launch reverts the linked review item's round (the role leaves inFlightReviewers) but no code path released the checkpoint binding. The slot then deadlocks: relaunch rejects ALREADY_LAUNCHED, a fresh report can never be recorded (CALLBACK_MISMATCH), verify reports incomplete forever, recover refuses while the generation counts as live, and start refuses ALREADY_IN_REVIEW. Release the binding when it is provably dead: 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; the original guard order (ALREADY_LAUNCHED before ALREADY_RECORDED) is preserved. Fixes osovv#43
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #43 at the source level. A reviewer launch that dies at transport (a host-terminal error before any protocol report) leaves its callId bound to the checkpoint generation forever: the tracked launch reverts the linked review item's round (the role leaves
inFlightReviewers), but no code path released the checkpoint binding, so the slot deadlocks — relaunch rejectsALREADY_LAUNCHED, a fresh report can never be recorded (CALLBACK_MISMATCH), verify reportsincompleteforever, recover refuses while the generation counts as live, and start refusesALREADY_IN_REVIEW.Verified the unconditional guard is still present in v2.5.0 (
recordCheckpointReviewerLaunchInStore) before preparing this PR.Change
Inside the
ALREADY_LAUNCHEDguard, release the binding when it is provably dead: 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; the original guard order (ALREADY_LAUNCHEDbeforeALREADY_RECORDED) is preserved. The linked record lookup is scoped to the owning session.Tests
records reviewer outcomes with call binding and rejects stale callbacksto launch through a tracked launch, so the duplicate-rejection assertion exercises the live-launch path (the unchanged behavior) rather than an untracked binding, which the fix now treats as dead;releases a provably dead reviewer binding after a transport-failed launch (issue #43): launch → transport death (revertReviewerLaunch) → relaunch accepted → result recorded → completed review still rejects;bun test src/plugins/workflow/checkpoints.test.ts: 25 pass / 0 fail; pre-commit hooks green (check-exports, oxlint, oxfmt, tsc, commitlint).Production evidence
The equivalent dist-level patch has run in production since 2026-11-05 on three installs (2.3.0-rc.8 and 2.4.0 plugin caches plus the global CLI), validated by a 5-scenario behavioral test (dead released; live still rejected; recorded still rejected; fresh generation binds; message shape unchanged). Four real deadlocked generations across three projects were unblocked by it.