Skip to content

feat(OMN-14854): required-check skip-vector guard (omniclaude canary) - #1917

Merged
jonahgabriel merged 2 commits into
devfrom
jonah/omn-14854-required-check-skip-vector-guard
Jul 20, 2026
Merged

jonahgabriel merged 2 commits into
devfrom
jonah/omn-14854-required-check-skip-vector-guard

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Jul 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Builds the Required-Check Skip-Vector Guard on the omniclaude canary only, per the design spec grounded against live gh api repos/OmniNode-ai/omniclaude/branches/dev/protection/required_status_checks (57 contexts, fetched 2026-07-20). Fails closed on four ways a required GitHub status check can silently stop gating a PR:

  1. A path-scoped pull_request/pull_request_target/merge_group trigger on the producing/caller workflow.
  2. An ungated job-level if: on the job that IS the required context (skipped == passing under branch protection — a silent bypass).
  3. An ungated caller-job if: on a uses: reusable-workflow call (the composed context is never even created — permanent PENDING).
  4. A required context's workflow missing a pull_request/pull_request_target/merge_group trigger entirely.

Ticket: OMN-14854

What shipped

  • .github/actions/required-check-skip-guard/_workflow_model.py — shared context→job resolver (Shape A/B/C per the design spec §1) + symbolic if: classifier.
  • .github/actions/required-check-skip-guard/validate_no_required_check_skip_vectors.py — the PR-time validator. Same script runs under pre-commit and CI (dual-run pattern, mirrors check-daemon-venv-skew.sh).
  • .github/actions/required-check-skip-guard/reconcile_manifest_vs_live.py + .github/workflows/required-check-manifest-reconcile.yml — privileged reconcile (push/schedule/workflow_dispatch only, never pull_request — reading required_status_checks needs Administration:Read, which GITHUB_TOKEN can never carry). Scoped to dev only in this PR: main's required set is verifiably different from dev's (confirmed live), so a per-branch manifest schema is an explicit, flagged-not-resolved follow-up.
  • .github/workflows/required-check-skip-guard-reusable.yml (workflow_call, mirrors deploy-gate-reusable.yml@main) + .github/workflows/required-check-skip-guard-caller.yml (local caller, wired into omniclaude's own PRs now).
  • .github/required-checks.yaml upgraded v2→v3 — now the operational input to the guard (was documentation-only). Exhaustive: all 57 live REQUIRED contexts + 1 ADVISORY row for the guard's own not-yet-required context.
  • .pre-commit-hooks/reject-required-check-skip-vector.sh + .pre-commit-config.yaml entry — same PR as the CI gate, per CLAUDE.md rule 5.
  • Unit tests: tests/ci/test_validate_no_required_check_skip_vectors.py (18 fixture cases) + tests/ci/test_reconcile_manifest_vs_live.py (4 cases) — 22 tests, all synthetic fixtures (never the live repo's own workflow files, so the suite doesn't drift with unrelated CI changes).

Branch protection: explicitly NOT modified

required-check-skip-guard / check-skip-vectors runs on every omniclaude PR starting with this one, but is NOT added to required_status_checks in this PR. Per the design spec's ordering (§5), the flip to REQUIRED is a follow-up that happens only after the check has posted green on dev at least twice — batching "add caller" and "flip to required" in one PR/session step is exactly the self-inflicted-wedge failure mode the ordering exists to prevent. Confirmed no branch-protection API calls were made in this session beyond read-only gh api .../protection/required_status_checks GETs used to build the manifest.

RED → GREEN proof

Synthetic fixture (paths-filter skip vector), captured before any repo code was touched:

=== RED (paths: filter present) ===
FAIL: 1 required-check skip vector(s) found:

  - [vector-1-paths] required context 'Demo Required Gate': producing workflow (demo-gate.yml) has a `paths`/`paths-ignore` filter on its `pull_request` trigger. A PR touching only excluded paths never runs this job, so the required check goes PENDING forever (or is silently absent depending on branch-protection strict mode).

Fix: remove the path filter / job-level if: for these required checks, or register an explicit skip_semantics contract in .github/required-checks.yaml with a cited ticket.
exit=1

=== GREEN (paths: filter removed) ===
PASS: no required-check skip vectors found.
exit=0

Real-repo run: 4 genuine pre-existing findings (not a guard bug)

Running the validator against omniclaude's own live .github/workflows/ + generated manifest surfaced 4 real, previously-invisible skip vectors already in production (not synthetic):

FAIL: 4 required-check skip vector(s) found:
  - [vector-2-ungated-job-if] 'Architecture Handshake': job 'check-handshake' in ci.yml re-checks needs.occ-preflight.result inside an if:
  - [vector-2-ungated-job-if] 'Cross-Repo Boundary Parity': job 'boundary-parity' in ci.yml, same shape
  - [vector-3-ungated-caller-if] 'gate / CodeRabbit Thread Check': caller job 'gate' in cr-thread-gate-caller.yml, actor-based if:
  - [vector-3-ungated-caller-if] 'cr-thread-gate / CodeRabbit Thread Check': caller job 'cr-thread-gate' in cr-thread-check.yml, same shape

These are genuine findings, not false positives — verified by hand against each workflow file. Filed as follow-up OMN-14856 for human triage/remediation (fix scope: build the guard, not blanket-remediate every finding it surfaces on day one). To land this PR without blocking on that unrelated remediation, these 4 manifest rows carry skip_semantics: neutral_ok with a mandatory cited rationale (the escape hatch is rationale-gated — an uncited neutral_ok is treated as never, covered by a dedicated test). skip_semantics: neutral_ok intentionally can never suppress vector-1 (path filter) or vector-4 (missing trigger) findings — only the conditional-if: vectors 2/3, which are documented-exception territory. After the escape hatch, the validator run against the real repo is:

PASS: no required-check skip vectors found.

Manifest ↔ live reconcile sanity

Offline reconcile() check against the exact 57 contexts fetched live from gh api repos/OmniNode-ai/omniclaude/branches/dev/protection/required_status_checks:

missing_from_manifest: set()
stale_in_manifest: set()

Open decisions (flagged, not resolved — per spec's own scope note)

  1. required-check-manifest-reconcile.yml needs a BRANCH_PROTECTION_PAT repo secret (Administration:Read) — GITHUB_TOKEN cannot read required_status_checks (not a configurable permission scope, a hard GitHub platform limitation). The workflow fails with a clear diagnostic, not a silent no-op, until that secret is provisioned. Same pattern as the existing CROSS_REPO_PAT used by cr-thread-gate.yml.
  2. main's required-checks manifest section (branch-specific schema) is a follow-up, not built here.
  3. producer_kind: cross_repo rows (occ-preflight, pr-title) leave the far-side reusable workflow (hosted in omnibase_core/onex_change_control) unaudited by this guard — only the local caller side is checked. Recommend installing the same pattern natively in those repos, independently scoped.
  4. cr-thread-gate-caller.yml and cr-thread-check.yml both call the same reusable and both produce required contexts — consolidation candidate, separate ticket.
  5. OMN-14856 (filed this session) tracks remediation of the 4 real findings above.

Local gates

  • uv run ruff format / uv run ruff check — clean
  • uv run mypy (new files) — clean
  • uv run pytest tests/ci/ -v — 144 passed (22 new + 122 pre-existing, zero regressions)
  • uv run pre-commit run --files <all changed> — all green, including the new reject-required-check-skip-vector hook (confirms the escape-hatch fix actually resolves the local gate against the real repo state)

dod_evidence: OMN-14854. RED/GREEN synthetic proof + real-repo run captured above; gh pr checks verified green post-push.

Evidence-Ticket: OMN-14854
Evidence-Source: OCC#4508

Fail-closed PR-time validator + reusable CI workflow + pre-commit hook
that catch four ways a required GitHub status check can silently stop
gating a PR: a path-scoped pull_request/pull_request_target/merge_group
trigger, an ungated job-level `if:` on the job that IS the required
context (skipped == passing under branch protection), an ungated
caller-job `if:` on a reusable-workflow call (the composed context is
never even created), and a missing pull_request/merge_group trigger
entirely.

- .github/actions/required-check-skip-guard/_workflow_model.py: shared
  context->job resolver (Shape A/B/C) + symbolic if: classifier.
- validate_no_required_check_skip_vectors.py: PR-time validator, run
  by both the pre-commit hook and the reusable CI workflow.
- reconcile_manifest_vs_live.py + required-check-manifest-reconcile.yml:
  privileged (push/schedule/workflow_dispatch only, never pull_request)
  manifest-vs-live-branch-protection reconcile, scoped to `dev` only
  (main's required set is verifiably different; a per-branch manifest
  section is an explicit follow-up, not resolved here).
- required-check-skip-guard-reusable.yml + -caller.yml: wired into
  omniclaude's own PRs now, but NOT added to required_status_checks in
  this PR -- per the design's ordering, the branch-protection flip is a
  follow-up after 2 independent green runs on dev, to avoid a
  self-inflicted wedge.
- .github/required-checks.yaml upgraded v2->v3: exhaustive 57-row
  REQUIRED manifest (every live dev required_status_checks context,
  verified via gh api) + 1 ADVISORY row for the guard's own not-yet-
  required context. schema v3 is now the operational input to the
  guard, not documentation-only.
- skip_semantics: neutral_ok escape hatch (rationale-mandatory) added
  to 4 rows for real pre-existing conditions the guard found in
  production (Architecture Handshake, Cross-Repo Boundary Parity,
  both CodeRabbit-thread-gate callers) -- tracked for remediation in
  OMN-14856, not fixed here (guard-build scope, not blanket cleanup).

RED-then-GREEN proof (paths: filter on a synthetic fixture) and the
real repo run (4 pre-existing findings, now neutral_ok-exempted with
a cited follow-up ticket) are in the PR body.

Ticket: OMN-14854
@coderabbitai

coderabbitai Bot commented Jul 20, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 34 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 75b7fff7-f4d1-4cd8-af20-6c698dc0c20f

📥 Commits

Reviewing files that changed from the base of the PR and between dc3998a and 3a0528a.

📒 Files selected for processing (11)
  • .github/actions/required-check-skip-guard/_workflow_model.py
  • .github/actions/required-check-skip-guard/reconcile_manifest_vs_live.py
  • .github/actions/required-check-skip-guard/validate_no_required_check_skip_vectors.py
  • .github/required-checks.yaml
  • .github/workflows/required-check-manifest-reconcile.yml
  • .github/workflows/required-check-skip-guard-caller.yml
  • .github/workflows/required-check-skip-guard-reusable.yml
  • .pre-commit-config.yaml
  • .pre-commit-hooks/reject-required-check-skip-vector.sh
  • tests/ci/test_reconcile_manifest_vs_live.py
  • tests/ci/test_validate_no_required_check_skip_vectors.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jonah/omn-14854-required-check-skip-vector-guard

Comment @coderabbitai help to get the list of available commands.

jonahgabriel added a commit to OmniNode-ai/onex_change_control that referenced this pull request Jul 20, 2026
Adds OCC Evidence-Source autobind receipts for OmniNode-ai/omniclaude#1917. Gates green; unresolved review threads empty.
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Hostile Reviewer — DEGRADED (informational)

Blocking findings (critical/error): 0
Total findings: 0
Models succeeded: none

Note: All reviewer models failed or were unavailable. Degraded results are informational during the pilot phase (OMN-8524) and do not block merge. Error: cli_review exited non-zero (all models failed or binary error)


Gate semantics (pilot phase)

Verdict Meaning Blocks merge?
passed No critical/error findings No
blocked CRITICAL or ERROR findings found Yes
degraded All models unavailable (infra) No (pilot)

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant