fix(ModelSync): retry self-fetch with network-error backoff before in-process fallback by NomenAK · Pull Request #14 · NomenAK/OmniRoute

NomenAK · 2026-05-13T03:27:03Z

Summary

Extracts the inline self-fetch in fetchProviderModelsForSync into an exported selfFetchWithRetry(url, opts) helper with linear-backoff retry logic
Retry contract: only network-level failures (ECONNREFUSED, fetch failed) are retried — these indicate the loopback HTTP listener is not yet accepting; HTTP responses (any status) are returned as-is
Default: maxRetries=5, backoffMs=200ms linear (200ms, 400ms, 600ms, 800ms between attempts)
In-process route (getProviderModels) remains as last-resort fallback after all retries exhausted
Updated model-sync-route.test.ts fallback test to expect 5 self-fetch attempts before in-process fallback

Context

At container boot, 17 ModelSync events fire one-shot HTTP self-fetches against http://127.0.0.1:PORT before the Next.js HTTP listener is fully accepting connections. The single-attempt contract produced 17 warnings on every boot. This PR adds retry-with-backoff to give the listener time to come up.

Known limitation: In this deployment the startup race window (Next.js warmup time) exceeds the default 2s retry window (~200ms * sum(1..5)), so all 17 connections still exhaust retries at boot. The in-process fallback continues to work correctly. Increasing backoffMs or maxRetries can extend the window at the cost of higher initial latency for each connection.

Test plan

tests/unit/api/sync-models-readiness.test.ts — 3 new tests: retry-succeeds-on-attempt-3, exhaust-then-fallback, HTTP-errors-returned-immediately
tests/unit/model-sync-route.test.ts — all 18 existing tests pass (updated fallback test assertion)
Sanity: maxRetries=1 prevents retry loop, causing attempt-count assertions to fail

Summary by CodeRabbit

Release Notes

Bug Fixes
- Enhanced provider model synchronization reliability through improved network request retry logic and optimized handling of concurrent operations, reducing redundant attempts during load.
Tests
- Added comprehensive unit test coverage for model synchronization readiness behavior, retry semantics under failure scenarios, and concurrent request coordination.

coderabbitai · 2026-05-13T03:27:12Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 318f1b08-98ee-4806-a7a4-32aea70cea91

📥 Commits

Reviewing files that changed from the base of the PR and between 9236552 and c01a791.

📒 Files selected for processing (3)

src/app/api/providers/[id]/sync-models/route.ts
tests/unit/api/sync-models-readiness.test.ts
tests/unit/model-sync-route.test.ts

Disabled knowledge base sources:

Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📝 Walkthrough

Walkthrough

This PR adds a shared loopback readiness gate and a testable retry wrapper for internal self-fetches in the provider model sync route. The readiness gate probes once per process, prevents boot-time retry amplification across concurrent callers, and the retry wrapper handles linear-backoff retries on network failures while treating HTTP responses as terminal. Integration delegates to in-process fallback on retry exhaustion.

Changes

Loopback Readiness Gate and Retry Wrapper

Layer / File(s)	Summary
Readiness Gate Infrastructure `src/app/api/providers/[id]/sync-models/route.ts`	Module-level cached promise gates all loopback probes behind a single shared sequence per process. Concurrent callers await the same probe to prevent retry amplification. Includes test reset helper.
Retry Wrapper Contract and Implementation `src/app/api/providers/[id]/sync-models/route.ts`	`SelfFetchWithRetryOptions` type defines injectable fetch, retry/backoff configuration, optional connection label, in-process fallback, and readiness gate bypass. `selfFetchWithRetry` awaits readiness, performs linear-backoff retries on network failures only, treats any HTTP response (2xx/4xx/5xx) as terminal, and falls back to in-process handler when exhausted.
Integration into Model Sync Route `src/app/api/providers/[id]/sync-models/route.ts`	`fetchProviderModelsForSync` refactored to use `selfFetchWithRetry`. Constructs safe loopback origin using `127.0.0.1` with whitelisted port logic, forwards internal headers and incoming cookie on every retry, supplies in-process fallback to `getProviderModels` route handler.
Readiness and Retry Test Coverage `tests/unit/api/sync-models-readiness.test.ts`	Comprehensive test suite validates retry/backoff semantics, loopback readiness gate behavior (concurrent callers share one probe, resolution on any HTTP status, timeout handling), and integration scenarios (gate-enabled concurrent calls share one probe plus one fetch per connection; disabled gate causes amplified retries).
Existing Test Compatibility Update `tests/unit/model-sync-route.test.ts`	Updated test assertions to expect 5 self-fetch retry attempts followed by exactly one upstream models call (total 6 fetch calls), matching new linear-backoff behavior.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

