Repository navigation
fix(doctor): enforce strictly-below hook-timeout contract at the boundary - #2513
Conversation
…dary CodeRabbit (#2510): the omni hook-timeout check warned only when timeoutMs < pollBudgetMs, but genie-config.ts's contract is pollBudgetMs MUST stay STRICTLY below the hook timeout — at timeoutMs === pollBudgetMs there is zero margin (CC can kill the hook the instant the budget expires), so it now warns on equal too. needSec = floor(pollBudgetMs/1000)+1 so the suggested timeout strictly exceeds the budget. Added the exact-boundary test.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the omni hook timeout guardrail check to enforce a strict inequality contract, requiring the hook timeout to be strictly greater than the poll budget. It adjusts the calculation of the minimum required seconds and updates the warning/pass logic and corresponding unit tests. The reviewer suggested making the hardcoded example in the suggestion message dynamic, as it can become misleading if the calculated minimum required seconds exceed 120.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| status: 'warn', | ||
| detail: `hook timeout ${params.timeoutSec}s (${timeoutMs}ms) < pollBudget ${params.pollBudgetMs}ms — CC will kill the hook before it can allow/deny or reach its ask fail-safe`, | ||
| detail: `hook timeout ${params.timeoutSec}s (${timeoutMs}ms) ≤ pollBudget ${params.pollBudgetMs}ms — CC may kill the hook before it can allow/deny or reach its ask fail-safe`, | ||
| suggestion: `Raise the PreToolUse \`genie hook dispatch\` timeout to ≥ ${needSec}s (e.g. 120) in ~/.claude/settings.json.`, |
There was a problem hiding this comment.
If pollBudgetMs is configured to a value larger than 120_000 ms (for example, 150_000 ms), needSec will be 151. In this scenario, the hardcoded example (e.g. 120) in the suggestion becomes misleading because 120 is less than the required minimum of 151 seconds. Making the example dynamic ensures it remains valid and helpful regardless of the configured pollBudgetMs.
| suggestion: `Raise the PreToolUse \`genie hook dispatch\` timeout to ≥ ${needSec}s (e.g. 120) in ~/.claude/settings.json.`, | |
| suggestion: `Raise the PreToolUse \`genie hook dispatch\` timeout to ≥ ${needSec}s (e.g. ${needSec >= 120 ? needSec + 10 : 120}) in ~/.claude/settings.json.`, |
Gemini (#2513): the suggestion's hardcoded '(e.g. 120)' contradicts needSec when pollBudgetMs > 120_000 (e.g. 150_000 → needSec 151 > 120). needSec is already the exact actionable minimum, so drop the parenthetical example rather than making it a conditional. Test asserts needSec (111) instead of the removed 120.
Finding (CodeRabbit on #2510)
src/genie-commands/doctor.ts— the omni hook-timeout guardrail warned only whentimeoutMs < pollBudgetMsand passed attimeoutMs === pollBudgetMs. Butsrc/types/genie-config.tsdocuments thatpollBudgetMsMUST stay strictly below the hook timeout: at exactly equal there is no margin — Claude Code can kill the hook the instant the poll budget expires, before it can allow/deny or reach its ask fail-safe.Fix
timeoutMs <= pollBudgetMs(equal now warns); reword</≥→≤/>.needSec = floor(pollBudgetMs/1000) + 1— the smallest whole-second timeout that strictly exceeds the budget (wasceil, which for the default 110 000 ms suggested110sthat still equals it).Test
Added the exact-boundary case CodeRabbit flagged as missing (
timeoutSec: 110,pollBudgetMs: 110_000→ warn, suggests111s); the pass test now requires strict>.Gates: 14 doctor tests pass;
bun run check+ build green.