Repository navigation
feat(OMN-15731): pilot label-gated CI (ci:ready) on onex_change_control - #6216
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 52 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 (7)
Comment |
Operator-authorized scoped pilot (2026-08-08): the `pre-commit` job -- the one CI job that runs unconditionally on every dev-targeting PR today -- is gated behind the `ci:ready` label. Unlabeled dev PRs remain unmergeable: the sole required context `CI Summary` reports FAILURE via a new explicit strict block, never green-by-skip. Rebased onto dev to pick up #6214's merged OMN-15731 contract (occ-preflight was BLOCKing with missing_contract -- this branch predated that merge and carried no contracts/OMN-15731.yaml at all). Adds three net-new dod_evidence items + PASS receipts, independently binding THIS PR (#6216): - dod-omn15731-fail-closed-proof-run-31277189469: live gh-api read of two job conclusions from CI run 31277189469 (this PR's own original unlabeled commit) -- Pre-commit=skipped, CI Summary=failure -- the pilot's AC(b) fail-closed proof read from the GitHub Actions run record. - dod-OmniNode-ai-onex_change_control-pr-6216: content-pinned probe against the test file this PR adds. - occ-self-bind-pr-6216: #6214's companion bound to a different PR (omnibase_infra#2693) and #6219's companion is CONFLICTING with dev (predates the #6214 merge), so neither supplies a receipt whose pr_number binds to #6216 itself; this self-bind does. Evidence-Ticket: OMN-15731
3215d72 to
cee8f2c
Compare
…l-gated CI pilot #6216 (OMN-15731 label-gated ci:ready pilot on pre-commit) landed on dev between this PR's original push and now, touching the same ci-summary strict-block region of ci.yml as this fix's test/type-check strict block. Both blocks are independent, additive fail-closed mechanisms for different jobs — kept both. Also corrected a stale comment on the pre-commit job's `if:` (added by #6216) that claimed test/type-check "already skip on PRs targeting dev by design" — that claim was exactly the bug this ticket fixes; test/type-check no longer carry that carve-out. Verified non-vacuous post-merge: all 23 tests across both OMN-15731 test files (this PR's + #6216's) pass against the merged ci.yml. Full local suite: 5938 passed / 47 skipped / 0 failed. mypy src/ tests/: clean, 398 files.
Same class as OMN-15755 (test/type-check): schema-purity, no-localhost-fallbacks, imperative-contract-guard, context-integrity-contracts, migration-conflicts, migration-inventory, kafka-boundary-parity, cross-repo-null-contract, seam-contract-coverage, ai-slop-check, no-new-os-environ, and url-authority-gate all carried an undocumented `github.base_ref != 'dev'` clause on top of their legitimate docs_only (OMN-14098) fast-lane guard. Since dev is this repo's default branch, all 12 silently skipped on essentially every ordinary dev PR. 9 of the 12 sat in ci-summary's needs guarded only by the generic failure/cancelled rollup (which doesn't catch skipped); the other 3 (ai-slop-check, no-new-os-environ, url-authority-gate) weren't in needs at all. Reviewed every job's steps for a genuine main-only rationale before touching it -- found none; all 12 operate on PR-diff or full-repo state equally validly on dev as on main. No job is documented as intentionally dev-excluded. Fix: remove the base_ref clause from all 12, add the 3 missing jobs to ci-summary's needs, and add a strict success-only fail-closed block in ci-summary for all 12 (docs_only-gated, mirroring the OMN-15755/OMN-15731 pattern). New rendered-bash regression test (tests/unit/scripts/test_ci_skip_hole_12_jobs_omn15768.py) proves non-vacuity: 50/52 cases fail against the pre-fix file, 52/52 pass at head. Same ci.yml region OMN-15755 (#6221/#6226) and OMN-15731 (#6216) both touch -- rebase/merge onto dev after those land to avoid a repeat of the #6221/#6216 conflict resolved this session. Full local suite: 5977 passed / 47 skipped / 0 failed. mypy src/ tests/: clean, 398 files. Cites OMN-15768.
Summary
Operator-authorized scoped pilot (2026-08-08 go-ahead recorded on OMN-15731): the
pre-commitjob -- the one CI job that runs unconditionally on every dev-targeting PR today -- is gated behind theci:readylabel. Unlabeled dev-targeting PRs remain unmergeable:CI Summary(the sole required status check ondev) reports FAILURE via a new explicit strict block, never green-by-skip.Ticket: OMN-15731 (design doc + pilot-authorization addendum). Scope: omnibase_infra + onex_change_control only — this PR is the OCC half. No other repo touched.
Why
pre-commit, nottest(the ticket's literal original ask)Live-reading
ci.ymlsurfaced a shape the design doc didn't anticipate:testandtype-checkalready skip entirely on dev-targeting PRs today (if: ... && (github.event_name != 'pull_request' || github.base_ref != 'dev')—smart-test-selectwas meant to replace them there and is declared but never consumed/wired). Consequences:test/type-checkfurther would add zero incremental runner-cost savings on the everyday dev-PR path — they already don't run there.main-targeting PRs would weaken the unconditional full-suite guarantee rootCLAUDE.mdrule M1: Implement Pydantic schema models v1 (Day Close + Ticket Contract) #4 requires at thedev→mainpromotion boundary — a real regression, not a pilot win.pre-commit(if: always()before this PR) is the job that actually costs CI time on every single dev PR, so it is the one gated — and only for dev-targeting PRs.push/merge_group/main-or-hotfix-targeting PRs are untouched (always()still holds for them), so themainpromotion boundary — wherePre-commitis a REQUIRED branch-protection context — keeps running it unconditionally exactly as before.What changed and why it's fail-closed
ci.ymlpre-commitjob:if: always() && (github.event_name != 'pull_request' || github.base_ref != 'dev' || contains(github.event.pull_request.labels.*.name, 'ci:ready')).ci-summary: new explicit strict success-only block forpre-commit(mirrors the existing OMN-15484/OMN-15669 pattern used formerge-hold-gate/contract-shape-v1) —skipped/cancelled/failureALL block. Without this, the genericcontains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled')rollup treats a label-skip as pass — the exact AC(b) trap named on OMN-15731.ci.ymlpull_request.typesextended withlabeled/unlabeled/ready_for_reviewso applying/removing the label retriggers evaluation without a new push..github/required-checks.yaml: the"Pre-commit"gate'sskip_semanticsflippednever→neutral_okwith a rationale citing this ticket — satisfiesrequired-check-skip-guard's (OMN-15241/OMN-14865) own sanctioned escape hatch (the hook flagged bothvector-2-ungated-job-ifandvector-5-ungated-needs-cascadeon first attempt — this is the fix, not a bypass). The row'sbranch: mainscope is unchanged: main-targeting PRs are unaffected by the label gate;devenforcement moves to the newci-summarystrict block.gh label create ci:ready(idempotent, already applied to the repo).Operator workflow
devas normal. Cheap checks (zone-filter, most standalone gates) run immediately;pre-commit(and henceCI Summary) reads red/pending until labeled.ci:readyto trigger the full pre-commit hook suite.CI Summaryreports its normal pass/fail verdict once the labeled run completes.main(dev→main promotion) are unaffected by this pilot —pre-commitalways runs there, unconditionally, exactly as before.Testing
uv run pytest tests/unit/scripts/test_label_gated_ci_pilot_omn15731.py -q→ 10 passed. Renders and executes the realci-summaryshell block (same technique astest_merge_hold_gate_wiring_omn15484.py): provessuccesspasses,skipped/failure/cancelledall fail closed, removing the strict check restores the pre-pilot bypass (RED-before control), the failure message names both the job and OMN-15731, and a control assertingtest/type-checkare untouched by this pilot.uv run mypy tests/unit/scripts/test_label_gated_ci_pilot_omn15731.py --strict→ clean.bash .pre-commit-hooks/reject-required-check-skip-vector.sh→PASS: no required-check skip vectors found.(both vectors it flagged pre-fix are now resolved via therequired-checks.yamlchange above).pre-commit run --all-filesvia the commit hook chain — all hooks passed.Acceptance criteria mapping (OMN-15731)
pre-commiton unlabeled dev-PR push — job-levelif:means it is never scheduled; live runner-telemetry verification is a follow-up once this PR is live.ci-summarystrict block is the load-bearing mechanism, proven by executing the real script against all four possible job results.if:evaluates true once labeled.required-check-skip-guardstays green — verified locally, see Testing above.ci:hold→ci:readylabel flipping — not in scope for this PR (design item 4; needspull-requests:writewiring, flagged as an open risk on the design comment).Risks / follow-ups (carried from the design + this PR's own findings)
test/type-checkalready skip on dev PRs for an unrelated, pre-existing reason (smart-test-selectdeclared but unwired) — this PR does not fix that gap; it is out of scope for OMN-15731 and flagged here for visibility, not silently absorbed into this pilot's claims.Evidence-Ticket: OMN-15731
Evidence-Source: OCC#6219