Repository navigation
feat(OMN-14135): fail-closed CI+pre-commit gate — every contract-declared handler must expose a dispatch entrypoint - #2285
Conversation
… must have a dispatch entrypoint A handler can be contract-declared, auto-wired, instantiated, boot clean, and GREEN in CI while having NO dispatch entrypoint at all. `_make_dispatch_callback` resolves `handle_async` then `handle`; finding neither it binds `_missing_handle`, which raises ModelOnexError on the FIRST real dispatch. Registration and routing do NOT imply executability. The dispatcher still registers and the payload still routes to it, so every upstream signal stays green — which is why the OMN-14488 route-coverage gates cannot see this (their fake handler HAS a `handle`), and why contract tests cannot: they prove INGRESS, never DISPATCH. HandlerA2ATask shipped exactly this way (OMN-14489): declared, wired, ingress-valid, CI-green, exposing only submit()/watch(). It is not alone — a platform census found 40 of 153 distinct contract-declared handlers have no entrypoint. Gate (canonical validator, not a bespoke script): - src/omnibase_infra/validators/handler_dispatch_entrypoint.py — uses the EXACT predicate auto-wiring uses, so the gate describes the runtime rather than itself. - Wired BOTH as a required CI step (ci.yml lint job, unconditional — the change-aware test selector could narrow a pytest-only gate away on the very PR that adds a bad handler) AND as a pre-commit hook. Detection without enforcement gets ignored (CLAUDE.md §5). - Pre-commit triggers on contract.yaml too: a contract can declare an entrypointless handler without any .py changing. Ratchet (config/validation/handler_dispatch_entrypoint_baseline.yaml), SHRINK-ONLY: - a handler NOT baselined with no entrypoint -> FAIL (no new instances, ever); - a handler baselined that GAINS an entrypoint -> FAIL until removed (a fixed handler still listed is STALE, so the list cannot rot). HandlerA2ATask is deliberately NOT baselined — it carries a real def-B entrypoint, and a test asserts it is never granted an exemption. RED proofs (against the shipped validator, not a surrogate): - HandlerA2ATask reverted to submit()/watch()-only -> exit 1 (EXISTS-but-WRONG, not absence) - HandlerLedgerProjection de-baselined, source untouched -> exit 1 under BOTH its contracts - a fixed handler left in the baseline -> exit 1 (stale-ratchet guard) - clean tree -> exit 0 The pytest gate imports the validator rather than re-implementing the scan, so it exercises the artifact that actually runs. Refs: OMN-14135 (parent), OMN-14489 (victim)
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 36 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 (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…dispatch-entrypoint gate (#4074) * evidence(OMN-14135): OCC companion for omnibase_infra#2285 — handler dispatch-entrypoint gate Evidence-Source: OmniNode-ai/omnibase_infra#2285 Evidence-Ticket: OMN-14135 Companion evidence for the fail-closed wiring-time guard OMN-14135 asks for. A handler can be contract-declared, auto-wired, boot clean and GREEN in CI while exposing NO dispatch entrypoint: _make_dispatch_callback resolves handle_async then handle, and finding neither binds _missing_handle, which raises on the FIRST real dispatch. Registration and routing do NOT imply executability, which is why the OMN-14488 route-coverage gates are structurally blind to it and why contract tests (which prove INGRESS, never DISPATCH) never saw it. HandlerA2ATask shipped this way; a census found 40 of 153 distinct contract-declared handlers have no entrypoint. Four product-observing evidence items, each pinned to PR head 29c92ef2 and each executed before being recorded: - runtime-predicate: the validator reuses the EXACT auto-wiring predicate (describes the runtime, not itself) - enforcement-ci-precommit: same validator wired as a required CI step AND a pre-commit hook (enforcement, not detection) - red-exists-but-wrong: HandlerA2ATask is NOT baselined and a test forbids ever baselining it; the ratchet also fails a FIXED handler left listed, so it cannot rot - non-vacuity: a collapsed scan fails closed, the predicate is proven to discriminate, and the test drives the SHIPPED validator rather than a re-implemented copy No check_value self-polls the companion PR; every one reads product content at the pinned product SHA. * evidence(OMN-14135): bind self-commit receipt to OCC PR #4074 (pr_ticket_mismatch fix) occ-preflight requires at least one PASS receipt per ticket to bind to the OCC PR under check (or one of its commit SHAs). All five receipts bound to product PR #2285, so eligibility failed with reason=pr_ticket_mismatch. The self-bind item is the one that is supposed to carry the OCC-side binding; the four product-observing items correctly stay bound to #2285 and keep reading product content at the pinned product SHA. * evidence(OMN-14135): rebind infra 2285 empty-rerun head
…merge
omnimarket#2285 merged at 2026-09-04T00:47:17Z as 8863472ce4730652d5074cdaad0cdf7f63215554,
declaring CATALOG ownership for the 24 public-schema relations this PR's grant
migrations target. `Application Database Domain Enforcement (OMN-15361)`
resolves the ownership manifest from `.proof-dependencies/omnimarket` at THIS
pin, so without the advance the gate still reads the pre-merge manifest and
still fails `Enforce schema qualification in changed SQL` with 24
`requires exactly one ownership declaration` lines.
Forward-only, as check_omnimarket_contract_pin_advance.py requires:
a0941e08 -> 8863472c, and 8863472c is omnimarket dev tip with a0941e08 as an
ancestor.
NO regeneration accompanies it, and that is a verified fact rather than an
omission. The pin file's own header requires regenerating the grants and the
rendered catalogs whenever it moves by hand, so both were run:
scripts/generate_application_database_table_grants.py --contracts-root
<omnimarket@8863472c>/src/omnimarket/nodes --write
-> loaded 69 db_io.db_tables declaration(s) plus 2 state_io and 1 legacy
instances updated: (none)
catalogs rendered: (none)
which is the expected result: #2285 changed `scripts/application-relation-ownership.yaml`
and the generated relation inventory, neither of which is a `db_io.db_tables`
input, so the derived TABLE grants cannot move. Re-proved with --check --prove:
PASS=55 FAIL=0 RESIDUAL=0 on every one of the six shipped profiles (local,
onex-dev, onex-prod, prod, stability-test, test).
The gate this unblocks now passes locally against the merged manifest:
`application_database_sql_gate=PASS`, exit 0 -- against `FAIL` with one line per
relation before #2285.
PROVENANCE NOTE, recorded rather than hidden: this one-line edit appeared in the
worktree at 00:49Z, two minutes after #2285 merged, written by an out-of-band
pin-refresh process rather than by this lane's own hand. It was not accepted on
sight -- the value was independently verified to equal #2285's merge commit,
the diff was confirmed to be exactly one line, and the regeneration the header
mandates was executed and proved to be a no-op before committing.
Refs: OMN-17440, OMN-17292, OMN-15361, OMN-15701
…et 27 -> 1 (#3162) * fix(OMN-17440): deliver the remaining 26 topology TABLE grants, ratchet 27 -> 1 The generated topology declares 65 TABLE grants. Before this change 38 were issued by a migration and 27 were issued by nothing, so a lane that has never been hand-patched -- a fresh staging namespace, a rebuilt onex-dev, prod -- authenticates fine and then refuses every write with InsufficientPrivilege while the runtime reports healthy and commits its offsets. That is the OMN-16993 / OMN-17379 shape, and it has now recurred five times in a fortnight (OMN-15701, OMN-16436, OMN-16993, OMN-17290, OMN-17374). MEASURED, NOT ASSUMED. Two probes, both run 2026-09-03 at dev 4053dc3: corpus `uv run python scripts/validation/check_topology_grant_delivery.py` -> `topology grant delivery: 27 undelivered (of 65 declared)`, exit 0 (the gate PASSES at its own ratchet -- the bound was the defect's bookkeeping, not its bound) lane read-only SELECT against information_schema.role_table_grants in omnidash_analytics on the .201 dev lane (compose project omnibase-infra), diffed against the checked-in topology: declared 65 present live 63 present live AND issued by a migration 38 present live and issued by NOTHING 26 <- delivered here declared, absent live 2 (nightly_loop_configs, delegation_shadow_comparisons; both relations absent) live, declared by nothing 4 (AC3, not this change) So this is not a hypothetical residual. All 26 are ALREADY live dependencies of the dev lane that exist only because somebody typed the GRANT by hand, which is exactly why the gap stayed invisible for as long as it did. WHAT LANDS: 21 new node-owned migrations, one per owning lineage, issuing the 26 grants. Each is a pure forward ADD next to an untouched creating migration -- none of the 21 parents is edited, because each is applied on the .201 dev lane with a recorded content_sha256 and an in-place repair would raise "conflicting migration checksum in canonical node history" (the OMN-16705 constraint that 083/087 and 0032/0033 already hit). Each file grants SELECT/INSERT/UPDATE and deliberately no DELETE, re-asserts its schema USAGE rather than assuming a sibling ran, and asserts BOTH the INSERT and the SELECT took, with the fail-loud `SELECT 1 / count(*)` shape flat 099 and node_projection_session_replay/0002 already use. Not one bulk grant file, for the OMN-15701 reason: a shared cross-node grant block is precisely how a relation added to a node later silently misses its grant. RATCHET: MAX_UNDELIVERED 27 -> 1. The 1 is nightly_loop_configs, which NO migration can deliver -- nothing in the corpus issues a CREATE TABLE for it, so there is no lineage to land a grant in and no relation for a grant to bite on. `test_the_two_tranches_together_are_the_whole_deliverable_set` is added so the bound is a FLOOR rather than a reading: it asserts the undelivered set is exactly that one pair, so a future change cannot satisfy the count by leaving a DIFFERENT grant undelivered. Closing it to 0 (AC2) is a contract decision -- either node_nightly_loop_controller grows a CREATE TABLE for the relation its contract declares under db_io.db_tables, or that declaration is removed -- and is deliberately not made by adding a GRANT for a relation that does not exist. The sequence half is unaffected: MAX_UNDELIVERED_SEQUENCES stays 0, because the sequence requirement is derived from DECLARED grants and the corpus's applied column shapes, not from delivered ones. Re-verified: 0 undelivered (bound 0). TDD, RED proven before GREEN. With the 21 SQL files moved aside and only the tests present: 29 failed, 37 passed. With them restored: 66 passed. The manifest exactness ratchet moves 150 -> 171 in the same change. Vendored into omnibase_infra first per the node-migration-vendor-parity-gate ordering, ahead of the omnimarket source PR; the 21 application-migrations.tsv rows are what make that infra-ahead-by-one state legal rather than drift, read by sync-node-migrations.sh --check as preserved history via the OMN-15717 legacy-declared exemption (`--check: in sync`, exit 0). Local gates, all green, no bypass of any kind: pre-commit run --all-files exit 0, 103 hooks Passed, 0 Failed scripts/sync-node-migrations.sh --check exit 0 scripts/validation/validate_application_migration_manifest.py exit 0 (171 active, 0 blocked, 2 historical, 30 cloud aliases) scripts/validation/validate_migration_sequence.py exit 0 scripts/validation/check_migration_rls_policy_atomicity.py exit 0, clean scripts/validation/validate_migration_freeze.py exit 0 uv run pytest tests/ci/test_topology_grant_delivery.py -q 66 passed uv run pytest tests/unit/scripts/validation/test_application_migration_manifest.py -q 20 passed scripts/validation/check_topology_grant_delivery.py 1 undelivered (bound 1) No lane mutation: the .201 probes were SELECT-only and read no secret value; prod, stability-test and judge were not contacted at all. Refs: OMN-17440 (AC1, AC2 to its floor), OMN-17374, OMN-17447, OMN-16993, OMN-17379, OMN-17377, OMN-15701, OMN-16436, OMN-17290, OMN-16705, OMN-15717 * fix(OMN-17440): advance the omnimarket contract pin to the ownership merge omnimarket#2285 merged at 2026-09-04T00:47:17Z as 8863472ce4730652d5074cdaad0cdf7f63215554, declaring CATALOG ownership for the 24 public-schema relations this PR's grant migrations target. `Application Database Domain Enforcement (OMN-15361)` resolves the ownership manifest from `.proof-dependencies/omnimarket` at THIS pin, so without the advance the gate still reads the pre-merge manifest and still fails `Enforce schema qualification in changed SQL` with 24 `requires exactly one ownership declaration` lines. Forward-only, as check_omnimarket_contract_pin_advance.py requires: a0941e08 -> 8863472c, and 8863472c is omnimarket dev tip with a0941e08 as an ancestor. NO regeneration accompanies it, and that is a verified fact rather than an omission. The pin file's own header requires regenerating the grants and the rendered catalogs whenever it moves by hand, so both were run: scripts/generate_application_database_table_grants.py --contracts-root <omnimarket@8863472c>/src/omnimarket/nodes --write -> loaded 69 db_io.db_tables declaration(s) plus 2 state_io and 1 legacy instances updated: (none) catalogs rendered: (none) which is the expected result: #2285 changed `scripts/application-relation-ownership.yaml` and the generated relation inventory, neither of which is a `db_io.db_tables` input, so the derived TABLE grants cannot move. Re-proved with --check --prove: PASS=55 FAIL=0 RESIDUAL=0 on every one of the six shipped profiles (local, onex-dev, onex-prod, prod, stability-test, test). The gate this unblocks now passes locally against the merged manifest: `application_database_sql_gate=PASS`, exit 0 -- against `FAIL` with one line per relation before #2285. PROVENANCE NOTE, recorded rather than hidden: this one-line edit appeared in the worktree at 00:49Z, two minutes after #2285 merged, written by an out-of-band pin-refresh process rather than by this lane's own hand. It was not accepted on sight -- the value was independently verified to equal #2285's merge commit, the diff was confirmed to be exactly one line, and the regeneration the header mandates was executed and proved to be a no-op before committing. Refs: OMN-17440, OMN-17292, OMN-15361, OMN-15701
Evidence-Source: OCC#4074
Evidence-Ticket: OMN-14135
Evidence-Commit: 61f2845c4c977971ed11f0ef0e12da841b36a094
Partially addresses OMN-14135 — this PR closes 1 of 6 known silent-death mechanisms. It does NOT close OMN-14135.
A green from this gate does NOT mean a handler is alive. It means the handler has a callable dispatch entrypoint. That is necessary but nowhere near sufficient.
At least six independent mechanisms silently kill a node while the runtime boots green. This gate catches one of them:
consumer_purpose: audit|projection+ contract absent from theservice_kernel.pyresult-applier allowlist → contractSKIPPEDat wiring, class never constructedhandle()/handle_async()→ auto-wiring binds_missing_handle→ raises on every dispatchFAILED→ non-strict mode logs a WARNING → boots greensubscribe_topicsbut has ZEROhandler_routing(6 of the 13 known-dead — the biggest cluster)discovery.py:46-107) — invisible to any manifest-derived census, including this gate'sunresolvable_handler) → PARTIAL death: contract wires, consumer group is live, individual handlers silently absentThe concrete trap this PR must not enable: adding a
handle()toHandlerLedgerProjectionand deleting its two baseline rows makes this gate go green whileevent_ledgerstill holds ZERO rows — because that contract isSKIPPEDat wiring (mechanism 1) and the class is never constructed. That is precisely how OMN-14134 shipped as a merged, CI-green,Done-marked, 100% runtime no-op. Do not read a green here as liveness.Empirically: 5 fully-entrypointed nodes are stone dead (
node_bus_forwarder_effect,node_pr_state_write_effect,node_runner_fleet_health_compute,node_runner_fleet_maintain_orchestrator,node_runner_health_snapshot_effect). This gate gives all five a clean bill of health. That is the honest measure of its scope.The primary gate — subscription-coverage (declared consumer + no registered subscription = HARD FAIL) plus
quarantined == 0— is tracked in OMN-14517 and is still owed. This PR is the cheap secondary: it catches mechanism 2 earlier (at commit/CI rather than at boot) with a precise error message. It is worth having. It is not the gate.The defect class
A handler can be contract-declared, auto-wired, instantiated, boot clean, and GREEN in CI — and still have NO dispatch entrypoint at all.
_make_dispatch_callback(runtime/auto_wiring/handler_wiring.py:412-462) resolves the runtime entrypoint by looking forhandle_async, thenhandle. Finding neither, it binds_missing_handle(:455-462), which raisesModelOnexErroron the first real dispatch.Registration and routing do NOT imply executability. The dispatcher still registers and the payload still routes to it, so every upstream signal stays green:
This is why nothing caught it. Contract tests prove INGRESS and never prove DISPATCH. The OMN-14488 route-coverage gates prove a payload routes to a registered dispatcher — but a dispatcher bound to
_missing_handleregisters and routes just fine, and that gate's fake handler has ahandle, so it is structurally blind here.HandlerA2ATaskshipped exactly this way (OMN-14489, merged in #2280): declared, wired, ingress-valid, CI-green, exposing onlysubmit()/watch(). It is not alone — a platform census found 40 of 153 distinct contract-declared handlers have no entrypoint.This PR is the fail-closed wiring-time guard OMN-14135 asks for ("a wiring-time guard/validator that fails closed when a routed handler lacks a callable entry point, so this class can't silently recur").
The gate
src/omnibase_infra/validators/handler_dispatch_entrypoint.py— a canonical validator (the establishedvalidators/pattern, not a bespoke script). It uses the exact predicate auto-wiring uses, so the gate describes the runtime rather than describing itself.Wired as enforcement, not detection (CLAUDE.md §5) — both in this PR:
ci.ymllint job. Deliberately not pytest-only: the change-aware test selector could narrow a pytest gate away on the very PR that adds a bad handler.handler-dispatch-entrypoint. Triggers oncontract.yamlas well as.py— a contract can declare an entrypointless handler without any Python changing.The pytest gate imports the validator rather than re-implementing the scan, so the tests exercise the artifact that actually runs; a test carrying its own copy of the predicate could stay green while the shipped validator was broken.
Ratchet: shrink-only, cannot rot
config/validation/handler_dispatch_entrypoint_baseline.yamlfreezes the 40 pre-existing violations (a no-baseline gate would be instantly red on 40 handlers and could never land).The list can only shrink. End state is empty, at which point
_missing_handlebecomes a wiring-time hard failure instead of a per-message one.HandlerA2ATaskis deliberately NOT baselined — it carries a real def-B entrypoint, andtest_a2a_task_is_not_baselinedasserts it can never be granted an exemption. Evidence asserts the desired end state, never the current broken one.dod_evidence — RED proofs against the SHIPPED validator
Each proof drives
python -m omnibase_infra.validators.handler_dispatch_entrypoint(the exact command CI and pre-commit run), not a surrogate:HandlerA2ATaskreverted tosubmit()/watch()-only (exists, contract-declared, wires cleanly)node_remote_agent_invoke_effect: HandlerA2ATaskHandlerLedgerProjectionde-baselined, source untouchedThis is RED against EXISTS-but-WRONG, not against absence. A gate that only fires when a handler is missing would be vacuous.
Non-vacuity is itself gated:
test_contracts_were_actually_scanned+ a validator-internal floor fail closed if the contract scan collapses (a gate over an empty set proves nothing), andtest_entrypoint_predicate_discriminatesproves the predicate distinguishes the two states rather than returning True unconditionally.Scope note
HandlerBuildLoopAppendis listed on OMN-14135 as a confirmed victim. It is not one at current dev head: it exposeshandle(self, envelope) -> ModelHandlerOutput[ModelBuildLoopAppendResult], so auto-wiring binds it and it will not hit_missing_handle. It is envelope-shaped rather than def-B (a different defect, OMN-14355), so it is out of scope here. The ticket text is stale on that point.HandlerLedgerProjectionis a genuine victim (entrypointless under both its contracts). It is baselined here and repaired separately under OMN-14516 — that PR must delete its two baseline rows in the same change, or the stale-ratchet guard fails it.Local gates
ruff format/ruff checkcleanmypycleanpre-commit runclean on all changed files (incl. the new hook, which passes)pytest tests/unit/runtime/auto_wiring/→ 348 passed