Repository navigation
ci(OMN-14127): run required CI Summary context as a no-needs GitHub-hosted fail-closed poller so it never wedges under self-hosted saturation - #2230
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe ChangesCI Summary Fail-Closed Poller
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…ns the OMN-13888 grandfather validator (#2231) * fix(OMN-14130): bump omnibase-core 0.46.3->0.46.5 (OMN-13888 OCC grandfather validator) omnibase_infra pinned omnibase-core==0.46.3, which predates OMN-13888 (per-entry OCC receipt hashing + append-only + supersession + dual-accept / grandfather, shipped in 0.46.5). occ-preflight installs the caller repo's pinned core version, so infra PRs were evaluating OCC eligibility with the pre-grandfather 0.46.3 validator. That validator re-applies the old 'all receipts must equal the current whole-file contract hash' rule and rejects any PR whose ticket contract received an append after merge (e.g. OMN-14127 shared between omniclaude #1870 and infra #2230 → contract_hash_mismatch on the legacy dod-omniclaude-pr-1870 receipt). Bump to ==0.46.5 (both dependency groups) + regenerate uv.lock + resync _FALLBACK_MATRIX in version_compatibility.py. Verified clean on 0.46.5: 25,453 tests collect with zero import/API breaks; full unit suite 21,076 passed (0 real failures); test_version_compatibility 25 passed. Unblocks OMN-14127 (#2230) once merged + rebased. See also OMN-14131 (harden occ-preflight so the built main wheel wins over a CWD pin). * fix(OMN-14130): refresh runner identity for core pin bump
…ed poller Port the proven omniclaude #1870 wedge fix to omnibase_infra. The required branch-protection context "CI Summary" was a needs-gated aggregator over ~20 self-hosted jobs; under fleet saturation those jobs never terminalized, so the required check-run was ABSENT (not failing/pending) and PRs wedged BLOCKED with no auto-recovery. - ci-summary is now a NO-needs, runs-on: ubuntu-latest, if: always(), fail-closed bounded-deadline poller. Its check-run instantiates immediately (the required context can never be absent) and it polls the current run's job conclusions via the GitHub jobs API. Zero self-hosted/LAN dependency. Name 'CI Summary' preserved byte-for-byte (rename would re-wedge every PR). - scripts/ci/ci_summary_gate.py: default-deny, fail-closed verdict (exit 0/1/2). Reproduces infra's exact original semantics (13 strict == success gates + 4 success||skipped gates as the completeness anchor) and adds a strictly-stronger default-deny sweep that catches detect-changes / plugin-env-service-completeness / compose-required-env-coverage / contract-path-preflight failures the old tests-gate greened on 'skipped'. Small soft-allowlist of genuinely-non-gating jobs (advisory / not-required), prefix-aware for reusable-workflow callers. - tests/ci/test_ci_summary_gate.py: 18 unit tests pinning the fail-closed verdict (strict-skip=fail, sweep catches leaves, allowlist ignored, empty run != vacuous green, docs-only success). - Removes the (unset, no-op) OMNI_REQUIRED_CI_RUNS_ON_JSON merge_group override from the ci-summary runs-on selector; the poller is runner-independent.
22e9695 to
1ec4807
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 1547-1570: The polling loop in the CI workflow bypasses the
deadline when the gh api fetch fails because the retry path hits continue before
the deadline check. Update the polling logic in the jobs-fetch loop so the
deadline is evaluated on every iteration before attempting gh api, ensuring
persistent API failures still fail closed within the bounded window; keep the
existing terminal-state handling in the ci_summary_gate.py polling flow intact.
In `@scripts/ci/ci_summary_gate.py`:
- Around line 166-175: The deduplication in dedup_latest() is still overwriting
same-name jobs when run_attempt matches, which can hide a failure behind a later
success. Update the JobState selection logic to keep the worst state for each
name within the same attempt rather than blindly replacing the previous entry,
while still ignoring older attempts. Add a regression test in ci_summary_gate.py
covering two leaf jobs with the same name in the same run_attempt where one
fails and one succeeds, and assert the gate fails closed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bb3896b1-6cdd-4c00-bfd5-dfd8cbf81716
📒 Files selected for processing (3)
.github/workflows/ci.ymlscripts/ci/ci_summary_gate.pytests/ci/test_ci_summary_gate.py
Summary
Ports the proven omniclaude #1870 wedge fix to
omnibase_infra. The required branch-protection contextCI Summary(the single umbrella gate for this repo, OMN-4497) was aneeds-gated aggregator over ~20 self-hosted jobs. Aneeds-gated job gets no GitHub check-run at all until itsneedsreach a terminal state, so under self-hosted fleet saturation the gate jobs never terminalized andCI Summarywas absent (not failing, not pending) — the PR wedgedBLOCKEDforever with 0 failing / 0 pending checks and no auto-recovery. Same wedge class fixed in omniclaude #1870.Closes OMN-14127 (workflow-structure half; runner-fleet capacity half remains OMN-13932 / OMN-14080).
Fix — no-
needs, GitHub-hosted, fail-closed pollerci-summaryis now a NO-needs,runs-on: ubuntu-latest,if: always(), fail-closed bounded-deadline poller:name: CI Summarypreserved byte-for-byte. A rename would name-mismatch branch protection and re-wedge every PR.DEADLINE_MINUTES: 90,timeout-minutes: 100(proven omniclaude values) — comfortably above thetests-gate30m budget + self-hosted queueing, and under the run-cancel window so the required context always posts terminal first.Gating logic —
scripts/ci/ci_summary_gate.py(exit 0/1/2 = success/fail/pending)Built from infra's ACTUAL
ci.yml(not copied blindly). It reproduces the exact strictness of the old needs-based condition and adds a strictly-stronger safety net:== success) — unconditional inci.yml, never legitimately skip; a skip fails closed (matches old== "success"):CI Tests Gate,Lint,ONEX Validators,Infra Node Handler Ownership,Migration Freeze Check,Fingerprint Check,Demo Loop Gate,Topic Enum Drift Check,Topic Naming Lint,Topic Drift Check,Arch Invariants (OMN-3343),Kafka Schema Handshake (OMN-3411),Writer-Migration Coupling Check.success||skipped) — carry a legitimate skip path (docs-only / event-scoped):Migration Integration Test,Contract Compliance,Contract Compliance Check,Contract Sync Gate (Wave C) [OMN-8915].ci.ymlneeds+ the pass/fail condition + dev branch-protection required contexts):Test-Failure Ratchet Gate(advisory OMN-13867),Version Pin Compliance(inneeds, never checked),Runtime Boot Smoke (compose)(advisory OMN-9120),Cross-Repo Migration Conflicts(not required),Kafka Boundary Compat (OMN-3256)(advisory/xfail-drift),AI-Slop Pattern Check (strict, PR diff),zone-filter. Matching is prefix-aware so reusable-workflow callers (zone-filter / …,Runtime Boot Smoke (compose) / …) are covered.Strictly stronger, not a rubber-stamp: the old
tests-gategreens whentest-parallelisskipped, so a failure indetect-changes/plugin-env-service-completeness/compose-required-env-coverage/contract-path-preflight(all of which skiptest-parallel) used to slip through silently. The default-deny sweep catches them.Dead-var scope correction (evidence, not assumption)
OMNI_REQUIRED_CI_RUNS_ON_JSONis 404 at both repo and org level (gh api …/actions/variables/OMNI_REQUIRED_CI_RUNS_ON_JSON→ Not Found) → a silent no-op today. It is NOT a targetedci-summarymitigation — it is the uniformOMNI_RUNNER_SELECTOR_V1merge_group runner-override on ~28 self-hosted jobs, and it is test-enforced bytest_merge_group_and_docker_workflows_have_runner_pool_overrides(any self-hosted job'sruns-onMUST contain it).This PR removes it from the
ci-summaryjob only (natural, via theubuntu-latestconversion — repo-wide count 29 → 28), which is what supersedes it for the wedge: the required context is now runner-independent. A repo-wide removal is deliberately NOT bundled here — it would break the enforcing test across ~28 jobs and change runner-placement policy for every heavy job, which is a separate decision, not part of the wedge fix.Local verification
python3YAML parse → 30 jobs;ci-summaryhasneeds: absent,runs-on: ubuntu-latest,if: always(),permissions: {actions: read, contents: read}, 2 steps.scripts/ci/ci_summary_gate.pysmoke: full-green →0; empty(saturation) →2; in-progress →2;detect-changesfail →1.uv run pytest tests/ci/test_ci_summary_gate.py→ 18 passed.uv run pytest tests/ci/test_ci_workflow_resilience.py→ 29 passed (incl. the runner-pool-override enforcement, which now correctly skips theubuntu-latestci-summary).tests/ci/{test_branch_protection_audit,test_ci_summary_gate,test_ci_workflow_resilience}.py→ 71 passed.ruff format --check+ruff checkclean;mypy scripts/ci/ci_summary_gate.py tests/ci/test_ci_summary_gate.py→ clean.actionlint .github/workflows/ci.yml→ 5 findings, byte-identical to theorigin/devbaseline (pre-existing shellcheck style/info in untouchedrun:blocks) — zero new findings. The poller's own shell block is clean.pre-commit run --files …→ exit 0, 76 hooks, 0 failures.OCC companion (independent, merged)
omnibase_infrais heavy →verify/occ-preflightrequires anEvidence-Source: OCC#<n>companion inonex_change_control. Per operator policy [[feedback_no_self_authored_evidence]], the implementing agent did NOT author it. The independent companion is OCC #3684 ("evidence(OMN-14127): bind infra CI summary poller", merged), wired below viaEvidence-Source: OCC#3684.verify / verifyis green.occ-preflight / eligibilityis blocked by a separate infra-side dep-pin issue (OMN-14130: infra pinned the pre-OMN-13888omnibase-core==0.46.3, so occ-preflight runs the pre-grandfather validator and rejects the shared-contract append); it clears after OMN-14130 (#2231) merges and this PR is rebased on dev.Evidence-Ticket: OMN-14127
Evidence-Source: OCC#3684
Summary by CodeRabbit
New Features
Bug Fixes