Repository navigation
feat(OMN-16774): wire event-chain tests as a required per-PR CI gate - #2939
Conversation
OMN-16767 killed the entire delegation chain on the .201 dev lane and every repo's CI stayed green. HandlerRoutingIntent was handed the projection arm's raw input_data dict instead of a validated ModelRoutingIntent, raised AttributeError: 'dict' object has no attribute 'payload', and every routing request went to the platform quarantine sink. Zero routing decisions; every delegation timed out. Nothing caught it because nothing drives the real dispatch path per PR: - Handler-isolation tests call handle() directly and never see wiring. - tests/unit/nodes/node_coding_agent/test_real_dispatch_multitopic_routing.py is the closest existing "real dispatch" suite, but it imports _make_dispatch_callback DIRECTLY, hand-building the callback and bypassing _prepare_handler_wiring — the exact arm-selection seam the defect lived in. That is why it was green throughout the outage. - omnimarket's delegation-regression-nightly.yml drives a real chain but is nightly and needs a live Redpanda + Postgres lane. Doctrine rule 5: detection not wired as a pre-merge gate is advisory and gets ignored. This adds tests/integration/chains/, a parameterized suite that drives whole chains through the REAL seam with zero infrastructure: raw JSON wire bytes -> EventBusInmemory -> the real EventBusSubcontractWiring consumer callback and its real _deserialize_to_envelope -> the real _prepare_handler_wiring arm selection -> MessageDispatchEngine.dispatch -> the handler -> DispatchResultApplier -> terminal on the bus. It asserts the two things the outage needed and nobody was checking: the terminal event actually lands, and NOTHING reaches the quarantine sink. New chains are added as ChainCase rows. Wired as a gate in the same PR: an unconditional "Event Chain Gate" job in ci.yml, registered in scripts/ci/ci_summary_gate.py::STRICT_GATE_JOBS. The default-deny sweep already fails CI Summary on any job failure; the STRICT_GATE_JOBS entry additionally fails closed on a skipped/absent job, so the gate cannot be silently retired. "CI Summary" is dev's only required status check, so this blocks merge with no branch-protection edit. Negative control (would_have_caught), run locally against this exact suite: dev @ e2110e1 (pre-fix) -> 2 failed, 2 passed + handler_wiring.py from PR #2937 -> 4 passed The failure reproduces the production log lines verbatim: Projection handler error: handler=HandlerMirrorRoutingIntent topic=onex.cmd.omnibase-infra.delegation-routing-request.v1 error_type=AttributeError error='dict' object has no attribute 'payload' ... routing malformed/erroring event to the platform quarantine sink onex.dlq.omnibase-infra.quarantine.v1 The no-db_io control row passes in both configurations, localizing the failure to db_io arm selection rather than the handler, bus, or engine. Because the fix (#2937) has not merged, the db_io row carries xfail(strict=True, broken_by="OMN-16767") — the same "known drift tracked in an open ticket" pattern kafka-boundary-compat already uses. The gate therefore lands green today and actively blocks every other chain, and the STRICT marker turns RED via XPASS the moment #2937 lands, forcing its removal. Verified both directions locally. No Kafka, no database, no bypass flags, no branch-protection mutations. Refs: OMN-16774, OMN-16767, OMN-16769, OMN-16773
|
Warning Review limit reachedNext included review available in 35 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 131 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 (4)
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)
…on-vacuous The first push marked the db_io row xfail(strict=True, broken_by="OMN-16767") because the chain is dead locally. CI disagreed: the row XPASSED on the runner, and a STRICT xfail turns an XPASS into a failure, so "Event Chain Gate" went RED for the wrong reason on PR #2939. Measured divergence, same repo source, same commit: local (uv-synced venv, omnibase-core 0.46.13) _prepare_handler_wiring selects _make_projection_dispatch_callback -> handler gets a raw dict -> quarantine -> chain dead -> row FAILS CI (shared env at ~/.cache/omni/ci-envs/omnibase_infra/<digest>/.venv) projection arm is NOT selected -> chain terminalizes -> row PASSES So the row's outcome tracks the resolved environment, not the code under gate. An environment-dependent expectation is not a gate, and a strict xfail that XPASSes on the enforcement surface is unlandable, so the marker is removed. CI is the surface that gates merges, and there the row passes on its own merits. Removing the marker alone would leave a worse problem: if the resolved core ever parses the db_io block away, the row would keep passing while exercising nothing. So this also adds an explicit precondition — a row declaring db_tables must find db_io.db_tables present on the PARSED contract, or it fails loudly with "gate precondition not met ... do not delete the row". A vacuous pass is now impossible; the row either exercises the OMN-16767 shape or says so. The local reproduction remains the negative control recorded on the PR: with handler_wiring.py from #2937 in place the suite is 4 passed, without it 2 failed / 2 passed, and the failure carries the verbatim production log lines. That evidence is unaffected by this change. The divergence itself is a finding, not a nuisance: the CI shared env does not reproduce the wiring behavior the dev lane exhibits, which is precisely the kind of gap that let OMN-16767 ship green. Reported as a residual on OMN-16774. Refs: OMN-16774, OMN-16767
#7324) * evidence: OCC companion pass 1 for OmniNode-ai/omnibase_infra#2939 * evidence: OCC companion self-bind for #7324 --------- Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
Correction to this PR body (post-merge, lane
|
handler_wiring.py |
Result |
|---|---|
dev @ 6efddd855 (fix present) |
4 passed |
swapped to 16d61691e (pre-fix, parent of #2937's merge) |
2 failed, 2 passed |
Failing rows: test_chain_reaches_terminal_and_never_quarantines[delegation-routing-with-db-io] and test_typed_def_b_handler_is_never_wired_to_the_projection_arm[delegation-routing-with-db-io]. The delegation-routing-no-db-io control row passes in both configurations, localizing the failure to db_io arm selection rather than the handler, bus, or engine.
Production signature reproduced verbatim:
Projection handler error: handler=HandlerMirrorRoutingIntent
topic=onex.cmd.omnibase-infra.delegation-routing-request.v1
error_type=AttributeError error='dict' object has no attribute 'payload'
Projection handler HandlerMirrorRoutingIntent has NO DLQ topic declared in
contract.event_bus.dlq_topics — routing malformed/erroring event to the
platform quarantine sink onex.dlq.omnibase-infra.quarantine.v1
The conclusion in the body is unchanged; only the mechanism description was stale.
Gate confirmed live
Event Chain Gate ran completed/success on this PR's merge head (run 33110270490), is unconditional in dev's ci.yml, and is registered in scripts/ci/ci_summary_gate.py::STRICT_GATE_JOBS on dev (line 127). dev's only required context is ["CI Summary"], so it blocks today. No branch-protection edit was made.
Companion half: omnimarket#2167 merged cff0d765 (OCC companion #7325 merged first, after an add/add union on contracts/OMN-16774.yaml).
Problem
OMN-16767 killed the entire delegation chain on the .201 dev lane, and every repo's CI was green the whole time.
HandlerRoutingIntentwas handed the projection arm's rawinput_datadict instead of a validatedModelRoutingIntent, raisedAttributeError: 'dict' object has no attribute 'payload'on its first dereference, the projection arm swallowed it, and every routing request went to the platform quarantine sink. Zero routing decisions were produced; every delegation timed out.Nothing caught it because nothing drives the real dispatch path per PR:
handle()directly — they never see wiring.tests/unit/nodes/node_coding_agent/test_real_dispatch_multitopic_routing.py_make_dispatch_callbackdirectly — hand-building the callback and bypassing_prepare_handler_wiring, the exact arm-selection function the defect lived in.delegation-regression-nightly.ymlonex-runharnessDoctrine rule 5: detection not wired as a pre-merge gate is advisory and gets ignored.
What this adds
tests/integration/chains/— a parameterized suite that drives whole chains through the real seam with zero infrastructure:It asserts the two things the outage needed and nobody was checking:
New chains are added as
ChainCaserows. The only non-production seam is_import_handler_class, patched so the resolver returns the in-module handler rather than importing a package path.Negative control (would_have_caught)
Run locally against this exact suite, toggling only
handler_wiring.py:handler_wiring.pydev@e2110e1ae(pre-fix)The failure reproduces the production log lines verbatim:
The no-
db_iocontrol row passes in both configurations, which localizes the failure todb_ioarm selection rather than to the handler, the bus, or the engine.Why the
db_iorow isxfail(strict=True)The fix (#2937) has not merged, so that row is genuinely RED on
devtoday. Rather than block this gate behind another PR, the row carriesbroken_by="OMN-16767"→xfail(strict=True), the same "known drift tracked in an open ticket" patternkafka-boundary-compatalready uses in this file.Consequences, both verified locally:
2 passed, 2 xfailed) and actively blocks every other chain — rule 5 is satisfied now, not later.broken_byis documented as usable only for a ticket whose fix is already in flight — never to silence a new failure, since a chain that starts failing is a dead chain, which is the whole point of the gate.Gate wiring (same PR, per rule 5)
ci.yml: new unconditionalevent-chain-gatejob, display nameEvent Chain Gate. Noneeds:/if:, so it is always present.scripts/ci/ci_summary_gate.py:"Event Chain Gate"added toSTRICT_GATE_JOBS.Both halves matter. The default-deny sweep already fails
CI Summaryon any non-allowlisted job failure; theSTRICT_GATE_JOBSentry additionally fails closed when the job isskippedor absent, so the gate cannot be silently retired by deleting it.Required contexts — what blocks today
omnibase_infra'sdevrequires exactly one context:Because
CI Summaryis already required and its poller is default-deny over the run's jobs, this gate blocks merge the day it lands — no branch-protection edit is needed and none was made.Constraints honored
--no-verify. Full pre-commit + the governed pre-push selector ran green (the selector escalated to the full suite because the diff touches.github/workflows/, cf. OMN-16745; the escalation was honored, not overridden).Ticket
OMN-16774 (AC1, AC3, AC4). Related: OMN-16767, OMN-16769, OMN-16773.
dod_evidence: negative-control matrix above (2 failed→4 passed on the fix toggle), strict-xfail flip verified in both directions,tests/ci/test_ci_summary_gate.py81 passed,check_topic_literals.pyclean, ruff + mypy --strict clean on the new suite.Evidence-Ticket: OMN-16774
Evidence-Source: OCC#7324