Repository navigation
feat(verify): configurable per-command verify timeout ([verify] command_timeout_min) - #897
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughVerification commands now use a configurable per-command timeout, defaulting to 30 minutes. Timed-out commands remain fixable verification failures, and failure messages report the configured limit. ChangesVerify timeout configuration
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant run_verify_commands
participant _run_shell_command
participant Engine
participant verify_command_results_outcome
run_verify_commands->>_run_shell_command: Run command with configured timeout in seconds
_run_shell_command-->>run_verify_commands: Return command result
Engine->>verify_command_results_outcome: Classify results with configured timeout in minutes
Suggested reviewers: Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely identifies the main change: adding a configurable per-command verification timeout through [verify] command_timeout_min. |
| Linked Issues check | |
Issue #857 requires a configurable timeout for [verify] commands. The PR adds VerifyPolicy.command_timeout_min with default 30 and validation of values >= 1. run_verify_commands uses the configured va… |
| Out of Scope Changes check | |
The changes remain connected to issue #857. Policy parsing, schema updates, documentation, timeout classification, operator timeout reporting, digest documentation, and tests support the configurable … |
| Docstring Coverage | |
Docstring coverage is 87.50% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 4 files. (9 skipped: 4 u… |
✨ Finishing Touches 💡 1
-
🛠️ Fix failing CI checks 💡
-
- Commit to this branch
- Create a new PR
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit reads the timeout line,
Thirty minutes set by default,
A longer gate can have more time,
A timed-out run reports its limit,
The policy shows the key to raise,
Then hops away through logs of hay.
Comment @coderabbitai help to get the list of available commands.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary
Every
[verify] commandsentry was killed after a fixed 30 minutes (verify.COMMAND_TIMEOUT_S). This adds[verify] command_timeout_min(default 30, must be >= 1) so a slow but healthy gate, such as a mutation suite, can be given a larger bound without patching the installed package.policy.py: newVerifyPolicy.command_timeout_min, parsed with_typed_int, rejected below 1, documented in the emitted template.verify.py:COMMAND_TIMEOUT_Sis gone.run_verify_commandspassescommand_timeout_min * 60to each command._probe_timed_outis renamed_timed_out_leg, since[verify]classification now uses it as well.core.tomlschema entry, regenerateddocs/reference/configuration.md, the FEATURES config line, and therunsetupdigest docstring. The field is not added to the config digest because it bounds a command and cannot name a program.Design notes
commandsis a list of strings, and the TUI lines editor would flatten table entries on save.timed out after N min (the per-command verify time limit)instead of a baretimed out. The first line,verify command failed (rc=-1): <cmd>, is byte-identical and the failure stays fixable. This text is written verbatim into the repair session's feedback, so it never namespolicy.tomland never suggests raising the limit. An agent edit there would fall outside dirty-tree accounting and outside the config digest.--reverifynames the key. This output goes to the operator, so it reads'<cmd>' timed out after N min — raise [verify] command_timeout_min in policy.toml if this gate is legitimately this slow.None, so existing direct calls keep today's exact text.Test plan
>= 1(0 and -5), non-integers (true,2.5,"60")[2700, 2700]and the default[1800])--reverifynames the limit and the key (420 s)policy.toml_run_shell_command.uv run pytest -q -n logical: 14427 passed, 113 skipped.uv run pyright: 0 errors.trunk check: no issues.Closes #857
Summary by CodeRabbit