-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix(cli): harden Ollama installer download #9703
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,6 +36,9 @@ const { | |
| setResolvedOllamaHost, | ||
| }: typeof import("../inference/local") = require("../inference/local"); | ||
|
|
||
| const OFFICIAL_OLLAMA_INSTALLER_URL = "https://ollama.com/install.sh"; | ||
| const OFFICIAL_OLLAMA_INSTALLER_PROCESS_TIMEOUT_MS = 190_000; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
ast-grep outline src/lib/onboard/install-ollama-linux.ts --items all --type function
python3 - <<'PY'
import re
from pathlib import Path
text = Path("src/lib/onboard/install-ollama-linux.ts").read_text()
process_timeout = int(
re.search(r"OFFICIAL_OLLAMA_INSTALLER_PROCESS_TIMEOUT_MS\s*=\s*([\d_]+)", text).group(1).replace("_", "")
)
max_time = int(re.search(r'"--max-time",\s*"(\d+)"', text).group(1))
retry_max_time = int(re.search(r'"--retry-max-time",\s*"(\d+)"', text).group(1))
required = (max_time + retry_max_time) * 1000
print(f"process timeout: {process_timeout}ms")
print(f"maximum curl wall time: at least {required}ms before scheduling margin")
if process_timeout < required:
raise SystemExit("FAIL: process timeout can interrupt an already-started final retry")
PYRepository: NVIDIA/NemoClaw Length of output: 1311 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- installer implementation ---'
sed -n '1,190p' src/lib/onboard/install-ollama-linux.ts
printf '%s\n' '--- runCaptureEx definitions and call sites ---'
rg -n -A12 -B8 'runCaptureEx|OFFICIAL_OLLAMA_INSTALLER_PROCESS_TIMEOUT_MS' src
printf '%s\n' '--- relevant tests ---'
rg -n -A15 -B8 'installOllama|retry-max-time|max-time|installer.*timeout|190_000' --glob '*.{ts,tsx}' .Repository: NVIDIA/NemoClaw Length of output: 50372 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- installer implementation ---'
sed -n '1,190p' src/lib/onboard/install-ollama-linux.ts
printf '%s\n' '--- execution helper references in src/lib ---'
rg -n -A15 -B8 'runCaptureEx' src/lib
printf '%s\n' '--- installer test setup and download tests ---'
sed -n '1,150p' src/lib/onboard/install-ollama-linux.test.ts
sed -n '380,530p' src/lib/onboard/install-ollama-linux.test.tsRepository: NVIDIA/NemoClaw Length of output: 50372 🌐 Web query:
💡 Result: In curl, the --retry-max-time option acts as a global timer for the entire retry process, while --max-time (or -m) acts as a local timer for each individual transfer attempt [1][2][3]. The semantics for these options are as follows: 1. --retry-max-time: This timer is initialized before the first transfer attempt [2][3]. Curl continues to attempt retries (subject to your --retry limit) as long as this timer has not exceeded the specified duration [2][3]. Crucially, this timer includes the entire duration of all transfer attempts and the sleep time (backoff) between retries [4]. If the timer has not reached the limit when curl prepares for a new retry, it will start another attempt, even if that attempt is likely to exceed the limit once it begins [2][3]. Once a transfer has started, it is allowed to complete even if it eventually runs past the --retry-max-time limit [2][3]. 2. --max-time: This option specifies the maximum time allowed for each individual transfer attempt [2][5]. If a single attempt exceeds this time, curl will terminate that specific attempt and (if configured) trigger a retry [1][3]. To summarize, if you want to strictly limit the total time of your operation including retries, you use --retry-max-time [1][2]. If you want to ensure that no single attempt hangs indefinitely, you use --max-time [2][5]. Setting --retry-max-time to zero disables the retry timeout [2][6]. Citations:
🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- runCaptureEx timeout behavior ---'
sed -n '336,410p' src/lib/runner.ts
printf '%s\n' '--- installer tests that inspect the download invocation ---'
sed -n '490,560p' src/lib/onboard/install-ollama-linux.test.ts
printf '%s\n' '--- all timeout assertions in the installer test ---'
rg -n -C3 'timeout|190_000|runCaptureExImpl|retry-max-time|max-time' src/lib/onboard/install-ollama-linux.test.tsRepository: NVIDIA/NemoClaw Length of output: 13661 Increase
🤖 Prompt for AI Agents |
||
|
|
||
| /** | ||
| * Install location modes. | ||
| * | ||
|
|
@@ -124,6 +127,8 @@ function detectJetpackVariant(opts: InstallOllamaLinuxOptions): "jetpack5" | "je | |
| */ | ||
| function runOfficialInstallScript(opts: InstallOllamaLinuxOptions): void { | ||
| const log = opts.log ?? ((m: string) => console.log(m)); | ||
| const errorLog = opts.errorLog ?? ((m: string) => console.error(m)); | ||
| const runCaptureExImpl = opts.runCaptureExImpl ?? runCaptureEx; | ||
| const runShellImpl = opts.runShellImpl ?? runShell; | ||
| ensureOllamaLinuxExtractionDependencies(opts); | ||
| log( | ||
|
|
@@ -134,9 +139,68 @@ function runOfficialInstallScript(opts: InstallOllamaLinuxOptions): void { | |
| if (versionPin) { | ||
| log(` Requesting Ollama ${MIN_OLLAMA_VERSION} from the installer.`); | ||
| } | ||
| runShellImpl(`set -o pipefail; curl -fsSL https://ollama.com/install.sh | ${versionPin}sh`, { | ||
| stdio: "inherit", | ||
| }); | ||
|
|
||
| const installerDirectory = fs.mkdtempSync( | ||
| nodePath.join(os.tmpdir(), "nemoclaw-ollama-installer-"), | ||
| ); | ||
| const installerPath = nodePath.join(installerDirectory, "install.sh"); | ||
|
|
||
| let failure: { exitCode: number; message: string } | null = null; | ||
| try { | ||
| fs.writeFileSync(installerPath, "", { flag: "wx", mode: 0o600 }); | ||
| const fetchResult = runCaptureExImpl( | ||
| [ | ||
| "curl", | ||
| "--fail", | ||
| "--show-error", | ||
| "--silent", | ||
| "--location", | ||
| "--proto", | ||
| "=https", | ||
| "--proto-redir", | ||
| "=https", | ||
| "--connect-timeout", | ||
| "10", | ||
| "--max-time", | ||
| "120", | ||
| "--retry", | ||
| "3", | ||
| "--retry-all-errors", | ||
| "--retry-delay", | ||
| "1", | ||
| "--retry-max-time", | ||
| "180", | ||
| "--output", | ||
| installerPath, | ||
| OFFICIAL_OLLAMA_INSTALLER_URL, | ||
| ], | ||
| { timeout: OFFICIAL_OLLAMA_INSTALLER_PROCESS_TIMEOUT_MS }, | ||
| ); | ||
| if (fetchResult.exitCode !== 0) { | ||
| failure = { | ||
| exitCode: fetchResult.exitCode ?? 1, | ||
| message: ` Ollama installer download failed after bounded retries (exit ${fetchResult.exitCode ?? "unknown"}).`, | ||
| }; | ||
| } else { | ||
| const installResult = runShellImpl(`${versionPin}sh ${shellQuote(installerPath)}`, { | ||
| ignoreError: true, | ||
| stdio: "inherit", | ||
| }); | ||
| if (installResult.error || installResult.status !== 0) { | ||
| failure = { | ||
| exitCode: installResult.status ?? 1, | ||
| message: ` Ollama installer failed (exit ${installResult.status ?? "unknown"}).`, | ||
| }; | ||
| } | ||
| } | ||
| } finally { | ||
| fs.rmSync(installerDirectory, { force: true, recursive: true }); | ||
| } | ||
|
|
||
| if (failure) { | ||
| errorLog(failure.message); | ||
| process.exit(failure.exitCode); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -176,9 +176,11 @@ describe("onboard Ollama upgrade version floor", () => { | |
| handleInstallOllamaSelection(null, "qwen3:8b", null, makeSelectionState(), menu), | ||
| /Unexpected process\.exit\(1\)/, | ||
| ); | ||
| const installer = commands.find((command) => command.includes("ollama.com/install.sh")); | ||
| const installer = commands.find((command) => command.includes("OLLAMA_VERSION=")); | ||
| assert.ok(installer); | ||
| assert.ok(installer.includes(`OLLAMA_VERSION=${MIN_OLLAMA_VERSION}`)); | ||
| assert.ok(!installer.includes("curl")); | ||
| assert.ok(!installer.includes("|")); | ||
|
Comment on lines
+179
to
+183
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Bind the assertion to the local installer command. The current selectors can match an unrelated shell command containing 📍 Affects 2 files
🤖 Prompt for AI AgentsSource: Path instructions |
||
| const surfaced = errors.join("\n"); | ||
| assert.ok(surfaced.includes(`did not deliver ${MIN_OLLAMA_VERSION} on this host`)); | ||
| assert.ok(!surfaced.includes("systemctl restart ollama")); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.