fix: unbounded embeddings wait (timeout=None) and typed 502 for allowlist misses

[seonghobae]

Summary

Two runtime regressions, fixed test-first. This PR is based on origin/main @ 5665b0ad.

1. Default /v1/embeddings path: math.isfinite(None) raises TypeError

• On the default path, /v1/embeddings calls complete_embeddings_batch(..., wait_timeout=orchestrator.client._resolved_model_timeout(agent)), and that value is None by default (the no-implicit-deadline default).
• complete_embeddings_batch always forwards it to backend.wait(job, timeout=wait_timeout). Its docstring already defines wait_timeout=None as "wait without an application deadline".
• ProviderEmbeddingBatchBackend.wait (batch_routing.py) did math.isfinite(timeout), which raised TypeError: must be real number, not NoneType. Member failover swallowed the error, so clients saw 503 embeddings_unavailable on every non-mock:// embedding request.
• History:
  • 284447fc introduced wait(timeout: float | None) with None = unbounded.
  • 56a34abc added inf -> None handling.
  • A later restack kept only the isfinite form, dropping None.
  • The existing regression test test_unbounded_synchronous_embedding_waits_for_provider_completion lives in tests/test_provider_embedding_batch_backend.py, which currently fails to collect on main. That's why the regression was missed.
• Fix: wait(..., timeout: float | None). None and non-finite values both mean threading.Event.wait(timeout=None) (block until terminal), matching the documented wait_timeout=None semantics and the existing inf handling. Finite deadlines are unchanged.
• Unchanged on purpose: the previous code did not reject NaN, negative, or bool timeouts. NaN was already treated as unbounded, and a negative value returns immediately after a poll. This PR doesn't add new rejections; it only restores None.

2. Allowlist misses return HTTP 500 instead of the typed 502

• d26fa132 classified an allowlist miss as ProviderUpstreamError(error_code="provider_connection_error", client_status=502, retryable=False, transport="chat").
• The EgressWeave rewrite (fix(security): route allowlisted provider validation through EgressWeave #1046) replaced both raise sites in ModelClient._validate_allowlisted_provider with a plain RuntimeError, which escaped provider failover as HTTP 500.
• Fix: a small ModelClient._provider_host_not_allowlisted(agent) helper returns the typed error again, and it is raised from both EgressWeave branches (except EgressNotAllowedError as exc: ... from exc and validated is None).
  • EgressWeave's validation, pinned-address reuse (DNS-rebinding protection), and message text ("<agent> provider host is not allowlisted", which never names the host) are unchanged.
  • Passthrough callers still relabel transport via classify_provider_failure.

Tests

• New file tests/test_embeddings_unbounded_wait.py (5 tests; fully mocked, no network or live provider calls). It goes in a new file so it doesn't touch tests/test_provider_embedding_batch_backend.py, which fix(tests): restore main's test signal; re-apply dropped OpenRouter availability/quality split #1266 edits.
  • backend wait(timeout=None) blocks until terminal
  • a finite deadline is still honoured
  • inf still blocks until terminal
  • complete_embeddings_batch with the default wait_timeout completes
  • the default server /v1/embeddings path, with ModelClient.timeout=None, returns 200 and backend.wait sees timeout=None
• Existing tests restored to green without modification:
  • tests/test_provider_reliability.py::test_unallowlisted_provider_host_is_classified_upstream_error
  • ::test_passthrough_allowlist_failure_reports_passthrough_transport
  • ::test_free_model_exhausted_allowlist_pool_fails_closed_as_502

RED on main (source identical to origin/main, new test file added)

uv run --no-sync python -m pytest -q -ra -p no:cacheprovider tests/test_embeddings_unbounded_wait.py \
  "tests/test_provider_reliability.py::test_unallowlisted_provider_host_is_classified_upstream_error" \
  "tests/test_provider_reliability.py::test_passthrough_allowlist_failure_reports_passthrough_transport" \
  "tests/test_provider_reliability.py::test_free_model_exhausted_allowlist_pool_fails_closed_as_502"

Result: 6 failed, 2 passed.

• TypeError: must be real number, not NoneType x2
• server 503 != 200
• RuntimeError: blocked_agent provider host is not allowlisted x2
• RuntimeError: all 2 candidate agents failed for role=worker

The 2 passes are the finite-deadline and inf guards.

GREEN with this PR

Same command: 8 passed.

Full suite versus main

Command: uv run --no-sync python -m pytest -q -ra -p no:cacheprovider --continue-on-collection-errors. Main is already red and is being handled separately.

	failed	passed	skipped	errors
main 5665b0ad	201	4777	6	31
this PR	198	4785	6	31

• The only ids that left the failure list are the 3 allowlist tests above.
• No failure or error id is new relative to main.
• The +8 passed are the 3 restored tests plus the 5 new ones.

Overlap with open PRs

I checked the diffs of #1262, #1264, #1265 and #1266 first.

• The orchestrator.py hunk here is around L3972-4002 (_validate_allowlisted_provider). The open PRs touch orchestrator.py at: refactor(reporting): move commercial report generators out of TaskOrchestrator (ADR 0124 step 1) #1262 imports/~L565/>=L12964; fix(conduct): answer with the synthesis and judge that answer (scoring unchanged) #1264 ~L9596; fix(judge): classify judge failures (misconfigured / unavailable / ordinary rejection) without penalising candidates #1265 L9235-9337 and L11920-12089; fix(tests): restore main's test signal; re-apply dropped OpenRouter availability/quality split #1266 L5571.
• None of them touch batch_routing.py, tests/test_provider_reliability.py, or the new test file.

Not verified locally

• The native decision_receipt extension isn't built in the local venv (same as the main baseline).
• CI test jobs are skipped for draft PRs by their if: conditions.
• No live provider calls were made.

Verified complete successor carryover — 2026-09-27

Canonical successor #1266 exact ca5efdc023cad70210f5b1d2114a1aa417009c81 completely carries this PR's valid delta.

• Its production source owns the same timeout=None/non-finite wait contract and typed, non-retryable allowlist 502 boundary.
• Existing successor suites retain the direct unbounded/infinite/coordinator and allowlist classification/failover cases.
• Successor 2eceb6474c2904ae308991e89595b1f44c852940 adds the remaining distinct finite-deadline and real default HTTP /v1/embeddings fixtures; Gap receipt ca5efdc023cad70210f5b1d2114a1aa417009c81 records the mapping.
• Exact successor test blob 8db8c9ec80757d25cd715aa3acaf191c7f613640 compiles and the isolated finite-deadline case passes. Fresh successor hosted Checks and independent approval remain open gates.

This PR is retired only because every valid production requirement and executable contract has a verified successor. It is not merged, released, or treated as completed delivery evidence.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[seonghobae]

Verified complete successor carryover: #1266 exact ca5efdc023cad70210f5b1d2114a1aa417009c81 owns both production repairs and all valid executable requirements. Existing successor tests cover direct unbounded/infinite/coordinator behavior and allowlist classification/failover; 2eceb647… adds the remaining finite-deadline and default HTTP /v1/embeddings timeout=None fixtures. Exact successor blob 8db8c9ec… compiles and its isolated finite test passes. Retiring this duplicate lane does not assert hosted GREEN, approval, protected integration, release, or delivery completion.