sweep: concurrent codex reviews via CODEX_JOBS worker pool - #48
Merged
Conversation
| let done = false | ||
| const timer = opts.timeoutMs === undefined ? undefined : setTimeout(() => { | ||
| extra += `\n${cmd}: process killed by SIGTERM (timeout ${opts.timeoutMs}ms)` | ||
| child.kill('SIGTERM') |
There was a problem hiding this comment.
🟠 src/review-sweep.ts:967 · footgun · conf 0.77
The timeout path only sends SIGTERM and then lets the normal close handler decide success from the child's exit code. A codex process that traps SIGTERM and exits 0 can still be accepted as ok, and one that does not exit leaves the worker pending and the sweep lock held indefinitely.
→ Fix: Track a timedOut flag and make timeout a failed outcome regardless of exit code; preferably add a short SIGKILL fallback so the promise always settles.
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.
A sweep's wall-clock was the SUM of its codex runs — each review is minutes, so a busy sweep serialized PRs behind each other for up to an hour while the lock held off every cron tick.
Now the loop collects candidates through the same cheap serial gates (dedup, throttle, diff cap, MAX_PRS, daily budget), then reviews them with a pool of up to
CODEX_JOBS(default 3) concurrent codex children. Only the codex spawn went async (execAsync, mirroring the kit'sexecresult shape); every gh call stays the kit's sync exec, and all shared-state mutation stays on the one JS thread between awaits. A quotalimitfrom any worker stops new launches while in-flight runs drain, then the un-started candidates get the usual deferred status.