fix(tests): stub startup-failure recovery so scheduler tests are GITHUB_ACTIONS-agnostic - #1896
Conversation
…UB_ACTIONS-agnostic recover_current_head_startup_failures() only runs inside inspect_pr() when os.environ["GITHUB_ACTIONS"] == "true" (#1846), so it silently never fired in a developer's local shell -- but GitHub Actions sets that variable for the entire job, including the pytest process that runs this very test file. 14 pre-existing tests (12 sharing the cancel_stale_pr_runs stub, 2 closing empty PRs) call inspect()/inspect_pr() with dry_run=False and never anticipated this side effect, so in CI they hit the real recovery path: one cluster used the fixture's placeholder headRefOid="head" and blew up in validate_git_sha, the other supplied a real sha but its narrow `run` stub didn't accept the recovery path's extra kwargs. Stub recover_current_head_startup_failures to a no-op `[]` in each, matching the existing pattern already used by test_inspect_pr_recovers_startup_failure_before_other_actions for the one test that intentionally exercises this integration. Reproduced and verified with GITHUB_ACTIONS=true set locally under both Python 3.12 and the CI-pinned 3.14: full suite 2833 passed, 1 skipped, 21 subtests, 100% coverage on the previously-failing job's tracked modules. Co-Authored-By: Claude Sonnet 5 <[email protected]>
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. |
|
Warning Review limit reachedNext included review available in 45 seconds. 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 (1)
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 |
Independent review verification (host 1)Verified against a fresh
LGTM from my side. (Also found and fixed a fresh regression while re-verifying: |
1 participant
Summary
recover_current_head_startup_failures()only runs insideinspect_pr()whenos.environ["GITHUB_ACTIONS"] == "true"(introduced by #1846), so it silently never fired in a developer's local shell. But GitHub Actions sets that variable for the entire job, including the pytest process that runs this test file — so in real CI, this path fires for real inside the test suite itself.14 pre-existing tests call
inspect()/inspect_pr()withdry_run=Falsewithout anticipating this side effect:cancel_stale_pr_runsstub pattern and rely onmake_pr()'s defaultheadRefOid="head"placeholder — this blows up insidevalidate_git_shawithValueError: invalid git sha: 'head'.runstub (lambda args: ...) doesn't accept the recovery path's extrastdin=keyword argument —TypeError: ...<lambda>() got an unexpected keyword argument 'stdin'.This is currently breaking the
agent-review-runtime-qualityrequired check on essentially every open PR whose diff touches thereview_repairsuite path — confirmed viagh api .../actions/workflows/349383189/runs, which shows this exactVerify scheduler and contextual-orchestrator review-repair contractsstep failing across ~20 unrelated branches in the last day (docs/gap-baseline-main-history-splice,fix/hourly-review-repair-callers-cron-format-drift,codex/repair-codeql-startup-materialization, etc.), none of which touch this code path in their own diffs.Fix
Stub
recover_current_head_startup_failuresto a no-oplambda repo, pr, *, dry_run: []in each of the 14 affected tests, matching the exact pattern already used by the one test that intentionally exercises this integration (test_inspect_pr_recovers_startup_failure_before_other_actions). Test-only change — no production code touched.Test plan
GITHUB_ACTIONS=true(the missing piece that made this invisible in every local run) — confirmed all 17 sub-failures before the fix, on both Python 3.12 and the CI-pinned 3.14.GITHUB_ACTIONS=true PYTHONPATH=. python -m pytest tests/test_pr_review_merge_scheduler.py -q→ 312 passed.--cov=... --cov-branch --cov-fail-under=100) → 2833 passed, 1 skipped, 21 subtests, 100% coverage on the tracked modules — matches the CI job's own gate exactly.python -m compileall -q tests/test_pr_review_merge_scheduler.pyclean.🤖 Generated with Claude Code