Repository navigation
review_checks: run heavy scanners when the lease policy fails; add missing-tools row - #2116
Merged
Merged
Conversation
…ssing-tools row When perun_policy.py heavy-acquire exited with an unexpected code (rc=2), lease_take set lease=none and returned 0, but the next call hit the "lease must be ok" guard and returned 1. Every later heavy scanner (semgrep, trivy, osv-scanner, ...) was then skipped with a false "heavy lease full", including under --full. lease=none now means no lease policy, so scanners run; only a genuinely full lease skips. The policy failure is reported once on stderr without adding a bogus not-checked row to whichever scanner hit it first. "Full" is now an explicit signal: heavy-acquire exits 1 and prints FULL. Python also exits 1 on any uncaught exception (for example a ModuleNotFoundError), which used to read as a full lease; it is now a policy failure and the scanners run unleased. stdout goes to a file, not $(...), because the lease pid is the caller's parent and must stay this shell. A tool that is not installed, or whose offline DB is missing (osv-scanner, trivy), is now listed in one closing "review_checks: missing tools:" line on stderr with an install or warm-the-DB hint for each. zizmor, bandit and hadolint are intentionally unleased: they are light single-file checks. Only the heavy scanners take the lease. Adds scripts/test-review-checks-lease-policy.sh, registered in scripts/ci-tests.d/ (3/10 pass against the parent's review_checks.sh, 10/10 after), plus a changelog.d fragment. Tests: 10/10 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
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.
Summary
Heavy scanners (gitleaks, osv-scanner, semgrep, trivy) were wrongly skipped as "heavy lease full" in two cases. First, when the lease policy returned an unusable state, the state went to
nonebut the nextlease_takecall returned 1. Second, whenperun_policy.pycrashed with an uncaught exception, exit 1 was read as "full". Now a lease counts as full only on exit 1 together with stdoutFULL; any other failure runs the scanners without a lease and reports the policy failure once.Also adds one closing
review_checks: missing tools:row that lists every scanner that is not installed or whose DB is blocked, each with its install or warm-DB hint. Seen in a recent full audit where semgrep, trivy and osv silently did not run.zizmor, bandit and hadolint stay unleased on purpose: they are light single-file checks.
Verification
bash scripts/test-review-checks-lease-policy.sh: 10/10 (new; covers rc=2, a traceback exit 1, a real FULL and the missing-tools row).skill_index.py --check, size ratchet, changelog fragments and the banlist grep pass against origin/main.🤖 Generated with Claude Code