Skip to content

ci(OMN-16013): close L4 external-context gap + contract-compliance fail-open - #1556

Merged
jonahgabriel merged 1 commit into
devfrom
jonah/omn-16013-gate-enforce-omnibase-core-l4-external-contexts
Aug 13, 2026
Merged

jonahgabriel merged 1 commit into
devfrom
jonah/omn-16013-gate-enforce-omnibase-core-l4-external-contexts

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Aug 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes the omnibase_core gaps found by the enforce-everything CI gate audit: two standalone validators living in separate workflow files that were structurally invisible to the CI Summary poller, and a latent fail-open in contract-compliance.

  • L4 EXPECTED_EXTERNAL_CONTEXTS (new layer in scripts/ci/ci_summary_gate.py): DB ownership CI twin (B1) (check-db-ownership.yml) and LLM refs drift check (OMN-11932) (check-llm-refs-drift.yml) are resolved against commits/{sha}/check-runs and asserted strict-success (present + completed + conclusion == 'success'; skipped is NOT accepted, unlike GATE_JOBS). Both files' on.pull_request.paths: filters are converted to always-fire + an in-job short-circuit before being asserted, so an asserted context can never be silently absent because touched paths didn't match.
  • contract-compliance fail-open fix: the job's empty-PR_NUMBER branch (hit on merge_group/push, neither of which populates PR_NUMBER) used to exit 0 — a vacuous SUCCESS that never ran a single DoD check_value, while GATE_JOBS still counted it as a provable pass. Converted to exit 1. The job's if: deliberately stays unconditional across pull_request/merge_group/push rather than narrowing to pull_request only — narrowing it would make GitHub post the job as skipped on merge_group/push instead, which GATE_JOBS' own completeness anchor accepts as GOOD, silently worse than the original bug. The repo's own reject-required-check-skip-vector pre-commit hook (OMN-14863) live-caught an earlier draft of this fix that narrowed the if:, confirming the vector before this PR was opened.
  • product-readiness-shadow.yml header correction: it claimed lint (shadow), typecheck (shadow), tests+coverage (shadow), reason-graph, and product-readiness / evaluate "stay NON-required and cannot wedge a merge." Live branch protection on dev shows all five already directly required. Comment corrected; no enforcement behavior change (they were already enforcing).

