Repository navigation
feat(OMN-14865): wire required-check skip-vector guard (fan-out to omnibase_infra) - #2369
Conversation
…nibase_infra) Fans out the PROVEN required-check skip-vector guard (OMN-14854 omniclaude canary, PR #1917 merged to omniclaude dev) into omnibase_infra, mirroring the canary pattern exactly: - required-check-skip-guard-caller.yml: local caller wired into every pull_request/merge_group targeting main/dev, no paths filter (design spec 4c anti-self-wedge). Calls omniclaude's reusable workflow pinned to the commit that introduced it (4d60e9363728eed52a238cd5b47bb15808d51ca0 -- origin/dev HEAD at fan-out time, not yet promoted to omniclaude main, hence the SHA pin rather than @main, matching this repo's existing pinned-ref convention for every other omniclaude-hosted reusable it calls). - .github/required-checks.yaml: regenerated to schema v3, exhaustive against the live 17 required_status_checks.contexts on omnibase_infra's dev branch (verified via gh api), plus 1 ADVISORY row for the guard's own new context. Supersedes the prior v1 prose manifest (documented `test.yml`, a workflow that no longer exists, and was never read by any script -- scripts/audit-branch-protection.py fetches required_status_checks live via gh api; the separate parity ratchet in scripts/enforcement_parity_manifest.yaml is untouched). - .pre-commit-hooks/reject-required-check-skip-vector.sh + .pre-commit-config.yaml entry: local pre-commit parity, resolving the canonical validator from the OMNI_HOME sibling omniclaude clone (mirrors the established omnibase_infra pattern in scripts/ci/check_deploy_scope_dod.py for shifting a hosted cross-repo gate left -- DRY, not a re-implementation). Verified locally: the validator (validate_no_required_check_skip_vectors.py) runs clean against every REQUIRED row in the new manifest -- zero path-scoped triggers, zero ungated job-level/caller if:, zero missing pull_request/ merge_group triggers across all 17 live contexts. Full pre-commit suite green. Scope: this PR makes the guard RUN on omnibase_infra (advisory, every PR) but does NOT flip branch protection. mode: ADVISORY stays until a follow-up PR adds required-check-skip-guard/check-skip-vectors to required_status_checks, gated on >=2 independent green runs on dev (same ordering as the canary). Closes OMN-14865
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 16 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 (1)
📝 WalkthroughWalkthroughThe pull request replaces the required-check manifest with schema v3 gates, adds a GitHub Actions caller for skip-vector validation, and registers a fail-closed pre-commit hook that runs the canonical validator locally. ChangesRequired-check skip-vector guard
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequestOrMergeGroup
participant RequiredCheckSkipGuardCaller
participant OccPreflight
participant RequiredCheckSkipGuard
PullRequestOrMergeGroup->>RequiredCheckSkipGuardCaller: trigger workflow
RequiredCheckSkipGuardCaller->>OccPreflight: run pinned preflight workflow
OccPreflight-->>RequiredCheckSkipGuardCaller: complete preflight
RequiredCheckSkipGuardCaller->>RequiredCheckSkipGuard: run dependent guard workflow
RequiredCheckSkipGuard-->>PullRequestOrMergeGroup: publish guard status
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.pre-commit-hooks/reject-required-check-skip-vector.sh (1)
15-17: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winKeep the local validator aligned with CI.
The local hook uses the mutable
$OMNI_HOME/omniclaudecheckout, while CI uses the pinned revision from Line [56]. Therefore, the “identical file” guarantee is not enforced and local/CI verdicts can diverge. Pin the local validator or verify that its file content matches the CI-pinned revision before execution.Also applies to: 23-26
🤖 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 @.pre-commit-hooks/reject-required-check-skip-vector.sh around lines 15 - 17, Update the local validator execution in reject-required-check-skip-vector.sh to use the same pinned revision as CI, or verify the mutable $OMNI_HOME/omniclaude validator content matches that pinned revision before running it. Preserve the existing validation behavior while enforcing that local and CI execute equivalent file content.
🤖 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/required-check-skip-guard-caller.yml:
- Around line 54-56: Update the required-check-skip-guard job that depends on
occ-preflight to run regardless of that dependency’s result by adding an
always() job condition, while preserving its existing needs and reusable
workflow configuration.
---
Nitpick comments:
In @.pre-commit-hooks/reject-required-check-skip-vector.sh:
- Around line 15-17: Update the local validator execution in
reject-required-check-skip-vector.sh to use the same pinned revision as CI, or
verify the mutable $OMNI_HOME/omniclaude validator content matches that pinned
revision before running it. Preserve the existing validation behavior while
enforcing that local and CI execute equivalent file content.
🪄 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: ab13b00e-40a0-44f8-a7a3-f3941b47640e
📒 Files selected for processing (4)
.github/required-checks.yaml.github/workflows/required-check-skip-guard-caller.yml.pre-commit-config.yaml.pre-commit-hooks/reject-required-check-skip-vector.sh
Summary
Fans out the PROVEN required-check skip-vector guard (OMN-14854 omniclaude canary, PR OmniNode-ai/omniclaude#1917, merged to omniclaude
dev) intoomnibase_infra, following the canary pattern exactly..github/workflows/required-check-skip-guard-caller.yml— local caller wired into everypull_request/merge_grouptargetingmain/dev. Deliberately nopaths:filter (design spec §4c anti-self-wedge — this is the one gate that must never be dodgeable by a path filter, since catching path-filter dodges is its job). Calls omniclaude's reusable workflow (required-check-skip-guard-reusable.yml) pinned to the commit that introduced it —4d60e9363728eed52a238cd5b47bb15808d51ca0(origin/dev HEAD at fan-out time; not yet promoted to omniclaudemain, hence a SHA pin rather than@main— this also matches omnibase_infra's existing convention of pinning every other omniclaude-hosted reusable it calls, e.g.deploy-gate-reusable.yml@ff230264...,reject-deploy-gate-skip.yml@ff230264...)..github/required-checks.yaml— regenerated to schema v3, generated FROM the liverequired_status_checks.contextson omnibase_infra'sdevbranch (gh api repos/OmniNode-ai/omnibase_infra/branches/dev/protection/required_status_checks --jq '.contexts', 17 contexts, verified 2026-07-20), each row mapped to its producing job, plus 1ADVISORYrow for the guard's own new context. This supersedes the prior v1 prose-only manifest, which documentedtest.yml(a workflow that no longer exists — CI isci.ymlnow) and was never read by any script (scripts/audit-branch-protection.pyfetchesrequired_status_checkslive viagh api; the separate parity ratchetscripts/enforcement_parity_manifest.yaml, OMN-14288, is its own machine-asserted file and is untouched by this change)..pre-commit-hooks/reject-required-check-skip-vector.sh+.pre-commit-config.yamlentry — local pre-commit parity (CLAUDE.md Rule feat: Complete Phase 2 infrastructure migration to ONEX nodes #5: enforcement ships with detection in the same PR). Resolves the canonical validator from theOMNI_HOMEsiblingomniclaudeclone, mirroring the established omnibase_infra pattern for shifting a hosted cross-repo gate left (scripts/ci/check_deploy_scope_dod.pydoes the same fordeploy-gate) — DRY, not a re-implementation; local and CI verdicts run the identical file.Verified locally (both the omniclaude validator directly, and the new pre-commit hook):
validate_no_required_check_skip_vectors.pyruns clean against everyREQUIREDrow in the new manifest — zero path-scoped triggers, zero ungated job-level/callerif:, zero missingpull_request/merge_grouptriggers, across all 17 live contexts. The manifest's 17REQUIREDnames are an exact 1:1 match against the livedevbranch-protection context list.Scope — explicitly NOT done in this PR
Per the canary's own ordering (design spec §5): branch protection on
devis NOT modified here. The new context (required-check-skip-guard / check-skip-vectors) runs on every PR/merge_group (mode:ADVISORYin the manifest) but is not added torequired_status_checks. Flipping it toREQUIREDis a follow-up, gated on ≥2 independent green runs of the new check ondev— tracked as a follow-up item under OMN-14865.Ticket
Closes OMN-14865 (https://linear.app/omninode/issue/OMN-14865)
Test plan
validate_no_required_check_skip_vectors.py --manifest .github/required-checks.yaml --workflows-dir .github/workflows→PASS: no required-check skip vectors found.gh api .../branches/dev/protection/required_status_checks --jq '.contexts'actionlintclean on the new workflow fileshellcheckclean on the new pre-commit hook scriptpre-commit run --all-files(scoped to changed files) green, including the newreject-required-check-skip-vectorhookgh pr checksgreen post-push (verifying next)Summary by CodeRabbit
New Features
devbranch.Bug Fixes
Evidence-Ticket: OMN-14865
Evidence-Source: OCC#4519