fix: make gateway reliability safe by default (#3438) - #3442
Conversation
The unset (None) reliability posture now resolves to a backpressured deployment β bounded admission ceiling + fair wait queue + graceful drain β instead of the old no-admission one. It is bind-aware: a non-loopback bind (a real deployment) resolves to the full production window, loopback keeps a snappy 5s drain. reliability="off" opts back into today's immediate-teardown behaviour; explicit reliability="default" keeps the legacy no-ceiling posture. Co-authored-by: MervinPraison <MervinPraison@users.noreply.github.com>
|
@coderabbitai review |
|
/review |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more β On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
β Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 12 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 Plus Run ID: π Files selected for processing (4)
β¨ Finishing Touchesπ§ͺ Generate unit tests (beta)
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 |
Greptile SummaryThis PR makes the gatewayβs unset reliability posture safe by default.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains, and the prior loopback-classification issue is addressed through IP-based loopback detection with regression coverage.
|
| Filename | Overview |
|---|---|
| src/praisonai-bot/praisonai_bot/bots/_reliability.py | Introduces bind-aware safe defaults and fixes the previously reported handling of noncanonical loopback IP addresses using numeric IP classification. |
| src/praisonai-bot/praisonai_bot/cli/features/gateway.py | Always resolves reliability in no-config startup and passes the effective bind host into resolution. |
| src/praisonai-bot/tests/unit/bots/test_reliability.py | Covers safe defaults, external and loopback drain windows, explicit legacy behavior, and the prior loopback-address finding. |
| src/praisonai-bot/tests/unit/bots/test_outbox_ordering.py | Updates ordering expectations for the new safe unset posture while preserving explicit legacy presets. |
Reviews (2): Last reviewed commit: "fix: recognize all loopback bind forms i..." | Re-trigger Greptile
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
1 similar comment
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
|
Claude finished @MervinPraison's task ββ View job Final Architecture Review β PR #3442 β
Phase 1 β Architecture review (per AGENTS.md)
Phase 2 β Fixes implementedGreptile finding (valid): Fix ( Test ( Files modified this review:
Skipped: Qodo (reviews paused) and CodeRabbit (rate-limited) left no actionable items; no other reviewer findings outstanding. Phase 3 β Verdictβ
Approve. The change is minimal, backward-compatible, correctly layered in the wrapper, and the one valid reviewer finding is now fixed with coverage.
Commit: |
Classify loopback numerically via ipaddress so 127.0.0.2, 127.255.255.255, and expanded IPv6 ::1 keep the snappy loopback drain instead of the 15s production window. Addresses Greptile review feedback. Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
|
@claude CI failed on HEAD Failed checks
Failures (extracted)(Could not extract pytest details β see job logs above.) Critical review firstBefore changing code or tests, decide which side is wrong:
SDK guardrails (AGENTS.md):
What to do
|
|
Claude finished @MervinPraison's task ββ View job Final Architecture Review + CI Triage β PR #3442 β
CI failure triage β verdict: pre-existing infra flake, not a code regressionThe three failing checks are not test failures. All three are GitHub API rate-limit (403) errors inside orchestration workflow scripts β nothing to do with the PR code:
These are transient GitHub-API rate limits on the actor account, not a breakage introduced by this PR. They fix themselves once the limit window resets (a re-run will pass). No code or test change can address them, and per the guardrails I will not weaken/skip any tests to go green. The actual pytest suite for this PR passes locally: (The triage message's "Could not extract pytest details" is because there were no pytest details β the failing jobs never ran pytest.) Phase 1 β Architecture review (per AGENTS.md)
Phase 2 β Reviewer findings
No additional bugs or gaps found on independent review. Phase 3 β Verdictβ Approve. The change is minimal, backward-compatible, correctly layered in the wrapper, and the one valid reviewer finding is already fixed with coverage (37 passed). The reported CI failures are transient GitHub-API rate limits in workflow orchestration, unrelated to this PR β a re-run should clear them. No code changes required.
|
Fixes #3438
Summary
The gateway shipped an unsafe default reliability posture: running it the obvious way (
BotOS(agent=...),praisonai gateway start, or a minimalgateway.yaml) gave no admission ceiling and only a 5s drain, so a burst could fan out unboundedly and a rolling restart could cut in-flight turns. Theproductionmachinery already existed β only the default was unsafe.This changes the unset (
None) posture thatresolve_reliabilityresolves to a safe-by-default deployment, keying off bind posture, keeping all explicit overrides:productionwindow (15s drain).reliability="off"β explicit opt-out to today's immediate-teardown / no-backpressure behaviour.reliability="default"β the explicit legacy posture (5s drain, no ceiling) for callers who ask for it.drain_timeout/max_concurrent_runs/admission_policystill always win.Changes
bots/_reliability.py: addbind_host-aware default resolution +_is_externally_boundhelper; unset posture now yields admission + drain; docstrings updated.cli/features/gateway.py: always resolve reliability in the no-config path and thread the bindhostso an external bind comes up with backpressure.test_reliability.pyandtest_outbox_ordering.py.Layer placement
Wrapper (
praisonai-botgateway runtime). No new public knobs added β only the default value each of the existing CLI/YAML/Python surfaces resolves to.Test plan
python -m pytest src/praisonai-bot/tests/unit/bots/test_reliability.py test_outbox_ordering.pyβ 36 passedpydantic/aiohttpoptional deps), unrelated to this changeGenerated with Claude Code