Repository navigation
fix(OMN-16678): one deterministic event-bus resolution authority (explicit > ONEX_EVENT_BUS_TYPE > probe) - #2915
Conversation
Two defects made the resolved transport unrepeatable: 1. ONEX_EVENT_BUS_TYPE was honoured by select_event_bus and silently ignored by resolve_default_bus, so setting it to pin `onex delegate` to the in-process bus did nothing. 2. probe_kafka degrades ANY Stage-2 metadata failure — including a plain 2s AdminClient.list_topics timeout against a healthy broker — to REACHABLE, which resolve_default_bus mapped to inmemory while select_event_bus mapped it to kafka. Measured: 20 consecutive calls, unchanged env, healthy broker -> kafka 14x / inmemory 6x (OmniNode-ai/knowledge_base#59). Adds backends/auto_configure.py::resolve_bus_type as the single authority both paths call, with one resolution order — explicit argument > ONEX_EVENT_BUS_TYPE > probe — and a total probe mapping: AUTHORITATIVE/HEALTHY -> kafka, DISCOVERED -> inmemory, REACHABLE -> EventBusResolutionAmbiguousError naming the ambiguity and both remedies. No new env-var surface; the ONEX_EVENT_BUS_TYPE read stays in the file that already owned it.
|
Warning Review limit reachedNext included review available in 1 minute. View limit detailsLimit details: You’ve used the included review currently available. Your 125 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
Comment |
|
| Verdict | Meaning | Blocks merge? |
|---|---|---|
passed |
No critical findings | No |
blocked |
CRITICAL findings found | Yes |
degraded |
All models unavailable (infra) | No (pilot) |
Powered by omniintelligence.review_pairing.cli_review — multi-model adversarial review (OMN-8468/OMN-8524)
#7227) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#2915 * evidence: OCC companion self-bind for #7227 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
Summary
Fixes OMN-16678. Two related defects meant nothing decided the event-bus transport repeatably. Both were proven with measurements captured in
OmniNode-ai/knowledge-base#59.Defect 1 — the override was honoured by one path and ignored by the other.
backends/auto_configure.py::select_event_busreadONEX_EVENT_BUS_TYPE;cli/cli_delegate.py::resolve_default_bus— the functiononex delegateactually calls when--busis omitted — never did. SettingONEX_EVENT_BUS_TYPE=inmemoryto pinonex delegateto the in-process bus did nothing, even thoughresolve_default_bus's own docstring claimed it "mirrors the SAME probe-then-select precedence."Defect 2 — the probe tier was a coin flip.
probe_kafkaStage 2 wrapsAdminClient.list_topics(timeout=2.0)in a bareexcept Exceptionthat degrades any failure — including a plain metadata timeout against a healthy broker — toREACHABLE.resolve_default_busmappedREACHABLEtoinmemory;select_event_busmapped the identical state tokafka. Measured over 20 consecutiveresolve_default_bus()calls with an unchanged environment and a healthy broker:kafkainmemory70/30 with zero input variation. A delegation that silently lands on
inmemorynever reaches the deployednode_delegate_skill_orchestratorconsumer and never lands a projection in the shareddelegation_eventstable — the OMN-14376 data-loss class, reintroduced through the indeterminate branch.What changed
backends/auto_configure.py::resolve_bus_typeis now the single authority both call sites route through.select_event_busowns construction only;resolve_default_busis a thin call into it.Resolution order (one order, every caller)
--busat the CLI, the newbus_type=onselect_event_bus. Never second-guessed, never probed against.ONEX_EVENT_BUS_TYPE—inmemory/kafka/cloud, case- and whitespace-insensitive. Empty or unset falls through.cloudresolves to the Kafka transport becauseservice_kernel.pyalready classifies it as broker-backed (_broker_required_types).EnumProbeState, no implicit default branch.No new env-var surface:
ONEX_EVENT_BUS_TYPEalready existed, and its read stays inauto_configure.py, the file that already owned it.cli_delegate.pystill reads no environment variable directly (check-env-reads).Indeterminate-probe decision (recorded per the ticket's DoD)
Chosen behavior: fail fast on
REACHABLE, raisingEventBusResolutionAmbiguousError.The probe tier is now total, and each state is classified by whether its verdict is conclusive:
AUTHORITATIVE/HEALTHYkafkaDISCOVEREDinmemoryREACHABLERationale, per
feedback_no_defensive_no_defaults: when the underlying signal is not repeatable, refusing is the only outcome that is. Both prior mappings ofREACHABLEwere defensible in isolation and contradicted each other in practice, and which one you got was decided by transient network timing rather than by configuration. The error names the ambiguity (probe state + probe reason) and both deterministic remedies (--bus kafka|inmemory, orONEX_EVENT_BUS_TYPE=kafka|inmemory), so an operator who wants the old "try Kafka anyway" behavior states it explicitly and gets it every time.An unrecognised
ONEX_EVENT_BUS_TYPEvalue likewise raises rather than degrading to "probe and hope" — a typo in an override must not read as "no override".Behavior change to call out:
select_event_buspreviously built a Kafka bus onREACHABLE"despite the probe result". That branch is gone. The runtime kernel path reaches it only when no override is set;ONEX_EVENT_BUS_TYPE=kafkarestores the old outcome deterministically.Tests (TDD, probe always mocked — no live broker in the unit suite)
New
tests/unit/backends/test_bus_resolution_order.py:cloudalias, case/whitespace normalisation, empty-string-is-unset, unrecognised value raisesDISCOVERED-is-determinate distinctionREACHABLEraises, the error names the ambiguity and both remedies, and the named remedy actually worksUpdated:
tests/unit/cli/test_cli_delegate.py— stub target moved toauto_configure.probe_kafka(cli_delegateno longer imports the probe, so patching the old name would be a silent no-op); added regressions for the override reaching the delegate path (probe stubbed to the opposite answer) and forREACHABLErefusing; autouse fixture clears ambientONEX_EVENT_BUS_TYPE.tests/unit/runtime/test_kernel_registry.py—test_reachable_with_explicit_servers_uses_kafkainverted to..._refuses_to_guess, with the contradiction it encoded documented in the docstring; added override-recovery and explicit-bus_typecoverage.Verification
uv run pytest tests/unit— 23814 passed, 40 skipped, full unit suite green.uv run mypy src/omnibase_infra/backends/auto_configure.py src/omnibase_infra/cli/cli_delegate.py --strict— clean.pre-commit run --all-files— clean (exit 0), includingcheck-env-reads.Follow-up
The knowledge-base "combining deployment tiers" guide honestly documents the coin-flip third outcome today. A small KB PR updates that section to the shipped behavior once this merges — the guide stays true to what actually ships.
Evidence-Ticket: OMN-16678
Evidence-Source: OCC#7227