Skip to content

fix(OMN-15868): BaseProjectionRunner retry-loop guard never enters - #2050

Merged
github-actions[bot] merged 1 commit into
devfrom
jonah/omn-15868-projection-runner-retry-loop
Aug 11, 2026
Merged

github-actions[bot] merged 1 commit into
devfrom
jonah/omn-15868-projection-runner-retry-loop

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Aug 11, 2026 •

Copy link
Copy Markdown
Contributor

OMN-15868 — BaseProjectionRunner.run() retry-loop guard never enters

Fixes the root cause behind every BaseProjectionRunner subclass (registration,
delegation, live-events, savings, session-outcome, etc.) silently exiting clean
(exit 0) instead of connecting to Kafka when deployed as a standalone k8s
process — observed live as CrashLoopBackOff on
omnimarket-projection-live-events-writer (OMN-15800, deploy run 31493137810).

Root cause

src/omnimarket/projection/runner.py:

409:        self._running = False
...
736:        while attempts < MAX_RETRY_ATTEMPTS and self._running is not False:

self._running is initialized to the literal False. False is not False
is always False (Python singleton identity), so the guard's second clause
is False on the very first evaluation — before a single AIOKafkaConsumer
is ever constructed. The loop body (the only site that builds
AIOKafkaConsumer, awaits self._consumer.start(), and sets
self._running = True on success) never executes, for any subclass, on any
invocation. Execution fell straight through to
logger.error("Consumer failed after %d retries", MAX_RETRY_ATTEMPTS) and
run() returned normally (exit code 0) with zero real connection
attempts
— Kubernetes reads a clean-but-immediate exit as CrashLoopBackOff.

Seam: self._running semantics before/after

Site Before After Notes
__init__ (runner.py:409) self._running = False unchanged readiness signal; unaffected
__init__ (new) — self._shutdown_requested = False new dedicated shutdown-intent flag
run() while-guard (runner.py:736) attempts < MAX_RETRY_ATTEMPTS and self._running is not False (always False, self-defeating) attempts < MAX_RETRY_ATTEMPTS and not self._shutdown_requested this is the actual bug fix
run() on successful start() (runner.py:765) self._running = True unchanged now reachable
run() per-message check (runner.py:774) if not self._running: break unchanged still the in-flight consume-loop break
_start_health_server_if_configured() readiness lambda (runner.py:434) self._running is True unchanged readiness probe still keyed off _running
shutdown() (runner.py:802) self._running = False self._running = False and self._shutdown_requested = True (new) shutdown now also stops the reconnect loop; every prior writer to _running is preserved

Every existing reader/writer of self._running was enumerated (grep -n "_running\b" src/omnimarket/projection/runner.py) and is listed above —
none were removed or repurposed. self._shutdown_requested has exactly one
writer (shutdown()) and one reader (the run() while-guard).

RED-then-GREEN evidence

tests/test_projection_runner_retry_loop_omn15868.py, run against pre-fix
code (git stash the fix, same test file):

FAILED tests/test_projection_runner_retry_loop_omn15868.py::test_run_retry_loop_actually_enters_and_retries_omn15868
FAILED tests/test_projection_runner_retry_loop_omn15868.py::test_run_retry_loop_recovers_and_sets_running_true_omn15868
2 failed, 3 warnings in 0.57s

Both failed with Failed: DID NOT RAISE <class 'tests.test_projection_runner_retry_loop_omn15868._ControlFlowStop'>
— the fake AIOKafkaConsumer.start() was never even called. Captured log
line (byte-exact): ERROR omnimarket.projection.runner:runner.py:794 Consumer failed after 10 retries, printed with zero elapsed loop
iterations — reproducing the ticket's live signature.

Post-fix (this PR), isolated run of this test file:

tests/test_projection_runner_retry_loop_omn15868.py::test_run_retry_loop_actually_enters_and_retries_omn15868 PASSED
tests/test_projection_runner_retry_loop_omn15868.py::test_run_retry_loop_recovers_and_sets_running_true_omn15868 PASSED
======================== 2 passed, 3 warnings in 0.26s =========================
  • AC1 (test_run_retry_loop_actually_enters_and_retries_omn15868): a fake
    AIOKafkaConsumer whose start() always raises proves the loop body is
    entered at all (start_call_count >= 1) and a real await asyncio.sleep()
    backoff is awaited with the correct exponential-backoff delay
    (RETRY_BASE_DELAY * 2**1).
  • AC2 (test_run_retry_loop_recovers_and_sets_running_true_omn15868): a
    fake consumer that fails twice then succeeds on the 3rd start() call
    results in self._running is True, proving the loop exits via the success
    path (message iteration begins) rather than the retry-exhaustion path.
  • AC3 (live re-verification on OMN-15800's onex-dev live-events writer):
    tracked as a downstream step in this build lane (rebuild → repin → redeploy
    → watch rollout), not verifiable from this PR alone.

Full local focused suite (-k projection, -m "not kafka"):
1408 passed, 15 skipped, 15089 deselected, 4 warnings. Full governed
pre-push suite (escalated to whole-repo because runner.py is a shared
module consumed by every projection-runner subclass):
15280 passed, 47 skipped, 3 deselected, 1 xfailed, 594.54s — zero failures.
ruff format / ruff check --fix / mypy --strict on the changed file:
all clean. pre-commit run --files <changed>: all hooks passed.

Also touched

tests/test_projection_kafka_auth_kwargs_omn15816.py carried a # ... this is a test-only workaround ... filed as its own ticket, not fixed here
comment plus a runner._running = True priming line, documenting this exact
defect as out of scope for OMN-15816. That ticket is this one; the comment
is updated to say so and the now-unnecessary priming is removed. No
assertion or test behavior changed — all 6 tests in that file still pass.

Scope note

Deliberately not bundled with OMN-15800's k8s-manifest PRs (#858/#859/#860/
#861, all already merged, all stay merged) — this is a shared-runner code
defect in omnimarket, not a manifest/infra defect.

dod_evidence: this PR is the dod_evidence for OMN-15868 — RED-then-GREEN test
run above, full governed pre-push suite green, pre-commit run --files
green. Downstream deploy verification (AC3) is tracked as a follow-up
build-lane step (image rebuild in omnibase_infra, digest repin in
omninode_infra, redeploy watch) and will be recorded against this ticket
once complete.

Evidence-Ticket: OMN-15868
Evidence-Source: OCC#6358

self._running is initialized to the literal False in __init__, and
run()'s while-guard read `self._running is not False` -- always False
on a freshly constructed runner (Python singleton identity), so the
loop body (the only site that constructs AIOKafkaConsumer, awaits
start(), and sets self._running = True) never executed for any
subclass, on any invocation. The process logged "Consumer failed
after 10 retries" with zero real attempts and exited 0, which
Kubernetes reports as CrashLoopBackOff (OMN-15800, deploy run
31493137810).

Fix: track shutdown intent with a dedicated self._shutdown_requested
sentinel, separate from the self._running readiness signal. The
while-guard now reads `not self._shutdown_requested`; shutdown() sets
both self._running = False (unchanged, readiness) and
self._shutdown_requested = True (new).

RED-then-GREEN: tests/test_projection_runner_retry_loop_omn15868.py
proves (1) a failing AIOKafkaConsumer.start() is actually attempted
and a real await asyncio.sleep() backoff observed -- RED against
pre-fix code (0 calls, 0 sleeps, loop skipped entirely), and (2) a
start() that fails twice then succeeds sets self._running = True and
exits via the success path, not the retry-exhaustion path -- RED
against pre-fix code (self._running never leaves False).

Also updates the stale test-only `_running = True` priming comment in
test_projection_kafka_auth_kwargs_omn15816.py, which documented this
exact defect as "filed as its own ticket, not fixed here" -- it is
fixed here now, and the priming is no longer necessary.
@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 14 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4d1a037f-295c-4a5e-9e7b-eed6b7134cc4

📥 Commits

Reviewing files that changed from the base of the PR and between 233c1a5 and 90a758e.

📒 Files selected for processing (3)
  • src/omnimarket/projection/runner.py
  • tests/test_projection_kafka_auth_kwargs_omn15816.py
  • tests/test_projection_runner_retry_loop_omn15868.py

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

✅ Architectural Review — PASSED

Errors: 0
Warnings: 0

What this checks

Rule Description
ARCH-TOPIC-001 No hardcoded Kafka topic strings in handler code
ARCH-DI-001 No event_bus=None bypass
ARCH-DI-002 No direct Handler instantiation outside workflow_runner/adapters
ARCH-DI-003 No reinvented DI containers
ARCH-TOPIC-002 contract.yaml topics follow `onex.{cmd

No architectural violations found.

Static architectural lint — no model inference (OMN-14176).

@github-actions

Copy link
Copy Markdown

✅ Hostile Reviewer — PASSED

Blocking findings (critical/error): 0
Total findings: 0
Models succeeded: qwen3-review,qwen3-review-b


Gate semantics

Verdict Meaning Blocks merge?
passed >=2 models succeeded, no critical/error findings No
blocked CRITICAL or ERROR findings found Yes
degraded Fewer than 2 models succeeded (infra unavailable/timeout) Yes (OMN-15110)

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)

jonahgabriel pushed a commit to OmniNode-ai/onex_change_control that referenced this pull request Aug 11, 2026
…6358)

* evidence: OCC companion pass 1 for OmniNode-ai/omnimarket#2050

* evidence: OCC companion self-bind for #6358

---------

Co-authored-by: node-occ-companion-effect <occ-companion-effect@omninode.ai>
@github-actions
github-actions Bot enabled auto-merge (squash) August 11, 2026 14:48
@github-actions
github-actions Bot merged commit b26be02 into dev Aug 11, 2026
183 of 194 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-15868-projection-runner-retry-loop branch August 25, 2026 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant