Skip to content

fix(onboard): retry and fallback for container reachability check #2453

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cv merged 5 commits into from
Apr 29, 2026
Merged

fix(onboard): retry and fallback for container reachability check #2453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7385d6e
fix(onboard): retry and fallback for container reachability check (#2…
jyaunches Apr 24, 2026
06f77ed
Merge remote-tracking branch 'origin/main' into pr/issue-2425
jyaunches Apr 25, 2026
a154279
refactor(onboard): extract ollama proxy into ollama-proxy.ts, add isP…
jyaunches Apr 25, 2026
d89b961
fix(onboard): address CodeRabbit review feedback on proxy health checks
jyaunches Apr 28, 2026
edb6a44
merge: resolve conflicts with main
jyaunches Apr 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 102 additions & 5 deletions src/lib/local-inference.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ describe("local inference helpers", () => {
"--add-host",
"host.openshell.internal:host-gateway",
CONTAINER_REACHABILITY_IMAGE,
"--connect-timeout",
"5",
"--max-time",
"10",
"-sf",
"http://host.openshell.internal:8000/v1/models",
]);
Expand All @@ -92,6 +96,10 @@ describe("local inference helpers", () => {
"--add-host",
"host.openshell.internal:host-gateway",
CONTAINER_REACHABILITY_IMAGE,
"--connect-timeout",
"5",
"--max-time",
"10",
"-sf",
`http://host.openshell.internal:${OLLAMA_CONTAINER_PORT}/api/tags`,
]);
Expand Down Expand Up @@ -149,14 +157,96 @@ describe("local inference helpers", () => {
let callCount = 0;
const mockCapture = () => {
callCount += 1;
return callCount === 1 ? '{"models":[]}' : "";
// Call 1: host check succeeds
if (callCount === 1) return '{"models":[]}';
// Calls 2-4: container check fails (3 retries)
// Calls 5-6: diagnostic commands fail
return "";
};
const result = validateLocalProvider("ollama-local", mockCapture);
const noopSleep = () => {};
const result = validateLocalProvider("ollama-local", mockCapture, noopSleep);
expect(result.ok).toBe(false);
expect(result.message).toMatch(
new RegExp(`host\\.openshell\\.internal:${OLLAMA_CONTAINER_PORT}`),
);
expect(result.message).toMatch(/auth proxy/);
expect(result.message).toMatch(/Docker container reachability check failed/);
expect(result.message).toMatch(/sandbox uses a different network path/);
expect(result.message).not.toMatch(/Ensure the Ollama auth proxy is running/);
expect(result.diagnostic).toMatch(/Docker command failed/);
});

it("succeeds after container check retry", () => {
let callCount = 0;
const mockCapture = () => {
callCount += 1;
// Call 1: host check succeeds
if (callCount === 1) return '{"models":[]}';
// Call 2: container attempt 1 fails
if (callCount === 2) return "";
// Call 3: container attempt 2 succeeds
return '{"models":[]}';
};
const sleepCalls: number[] = [];
const mockSleep = (s: number) => { sleepCalls.push(s); };
const result = validateLocalProvider("ollama-local", mockCapture, mockSleep);
expect(result).toEqual({ ok: true });
expect(sleepCalls).toEqual([2]);
});

it("includes HTTP diagnostic when retries exhausted and diagnostic commands succeed", () => {
let callCount = 0;
const mockCapture = () => {
callCount += 1;
// Call 1: host check succeeds
if (callCount === 1) return '{"models":[]}';
// Calls 2-4: container check fails (3 retries)
if (callCount <= 4) return "";
// Call 5: diagnostic HTTP status
if (callCount === 5) return "502";
// Call 6: diagnostic /etc/hosts
return "172.17.0.1\thost.openshell.internal";
};
const sleepCalls: number[] = [];
const mockSleep = (s: number) => { sleepCalls.push(s); };
const result = validateLocalProvider("ollama-local", mockCapture, mockSleep);
expect(result.ok).toBe(false);
expect(result.diagnostic).toMatch(/HTTP 502/);
expect(result.diagnostic).toMatch(/host-gateway resolved to/);
expect(sleepCalls).toEqual([2, 2]);
});

it("includes docker-failed diagnostic when diagnostic commands also fail", () => {
let callCount = 0;
const mockCapture = () => {
callCount += 1;
if (callCount === 1) return '{"models":[]}';
return "";
};
const noopSleep = () => {};
const result = validateLocalProvider("ollama-local", mockCapture, noopSleep);
expect(result.ok).toBe(false);
expect(result.diagnostic).toMatch(/Docker command failed/);
});

it("calls sleepFn between container check retries", () => {
let callCount = 0;
const mockCapture = () => {
callCount += 1;
if (callCount === 1) return '{"models":[]}';
return "";
};
const sleepCalls: number[] = [];
const mockSleep = (s: number) => { sleepCalls.push(s); };
validateLocalProvider("ollama-local", mockCapture, mockSleep);
expect(sleepCalls).toEqual([2, 2]);
});

it("does not retry when host check fails", () => {
const sleepCalls: number[] = [];
const mockSleep = (s: number) => { sleepCalls.push(s); };
const result = validateLocalProvider("ollama-local", () => "", mockSleep);
expect(result.ok).toBe(false);
expect(sleepCalls).toEqual([]);
});

it("returns a clear error when vllm-local is unavailable", () => {
Expand Down Expand Up @@ -211,11 +301,18 @@ describe("local inference helpers", () => {
let callCount = 0;
const mockCapture = () => {
callCount += 1;
return callCount === 1 ? '{"data":[]}' : "";
// Call 1: host check succeeds
if (callCount === 1) return '{"data":[]}';
// Calls 2+: container check + diagnostics all fail
return "";
};
const result = validateLocalProvider("vllm-local", mockCapture);
const noopSleep = () => {};
const result = validateLocalProvider("vllm-local", mockCapture, noopSleep);
expect(result.ok).toBe(false);
expect(result.message).toMatch(/host\.openshell\.internal:8000/);
expect(result.message).toMatch(/Docker container reachability check failed/);
expect(result.message).toMatch(/sandbox uses a different network path/);
expect(result.message).not.toMatch(/Ensure the server is reachable from containers/);
});

it("treats unknown local providers as already valid", () => {
Expand Down
97 changes: 92 additions & 5 deletions src/lib/local-inference.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { runCurlProbe } from "./http-probe";
const { shellQuote, runCapture } = require("./runner");

import { VLLM_PORT, OLLAMA_PORT, OLLAMA_PROXY_PORT } from "./ports";
import { sleepSeconds } from "./wait";

// eslint-disable-next-line @typescript-eslint/no-require-imports
const { isWsl } = require("./platform");
Expand All @@ -35,6 +36,7 @@ export interface GpuInfo {
export interface ValidationResult {
ok: boolean;
message?: string;
diagnostic?: string;
}

export interface LocalProviderHealthStatus {
Expand Down Expand Up @@ -177,6 +179,10 @@ export function getLocalProviderContainerReachabilityCheck(provider: string): st
"--add-host",
"host.openshell.internal:host-gateway",
CONTAINER_REACHABILITY_IMAGE,
"--connect-timeout",
"5",
"--max-time",
"10",
"-sf",
`http://host.openshell.internal:${VLLM_PORT}/v1/models`,
];
Expand All @@ -190,6 +196,10 @@ export function getLocalProviderContainerReachabilityCheck(provider: string): st
"--add-host",
"host.openshell.internal:host-gateway",
CONTAINER_REACHABILITY_IMAGE,
"--connect-timeout",
"5",
"--max-time",
"10",
"-sf",
`http://host.openshell.internal:${OLLAMA_CONTAINER_PORT}/api/tags`,
];
Expand All @@ -198,9 +208,13 @@ export function getLocalProviderContainerReachabilityCheck(provider: string): st
}
}

const CONTAINER_CHECK_MAX_ATTEMPTS = 3;
const CONTAINER_CHECK_RETRY_DELAY_SECS = 2;

export function validateLocalProvider(
provider: string,
runCaptureImpl?: RunCaptureFn,
sleepFn?: (seconds: number) => void,
): ValidationResult {
if (provider === "ollama-local") {
const portValidation = validateOllamaPortConfiguration();
Expand All @@ -210,6 +224,7 @@ export function validateLocalProvider(
}

const capture = runCaptureImpl ?? runCapture;
const sleep = sleepFn ?? sleepSeconds;
const command = getLocalProviderHealthCheck(provider);
if (!command) {
return { ok: true };
Expand Down Expand Up @@ -238,30 +253,102 @@ export function validateLocalProvider(
return { ok: true };
}

const containerOutput = capture(containerCommand, { ignoreError: true });
if (containerOutput) {
return { ok: true };
// Retry container reachability check with backoff
for (let attempt = 1; attempt <= CONTAINER_CHECK_MAX_ATTEMPTS; attempt++) {
const containerOutput = capture(containerCommand, { ignoreError: true });
if (containerOutput) {
return { ok: true };
}
if (attempt < CONTAINER_CHECK_MAX_ATTEMPTS) {
sleep(CONTAINER_CHECK_RETRY_DELAY_SECS);
}
}

// All retries exhausted — collect diagnostics
const diagnostic = collectContainerDiagnostic(provider, capture);

switch (provider) {
case "vllm-local":
return {
ok: false,
message: `Local vLLM is responding on 127.0.0.1, but containers cannot reach http://host.openshell.internal:${VLLM_PORT}. Ensure the server is reachable from containers, not only from the host shell.`,
message: `Local vLLM is responding on 127.0.0.1, but the Docker container reachability check failed for http://host.openshell.internal:${VLLM_PORT}. This may be a Docker networking issue — the sandbox uses a different network path and may still work.`,
diagnostic,
};
case "ollama-local":
return {
ok: false,
message: `Local Ollama is responding on 127.0.0.1, but containers cannot reach the auth proxy at http://host.openshell.internal:${OLLAMA_CONTAINER_PORT}. Ensure the Ollama auth proxy is running.`,
message: `Local Ollama is responding on 127.0.0.1, but the Docker container reachability check failed for http://host.openshell.internal:${OLLAMA_CONTAINER_PORT}. This may be a Docker networking issue — the sandbox uses a different network path and may still work.`,
diagnostic,
};
default:
return {
ok: false,
message: "The selected local inference provider is unavailable from containers.",
diagnostic,
};
}
}

function getContainerCheckUrl(provider: string): string {
switch (provider) {
case "vllm-local":
return `http://host.openshell.internal:${VLLM_PORT}/v1/models`;
case "ollama-local":
return `http://host.openshell.internal:${OLLAMA_CONTAINER_PORT}/api/tags`;
default:
return "http://host.openshell.internal/";
}
}

function collectContainerDiagnostic(provider: string, capture: RunCaptureFn): string {
const url = getContainerCheckUrl(provider);
try {
// Get HTTP status code
const httpStatus = capture(
[
"docker", "run", "--rm",
"--add-host", "host.openshell.internal:host-gateway",
CONTAINER_REACHABILITY_IMAGE,
"-s", "-o", "/dev/null", "-w", "%{http_code}",
"--connect-timeout", "5", "--max-time", "10",
url,
],
{ ignoreError: true },
);

// Get /etc/hosts to see host-gateway resolution
const hostsOutput = capture(
[
"docker", "run", "--rm",
"--add-host", "host.openshell.internal:host-gateway",
CONTAINER_REACHABILITY_IMAGE,
"cat", "/etc/hosts",
],
{ ignoreError: true },
);

if (!httpStatus && !hostsOutput) {
return `Docker command failed (image pull error or runtime failure). Retried ${CONTAINER_CHECK_MAX_ATTEMPTS} times.`;
}

const parts: string[] = [];
if (httpStatus) {
parts.push(`Container curl returned HTTP ${httpStatus.trim()}`);
}
if (hostsOutput) {
const gwLine = hostsOutput.split(/\r?\n/).find((l: string) => l.includes("host.openshell.internal"));
if (gwLine) {
const ip = gwLine.trim().split(/\s+/)[0];
parts.push(`host-gateway resolved to: ${ip}`);
}
}
parts.push(`Retried ${CONTAINER_CHECK_MAX_ATTEMPTS} times over ~${(CONTAINER_CHECK_MAX_ATTEMPTS - 1) * CONTAINER_CHECK_RETRY_DELAY_SECS}s`);
return parts.join(". ") + ".";
} catch {
return `Docker command failed (image pull error or runtime failure). Retried ${CONTAINER_CHECK_MAX_ATTEMPTS} times.`;
}
}

export function parseOllamaList(output: string | null | undefined): string[] {
return String(output || "")
.split(/\r?\n/)
Expand Down
36 changes: 36 additions & 0 deletions src/lib/onboard-ollama-proxy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,40 @@ function getOllamaProxyToken(): string | null {
return ollamaProxyToken;
}

/**
* Check whether the Ollama auth proxy is actually healthy — not just that
* the PID exists, but that the proxy endpoint responds to HTTP requests.
*
* This is the correct check for the setupInference fallback: if the
* container reachability test fails (Docker bridge issue) but the proxy
* is confirmed healthy on the host, onboarding can safely continue.
*/
function isProxyHealthy(): boolean {
// 1. PID check — informational, but don't early-return on failure.
// The proxy may have been restarted with a new PID that isn't in our
// PID file, so the HTTP probe is the authoritative signal.
const pid = loadPersistedProxyPid();
const hasValidPid = isOllamaProxyProcess(pid);

// 2. HTTP probe — confirm the proxy actually responds. This is the
// authoritative check: a successful probe wins even if the PID file
// is missing or stale (e.g., after a manual restart).
const proxyUrl = `http://127.0.0.1:${OLLAMA_PROXY_PORT}/api/tags`;
const token = loadPersistedProxyToken();
const probeCmd = token
? ["curl", "-sf", "--connect-timeout", "3", "--max-time", "5",
"-H", `Authorization: Bearer ${token}`, proxyUrl]
: ["curl", "-sf", "--connect-timeout", "3", "--max-time", "5", proxyUrl];

const output = runCapture(probeCmd, { ignoreError: true });
if (output) return true;

// HTTP probe failed — fall back to PID as a weaker signal.
// This covers edge cases where the probe transiently fails but the
// process is confirmed alive.
return hasValidPid;
Comment on lines +256 to +268

@coderabbitai coderabbitai Bot Apr 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Use in-memory token for health probe to avoid misclassifying proxy health

Line 256 uses loadPersistedProxyToken(), which can be null during onboarding even when a valid in-memory token exists. That makes the probe unauthenticated and can incorrectly fall back to PID-only health (or fail when PID is stale).

💡 Proposed fix 
-  const token = loadPersistedProxyToken();
+  const token = getOllamaProxyToken();
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/lib/onboard-ollama-proxy.ts` around lines 256 - 268, The health probe
uses loadPersistedProxyToken() which can be null during onboarding even though a
valid in-memory token exists; update the probe logic to prefer the in-memory
proxy token (e.g., the runtime variable that holds the current token) before
falling back to loadPersistedProxyToken(), then build probeCmd with that chosen
token (still falling back to no-Auth when none exists) so the Authorization
header uses the in-memory token when available; keep references to probeCmd,
proxyUrl, runCapture, and hasValidPid so the authenticated HTTP probe is
attempted first and only then fallback to PID.

}

async function promptOllamaModel(gpu = null) {
const installed = getOllamaModelOptions();
const options = installed.length > 0 ? installed : getBootstrapOllamaModelOptions(gpu);
Expand Down Expand Up @@ -308,6 +342,8 @@ function prepareOllamaModel(model, installedModels = []) {
module.exports = {
ensureOllamaAuthProxy,
getOllamaProxyToken,
isProxyHealthy,
killStaleProxy,
persistProxyToken,
startOllamaAuthProxy,
promptOllamaModel,
Expand Down
Loading
Loading