fix: extend TUI idle-reap grace for do:release/do:pr/do:rpr review loops#2085
Merged
Conversation
A slow multi-reviewer pass (e.g. codex reading a large diff) can go silent in the wrapped TUI for well over the 3-minute default idle window, and the runner reaped the still-waiting release agent as a false idle-complete success before it ever reached the merge gate — observed on agent-61508f36, PR #2084, which sat open+unmerged for hours. Adds createReviewLoopTracker (server/lib/tuiHandshake.js), mirroring the existing merge-queue idle-grace mechanism (#2074): latches on multi-reviewer-loop chrome ("Review plan:", "Review pass", "Multi-Reviewer", "review loop"), extends the idle reaper's grace to 15 minutes while active, and surfaces a still-elapsed timeout as a needs-manual-finish review-loop-idle-timeout failure instead of a silent completed status.
…lock Local review gate caught a duplicated inline copy of decideIdleReap() across two describe blocks; consolidated into the existing #2074 block.
…ositive latch 'review pass' / 'review loop' as bare substrings would latch on ordinary CoS agent narration (this project's own CLAUDE.md "self-review pass" convention, and this repo's bundled slashdo docs saying "review loop"), not just an actual do:release/do:pr/do:rpr multi-reviewer run. Replaced the review-pass marker with a digit/slash-anchored banner regex and dropped the bare 'review loop' substring.
…r shapes
Second-round finding: the bare 'multi-reviewer' substring and unanchored
'review plan:' still false-positived on this repo's own bundled slashdo
docs (which describe these banners in prose dozens of times, including
the literal instruction text "Review plan: {REVIEW_AGENTS}..."). Anchored
all three markers to the actual rendered shape only runtime output has:
a bracketed agent list, a digit/slash pass counter, or the literal
Multi-Reviewer Summary heading. Also fixed the changelog wording to
match.
…e new detector
The changelog entry describing the review-loop markers reproduced the
literal trigger strings ("Review plan: [...]", "Multi-Reviewer Summary"),
which would false-latch createReviewLoopTracker the next time a release
run reads this file. Reworded to describe the banners without
reproducing them, matching the care already taken in the source
comments.
… its own doc codex review [P3] found that lib/slashdo/lib/multi-reviewer-loop.md's own aggregate-report example block renders the literal heading "## Multi-Reviewer Summary" as sample output, so REVIEW_SUMMARY_PATTERN would false-latch createReviewLoopTracker whenever an agent reads or quotes that one doc file. Removed the pattern — the review-plan banner alone is a complete signal (it prints once, unconditionally, before any reviewer pass begins, and the tracker latches permanently once set), and both remaining patterns were verified clean against every bundled slashdo doc.
codex review [P2] found reviewLoop.observe() only checked the current onData chunk, so a real TUI splitting the one-shot Review plan:/Review pass banner across two chunks (plausible during token-by-token streaming) would never latch the tracker, silently reintroducing the original idle-reap bug this PR fixes. createReviewLoopTracker now keeps a small bounded rolling tail (512 chars, well over the banner's ~60-char size) and tests the concatenated buffer, so a split marker is still detected on the very next chunk.
…reap # Conflicts: # .changelog/NEXT.md
…reap # Conflicts: # .changelog/NEXT.md
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
1 participant
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
A CoS TUI agent running
/do:release(agent-61508f36) correctly backgrounded a slowcodexmulti-reviewer pass and polled for it rather than blocking — but the reviewer's silent working stretch exceeded the 3-minute default idle window, and the runner reaped the still-waiting agent as a falseidle-completesuccess before it ever reached the merge gate. PR #2084 sat open+unmerged for hours while the CoS dashboard reported the task as "completed".This generalizes the existing
/do:next --swarmmerge-queue idle-grace fix (#2074) to also cover the multi-reviewer loop used by/do:release,/do:pr, and/do:rpr:createReviewLoopTracker(server/lib/tuiHandshake.js) latches when TUI output shows a multi-reviewer loop is active ("Review plan:", "Review pass", "Multi-Reviewer", "review loop").MERGE_QUEUE_IDLE_TIMEOUT_MS).review-loop-idle-timeoutfailure (needs-manual-finish) instead of a silentidle-completesuccess.Test plan
server/lib/tuiHandshake.test.js— new marker/tracker unit testsserver/services/agentTuiSpawning.test.js— new idle-reap decision-matrix tests (consolidated into the existing CoS swarm agents reaped as 'idle-complete' during Phase C merge queue — leaves PRs unmerged/uncleaned #2074 describe block)npm test) passes: 15,770 passed, 154 skipped (DB suites, no test DB configured)