From 7091ca9bf84c0ccbdf93e6ccd36cadf96c2f5439 Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Wed, 26 Aug 2026 09:00:57 -0400 Subject: [PATCH 1/7] 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 --- .github/required-checks.yaml | 117 ++++-- .github/workflows/ci.yml | 269 ++++++++++++-- .../unit/validation/test_ci_workflow_shape.py | 24 +- ...est_draft_state_admission_gate_omn16215.py | 21 +- ...y_gate_docs_only_short_circuit_omn16625.py | 346 ++++++++++++++++++ 5 files changed, 693 insertions(+), 84 deletions(-) create mode 100644 tests/unit/validation/test_quality_gate_docs_only_short_circuit_omn16625.py diff --git a/.github/required-checks.yaml b/.github/required-checks.yaml index f9789e7350..14b1eb230a 100644 --- a/.github/required-checks.yaml +++ b/.github/required-checks.yaml @@ -314,10 +314,12 @@ gates: producer_kind: local workflow: ci.yml job_path: ["zone-filter"] - skip_semantics: neutral_ok - rationale: "Job 'zone-filter' has a job-level `if: always() && needs.quality-gate.result == 'success'` - — vector-2 (ungated job-level if referencing needs.*). A skipped zone-filter counts as passing under - branch protection. skip_semantics: neutral_ok pending triage in OMN-14864." + skip_semantics: never + rationale: "OMN-16625: 'zone-filter' detached from `needs: [quality-gate]` and its job-level `if:` + simplified to `always()` -- it must now be the FIRST job to compute docs_only (in parallel with + quality-gate's leaves) so those leaves can gate on it too. `if: always()` is provably ALWAYS_TRUE_FOR_PR, + closing the vector-2 finding this row previously carried a neutral_ok override for (OMN-14864); + skip_semantics reverted to never since a skip is no longer even reachable." - name: "Contract Compliance Check" mode: REQUIRED @@ -406,8 +408,13 @@ gates: producer_kind: local workflow: ci.yml job_path: ["sdk-boundary-check"] - skip_semantics: never - rationale: "Live required status check on omnibase_core dev branch protection." + skip_semantics: neutral_ok + rationale: "OMN-16625: job-level `if: always() && needs.zone-filter.outputs.docs_only != 'true'` added + to 'sdk-boundary-check' (vector-2 -- ungated job-level if referencing needs.*). A skipped 'sdk-boundary-check' + counts as passing under branch protection, but this is a deliberate, re-derived skip: quality-gate + independently re-reads needs.zone-filter.outputs.docs_only before ever treating a skipped leaf as + passing (mirrors the Zone Filter row's own precedent above, and tests-gate's proven per-upstream + skip policy, OMN-15315)." - name: "Detect Secrets" mode: REQUIRED @@ -415,8 +422,13 @@ gates: producer_kind: local workflow: ci.yml job_path: ["detect-secrets"] - skip_semantics: never - rationale: "Live required status check on omnibase_core dev branch protection." + skip_semantics: neutral_ok + rationale: "OMN-16625: job-level `if: always() && needs.zone-filter.outputs.docs_only != 'true'` added + to 'detect-secrets' (vector-2 -- ungated job-level if referencing needs.*). A skipped 'detect-secrets' + counts as passing under branch protection, but this is a deliberate, re-derived skip: quality-gate + independently re-reads needs.zone-filter.outputs.docs_only before ever treating a skipped leaf as + passing (mirrors the Zone Filter row's own precedent above, and tests-gate's proven per-upstream + skip policy, OMN-15315)." - name: "Reject localhost/env fallbacks (OMN-7227)" mode: REQUIRED @@ -424,8 +436,13 @@ gates: producer_kind: local workflow: ci.yml job_path: ["no-env-fallbacks"] - skip_semantics: never - rationale: "Live required status check on omnibase_core dev branch protection." + skip_semantics: neutral_ok + rationale: "OMN-16625: job-level `if: always() && needs.zone-filter.outputs.docs_only != 'true'` added + to 'no-env-fallbacks' (vector-2 -- ungated job-level if referencing needs.*). A skipped 'no-env-fallbacks' + counts as passing under branch protection, but this is a deliberate, re-derived skip: quality-gate + independently re-reads needs.zone-filter.outputs.docs_only before ever treating a skipped leaf as + passing (mirrors the Zone Filter row's own precedent above, and tests-gate's proven per-upstream + skip policy, OMN-15315)." - name: "Exports Validation" mode: REQUIRED @@ -433,8 +450,13 @@ gates: producer_kind: local workflow: ci.yml job_path: ["exports-validation"] - skip_semantics: never - rationale: "Live required status check on omnibase_core dev branch protection." + skip_semantics: neutral_ok + rationale: "OMN-16625: job-level `if: always() && needs.zone-filter.outputs.docs_only != 'true'` added + to 'exports-validation' (vector-2 -- ungated job-level if referencing needs.*). A skipped 'exports-validation' + counts as passing under branch protection, but this is a deliberate, re-derived skip: quality-gate + independently re-reads needs.zone-filter.outputs.docs_only before ever treating a skipped leaf as + passing (mirrors the Zone Filter row's own precedent above, and tests-gate's proven per-upstream + skip policy, OMN-15315)." - name: "Pyright Type Checking" mode: REQUIRED @@ -442,8 +464,12 @@ gates: producer_kind: local workflow: ci.yml job_path: ["pyright"] - skip_semantics: never - rationale: "Live required status check on omnibase_core dev branch protection." + skip_semantics: neutral_ok + rationale: "OMN-16625: job-level `if: always() && needs.zone-filter.outputs.docs_only != 'true'` added + to 'pyright' (vector-2 -- ungated job-level if referencing needs.*). A skipped 'pyright' counts + as passing under branch protection, but this is a deliberate, re-derived skip: quality-gate independently + re-reads needs.zone-filter.outputs.docs_only before ever treating a skipped leaf as passing (mirrors + the Zone Filter row's own precedent above, and tests-gate's proven per-upstream skip policy, OMN-15315)." - name: "Code Quality" mode: REQUIRED @@ -478,8 +504,13 @@ gates: producer_kind: local workflow: ci.yml job_path: ["check-deterministic-skills"] - skip_semantics: never - rationale: "Live required status check on omnibase_core dev branch protection." + skip_semantics: neutral_ok + rationale: "OMN-16625: job-level `if: always() && needs.zone-filter.outputs.docs_only != 'true'` added + to 'check-deterministic-skills' (vector-2 -- ungated job-level if referencing needs.*). A skipped + 'check-deterministic-skills' counts as passing under branch protection, but this is a deliberate, + re-derived skip: quality-gate independently re-reads needs.zone-filter.outputs.docs_only before + ever treating a skipped leaf as passing (mirrors the Zone Filter row's own precedent above, and + tests-gate's proven per-upstream skip policy, OMN-15315)." - name: "Documentation Validation" mode: REQUIRED @@ -487,8 +518,13 @@ gates: producer_kind: local workflow: ci.yml job_path: ["docs-validation"] - skip_semantics: never - rationale: "Live required status check on omnibase_core dev branch protection." + skip_semantics: neutral_ok + rationale: "OMN-16625: job-level `if: always() && needs.zone-filter.outputs.docs_only != 'true'` added + to 'docs-validation' (vector-2 -- ungated job-level if referencing needs.*). A skipped 'docs-validation' + counts as passing under branch protection, but this is a deliberate, re-derived skip: quality-gate + independently re-reads needs.zone-filter.outputs.docs_only before ever treating a skipped leaf as + passing (mirrors the Zone Filter row's own precedent above, and tests-gate's proven per-upstream + skip policy, OMN-15315)." - name: "Node Purity Check" mode: REQUIRED @@ -496,8 +532,13 @@ gates: producer_kind: local workflow: ci.yml job_path: ["node-purity-check"] - skip_semantics: never - rationale: "Live required status check on omnibase_core dev branch protection." + skip_semantics: neutral_ok + rationale: "OMN-16625: job-level `if: always() && needs.zone-filter.outputs.docs_only != 'true'` added + to 'node-purity-check' (vector-2 -- ungated job-level if referencing needs.*). A skipped 'node-purity-check' + counts as passing under branch protection, but this is a deliberate, re-derived skip: quality-gate + independently re-reads needs.zone-filter.outputs.docs_only before ever treating a skipped leaf as + passing (mirrors the Zone Filter row's own precedent above, and tests-gate's proven per-upstream + skip policy, OMN-15315)." - name: "Enum Governance Check" mode: REQUIRED @@ -514,8 +555,13 @@ gates: producer_kind: local workflow: ci.yml job_path: ["contract-config-compliance"] - skip_semantics: never - rationale: "Live required status check on omnibase_core dev branch protection." + skip_semantics: neutral_ok + rationale: "OMN-16625: job-level `if: always() && needs.zone-filter.outputs.docs_only != 'true'` added + to 'contract-config-compliance' (vector-2 -- ungated job-level if referencing needs.*). A skipped + 'contract-config-compliance' counts as passing under branch protection, but this is a deliberate, + re-derived skip: quality-gate independently re-reads needs.zone-filter.outputs.docs_only before + ever treating a skipped leaf as passing (mirrors the Zone Filter row's own precedent above, and + tests-gate's proven per-upstream skip policy, OMN-15315)." - name: "Breaking-Schema-Change (OMN-12621)" mode: REQUIRED @@ -523,8 +569,13 @@ gates: producer_kind: local workflow: ci.yml job_path: ["breaking-schema-change"] - skip_semantics: never - rationale: "Live required status check on omnibase_core dev branch protection." + skip_semantics: neutral_ok + rationale: "OMN-16625: job-level `if: always() && needs.zone-filter.outputs.docs_only != 'true'` added + to 'breaking-schema-change' (vector-2 -- ungated job-level if referencing needs.*). A skipped 'breaking-schema-change' + counts as passing under branch protection, but this is a deliberate, re-derived skip: quality-gate + independently re-reads needs.zone-filter.outputs.docs_only before ever treating a skipped leaf as + passing (mirrors the Zone Filter row's own precedent above, and tests-gate's proven per-upstream + skip policy, OMN-15315)." - name: "Demo-Path Topic Coherence (OMN-12777)" mode: REQUIRED @@ -532,8 +583,13 @@ gates: producer_kind: local workflow: ci.yml job_path: ["demo-path-topic-coherence"] - skip_semantics: never - rationale: "Live required status check on omnibase_core dev branch protection." + skip_semantics: neutral_ok + rationale: "OMN-16625: job-level `if: always() && needs.zone-filter.outputs.docs_only != 'true'` added + to 'demo-path-topic-coherence' (vector-2 -- ungated job-level if referencing needs.*). A skipped + 'demo-path-topic-coherence' counts as passing under branch protection, but this is a deliberate, + re-derived skip: quality-gate independently re-reads needs.zone-filter.outputs.docs_only before + ever treating a skipped leaf as passing (mirrors the Zone Filter row's own precedent above, and + tests-gate's proven per-upstream skip policy, OMN-15315)." - name: "Dispatch-Surface Test Required (OMN-12960)" mode: REQUIRED @@ -541,8 +597,13 @@ gates: producer_kind: local workflow: ci.yml job_path: ["dispatch-surface-test-required"] - skip_semantics: never - rationale: "Live required status check on omnibase_core dev branch protection." + skip_semantics: neutral_ok + rationale: "OMN-16625: job-level `if: always() && needs.zone-filter.outputs.docs_only != 'true'` added + to 'dispatch-surface-test-required' (vector-2 -- ungated job-level if referencing needs.*). A skipped + 'dispatch-surface-test-required' counts as passing under branch protection, but this is a deliberate, + re-derived skip: quality-gate independently re-reads needs.zone-filter.outputs.docs_only before + ever treating a skipped leaf as passing (mirrors the Zone Filter row's own precedent above, and + tests-gate's proven per-upstream skip policy, OMN-15315)." - name: "Quality Gate" mode: REQUIRED diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a23e2a721c..c6ce64932b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -137,6 +137,14 @@ jobs: # Pyright complements mypy (in lint job) - catches different type error categories pyright: name: Pyright Type Checking + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' # OMNI_RUNNER_SELECTOR_V1 - trusted events default to self-hosted omnibase-ci; pull_request defaults to ubuntu-latest runs-on: >- ${{ @@ -193,6 +201,14 @@ jobs: # Quick check (~5 min) ensures __all__ exports are valid before expensive test splits exports-validation: name: Exports Validation + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' # OMNI_RUNNER_SELECTOR_V1 - trusted events default to self-hosted omnibase-ci; pull_request defaults to ubuntu-latest runs-on: >- ${{ @@ -438,6 +454,14 @@ jobs: # intra-core sub-package layering. Blocking gate (wired into quality-gate). lint-imports: name: Import Layering Oracle + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' # OMNI_RUNNER_SELECTOR_V1 - trusted events default to self-hosted omnibase-ci; pull_request defaults to ubuntu-latest runs-on: >- ${{ @@ -554,6 +578,14 @@ jobs: # node` (or a Kafka publish) and surfaces `SkillRoutingError`. Blocking gate. check-deterministic-skills: name: Deterministic Skill Routing + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' # OMNI_RUNNER_SELECTOR_V1 - trusted events default to self-hosted omnibase-ci; pull_request defaults to ubuntu-latest runs-on: >- ${{ @@ -605,6 +637,14 @@ jobs: # Rationale: Doc link validation is independent of code correctness docs-validation: name: Documentation Validation + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' # OMNI_RUNNER_SELECTOR_V1 - trusted events default to self-hosted omnibase-ci; pull_request defaults to ubuntu-latest runs-on: >- ${{ @@ -656,6 +696,14 @@ jobs: # Ensures declarative nodes (COMPUTE, REDUCER) maintain purity guarantees node-purity-check: name: Node Purity Check + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' # OMNI_RUNNER_SELECTOR_V1 - trusted events default to self-hosted omnibase-ci; pull_request defaults to ubuntu-latest runs-on: >- ${{ @@ -748,6 +796,14 @@ jobs: # Contract-config compliance validator (OMN-10688) contract-config-compliance: name: Contract-Config Compliance (OMN-10688) + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' # OMNI_RUNNER_SELECTOR_V1 - trusted events default to self-hosted omnibase-ci; pull_request defaults to ubuntu-latest runs-on: >- ${{ @@ -798,6 +854,14 @@ jobs: # Breaking-schema-change validator (OMN-12621) breaking-schema-change: name: Breaking-Schema-Change (OMN-12621) + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' # OMNI_RUNNER_SELECTOR_V1 - trusted events default to self-hosted omnibase-ci; pull_request defaults to ubuntu-latest runs-on: >- ${{ @@ -843,6 +907,14 @@ jobs: # Rejects os.getenv/os.environ.get with localhost fallback defaults in src/ and scripts/. no-env-fallbacks: name: Reject localhost/env fallbacks (OMN-7227) + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' # OMNI_RUNNER_SELECTOR_V1 - trusted events default to self-hosted omnibase-ci; pull_request defaults to ubuntu-latest runs-on: >- ${{ @@ -869,6 +941,14 @@ jobs: # Uses detect-secrets-hook for proper baseline comparison (handles type+hash equality) detect-secrets: name: Detect Secrets + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' # OMNI_RUNNER_SELECTOR_V1 - trusted events default to self-hosted omnibase-ci; pull_request defaults to ubuntu-latest runs-on: >- ${{ @@ -1013,6 +1093,14 @@ jobs: # [project.dependencies] (they belong in [project.optional-dependencies]). sdk-boundary-check: name: SDK Boundary Guard + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' runs-on: >- ${{ github.event_name != 'pull_request' @@ -1091,6 +1179,14 @@ jobs: # widget topics without producers, hand-authored topic literals. demo-path-topic-coherence: name: Demo-Path Topic Coherence (OMN-12777) + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' # OMNI_RUNNER_SELECTOR_V1 - trusted events default to self-hosted omnibase-ci; pull_request defaults to ubuntu-latest runs-on: >- ${{ @@ -1143,6 +1239,14 @@ jobs: # Dispatch-surface real-dispatch-test gate (OMN-12960) dispatch-surface-test-required: name: Dispatch-Surface Test Required (OMN-12960) + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' # OMNI_RUNNER_SELECTOR_V1 - trusted events default to self-hosted omnibase-ci; pull_request defaults to ubuntu-latest runs-on: >- ${{ @@ -2484,6 +2588,14 @@ jobs: # fails when an allowlisted FQN no longer exists (count-only-down discipline). no-noncanonical-lifecycle-classes: name: Non-Canonical Lifecycle-Class Ratchet (OMN-14350) + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' # OMNI_RUNNER_SELECTOR_V1 - trusted events default to self-hosted omnibase-ci; pull_request defaults to ubuntu-latest runs-on: >- ${{ @@ -2526,6 +2638,14 @@ jobs: # frozen baseline count down (it may never grow). pydantic-extra-forbid: name: Pydantic extra="forbid" Ratchet (OMN-14515) + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' # OMNI_RUNNER_SELECTOR_V1 - trusted events default to self-hosted omnibase-ci; pull_request defaults to ubuntu-latest runs-on: >- ${{ @@ -2580,6 +2700,14 @@ jobs: # would itself add to the very surface this ratchet governs. pull-request-workflow-ratchet: name: Pull-Request Workflow Ratchet (OMN-14907) + needs: [zone-filter] + # OMN-16625: docs-only diffs skip this leaf (NOT one of the 10 + # SPEC_REQUIRED_VALIDATOR_JOBS in scripts/ci/ci_summary_gate.py, so this + # is safe to gate). quality-gate re-derives docs_only itself from + # needs.zone-filter.outputs.docs_only before accepting a `skipped` + # conclusion here as passing -- mirrors tests-gate's existing + # per-upstream skip policy for test-parallel/tests-integration (OMN-15315). + if: always() && needs.zone-filter.outputs.docs_only != 'true' # OMNI_RUNNER_SELECTOR_V1 - trusted events default to self-hosted omnibase-ci; pull_request defaults to ubuntu-latest runs-on: >- ${{ @@ -2733,7 +2861,13 @@ jobs: # tests-gate reports PASSED on zero tests run. ci-summary's independent # GATE_JOBS entry ("Contract Compliance Check", scripts/ci/ci_summary_gate.py) # keeps this job required and merge-blocking without that cascade. - needs: [lint, pyright, exports-validation, mypy-validation-scripts, core-infra-boundary, lint-imports, check-deterministic-skills, docs-validation, node-purity-check, enum-governance, detect-secrets, sdk-boundary-check, contract-config-compliance, breaking-schema-change, no-env-fallbacks, demo-path-topic-coherence, dispatch-surface-test-required, naming-conventions, pydantic-patterns, aislop-patterns, doc-content-scan, no-new-os-environ, no-noncanonical-lifecycle-classes, pydantic-extra-forbid, pull-request-workflow-ratchet, spdx-headers] + # + # OMN-16625: zone-filter added as a need (read for its `docs_only` output + # only, exactly like tests-gate reads it -- never for its `.result`). This + # job itself stays `if: always()` so it is never a skip-vector; the step + # below re-derives docs_only itself before ever admitting a `skipped` leaf + # as passing, mirroring tests-gate's per-upstream skip policy (OMN-15315). + needs: [lint, pyright, exports-validation, mypy-validation-scripts, core-infra-boundary, lint-imports, check-deterministic-skills, docs-validation, node-purity-check, enum-governance, detect-secrets, sdk-boundary-check, contract-config-compliance, breaking-schema-change, no-env-fallbacks, demo-path-topic-coherence, dispatch-surface-test-required, naming-conventions, pydantic-patterns, aislop-patterns, doc-content-scan, no-new-os-environ, no-noncanonical-lifecycle-classes, pydantic-extra-forbid, pull-request-workflow-ratchet, spdx-headers, zone-filter] if: always() steps: @@ -2742,6 +2876,8 @@ jobs: echo "## Quality Gate Results" >> $GITHUB_STEP_SUMMARY echo "" >> $GITHUB_STEP_SUMMARY + docs_only="${{ needs.zone-filter.outputs.docs_only }}" + lint="${{ needs.lint.result }}" pyright="${{ needs.pyright.result }}" exports="${{ needs.exports-validation.result }}" @@ -2769,6 +2905,8 @@ jobs: pr_workflow_ratchet="${{ needs.pull-request-workflow-ratchet.result }}" spdx_headers="${{ needs.spdx-headers.result }}" + echo "| zone-filter docs_only | ${docs_only:-(unset)} |" >> $GITHUB_STEP_SUMMARY + # Report each check echo "| Check | Result |" >> $GITHUB_STEP_SUMMARY echo "|-------|--------|" >> $GITHUB_STEP_SUMMARY @@ -2799,43 +2937,89 @@ jobs: echo "| Pull-Request Workflow Ratchet (OMN-14907) | $pr_workflow_ratchet |" >> $GITHUB_STEP_SUMMARY echo "| SPDX Headers (OMN-13576) | $spdx_headers |" >> $GITHUB_STEP_SUMMARY - # Gate logic: all must pass - if [[ "$lint" == "success" ]] && \ - [[ "$pyright" == "success" ]] && \ - [[ "$exports" == "success" ]] && \ - [[ "$mypy_scripts" == "success" ]] && \ - [[ "$core_infra" == "success" ]] && \ - [[ "$lint_imports" == "success" ]] && \ - [[ "$det_skills" == "success" ]] && \ - [[ "$docs" == "success" ]] && \ - [[ "$purity" == "success" ]] && \ - [[ "$enum_gov" == "success" ]] && \ - [[ "$secrets" == "success" ]] && \ - [[ "$sdk_boundary" == "success" ]] && \ - [[ "$contract_config" == "success" ]] && \ - [[ "$breaking_schema" == "success" ]] && \ - [[ "$no_env_fallbacks" == "success" ]] && \ - [[ "$demo_path_gate" == "success" ]] && \ - [[ "$dispatch_surface" == "success" ]] && \ - [[ "$naming" == "success" ]] && \ - [[ "$pydantic" == "success" ]] && \ - [[ "$aislop" == "success" ]] && \ - [[ "$doc_content" == "success" ]] && \ - [[ "$no_new_os_environ" == "success" ]] && \ - [[ "$noncanonical_classes" == "success" ]] && \ - [[ "$extra_forbid" == "success" ]] && \ - [[ "$pr_workflow_ratchet" == "success" ]] && \ - [[ "$spdx_headers" == "success" ]]; then - echo "" >> $GITHUB_STEP_SUMMARY + # OMN-16625: gate logic split in two tiers. + # + # TIER 1 -- STRICT (spec-required validators, validator-requirements.yaml + # model_b_rollup_enforcement / SPEC_REQUIRED_VALIDATOR_JOBS in + # scripts/ci/ci_summary_gate.py). These 10 run UNCONDITIONALLY in ci.yml + # (no docs_only `if:` was added to them) and must be EXACTLY `success` + # on every diff, including a docs-only one -- doc-content-scan is "the + # length validator" the operator's docs-only-CI ruling explicitly keeps + # running; a `skipped` here is never legitimate. + for check in \ + "lint=$lint" \ + "mypy_scripts=$mypy_scripts" \ + "core_infra=$core_infra" \ + "enum_gov=$enum_gov" \ + "naming=$naming" \ + "pydantic=$pydantic" \ + "aislop=$aislop" \ + "doc_content=$doc_content" \ + "no_new_os_environ=$no_new_os_environ" \ + "spdx_headers=$spdx_headers"; do + NAME="${check%%=*}" + RESULT="${check#*=}" + if [[ "$RESULT" != "success" ]]; then + echo "" >> $GITHUB_STEP_SUMMARY + echo "**Quality Gate: FAILED**" >> $GITHUB_STEP_SUMMARY + echo "::error::$NAME (spec-required validator) did not pass (result: $RESULT) -- this check is never allowed to skip, docs-only diff or not." + exit 1 + fi + done + + # TIER 2 -- SKIP-TOLERANT (the 16 leaves gated on docs_only in this PR). + # Admits a `skipped` result ONLY when zone-filter's own docs_only output + # (re-derived here, never trusted from a leaf's raw `.result`) says + # 'true' -- exactly tests-gate's existing per-upstream skip policy for + # test-parallel/tests-integration (OMN-15315), applied to Phase 1. + for check in \ + "pyright=$pyright" \ + "exports=$exports" \ + "lint_imports=$lint_imports" \ + "det_skills=$det_skills" \ + "docs=$docs" \ + "purity=$purity" \ + "secrets=$secrets" \ + "sdk_boundary=$sdk_boundary" \ + "contract_config=$contract_config" \ + "breaking_schema=$breaking_schema" \ + "no_env_fallbacks=$no_env_fallbacks" \ + "demo_path_gate=$demo_path_gate" \ + "dispatch_surface=$dispatch_surface" \ + "noncanonical_classes=$noncanonical_classes" \ + "extra_forbid=$extra_forbid" \ + "pr_workflow_ratchet=$pr_workflow_ratchet"; do + NAME="${check%%=*}" + RESULT="${check#*=}" + case "$RESULT" in + success) + : ;; + skipped) + if [[ "$docs_only" != "true" ]]; then + echo "" >> $GITHUB_STEP_SUMMARY + echo "**Quality Gate: FAILED**" >> $GITHUB_STEP_SUMMARY + echo "::error::$NAME was SKIPPED on a non-docs-only change (zone-filter docs_only='${docs_only:-unset}') -- the absence of a validator result is not a passing one." + exit 1 + fi + ;; + *) + echo "" >> $GITHUB_STEP_SUMMARY + echo "**Quality Gate: FAILED**" >> $GITHUB_STEP_SUMMARY + echo "::error::$NAME returned unrecognised result '$RESULT' -- failing closed." + exit 1 + ;; + esac + done + + echo "" >> $GITHUB_STEP_SUMMARY + if [[ "$docs_only" == "true" ]]; then + echo "**Quality Gate: PASSED** (docs-only diff -- heavy leaf validators short-circuited to skipped; spec-required validators + doc-content-scan ran for real)" >> $GITHUB_STEP_SUMMARY + echo "Quality gate passed: docs-only diff, non-spec-required leaves short-circuited." + else echo "**Quality Gate: PASSED**" >> $GITHUB_STEP_SUMMARY echo "All quality checks passed." - exit 0 - else - echo "" >> $GITHUB_STEP_SUMMARY - echo "**Quality Gate: FAILED**" >> $GITHUB_STEP_SUMMARY - echo "Quality gate failed. See individual check results above." - exit 1 fi + exit 0 # --------------------------------------------------------------------------- # Phase 1.6a - Zone Filter (OMN-10356, reusable in OMN-10400) @@ -2843,10 +3027,21 @@ jobs: # Runs the caller branch's classifier inline. The reusable workflow added in # this PR cannot be called by required CI until it exists on main, because # GitHub cannot materialize jobs for a new local workflow_call target. + # + # OMN-16625: detached from `needs: [quality-gate]`. This job used to run + # AFTER quality-gate's ~26 leaf validators finished -- too late to gate + # those leaves themselves on docs_only, only useful for the Phase 2 test + # matrix that already depended on quality-gate succeeding anyway. Zero + # `needs` here means docs_only is known immediately, in parallel with + # quality-gate's leaves, which is what lets those leaves gate on it too. + # Every downstream consumer (detect-changes, test-parallel, + # tests-integration) already independently re-checks + # `needs.quality-gate.result == 'success'` in its own `if:`, so this + # detachment changes nothing about when the Phase 2 test matrix runs -- + # only how early docs_only becomes available to Phase 1. zone-filter: name: Zone Filter (docs-only check) - needs: [quality-gate] - if: always() && needs.quality-gate.result == 'success' + if: always() runs-on: >- ${{ github.event_name != 'pull_request' diff --git a/tests/unit/validation/test_ci_workflow_shape.py b/tests/unit/validation/test_ci_workflow_shape.py index ead91a2a4a..6f552fe801 100644 --- a/tests/unit/validation/test_ci_workflow_shape.py +++ b/tests/unit/validation/test_ci_workflow_shape.py @@ -68,7 +68,7 @@ # diff first; this deliberately avoids another partial execution-surface list. AUDITED_GUARD_JOB_EXECUTION_CONTRACTS = { "exports-validation": ( - "54ac4c9daa5992acfd905b46e1d994db0091303b779bc179da509a9ed9184f4e" # pragma: allowlist secret + "992688f96b514d884eed9932a5f3676ebc4c1fb2c633e626a7eb5c9fa6348c75" # pragma: allowlist secret ), "mypy-validation-scripts": ( "2423622dc22413cc48dc86c623484359251b1046235dc61d3b8fd947512cd69e" # pragma: allowlist secret @@ -77,34 +77,34 @@ "403fa52e3d6f4b8954bc692c6ec40d133278cc00cc34807fa7552732866275ad" # pragma: allowlist secret ), "check-deterministic-skills": ( - "8f0e02654254289640c6298ed043de77e3b3ce203421c8e1e4f229a9afd2e717" # pragma: allowlist secret + "e9092b6a2e64c9dcc5d1d89ac7171a0e84547dbd1dfc4c962660c522fba99e7a" # pragma: allowlist secret ), "node-purity-check": ( - "b633627655c01a98bcce39c4e998529c976659f10cdd70f1a5969772bf8444eb" # pragma: allowlist secret + "11a091eb4a9382801797ca89fef8090e43385b5b1c991949d7b4ed13afef3b06" # pragma: allowlist secret ), "enum-governance": ( "a46c3a30ecb40d58ad032a1920f24b93cef38716bccecd708e7a41014499727b" # pragma: allowlist secret ), "contract-config-compliance": ( - "9395479aa043f2b166bcc63b7229c3328f28be83659e395d64a15c520f4f43f6" # pragma: allowlist secret + "e37fad25fe594d66ccb0bd113e5be34a418301a06121500936ad8d8eb11e677c" # pragma: allowlist secret ), "breaking-schema-change": ( - "ee0658f793f00e8e6f179d913343d37504e128dcf73a459b075230adae4a64d8" # pragma: allowlist secret + "9409004bc80f756cb90e51d387dc888fd3a2f62b70d45ea0d87bb32dcbc68389" # pragma: allowlist secret ), "no-env-fallbacks": ( - "a0f24383df3a8974b1615035d4df05eb67924a1cda40d9c7a7f85379d816a475" # pragma: allowlist secret + "e6ee4cb21d27caaba2d3a0514683f4df9ab448d588e7ce107f1c610f4c928cb7" # pragma: allowlist secret ), "detect-secrets": ( - "e52c18585e93fd68b8ea8e1db1689fb5bd255e352e176e362b359ede4f346d9b" # pragma: allowlist secret + "4c87091b2cd5322878766b784d30158edbbd607fb304ea416df805067a0eb0c2" # pragma: allowlist secret ), "sdk-boundary-check": ( - "6701fd3e6101f2107010ee2b6fc436acb2aaac73fb136e8ef237d41f0b7b70bb" # pragma: allowlist secret + "30800b3a56c20e4c0d8b0365b17cb847f6c8149264aa03a8fe8e254136fc6cba" # pragma: allowlist secret ), "demo-path-topic-coherence": ( - "94bf6dfc92a03f6c281789316292e59c61ef6d7cd3aca07aae1f2e710d5da094" # pragma: allowlist secret + "8ff2f91d5081d541f624c2e55562b05dd1945d6c0a6913a26c5099b4dd455e72" # pragma: allowlist secret ), "dispatch-surface-test-required": ( - "7f292453c6e38618c81f8e39e6361183e936ed6fbcdf9823b3a4b6cb1d1fcd81" # pragma: allowlist secret + "6392730ab2e8a48efa83053c3dbb571a4f52c271e188b8e038a8be31535bedc4" # pragma: allowlist secret ), "naming-conventions": ( "4c018ad075660fc1c8d68c65c84c00b5c7bcd82ba4a70267baa7f6e2ce377aa7" # pragma: allowlist secret @@ -128,10 +128,10 @@ "6d4d2c5bb43d103924a336575a5022d17cedf8e086ceff477121255d52e71dca" # pragma: allowlist secret ), "no-noncanonical-lifecycle-classes": ( - "5cae23b307954cd43c5e17198715700a045986a07bb3f5a2941c334936de6b5a" # pragma: allowlist secret + "1b72b1142f9e17cff1a1299435b989685423de14563cf081fbb9f6323f929264" # pragma: allowlist secret ), "pull-request-workflow-ratchet": ( - "dbd395aa65670ec4c9afff78899ba001ea011ac80d8daaf1867bdfa11bb85bdb" # pragma: allowlist secret + "fe67a135dbeae73e46d4908920ba75b4419cb4ccd181b4d4c12d559f5f216a1f" # pragma: allowlist secret ), } diff --git a/tests/unit/validation/test_draft_state_admission_gate_omn16215.py b/tests/unit/validation/test_draft_state_admission_gate_omn16215.py index 3c51672c1c..8ca0fa2317 100644 --- a/tests/unit/validation/test_draft_state_admission_gate_omn16215.py +++ b/tests/unit/validation/test_draft_state_admission_gate_omn16215.py @@ -23,13 +23,20 @@ out to up to 40 shards at up to 70m each, `tests-integration` to 4 shards at up to 30m each -- together the overwhelming majority of this workflow's runner-minutes and wall-clock, regardless of which runner pool serves them. -The ~50 quality-gate leaves are left ungated: each is a single-shard job -completing in a few minutes, each is already required at STRICT (not -skip-tolerant) success via `SPEC_REQUIRED_VALIDATOR_JOBS` in -`scripts/ci/ci_summary_gate.py`, and touching 50 independent `if:` clauses -individually for comparatively small marginal savings is a large blast-radius -change this ticket's scoping guidance ("minimal always-on set") argues -against. +The ~50 quality-gate leaves are left ungated BY THIS TICKET (OMN-16215): +each is a single-shard job completing in a few minutes, each was already +required at STRICT (not skip-tolerant) success, and touching 50 independent +`if:` clauses individually for comparatively small marginal savings was a +large blast-radius change this ticket's scoping guidance ("minimal always-on +set") argued against for the DRAFT-state gate specifically. + +Update (OMN-16625): a separate, later ticket DID subsequently gate 16 of +these leaves -- but on `docs_only`, not on draft state, and via the +tests-gate-style skip-tolerant re-deriving pattern, not a bare `if:`. The +`_UNGATED_QUALITY_GATE_LEAF_SAMPLE` assertions below only pin the absence of +"draft" in each leaf's `if:` condition, so they remain true and unchanged -- +this note exists so a reader doesn't conclude from the sample name that these +jobs still have no `if:` at all. Fail-closed proof is structural, not new code: `test-parallel`/ `tests-integration` feed `Tests Gate` (`tests-gate` job), whose existing diff --git a/tests/unit/validation/test_quality_gate_docs_only_short_circuit_omn16625.py b/tests/unit/validation/test_quality_gate_docs_only_short_circuit_omn16625.py new file mode 100644 index 0000000000..8a0ccc086c --- /dev/null +++ b/tests/unit/validation/test_quality_gate_docs_only_short_circuit_omn16625.py @@ -0,0 +1,346 @@ +# SPDX-FileCopyrightText: 2025 OmniNode.ai Inc. +# SPDX-License-Identifier: MIT +"""OMN-16625: markdown-only diffs skip heavy CI (quality-gate leaves). + +Operator ruling (verbatim): "Changes to markdown files should not trigger CI +except for the length validator." "The length validator" is the existing +`doc-content-scan` job today (stand-in for the not-yet-merged OMN-16589 +kb-doc-gate) -- it and the 9 other `SPEC_REQUIRED_VALIDATOR_JOBS` members +(scripts/ci/ci_summary_gate.py) must keep running to strict success on every +diff, docs-only or not. + +Before this ticket, `zone-filter` (the existing docs-only detector, +scripts/zone_diff_filter.py) ran AFTER `quality-gate`'s ~26 leaf validator +jobs (`needs: [quality-gate]`) -- too late in the DAG to gate those leaves on +`docs_only`. Only the Phase 2 test-execution matrix (`test-parallel`, +`detect-changes`, and transitively `tests-integration`/`shadow-compare`) was +already gated, via `tests-gate`'s existing OMN-15315 per-upstream skip +policy. + +This ticket: + 1. Detaches `zone-filter` from `needs: [quality-gate]` so `docs_only` is + known immediately, in parallel with quality-gate's leaves. + 2. Gates the 16 NON-spec-required quality-gate leaves on `docs_only` via + `needs: [zone-filter]` + `if: always() && needs.zone-filter.outputs.docs_only != 'true'`. + 3. Rewrites `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 itself 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 -- a skip on a + non-docs-only diff still fails `quality-gate` closed. + 4. Leaves `contract-compliance`, `boundary-validation`, and + `occ-companion-merged` fully untouched -- `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, not bundled here. + +Fail-closed proof is structural: `quality-gate` is a `GATE_JOBS` completeness +anchor in `ci_summary_gate.py` (must be present+completed+success|skipped), +so a `quality-gate` failure fails `CI Summary` (the sole required +branch-protection context on `dev`, verified live via `gh api graphql`) +closed. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest +import yaml + +pytestmark = pytest.mark.unit + +REPO_ROOT = Path(__file__).resolve().parents[3] +WORKFLOW_PATH = REPO_ROOT / ".github" / "workflows" / "ci.yml" +MANIFEST_PATH = REPO_ROOT / ".github" / "required-checks.yaml" + +DOCS_ONLY_IF = "always() && needs.zone-filter.outputs.docs_only != 'true'" + +# The 16 quality-gate leaves this ticket gates on docs_only -- everything in +# quality-gate's `needs:` list except the 10 SPEC_REQUIRED_VALIDATOR_JOBS +# members (see _STRICT_SPEC_REQUIRED_LEAVES below). +_DOCS_ONLY_GATED_LEAVES = ( + "pyright", + "exports-validation", + "lint-imports", + "check-deterministic-skills", + "docs-validation", + "node-purity-check", + "detect-secrets", + "sdk-boundary-check", + "contract-config-compliance", + "breaking-schema-change", + "no-env-fallbacks", + "demo-path-topic-coherence", + "dispatch-surface-test-required", + "no-noncanonical-lifecycle-classes", + "pydantic-extra-forbid", + "pull-request-workflow-ratchet", +) + +# scripts/ci/ci_summary_gate.py SPEC_REQUIRED_VALIDATOR_JOBS members that are +# ALSO quality-gate leaves (job KEY, not the ci_summary_gate.py job NAME). +# These must stay unconditional and strictly-success on every diff, docs-only +# included -- validator-requirements.yaml governs them, and changing that +# spec is explicitly out of this ticket's scope. +_STRICT_SPEC_REQUIRED_LEAVES = ( + "lint", + "mypy-validation-scripts", + "core-infra-boundary", + "enum-governance", + "naming-conventions", + "pydantic-patterns", + "aislop-patterns", + "doc-content-scan", + "no-new-os-environ", + "spdx-headers", +) + +# Manifest job_path values among the 16 gated leaves that carry a +# `mode: REQUIRED` row in required-checks.yaml (4 of the 16 -- lint-imports, +# no-noncanonical-lifecycle-classes, pydantic-extra-forbid, +# pull-request-workflow-ratchet -- were already removed/never added, per the +# manifest's own 2026-07-25 reconcile note, and need no row update). +_GATED_LEAVES_WITH_MANIFEST_ROWS = ( + "pyright", + "exports-validation", + "check-deterministic-skills", + "docs-validation", + "node-purity-check", + "detect-secrets", + "sdk-boundary-check", + "contract-config-compliance", + "breaking-schema-change", + "no-env-fallbacks", + "demo-path-topic-coherence", + "dispatch-surface-test-required", +) + + +def _ci_workflow() -> dict[str, object]: + data = yaml.safe_load(WORKFLOW_PATH.read_text()) + assert isinstance(data, dict) + return data + + +def _ci_job(name: str) -> dict[str, object]: + jobs = _ci_workflow()["jobs"] + assert isinstance(jobs, dict) + job = jobs[name] + assert isinstance(job, dict) + return job + + +def _quality_gate_run_script() -> str: + steps = _ci_job("quality-gate")["steps"] + assert isinstance(steps, list) + run = steps[0]["run"] + assert isinstance(run, str) + return run + + +def _manifest_gates() -> list[dict[str, object]]: + data = yaml.safe_load(MANIFEST_PATH.read_text()) + gates = data["gates"] + assert isinstance(gates, list) + return gates + + +def _manifest_row_for_job_path(job_id: str) -> dict[str, object]: + for gate in _manifest_gates(): + if gate.get("job_path") == [job_id]: + return gate + raise AssertionError(f"no required-checks.yaml row with job_path == [{job_id!r}]") + + +class TestZoneFilterIsDetachedAndAlwaysTrue: + """zone-filter must compute docs_only immediately, not after + quality-gate's leaves -- otherwise those leaves cannot gate on it.""" + + def test_zone_filter_has_no_needs(self) -> None: + job = _ci_job("zone-filter") + assert "needs" not in job or not job["needs"] + + def test_zone_filter_if_is_provably_always_true(self) -> None: + job = _ci_job("zone-filter") + assert str(job["if"]).strip() == "always()" + + def test_zone_filter_no_longer_depends_on_quality_gate_result(self) -> None: + """RED-before control: the pre-OMN-16625 shape gated zone-filter's + own `if:` on `needs.quality-gate.result == 'success'` -- pin that + the live condition has moved past it.""" + pre_gate_if = "always() && needs.quality-gate.result == 'success'" + live_if = str(_ci_job("zone-filter")["if"]).strip() + assert live_if != pre_gate_if + assert "quality-gate" not in live_if + + +class TestDocsOnlyGatedLeaves: + @pytest.mark.parametrize("job_id", _DOCS_ONLY_GATED_LEAVES) + def test_leaf_needs_zone_filter(self, job_id: str) -> None: + job = _ci_job(job_id) + needs = job.get("needs") + assert needs == ["zone-filter"], f"{job_id}: needs={needs!r}" + + @pytest.mark.parametrize("job_id", _DOCS_ONLY_GATED_LEAVES) + def test_leaf_if_gates_on_docs_only(self, job_id: str) -> None: + job = _ci_job(job_id) + condition = str(job["if"]).strip() + assert condition == DOCS_ONLY_IF, f"{job_id}: if={condition!r}" + + @pytest.mark.parametrize("job_id", _DOCS_ONLY_GATED_LEAVES) + def test_leaf_is_not_a_spec_required_validator(self, job_id: str) -> None: + """A gated leaf must never overlap the strict spec-required set -- + that would silently let a spec-required validator skip.""" + assert job_id not in _STRICT_SPEC_REQUIRED_LEAVES + + +class TestStrictSpecRequiredLeavesStayUnconditional: + """The 10 SPEC_REQUIRED_VALIDATOR_JOBS members must run to STRICT + success on every diff, including docs-only -- this is "the length + validator" carve-out from the operator's ruling.""" + + @pytest.mark.parametrize("job_id", _STRICT_SPEC_REQUIRED_LEAVES) + def test_leaf_has_no_docs_only_if(self, job_id: str) -> None: + job = _ci_job(job_id) + condition = str(job.get("if", "")) + assert "docs_only" not in condition, ( + f"{job_id} is spec-required (validator-requirements.yaml) and must " + "run unconditionally -- it must never gate on docs_only" + ) + + @pytest.mark.parametrize("job_id", _STRICT_SPEC_REQUIRED_LEAVES) + def test_leaf_does_not_need_zone_filter(self, job_id: str) -> None: + job = _ci_job(job_id) + needs = job.get("needs") + if needs: + assert isinstance(needs, list) + assert "zone-filter" not in needs, ( + f"{job_id} is spec-required and must not depend on zone-filter" + ) + + def test_doc_content_scan_is_the_carved_out_length_validator(self) -> None: + """Explicit single-job pin for the operator's exact carve-out + phrase ("the length validator") -- doc-content-scan is its current + stand-in until OMN-16589's kb-doc-gate lands.""" + job = _ci_job("doc-content-scan") + assert "if" not in job + assert "needs" not in job or not job["needs"] + + +class TestQualityGateAggregatorReDerivesDocsOnly: + def test_quality_gate_itself_stays_unconditional(self) -> None: + """quality-gate must never itself be a skip-vector -- it always + runs and always produces a real success/failure, mirroring + tests-gate.""" + assert str(_ci_job("quality-gate")["if"]).strip() == "always()" + + def test_quality_gate_needs_zone_filter(self) -> None: + needs = _ci_job("quality-gate")["needs"] + assert isinstance(needs, list) + assert "zone-filter" in needs + + def test_quality_gate_needs_every_gated_leaf_and_every_strict_leaf(self) -> None: + raw_needs = _ci_job("quality-gate")["needs"] + assert isinstance(raw_needs, list) + needs = set(raw_needs) + for job_id in _DOCS_ONLY_GATED_LEAVES + _STRICT_SPEC_REQUIRED_LEAVES: + assert job_id in needs, f"quality-gate no longer needs {job_id!r}" + + def test_step_reads_docs_only_from_zone_filter_output(self) -> None: + script = _quality_gate_run_script() + assert 'docs_only="${{ needs.zone-filter.outputs.docs_only }}"' in script, ( + "quality-gate must re-derive docs_only from zone-filter's own output" + ) + + def test_step_never_trusts_zone_filter_result_directly(self) -> None: + """Only the `docs_only` OUTPUT may be read -- never + `needs.zone-filter.result` (that would be trusting the detector + job's own conclusion instead of its computed verdict, which is not + the same thing zone-filter's own `if: always()` makes it -- a + successful run always concludes 'success' regardless of docs_only).""" + script = _quality_gate_run_script() + assert "needs.zone-filter.result" not in script + + @pytest.mark.parametrize("job_id", _STRICT_SPEC_REQUIRED_LEAVES) + def test_strict_leaf_var_is_checked_in_the_strict_tier(self, job_id: str) -> None: + script = _quality_gate_run_script() + tier1, _, _tier2 = script.partition("TIER 2") + assert job_id.replace("-", "_") in tier1 or _var_alias(job_id) in tier1 + + def test_skip_is_admitted_only_when_docs_only_true(self) -> None: + script = _quality_gate_run_script() + assert 'case "$RESULT" in' in script + assert "skipped)" in script + assert '"$docs_only" != "true"' in script + + def test_a_skip_on_a_non_docs_only_diff_fails_the_gate(self) -> None: + """Structural proof the skip-tolerant loop fails closed: the + `skipped)` branch's guard clause is followed by an `exit 1`.""" + script = _quality_gate_run_script() + skipped_branch = script.split("skipped)", 1)[1].split("failure|cancelled", 1)[0] + assert "exit 1" in skipped_branch + assert '"$docs_only" != "true"' in skipped_branch + + +# Job keys use hyphens; the bash variable names quality-gate's step assigns +# for the 10 strict leaves are hand-abbreviated, not a mechanical +# hyphen->underscore transform (matching the pre-existing style already in +# the step for the other ~40 vars) -- map the two that diverge. +_STRICT_VAR_ALIASES = { + "mypy-validation-scripts": "mypy_scripts", + "core-infra-boundary": "core_infra", + "enum-governance": "enum_gov", + "naming-conventions": "naming", + "pydantic-patterns": "pydantic", + "aislop-patterns": "aislop", + "doc-content-scan": "doc_content", + "no-new-os-environ": "no_new_os_environ", +} + + +def _var_alias(job_id: str) -> str: + return _STRICT_VAR_ALIASES.get(job_id, job_id.replace("-", "_")) + + +class TestDeliberatelyUntouchedJobs: + """contract-compliance / boundary-validation / occ-companion-merged are + direct, unwrapped GATE_JOBS entries with no re-deriving aggregator (see + module docstring) -- gating them here would repeat a PREVIOUSLY REJECTED + change (see contract-compliance's own in-file comment, OMN-14863).""" + + def test_contract_compliance_if_does_not_reference_docs_only(self) -> None: + condition = str(_ci_job("contract-compliance").get("if", "")) + assert "docs_only" not in condition + + def test_boundary_validation_has_no_if_at_all(self) -> None: + assert "if" not in _ci_job("boundary-validation") + + def test_occ_companion_merged_has_no_if_at_all(self) -> None: + assert "if" not in _ci_job("occ-companion-merged") + + +class TestRequiredChecksManifestOverrides: + @pytest.mark.parametrize("job_id", _GATED_LEAVES_WITH_MANIFEST_ROWS) + def test_row_has_neutral_ok_with_ticket_cited_rationale(self, job_id: str) -> None: + row = _manifest_row_for_job_path(job_id) + assert row["skip_semantics"] == "neutral_ok" + rationale = str(row["rationale"]) + assert re.search(r"\bOMN-\d+\b", rationale), ( + f"{job_id} row's rationale must cite a tracking ticket " + "(validate_no_required_check_skip_vectors.py only honors " + "neutral_ok with a cited ticket)" + ) + assert "OMN-16625" in rationale + + def test_zone_filter_row_reverted_to_never_now_that_if_is_always_true( + self, + ) -> None: + """zone-filter's `if:` is now provably ALWAYS_TRUE_FOR_PR (see + TestZoneFilterIsDetachedAndAlwaysTrue) -- the neutral_ok override + this row previously needed (OMN-14864) is no longer reachable.""" + row = _manifest_row_for_job_path("zone-filter") + assert row["skip_semantics"] == "never" From c91ecaf00b988a9dfa091038b4bf47e71d688f47 Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Sat, 29 Aug 2026 08:28:57 -0400 Subject: [PATCH 2/7] 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. --- ...est_detect_test_paths_ci_contract_omn16917.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/unit/scripts/ci/test_detect_test_paths_ci_contract_omn16917.py b/tests/unit/scripts/ci/test_detect_test_paths_ci_contract_omn16917.py index 82e3d232f5..4ceb424088 100644 --- a/tests/unit/scripts/ci/test_detect_test_paths_ci_contract_omn16917.py +++ b/tests/unit/scripts/ci/test_detect_test_paths_ci_contract_omn16917.py @@ -127,7 +127,17 @@ def test_omn16625_diff_selects_the_ci_contract_class_not_the_unit_tree() -> None def test_omn16625_diff_selects_its_own_touched_test_modules_at_file_grain() -> None: - """The OMN-16745 grain lesson: a touched test module narrows to ITSELF.""" + """The OMN-16745 grain lesson: a touched test module narrows to ITSELF. + + The real OMN-16625 test module reads `.github/workflows/ci.yml` and + `.github/required-checks.yaml` off disk (it asserts the quality-gate's + docs-only short circuit against those manifests), so once it lands it is + ALSO a member of the derived CI-contract class -- membership in the two + sets is not exclusive. What OMN-16745 actually guarantees, and what this + test proves, is narrower: a touched test module is always selected by + name, whether or not it independently qualifies for the derived class, + and it is never widened to its containing directory. + """ selection = compute_selection( changed_files=OMN_16625_DIFF, adjacency_path=ADJ, @@ -135,12 +145,10 @@ def test_omn16625_diff_selects_its_own_touched_test_modules_at_file_grain() -> N repo_root=REPO_ROOT, ) - # This module carries no `.github` literal, so it is NOT in the derived - # class — it is selected purely because the diff touches it. touched = ( "tests/unit/validation/test_quality_gate_docs_only_short_circuit_omn16625.py" ) - assert touched not in ci_contract_test_paths(REPO_ROOT) + assert touched in ci_contract_test_paths(REPO_ROOT) assert touched in selection.selected_paths # ...and it is NOT widened to its containing directory. assert "tests/unit/validation/" not in selection.selected_paths From 8c05c028b2fad8992ba0881b78701cc273f14e6a Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Sat, 29 Aug 2026 09:39:52 -0400 Subject: [PATCH 3/7] 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. --- .github/workflows/ci.yml | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c6ce64932b..a14e2e5873 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3613,8 +3613,17 @@ jobs: }} # zone-filter is in `needs:` so this gate can re-evaluate the SAME # precondition the test jobs are gated on (OMN-15315). It is read for its - # `docs_only` output only, never for its `.result`. - needs: [test-parallel, tests-integration, zone-filter] + # `docs_only` output only, never for its `.result`. quality-gate is ALSO + # in `needs:` (OMN-16625, CodeRabbit finding on PR#1618): since OMN-16625 + # detached zone-filter from `needs: [quality-gate]` so `docs_only` is + # known in parallel with quality-gate's leaves instead of after them, + # `docs_only` is now computed from the diff's file paths alone and no + # longer implies quality-gate succeeded — a genuinely docs-only diff can + # still fail quality-gate (e.g. a real doc-content-scan violation), which + # skips test-parallel/tests-integration via their own `quality-gate == + # success` precondition. Without this, that skip would be indistinguishable + # from a legitimate docs-only skip and this gate would wrongly pass. + needs: [test-parallel, tests-integration, zone-filter, quality-gate] if: always() steps: @@ -3626,12 +3635,14 @@ jobs: parallel="${{ needs.test-parallel.result }}" integration="${{ needs.tests-integration.result }}" docs_only="${{ needs.zone-filter.outputs.docs_only }}" + quality="${{ needs.quality-gate.result }}" echo "| Check | Result |" >> "$GITHUB_STEP_SUMMARY" echo "|-------|--------|" >> "$GITHUB_STEP_SUMMARY" echo "| Parallel Tests (dynamic splits) | $parallel |" >> "$GITHUB_STEP_SUMMARY" echo "| Integration Tests (4 splits) | $integration |" >> "$GITHUB_STEP_SUMMARY" echo "| zone-filter docs_only | ${docs_only:-(unset)} |" >> "$GITHUB_STEP_SUMMARY" + echo "| quality-gate | $quality |" >> "$GITHUB_STEP_SUMMARY" # OMN-15315 (OMN-15304 vector 6 — result triage). The previous pass # condition admitted `success` OR `skipped` for both upstreams, so a @@ -3650,13 +3661,14 @@ jobs: # because the vector-6 analyzer scans this whole `run:` blob for # result tokens and a comment would register as an untriaged # upstream), so exactly ONE of their skips is legitimate — a - # docs-only PR. This - # gate re-evaluates that precondition itself and admits `skipped` only - # when `docs_only == 'true'`. A skip caused by a non-success - # `quality-gate` is NOT admitted: tests that never ran because quality - # failed are absent tests, and this gate must say so. `zone-filter` - # itself is gated on `quality-gate` succeeding, so if quality fails, - # `docs_only` is empty and this gate fails closed on that path too. + # docs-only PR whose quality-gate ALSO succeeded. This gate + # re-evaluates that full precondition itself (OMN-16625: docs_only + # AND quality-gate success, not docs_only alone — see the `needs:` + # comment above for why quality-gate's own success can no longer be + # inferred from docs_only being set) and admits `skipped` only when + # both hold. A skip caused by a non-success `quality-gate` is NOT + # admitted: tests that never ran because quality failed are absent + # tests, and this gate must say so. # # The gate stops at the first offending upstream; the per-upstream # results are already written to the job summary above. @@ -3670,12 +3682,12 @@ jobs: echo "$NAME: success" ;; skipped) - if [ "$docs_only" != "true" ]; then + if [ "$docs_only" != "true" ] || [ "$quality" != "success" ]; then echo "**Tests Gate: FAILED**" >> "$GITHUB_STEP_SUMMARY" - echo "::error::$NAME was SKIPPED on a non-docs-only change (zone-filter docs_only='${docs_only:-unset}') — the absence of a test result is not a passing one." + echo "::error::$NAME was SKIPPED but the docs-only precondition does not hold (zone-filter docs_only='${docs_only:-unset}', quality-gate result='${quality:-unset}') — the absence of a test result is not a passing one." exit 1 fi - echo "$NAME: skipped (docs-only change — declared precondition not met)" + echo "$NAME: skipped (docs-only change, quality-gate succeeded — declared precondition met)" ;; failure|cancelled) echo "**Tests Gate: FAILED**" >> "$GITHUB_STEP_SUMMARY" From 2828391ce6c69f73d8932039c35d4cfc8681f802 Mon Sep 17 00:00:00 2001 From: Jonah Gray Date: Sat, 29 Aug 2026 08:15:30 -0400 Subject: [PATCH 4/7] 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 --- CHANGELOG.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f906228c10..55d8951a52 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,12 +4,19 @@ ### Breaking Changes - bump omnibase_core to v0.47.0 — `ModelWidgetConfigStatusGrid.status_colors` is **removed** (#1612). The field carried a `Mapping[str, str]` of raw hex colours in the config's own default, which is the decision the theme contract exists to make; it is replaced by `severity_roles: tuple[ModelSeverityRole, ...]` naming theme tokens, and colours now resolve through the active theme instance. The model is `extra="forbid"`, so any caller still passing `status_colors=` raises. Minor bump per pre-1.0 semver convention for a breaking public-API deletion. +- **`ModelEventEnvelope` now declares `extra="forbid"` (#1611).** `extra` was previously **undeclared** on this model, so Pydantic's `"ignore"` default applied and any undeclared keyword was silently discarded at construction. Constructing the envelope with a keyword it does not declare now raises `ValidationError ... [type=extra_forbidden]` instead. This half of #1611 is **not additive** and is the reason v0.47.0 is a breaking release for envelope producers, independent of the `status_colors` deletion above. + + **Who this breaks:** any producer passing a keyword `ModelEventEnvelope` does not declare. The declared set is `payload`, `envelope_id`, `envelope_timestamp`, `correlation_id`, `source_tool`, `target_tool`, `metadata`, `security_context`, `event_type`, `payload_type`, `payload_schema_version`, `priority`, `timeout_seconds`, `retry_count`, `request_id`, `trace_id`, `span_id`, `tenant_id`, `onex_version`, `envelope_version`. + + **Migration:** producer identity belongs in the long-declared `source_tool` field — `source=` was never a field on this model and was being dropped on every call before 0.47.0. Rename `source=` to `source_tool=`. There is no `source_version` field; the envelope's version fields are `onex_version`, `envelope_version`, and `payload_schema_version`. Measured blast radius on the first consumer to resolve 0.47.0 (omnibase_infra): 11 production call sites and 138 failing tests, all one signature. + + **Why it is correct anyway:** silently discarding a producer's attribution is the failure class #1611 was opened to close, and `extra="forbid"` is mandatory on every model in this repo (enforced by `omnibase_core.validators.pydantic_extra_forbid`). 0.47.0 does not create the data loss — it converts a silent loss into a loud failure. ### Features - feat theme instances + theme catalog — `ModelThemeInstance`, `ModelThemeCatalog`, `ModelThemeCatalogEntry`, `ModelThemeActivation`, `util_theme_catalog`, and the `onex.theme.{light,dark,warm}` 1.0.0 theme contracts (#1608) - feat one versioned widget envelope — `ModelWidgetEnvelope`, `ModelWidgetProvenance`, `util_widget_envelope` (#1610) - feat semantic severity for StatusGrid — `EnumStatusSeverity`, `EnumStatusSecondaryKind`, `ModelSeverityRole` + `DEFAULT_SEVERITY_ROLES`, `ModelSeverityVerdict`, `ModelStatusSecondary` (#1612) -- feat the transport envelope carries the tenant dimension — an OPTIONAL `tenant` field on `ModelEventEnvelope` (additive; the model stays `extra="forbid"`) (#1611) +- feat the transport envelope carries the tenant dimension — an OPTIONAL `tenant_id` field on `ModelEventEnvelope`, attribution only, never proof of entitlement (#1611). This half is additive. The same PR's `extra="forbid"` declaration is **not** — see Breaking Changes above. - feat `onex user-config` CLI surface + `ModelCliUserConfig` family, unifying the `~/.onex/config.yaml` schema across all CLI writers (#1604) ### Changes From 8453a12982235756eb0b277527785ae147169d9e Mon Sep 17 00:00:00 2001 From: Jonah Gray Date: Sat, 29 Aug 2026 09:41:36 -0400 Subject: [PATCH 5/7] 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 259e9bc39, fdf4c232d8, 3c94923a6b). 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 --- .github/workflows/ci.yml | 58 +++++++++++++++---- tests/unit/scripts/ci/test_ci_summary_gate.py | 10 +++- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a14e2e5873..e77e980e83 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3757,11 +3757,16 @@ jobs: # `reject-required-check-skip-vector` pre-commit hook (OMN-14863) exists # to catch (confirmed live: it rejected an earlier draft of this fix that # narrowed the `if:`). Keeping `if:` unconditional means the job always - # runs to completion; the fix is entirely in the step below, which now - # `exit 1`s instead of `exit 0` on merge_group/push -- a `failure` - # conclusion is caught by the default-deny sweep regardless of GATE_JOBS/ - # STRICT_SUCCESS_JOBS membership, so this fails closed without ever - # relying on an `if:`-driven skip. + # runs to completion; the step below fails closed (`exit 1`, caught by + # the default-deny sweep regardless of GATE_JOBS/STRICT_SUCCESS_JOBS + # membership) whenever it cannot resolve a PR number to validate against + # -- never an `if:`-driven skip. OMN-16346: on `push` that used to be + # EVERY event (permanently reddening dev's post-merge signal, since push + # never populates github.event.pull_request.number); the step now + # resolves the merge commit's originating PR via the associated-PR API + # first and only falls through to fail-closed if that resolution itself + # comes up empty. `merge_group` is unchanged (still fails closed) -- no + # live dev merge queue exercises it today. if: github.event_name == 'pull_request' || github.event_name == 'merge_group' || github.event_name == 'push' steps: - uses: actions/checkout@v7 @@ -3880,14 +3885,43 @@ jobs: working-directory: onex_change_control run: | set -euo pipefail + # OMN-16346: the job-level `if:` above deliberately admits + # pull_request, merge_group, AND push (it cannot be narrowed to + # pull_request -- see that comment), so this branch is NOT an + # invariant-broke escape hatch, it is the normal path on every dev + # push. Prior text here claimed the job "now only runs on + # pull_request" and unconditionally exited 1; that made + # push-event runs (i.e. every post-merge dev commit) fail closed + # by construction, permanently reddening dev's post-merge signal + # (OMN-16013 / OMN-16055 / OMN-16346). + # + # Push-path fix: resolve the PR that produced this push's commit + # and validate against IT, instead of failing blind. GitHub + # associates a squash-merge commit with its originating PR (the + # documented "List pull requests associated with a commit" API), + # so for the ordinary squash-merge-to-dev case this evaluates the + # exact same check_values the PR's own pull_request run already + # evaluated -- not a vacuous pass, not a skip, and not a second + # independent judgment that could disagree with the PR verdict. + if [ -z "${PR_NUMBER:-}" ] && [ "${{ github.event_name }}" = "push" ]; then + RESOLVED_PR="$(gh api "repos/${{ github.repository }}/commits/${{ github.sha }}/pulls" \ + --jq '[.[] | select(.merged_at != null)] | sort_by(.merged_at) | last | .number // empty' \ + 2>/dev/null || true)" + if [ -n "$RESOLVED_PR" ]; then + echo "::notice::event=push, no PR in event payload -- resolved merged PR #${RESOLVED_PR} from commit ${{ github.sha }} via the associated-PR API" + PR_NUMBER="$RESOLVED_PR" + fi + fi if [ -z "${PR_NUMBER:-}" ]; then - # enforce-everything gate audit (fail-closed, not fail-open): this - # job now only runs `if: github.event_name == 'pull_request'` - # (see the job-level `if:` above), which always populates - # PR_NUMBER. Reaching this branch means that invariant broke some - # other way -- fail closed (exit 1), never vacuously pass a DoD - # check that did not actually run a single check_value. - echo "::error::No PR number (event=${{ github.event_name }}); DoD check_values are PR-scoped and this job should only run on pull_request. Failing closed." + # Still unresolved: a pull_request run missing PR_NUMBER (the + # invariant above broke some other way), a merge_group run (no + # live dev merge queue today; this repo has no merge_group + # associated-PR resolution yet -- see OMN-16346 follow-up if one + # is ever enabled), or a push whose commit genuinely has no + # associated PR (direct push, rebase merge, force push). Fail + # closed (exit 1), never vacuously pass a DoD check that did not + # actually run a single check_value. + echo "::error::No PR number (event=${{ github.event_name }}); DoD check_values are PR-scoped and could not be resolved for this event. Failing closed." exit 1 fi uv run python scripts/ci/run_contract_compliance_check.py \ diff --git a/tests/unit/scripts/ci/test_ci_summary_gate.py b/tests/unit/scripts/ci/test_ci_summary_gate.py index d3c78f5514..e17c35456a 100644 --- a/tests/unit/scripts/ci/test_ci_summary_gate.py +++ b/tests/unit/scripts/ci/test_ci_summary_gate.py @@ -556,7 +556,15 @@ def test_empty_pr_number_branch_fails_closed_not_open(self) -> None: text = CI_YML.read_text(encoding="utf-8") marker = 'if [ -z "${PR_NUMBER:-}" ]; then' idx = text.index(marker) - branch = text[idx : idx + 400].split("\n fi", 1)[0] + # OMN-16346: bound the branch by its own terminator only. This used to + # slice a fixed `text[idx : idx + 400]` window first, which silently + # made the pin depend on how many COMMENT characters happen to sit + # between the `if` and the `exit 1` -- adding an explanatory comment + # inside the branch pushed `exit 1` past offset 400 and failed this + # test while the fail-closed property it guards was fully intact. The + # `\n fi` split already bounds the branch exactly, so the + # character cap was never load-bearing, only brittle. + branch = text[idx:].split("\n fi", 1)[0] assert "exit 1" in branch, branch assert "exit 0" not in branch, branch From de880d5508054a02d29bee5d7a3aa00bc1cbd031 Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Sat, 29 Aug 2026 10:09:02 -0400 Subject: [PATCH 6/7] 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..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..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. --- .github/required-checks.yaml | 6 ++- .github/workflows/ci.yml | 87 +++++++++++++++++++++++------------- 2 files changed, 60 insertions(+), 33 deletions(-) diff --git a/.github/required-checks.yaml b/.github/required-checks.yaml index 14b1eb230a..653f856419 100644 --- a/.github/required-checks.yaml +++ b/.github/required-checks.yaml @@ -612,7 +612,11 @@ gates: workflow: ci.yml job_path: ["quality-gate"] skip_semantics: never - rationale: "Live required status check on omnibase_core dev branch protection." + rationale: "Live required status check on omnibase_core dev branch protection. OMN-16625's two-tier + docs_only short-circuit inlines the literal `${{ needs..result }}` expression in each aggregator + for-loop's item list (tests-gate's proven shape) rather than routing through a preamble-assigned + shell variable, so the vector-6 analyzer proves the triage fail-closed on its own -- no result_triage + waiver needed." - name: "Tests Gate" mode: REQUIRED diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e77e980e83..5bb4174a2a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2946,25 +2946,45 @@ jobs: # on every diff, including a docs-only one -- doc-content-scan is "the # length validator" the operator's docs-only-CI ruling explicitly keeps # running; a `skipped` here is never legitimate. + # + # The `${{ needs..result }}` expressions are inlined directly in + # the for-loop list (not the preamble-assigned `$lint`-style vars used + # for the summary table above) -- this is deliberate, not cosmetic: + # the OMN-15304 vector-6 skip-guard's aggregator-loop recognizer keys + # on the literal GHA expression appearing in the loop's own item list + # (`_rebind_aggregator_loop`, mirroring tests-gate's proven shape + # below). A loop fed through an already-bound shell variable is + # invisible to that recognizer and reports TRIAGE_UNVERIFIABLE even + # though the case statement is functionally fail-closed. for check in \ - "lint=$lint" \ - "mypy_scripts=$mypy_scripts" \ - "core_infra=$core_infra" \ - "enum_gov=$enum_gov" \ - "naming=$naming" \ - "pydantic=$pydantic" \ - "aislop=$aislop" \ - "doc_content=$doc_content" \ - "no_new_os_environ=$no_new_os_environ" \ - "spdx_headers=$spdx_headers"; do + "lint=${{ needs.lint.result }}" \ + "mypy_scripts=${{ needs.mypy-validation-scripts.result }}" \ + "core_infra=${{ needs.core-infra-boundary.result }}" \ + "enum_gov=${{ needs.enum-governance.result }}" \ + "naming=${{ needs.naming-conventions.result }}" \ + "pydantic=${{ needs.pydantic-patterns.result }}" \ + "aislop=${{ needs.aislop-patterns.result }}" \ + "doc_content=${{ needs.doc-content-scan.result }}" \ + "no_new_os_environ=${{ needs.no-new-os-environ.result }}" \ + "spdx_headers=${{ needs.spdx-headers.result }}"; do NAME="${check%%=*}" RESULT="${check#*=}" - if [[ "$RESULT" != "success" ]]; then - echo "" >> $GITHUB_STEP_SUMMARY - echo "**Quality Gate: FAILED**" >> $GITHUB_STEP_SUMMARY - echo "::error::$NAME (spec-required validator) did not pass (result: $RESULT) -- this check is never allowed to skip, docs-only diff or not." - exit 1 - fi + case "$RESULT" in + success) + : ;; + failure|cancelled|skipped) + echo "" >> $GITHUB_STEP_SUMMARY + echo "**Quality Gate: FAILED**" >> $GITHUB_STEP_SUMMARY + echo "::error::$NAME (spec-required validator) did not pass (result: $RESULT) -- this check is never allowed to skip, docs-only diff or not." + exit 1 + ;; + *) + echo "" >> $GITHUB_STEP_SUMMARY + echo "**Quality Gate: FAILED**" >> $GITHUB_STEP_SUMMARY + echo "::error::$NAME returned unrecognised result '$RESULT' -- failing closed." + exit 1 + ;; + esac done # TIER 2 -- SKIP-TOLERANT (the 16 leaves gated on docs_only in this PR). @@ -2972,23 +2992,26 @@ jobs: # (re-derived here, never trusted from a leaf's raw `.result`) says # 'true' -- exactly tests-gate's existing per-upstream skip policy for # test-parallel/tests-integration (OMN-15315), applied to Phase 1. + # + # Literal `${{ needs..result }}` expressions inlined here for the + # same OMN-15304 vector-6 recognizer reason as TIER 1 above. for check in \ - "pyright=$pyright" \ - "exports=$exports" \ - "lint_imports=$lint_imports" \ - "det_skills=$det_skills" \ - "docs=$docs" \ - "purity=$purity" \ - "secrets=$secrets" \ - "sdk_boundary=$sdk_boundary" \ - "contract_config=$contract_config" \ - "breaking_schema=$breaking_schema" \ - "no_env_fallbacks=$no_env_fallbacks" \ - "demo_path_gate=$demo_path_gate" \ - "dispatch_surface=$dispatch_surface" \ - "noncanonical_classes=$noncanonical_classes" \ - "extra_forbid=$extra_forbid" \ - "pr_workflow_ratchet=$pr_workflow_ratchet"; do + "pyright=${{ needs.pyright.result }}" \ + "exports=${{ needs.exports-validation.result }}" \ + "lint_imports=${{ needs.lint-imports.result }}" \ + "det_skills=${{ needs.check-deterministic-skills.result }}" \ + "docs=${{ needs.docs-validation.result }}" \ + "purity=${{ needs.node-purity-check.result }}" \ + "secrets=${{ needs.detect-secrets.result }}" \ + "sdk_boundary=${{ needs.sdk-boundary-check.result }}" \ + "contract_config=${{ needs.contract-config-compliance.result }}" \ + "breaking_schema=${{ needs.breaking-schema-change.result }}" \ + "no_env_fallbacks=${{ needs.no-env-fallbacks.result }}" \ + "demo_path_gate=${{ needs.demo-path-topic-coherence.result }}" \ + "dispatch_surface=${{ needs.dispatch-surface-test-required.result }}" \ + "noncanonical_classes=${{ needs.no-noncanonical-lifecycle-classes.result }}" \ + "extra_forbid=${{ needs.pydantic-extra-forbid.result }}" \ + "pr_workflow_ratchet=${{ needs.pull-request-workflow-ratchet.result }}"; do NAME="${check%%=*}" RESULT="${check#*=}" case "$RESULT" in From 3bb626e00ea40598a16eb9c4bf6fae14a538f202 Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Sat, 29 Aug 2026 10:29:00 -0400 Subject: [PATCH 7/7] 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..result }}\` -- but GitHub Actions templates EVERY \${{ ... }} occurrence in a run: block, including inside a bash \`#\` comment, before the shell ever sees it. \`needs..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. --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5bb4174a2a..161de8b944 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2947,7 +2947,7 @@ jobs: # length validator" the operator's docs-only-CI ruling explicitly keeps # running; a `skipped` here is never legitimate. # - # The `${{ needs..result }}` expressions are inlined directly in + # The `needs..result` GHA expressions are inlined directly in # the for-loop list (not the preamble-assigned `$lint`-style vars used # for the summary table above) -- this is deliberate, not cosmetic: # the OMN-15304 vector-6 skip-guard's aggregator-loop recognizer keys @@ -2993,7 +2993,7 @@ jobs: # 'true' -- exactly tests-gate's existing per-upstream skip policy for # test-parallel/tests-integration (OMN-15315), applied to Phase 1. # - # Literal `${{ needs..result }}` expressions inlined here for the + # Literal `needs..result` GHA expressions inlined here for the # same OMN-15304 vector-6 recognizer reason as TIER 1 above. for check in \ "pyright=${{ needs.pyright.result }}" \