Repository navigation
fix(OMN-10168): add orchestrator dispatcher coverage gate - #1440
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds an opt-in strict dispatcher coverage validation (controlled by ONEX_STRICT_DISPATCHER_COVERAGE) to auto-wiring: it derives orchestrator start-command aliases, computes live message types from prepared handlers, detects missing coverage or prep failures, and aborts wiring when gaps are found. Includes contract and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as AutoWiring
participant Manifest as Manifest
participant Contracts as OrchestratorContracts
participant Handlers as HandlerPrep
participant Validator as CoverageValidator
participant Engine as MessageEngine
rect rgba(100,150,200,0.5)
Note over Client: Strict mode enabled (optional)
end
Client->>Contracts: Scan discovered orchestrator contracts
Contracts-->>Client: Start-command aliases (from subscribe topics)
Client->>Handlers: Prepare/import handlers
Handlers-->>Client: Prepared handlers and their message types
Handlers-->>Client: Prep failures (if any)
Client->>Validator: Build live message-type set and compare to aliases
Validator-->>Client: Coverage result (gaps or OK)
alt Gaps or prep failures
Client->>Engine: Abort wiring (raise ModelOnexError)
else All covered
Client->>Engine: Commit dispatchers/subscriptions
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 (2)
contracts/OMN-10168.yaml (1)
36-38: Consider importing a public function or module for dod-002 verification.The check imports
_strict_dispatcher_coverage_enabled, a private (underscore-prefixed) function. Private functions are implementation details and may be renamed/removed without notice. If this check is meant to verify the feature exists, consider importing the module itself or exposing a public entry point for contract verification.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/OMN-10168.yaml` around lines 36 - 38, The check currently imports the private function _strict_dispatcher_coverage_enabled from omnibase_infra.runtime.auto_wiring.handler_wiring; change the check to rely on a public symbol or the module itself to avoid depending on an internal name. Either import the module (omnibase_infra.runtime.auto_wiring.handler_wiring) and assert the public API exists via hasattr or import a newly exposed public function (e.g., strict_dispatcher_coverage_enabled) and assert it is callable; update the check_value command to reference that module or public symbol instead of the underscore-prefixed _strict_dispatcher_coverage_enabled.tests/unit/runtime/auto_wiring/test_orchestrator_dispatcher_coverage.py (1)
83-188: Well-designed test suite covering key scenarios.The tests comprehensively cover the strict dispatcher coverage feature: default-off behavior, strict rejection, explicit alias acceptance, topic-derived alias acceptance, and preparation failure propagation. Consider adding a test for non-orchestrator contracts to verify they're not subject to the strict check, but this is optional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/runtime/auto_wiring/test_orchestrator_dispatcher_coverage.py` around lines 83 - 188, Add an additional unit test in tests/unit/runtime/auto_wiring/test_orchestrator_dispatcher_coverage.py that verifies non-orchestrator contracts are not enforced by the strict dispatcher coverage check: create a manifest using ModelAutoWiringManifest with a contract built via _make_contract that is not an orchestrator (i.e., differs from pr_lifecycle_orchestrator/START_ALIAS), set ONEX_STRICT_DISPATCHER_COVERAGE to "1", patch _import_handler_class to return FakeHandler, call wire_from_manifest (or MessageDispatchEngine as in other tests) and assert that wiring proceeds (total_wired or total_skipped as appropriate) and no ModelOnexError is raised; reference symbols: ModelAutoWiringManifest, _make_contract, START_ALIAS, wire_from_manifest, and _import_handler_class to locate where to add the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@contracts/OMN-10168.yaml`:
- Around line 36-38: The check currently imports the private function
_strict_dispatcher_coverage_enabled from
omnibase_infra.runtime.auto_wiring.handler_wiring; change the check to rely on a
public symbol or the module itself to avoid depending on an internal name.
Either import the module (omnibase_infra.runtime.auto_wiring.handler_wiring) and
assert the public API exists via hasattr or import a newly exposed public
function (e.g., strict_dispatcher_coverage_enabled) and assert it is callable;
update the check_value command to reference that module or public symbol instead
of the underscore-prefixed _strict_dispatcher_coverage_enabled.
In `@tests/unit/runtime/auto_wiring/test_orchestrator_dispatcher_coverage.py`:
- Around line 83-188: Add an additional unit test in
tests/unit/runtime/auto_wiring/test_orchestrator_dispatcher_coverage.py that
verifies non-orchestrator contracts are not enforced by the strict dispatcher
coverage check: create a manifest using ModelAutoWiringManifest with a contract
built via _make_contract that is not an orchestrator (i.e., differs from
pr_lifecycle_orchestrator/START_ALIAS), set ONEX_STRICT_DISPATCHER_COVERAGE to
"1", patch _import_handler_class to return FakeHandler, call wire_from_manifest
(or MessageDispatchEngine as in other tests) and assert that wiring proceeds
(total_wired or total_skipped as appropriate) and no ModelOnexError is raised;
reference symbols: ModelAutoWiringManifest, _make_contract, START_ALIAS,
wire_from_manifest, and _import_handler_class to locate where to add the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b67c28e8-9063-4413-b34a-5ae677502659
📒 Files selected for processing (3)
contracts/OMN-10168.yamlsrc/omnibase_infra/runtime/auto_wiring/handler_wiring.pytests/unit/runtime/auto_wiring/test_orchestrator_dispatcher_coverage.py
jonahgabriel
force-pushed
the
jonah/omn-10168-dispatcher-coverage
branch
from
April 28, 2026 07:10
6953320 to
bad8174
Compare
jonahgabriel
removed this pull request from the merge queue due to a manual request
Apr 28, 2026
jonahgabriel
added this pull request to the merge queue
Apr 28, 2026
This was referenced Sep 6, 2026
This was referenced Sep 15, 2026
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.
Summary
ONEX_STRICT_DISPATCHER_COVERAGEenforcement before auto-wiring commits dispatchers/subscriptions.onex.cmd.*.*-start.v1topic has no live dispatcher message type for the topic-derived event_type alias.event_typealiases, topic-derived aliases, and handler-preparation failures.OMN-10168.SEAM-4 audit
Parser-only audit used fresh
OMN-10168worktrees, not the stale canonical checkout:omnibase_infra/src/omnibase_infra/nodes,omnimarket/src/omnimarket/nodespr_lifecycle_orchestratoris already covered by its contract alias fromomnimarket#431:omnimarket.pr-lifecycle-orchestrator-start. No omnimarket code change was required for this task.Verification
uv run pytest tests/unit/runtime/auto_wiring/test_orchestrator_dispatcher_coverage.py tests/integration/test_orchestrator_dispatcher_coverage_integration.py tests/unit/runtime/auto_wiring/test_wiring.py tests/unit/runtime/auto_wiring/test_handler_wiring_event_type_alias.py -q-> 53 passeduv run validate-yaml contracts/OMN-10168.yaml-> passeduv run ruff check src/omnibase_infra/runtime/auto_wiring/handler_wiring.py tests/unit/runtime/auto_wiring/test_orchestrator_dispatcher_coverage.py-> passeduv run ruff check --fix tests/integration/test_orchestrator_dispatcher_coverage_integration.py-> passeduv run ruff format --check src/omnibase_infra/runtime/auto_wiring/handler_wiring.py tests/unit/runtime/auto_wiring/test_orchestrator_dispatcher_coverage.py-> passeduv run ruff format tests/integration/test_orchestrator_dispatcher_coverage_integration.py-> passeduv run mypy src/omnibase_infra/runtime/auto_wiring/handler_wiring.py --strict-> passedgit diff --check-> passedpre-commit run --files src/omnibase_infra/runtime/auto_wiring/handler_wiring.py tests/unit/runtime/auto_wiring/test_orchestrator_dispatcher_coverage.py tests/integration/test_orchestrator_dispatcher_coverage_integration.py contracts/OMN-10168.yaml-> passedNo live
.201deploy/restart was performed.Refs OMN-10168.
Summary by CodeRabbit
New Features
Tests
Chores