Repository navigation
fix(OMN-8724): coerce dict payload into typed handler model at dispatch boundary + SkillRoutingError envelope - #1983
Conversation
…ch boundary + emit SkillRoutingError envelope Systemic fix for the dict-not-typed dispatch-boundary class (same family as OMN-13141). _invoke_handle_method now validates a raw decoded dict into the handler's single annotated BaseModel parameter before calling it. Root cause: PEP 563 string annotations (every node handler uses from __future__ import annotations) meant the parameter annotation was the literal string, so the BaseModel-subclass check never matched — fixed via inspect.signature(eval_str=True). A typed single-parameter handler reached via operation_match (e.g. NodeGoldenChainSweep.handle(self, request: GoldenChainSweepRequest)) crashed with AttributeError on the first attribute access via onex node, while the module __main__ entrypoint (which builds the model itself) worked. Also: cli_node default mode now emits the documented SkillRoutingError JSON envelope on non-zero exit instead of leaving only a stderr traceback (the golden_chain_sweep skill documents this contract). RuntimeLocal gains a public last_error property to populate the envelope. Real-dispatch-path test boots a typed operation_match contract through RuntimeLocal.run_async() — reproduces the crash on origin/dev, passes with fix. Evidence-Source: OCC#PENDING Evidence-Ticket: OMN-8724
|
Warning Review limit reached
More reviews will be available in 2 hours, 13 minutes, and 21 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
OMN-8724 — coerce dict payload into typed handler model at the dispatch boundary
Fixes the confirmed
node_golden_chain_sweepcrash via the canonicalonex nodepath (BUG A in the integration-tooling triage):AttributeError: 'dict' object has no attribute 'chains'athandler_golden_chain_sweep.pybecauseruntime_local_adapter._invoke_handle_methodhanded the raw decoded dict tohandle(self, request: GoldenChainSweepRequest)instead of the typed model.Layer decision — A1 (systemic), not A2 (narrow)
Chose A1: coerce the dict into the handler's declared input type inside
omnibase_infra.runtime.runtime_local_adapter, fixing the whole dict-not-typed dispatch-boundary class (same family as OMN-13141 / the savings_estimation.get()bug) — not a per-nodemodel_validateband-aid. Blast radius is bounded and regression-free: coercion fires only when (a) the payload is a rawdictAND (b) the handler's single positional parameter is annotated with a concreteBaseModelsubclass. That combination previously always crashed, so the only behavior change is crash → success. All pre-existing pass-through paths (already-a-model, name-heuristic, kwargs-fanout, zero/var-keyword params) are unchanged.Root cause completion — PEP 563 string annotations
Every node handler uses
from __future__ import annotations, soinspect.signature(...).parameters[0].annotationwas the literal string"GoldenChainSweepRequest", and theissubclass(annotation, BaseModel)check never matched. Fixed by resolving forward refs withinspect.signature(handle_method, eval_str=True)(graceful fallback to unevaluated onNameError).Step 3 — SkillRoutingError envelope
cli_nodedefault (non-receipt) mode now emits the documentedSkillRoutingErrorJSON envelope to stdout on non-zero exit instead of leaving only a stderr traceback. Thegolden_chain_sweepSKILL.md documents this contract ("Non-zero exit emits aSkillRoutingErrorJSON envelope — surface it verbatim"). Envelope shape matchesomnibase_core.cli.cli_run_node._emit_error.RuntimeLocalgains a publiclast_errorproperty to populatemessage.CI gate — real dispatch path
tests/unit/runtime/test_runtime_local_operation_match_typed_handler.pyboots a typed single-model-parameter handler throughRuntimeLocal.run_async()(the realoperation_matchdispatch path) and asserts the run COMPLETES. This is the test that "would have caught this" — handler-isolation tests pass while the live path crashed. Pluscli_nodetest asserting theSkillRoutingErrorenvelope on a failed default-mode run.Verification (local, in worktree)
uv run ruff format/ruff check --fixon changed files — cleanuv run mypy src/omnibase_infra/runtime/runtime_local_adapter.py src/omnibase_infra/runtime/runtime_local.py src/omnibase_infra/cli/cli_node.py --strict— Success, 3 filesAttributeError: 'dict' object has no attribute 'correlation_id') on origin/dev, PASSES with fixuv run pytest tests/unit/runtime/ tests/unit/cli/— 5157 passed (the only failures are an env-artifact skip-test that requires omnimarket installed, and a load-sensitive perf stress test that passes in isolation — both pre-existing, unrelated)pre-commit runon changed files — all hooks pass (incl. union budget gate)uv run onex node node_golden_chain_sweep→result=completed(wasresult=failed+ traceback on origin/dev)..onex_state/workflow_result.jsonterminal_payloadstatus: pass.Companion PR
omnimarket
jonah/omn-8724-golden-chain-sweep-downgrade-claimadds the runtime-injectedcorrelation_idfield toGoldenChainSweepRequestand downgrades the node/skill evidence claim (the node is pure compute over caller-supplied rows — never live evidence). Both land independently on dev.dod_evidence / OCC pairing
Paired OCC receipt PR in onex_change_control carries
contracts/OMN-8724.yaml+ PASS receipts underdrift/dod_receipts/OMN-8724/.Evidence-Source: OCC#2626
Evidence-Ticket: OMN-8724
Closes OMN-8724 (crash + SkillRoutingError halves). Parent: OMN-8718.