Skip to content

Commit d1c2f65

Browse files
committed
refactor(onboard): one router readiness predicate for both acceptance paths
Review follow-up. The startup poll and the final health snapshot each composed the readiness rule at their own call site, which is the shape that let the two paths carry different definitions of readiness in the first place. Fold the complete rule -- /health answered 2xx and names at least one healthy endpoint -- into the existing local predicate, which now takes the snapshot itself, and have both acceptance paths call it. Retry, timeout, and diagnostic behavior are unchanged, and production line count is neutral. Also correct the stale comment on the isRouterHealthy stub in the zero-healthy-endpoints case: the startup poll no longer calls isRouterHealthy at all, so the stub's job is to trip a regression back to the old boolean poll. Signed-off-by: Udaya Tejas <udayatejas2004@gmail.com>
1 parent c71cc6b commit d1c2f65

2 files changed

Lines changed: 9 additions & 8 deletions

File tree

src/lib/onboard/model-router.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ export async function startModelRouter(
469469
);
470470
healthAttempts += 1;
471471
const pollSnapshot = await deps.getRouterHealthSnapshot(port, healthTimeoutMs);
472-
const healthy = pollSnapshot.healthy && hasHealthyEndpoint(pollSnapshot.body);
472+
const healthy = isRouterSnapshotReady(pollSnapshot);
473473
const processAlive = deps.isProcessAlive(pid);
474474
if (healthy && processAlive) return pid;
475475
if (!processAlive) {
@@ -485,7 +485,7 @@ export async function startModelRouter(
485485
const finalSnapshot: RouterHealthSnapshot = childExited
486486
? { healthy: false, body: null }
487487
: await deps.getRouterHealthSnapshot(port, ROUTER_FINAL_HEALTH_SNAPSHOT_TIMEOUT_MS);
488-
if (finalSnapshot.healthy && hasHealthyEndpoint(finalSnapshot.body) && deps.isProcessAlive(pid)) {
488+
if (isRouterSnapshotReady(finalSnapshot) && deps.isProcessAlive(pid)) {
489489
return pid;
490490
}
491491
try {
@@ -504,11 +504,11 @@ export async function startModelRouter(
504504
);
505505
}
506506

507-
/** True when the parsed /health body names at least one healthy endpoint. */
508-
function hasHealthyEndpoint(body: string | null): boolean {
509-
if (!body) return false;
507+
/** Router readiness: /health answered 2xx and names at least one healthy endpoint. */
508+
function isRouterSnapshotReady(snapshot: RouterHealthSnapshot): boolean {
509+
if (!snapshot.healthy || !snapshot.body) return false;
510510
try {
511-
const parsed = JSON.parse(body) as { healthy_endpoints?: readonly unknown[] };
511+
const parsed = JSON.parse(snapshot.body) as { healthy_endpoints?: readonly unknown[] };
512512
return Array.isArray(parsed?.healthy_endpoints) && parsed.healthy_endpoints.length > 0;
513513
} catch {
514514
return false;

test/onboard-model-router.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -749,8 +749,9 @@ describe("onboard Model Router setup", () => {
749749
}),
750750
resolveProviderCredential: () => null,
751751
buildSubprocessEnv: () => ({}),
752-
// The pre-spawn port guard calls isRouterHealthy without a timeout;
753-
// only the startup poll passes one. Answer 2xx for the poll alone.
752+
// The pre-spawn port guard calls isRouterHealthy without a timeout.
753+
// Return true for timeout-bearing calls so a regression to the old
754+
// boolean startup poll cannot accept zero healthy endpoints.
754755
isRouterHealthy: async (_port: number, timeoutMs) => timeoutMs !== undefined,
755756
getRouterHealthSnapshot: async () => ({ healthy: true, body: allUnhealthyBody }),
756757
sleep: async () => undefined,

0 commit comments

Comments
 (0)