Repository navigation
feat(OMN-14666): WS7 fan-out #2 — wire omnimarket arch-* validators as pre-commit hooks (DRY) + fail-loud meta-gate - #1783
Merged
jonahgabriel merged 1 commit intoJul 16, 2026
Conversation
…s pre-commit hooks (DRY) + fail-loud meta-gate Applies the WS7 CI<->pre-commit gate-parity pattern (plan docs/plans/2026-07-15-ci-precommit-gate-parity-plan.md §4; canary omniclaude#1904 / OMN-14655, MERGED) to omnimarket — fan-out #2 only. What this does - Wire two STATIC arch/registry CI checks as `repo: local` pre-commit hooks invoking the EXACT ci.yml entrypoint (DRY, no re-impl, plan §2/§5): * arch-node-metadata-dependency-scope entry `uv run python scripts/ci/check_node_metadata_dependencies.py` (byte-identical to ci.yml:200) * arch-unimported-handler-check entry `uv run python scripts/ci/check_unimported_handlers.py` (byte-identical to ci.yml:259) Per-check STATIC classification receipt (plan §3a/D3) in the config comment: both are pure AST/stdlib scans, NO sibling checkout, NO env vars, NO network, NO runtime execution. - Fix DRIFT-5 false-green: `default_install_hook_types: [pre-commit]` -> `[pre-commit, pre-push, commit-msg]`, so the strict-mypy (stages:[pre-push]) and reject-deploy-gate-skip-token-commit-msg (stages:[commit-msg]) hooks — both required-CI mirrors that `pre-commit install` NEVER installed before — now actually install and enforce. - Add the fail-loud meta-gate + pin-parity ratchet (plan §3d.7/§3d.8), ported from the canary: scripts/validation/validate_precommit_fail_loud.py + validate_precommit_pin_parity.py, the two `repo: local` hooks, and the CI mirror .github/workflows/precommit-fail-loud-gate.yml. pin-parity PIN_PAIRS = no-noncanonical-lifecycle-classes (pre-commit rev 63635097… already == the ci.yml:1498 `omnibase-core @ git+…@<sha>` pin). Net-negative-surface (plan §5/D11): converts two never-installed hooks (strict-mypy, commit-msg skip-token) into installed, enforcing hooks, and the fail-loud meta-gate retires the exit-0-on-missing false-green CLASS (mechanism, not rule — CLAUDE.md §5) rather than adding advisory surface. Scope guard (canary): omnimarket ONLY, do NOT fan out. Excluded and tracked separately: canonical handler-shape GROWTH ratchet (OMN-14563); runtime/compliance/aislop sweeps + contract-compliance (sibling-checkout / PR-diff, CI-only or later); reject-node-topics.py inline bash (needs §2 D2 entrypoint extraction); run_quality_gate_calibration.py (imports+executes handler code = RUNTIME); topic-drift/enum-drift (dep classification pending). dod_evidence (OMN-14666, proof gate plan §4; run from worktree off origin/dev): - BASELINE GREEN: both arch checks pass on clean tree via the exact CI command. - SEEDED-BREAK REJECTED (parity = same script -> same message CI emits): unimported-handler (unwired Handler class) -> exit 1; metadata-dep (undeclared `import qdrant_client`) -> `missing dependencies for imports: qdrant-client`, exit 1; pin-parity (flipped no-noncanonical rev) -> FAIL on divergence, exit 1. Each reverted; tree clean before commit. - META-GATE GREEN on final config via CI invocation (uv run --no-project --with pyyaml python3 …): fail-loud + pin-parity both OK. - BUDGET: 4 new hooks measured via pre-commit run = 1.22/1.60/0.23/0.22s (~3.3s combined), under the <15s pre-commit budget (plan §4). - The only pre-commit red is the PRE-EXISTING topic-naming-lint hard-exit-2 when the ../omnibase_infra sibling is unresolved — the §3d.2 env-parity prerequisite, NOT introduced here; passes with OMNIBASE_INFRA_PATH set. Out of scope. Related: OMN-14655 (parent/canary), OMN-14643 (epic WS7), OMN-14563 (excluded).
jonahgabriel
added a commit
to OmniNode-ai/onex_change_control
that referenced
this pull request
Jul 16, 2026
…imarket#1783 (#4266) * evidence(OMN-14666): author OCC companion for OmniNode-ai/omnimarket#1783 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head 10b52f340ebba44047a9c7dd896f0957261ec658. * evidence(OMN-14666): self-bind OCC#4266 + rebind contract_sha256 * evidence(OMN-14666): normalize OCC 4266 checks --------- Co-authored-by: omnimarket-bot <bot@omninode.ai>
✅ Architectural Review — PASSEDErrors: 0 What this checks
No architectural violations found. Static architectural lint — no model inference (OMN-14176). |
✅ Hostile Reviewer — PASSEDBlocking findings (critical/error): 0 Gate semantics (pilot phase)
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524) |
jonahgabriel
marked this pull request as ready for review
July 16, 2026 10:39
📝 WalkthroughWalkthroughAdds fail-loud and pin-parity validation scripts, registers architecture and governance hooks in pre-commit, expands installed stages, and introduces a path-filtered GitHub Actions gate. ChangesPre-commit governance
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubEvent
participant Preflight
participant FailLoudValidator
participant PinParityValidator
GitHubEvent->>Preflight: Trigger occ-preflight
Preflight->>FailLoudValidator: Run fail-loud validation
Preflight->>PinParityValidator: Run pin-parity validation
FailLoudValidator-->>GitHubEvent: Return status
PinParityValidator-->>GitHubEvent: Return status
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
scripts/validation/validate_precommit_pin_parity.py (1)
39-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
_CI_PIN_REhardcodesomnibase_core, so therepo_substringfield inPIN_PAIRSisn't actually generic.The tuple comment (lines 37-38) implies
PIN_PAIRSsupports arbitrary upstream repos, but_CI_PIN_REonly ever matchesomnibase_coreURLs. Adding a pair for a different repo would always report "no CI-pinned SHA found" regardless of the substring supplied — fails closed, but misleading for the next person who adds an entry per the docstring's own instructions.♻️ Suggested direction
-_CI_PIN_RE = re.compile( - r"omnibase-core\s*@\s*git\+https://github\.com/OmniNode-ai/omnibase_core" - r"@([0-9a-f]{40})" -) +def _ci_pin_re(repo_substring: str) -> re.Pattern[str]: + return re.compile( + rf"git\+https://{re.escape(repo_substring)}@([0-9a-f]{{40}})" + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/validation/validate_precommit_pin_parity.py` around lines 39 - 67, Update _CI_PIN_RE and _find_ci_pins so CI pin matching derives the repository pattern from each PIN_PAIRS repo_substring instead of hardcoding omnibase_core. Preserve extraction of the 40-character SHA and ensure arbitrary configured repository substrings can be matched..github/workflows/precommit-fail-loud-gate.yml (1)
65-66: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd
persist-credentials: falseto the checkout step.Static analysis flags this checkout as leaving the GitHub token persisted in the runner's git config. Job permissions are read-only, limiting blast radius, but explicitly disabling persistence is a low-cost hardening step.
🔒 Proposed fix
- name: Checkout code uses: actions/checkout@v7 + with: + persist-credentials: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/precommit-fail-loud-gate.yml around lines 65 - 66, Update the “Checkout code” step using actions/checkout@v7 to set persist-credentials to false, ensuring the GitHub token is not retained in the runner’s git configuration.Source: Linters/SAST tools
.pre-commit-config.yaml (1)
795-817: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
always_run: truemakes thefiles:regex on these two hooks dead code.Per pre-commit's own docs,
always_run: truecauses a hook to ignore itsfiles/exclude/typessettings entirely — "no-commit-to-branch is configured by default to always_run. As a result, it will ignore any setting of files, exclude, types or exclude_types." Bothprecommit-fail-loud-meta-gate(802) andprecommit-pin-parity(816) combinealways_run: truewith afiles:pattern, so those hooks actually run on every commit, not only when.pre-commit-config.yaml/scripts orci.ymlchange as the regex implies. Functionally harmless (the checks are cheap and idempotent), but the config misrepresents its own scoping intent.Either drop
always_run: true(lettingfiles:gate execution as apparently intended) or drop the now-vestigialfiles:field to make the always-on behavior explicit.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.pre-commit-config.yaml around lines 795 - 817, Resolve the contradictory trigger configuration for precommit-fail-loud-meta-gate and precommit-pin-parity: either remove always_run: true so each hook is gated by its existing files pattern, or remove the files pattern to explicitly retain always-on execution. Apply the same choice consistently to both hooks and preserve their existing stages and commands.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/precommit-fail-loud-gate.yml:
- Around line 65-66: Update the “Checkout code” step using actions/checkout@v7
to set persist-credentials to false, ensuring the GitHub token is not retained
in the runner’s git configuration.
In @.pre-commit-config.yaml:
- Around line 795-817: Resolve the contradictory trigger configuration for
precommit-fail-loud-meta-gate and precommit-pin-parity: either remove
always_run: true so each hook is gated by its existing files pattern, or remove
the files pattern to explicitly retain always-on execution. Apply the same
choice consistently to both hooks and preserve their existing stages and
commands.
In `@scripts/validation/validate_precommit_pin_parity.py`:
- Around line 39-67: Update _CI_PIN_RE and _find_ci_pins so CI pin matching
derives the repository pattern from each PIN_PAIRS repo_substring instead of
hardcoding omnibase_core. Preserve extraction of the 40-character SHA and ensure
arbitrary configured repository substrings can be matched.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1e746727-aa9d-4cf0-af21-b587808c504d
📒 Files selected for processing (4)
.github/workflows/precommit-fail-loud-gate.yml.pre-commit-config.yamlscripts/validation/validate_precommit_fail_loud.pyscripts/validation/validate_precommit_pin_parity.py
jonahgabriel
added a commit
that referenced
this pull request
Jul 16, 2026
The WS7 #2 (pre-lesson) fail-loud gate still carried all three false-green vectors the later fan-out repos designed out: 1. runs-on routed only public-fork PRs to hosted, leaving trusted internal PRs on the self-hosted omnibase-ci fleet (saturated/absent fleet = queued/cancelled required check = false-green window). 2. needs: occ-preflight — the exact skip-then-vacuous-green vector the OMN-14666 canary (this repo, #1783) surfaced: an occ-preflight failure SKIPPED the byte-match proof; only a manual rerun made it fire. OCC eligibility is already enforced by the independent occ-preflight job in ci.yml, so the coupling was redundant as well as harmful. 3. paths: filter — a path-filtered required check silently does not run on PRs that miss the filter. Canonicalize to match the other five parity gates: runs-on: ubuntu-latest (hosted-pinned, unconditional), no needs, no paths filter. The gate now fires on every PR to main/dev and passes/fails purely on its own byte-match evidence. Canary proven on omnibase_core#1450 (job ran on ubuntu-latest, passed 13s).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OMN-14666 — WS7 arch-* pre-commit parity → omnimarket (canary fan-out #2)
Applies the merged WS7 CI↔pre-commit gate-parity pattern (canary omniclaude#1904 / OMN-14655, MERGED) to omnimarket only (plan
docs/plans/2026-07-15-ci-precommit-gate-parity-plan.md§4 fan-out order: omniclaude → omnimarket). Parent: OMN-14655. Epic: OMN-14643 (WS7).Draft until CI confirms the local↔CI byte-match (parity-diff leg) on the pushed run.
What this does
repo: localpre-commit hooks, each invoking the exact ci.yml entrypoint (DRY — no re-impl, plan §2/§5):arch-node-metadata-dependency-scope→uv run python scripts/ci/check_node_metadata_dependencies.py(byte-identical to ci.yml:200)arch-unimported-handler-check→uv run python scripts/ci/check_unimported_handlers.py(byte-identical to ci.yml:259)default_install_hook_types: [pre-commit]→[pre-commit, pre-push, commit-msg]. Before this, the strict-mypy (stages:[pre-push]) and the commit-msg skip-token hook — both required-CI mirrors — were never installed bypre-commit install.scripts/validation/validate_precommit_fail_loud.py+validate_precommit_pin_parity.py, the tworepo: localhooks, and the CI mirror.github/workflows/precommit-fail-loud-gate.yml. pin-parityPIN_PAIRS = no-noncanonical-lifecycle-classes(pre-commit rev63635097…already == ci.yml:1498omnibase-core @ git+…@<sha>pin).Per-check STATIC classification receipt (plan §3a / D3)
check_node_metadata_dependencies.pycheck_unimported_handlers.pyNet-negative-surface (plan §5 / D11)
Converts two never-installed hooks (strict-mypy, commit-msg skip-token) into installed/enforcing hooks, and the fail-loud meta-gate retires the exit-0-on-missing false-green class (mechanism, not rule — CLAUDE.md §5). Not a pure add.
Explicitly EXCLUDED (tracked separately)
canonical handler-shape GROWTH ratchet → OMN-14563; runtime/compliance/aislop sweeps + contract-compliance (sibling-checkout / PR-diff → CI-only or later);
reject-node-topics.pyinline bash (needs §2 D2 entrypoint extraction);run_quality_gate_calibration.py(imports+executes handler code = RUNTIME); topic-drift/enum-drift (dep classification pending).dod_evidence (proof gate, plan §4 — run from a worktree off origin/dev @0f923d14)
ONCP node metadata dependency scope: PASS;OK: all Handler* classes … imported).HandlerSeededDeadBreakOMN14666→ERROR: 1 Handler class(es) with no wiring evidence, exit 1.import qdrant_client→metadata.yaml missing dependencies for imports: qdrant-client, exit 1.no-noncanonicalrev → FAIL on divergence, exit 1.uv run --no-project --with pyyaml python3 …): fail-loud + pin-parity bothOK.pre-commit run= 1.22s / 1.60s / 0.23s / 0.22s (~3.3s combined), under the <15s pre-commit budget.git commitpre-commit run: all hooks Passed (including the 4 new ones). The one environmental note: pre-existingtopic-naming-linthard-exits when../omnibase_infrais unresolved (§3d.2 prerequisite, NOT introduced here); passes withOMNIBASE_INFRA_PATHset to the canonical clone (verified). Out of scope for this canary.OCC Evidence-Source companion will bind via OCC autobind; receipt-gate cites OMN-14666.
Evidence-Ticket: OMN-14666
Evidence-Source: OCC#4266
Evidence-Commit: bd01133fb09bc1e0e64962858443a08f17272542
Evidence-Head: 10b52f3
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Status update 2026-07-16 (overnight WS7 canary closeout)
onex_change_control#4266is MERGED (2026-07-16T10:20:37Z, all 41 checks green). The Evidence-Source binding is now durable, not dry-run.10b52f34): ci.yml'slintjob actually ran (not skipped) and passed both parity steps that mirror the new pre-commit hooks —ONCP node metadata dependency scope(scripts/ci/check_node_metadata_dependencies.py, ci.yml:200) andUnimported-handler check(scripts/ci/check_unimported_handlers.py, ci.yml:259). CI executes byte-identical entrypoints to therepo: localhooks (run 29489413755, lint job 87595782981).needs: occ-preflightjob SKIPPED. The merge re-triggered all workflows and they now pass. Not four independent defects.Both un-draft conditions ("Draft until CI confirms the parity-diff leg" + "un-drafts once companion #4266 merges") are now satisfied. Merge remains Codex's.