capy

🐰 A readiness gate hops through startup,
Linear retries backoff with each flop—
Concurrent callers won't amplify,
They share one probe that won't stop.
Fallback ensures the flow won't drop! ✨

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/modelsync-self-fetch-readiness-2026-05-13

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

NomenAK · 2026-05-13T04:15:23Z

Follow-up commits (readiness gate + IPv4 fix):

Problem diagnosed: The 5-retry-per-connection contract amplified ProxyFetch undici failures from 17 -> 85. Three follow-up commits address this:

Commit 19498149 — Shared readiness gate (ensureLoopbackServerReady): all 17 boot-time connections await the same one-shot HTTP probe before firing, eliminating parallel retry amplification.

Commit b64d534c — Removed cache:no-store from selfFetchWithRetry (was causing Next.js-patched fetch to behave unexpectedly in the ProxyFetch native-fallback path outside route handler context).

Commit 203fedfd — Root cause fix: the model sync scheduler constructs its self-fetch URL as http://localhost:PORT but localhost resolves to ::1 (IPv6) first in Docker containers. The Next.js server binds to 0.0.0.0 (IPv4 only), so ::1 connections fail with TypeError: fetch failed. Fixed by always using http://127.0.0.1:PORT for the loopback self-fetch URL.

Live validation (post-deploy):

ModelSync self-fetch failed: 0 (was 17/boot)
ProxyFetch undici failures: 1 (just the readiness probe itself hitting undici before native fallback; was 85/boot)
Loopback readiness probe failed: 0
Cycle result: 17/17 synced in 1622ms — all connections succeed via HTTP self-fetch

Tests: 8/8 pass including shared-probe concurrency test and sanity check confirming amplification when gate disabled.

…-process fallback At container boot, 17 ModelSync events fire one-shot HTTP self-fetches against the in-process server before its HTTP listener is fully ready. The single-attempt contract produced a noisy 17-warning burst on every boot and contributed to the 19/h ProxyFetch undici-dispatcher-failed metric. Extracts the self-fetch into selfFetchWithRetry(url, opts) with: - maxRetries=5 with linear backoff (200ms * (attempt+1) by default) - Retry only on network-level failures (ECONNREFUSED, fetch failed) which indicate the loopback listener is not yet accepting connections - HTTP responses (2xx, 4xx, 5xx) returned as-is: server is up, propagate result - In-process route still used as last-resort fallback after all retries exhausted - Pure-function shape with injectable fetch + in-process callback for testing Tests cover: retry-succeeds-on-attempt-3, exhaust-then-fallback, HTTP-errors-returned-immediately. Updated model-sync-route.test.ts fallback test to expect 5 self-fetch retries before in-process fallback. Sanity check confirmed: maxRetries=1 prevents retry loop, causing attempt-count assertions in retry tests to fail as expected.

… amplification The prior commit reduced per-connection self-fetch failures via retry, but the 5-attempt-per-connection retry against a not-yet-ready loopback server amplified the ProxyFetch undici-failure metric from 17/boot to 85/boot (17 connections x 5 attempts each). This commit adds ensureLoopbackServerReady() -- a module-level shared promise that all selfFetchWithRetry callers await before firing. The actual HTTP readiness probe runs exactly once per process; the 17 boot-time connections all share the same promise, so the loopback HTTP server only sees one readiness probe sequence instead of 17 x 5 = 85 attempts. After readiness, each connection makes one HTTP self-fetch -- no retry amplification possible because the server is confirmed up. Readiness endpoint: /api/providers/__readiness_probe__/models (returns 404 once Next.js is dispatching; any HTTP status confirms readiness). Reduced default maxRetries from 5 to 3: the gate handles the boot race; retries are now only for transient failures after the server is confirmed up. Tests (8 total, all pass): - Existing retry tests updated with skipReadinessGate: true for isolation - ensureLoopbackServerReady: 17 callers share ONE probe sequence (<=15 attempts) - ensureLoopbackServerReady: rejects after maxWaitMs timeout - ensureLoopbackServerReady: resolves on 4xx (any HTTP status = server up) - selfFetchWithRetry with gate: 17 callers -> 1 probe sequence + 17 fetches - Sanity check: disabling gate -> >17 fetch attempts confirmed (amplification) Resolves Section F follow-up concerns from review.

…es Next.js fetch override failure ProxyFetch falls back to Next.js-patched originalFetch when undici fails for loopback (127.0.0.1). Next.js-patched fetch fails with TypeError: fetch failed for cache:no-store requests made outside of a Next.js request-context (which is the case for the ProxyFetch native fallback path). Without cache:no-store the request is passed through to Node.js built-in fetch successfully. The readiness probe works (no cache directive) but the model fetches failed (had cache:no-store), explaining the 17 self-fetch warnings despite the gate. Remove cache:no-store from selfFetchWithRetry. The in-process fallback already handles any response caching concerns; the no-store directive was purely defensive and is not needed for loopback internal traffic.

…d localhost->IPv6 resolution failure In Docker containers, "localhost" resolves to both ::1 (IPv6) and 127.0.0.1 (IPv4). Node.js may prefer ::1 for the connection attempt. The Next.js HTTP server binds to 0.0.0.0 (IPv4 only), so ::1 connections are refused with TypeError: fetch failed — even though the server is fully up and ::1 would ordinarily be fine on a host with IPv6 loopback. The readiness probe already hardcodes http://127.0.0.1:20128, so it succeeds. The model fetches used safeOrigin derived from the incoming request URL, which the scheduler sends as http://localhost:20128 — that resolves to ::1 first and fails. This caused all 17 connections to exhaust their 3 retries (51 ProxyFetch warnings) and fall back to in-process despite the readiness gate resolving. Fix: always construct safeOrigin as http://127.0.0.1:PORT, never http://localhost:PORT.

