Repository navigation
ci(OMN-14104): gate CI fanout behind OCC preflight - #2229
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 45 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. 📝 WalkthroughWalkthroughThis PR adds a centralized ChangesOCC Preflight Gate Rollout
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 |
e79bd09 to
68d22b2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/dep-provenance-gate.yml (1)
43-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftOcc-preflight block duplicated verbatim across ~40 workflows.
The same
uses:reference, pinned SHA, permissions, andwith:inputs are copy-pasted into every gated workflow. Since a dedicated pinned wrapper (call-occ-preflight.yml) already exists per the stack's Layer 1, routing all callers through it instead of duplicating the reusable-workflow reference would centralize future pin rotations to a single file.🤖 Prompt for 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. In @.github/workflows/dep-provenance-gate.yml around lines 43 - 52, The occ-preflight configuration is duplicated here instead of using the existing pinned wrapper, so update the workflow to call call-occ-preflight.yml rather than repeating the OmniNode-ai/omnibase_core reusable-workflow reference, permissions, and inputs. Keep the same contracts-dir and receipts-dir settings, but route through the wrapper so pin rotations and future changes are centralized in one place..github/workflows/docker-build.yml (1)
46-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftCentralize the occ-preflight pin behind the local wrapper — The wiring here is fine, but this external pin is duplicated across the rollout; have per-workflow jobs call
./.github/workflows/call-occ-preflight.ymlso future pin bumps touch one file instead of many.🤖 Prompt for 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. In @.github/workflows/docker-build.yml around lines 46 - 61, The occ-preflight reusable workflow is pinned directly here, which duplicates the version across workflows and makes bumps harder to manage. Update the docker-build workflow to invoke the local wrapper job that calls ./.github/workflows/call-occ-preflight.yml instead of referencing OmniNode-ai/omnibase_core/.github/workflows/occ-preflight.yml directly, so the pin is centralized in one place.
🤖 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 56-64: The occ-preflight gating in the CI workflow should not skip
the required job when preflight fails. In the CI workflow’s CI Summary job, keep
the job-level condition as always() and move the occ-preflight result handling
into the job steps so the job can fail explicitly when
needs.occ-preflight.result is not success. Use the existing occ-preflight
dependency and CI Summary job names to locate the logic, and ensure the required
branch-protection check remains red instead of skipped when preflight is
unhealthy.
---
Nitpick comments:
In @.github/workflows/dep-provenance-gate.yml:
- Around line 43-52: The occ-preflight configuration is duplicated here instead
of using the existing pinned wrapper, so update the workflow to call
call-occ-preflight.yml rather than repeating the OmniNode-ai/omnibase_core
reusable-workflow reference, permissions, and inputs. Keep the same
contracts-dir and receipts-dir settings, but route through the wrapper so pin
rotations and future changes are centralized in one place.
In @.github/workflows/docker-build.yml:
- Around line 46-61: The occ-preflight reusable workflow is pinned directly
here, which duplicates the version across workflows and makes bumps harder to
manage. Update the docker-build workflow to invoke the local wrapper job that
calls ./.github/workflows/call-occ-preflight.yml instead of referencing
OmniNode-ai/omnibase_core/.github/workflows/occ-preflight.yml directly, so the
pin is centralized in one place.
🪄 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: 1a0fba1c-f966-49ac-ad31-efb3f9a047d8
📒 Files selected for processing (43)
.github/workflows/artifact-reconciliation-webhook.yml.github/workflows/auto-merge.yml.github/workflows/call-occ-preflight.yml.github/workflows/call-receipt-gate.yml.github/workflows/call-reject-skip.yml.github/workflows/canonical-inference-gate.yml.github/workflows/check-handshake.yml.github/workflows/check-sibling-compat.yml.github/workflows/ci.yml.github/workflows/compose-config-lint.yml.github/workflows/contract-validation.yml.github/workflows/cr-thread-gate-caller.yml.github/workflows/cr-thread-gate.yml.github/workflows/dep-provenance-gate.yml.github/workflows/deploy-gate.yml.github/workflows/dispatch-parity-gate.yml.github/workflows/dispatcher-route-coverage.yml.github/workflows/docker-build.yml.github/workflows/docs-validate.yml.github/workflows/env-parity.yml.github/workflows/fresh-deploy-fitness.yml.github/workflows/imperative-contract-guard.yml.github/workflows/integration-test-check.yml.github/workflows/lane-census-staleness.yml.github/workflows/main-target-guard.yml.github/workflows/no-bare-compose-teardown.yml.github/workflows/node-migration-sync.yml.github/workflows/non-dev-base-guard.yml.github/workflows/omni-standards-compliance.yml.github/workflows/omnigate.yml.github/workflows/pr-title-check.yml.github/workflows/prod-promotion-lineage.yml.github/workflows/receipt-honesty.yml.github/workflows/runner-disk-preflight.yml.github/workflows/runner-image-build-smoke.yml.github/workflows/runner-routing-audit.yml.github/workflows/security-scan.yml.github/workflows/seed-provenance-check.yml.github/workflows/skill-node-mapping-sync.yml.github/workflows/stale-todo-gate.yml.github/workflows/url-authority-gate.yml.github/workflows/validate-validator-requirements.yml.github/workflows/validator-runtime-profiles.yml
05675e6 to
7fa7e35
Compare
CodeRabbit flagged that gating the required CI Summary aggregate with `if: needs.occ-preflight.result == 'success' && (always())` turns a RED occ-preflight into a SKIPPED CI Summary. A skipped required branch-protection check reports as success, so a failed OCC preflight could be merged. Keep `if: always()` and enforce OCC eligibility as the first step, failing explicitly when preflight did not succeed so the required check stays RED. CI Summary is the single umbrella required gate (OMN-4497, .github/required-checks.yaml), so a red CI Summary blocks the merge even though the runner-heavy fanout is (intentionally) skipped on preflight failure.
The 06:58Z run batch on the prior push hit a transient GitHub Actions startup_failure across all workflows (zero jobs, name==path, no diff cause — untouched workflows failed too). Infra recovered by 07:01Z (sibling branch runs queuing/running normally). startup_failure runs are non-retriable, so this empty commit re-triggers a fresh CI cycle. No code change.
…dev SHA The 43 workflow callers pinned the occ-preflight reusable (and its core-ref input) to 7975803283 — a commit on the omnibase_core OMN-14104 PR branch (#1396, 'ci: use dev for OCC preflight compat checkout'). That PR squash-merged to omnibase_core dev (8a47e092), deleting the branch and ORPHANING 7975803283. GitHub Actions can no longer resolve the reusable at an unreachable SHA, so every caller on this branch hit startup_failure ('workflow file issue', zero jobs) — all 43 workflows red. Re-pin all 86 references (43 uses:@ + 43 core-ref) to 8a47e092002dd1338599fa0733feda469436acbf, the merged commit that last defined occ-preflight.yml on omnibase_core dev (ancestor of dev HEAD, reachable). Input interface is identical (contracts-dir/receipts-dir/core-ref, all optional) — verified against both SHAs. Also rebased onto origin/dev to pick up the 4 missing commits (incl. OMN-14130 core 0.46.5 bump for the grandfather validator).
7fa7e35 to
2009694
Compare
Superseded by #2230 (OMN-14127) — recommend closeThis PR (OMN-14104) gates omnibase_infra CI fanout behind an #2230 (OMN-14127) merged to Consequences:
Recommendation: close as superseded by #2230 (operator decision — not auto-closing). Diagnostic captured for the record (root-cause of the all-red state)The original all-red was not flaky CI: every workflow on the branch |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
56-68: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMissing
timeout-minuteson the new gating job.
occ-preflightis now the first dependency for the entire CI fanout, but unlike every other job in this file it has notimeout-minutes. If the reusable workflow hangs, it defaults to GitHub's 360-minute job timeout, stalling the whole pipeline (and every job that depends on it) far longer than any other gate here.⏱️ Suggested fix
occ-preflight: + timeout-minutes: 15 # self-gating-ok: target is in omnibase_core, not a required check on omnibase_infra main uses: OmniNode-ai/omnibase_core/.github/workflows/occ-preflight.yml@8a47e092002dd1338599fa0733feda469436acbf🤖 Prompt for 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. In @.github/workflows/ci.yml around lines 56 - 68, The occ-preflight gating job is missing a timeout, so it can block the entire CI fanout for far too long if the reusable workflow hangs. Add a timeout-minutes setting to occ-preflight in the CI workflow, matching the pattern used by the other jobs in this file, so the gate fails fast instead of waiting for GitHub’s default timeout.
958-961: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
ifonly checksocc-preflight/zone-filter, not the other listed needs.Adding a custom
ifhere replaces the implicitsuccess()check that would otherwise require every job inneedsto succeed. Sincetest-parallel'sifonly checksocc-preflight.resultandzone-filter.outputs.docs_only, a failure in e.g.arch-invariantsorschema-handshakewould not preventtest-parallelfrom running (it would still wait for those jobs to complete, but not to succeed). This is pre-existing behavior unrelated to this PR's OCC gating, and merge-blocking is still enforced independently viaci-summary's strict gate checks, so it isn't a functional regression — just worth being aware of.🤖 Prompt for 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. In @.github/workflows/ci.yml around lines 958 - 961, The `test-parallel` job’s custom `if` in `.github/workflows/ci.yml` overrides the default success check for all listed `needs`, so it can run even if jobs like `arch-invariants` or `schema-handshake` fail. Update the `test-parallel` condition to preserve the implicit all-needs-success behavior while still honoring the `occ-preflight` and `zone-filter` gating, using the existing `test-parallel` job definition and its `needs` list as the reference point.
🤖 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.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 56-68: The occ-preflight gating job is missing a timeout, so it
can block the entire CI fanout for far too long if the reusable workflow hangs.
Add a timeout-minutes setting to occ-preflight in the CI workflow, matching the
pattern used by the other jobs in this file, so the gate fails fast instead of
waiting for GitHub’s default timeout.
- Around line 958-961: The `test-parallel` job’s custom `if` in
`.github/workflows/ci.yml` overrides the default success check for all listed
`needs`, so it can run even if jobs like `arch-invariants` or `schema-handshake`
fail. Update the `test-parallel` condition to preserve the implicit
all-needs-success behavior while still honoring the `occ-preflight` and
`zone-filter` gating, using the existing `test-parallel` job definition and its
`needs` list as the reference point.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 061da79b-315b-4ecd-acd6-a866997de1d5
📒 Files selected for processing (44)
.github/workflows/artifact-reconciliation-webhook.yml.github/workflows/auto-merge.yml.github/workflows/call-occ-preflight.yml.github/workflows/call-receipt-gate.yml.github/workflows/call-reject-skip.yml.github/workflows/canonical-inference-gate.yml.github/workflows/check-handshake.yml.github/workflows/check-sibling-compat.yml.github/workflows/ci.yml.github/workflows/compose-config-lint.yml.github/workflows/contract-validation.yml.github/workflows/cr-thread-gate-caller.yml.github/workflows/cr-thread-gate.yml.github/workflows/dep-provenance-gate.yml.github/workflows/deploy-gate.yml.github/workflows/dispatch-parity-gate.yml.github/workflows/dispatcher-route-coverage.yml.github/workflows/docker-build.yml.github/workflows/docs-validate.yml.github/workflows/env-parity.yml.github/workflows/fresh-deploy-fitness.yml.github/workflows/imperative-contract-guard.yml.github/workflows/integration-test-check.yml.github/workflows/lane-census-staleness.yml.github/workflows/main-target-guard.yml.github/workflows/no-bare-compose-teardown.yml.github/workflows/node-migration-sync.yml.github/workflows/non-dev-base-guard.yml.github/workflows/omni-standards-compliance.yml.github/workflows/omnigate.yml.github/workflows/pr-title-check.yml.github/workflows/prod-promotion-lineage.yml.github/workflows/receipt-honesty.yml.github/workflows/runner-disk-preflight.yml.github/workflows/runner-image-build-smoke.yml.github/workflows/runner-routing-audit.yml.github/workflows/security-scan.yml.github/workflows/seed-provenance-check.yml.github/workflows/skill-node-mapping-sync.yml.github/workflows/stale-todo-gate.yml.github/workflows/url-authority-gate.yml.github/workflows/validate-validator-requirements.yml.github/workflows/validator-runtime-profiles.ymlscripts/ci/ci_summary_gate.py
🚧 Files skipped from review as they are similar to previous changes (42)
- .github/workflows/compose-config-lint.yml
- .github/workflows/dep-provenance-gate.yml
- .github/workflows/node-migration-sync.yml
- .github/workflows/prod-promotion-lineage.yml
- .github/workflows/pr-title-check.yml
- .github/workflows/security-scan.yml
- .github/workflows/call-receipt-gate.yml
- .github/workflows/no-bare-compose-teardown.yml
- .github/workflows/contract-validation.yml
- .github/workflows/integration-test-check.yml
- .github/workflows/skill-node-mapping-sync.yml
- .github/workflows/lane-census-staleness.yml
- .github/workflows/validator-runtime-profiles.yml
- .github/workflows/dispatch-parity-gate.yml
- .github/workflows/artifact-reconciliation-webhook.yml
- .github/workflows/seed-provenance-check.yml
- .github/workflows/imperative-contract-guard.yml
- .github/workflows/check-sibling-compat.yml
- .github/workflows/omnigate.yml
- .github/workflows/cr-thread-gate.yml
- .github/workflows/call-occ-preflight.yml
- .github/workflows/deploy-gate.yml
- .github/workflows/receipt-honesty.yml
- .github/workflows/stale-todo-gate.yml
- .github/workflows/runner-image-build-smoke.yml
- .github/workflows/docs-validate.yml
- .github/workflows/runner-routing-audit.yml
- .github/workflows/auto-merge.yml
- .github/workflows/cr-thread-gate-caller.yml
- .github/workflows/main-target-guard.yml
- .github/workflows/non-dev-base-guard.yml
- .github/workflows/env-parity.yml
- .github/workflows/omni-standards-compliance.yml
- .github/workflows/dispatcher-route-coverage.yml
- .github/workflows/canonical-inference-gate.yml
- .github/workflows/fresh-deploy-fitness.yml
- .github/workflows/call-reject-skip.yml
- .github/workflows/url-authority-gate.yml
- .github/workflows/check-handshake.yml
- .github/workflows/runner-disk-preflight.yml
- .github/workflows/docker-build.yml
- .github/workflows/validate-validator-requirements.yml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/ci/test_ci_workflow_resilience.py (1)
784-789: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAssumes every
usesjob in these workflows isocc-preflight.The branch treats any job with a
useskey as the occ-preflight caller and requires it to match the pinned SHA suffix. If a future job in one of these three workflows legitimately calls a different reusable workflow, this assertion will fail for the wrong reason. Scoping the check to theocc-preflightjob name would keep the test focused and avoid unrelated false failures.♻️ Suggested tightening
for job_name, job in workflow["jobs"].items(): - if "uses" in job: + if job_name == "occ-preflight" and "uses" in job: assert job["uses"].endswith( "occ-preflight.yml@8a47e092002dd1338599fa0733feda469436acbf" ) continue🤖 Prompt for 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. In `@tests/ci/test_ci_workflow_resilience.py` around lines 784 - 789, The workflow resilience test is too broad because it assumes every job with a `uses` key is the `occ-preflight` reusable workflow caller. Tighten the assertion inside the workflow job loop in `test_ci_workflow_resilience` so it only validates the job named `occ-preflight`, and leave other reusable workflow jobs unverified to avoid false failures. Keep the pinned SHA check on that specific job’s `uses` value only.
🤖 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.
Nitpick comments:
In `@tests/ci/test_ci_workflow_resilience.py`:
- Around line 784-789: The workflow resilience test is too broad because it
assumes every job with a `uses` key is the `occ-preflight` reusable workflow
caller. Tighten the assertion inside the workflow job loop in
`test_ci_workflow_resilience` so it only validates the job named
`occ-preflight`, and leave other reusable workflow jobs unverified to avoid
false failures. Keep the pinned SHA check on that specific job’s `uses` value
only.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2051b940-63bf-4744-9f2b-1862ad121282
📒 Files selected for processing (2)
tests/ci/test_ci_workflow_resilience.pytests/fixtures/dispatch_parity/baseline-selection-v2.json
✅ Files skipped from review due to trivial changes (1)
- tests/fixtures/dispatch_parity/baseline-selection-v2.json
Summary
Gate omnibase_infra CI fanout behind OCC preflight so OCC eligibility fails before runner-heavy validation starts.
Changes
occ-preflightas the first CI dependency.always()aggregators so they do not run after OCC failure.Test plan
.github/workflows/ci.yml.github/workflows/ci.ymlRuntime startup gate
Type safety checklist
metadata["key"]ormetadata.get("key")string literal access on Pydantic model fieldsmetadata: dict[str, Any]fields without TypedDict or# ONEX_EXCLUDE:commentexcept Exception— must use narrowed type, or minimal-scope boundary withlogger.exception(...)+ degrade comment, or typed wrap/re-raisedocker-compose.infra.ymlwith required (:?) env vars, updatetests/integration/docker/test_docker_integration.pyfixture dict intest_compose_config_validStrict-gate ordering
Related issues
OMN-14104
Evidence-Ticket: OMN-14104
Evidence-Source: OCC#3674
Summary by CodeRabbit