Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 54 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2253,13 +2253,64 @@ jobs:
# ($GITHUB_WORKSPACE), not the OCC clone. Pointing it at OCC is the original
# defect: a check_value's cwd became the receipt store, so the only file an author
# could reach was their own receipt -- which is why ~32% of the corpus greps itself.
#
# OMN-16508: the empty-PR_NUMBER branch below used to `exit 0` -- a VACUOUS
# PASS. This job's `if:` admits push and merge_group, neither of which
# populates `github.event.pull_request.number`, so on those events the step
# reported success having evaluated zero check_values. That was not
# cosmetic: "Contract Compliance Check" is a GATE_JOBS entry in
# scripts/ci/ci_summary_gate.py, and CI Summary is this repo's sole required
# branch-protection context, so the umbrella poller counted the vacuous run
# as a PROVABLE pass. A green gate that checked nothing never prompts
# investigation the way a red one does.
#
# Live surface: `on.push.branches` here is [main], so this never fired on a
# dev commit -- the two real events are push-to-main (release-synced
# fast-forwards) and merge_group. merge_group is unexercised today (no repo
# in the registry has a queue on dev as of 2026-08-24) but is the more
# dangerous half: it would vacate this gate on the merge path itself the
# instant a queue is re-enabled, with zero further code change.
#
# The fix is RESOLUTION, not narrowing. Excluding push/merge_group with a
# job-level `if:` would publish a `skipped` check run, which branch
# protection counts as passing -- the same skip-vector class the
# `reject-required-check-skip-vector` hook (OMN-14863) rejects, and which
# ci_summary_gate's external-context assertion treats as a hard failure.
# So the job keeps running on all three events and resolves WHICH PR's
# check_values apply: the event's own number on pull_request (no API call,
# behaviourally identical to pre-fix), the `pr-<n>-` segment of the
# gh-readonly-queue ref on merge_group (no API call), or the merged source
# PR of the pushed commit via `gh api repos/{repo}/commits/{sha}/pulls`.
# Unresolved -- including a transient gh failure -- exits 1. Pinned by
# tests/ci/test_resolve_contract_compliance_pr.py.
- name: Resolve the PR whose DoD check_values govern this run (OMN-16508)
id: dod-pr
run: |
set -euo pipefail
RESOLVED_PR="$(python3 scripts/ci/resolve_contract_compliance_pr.py \
--repo "${{ github.repository }}" \
--pr-number "${PR_NUMBER:-}" \
--merge-group-ref "${MERGE_GROUP_REF:-}" \
--sha "${{ github.sha }}" \
--event-name "${{ github.event_name }}")"
echo "pr_number=${RESOLVED_PR}" >> "$GITHUB_OUTPUT"
echo "Contract-compliance DoD scope resolved to PR #${RESOLVED_PR}." >> "$GITHUB_STEP_SUMMARY"
env:
GH_TOKEN: ${{ github.token }}
PR_NUMBER: ${{ github.event.pull_request.number }}
MERGE_GROUP_REF: ${{ github.event.merge_group.head_ref }}

- name: Run DoD contract compliance against the product tree (OMN-14436)
working-directory: onex_change_control
run: |
set -euo pipefail
if [ -z "${PR_NUMBER:-}" ]; then
echo "::notice::No PR number (event=${{ github.event_name }}); DoD check_values are PR-scoped. Skipping."
exit 0
# Unreachable via the resolver step above, which already exits 1 on an
# unresolvable scope. Kept as a defence-in-depth fail-closed floor:
# never run this gate to a green conclusion without a PR whose
# check_values were actually evaluated (OMN-16508).
echo "::error::No PR number resolved (event=${{ github.event_name }}); DoD check_values are PR-scoped. Failing closed."
exit 1
fi
uv run python scripts/ci/run_contract_compliance_check.py \
--pr "$PR_NUMBER" \
Expand All @@ -2269,7 +2320,7 @@ jobs:
--legacy-allowlist "$GITHUB_WORKSPACE/onex_change_control/scripts/ci/dod_runner_legacy_allowlist.txt"
env:
GH_TOKEN: ${{ github.token }}
PR_NUMBER: ${{ github.event.pull_request.number }}
PR_NUMBER: ${{ steps.dod-pr.outputs.pr_number }}