Gate evidence

  • In-run jobs (ci.yml): 59 total jobs in ci.yml. These are covered by the pre-existing default-deny sweep (sweep_failures) plus the GATE_JOBS (5) / STRICT_SUCCESS_JOBS (1) / SPEC_REQUIRED_VALIDATOR_JOBS (12) completeness anchors — every job not explicitly in SOFT_ALLOWLIST (2: Version Pin Compliance, Contract Compliance) already fails the gate if it's present+completed+not-good. This PR does not change that layer's structure, only adds L4 below it.
  • L4 EXPECTED_EXTERNAL_CONTEXTS: 2 new entries (DB ownership CI twin (B1), LLM refs drift check (OMN-11932)), both previously zero-coverage (their workflow files had no relationship to CI Summary at all).
  • Direct branch-protection contexts on dev: 37 (fetched live via gh api repos/OmniNode-ai/omnibase_core/branches/dev/protection/required_status_checks, snapshotted at tests/unit/scripts/ci/fixtures/required_status_checks_snapshot.json).
  • Completeness test (tests/unit/scripts/ci/test_ci_summary_gate.py::TestEnforceEverythingCompleteness::test_every_pr_triggered_job_is_classified): enumerates every job that can fire on pull_request targeting main/dev across all .github/workflows/*.yml (excluding ci.yml, whose jobs are covered by the sweep above) and asserts each resolves into EXPECTED_EXTERNAL_CONTEXTS, the committed required-status-checks snapshot, or an explicit exemption reason. Falsification-verified: removing any one classification entry flips this test red (confirmed locally before opening this PR).
  • Full local test suite (pre-push governed impacted-test selector escalated to full-suite because this PR touches scripts/ci/, per its own fail-closed policy): 43923 passed, 53 skipped, 3 xpassed in 5811.68s.

Exemptions (all carry a reason)

Pre-existing (unchanged, re-verified during this audit):

  • Version Pin Compliance (SOFT_ALLOWLIST) — continue-on-error: true, advisory pending OMN-5132.
  • Contract Compliance (SOFT_ALLOWLIST) — orphan job (compliance), not in any needs:, not a required branch-protection context; distinct from the gating Contract Compliance Check (pinned by a dedicated name-collision-guard test in this PR).
  • cross-repo-validation.yml — no pull_request trigger by design (schedule/dashboard feed).
  • 17 retired validator-*.yml files — workflow_dispatch-only after the OMN-14877/OMN-14430 migration; live twins are unconditional inline ci.yml jobs already covered by the sweep. Pinned by test_retired_validators_are_workflow_dispatch_only_or_directly_required.

New (added by the completeness test's classification tables, tests/unit/scripts/ci/test_ci_summary_gate.py):

  • auto-merge.yml (pre-check, auto-merge) — auto-merge enablement automation, not a PR content validator; GitHub's native auto-merge still waits on every required status check.
  • auto-tag-on-merge.yml (auto-tag) — triggers only on pull_request.closed, post-merge tag automation.
  • propagate-config.yml (propagate, dry-run-ci-check) — config-propagation automation path-scoped to its own script/config; nothing to dry-run when those paths are untouched, unlike a drift check meaningful on every PR.
  • todo-audit-on-merge.yml (todo-audit) — triggers only on pull_request.closed, post-merge ticket audit.

Test plan

  • uv run pytest tests/unit/scripts/ci/test_ci_summary_gate.py -n 0 -q — 54 passed (includes 22 new tests: L4 fixtures, contract-compliance static pins incl. the skip-vector guard, name-collision guard, and the completeness sweep).
  • uv run pytest tests/unit/scripts/ci/ (full directory) — 416 passed.
  • uv run ruff format --check / uv run ruff check — clean on both touched files.
  • uv run mypy scripts/ci/ci_summary_gate.py tests/unit/scripts/ci/test_ci_summary_gate.py --strict — 0 errors.
  • actionlint on all 4 touched workflow files — 0 errors (pre-existing shellcheck style/info notices only, none new).
  • bash -n on every extracted shell fragment (poll loop, both new short-circuit steps, contract-compliance step) — all syntactically valid.
  • Falsification control: manually removed one DIRECT_REQUIRED_JOB_CONTEXTS entry and confirmed test_every_pr_triggered_job_is_classified goes red; manually simulated a red check-db-ownership check-run and confirmed evaluate() returns FAILURE; confirmed omitting --external-check-runs-file entirely returns PENDING (never a vacuous SUCCESS).
  • Live pre-commit hook reject-required-check-skip-vector (OMN-14863) — passed (after reverting an earlier draft that had narrowed contract-compliance's if:, which the hook correctly rejected).
  • Full local suite via the pre-push governed impacted-test selector (escalated to full-suite by its own scripts/ci/-touch policy) — 43923 passed, 53 skipped, 3 xpassed.

Branch-protection: no changes needed. CI Summary is already the required umbrella context on dev/main; this PR only strengthens what feeds it.

Evidence-Ticket: OMN-16013
Evidence-Source: OCC#6440

…force-everything audit)

Adds an L4 EXPECTED_EXTERNAL_CONTEXTS layer to scripts/ci/ci_summary_gate.py
so "DB ownership CI twin (B1)" and "LLM refs drift check (OMN-11932)" -- the
two validators living in separate workflow files, structurally invisible to
the in-run jobs sweep -- are resolved against commits/{sha}/check-runs and
asserted strict-success by the CI Summary poller. Both workflows' pull_request
paths filters are converted to always-fire + in-job short-circuit first, so an
asserted context can never be silently absent because touched paths didn't
match.

Also fixes a latent fail-open in contract-compliance (ci.yml): its
empty-PR_NUMBER branch used to `exit 0` on merge_group/push (neither
populates PR_NUMBER), a vacuous SUCCESS that GATE_JOBS/STRICT_SUCCESS_JOBS
still counted as a provable pass. Converted to `exit 1`. The job's `if:`
deliberately stays unconditional across all three events rather than
narrowing to pull_request-only -- narrowing it would let GitHub post a
`skipped` conclusion on merge_group/push instead, which GATE_JOBS' own
completeness anchor accepts as GOOD, silently *worse* than the original bug.
The reject-required-check-skip-vector pre-commit hook (OMN-14863) live-caught
an earlier draft of this fix that narrowed the `if:`, confirming the vector.

Corrects product-readiness-shadow.yml's header, which claimed its shadow
contexts "stay NON-required" -- live branch protection shows all five already
directly required on dev.

tests/unit/scripts/ci/test_ci_summary_gate.py: L4 evaluate_external fixtures,
static pins for the contract-compliance fix (including the unconditional-if:
skip-vector guard), and a completeness sweep
(test_every_pr_triggered_job_is_classified) that enumerates every
PR-triggered job across .github/workflows/*.yml and fails until each is
classified into EXPECTED_EXTERNAL_CONTEXTS, the committed
required-status-checks snapshot, or an explicit exemption reason.
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

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: 39 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: 9cd23381-6f8a-46c6-bc43-c57b559bfea8

📥 Commits

Reviewing files that changed from the base of the PR and between 95b310b and 73ddbdf.

📒 Files selected for processing (7)
  • .github/workflows/check-db-ownership.yml
  • .github/workflows/check-llm-refs-drift.yml
  • .github/workflows/ci.yml
  • .github/workflows/product-readiness-shadow.yml
  • scripts/ci/ci_summary_gate.py
  • tests/unit/scripts/ci/fixtures/required_status_checks_snapshot.json
  • tests/unit/scripts/ci/test_ci_summary_gate.py

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

@jonahgabriel

Copy link
Copy Markdown
Contributor Author

Re-triggering PR events now that Evidence-Source is stamped (OCC#6440), to get a clean post-evidence run across occ-preflight-dependent checks.

@jonahgabriel jonahgabriel reopened this Aug 13, 2026
jonahgabriel pushed a commit to OmniNode-ai/onex_change_control that referenced this pull request Aug 13, 2026
…#6440)

* evidence: OCC companion pass 1 for OmniNode-ai/omnibase_core#1556

* evidence: OCC companion self-bind for #6440

---------

Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
@jonahgabriel

Copy link
Copy Markdown
Contributor Author

Re-triggering once more now that the OCC companion (onex_change_control#6440) has merged, so OCC Companion Merged Gate + CI Summary get a fresh poll.

@jonahgabriel jonahgabriel reopened this Aug 13, 2026
@jonahgabriel
jonahgabriel merged commit 39cf6d9 into dev Aug 13, 2026
755 of 869 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-16013-gate-enforce-omnibase-core-l4-external-contexts branch August 13, 2026 19:04
jonahgabriel pushed a commit that referenced this pull request Aug 29, 2026
…nstead of failing closed

Every dev push has been red on this required check since PR #1556
(OMN-16013): the job's DoD check_values are PR-scoped
(github.event.pull_request.number), which push never populates, so it
hit the deliberate exit-1 fail-closed branch on 100% of post-merge
commits (confirmed on 259e9bc, fdf4c23, 3c94923). The if:
condition cannot be narrowed to pull_request-only -- doing so makes
GitHub report `skipped`, which the GATE_JOBS completeness anchor
counts as GOOD, the exact skip-vector the OMN-14863 pre-commit hook
exists to reject.

Fix: on push, resolve the merge commit's originating PR via
`gh api repos/{repo}/commits/{sha}/pulls` (GitHub associates a
squash-merge commit with its source PR) and validate against that PR
number -- the same check_values the PR's own pull_request run already
evaluated, not a vacuous pass and not a skip. Falls through to the
existing fail-closed exit 1 if no merged PR resolves. pull_request
path is untouched (PR_NUMBER already populated, new branch never
executes); merge_group is untouched (still fails closed, no live dev
merge queue to exercise it).

Scope-checked the other 3 repos calling run_contract_compliance_check.py
directly: omnibase_infra (exit-0 vacuous skip on empty PR_NUMBER,
different unaddressed fail-open risk, out of scope here),
omnibase_compat (step-level `if:` skip, job reports success, out of
scope), onex_change_control (job `if:` never admits push at all,
unaffected).

OMN-16346
jonahgabriel added a commit that referenced this pull request Aug 29, 2026
…nstead of failing closed (#1619)

* fix(OMN-16346): Contract Compliance Check resolves push commit's PR instead of failing closed

Every dev push has been red on this required check since PR #1556
(OMN-16013): the job's DoD check_values are PR-scoped
(github.event.pull_request.number), which push never populates, so it
hit the deliberate exit-1 fail-closed branch on 100% of post-merge
commits (confirmed on 259e9bc, fdf4c23, 3c94923). The if:
condition cannot be narrowed to pull_request-only -- doing so makes
GitHub report `skipped`, which the GATE_JOBS completeness anchor
counts as GOOD, the exact skip-vector the OMN-14863 pre-commit hook
exists to reject.

Fix: on push, resolve the merge commit's originating PR via
`gh api repos/{repo}/commits/{sha}/pulls` (GitHub associates a
squash-merge commit with its source PR) and validate against that PR
number -- the same check_values the PR's own pull_request run already
evaluated, not a vacuous pass and not a skip. Falls through to the
existing fail-closed exit 1 if no merged PR resolves. pull_request
path is untouched (PR_NUMBER already populated, new branch never
executes); merge_group is untouched (still fails closed, no live dev
merge queue to exercise it).

Scope-checked the other 3 repos calling run_contract_compliance_check.py
directly: omnibase_infra (exit-0 vacuous skip on empty PR_NUMBER,
different unaddressed fail-open risk, out of scope here),
omnibase_compat (step-level `if:` skip, job reports success, out of
scope), onex_change_control (job `if:` never admits push at all,
unaffected).

OMN-16346

* test(OMN-16346): bound the fail-closed pin by its own terminator, not a fixed 400-char window

test_empty_pr_number_branch_fails_closed_not_open extracted the
empty-PR_NUMBER branch as `text[idx : idx + 400].split("\n          fi")[0]`.
The character cap was never load-bearing -- the `\n          fi` split already
bounds the branch exactly -- but it made the pin depend on how many COMMENT
characters happen to sit between the `if` and the `exit 1`.

This PR's own change adds an eight-line comment inside that branch explaining
why the three unresolved-PR_NUMBER cases fail closed. That pushed `exit 1`
from roughly offset 250 to offset ~700, so the 400-char window no longer
contained it and the test failed -- while the property it guards was fully
intact. Verified directly against the committed ci.yml, splitting on the
terminator alone: branch length 784, `"exit 1" in branch` True,
`"exit 0" in branch` False.

Dropping the cap makes the extraction match the assertion's stated intent
(the whole branch), so the pin now fails only when the branch actually stops
failing closed.

Verified: tests/unit/scripts/ci/test_ci_summary_gate.py -> 54 passed.
ruff format + ruff check clean on the touched file.

OMN-16346

---------

Co-authored-by: jonahgabriel <jonah.neugass@gmail.com>
jonahgabriel added a commit that referenced this pull request Aug 29, 2026
…nstead of failing closed (#1619)

* fix(OMN-16346): Contract Compliance Check resolves push commit's PR instead of failing closed

Every dev push has been red on this required check since PR #1556
(OMN-16013): the job's DoD check_values are PR-scoped
(github.event.pull_request.number), which push never populates, so it
hit the deliberate exit-1 fail-closed branch on 100% of post-merge
commits (confirmed on 259e9bc, fdf4c23, 3c94923). The if:
condition cannot be narrowed to pull_request-only -- doing so makes
GitHub report `skipped`, which the GATE_JOBS completeness anchor
counts as GOOD, the exact skip-vector the OMN-14863 pre-commit hook
exists to reject.

Fix: on push, resolve the merge commit's originating PR via
`gh api repos/{repo}/commits/{sha}/pulls` (GitHub associates a
squash-merge commit with its source PR) and validate against that PR
number -- the same check_values the PR's own pull_request run already
evaluated, not a vacuous pass and not a skip. Falls through to the
existing fail-closed exit 1 if no merged PR resolves. pull_request
path is untouched (PR_NUMBER already populated, new branch never
executes); merge_group is untouched (still fails closed, no live dev
merge queue to exercise it).

Scope-checked the other 3 repos calling run_contract_compliance_check.py
directly: omnibase_infra (exit-0 vacuous skip on empty PR_NUMBER,
different unaddressed fail-open risk, out of scope here),
omnibase_compat (step-level `if:` skip, job reports success, out of
scope), onex_change_control (job `if:` never admits push at all,
unaffected).

OMN-16346

* test(OMN-16346): bound the fail-closed pin by its own terminator, not a fixed 400-char window

test_empty_pr_number_branch_fails_closed_not_open extracted the
empty-PR_NUMBER branch as `text[idx : idx + 400].split("\n          fi")[0]`.
The character cap was never load-bearing -- the `\n          fi` split already
bounds the branch exactly -- but it made the pin depend on how many COMMENT
characters happen to sit between the `if` and the `exit 1`.

This PR's own change adds an eight-line comment inside that branch explaining
why the three unresolved-PR_NUMBER cases fail closed. That pushed `exit 1`
from roughly offset 250 to offset ~700, so the 400-char window no longer
contained it and the test failed -- while the property it guards was fully
intact. Verified directly against the committed ci.yml, splitting on the
terminator alone: branch length 784, `"exit 1" in branch` True,
`"exit 0" in branch` False.

Dropping the cap makes the extraction match the assertion's stated intent
(the whole branch), so the pin now fails only when the branch actually stops
failing closed.

Verified: tests/unit/scripts/ci/test_ci_summary_gate.py -> 54 passed.
ruff format + ruff check clean on the touched file.

OMN-16346

---------

Co-authored-by: jonahgabriel <jonah.neugass@gmail.com>
github-actions Bot pushed a commit that referenced this pull request Aug 29, 2026
…#1618)

* fix(OMN-16625): gate quality-gate's non-spec-required leaves on docs_only

Operator ruling (verbatim): "Changes to markdown files should not trigger
CI except for the length validator."

zone-filter (the existing docs-only detector, scripts/zone_diff_filter.py)
previously ran AFTER quality-gate's ~26 leaf validator jobs (needs:
[quality-gate]) -- too late in the DAG to gate those leaves. Only the
Phase 2 test-execution matrix (test-parallel/tests-integration/
shadow-compare) was already gated, via tests-gate's existing OMN-15315
per-upstream skip policy.

- Detach zone-filter from needs: [quality-gate]; it now runs immediately
  (if: always(), no needs), in parallel with quality-gate's leaves, so
  docs_only is known up front.
- Gate the 16 non-spec-required quality-gate leaves on docs_only via
  needs: [zone-filter] + if: always() && needs.zone-filter.outputs.docs_only
  != 'true'.
- Rewrite quality-gate's own bash aggregation into the same skip-tolerant
  per-check-loop shape tests-gate already uses: quality-gate itself stays
  if: always() (never a skip-vector) and re-derives docs_only from
  needs.zone-filter.outputs.docs_only before ever admitting a leaf's
  skipped conclusion as passing. The 10 SPEC_REQUIRED_VALIDATOR_JOBS
  members (validator-requirements.yaml) -- including doc-content-scan,
  "the length validator" the operator's ruling carves out -- stay
  unconditional and strictly required.
- required-checks.yaml: skip_semantics: neutral_ok + ticket-cited rationale
  added to the 12 gated leaves that carry a REQUIRED row (mirrors the
  existing Zone Filter row's own precedent); Zone Filter's own row reverted
  to never now that its if: is provably always-true.

Deliberately untouched: contract-compliance, boundary-validation,
occ-companion-merged -- contract-compliance's own in-file comment
documents a previously-rejected attempt to narrow its if: (blocked by the
reject-required-check-skip-vector guard, OMN-14863) because it is a
direct, unwrapped GATE_JOBS entry with no re-deriving aggregator; building
that safe wrapper is separable follow-up scope.

RED-first tests in tests/unit/validation/test_quality_gate_docs_only_short_circuit_omn16625.py
(105 tests, confirmed RED against the pre-change workflow, GREEN after).
validate_no_required_check_skip_vectors.py passes locally. Digests in
AUDITED_GUARD_JOB_EXECUTION_CONTRACTS (test_ci_workflow_shape.py) recomputed
for the 12 changed jobs.

OMN-16625

* fix(OMN-16625): correct OMN-16917's own selector-test assumption for the real diff

The real OMN-16625 test module reads .github/workflows/ci.yml and
.github/required-checks.yaml off disk, so once it lands it is legitimately
a member of the derived CI-contract class as well as being selected by
diff-touch file grain -- the two are not exclusive. The OMN-16917 test
asserted the opposite because it was written against a synthetic diff
before this file existed on disk.

* fix(OMN-16625): tests-gate must also require quality-gate success, not just docs_only

CodeRabbit finding on PR#1618: detaching zone-filter from
needs:[quality-gate] means docs_only is now computed purely from the
diff's file paths and no longer implies quality-gate succeeded. A
genuinely docs-only diff whose quality-gate leg still fails (e.g. a real
doc-content-scan violation) would skip test-parallel/tests-integration
via their own quality-gate==success precondition, and tests-gate would
wrongly accept that skip as the legitimate docs-only case. Add
quality-gate to tests-gate's needs and require its result to be
success before admitting a skip.

* docs(OMN-16927): the v0.47.0 notes called #1611 additive; its extra="forbid" half is breaking (#1617)

The v0.47.0 CHANGELOG filed #1611 under Features as:

    an OPTIONAL `tenant` field on `ModelEventEnvelope`
    (additive; the model stays `extra="forbid"`)

Three things in that line are wrong, and the third is the one that cost a
fleet re-pin:

1. The field is `tenant_id`, not `tenant`.
2. "additive" describes only half the PR.
3. "the model STAYS extra=forbid" asserts a pre-existing state that never
   existed. `extra` was UNDECLARED on ModelEventEnvelope in every commit of
   its history, so Pydantic's "ignore" default applied. #1611 INTRODUCED
   extra="forbid". That wording is precisely what made the release read as
   safe to cut and to cascade.

The tenant_id half is additive and stays under Features, now correctly
named and cross-referenced. The extra="forbid" half moves to Breaking
Changes with the declared-field list, the `source=` -> `source_tool=`
migration, and the measured blast radius.

No code change: the model is correct as shipped. extra="forbid" is
mandatory on every model in this repo (validators/pydantic_extra_forbid),
and silently discarding a producer's attribution is the exact failure class
the PR was opened to close. Only the notes were wrong.

Evidence-Ticket: OMN-16927

Co-authored-by: jonahgabriel <jonah.neugass@gmail.com>

* fix(OMN-16346): Contract Compliance Check resolves push commit's PR instead of failing closed (#1619)

* fix(OMN-16346): Contract Compliance Check resolves push commit's PR instead of failing closed

Every dev push has been red on this required check since PR #1556
(OMN-16013): the job's DoD check_values are PR-scoped
(github.event.pull_request.number), which push never populates, so it
hit the deliberate exit-1 fail-closed branch on 100% of post-merge
commits (confirmed on 259e9bc, fdf4c23, 3c94923). The if:
condition cannot be narrowed to pull_request-only -- doing so makes
GitHub report `skipped`, which the GATE_JOBS completeness anchor
counts as GOOD, the exact skip-vector the OMN-14863 pre-commit hook
exists to reject.

Fix: on push, resolve the merge commit's originating PR via
`gh api repos/{repo}/commits/{sha}/pulls` (GitHub associates a
squash-merge commit with its source PR) and validate against that PR
number -- the same check_values the PR's own pull_request run already
evaluated, not a vacuous pass and not a skip. Falls through to the
existing fail-closed exit 1 if no merged PR resolves. pull_request
path is untouched (PR_NUMBER already populated, new branch never
executes); merge_group is untouched (still fails closed, no live dev
merge queue to exercise it).

Scope-checked the other 3 repos calling run_contract_compliance_check.py
directly: omnibase_infra (exit-0 vacuous skip on empty PR_NUMBER,
different unaddressed fail-open risk, out of scope here),
omnibase_compat (step-level `if:` skip, job reports success, out of
scope), onex_change_control (job `if:` never admits push at all,
unaffected).

OMN-16346

* test(OMN-16346): bound the fail-closed pin by its own terminator, not a fixed 400-char window

test_empty_pr_number_branch_fails_closed_not_open extracted the
empty-PR_NUMBER branch as `text[idx : idx + 400].split("\n          fi")[0]`.
The character cap was never load-bearing -- the `\n          fi` split already
bounds the branch exactly -- but it made the pin depend on how many COMMENT
characters happen to sit between the `if` and the `exit 1`.

This PR's own change adds an eight-line comment inside that branch explaining
why the three unresolved-PR_NUMBER cases fail closed. That pushed `exit 1`
from roughly offset 250 to offset ~700, so the 400-char window no longer
contained it and the test failed -- while the property it guards was fully
intact. Verified directly against the committed ci.yml, splitting on the
terminator alone: branch length 784, `"exit 1" in branch` True,
`"exit 0" in branch` False.

Dropping the cap makes the extraction match the assertion's stated intent
(the whole branch), so the pin now fails only when the branch actually stops
failing closed.

Verified: tests/unit/scripts/ci/test_ci_summary_gate.py -> 54 passed.
ruff format + ruff check clean on the touched file.

OMN-16346

---------

Co-authored-by: jonahgabriel <jonah.neugass@gmail.com>

* fix(OMN-16625): make quality-gate's tier loops match the OMN-15304 aggregator-loop shape

The vector-6 skip-guard's aggregator-loop recognizer only rebinds a for-loop's
value token to the fail-closed sentinel when the literal `${{ needs.<job>.result
}}` GHA expression is inlined directly in the loop's own item list (the exact
shape tests-gate already uses). The previous commit routed both TIER 1 and
TIER 2 through the preamble-assigned `$lint`-style shell variables instead, which
is functionally identical at runtime but statically invisible to the recognizer
-- `required-check-skip-guard / check-skip-vectors` failed on push with
vector-6-result-triage-unverifiable for all 26 consumed upstreams (CI run
33255918432, PR #1618). Confirmed pre-existing PASS on unmodified origin/dev and
FAIL on this branch before the fix, via local run of
omniclaude/.github/actions/required-check-skip-guard/validate_no_required_check_skip_vectors.py
against both trees -- isolating the regression to this diff, not the guard
itself or the OMN-16917 selector fix it depends on.

Inlines the literal `${{ needs.<job>.result }}` expression in each for-loop
item for both tiers; preamble variable assignments are untouched (still feed
the summary table). No behavior change -- same two-tier fail-closed triage,
now provably so.

Also drops the `result_triage: waived` manifest row added for the previous
(unverifiable) shape -- the guard now proves Quality Gate fail-closed on its
own, so the waiver escape hatch is no longer needed. Local guard run
confirms PASS with zero manifest waivers for this job.

* fix(OMN-16625): stop templating a literal comment as a GHA expression

The two explanatory comments added in the previous commit wrote the
placeholder as \`\${{ needs.<job>.result }}\` -- but GitHub Actions templates
EVERY \${{ ... }} occurrence in a run: block, including inside a bash \`#\`
comment, before the shell ever sees it. \`needs.<job>.result\` is not valid
GHA expression syntax (the literal \`<\` token), so the workflow file failed
to even start on push (run 33257208424, "This run likely failed because of
a workflow file issue", zero jobs scheduled) -- confirmed via
\`actionlint .github/workflows/ci.yml\`, which reported the exact same
"unexpected token '<'" parse error locally. Reworded both comments to drop
the \${{ }} wrapper around the placeholder; actionlint now reports zero
non-shellcheck findings (matching origin/dev's own pre-existing shellcheck-only
baseline), and the OMN-15304 skip-guard still passes locally.

---------

Co-authored-by: jonahgabriel <jonah.neugass@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant