fix(actions): retire review scans when PRs return to draft - #1869
Merged
Conversation
|
Warning Review limit reachedNext included review available in 47 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
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 |
This was referenced Sep 4, 2026
seonghobae
added a commit
that referenced
this pull request
Sep 4, 2026
Full-suite run after the first commit surfaced two more casualties of the same class of issue: - test_github_hourly_conflict_repair.py::test_central_repository_has_hourly_self_caller asserted the same stale hourly cron format (#1860) for the .github self-caller entry that the first commit already fixed for the other 17 hourly-review-repair.yml targets; missed because it lives in a different test module than test_hourly_review_repair_callers.py. - test_docs_only_pr_runner_admission.py's changed-scope byte-identity check only stripped lines literally starting with "if:", so it never handled a YAML block-scalar `if: >-` condition split across multiple lines. #1869 gave strix.yml's changed-scope job exactly that (a multi-line if: adding a converted_to_draft skip, intentionally scoped to Strix/Noema per that PR's own description and not needed by security-scan.yml/sast-semgrep.yml's unrelated SAST/dependency jobs), which the test's own normalization couldn't see past -- flagging a real difference in the job body that does not exist. Made the normalization indentation-aware so it skips the entire if: block (its continuation lines included), not just its first line. python3 -m pytest tests/test_github_hourly_conflict_repair.py tests/test_docs_only_pr_runner_admission.py -q -- 14 passed. Full coverage run -m pytest tests now at 2851 passed, 1 skipped, 21 subtests passed, 0 failed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2 tasks
seonghobae
added a commit
that referenced
this pull request
Sep 4, 2026
…s after #1869 #1869 renamed CLOSED_PR_NUMBER to INACTIVE_PR_NUMBER, added an INACTIVE_PR_HEAD_SHA/PR_ACTION live-target re-verification gate before cancellation, and gave strix.yml's changed-scope job a multi-line folded-scalar `if:` for the new converted_to_draft skip. The close-cleanup test still referenced the old step name/env var and never modeled the new live-PR gh call, and the drift contract's line-based `if:` filter left the folded scalar's continuation lines in place, reporting spurious drift against the other four gate copies. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
seonghobae
added a commit
that referenced
this pull request
Sep 5, 2026
…st (#1877) * fix(tests): repair changed-scope drift and stale noema cancel-step test Two pre-existing tests/ failures blocked the unscoped pytest discovery run in agent-review-runtime-quality-ci.yml's review-repair contracts step, unrelated to the hourly-cron fixes in #1875: - strix.yml's changed-scope job if: had drifted onto a multi-line `>-` block scalar when PR #1869 added converted_to_draft handling, and the extra continuation lines broke byte-identical parity with security-scan.yml/sast-semgrep.yml's copies. Collapsed back to one physical if: line with the same expression. - test_noema_close_cleanup_selects_only_the_closed_pr_across_shared_display_titles still targeted the pre-#1869 step name/env vars (CLOSED_PR_NUMBER) that PR #1869 renamed to "...for the inactive pull request" / INACTIVE_PR_NUMBER/INACTIVE_PR_HEAD_SHA/PR_ACTION when it generalized noema-review.yml's cleanup to also cover converted_to_draft and added a live_target_matches re-verification. tests/test_noema_review_gate.py's equivalent tests were already updated; this one was missed. Updated the step name/env vars and taught the fake gh to answer the new live-PR lookup -- the PR #1507 pull_requests[] cancellation-scoping invariant it protects is unchanged and still correctly implemented in production. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX * fix(tests): mock fetch_pr in Strix rerun job selection test test_dispatch_strix_reruns_scan_job_not_sibling_publisher only mocked rerun_actions_job, leaving dispatch_strix_evidence's live_dispatch_head_matches call to invoke the real fetch_pr. In any environment with a real gh CLI on PATH this hits the actual GitHub API for a synthetic PR that does not exist there, returning a live/head mismatch ("stale_head") instead of the expected "rerun"; without gh at all it fails even earlier with a missing executable. Add monkeypatch.setattr(sched, "fetch_pr", lambda *_args: [pr]) so the live-head re-read observes the same fixture pr as authoritative, consistent with how every other GitHub call in this test path is already isolated. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX * fix(tests): align Strix admission assertion * fix(tests): align daily review recovery fixtures --------- Co-authored-by: Claude <noreply@anthropic.com>
seonghobae
added a commit
that referenced
this pull request
Sep 5, 2026
#1883) The admission-controller feature burst (#1859-#1869) shipped review_admission_controller.py, pr_review_merge_scheduler_core.py's SchedulerAdmissionGate, and (separately, pre-existing) a coverage gap in audit_codeql_default_setup_rollout.py without full test coverage or docstrings, breaking the required 100% coverage/docstring gate for every PR in this repository regardless of that PR's own diff. The original fix for this landed on .github#1871, which was later closed in favor of narrower successors (#1877 for the stale schedule oracles, #1879 for HTTP error response bodies) -- but the coverage and docstring portion of #1871's delta was dropped in that narrowing and never reached main. This PR recovers exactly that portion from #1871's still-present branch (fix/hourly-review-repair-callers-cron- format-drift) and completes it: - review_admission_controller.py: 85% -> 100% coverage (new tests/test_review_admission_controller.py), 14 missing docstrings added across its WorkerBoundary/AdmissionRequest/RequestRecord/ DispatchLease/ControllerState/DispatchPlan dataclasses and methods. - audit_codeql_default_setup_rollout.py: 79% -> 100% coverage (new tests/test_codeql_default_setup_rollout.py), 2 missing docstrings added (parse_args, main). - pr_review_merge_scheduler_core.py's SchedulerAdmissionGate: 3 missing docstrings added (__init__ and its two nested closures, lease/reconcile_state). Additionally closed pr_review_merge_scheduler_core.py's own separate, longer-standing coverage gap (98% -> 100%, unrelated to the admission-controller work) discovered while verifying this fix would actually bring main to a green gate rather than a differently-shaped 99%: the durable admission gate's own bounded-budget/stale-head branches across every dispatch call site (9 "admission_deferred" checks across post_update_branch_followup/dispatch_draft_review_only/ inspect_pr, plus dispatch_strix_evidence's own two "admission_deferred"/ "stale_head" pairs), reconcile()'s live-head-moved and still-running branches, rotating_pr_window's/dispatch_draft_review_only's/the workflow-run classifier's/the empty-PR-close path's/main()'s own --admission-state-path wiring's remaining gaps, and two untestable package-import fallback lines marked `# pragma: no cover - package import path` matching this file's established convention for that exact pattern. Full local triad: 2875 passed, 1 skipped; coverage 100%; interrogate 100%. Co-authored-by: Claude Sonnet 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
1 participant
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
converted_to_draft;ready_for_reviewremains the authoritative restart eventQueue evidence
At audit time, 11 of 12 running Strix jobs belonged to Draft PRs. Those jobs occupied GitHub-hosted runners despite no merge-ready evidence being needed until
ready_for_review.Verification
uv run pytest -q tests/test_noema_review_gate.py tests/test_required_workflow_queue_contract.py— 185 passedactionlint .github/workflows/strix.yml .github/workflows/noema-review.ymlgit diff --checkThe broader
scripts/ci/test_strix_quick_gate.shcurrently fails on pre-existing main-branch string expectations unrelated to this diff (admitted-concurrency wording and scheduler cron/fallback text); this PR does not treat that baseline failure as passing evidence.