Repository navigation
feat(OMN-15570): gateway link-health projection (G3, gateway lift Phase 0) - #2692
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 37 minutes Limit 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. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (40)
Comment |
❓ Hostile Reviewer — UNKNOWNBlocking findings (critical): 0 Gate semantics (pilot phase)
Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8468/OMN-8524) |
…ibase_infra#2692 (#6215) * evidence(OMN-15570): author OCC companion for OmniNode-ai/omnibase_infra#2692 OCC companion by node_pr_lifecycle_fix_effect (OMN-13317 F1 / OMN-13990 / OMN-14285). Product PR head bfa0b093646471667a265e4d884af53857fa2e10. * evidence(OMN-15570): self-bind OCC#6215 + rebind contract_sha256 --------- Co-authored-by: omnimarket-bot <bot@omninode.ai>
9b49434 to
384a24f
Compare
…publishes Reconciliation finding D1: publish_status (DEGRADED) publishes directly onto the local bus's canonical outbound topic -- exactly the topic the forwarder's own outbound consumer (NodeGatewayDelivery polling local_consumer, the SAME KafkaTransport object local_bus publishes into in runtime/gateway_forwarder.py:run_gateway_forwarder) is subscribed to. The untagged envelope does not match _forward_outbound_message's loopback skip (only "cloud-to-local"), so it falls through to _prepare_outbound and leaks to the cloud leg -- directly contradicting test_publish_status_degraded_goes_to_local_bus_not_cloud, which only passes today because it uses a bare _MockGatewayBus with no consumer loop (feedback_real_dispatch_path_tests gap). Fix: local-only publishes are now stamped with gateway_direction="local-mirror"; _forward_outbound_message and validate_outbound_message skip both "cloud-to-local" (existing) and "local-mirror" (new) via a shared _LOCAL_ONLY_DIRECTIONS set. The stamping helper is a module-level function, not a method, to stay under the ONEX Pattern Validation method-count threshold on ServiceGatewayForwarder. New test test_real_outbound_consumer_loop_does_not_reforward_degraded_status wires the REAL NodeGatewayDelivery consumer loop against a fake transport that actually connects publish (local_bus) to poll (local_consumer) -- the same object playing both roles, matching the real runtime wiring -- unlike every existing test in this file, which uses two disconnected fakes and so cannot observe this class of bug. Verified RED against the parent commit (f0cdfe4, pre-fix): DEGRADED payload reached the cloud leg. GREEN after this fix. The matching heartbeat-local-mirror half of this bug (#2692, OMN-15570) gets the same fix applied on that branch separately, since it stacks on this one and its own diff must be independently correct. Ticket: OMN-15742 Evidence-Ticket: OMN-15742
…ateway forwarder (#2688) * fix(OMN-15742): bounded reconnect backoff + DEGRADED bus status for gateway forwarder The gateway forwarder's delivery-loop supervision previously raced delivery.wait() against shutdown_event.wait() with no retry: any cloud-leg delivery failure (broker drop, transient network fault) propagated straight out of run_gateway_forwarder and terminated the process (docs/design/2026-08-08-gateway-node-architecture-lift.md axis #10, live restart-count evidence in that doc's 1.2). - runtime/gateway_forwarder.py: new _supervise_gateway_delivery loop wraps the existing delivery.wait()/shutdown race. A delivery failure now retries with bounded exponential backoff + jitter instead of exiting. Reconnect state only resets after the delivery loop survives a full heartbeat_interval_seconds "recovery confirm" window without failing again (a bare delivery.start() succeeding only proves tasks were scheduled, not that the cloud leg is back). Process still exits on shutdown or on an unrecoverable (non-connectivity) failure -- config/heartbeat-task errors. - Once a failure window crosses the contract-declared degraded_after_seconds, one DEGRADED status event publishes via the new ServiceGatewayForwarder.publish_status(), reusing the existing gateway-heartbeat.v1 event shape/topic (ModelGatewayHeartbeat gains status="degraded" + consecutive_failures + detail). Published on the LOCAL bus (not cloud) so DEGRADED stays observable while the cloud leg that caused it is itself unreachable. - contract.yaml: liveness block gains reconnect_backoff_initial_seconds / _max_seconds / _jitter_seconds and degraded_after_seconds (contract_version/node_version patch bump 0.1.0 -> 0.1.1); ModelGatewayForwarderConfig gains the matching typed fields + a max>=initial validator. - Data-plane loop (NodeGatewayDelivery, KafkaTransport) is untouched; supervision wraps it per scope. Also vendors 2 omnimarket node migrations (node_canary_score_reducer/0003, node_projection_registration/0004) via scripts/sync-node-migrations.sh -- required by the repo-wide node-migration-sync pre-commit/CI hook on any omnibase_infra branch. infra#2683 (OMN-15732, split-migration rework) was NOT merged at push time; these 2 files duplicate that in-flight resolution and this PR must be rebased onto dev (picking up #2683's shape) once it lands. No _LEGACY_DEFAULT_SCHEMA_SQL_EXACT_PATHS or exemption/ validator logic touched. Ticket: OMN-15742 (G2, gateway-lift Phase 0) * fix(OMN-15742): stop outbound consumer loop re-forwarding local-only publishes Reconciliation finding D1: publish_status (DEGRADED) publishes directly onto the local bus's canonical outbound topic -- exactly the topic the forwarder's own outbound consumer (NodeGatewayDelivery polling local_consumer, the SAME KafkaTransport object local_bus publishes into in runtime/gateway_forwarder.py:run_gateway_forwarder) is subscribed to. The untagged envelope does not match _forward_outbound_message's loopback skip (only "cloud-to-local"), so it falls through to _prepare_outbound and leaks to the cloud leg -- directly contradicting test_publish_status_degraded_goes_to_local_bus_not_cloud, which only passes today because it uses a bare _MockGatewayBus with no consumer loop (feedback_real_dispatch_path_tests gap). Fix: local-only publishes are now stamped with gateway_direction="local-mirror"; _forward_outbound_message and validate_outbound_message skip both "cloud-to-local" (existing) and "local-mirror" (new) via a shared _LOCAL_ONLY_DIRECTIONS set. The stamping helper is a module-level function, not a method, to stay under the ONEX Pattern Validation method-count threshold on ServiceGatewayForwarder. New test test_real_outbound_consumer_loop_does_not_reforward_degraded_status wires the REAL NodeGatewayDelivery consumer loop against a fake transport that actually connects publish (local_bus) to poll (local_consumer) -- the same object playing both roles, matching the real runtime wiring -- unlike every existing test in this file, which uses two disconnected fakes and so cannot observe this class of bug. Verified RED against the parent commit (f0cdfe4, pre-fix): DEGRADED payload reached the cloud leg. GREEN after this fix. The matching heartbeat-local-mirror half of this bug (#2692, OMN-15570) gets the same fix applied on that branch separately, since it stacks on this one and its own diff must be independently correct. Ticket: OMN-15742 Evidence-Ticket: OMN-15742 * fix(OMN-15742): stop test _Source.poll() from starving the asyncio event loop Root cause of every anomalously long/hanging local pre-push run on this branch throughout this session (observed: 10h33m on the original Mac before this session started per the rolling ledger, then repeated 33min-2h+ hangs on .200 across multiple push attempts -- all traced to the SAME mechanism via a --timeout=60 diagnostic run that caught the stack mid-hang inside NodeGatewayDelivery._run_direction's `while True:` loop). _Source.poll() (a test fake in test_gateway_delivery_service.py) returned `[]` with zero internal `await` suspension points. A coroutine with no real suspension point does not yield control back to the asyncio event loop when awaited -- so when test_real_outbound_consumer_loop_does_not_reforward_degraded_status (added by the prior commit on this branch) drives a REAL NodeGatewayDelivery task loop with _Source as the "idle" cloud consumer, `_run_direction`'s `while True: await source.poll(...)` busy-spins the event loop forever. `task.cancel()` only takes effect at the next real suspension point, so the spinning task is uncancellable: `delivery.stop()` (called in the test's `finally`) awaits `asyncio.gather(*tasks)` on tasks that can never be scheduled to observe their own cancellation, deadlocking the entire test process indefinitely -- explaining every "still running, 100% CPU, no progress" observation this session mistook for slowness rather than a hang. Fix: _Source.poll() now honors timeout_ms via asyncio.sleep(timeout_ms / 1000) before returning empty, matching how a real transport (and this same file's _SharedLocalTransport.poll(), which correctly uses asyncio.wait_for(..., timeout=...)) actually behaves. Every other _Source usage in this file calls deliver_message() directly and never invokes poll() at all, so this costs exactly one test ~50ms and changes nothing else (verified: full file 5/5 passed in 0.20s, vs. hanging past a 60s per-test timeout pre-fix). Ticket: OMN-15742 * fix(OMN-15742): wire required canary field into remaining gateway test fixtures Two gaps surfaced by the fixed full local suite run (previous commit removed the busy-spin hang that was masking these): - test_gateway_forwarder_config.py: the two reconnect_backoff tests (added by this branch's own reconnect-supervision commit, authored before this branch was rebased onto dev's #2690/OMN-15741 G1 commit) never got canary=_canary() added, unlike every other constructor call in the same file. - test_gateway_forwarder_service_handler_seam.py: pre-existing dev-tip gap (unrelated to this branch's diff) -- same class of fix already applied independently on the OMN-15781 branch. Ticket: OMN-15742
…hdog (#2700) * fix(OMN-15742): bounded reconnect backoff + DEGRADED bus status for gateway forwarder The gateway forwarder's delivery-loop supervision previously raced delivery.wait() against shutdown_event.wait() with no retry: any cloud-leg delivery failure (broker drop, transient network fault) propagated straight out of run_gateway_forwarder and terminated the process (docs/design/2026-08-08-gateway-node-architecture-lift.md axis #10, live restart-count evidence in that doc's 1.2). - runtime/gateway_forwarder.py: new _supervise_gateway_delivery loop wraps the existing delivery.wait()/shutdown race. A delivery failure now retries with bounded exponential backoff + jitter instead of exiting. Reconnect state only resets after the delivery loop survives a full heartbeat_interval_seconds "recovery confirm" window without failing again (a bare delivery.start() succeeding only proves tasks were scheduled, not that the cloud leg is back). Process still exits on shutdown or on an unrecoverable (non-connectivity) failure -- config/heartbeat-task errors. - Once a failure window crosses the contract-declared degraded_after_seconds, one DEGRADED status event publishes via the new ServiceGatewayForwarder.publish_status(), reusing the existing gateway-heartbeat.v1 event shape/topic (ModelGatewayHeartbeat gains status="degraded" + consecutive_failures + detail). Published on the LOCAL bus (not cloud) so DEGRADED stays observable while the cloud leg that caused it is itself unreachable. - contract.yaml: liveness block gains reconnect_backoff_initial_seconds / _max_seconds / _jitter_seconds and degraded_after_seconds (contract_version/node_version patch bump 0.1.0 -> 0.1.1); ModelGatewayForwarderConfig gains the matching typed fields + a max>=initial validator. - Data-plane loop (NodeGatewayDelivery, KafkaTransport) is untouched; supervision wraps it per scope. Also vendors 2 omnimarket node migrations (node_canary_score_reducer/0003, node_projection_registration/0004) via scripts/sync-node-migrations.sh -- required by the repo-wide node-migration-sync pre-commit/CI hook on any omnibase_infra branch. infra#2683 (OMN-15732, split-migration rework) was NOT merged at push time; these 2 files duplicate that in-flight resolution and this PR must be rebased onto dev (picking up #2683's shape) once it lands. No _LEGACY_DEFAULT_SCHEMA_SQL_EXACT_PATHS or exemption/ validator logic touched. Ticket: OMN-15742 (G2, gateway-lift Phase 0) * fix(OMN-15742): stop outbound consumer loop re-forwarding local-only publishes Reconciliation finding D1: publish_status (DEGRADED) publishes directly onto the local bus's canonical outbound topic -- exactly the topic the forwarder's own outbound consumer (NodeGatewayDelivery polling local_consumer, the SAME KafkaTransport object local_bus publishes into in runtime/gateway_forwarder.py:run_gateway_forwarder) is subscribed to. The untagged envelope does not match _forward_outbound_message's loopback skip (only "cloud-to-local"), so it falls through to _prepare_outbound and leaks to the cloud leg -- directly contradicting test_publish_status_degraded_goes_to_local_bus_not_cloud, which only passes today because it uses a bare _MockGatewayBus with no consumer loop (feedback_real_dispatch_path_tests gap). Fix: local-only publishes are now stamped with gateway_direction="local-mirror"; _forward_outbound_message and validate_outbound_message skip both "cloud-to-local" (existing) and "local-mirror" (new) via a shared _LOCAL_ONLY_DIRECTIONS set. The stamping helper is a module-level function, not a method, to stay under the ONEX Pattern Validation method-count threshold on ServiceGatewayForwarder. New test test_real_outbound_consumer_loop_does_not_reforward_degraded_status wires the REAL NodeGatewayDelivery consumer loop against a fake transport that actually connects publish (local_bus) to poll (local_consumer) -- the same object playing both roles, matching the real runtime wiring -- unlike every existing test in this file, which uses two disconnected fakes and so cannot observe this class of bug. Verified RED against the parent commit (f0cdfe4, pre-fix): DEGRADED payload reached the cloud leg. GREEN after this fix. The matching heartbeat-local-mirror half of this bug (#2692, OMN-15570) gets the same fix applied on that branch separately, since it stacks on this one and its own diff must be independently correct. Ticket: OMN-15742 Evidence-Ticket: OMN-15742 * fix(OMN-15742): stop test _Source.poll() from starving the asyncio event loop Root cause of every anomalously long/hanging local pre-push run on this branch throughout this session (observed: 10h33m on the original Mac before this session started per the rolling ledger, then repeated 33min-2h+ hangs on .200 across multiple push attempts -- all traced to the SAME mechanism via a --timeout=60 diagnostic run that caught the stack mid-hang inside NodeGatewayDelivery._run_direction's `while True:` loop). _Source.poll() (a test fake in test_gateway_delivery_service.py) returned `[]` with zero internal `await` suspension points. A coroutine with no real suspension point does not yield control back to the asyncio event loop when awaited -- so when test_real_outbound_consumer_loop_does_not_reforward_degraded_status (added by the prior commit on this branch) drives a REAL NodeGatewayDelivery task loop with _Source as the "idle" cloud consumer, `_run_direction`'s `while True: await source.poll(...)` busy-spins the event loop forever. `task.cancel()` only takes effect at the next real suspension point, so the spinning task is uncancellable: `delivery.stop()` (called in the test's `finally`) awaits `asyncio.gather(*tasks)` on tasks that can never be scheduled to observe their own cancellation, deadlocking the entire test process indefinitely -- explaining every "still running, 100% CPU, no progress" observation this session mistook for slowness rather than a hang. Fix: _Source.poll() now honors timeout_ms via asyncio.sleep(timeout_ms / 1000) before returning empty, matching how a real transport (and this same file's _SharedLocalTransport.poll(), which correctly uses asyncio.wait_for(..., timeout=...)) actually behaves. Every other _Source usage in this file calls deliver_message() directly and never invokes poll() at all, so this costs exactly one test ~50ms and changes nothing else (verified: full file 5/5 passed in 0.20s, vs. hanging past a 60s per-test timeout pre-fix). Ticket: OMN-15742 * fix(OMN-15742): wire required canary field into remaining gateway test fixtures Two gaps surfaced by the fixed full local suite run (previous commit removed the busy-spin hang that was masking these): - test_gateway_forwarder_config.py: the two reconnect_backoff tests (added by this branch's own reconnect-supervision commit, authored before this branch was rebased onto dev's #2690/OMN-15741 G1 commit) never got canary=_canary() added, unlike every other constructor call in the same file. - test_gateway_forwarder_service_handler_seam.py: pre-existing dev-tip gap (unrelated to this branch's diff) -- same class of fix already applied independently on the OMN-15781 branch. Ticket: OMN-15742 * fix(OMN-15748): gateway poison-pill quarantine + membership-loss watchdog decode_message() ran before the try/except that nacks, so any undecodable record on a mirrored source topic crashed the process (live-observed 2026-08-08T16:50Z). Nacking it (the ordinary failure path) would also be wrong for a permanently malformed record: nack seeks back to the same offset and re-crashes forever. Quarantine instead: log, best-effort dead-letter via the DLQ topic derived from the record's own topic, commit past the offset, keep the loop alive. Also adds an independent membership-loss watchdog to NodeGatewayDelivery. The live 2026-08-09T10:03:07Z incident was a clean single LeaveGroup with zero exception (aiokafka's client-side max_poll_interval_ms idle-eviction, fired from a separate asyncio task than the poll loop) -- the existing exception-triggered reconnect supervision in runtime/gateway_forwarder.py cannot observe a task that is alive, hung, and never exceptions. The watchdog tracks per-direction last-progress timestamps (updated on poll() completion and deliver_message completion) plus a best-effort consumer.assignment() membership probe, and on staleness force-recreates just the affected direction's transport (a fresh client forces a new group join, sidestepping the permanently-stuck idle clock) and publishes a DEGRADED status. Reuses the previously contract-declared but unwired max_silence_window_seconds config field as the staleness threshold. TDD: both regression tests are RED against pre-fix source (verified via git stash of the source-only diff) and GREEN after. OMN-15748 * fix(OMN-15748): consumer-scoped watchdog recovery, no CancelledError escape Fixes two verifier-confirmed defects in the membership-loss watchdog (mergesweep-0809-poisonpath-verify): - wait()/_run_direction: a watchdog stale_task.cancel() made the supervisor's delivery.wait() (a frozen asyncio.gather snapshot) observe a cancelled child and propagate CancelledError -- a BaseException that escapes gateway_forwarder.py's except Exception and kills the process on every recovery. _run_direction now catches CancelledError and returns cleanly when its direction is watchdog-recovering; wait() re-reads the live task set after every completion instead of gathering a one-time snapshot, so a swapped-in replacement task stays supervised. - Recovery no longer calls close()+start() (which stops BOTH the consumer and producer on a KafkaTransport instance shared across directions -- one direction's consumer is the same object the other direction's publish and status/heartbeat emission use as a producer). Adds KafkaTransport.restart_consumer(), a consumer-only recreate that never touches the producer; _recover_stalled_direction now calls it exclusively. De-vacuums the watchdog regression test (arms delivery.wait() before the stall, matching production ordering) and adds direct KafkaTransport-level coverage proving the producer is untouched across a restart_consumer() call. * fix(OMN-15748): address CodeRabbit findings on watchdog wait() + log sanitization Two live (non-outdated) CodeRabbit threads on infra#2700: - wait()'s asyncio.wait was unbounded, waking only on a tracked task's completion. _recover_stalled_direction splices the replacement task into self._tasks strictly AFTER the stale task has already finished and this loop has already woken/rebuilt `watched` from the stale-only list -- so an unbounded wait then blocks on the remaining old tasks with no further wakeup until one of THEM completes. A real exception on the recovered direction would go unsupervised (only surfacing as an "exception was never retrieved" warning at GC). Fixed by bounding the wait with _watchdog_tick_seconds so the loop always revisits self._tasks, and by also picking up an already-completed-with-exception replacement on that rescan. Added test_watchdog_replacement_task_failure_still_reaches_wait, verified RED against the unbounded wait() (reverted locally, re-applied after) and GREEN after. - _quarantine_undecodable_message logged the raw exception via exc_info=error, bypassing the sanitize_error_message() the DLQ payload path already uses a few lines up. A pydantic ValidationError from decode_message() embeds the offending record content, which is untrusted tenant bus traffic that can carry PII/credential-bearing values. Now logs the sanitized message, matching the DLQ path. * test(OMN-15748): harden test_monitor_logs.py subprocess timeouts against CI fleet contention The self-hosted CI fleet runs many concurrent pytest-split shards per job; interpreter spawn/import can occasionally queue behind that CPU contention. A hardcoded 10s subprocess.run(..., timeout=10) tripped this twice live on unrelated PRs' Split 12/15 (infra#2697 and this PR, both times as three subprocess.TimeoutExpired failures at exactly the 10s bound, unrelated to either PR's own diff -- this file isn't touched by either PR's product diff). Bumped to a shared _SUBPROCESS_TIMEOUT_SECONDS = 30 constant across all three call sites: still tight enough to catch a genuine hang, with real headroom against fleet contention.
…se 0) Adds node_gateway_link_health_projection_compute (COMPUTE) and node_gateway_link_health_write_effect (EFFECT), mirroring the canonical node_pr_state_projection_compute / node_pr_state_write_effect pattern. Consumes onex.evt.omnibase-infra.gateway-heartbeat.v1 (published today by node_bus_forwarder_effect / ServiceGatewayForwarder) and materializes a latest-known-state gateway_link_health table (one row per tenant edge, never deleted). A read-time view, gateway_link_health_status, computes health_status by diffing NOW() against last_seen_at against the contract's max_silence_window_seconds threshold (node_bus_forwarder_effect/ contract.yaml:61) -- absence of progress shows up as a stale row, never a missing one, per the ticket's alerting requirement. SCOPE DISCLOSURE: G1 (OMN-15741, path-verifying healthcheck) and G2 (OMN-15742, reconnect supervision) have not merged as of this ticket, so this consumes ONLY the heartbeat topic that ships today. ModelGatewayHeartbeat carries no lag data -- lag_messages/lag_seconds are persisted as nullable columns and the status view's DEGRADED_LAG arms stay inert (NULL comparisons) until a future producer populates them. Only the silence-window threshold is enforceable today. Migration-sync trap: branched after infra#2683 (OMN-15732) merged, so dev's split-migration shape already satisfies the hook; this node's own migration (098_create_gateway_link_health.sql) uses the flat infra-native numbered convention (matching 091_pr_state.sql), not the docker/migrations/forward/nodes/ vendored-from-omnimarket convention -- this node lives in omnibase_infra itself, so it is not subject to sync-node-migrations.sh at all. Verified scripts/sync-node-migrations.sh --check remains clean. Projection + contract only, no omnidash widget (Bret's lane) -- read surface is gateway_link_health_status, queryable directly today; see PR body. Ticket: OMN-15570
The prior CI Summary run (31276973995) polled before Canonical Inference Gate / CodeQL / Duplication Sweep / Omni Standards Gate / Type Safety Validation / URL Authority Gate finished and failed closed on that timing (all six passed once complete, confirmed via gh pr checks + gh api runs). CI Summary has no rerun path (gh run rerun refuses this run: 'workflow file may be broken') and only polls a fresh ci.yml run, so this empty commit re-triggers ci.yml. No content change. Ticket: OMN-15570
…ape ratchet
Canonical Inference Gate / CI Summary flagged
node_gateway_link_health_write_effect as non-canonical
(nonadaptable-single:envelope:object, OMN-14355): new nodes must be born
canonical (def-B, typed-payload handle()), and this repo's every existing
intent-routed write-effect precedent (HandlerPrStateUpsert,
HandlerLedgerAppend) is grandfathered/baselined -- there is no live
canonical exemplar to copy for this exact invocation path.
Root cause: IntentEffectDispatchBridge (OMN-14516, generic bridge shared
by every intent-routed projection) always calls
handle({"payload": ..., "correlation_id": ...}) regardless of this
parameter's static type -- so typing it as
ModelPayloadGatewayLinkHealthUpsert directly would assert a contract the
runtime does not keep, and a Model | dict union fails the checker's own
"dict"-substring exclusion.
Fix: rename the parameter envelope -> payload. canonical_handler_shape.py's
MAGIC_PARAM_NAMES allowlist ("payload" among them) is checked before the
type-annotation heuristic, so this is canonical by the checker's own
definition without misrepresenting the parameter's actual runtime type.
No behavior change -- same coercion logic, same dict/typed-object handling.
Ticket: OMN-15570
…L thresholds (OMN-15762) Producer/consumer topic mismatch (reconciliation lane finding): NodeGatewayLinkHealthProjectionCompute subscribes to onex.evt.omnibase-infra.gateway-heartbeat.v1, but ServiceGatewayForwarder.publish_heartbeat only ever published to the cloud, tenant-prefixed wire topic -- there was no local producer, so this projection would never observe a live event. Bus-is-transport: in-cluster consumers read local canonical topics, not the cloud wire. Fix: publish_heartbeat now also mirrors the same, untransformed envelope onto the local bus's canonical topic, reusing the exact dual-publish shape publish_status (OMN-15742/G2) already established for the DEGRADED transition rather than inventing a second mechanism. Same envelope object both places (envelope_id/correlation_id stay identical across legs for one liveness tick). 098_create_gateway_link_health.sql's health-threshold literals (max_silence_window_seconds=60, lag_threshold_messages=500, lag_threshold_seconds=120) are a 4th independent copy of the contract's liveness block (OMN-15762 gateway-config-triplication class), with no existing repo pattern to materialize a contract value into an already-applied SQL view at migration time. Hard-linked OMN-15762 in the migration header and added tests/unit/db/test_migration_098.py:: test_098_view_liveness_thresholds_match_contract_omn_15762, which regex-extracts the view's literals and fails the suite the moment they diverge from node_bus_forwarder_effect/contract.yaml. The 3 exemption-surface additions (infra-node-allowlist.txt, validation_exemptions.yaml, check_subscribe_wiring_health.py) were already disclosed under "Sanctioned allowlist additions" in this PR's body -- re-confirmed here, not newly added. Ticket: OMN-15570
dev landed 098_create_omninode_internal_schema.sql and 099_create_omninode_internal_live_events.sql while this branch was open. git rebase does not see two differently-named files added at the same ordinal as a conflict, so the duplicate 098 would have merged silently. dev is authoritative on migration numbering; this branch adapts. Renames the forward + rollback SQL and the pinning test, updates the cross-references inside them, and restamps the schema fingerprint (85 migration files, verified).
G1 and G2 had not merged when this branch was written; both have since merged, so three comments asserting otherwise were factually wrong. The disclosure's conclusion is unchanged and still correct: lag_messages and lag_seconds remain always NULL, because G2 widened the heartbeat with status/consecutive_failures/detail rather than with lag telemetry. Only the stated reason needed fixing. Also records the gap that G2's arrival opened: this projection does not read the new status field, so a degraded-but-heartbeating edge still projects as healthy-by-recency. Named in the contract rather than left silent; not fixed here.
6357155 to
56a8ad6
Compare
…link-health projection Before this, health was derived from recency alone. G2 (OMN-15742) added a real status field to the heartbeat, so an edge could report itself degraded while heartbeating perfectly on schedule -- and the projection scored it HEALTHY. A health surface that renders a self-reported-degraded edge as healthy is worse than no surface, because the wrong answer is indistinguishable from the right one. Persists reported_status and consecutive_failures, and adds a DEGRADED_SELF_REPORTED arm to the status view. Precedence is fixed and documented above the view body: stale > self-reported > lag > healthy. Stale outranks the self-report because a silent edge's last self-report is stale too; the self-report outranks lag because it is a first-party statement rather than an inference. reported_status is free TEXT and the view tests <> 'active' rather than matching a closed set, so a status the producer adds later reads as not-healthy instead of crashing the projection or being scored healthy by omission. consecutive_failures is persisted as evidence only -- the producer already folds it into status, and re-deriving a verdict here would let the two disagree about the same edge. The projection requires both fields rather than defaulting them: defaulting a missing status to 'active' would score a malformed producer HEALTHY, and a wrongly-healthy verdict is invisible, unlike silence. Verified by applying the real migration to a throwaway Postgres cluster and reading health_status back out of the actual view -- 13 cases covering each verdict, both simultaneous-signal precedence pairs, degraded/recovery and silence/return transitions, and in-place update on recovery. Confirmed RED-before: deleting the new CASE arm fails 4 of them, including the degraded-beats-recency case.
…y-link-health-projection # Conflicts: # scripts/ci/infra-node-allowlist.txt # tests/fixtures/dispatch_parity/baseline-selection-v2.json
fb01c31 to
29b1308
Compare
Summary
G3 of the gateway lift Phase 0 plan (
docs/design/2026-08-08-gateway-node-architecture-lift.md, commit 9422d30b1, §3): a link-health projection materializing per-tenant-edge gateway liveness, driven by the contract's existing liveness thresholds, alerting on absence of progress rather than a missing row.Adds two nodes mirroring the canonical
node_pr_state_projection_compute/node_pr_state_write_effectingress→projection pattern:node_gateway_link_health_projection_compute(COMPUTE) — subscribes toonex.evt.omnibase-infra.gateway-heartbeat.v1, emits agateway_link_health.upsertintent.node_gateway_link_health_write_effect(EFFECT) — consumes the intent, UPSERTs one row pertenant_idintopublic.gateway_link_health(latest-known-state,ON CONFLICT (tenant_id) DO UPDATE— the row is never deleted).docker/migrations/forward/100_create_gateway_link_health.sql— creates the table plus a read-time view,gateway_link_health_status. This is the load-bearing design choice: health is evaluated at query time, not stored at write time, so a tenant edge that goes silent shows up as a stale row, not a missing one — matching the ticket's "alert on absence of progress" requirement exactly. The view combines two independent signals: recency (NOW()vslast_seen_atagainst a 60s silence window) and the edge's own self-reported status. See the precedence table below.Rebase onto current
dev— 2026-08-18The branch had been open 9 days and
devmoved substantially underneath it. It has been rebased onto currentdevand adapted todev's interfaces —devis authoritative throughout; this branch adapts to it, never the reverse.The gateway-forwarder half of this PR is gone, because
devalready has itThe original 43-file diff has become 35 files. Nothing was lost. Eight files — the forwarder runtime, its config and heartbeat models, the bus-forwarder contract, and their tests — landed on
devindependently via #2736, including the_supervise_gateway_deliveryrestart/backoff supervisor this branch introduced. Those files are now byte-identical between this branch anddev, so they simply stopped being a diff. Verified per file rather than assumed.What remains is exactly this ticket's own scope: the durable projection, its two nodes, the migration, and their tests.
Migration renumbered
098→100(silent collision, caught by hand)devlanded098_create_omninode_internal_schema.sqland099_create_omninode_internal_live_events.sqlwhile this branch was open. This branch's migration was also numbered098.git rebasereported no conflict. Two differently-named files added at the same ordinal are not a textual conflict, so the duplicate098would have merged silently and been found later, on a deployment, by whichever ordering the applier happened to pick.devowns the numbering; this branch moves to100. The forward SQL, the rollback SQL, and the pinning test were renamed, their cross-references updated, and the schema fingerprint restamped (85 migration files,verifypasses).Scope disclosure corrected — it had become false
Three comments (the migration header, the compute contract, and the payload model docstring) asserted that G1 (#2690) and G2 (#2688) "have not merged as of this ticket." Both have since merged, so that was simply wrong and has been fixed.
The conclusion those comments drew is still correct, for a different reason than stated:
lag_messagesandlag_secondsare still alwaysNULL, because G2 widenedModelGatewayHeartbeatwithstatus/consecutive_failures/detail— not with lag telemetry. The inert-lag-arm reasoning stands unchanged.The gap this rebase exposed — resolved in this branch
The rebase surfaced a real correctness problem, and it has been fixed here rather than deferred.
G2 added a
statusfield ("active"/"degraded") and aconsecutive_failurescounter to the very heartbeat topic this projection consumes, and the projection was not reading either. Health came from recency alone, so an edge that was heartbeating perfectly on schedule while reporting itself degraded scoredHEALTHY. That is the worst failure mode an observability surface can have: the wrong answer is indistinguishable from the right one, so nobody goes looking.This branch now persists both fields and adds a
DEGRADED_SELF_REPORTEDverdict.Precedence is fixed, documented above the view body, and tested —
stale > self-reported > lag > healthy:UNHEALTHYDEGRADED_SELF_REPORTEDDEGRADED_LAGHEALTHYTwo design choices worth flagging for review, both deliberate:
reported_statusis freeTEXT, and the view tests<> 'active'rather than matching a closed set. A status the producer adds later (a drain state, say) is therefore read as not-healthy rather than either crashing the projection or being scored healthy by omission. Validating against aLiteralhere would stall the whole projection the first time the producer grew a state.consecutive_failuresdrives no verdict of its own. The producer already folds it intostatus; re-deriving a second verdict from it here would let the two disagree about the same edge. It is persisted as the evidence behind a degraded status, so an operator can tell a blip from a sustained failure without consulting process memory.The projection requires both fields rather than defaulting them. Defaulting a missing status to
"active"would score a malformed or non-canonical producerHEALTHY— and that is the one error this surface must not make, since silence is already visible as staleness but a wrongly-healthy verdict is not.How the verdicts are proven
The verdict is SQL, not Python, so a unit test that greps the migration text would prove the string is present, not that the arms evaluate correctly.
tests/integration/migrations/test_100_gateway_link_health_verdicts.pyapplies the real migration to a throwaway Postgres cluster (reusing the existing ephemeral-cluster fixture) and readshealth_statusback out of the actual view — 13 cases, all passing:NULLlag never decides anything;consecutive_failuresreadable but verdict-neutral;Confirmed RED-before: deleting the new
WHEN reported_status <> 'active'arm fails 4 of those tests, including the degraded-beats-recency case. The suite is not vacuous.A cheap always-runs guard in
tests/unit/db/test_migration_100.pypins the order of the CASE arms, sinceCASEstops at the first match and the order is the precedence — that one still fires when Postgres tooling is absent and the live suite skips.What remains open on the ticket
Lag is the one declared threshold family still unobservable: G1 and G2 both landed without adding lag telemetry, so
lag_messages/lag_secondsstayNULLand their arms stay inert. That is a materially smaller gap than before — silence is observable throughlast_seen_at, and an unwell-but-talking edge is now observable throughstatus.The ticket's remaining residual is the gated
.201live readback, which is blocked on OMN-15521 providing the sanctioned deploy path and is explicitly approval-gated. Not claimed here.Landing order — cleared
The original landing-order block is obsolete: #2689, #2688, #2690, and #2687 have all merged. Nothing gates this PR.
Scope disclosure (read before trusting lag columns)
lag_messages/lag_secondsare persisted as nullable columns and alwaysNULLtoday — no producer publishes lag data on the heartbeat topic.DEGRADED_LAGarms (lag_threshold_messages: 500,lag_threshold_seconds: 120) are inert (NULLcomparisons are neither TRUE nor FALSE in SQL) until a future producer populates them — they never contribute a false verdict.No omnidash widget — projection + contract only. Read surface for Bret:
SELECT tenant_id, health_status, seconds_since_last_seen, reported_status, consecutive_failures FROM gateway_link_health_status WHERE health_status != 'HEALTHY';— a normal projection-style read, no bespoke endpoint.Heartbeat local-bus mirror
ServiceGatewayForwarder.publish_heartbeatnow mirrors the same, untransformed envelope onto the local bus alongside the cloud wire topic. The projection subscribes on the local canonical topic (in-cluster consumers read local topics, never the tenant-prefixed cloud wire topic); before this, heartbeats only ever reached the cloud leg and the projection would never have seen a live event.This reuses the exact dual-publish shape
publish_statusalready established, and publishes the same envelope object to both legs so the envelope and correlation identifiers stay identical across them rather than minting two envelopes for one liveness tick. Confirmed against currentdev:_publish_with_delivery_retry,_local_bus, and_build_status_envelopeall exist there with the signatures this change uses.Sanctioned allowlist additions (documented, not gate-weakening)
scripts/ci/infra-node-allowlist.txt— 4 entries for the new handler files, same class and justification as the existingnode_pr_state_*entries (platform/infra observability, not business-domain).src/omnibase_infra/validation/validation_exemptions.yaml— 3 entries fortenant_id/principal_idfields typedstrrather thanUUID, same seam and justification as the existingmodel_gateway_heartbeat.pyexemptions immediately above them (these payloads carry the heartbeat's DNS-safe tenant slug and MSK principal verbatim; the principal'st-prefix makesUUIDan invalid representation).scripts/check_subscribe_wiring_health.pybaseline allowlist — 2 entries: thegateway-link-health-upsert.v1command topic (intent-routed, not Kafka-published, same pattern as the existingpr-state-upsert.v1entry) and thegateway-heartbeat.v1event topic (published imperatively byServiceGatewayForwarderrather than via a contract-declared publish list — a pre-existing gap innode_bus_forwarder_effect's own contract that this PR's new consumer surfaces for the first time, not one it introduces).pyproject.toml— both new nodes registered as entry points, required for contract auto-discovery.Verification
Run on this session's host, confirmed
Stickybeatz-Studio.localviahostnamebefore any host-identity claim.mypy --stricton both new node packages: no issues in 16 source files.dev's: exactly the 2 new dispatchers, the 2 new probes, and their derived counters (contracts 113→115, routes 124→126). Nodeventry dropped, so no regeneration was needed.scripts/check_schema_fingerprint.py verify— passes at 85 migration files.095–099fromdev,100from this branch. No duplicates.Not merged by this lane. No live
.201or AWS mutation — build and PR only.Ticket: OMN-15570
Evidence-Ticket: OMN-15570
Evidence-Source: OCC#6215