# ============================================================================
# Test-Failure Ratchet Gate (OMN-13027, retro A-8)
Expand Down
180 changes: 180 additions & 0 deletions scripts/ci/resolve_contract_compliance_pr.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
# SPDX-FileCopyrightText: 2025 OmniNode.ai Inc.
# SPDX-License-Identifier: MIT
"""Resolve the PR whose DoD ``check_values`` ``contract-compliance`` must run.

OMN-16508. ``ci.yml``'s ``contract-compliance`` job invokes
``run_contract_compliance_check.py --pr <n>``. ``<n>`` came straight from
``github.event.pull_request.number``, which only the ``pull_request`` event
populates -- but the job's ``if:`` also admits ``push`` and ``merge_group``.
The pre-fix step met the empty case with ``exit 0``.

That ``exit 0`` was a **vacuous pass**, and a load-bearing one:
``"Contract Compliance Check"`` is a ``GATE_JOBS`` entry in
:mod:`scripts.ci.ci_summary_gate`, and ``CI Summary`` is the sole required
branch-protection context on this repo. The umbrella poller therefore counted a
run that evaluated zero ``check_values`` as a *provable* pass. Unlike a red
gate, a green one that checked nothing never prompts investigation.

Live fail-open surface, stated precisely: ``ci.yml``'s ``on.push.branches`` is
``[main]``, so this never fired on a ``dev`` commit. The two real events are
``push`` to ``main`` (release-synced fast-forwards) and ``merge_group`` --
the latter unexercised today (no repo in the registry has a queue on ``dev`` as
of 2026-08-24) but a latent bypass of the repo's only required context the
moment a queue is re-enabled.

Resolution, not narrowing
-------------------------
Excluding ``push``/``merge_group`` with a job-level ``if:`` would publish a
``skipped`` check run, which branch protection counts as passing -- the same
skip-vector class ``reject-required-check-skip-vector`` (OMN-14863) rejects,
and which ``ci_summary_gate``'s external-context assertion treats as a hard
failure. So the job keeps running on all three events and this module answers
*which* PR's ``check_values`` apply:

``pull_request``
``github.event.pull_request.number``, verbatim. No API call -- this path
stays behaviourally identical to pre-fix.
``merge_group``
The queue ref carries it. GitHub names the branch
``refs/heads/gh-readonly-queue/<base>/pr-<n>-<sha>``, so no API call is
needed here either.
``push``
``gh api repos/{repo}/commits/{sha}/pulls`` -- GitHub associates a
squash-merge commit with its source PR. Only a MERGED association counts,
and a PR whose ``merge_commit_sha`` is exactly this SHA wins over one that
merely contains the commit.

Anything unresolved -- including a transient ``gh`` failure -- exits non-zero.
A gate that cannot determine what to check must not report that it checked.
"""

from __future__ import annotations

import argparse
import json
import re
import subprocess # fixed argv, no shell, trusted gh binary
import sys
from typing import Any, Protocol

EXIT_PASS = 0
EXIT_FAIL = 1

# GitHub's merge-queue branch shape: refs/heads/gh-readonly-queue/<base>/pr-<n>-<sha>
_MERGE_QUEUE_REF = re.compile(r"gh-readonly-queue/[^/]+/pr-(\d+)-")


class CommitPullsFetcher(Protocol):
"""Fetches the PRs GitHub associates with a commit."""

def __call__(self, repo: str, sha: str) -> list[dict[str, Any]]: ...


def parse_merge_queue_ref(ref: str) -> int | None:
"""Return the source PR number carried by a merge-queue branch ref."""
match = _MERGE_QUEUE_REF.search(ref)
return int(match.group(1)) if match else None


def fetch_commit_pulls_via_gh(repo: str, sha: str) -> list[dict[str, Any]]:
"""Ask the GitHub API which PRs are associated with ``sha``."""
completed = subprocess.run(
[
"gh",
"api",
f"repos/{repo}/commits/{sha}/pulls",
"-H",
"Accept: application/vnd.github+json",
],
capture_output=True,
text=True,
check=True,
timeout=60,
)
payload = json.loads(completed.stdout)
if not isinstance(payload, list):
raise RuntimeError(f"unexpected /pulls payload shape: {type(payload).__name__}")
return [entry for entry in payload if isinstance(entry, dict)]


def resolve_pr_number(
*,
repo: str,
pr_number: str,
merge_group_ref: str,
sha: str,
fetch_commit_pulls: CommitPullsFetcher,
) -> int | None:
"""Resolve the governing PR number, or ``None`` to fail closed."""
if pr_number.strip():
return int(pr_number.strip())

from_queue = parse_merge_queue_ref(merge_group_ref)
if from_queue is not None:
return from_queue

if not sha.strip():
return None

try:
associated = fetch_commit_pulls(repo, sha.strip())
except Exception as exc: # noqa: BLE001 - any failure must fail closed
print(
f"::warning::could not list PRs for commit {sha}: {exc}",
file=sys.stderr,
)
return None

merged = [entry for entry in associated if entry.get("merged_at")]
if not merged:
return None

# A commit can be associated with several PRs; prefer the one that
# actually produced it.
for entry in merged:
if entry.get("merge_commit_sha") == sha.strip():
return int(entry["number"])
return int(merged[0]["number"])


def main(
argv: list[str] | None = None,
*,
fetch_commit_pulls: CommitPullsFetcher = fetch_commit_pulls_via_gh,
) -> int:
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("--repo", required=True, help="owner/name of this repository")
parser.add_argument(
"--pr-number", default="", help="github.event.pull_request.number"
)
parser.add_argument(
"--merge-group-ref", default="", help="github.event.merge_group.head_ref"
)
parser.add_argument("--sha", default="", help="the commit to resolve a PR for")
parser.add_argument("--event-name", default="", help="github.event_name, for logs")
args = parser.parse_args(argv)

resolved = resolve_pr_number(
repo=args.repo,
pr_number=args.pr_number,
merge_group_ref=args.merge_group_ref,
sha=args.sha,
fetch_commit_pulls=fetch_commit_pulls,
)
if resolved is None:
print(
f"::error::Could not resolve a merged PR for event="
f"{args.event_name or '?'} sha={args.sha or '?'}. DoD check_values "
f"are PR-scoped, so there is nothing to evaluate -- failing closed "
f"rather than reporting a green gate that ran zero checks "
f"(OMN-16508).",
file=sys.stderr,
)
return EXIT_FAIL

print(resolved)
return EXIT_PASS


if __name__ == "__main__":
raise SystemExit(main())
118 changes: 118 additions & 0 deletions tests/ci/test_incident_replay_omn16508.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
# SPDX-FileCopyrightText: 2025 OmniNode.ai Inc.
# SPDX-License-Identifier: MIT

"""Incident replay for OMN-16508 -- the contract-compliance vacuous pass.

The guard under replay is :mod:`scripts.ci.resolve_contract_compliance_pr`, and
the incident it exists to catch is a **false green**: ``ci.yml``'s
``contract-compliance`` step met an empty ``PR_NUMBER`` with ``exit 0``, so on
``push`` and ``merge_group`` the required gate reported success having evaluated
zero DoD ``check_values``. ``"Contract Compliance Check"`` is a ``GATE_JOBS``
entry in :mod:`scripts.ci.ci_summary_gate`, and ``CI Summary`` is this repo's
sole required branch-protection context, so the umbrella poller counted that
run as a *provable* pass.

Both artifacts here are verbatim GitHub API responses, captured with ``gh api``
and committed unmodified. Neither is retyped or trimmed -- the whole point of
OMN-15547's rule is that a plausible-looking reconstruction can only exhibit
failure modes its author already imagined.

* **Reject direction (the incident).**
``tests/fixtures/omn16508/commit-6d7090da-pulls.gh-api.json.captured`` is what
``repos/OmniNode-ai/omnibase_infra/commits/{sha}/pulls`` really returns for a
commit no merged PR ever produced -- ``6d7090da``, the head of PR #2890, which
was closed without merging. The answer is an empty list, and that emptiness is
precisely the state the pre-fix step turned into a green gate. The guard must
say NO here.
* **Accept control.**
``tests/fixtures/omn16508/commit-5b904d88-pulls.gh-api.json.captured`` is the
21KB response for ``5b904d88``, a real commit on ``main`` whose source PR
(#2378) merged and whose ``merge_commit_sha`` is that same SHA. Without this
half, a guard hard-wired to ``return None`` would replay the incident
perfectly and still be useless: it would fail every push-to-``main`` run
closed instead of open. R5 only demands the reject half for a ``false_green``
case; the control is here because a reject-only proof cannot tell a working
guard from one that is stuck shut.
"""

from __future__ import annotations

import json
from pathlib import Path
from typing import Any

import pytest

from scripts.ci.resolve_contract_compliance_pr import resolve_pr_number

pytestmark = pytest.mark.unit

REPO = "OmniNode-ai/omnibase_infra"
FIXTURES = Path(__file__).resolve().parents[2] / "tests" / "fixtures" / "omn16508"

# The commit no merged PR produced (head of closed-unmerged PR #2890).
UNMERGED_COMMIT = "6d7090daa3c410d9ee51ddc3a0b61a82a8a25662"
UNMERGED_CAPTURE = FIXTURES / "commit-6d7090da-pulls.gh-api.json.captured"

# A real `main` commit whose source PR merged.
MERGED_COMMIT = "5b904d881ba51a697e5b3d50b28460abbb2fd5aa"
MERGED_CAPTURE = FIXTURES / "commit-5b904d88-pulls.gh-api.json.captured"


def _replay(capture: Path) -> Any:
"""Serve the captured bytes where the guard would call the live API."""
payload = json.loads(capture.read_text(encoding="utf-8"))

def _fetch(repo: str, sha: str) -> list[dict[str, Any]]:
return list(payload)

return _fetch


def test_the_real_no_merged_pr_response_is_rejected_not_vacuously_passed() -> None:
"""The incident input. Pre-fix this was `exit 0`; it must now resolve to nothing."""
resolved = resolve_pr_number(
repo=REPO,
pr_number="",
merge_group_ref="",
sha=UNMERGED_COMMIT,
fetch_commit_pulls=_replay(UNMERGED_CAPTURE),
)
assert resolved is None, (
"GitHub reports no merged PR for this commit, so there are no PR-scoped "
"DoD check_values to evaluate. Resolving anything here reinstates the "
"OMN-16508 vacuous pass: a required gate reporting green on zero checks."
)


def test_the_real_merged_pr_response_still_resolves_its_source_pr() -> None:
"""The control: fail-closed must not mean fail-always."""
resolved = resolve_pr_number(
repo=REPO,
pr_number="",
merge_group_ref="",
sha=MERGED_COMMIT,
fetch_commit_pulls=_replay(MERGED_CAPTURE),
)
assert resolved == 2378, (
"a guard that rejected this too would replay the incident and still be "
"useless -- every push-to-main run would fail closed on a commit whose "
"check_values were in fact evaluated by PR #2378's own run"
)


def test_the_captured_responses_are_the_shapes_this_replay_claims() -> None:
"""Guards the replay itself: a re-capture that drifted must not pass silently."""
unmerged = json.loads(UNMERGED_CAPTURE.read_text(encoding="utf-8"))
assert [entry for entry in unmerged if entry.get("merged_at")] == [], (
f"{UNMERGED_CAPTURE.name} no longer represents a commit without a merged "
f"source PR, so it can no longer exhibit the OMN-16508 failure"
)

merged = json.loads(MERGED_CAPTURE.read_text(encoding="utf-8"))
winners = [
entry
for entry in merged
if entry.get("merged_at") and entry.get("merge_commit_sha") == MERGED_COMMIT
]
assert [entry["number"] for entry in winners] == [2378]
Loading
Loading