…terns Fixes the 4 fixable alerts opened in the recent scan and adds enforceable guardrails so future development follows the same pattern. Code fixes: - src/mitm/cert/install.ts: pass certPath/certName/action via exec()'s env option instead of string-interpolating them into the bash script (CodeQL js/shell-command-injection-from-environment cheaper-inference#225) - scripts/docs/{gen-provider-reference,add-frontmatter,fix-internal-links}: escape backslash before other regex/markdown metacharacters (CodeQL js/incomplete-sanitization cheaper-inference#227, cheaper-inference#228, cheaper-inference#229) Documentation (mandatory patterns): - docs/security/PUBLIC_CREDS.md — embedding public upstream OAuth/Firebase identifiers via resolvePublicCred(); never as string literals - docs/security/ERROR_SANITIZATION.md — routing every error response through sanitizeErrorMessage()/buildErrorBody(); never raw err.stack/err.message - CLAUDE.md: 4 new Hard Rules (#11-#14) + Security section + scenario notes - AGENTS.md, CONTRIBUTING.md: cross-reference the two new docs - SECURITY.md: extended Hard Security Rules with the new mandatory patterns - docs/README.md: index entries pointing to the two new docs Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…precedent CodeQL js/stack-trace-exposure does not recognize sanitizers reached through a custom helper indirection — flagging callsites like open-sse/utils/error.ts::errorResponse and open-sse/executors/cursor.ts::buildErrorResponse even though both route through sanitizeErrorMessage(). Record the dismissal precedent (alerts cheaper-inference#224 and cheaper-inference#231, May 2026): - Add a "Known CodeQL limitation" section in docs/security/ERROR_SANITIZATION.md documenting how to handle future occurrences (verify callchain → verify test coverage → dismiss with reference, do NOT duplicate the pattern inline). - Extend CLAUDE.md Hard Rule #14 with the precedent so the next engineer doesn't try to "fix" the false positive by weakening the shared sanitizer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…terns Fixes the 4 fixable alerts opened in the recent scan and adds enforceable guardrails so future development follows the same pattern. Code fixes: - src/mitm/cert/install.ts: pass certPath/certName/action via exec()'s env option instead of string-interpolating them into the bash script (CodeQL js/shell-command-injection-from-environment cheaper-inference#225) - scripts/docs/{gen-provider-reference,add-frontmatter,fix-internal-links}: escape backslash before other regex/markdown metacharacters (CodeQL js/incomplete-sanitization cheaper-inference#227, cheaper-inference#228, cheaper-inference#229) Documentation (mandatory patterns): - docs/security/PUBLIC_CREDS.md — embedding public upstream OAuth/Firebase identifiers via resolvePublicCred(); never as string literals - docs/security/ERROR_SANITIZATION.md — routing every error response through sanitizeErrorMessage()/buildErrorBody(); never raw err.stack/err.message - CLAUDE.md: 4 new Hard Rules (#11-#14) + Security section + scenario notes - AGENTS.md, CONTRIBUTING.md: cross-reference the two new docs - SECURITY.md: extended Hard Security Rules with the new mandatory patterns - docs/README.md: index entries pointing to the two new docs Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…precedent CodeQL js/stack-trace-exposure does not recognize sanitizers reached through a custom helper indirection — flagging callsites like open-sse/utils/error.ts::errorResponse and open-sse/executors/cursor.ts::buildErrorResponse even though both route through sanitizeErrorMessage(). Record the dismissal precedent (alerts cheaper-inference#224 and cheaper-inference#231, May 2026): - Add a "Known CodeQL limitation" section in docs/security/ERROR_SANITIZATION.md - Extend CLAUDE.md Hard Rule #14 with the precedent Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…-process fallback (#14) * fix(ModelSync): retry self-fetch with network-error backoff before in-process fallback At container boot, 17 ModelSync events fire one-shot HTTP self-fetches against the in-process server before its HTTP listener is fully ready. The single-attempt contract produced a noisy 17-warning burst on every boot and contributed to the 19/h ProxyFetch undici-dispatcher-failed metric. Extracts the self-fetch into selfFetchWithRetry(url, opts) with: - maxRetries=5 with linear backoff (200ms * (attempt+1) by default) - Retry only on network-level failures (ECONNREFUSED, fetch failed) which indicate the loopback listener is not yet accepting connections - HTTP responses (2xx, 4xx, 5xx) returned as-is: server is up, propagate result - In-process route still used as last-resort fallback after all retries exhausted - Pure-function shape with injectable fetch + in-process callback for testing Tests cover: retry-succeeds-on-attempt-3, exhaust-then-fallback, HTTP-errors-returned-immediately. Updated model-sync-route.test.ts fallback test to expect 5 self-fetch retries before in-process fallback. Sanity check confirmed: maxRetries=1 prevents retry loop, causing attempt-count assertions in retry tests to fail as expected. * fix(ModelSync): add shared loopback readiness gate to eliminate retry amplification The prior commit reduced per-connection self-fetch failures via retry, but the 5-attempt-per-connection retry against a not-yet-ready loopback server amplified the ProxyFetch undici-failure metric from 17/boot to 85/boot (17 connections x 5 attempts each). This commit adds ensureLoopbackServerReady() -- a module-level shared promise that all selfFetchWithRetry callers await before firing. The actual HTTP readiness probe runs exactly once per process; the 17 boot-time connections all share the same promise, so the loopback HTTP server only sees one readiness probe sequence instead of 17 x 5 = 85 attempts. After readiness, each connection makes one HTTP self-fetch -- no retry amplification possible because the server is confirmed up. Readiness endpoint: /api/providers/__readiness_probe__/models (returns 404 once Next.js is dispatching; any HTTP status confirms readiness). Reduced default maxRetries from 5 to 3: the gate handles the boot race; retries are now only for transient failures after the server is confirmed up. Tests (8 total, all pass): - Existing retry tests updated with skipReadinessGate: true for isolation - ensureLoopbackServerReady: 17 callers share ONE probe sequence (<=15 attempts) - ensureLoopbackServerReady: rejects after maxWaitMs timeout - ensureLoopbackServerReady: resolves on 4xx (any HTTP status = server up) - selfFetchWithRetry with gate: 17 callers -> 1 probe sequence + 17 fetches - Sanity check: disabling gate -> >17 fetch attempts confirmed (amplification) Resolves Section F follow-up concerns from review. * fix(ModelSync): drop cache:no-store from loopback self-fetch — bypasses Next.js fetch override failure ProxyFetch falls back to Next.js-patched originalFetch when undici fails for loopback (127.0.0.1). Next.js-patched fetch fails with TypeError: fetch failed for cache:no-store requests made outside of a Next.js request-context (which is the case for the ProxyFetch native fallback path). Without cache:no-store the request is passed through to Node.js built-in fetch successfully. The readiness probe works (no cache directive) but the model fetches failed (had cache:no-store), explaining the 17 self-fetch warnings despite the gate. Remove cache:no-store from selfFetchWithRetry. The in-process fallback already handles any response caching concerns; the no-store directive was purely defensive and is not needed for loopback internal traffic. * fix(ModelSync): force IPv4 (127.0.0.1) for loopback self-fetch — avoid localhost->IPv6 resolution failure In Docker containers, "localhost" resolves to both ::1 (IPv6) and 127.0.0.1 (IPv4). Node.js may prefer ::1 for the connection attempt. The Next.js HTTP server binds to 0.0.0.0 (IPv4 only), so ::1 connections are refused with TypeError: fetch failed — even though the server is fully up and ::1 would ordinarily be fine on a host with IPv6 loopback. The readiness probe already hardcodes http://127.0.0.1:20128, so it succeeds. The model fetches used safeOrigin derived from the incoming request URL, which the scheduler sends as http://localhost:20128 — that resolves to ::1 first and fails. This caused all 17 connections to exhaust their 3 retries (51 ProxyFetch warnings) and fall back to in-process despite the readiness gate resolving. Fix: always construct safeOrigin as http://127.0.0.1:PORT, never http://localhost:PORT. --------- Co-authored-by: OmniRoute Ops <ops@nomenak.dev>

