Skip to content

Commit c71cc6b

Browse files
committed
fix(onboard): require a healthy endpoint before the router startup poll succeeds
The Model Router startup poll accepted any 2xx /health, while the final snapshot taken after the poll required the body to name at least one healthy endpoint. When the routed credential is rejected, every upstream fails fast, /health answers 200 with an empty healthy_endpoints list well inside the 3-second liveness budget, and onboarding reported the router started before every sandbox request returned 401. Read the body in the poll as well, so both acceptance paths apply the rule the file already documents. The poll keeps its 3-second request budget and its full retry window, so a router that needs longer to bring endpoints up is still accepted as soon as /health names one. Signed-off-by: Udaya Tejas <udayatejas2004@gmail.com>
1 parent 7d516f2 commit c71cc6b

3 files changed

Lines changed: 49 additions & 56 deletions

File tree

src/lib/onboard/model-router-process.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,8 @@ const ROUTER_HEALTH_BODY_MAX_BYTES = 64 * 1024;
4141

4242
/**
4343
* Fetch /health and keep the response body for diagnosis (#8962). Unlike
44-
* `isRouterHealthy`, this waits for the body, so callers pass a longer
45-
* timeout; `startModelRouter` uses 30 seconds. LiteLLM's /health probes
44+
* `isRouterHealthy`, this waits for the body, so a caller that must read it
45+
* budgets for it; the final startup snapshot uses 30 seconds. LiteLLM's /health probes
4646
* every upstream endpoint per request and can answer well after the
4747
* 3-second liveness budget. The timeout is a wall-clock deadline, not a
4848
* socket idle timeout, so a responder that trickles bytes cannot hold the

src/lib/onboard/model-router.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ const ROUTER_HEALTH_INTERVAL_MS = 2000;
6161
const ROUTER_STARTUP_TIMEOUT_MS = 10 * 60_000;
6262
// LiteLLM's /health live-probes every upstream endpoint per request, so it
6363
// can need far longer than the 3-second liveness budget to answer (#8962).
64-
// The startup poll keeps the 3-second budget: the status-only liveness
65-
// probe must not accept a fast 200 that names zero healthy endpoints, so
66-
// recovery for a slow-but-healthy router runs through the body-checked
64+
// The startup poll keeps the 3-second budget and reads the body, so it
65+
// never accepts a fast 200 that names zero healthy endpoints; recovery for
66+
// a router whose /health outruns that budget runs through the body-checked
6767
// final snapshot after the poll exhausts its retries.
6868
const ROUTER_FINAL_HEALTH_SNAPSHOT_TIMEOUT_MS = 30_000;
6969
const ROUTER_LOG_TAIL_LINES = 20;
@@ -468,7 +468,8 @@ export async function startModelRouter(
468468
Math.min(ROUTER_HEALTH_REQUEST_TIMEOUT_MS, Math.ceil(remainingMs)),
469469
);
470470
healthAttempts += 1;
471-
const healthy = await deps.isRouterHealthy(port, healthTimeoutMs);
471+
const pollSnapshot = await deps.getRouterHealthSnapshot(port, healthTimeoutMs);
472+
const healthy = pollSnapshot.healthy && hasHealthyEndpoint(pollSnapshot.body);
472473
const processAlive = deps.isProcessAlive(pid);
473474
if (healthy && processAlive) return pid;
474475
if (!processAlive) {

test/onboard-model-router.test.ts

Lines changed: 42 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ const MODEL_ROUTER_FINGERPRINT_FILE = ".nemoclaw-source-fingerprint";
4343
const MODEL_ROUTER_TEST_SOURCE_SHA = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
4444
const MODEL_ROUTER_TEST_VERSION = "0.1.0";
4545
const NVIDIA_TEST_CREDENTIAL = "nvapi-TEST-NOT-A-REAL-ROUTER-KEY";
46+
const ROUTER_HEALTHY_BODY = JSON.stringify({
47+
healthy_endpoints: [{ api_base: "https://integrate.api.nvidia.com/v1" }],
48+
unhealthy_endpoints: [],
49+
});
4650

4751
type PrepareCall = {
4852
venvDir: string;
@@ -295,7 +299,6 @@ describe("onboard Model Router setup", () => {
295299
const port = 45_678;
296300
const healthChecks: number[] = [];
297301
const sleepCalls: number[] = [];
298-
let healthProbe = 0;
299302
let pid: number | null = null;
300303

301304
const blueprintDir = path.join(rootDir, "nemoclaw-blueprint");
@@ -350,8 +353,11 @@ describe("onboard Model Router setup", () => {
350353
name === "ROUTER_API_KEY" ? "router-secret" : null,
351354
isRouterHealthy: async (routerPort) => {
352355
healthChecks.push(routerPort);
353-
healthProbe += 1;
354-
return healthProbe > 1;
356+
return false;
357+
},
358+
getRouterHealthSnapshot: async (routerPort) => {
359+
healthChecks.push(routerPort);
360+
return { healthy: true, body: ROUTER_HEALTHY_BODY };
355361
},
356362
sleep: async (milliseconds) => {
357363
sleepCalls.push(milliseconds);
@@ -406,7 +412,6 @@ describe("onboard Model Router setup", () => {
406412
const homeDir = path.join(tmpDir, "home");
407413
const routerCommand = path.join(tmpDir, "managed", "model-router");
408414
const port = 45_692;
409-
let healthProbe = 0;
410415
let pid: number | null = null;
411416

412417
fs.mkdirSync(path.join(rootDir, "nemoclaw-blueprint", "router"), { recursive: true });
@@ -433,10 +438,8 @@ describe("onboard Model Router setup", () => {
433438
homeDir,
434439
ensureModelRouterCommand: () => routerCommand,
435440
resolveProviderCredential: () => null,
436-
isRouterHealthy: async () => {
437-
healthProbe += 1;
438-
return healthProbe > 1;
439-
},
441+
isRouterHealthy: async () => false,
442+
getRouterHealthSnapshot: async () => ({ healthy: true, body: ROUTER_HEALTHY_BODY }),
440443
sleep: async () => undefined,
441444
},
442445
);
@@ -475,9 +478,10 @@ describe("onboard Model Router setup", () => {
475478
}),
476479
resolveProviderCredential: () => null,
477480
buildSubprocessEnv: () => ({}),
478-
isRouterHealthy: async () => {
481+
isRouterHealthy: async () => false,
482+
getRouterHealthSnapshot: async () => {
479483
healthProbe += 1;
480-
return healthProbe > 61;
484+
return { healthy: healthProbe > 60, body: ROUTER_HEALTHY_BODY };
481485
},
482486
sleep,
483487
isProcessAlive: () => true,
@@ -487,7 +491,7 @@ describe("onboard Model Router setup", () => {
487491
);
488492

489493
assert.equal(startedPid, pid);
490-
assert.equal(healthProbe, 62);
494+
assert.equal(healthProbe, 61);
491495
assert.equal(sleep.mock.calls.length, 61);
492496
assert.equal(terminateProcess.mock.calls.length, 0);
493497
});
@@ -496,7 +500,7 @@ describe("onboard Model Router setup", () => {
496500
const pid = 12_345;
497501
const sleep = vi.fn(async () => undefined);
498502
const terminateProcess = vi.fn();
499-
const isRouterHealthy = vi.fn(async () => false);
503+
const getRouterHealthSnapshot = vi.fn(async () => ({ healthy: false, body: null }));
500504

501505
await assert.rejects(
502506
startModelRouter(
@@ -515,8 +519,8 @@ describe("onboard Model Router setup", () => {
515519
}),
516520
resolveProviderCredential: () => null,
517521
buildSubprocessEnv: () => ({}),
518-
isRouterHealthy,
519-
getRouterHealthSnapshot: async () => ({ healthy: false, body: null }),
522+
isRouterHealthy: async () => false,
523+
getRouterHealthSnapshot,
520524
sleep,
521525
isProcessAlive: () => true,
522526
terminateProcess,
@@ -526,15 +530,14 @@ describe("onboard Model Router setup", () => {
526530
/failed to become healthy on port 45680 within 600 seconds \(completed health checks: 300\)/,
527531
);
528532

529-
assert.equal(isRouterHealthy.mock.calls.length, 301);
533+
assert.equal(getRouterHealthSnapshot.mock.calls.length, 301);
530534
assert.equal(sleep.mock.calls.length, 300);
531535
assert.deepEqual(terminateProcess.mock.calls, [[pid]]);
532536
});
533537

534538
it("sets OPENAI_API_KEY to the routed credential when an ambient OPENAI_API_KEY exists (#8962)", async () => {
535539
const pid = 12_345;
536540
let spawnedEnv: Record<string, string> | null = null;
537-
let healthProbe = 0;
538541

539542
await startModelRouter(
540543
{ port: 45_690, pool_config_path: "router/test-pool.yaml", credential_env: "ROUTER_API_KEY" },
@@ -558,10 +561,8 @@ describe("onboard Model Router setup", () => {
558561
resolveProviderCredential: (name) =>
559562
({ ROUTER_API_KEY: "router-secret", OPENAI_API_KEY: "stale-openai" })[name] ?? null,
560563
buildSubprocessEnv: (extra) => ({ ...extra }),
561-
isRouterHealthy: async () => {
562-
healthProbe += 1;
563-
return healthProbe > 1;
564-
},
564+
isRouterHealthy: async () => false,
565+
getRouterHealthSnapshot: async () => ({ healthy: true, body: ROUTER_HEALTHY_BODY }),
565566
sleep: async () => undefined,
566567
isProcessAlive: () => true,
567568
terminateProcess: () => undefined,
@@ -643,10 +644,6 @@ describe("onboard Model Router setup", () => {
643644
it("returns the router PID when the final health snapshot proves recovery (#8962)", async () => {
644645
const pid = 12_345;
645646
const terminateProcess = vi.fn();
646-
const healthyBody = JSON.stringify({
647-
healthy_endpoints: [{ api_base: "https://integrate.api.nvidia.com/v1" }],
648-
unhealthy_endpoints: [],
649-
});
650647

651648
const startedPid = await startModelRouter(
652649
{ port: 45_693, pool_config_path: "router/test-pool.yaml" },
@@ -665,7 +662,12 @@ describe("onboard Model Router setup", () => {
665662
resolveProviderCredential: () => null,
666663
buildSubprocessEnv: () => ({}),
667664
isRouterHealthy: async () => false,
668-
getRouterHealthSnapshot: async () => ({ healthy: true, body: healthyBody }),
665+
// /health outruns the poll's 3-second budget and answers only within
666+
// the 30-second final-snapshot budget.
667+
getRouterHealthSnapshot: async (_port: number, timeoutMs = 0) => ({
668+
healthy: timeoutMs >= 30_000,
669+
body: timeoutMs >= 30_000 ? ROUTER_HEALTHY_BODY : null,
670+
}),
669671
sleep: async () => undefined,
670672
isProcessAlive: () => true,
671673
terminateProcess,
@@ -722,7 +724,7 @@ describe("onboard Model Router setup", () => {
722724
);
723725
});
724726

725-
it("still fails when the final snapshot is 2xx with zero healthy endpoints (#8962)", async () => {
727+
it("still fails when the poll and final snapshot are 2xx with zero healthy endpoints (#8962)", async () => {
726728
const pid = 12_345;
727729
const terminateProcess = vi.fn();
728730
const allUnhealthyBody = JSON.stringify({
@@ -747,7 +749,9 @@ describe("onboard Model Router setup", () => {
747749
}),
748750
resolveProviderCredential: () => null,
749751
buildSubprocessEnv: () => ({}),
750-
isRouterHealthy: async () => false,
752+
// The pre-spawn port guard calls isRouterHealthy without a timeout;
753+
// only the startup poll passes one. Answer 2xx for the poll alone.
754+
isRouterHealthy: async (_port: number, timeoutMs) => timeoutMs !== undefined,
751755
getRouterHealthSnapshot: async () => ({ healthy: true, body: allUnhealthyBody }),
752756
sleep: async () => undefined,
753757
isProcessAlive: () => true,
@@ -827,7 +831,6 @@ describe("onboard Model Router setup", () => {
827831
const pid = 12_345;
828832
let spawnedEnv: Record<string, string> | null = null;
829833

830-
let healthProbe = 0;
831834
await startModelRouter(
832835
{
833836
port: 45_695,
@@ -853,10 +856,8 @@ describe("onboard Model Router setup", () => {
853856
resolveProviderCredential: (name) =>
854857
({ ROUTER_API_KEY: "router-secret", OPENAI_API_KEY: "operator-openai" })[name] ?? null,
855858
buildSubprocessEnv: (extra) => ({ ...extra }),
856-
isRouterHealthy: async () => {
857-
healthProbe += 1;
858-
return healthProbe > 1;
859-
},
859+
isRouterHealthy: async () => false,
860+
getRouterHealthSnapshot: async () => ({ healthy: true, body: ROUTER_HEALTHY_BODY }),
860861
sleep: async () => undefined,
861862
isProcessAlive: () => true,
862863
terminateProcess: () => undefined,
@@ -874,7 +875,6 @@ describe("onboard Model Router setup", () => {
874875
it("preserves routed credential fallback for an unproven pool (#8962)", async () => {
875876
const pid = 12_345;
876877
let spawnedEnv: Record<string, string> | null = null;
877-
let healthProbe = 0;
878878

879879
await startModelRouter(
880880
{
@@ -900,10 +900,8 @@ describe("onboard Model Router setup", () => {
900900
readPoolConfig: () => 'models:\n - litellm_model: "openai/gpt-test"\n',
901901
resolveProviderCredential: (name) => (name === "ROUTER_API_KEY" ? "router-secret" : null),
902902
buildSubprocessEnv: (extra) => ({ ...extra }),
903-
isRouterHealthy: async () => {
904-
healthProbe += 1;
905-
return healthProbe > 1;
906-
},
903+
isRouterHealthy: async () => false,
904+
getRouterHealthSnapshot: async () => ({ healthy: true, body: ROUTER_HEALTHY_BODY }),
907905
sleep: async () => undefined,
908906
isProcessAlive: () => true,
909907
terminateProcess: () => undefined,
@@ -924,9 +922,9 @@ describe("onboard Model Router setup", () => {
924922
const sleep = vi.fn(async (milliseconds: number) => {
925923
nowMs += milliseconds;
926924
});
927-
const isRouterHealthy = vi.fn(async (_port: number, timeoutMs = 0) => {
925+
const getRouterHealthSnapshot = vi.fn(async (_port: number, timeoutMs = 0) => {
928926
nowMs += timeoutMs;
929-
return false;
927+
return { healthy: false, body: null };
930928
});
931929

932930
await assert.rejects(
@@ -946,11 +944,8 @@ describe("onboard Model Router setup", () => {
946944
}),
947945
resolveProviderCredential: () => null,
948946
buildSubprocessEnv: () => ({}),
949-
isRouterHealthy,
950-
getRouterHealthSnapshot: async (_port: number, timeoutMs = 0) => {
951-
nowMs += timeoutMs;
952-
return { healthy: false, body: null };
953-
},
947+
isRouterHealthy: async () => false,
948+
getRouterHealthSnapshot,
954949
sleep,
955950
now: () => nowMs,
956951
isProcessAlive: () => true,
@@ -965,7 +960,7 @@ describe("onboard Model Router setup", () => {
965960
);
966961

967962
assert.equal(nowMs, 600_000);
968-
assert.equal(isRouterHealthy.mock.calls.length, 115);
963+
assert.equal(getRouterHealthSnapshot.mock.calls.length, 115);
969964
assert.equal(sleep.mock.calls.length, 114);
970965
assert.deepEqual(terminateProcess.mock.calls, [[pid]]);
971966
});
@@ -980,7 +975,6 @@ describe("onboard Model Router setup", () => {
980975
const mkdirSync = vi.fn();
981976
const proxyConfigArgs: string[][] = [];
982977
const proxyArgs: string[][] = [];
983-
let healthProbe = 0;
984978
vi.stubEnv("HOME", homeDir);
985979
vi.stubEnv("NEMOCLAW_GATEWAY_PORT", "9123");
986980
vi.resetModules();
@@ -1008,10 +1002,8 @@ describe("onboard Model Router setup", () => {
10081002
},
10091003
resolveProviderCredential: () => null,
10101004
buildSubprocessEnv: () => ({}),
1011-
isRouterHealthy: async () => {
1012-
healthProbe += 1;
1013-
return healthProbe > 1;
1014-
},
1005+
isRouterHealthy: async () => false,
1006+
getRouterHealthSnapshot: async () => ({ healthy: true, body: ROUTER_HEALTHY_BODY }),
10151007
sleep: async () => undefined,
10161008
isProcessAlive: () => true,
10171009
terminateProcess: () => undefined,

0 commit comments

Comments
 (0)