refactor(review): delete the vestigial timeout sentinel (review feedback)

main

Commit

peyton-alt2mo ago

Review feedback asked whether the two timeout helpers could collapse to max(). Tracing every ReviewerTimeout reader shows the truth is stronger on one side and subtler on the other:

  • resolveReviewerTimeoutArg is deleted outright, not simplified. Its -1 'disabled' sentinel was load-bearing in the old three-state world (positive/default-20m/disabled); with the default gone, both remaining consumers (reviewerTimeout, judgeTimeoutArg) use a plain > 0 check, so -1 and 0 were indistinguishable everywhere and the mapping did nothing. The --timeout flag value now flows into RunConfig unmapped. This also removes the stale doc comment Copilot flagged ('the flag's default is nonzero').

  • judgeTimeoutArg stays as a named function — the point is its contract (the judge is one bounded call and must never be uncapped, whatever the reviewer setting says), not the arithmetic — but its body is now max(reviewerArg, 0) as suggested (builtin max; math.Max is float64). A negative can still arrive via an explicit '--timeout -5m'.

  • types/reviewer.go's ReviewerTimeout doc still described the deleted defaultReviewerTimeout three-state; rewritten to the two-state contract.

Tests updated to pin the flag->RunConfig two-state flow directly.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com Entire-Checkpoint: 01KX0NQY37TYQ2B8DF5CGBA8FJ

Checkpoints

test(review): pin that codex $name skills survive the legacy repair + native invocation

Claude Code
View session
Checkpoint 1