OmniRoute Ops added 4 commits May 13, 2026 08:11

NomenAK force-pushed the fix/modelsync-self-fetch-readiness-2026-05-13 branch from 203fedf to c01a791 Compare May 13, 2026 08:11

NomenAK merged commit 48c35a2 into main May 13, 2026
2 of 6 checks passed

NomenAK deleted the fix/modelsync-self-fetch-readiness-2026-05-13 branch May 13, 2026 08:11

coderabbitai Bot mentioned this pull request May 13, 2026

chore(ModelSync): sync JSDoc default + force IPv4 in scheduler URL #16

Merged

Navigation Menu

Repository navigation

fix(ModelSync): retry self-fetch with network-error backoff before in-process fallback - #14

fix(ModelSync): retry self-fetch with network-error backoff before in-process fallback#14
NomenAK merged 4 commits into
mainfrom
fix/modelsync-self-fetch-readiness-2026-05-13

NomenAK commented May 13, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 13, 2026 •

edited

Loading

Review failed

Walkthrough

Changes

Estimated code review effort

Suggested labels

Uh oh!

NomenAK commented May 13, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

NomenAK commented May 13, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Context

Test plan

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Estimated code review effort

Suggested labels

Uh oh!

NomenAK commented May 13, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

NomenAK commented May 13, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 13, 2026 •

edited

Loading