Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Review feedback addressed — ea0474cc2

Thanks for the thorough pass — all 14 findings handled, threads replied inline.

Important (4) — all fixed

• 🔴 Bind failure no longer aborts the consumer — degrades to probe-less and keeps draining (verified live: 2nd consumer on a taken :8090 logs the failure and still polls).
• 🔴 Port collision — pluggable discovery starts at 8091 now; 8090 reserved for the consumer.
• 🔴 stop() is defensive (can't mask run()'s exception in the finally) + warns on a lingering thread.
• 🔴 Empty HEALTH_PORT hits the clean opt-out (_env treats "" as unset) instead of crashing.

Suggestions (10) — all addressed

• LivenessServer hardening: double-start guard + reset-on-stop, serve_forever crash logged, log_error routed to the logger (was hidden by log_message=pass), wfile.write guarded against client disconnects, single clock read.
• Types: _httpd/_thread → HTTPServer|None / Thread|None (dropped Any).
• run-worker.sh: status line shows the effective -p override.
• Docstrings: helper trigger phrased on port; 0.0.0.0 documented; design check confirmed — a fast-failing loop stays green by design (liveness must not couple to backend reachability; rationale now in is_poll_stale).
• Tests: top-of-poll stamp test, /healthz+/livez+404 coverage, double-start rejection.

Verified: 8/8 heartbeat/liveness unit tests green; live /health → 200; bind-conflict → graceful degrade. Pre-commit clean.

SonarCloud issues addressed — c2a62fcc2

• consumer.py (Major) — switched the liveness-bind except block from logger.error to logger.exception (preserves the traceback).
• test_pg_queue_consumer.py (Minor) — lifted the walrus := out of the PgQueueConsumer(...) argument list into a plain client = MagicMock() first.

Heartbeat/run-loop tests still green; pre-commit clean.

sequenceDiagram
    participant main
    participant PgQueueConsumer
    participant LivenessServer
    participant DaemonThread
    participant K8sProbe

    main->>PgQueueConsumer: construct (seeds _last_poll_monotonic)
    main->>LivenessServer: _maybe_start_health_server(port, stale_after)
    LivenessServer->>DaemonThread: start (HTTPServer on :8090)
    main->>PgQueueConsumer: run() → poll loop

    loop poll_once()
        PgQueueConsumer->>PgQueueConsumer: stamp _last_poll_monotonic
        PgQueueConsumer->>PgQueueConsumer: read + handle tasks
    end

    K8sProbe->>DaemonThread: GET /health
    DaemonThread->>PgQueueConsumer: seconds_since_last_poll()
    alt age ≤ stale_after
        DaemonThread-->>K8sProbe: "200 {status: healthy}"
    else "age > stale_after"
        DaemonThread-->>K8sProbe: "503 {status: unhealthy}"
    end

    main->>LivenessServer: stop() [finally]
    LivenessServer->>DaemonThread: httpd.shutdown() → join(5s)

_{Reviews (2): Last reviewed commit: "UN-3544 [FIX] Address Greptile review fe..." | Re-trigger Greptile}

Greptile feedback addressed — b54efabcc

Thanks @greptile-apps (4/5). Both findings handled:

• Query string → 404 (consumer.py:413) — fixed. Handler now matches urlsplit(self.path).path, so /health?probe=k8s → 200. Covered by a new query-string case in the alias test.
• Long-running tasks trip the probe (outside-diff) — working-as-designed, now documented prominently. Stamping at the top of poll_once and freezing during task execution is deliberate (it's what lets a genuinely wedged loop be detected). The consequence — WORKER_PG_QUEUE_CONSUMER_HEALTH_STALE_SECONDS doubles as an upper bound on single-task wall-clock — is now spelled out on _DEFAULT_HEALTH_STALE_SECONDS with your formula: raise it above max(batch_size × worst_case_task_seconds, backoff_max) for longer tasks. The 60s default is safe for the only currently-routed leaf (send_webhook_notification, sub-second); longer tasks belong to the pipeline phase, which will set its own threshold.

8/8 heartbeat/liveness tests green; pre-commit clean.

Both fixes look correct. The urlsplit(self.path).path strip handles query strings cleanly, and the stale-threshold documentation on _DEFAULT_HEALTH_STALE_SECONDS is exactly the right place for it — it's the first thing an operator will read when tuning.

A couple of small observations on the final state:

test_double_start_is_rejected — the server.start() / server.stop() after the RuntimeError block tests restart-after-stop, which is a nice extra. Worth confirming stop() actually resets _httpd and _thread to None — it does (finally: self._httpd = None; self._thread = None), so that path is safe.

_maybe_start_health_server swallowing OSError — already deliberate and documented ("the probe is auxiliary"), just confirming the graceful-degrade path is the right call here. It is.

Nothing blocking. The implementation is clean, the test matrix is thorough (fresh/stale threshold, real bind-and-GET, alias+query-string, 404 guard, double-start, restart-after-stop), and the run-worker.sh port reservation/collision notes are clear. LGTM.

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud