Repository navigation
fix(OMN-14489): node_remote_agent_invoke_effect contract → canonical omnibase_core models (unstarve 19-in/0-out) - #2280
Conversation
…onical omnibase_core models The contract declared local STUB input_model/output_model (ModelInvocationCommand required agent_id + extra=forbid; ModelAgentTaskLifecycleEvent used a different status enum) while the producer (delegation orchestrator's InvocationCommand publish) and the consumer (HandlerA2ATask) use the RICH omnibase_core.models.delegation classes (task_id/invocation_kind/target_ref/ agent_protocol/model_backend, no agent_id). routing_strategy=operation_match skips payload-type validation at dispatch so it did not DLQ, but validate_runtime_local_ingress_payload resolves contract.input_model and model_validates the payload against it — the stub rejected the real payload, so the node starved: remote-agent-invoke.v1 HW=19 IN / agent-task-lifecycle.v1 HW=0 OUT on the stability lane. A latent silent break the platform's move to payload_type_match would turn into a hard DLQ. Fix: repoint input_model -> omnibase_core.models.delegation.model_invocation_command, output_model -> omnibase_core.models.delegation.model_agent_task_lifecycle_event; delete the now-unused local stub models (net-negative-surface). Cross-boundary test (single suite, OMN-14208 shape): builds the producer's real rich payload and drives it through the REAL validate_runtime_local_ingress_payload against the CONTRACT-declared input_model, asserting no field drop; plus a seam-identity check that the contract's declared class IS the class HandlerA2ATask consumes. RED against the stub contract (producer payload -> 6 validation errors: missing agent_id + forbidden rich extras), GREEN against the canonical contract. Updates the two prior tests that locked in the stub module.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
… the class platform-wide The contract repoint alone left this branch green-but-unreachable: HandlerA2ATask exposed only submit()/watch() and NO handle()/handle_async(), so auto-wiring bound _missing_handle and every dispatch raised ModelOnexError. The A2A remote-agent branch passed ingress validation and then died at the first real message. Fix (PART 1): - HandlerA2ATask.handle(request: ModelInvocationCommand) -> ModelAgentTaskLifecycleEvent (canonical def B; no ModelEventEnvelope import). The RETURN value is the node's only publish path on the auto-wired runtime — zero-arg discovery installs a NoopLifecycleEventSink, so sink writes are dropped. watch() is not driven from here: it reads the submitted row back, which the Noop repository cannot serve. - contract.yaml declares event_model: ModelInvocationCommand on the handler entry. Without it the dispatch callback falls to the untyped arm and hands the handler a raw ModelEventEnvelope, and the dispatcher is indexed only under topic-derived message_type aliases while the producer stamps event_type with the payload CLASS name. Mechanism (PART 2) - a rule is not a mechanism: - tests/unit/runtime/auto_wiring/test_handler_dispatch_entrypoint_coverage_omn14489.py asserts EVERY contract-declared handler exposes handle/handle_async. Census found 44 of 158 already carry this defect; they are frozen in a SHRINK-ONLY ratchet allowlist (a new instance fails; a fixed handler must be removed from the list, so it can never go stale). Burn-down + the eventual flip of _missing_handle to a wiring-time hard failure: OMN-14510. - OMN-14488's route-coverage gate does NOT catch this: a dispatcher bound to _missing_handle still registers and still routes, and its _FakeHandler has a handle(). Registered and routable are not executable. RED proof (against the EXISTS-but-WRONG state, not mere absence): reverting only the handler turns 4 tests red - including the platform-wide gate - with the literal runtime error "Auto-wired handler HandlerA2ATask does not expose a callable handle() or handle_async() dispatch entrypoint." The 4 non-vacuity/discriminator tests stay green. Follow-ups filed: OMN-14511 (the node has NO DI - the auto-wired handler still fails closed at _resolve_target on an empty target registry, so this branch is reachable but not yet end-to-end), OMN-14512 (URL-authority baseline drift reds pre-commit --all-files on a clean tree). Closes OMN-14489
OMN-14489 — node_remote_agent_invoke_effect contract → canonical omnibase_core models
DO NOT MERGE yet — Codex lands; owes a non-implementer OCC evidence companion.
Problem (live signal)
The contract declared LOCAL STUB
input_model/output_model:ModelInvocationCommand(stub) requiredagent_id,extra="forbid", and lacked the rich fields.ModelAgentTaskLifecycleEvent(stub) used a differentstatusenum.But the producer (the delegation orchestrator's
InvocationCommandpublish) and the consumer (HandlerA2ATask) both use the RICHomnibase_core.models.delegationclasses:ModelInvocationCommand={task_id, correlation_id, invocation_kind, target_ref, agent_protocol?, model_backend?, payload?}(noagent_id),ModelAgentTaskLifecycleEvent={task_id, correlation_id, lifecycle_type, occurred_at, ...}.routing_strategy=operation_matchskips payload-type validation at dispatch, so it did not DLQ — butvalidate_runtime_local_ingress_payloadresolvescontract.input_modelandmodel_validates the payload against it. The stub rejected the real payload (missingagent_id; forbidden extrastask_id/invocation_kind/target_ref/agent_protocol/model_backend), so the node starved:remote-agent-invoke.v1HW=19 IN /agent-task-lifecycle.v1HW=0 OUT on the stability lane (verified live viarpk topic describe). A latent silent break the platform's move topayload_type_matchwould turn into a hard DLQ.Fix
input_model→omnibase_core.models.delegation.model_invocation_command.ModelInvocationCommandoutput_model→omnibase_core.models.delegation.model_agent_task_lifecycle_event.ModelAgentTaskLifecycleEventmodels/package — net-negative-surface; only the contract + two tests referenced them).dod_evidence (RED → GREEN, cross-boundary — OMN-14208 shape, single suite)
tests/integration/test_remote_agent_invoke_effect_contract.pybuilds the producer's real rich payload and drives it through the REALvalidate_runtime_local_ingress_payloadagainst the contract-declared input_model (not a hardcoded class), asserting no field drop; plus a seam-identity test that the contract's declared class is the classHandlerA2ATaskconsumes, and that the contract's output_model validates the rich lifecycle event the handler emits.missing agent_id+ 5 forbidden rich extras).task_id/invocation_kind/target_ref.test_node_scaffold.py, this file) that locked in the stub module (exists-but-wrong).Local (
env -u PYTHONPATH uv run pytest): node contract + scaffold suites → 29 passed;+ test_auto_wiring_real_manifest→ 31 passed (discovery still parses the repointed contract);runtime_local_ingresssuite → 28 passed. ruff format/check clean; full pre-commit passed (Contract Validation, Architecture, Any-type, operation_match gate all green).Closes OMN-14489.
Evidence-Source: OCC#4072
Evidence-Commit: eab4490c5d9618ee70241c54ee8a52e37a138093
Evidence-Ticket: OMN-14489
UPDATE (commit
38d78e6b) — the repoint alone left this branch DEAD. Completing OMN-14489.The contract repoint above was necessary but not sufficient.
HandlerA2ATaskexposed onlysubmit()/watch()and nohandle()/handle_async(). Auto-wiring's_make_dispatch_callback(runtime/auto_wiring/handler_wiring.py:412-462) looks forhandle_async, thenhandle; finding neither it binds_missing_handle, which raises on every dispatch:So the branch passed ingress validation (fixed above) and then died at the first real dispatch. Landing the repoint alone would have been a false Done.
PART 1 — make the branch reachable
HandlerA2ATask.handle(request: ModelInvocationCommand) -> ModelAgentTaskLifecycleEvent— canonical def B; does not importModelEventEnvelope(OMN-14355 gate stays satisfied). The return value is the node's only publish path on the auto-wired runtime: zero-arg runtime discovery installs aNoopLifecycleEventSink, so lifecycle events written through the sink are dropped.watch()is deliberately not driven fromhandle()— it reads the submitted row back, which the zero-argNoopRemoteTaskStateRepositorycannot serve.contract.yamldeclaresevent_model: ModelInvocationCommandon the handler entry (contract_version 0.1.2 → 0.1.3). This is load-bearing, not cosmetic: with noevent_modelthe dispatch callback takes the untyped arm and hands the handler a rawModelEventEnvelope, and the dispatcher is indexed only under topic-derivedmessage_typealiases while the producer stampsevent_typewith the payload class name (service_dispatch_result_applier.py:330). Declaring it registers"ModelInvocationCommand"as a message_type alias and type-scopes the route (OMN-12416).PART 2 — the mechanism (a rule is not a mechanism)
tests/unit/runtime/auto_wiring/test_handler_dispatch_entrypoint_coverage_omn14489.pyenforces the invariant platform-wide: every contract-declared handler MUST exposehandle/handle_async.A census of all 158 contract-declared handlers in the repo found 44 already carry this exact defect (e.g.
HandlerScoreModels→ onlyscore_candidates();HandlerRoutingInitiate→ onlyinitiate_routing()). Projection handlers are not exempt —_make_projection_dispatch_callbackalso callshandler_instance.handle(...). They are frozen in a shrink-only ratchet (_KNOWN_ENTRYPOINTLESS):Burn-down + the eventual flip of
_missing_handleto a wiring-time hard failure: OMN-14510.Why #2278 (OMN-14488) does NOT catch this — I checked the live diff before building. Its route-coverage gate proves a payload routes to a registered dispatcher, but a dispatcher bound to
_missing_handlestill registers and still routes, and its_FakeHandlerhas ahandle(). Its operation_match gate validates the ingressinput_model, upstream of dispatch. Registered and routable are not executable — that is the third seam, and it is what this gate closes. No overlap; nothing in #2278 was duplicated.RED proof — against the EXISTS-but-WRONG state, not mere absence
A gate that only fires when a handler is absent would be vacuous. Reverting only
handler_a2a_task.py(contract + tests unchanged) — i.e. the handler as it exists ondev: contract-declared, wired, ingress-valid, entrypoint-less — turns 4 tests RED, including the platform-wide gate:The 4 that stay green are the non-vacuity/discriminator tests (scan-actually-ran, allowlist-shrink-only, predicate-discriminates). The dispatch tests drive the real
_make_dispatch_callbackover the real handler class — no fake handler, no patched entrypoint.Scope honesty — what this does NOT claim
This branch is now REACHABLE, not end-to-end functional. Nothing in
src/ever injects atarget_registry/repository/lifecycle_sink, so the auto-wired zero-arg handler still fails closed at_resolve_target(KeyError→ValueError: Unknown target_ref) on any real command. The 19-IN / 0-OUT starvation does not clear until that DI lands. Filed as OMN-14511.proof_class: code-only+ local dispatch-path test — not live-readback.Also filed: OMN-14512 — the URL-authority gate reds
pre-commit --all-fileson a clean tree (baseline drift, pre-existing; verified by stashing all work). It passes on this PR's changed files.