Skip to content

refactor(OMN-10171): replace validate_skill_backing_node with omnibase_core shim (SEAM-2) - #1573

Merged
jonahgabriel merged 1 commit into
mainfrom
jonah/omn-10171-reconcile-liveness-validators
May 15, 2026
Merged

jonahgabriel merged 1 commit into
mainfrom
jonah/omn-10171-reconcile-liveness-validators

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented May 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Replaces the 450-line inline implementation in plugins/onex/skills/_lib/validate_skill_backing_node.py with a thin shim that delegates all logic to omnibase_core.validation.validator_skill_backing_node
  • Re-exports full public API for backward compat: load_allowlist, extract_backing_node, check_node_liveness, scan, _omnimarket_available, _resolve_omnimarket_nodes_root
  • Preserves NodeViolation = SkillLivenessViolation alias for validate_skill_aspiration.py
  • Imports are lazy (deferred to call time) to allow the shim to be importable during the transitional window

Merge dependency (BLOCKED)

This PR must NOT merge until:

  1. omnibase_core PR feat(hooks): cross-platform terminal identity [OMN-7295] #1069 (feat(OMN-10171): add skill-backing-node liveness validator) merges
  2. A new omnibase_core version is published to PyPI
  3. The omnibase-core pin in this repo's pyproject.toml is updated to that version

Until then, CI will show a failure on the validate-skill-backing-node pre-commit hook because omnibase_core.validation.validator_skill_backing_node doesn't exist in v0.40.1.

After omnibase_core publishes

  1. Update pyproject.toml: change omnibase-core==0.40.1 to the new version
  2. Run uv sync to update the lock
  3. Push — CI should go green

Test plan

  • Pre-commit passes on all hooks except validate-skill-backing-node (expected, unblocked via SKIP= during commit)
  • All other omniclaude hooks pass
  • After omnibase_core version bump: uv sync && uv run pytest tests/ passes

Evidence-Source: OCC#1011
Evidence-Ticket: OMN-10171

Summary by CodeRabbit

  • Refactor
    • Reorganized skill backing node validation logic. CLI functionality and public API remain unchanged. Minor parameter naming adjustment for consistency.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 12, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The validator file transitions from standalone implementation to a thin pre-commit/CI shim. All core logic—allowlist loading, backing-node extraction, liveness checking, scanning—now lazily delegates to omnibase_core. In-file implementations (~405 lines) are removed; lazy wrappers, type re-exports, and a delegating CLI entry point (~68 lines) are added. Backward compatibility is preserved through lazy re-exports of violation types.

Changes

Validator Shim Delegation

Layer / File(s) Summary
Shim foundation: docstring and lazy-loading infrastructure
plugins/onex/skills/_lib/validate_skill_backing_node.py
Documentation updated to identify this as a pre-commit/CI shim; lazy-loading infrastructure (_core() function and TYPE_CHECKING-only imports) introduced to dynamically load the canonical core module. Wrapper functions forward calls to omnibase_core for allowlist loading, backing-node extraction, node liveness checking, scanning, and internal resolution helpers.
Backward-compatible type re-exports
plugins/onex/skills/_lib/validate_skill_backing_node.py
NodeViolation and SkillLivenessViolation are re-exported as lazy aliases to the canonical module's types (or None if import fails). The __all__ list is updated to expose delegated functions and violation names for external callers.
CLI entry point delegation
plugins/onex/skills/_lib/validate_skill_backing_node.py
main() CLI entry point now delegates scanning and validation logic to core.main(args) while keeping the same default root path computation (four directory levels above the shim).

🎯 4 (Complex) | ⏱️ ~45 minutes


A shim so lean and so neat,
Delegates work with such fleet,
No logic stays here,
The core's crystal clear,
One module, one purpose, complete! 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: converting a validator implementation into a thin shim delegating to omnibase_core, which matches the primary purpose of this refactoring.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jonah/omn-10171-reconcile-liveness-validators

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

@github-actions

github-actions Bot commented May 12, 2026 •

Copy link
Copy Markdown
Contributor

⚠️ Hostile Reviewer — DEGRADED (informational)

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

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


Gate semantics (pilot phase)

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

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

jonahgabriel added a commit to OmniNode-ai/onex_change_control that referenced this pull request May 13, 2026
…1572, #1573

- contracts/OMN-9906.yaml + dod-001 receipt bound to omniclaude#1572
- contracts/OMN-10171.yaml + dod-001 receipt bound to omniclaude#1573

Delivery PRs:
- OmniNode-ai/omniclaude#1572
- OmniNode-ai/omniclaude#1573
jonahgabriel added a commit to OmniNode-ai/onex_change_control that referenced this pull request May 13, 2026
…1572, #1573

- contracts/OMN-9906.yaml + dod-001 receipt bound to omniclaude#1572
- contracts/OMN-10171.yaml + dod-001 receipt bound to omniclaude#1573

Delivery PRs:
- OmniNode-ai/omniclaude#1572
- OmniNode-ai/omniclaude#1573
jonahgabriel added a commit to OmniNode-ai/onex_change_control that referenced this pull request May 13, 2026
* chore(OMN-10492): add OCC contract binding omniclaude PR #1571

- contracts/OMN-10492.yaml — binds delivery PR omniclaude#1571 (replace raw
  Postgres probe with runtime HTTP health check in check_database_health)
- drift/dod_receipts/OMN-10492/dod-001/command.yaml — receipt bound to PR #1571

Delivery PR: OmniNode-ai/omniclaude#1571

* chore(OMN-10492): add self-binding dod-occ-pr receipt for OCC#974

* chore(OMN-9906,OMN-10171): add OCC contracts binding omniclaude PRs #1572, #1573

- contracts/OMN-9906.yaml + dod-001 receipt bound to omniclaude#1572
- contracts/OMN-10171.yaml + dod-001 receipt bound to omniclaude#1573

Delivery PRs:
- OmniNode-ai/omniclaude#1572
- OmniNode-ai/omniclaude#1573

* chore(OMN-9906,OMN-10171): add self-binding dod-occ-pr receipts for OCC#974

* fix(OMN-10492): make OCC evidence checks executable
…e_core shim (SEAM-2)

Replaces the 450-line inline implementation with a thin shim that delegates
all logic to omnibase_core.validation.validator_skill_backing_node.

Public API re-exported for backward compat: load_allowlist, extract_backing_node,
check_node_liveness, scan, _omnimarket_available, _resolve_omnimarket_nodes_root.
NodeViolation alias preserved for validate_skill_aspiration.py.

BLOCKED ON: omnibase_core PR #1069 (feat(OMN-10171)) must merge and version bump
before this PR can merge. omniclaude pins omnibase-core==0.40.1; the new module
omnibase_core.validation.validator_skill_backing_node is not in that version.
After omnibase_core publishes, update the pin and merge this PR.
@jonahgabriel
jonahgabriel force-pushed the jonah/omn-10171-reconcile-liveness-validators branch from 06f7d5b to 339d62b Compare May 15, 2026 00:34
@jonahgabriel
jonahgabriel marked this pull request as ready for review May 15, 2026 08:24
@jonahgabriel
jonahgabriel added this pull request to the merge queue May 15, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
plugins/onex/skills/_lib/validate_skill_backing_node.py (1)

80-88: 💤 Low value

Consider a sentinel class for clearer failure during transitional window.

If omnibase_core import fails, NodeViolation = None causes a cryptic TypeError if callers do isinstance(obj, NodeViolation). While the comment at lines 76-79 documents this, a sentinel class providing a descriptive error would improve debugging during the blocked transitional period.

♻️ Optional: Sentinel class for clearer error
 try:
     from omnibase_core.validation.validator_skill_backing_node import (  # noqa: E402
         SkillLivenessViolation,
     )
 
     NodeViolation = SkillLivenessViolation
 except ImportError:
-    SkillLivenessViolation = None  # type: ignore[assignment,misc]
-    NodeViolation = None  # type: ignore[assignment]
+    class _MissingCoreViolation:
+        """Sentinel: omnibase_core.validation.validator_skill_backing_node not available."""
+        def __init__(self, *args, **kwargs):
+            raise ImportError(
+                "SkillLivenessViolation requires omnibase_core >= 0.40.2. "
+                "See OMN-10171 for version bump timeline."
+            )
+    SkillLivenessViolation = _MissingCoreViolation  # type: ignore[assignment,misc]
+    NodeViolation = _MissingCoreViolation  # type: ignore[assignment]
🤖 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 `@plugins/onex/skills/_lib/validate_skill_backing_node.py` around lines 80 -
88, Import failure sets NodeViolation to None which causes TypeError on
isinstance(obj, NodeViolation); replace the None fallback with a small sentinel
class (e.g., _OmnibaseMissingNodeViolationSentinel) and assign both
SkillLivenessViolation and NodeViolation to that sentinel in the except
ImportError block so isinstance checks still work and debugging is clearer;
implement the sentinel with a descriptive __repr__ / class docstring (or a
_missing_package attribute) so callers produce an informative message when they
inspect it.
🤖 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 `@plugins/onex/skills/_lib/validate_skill_backing_node.py`:
- Around line 80-88: Import failure sets NodeViolation to None which causes
TypeError on isinstance(obj, NodeViolation); replace the None fallback with a
small sentinel class (e.g., _OmnibaseMissingNodeViolationSentinel) and assign
both SkillLivenessViolation and NodeViolation to that sentinel in the except
ImportError block so isinstance checks still work and debugging is clearer;
implement the sentinel with a descriptive __repr__ / class docstring (or a
_missing_package attribute) so callers produce an informative message when they
inspect it.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0540d974-9311-46bc-a0d0-4b71ecc82058

📥 Commits

Reviewing files that changed from the base of the PR and between 771f5e9 and 339d62b.

📒 Files selected for processing (1)
  • plugins/onex/skills/_lib/validate_skill_backing_node.py

Merged via the queue into main with commit 41408e2 May 15, 2026
93 of 94 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-10171-reconcile-liveness-validators branch May 15, 2026 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant