From a3074d998fcf56e78c3cba5bb66dc081dce5f5cc Mon Sep 17 00:00:00 2001 From: Dongni Yang Date: Wed, 13 May 2026 15:39:05 +0800 Subject: [PATCH 1/7] fix(onboard): pin openshell fetch to blueprint max_openshell_version (#3404) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reactive blueprint bump in main only kicks the can down the road — the installer would re-trip the compatibility gate on the next OpenShell release bump. This makes the installer self-correct by querying published OpenShell releases and pinning the fetch to the highest release ≤ the blueprint's max_openshell_version. If no compatible release exists, the install fails with a clear error naming both the published latest and the configured max. - Adds a pure resolver (resolveOpenshellInstallVersion) plus boundary-safe tag parser (parseOpenshellReleaseTag) to onboard/openshell-install.ts. - installOpenshell() in onboard.ts lists release tags via gh/curl and passes the resolved version through NEMOCLAW_OPENSHELL_PIN_VERSION. - install-openshell.sh honours that env (validated against ^X.Y.Z$) and falls back to the hardcoded pin only when the resolver was unable to reach GitHub. - Unit tests cover the four required cases: latest > max → highest ≤ max; latest ≤ max → latest unchanged; no release ≤ max → clear error; max missing → no-max fallback. Plus malformed-input boundary cases and the QA repro (latest=0.0.38, max=0.0.36 → 0.0.36). Signed-off-by: Dongni Yang Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/install-openshell.sh | 12 ++- src/lib/onboard.ts | 101 ++++++++++++++++++++++++- src/lib/onboard/openshell-install.ts | 71 +++++++++++++++++ test/onboard.test.ts | 109 +++++++++++++++++++++++++++ 4 files changed, 291 insertions(+), 2 deletions(-) diff --git a/scripts/install-openshell.sh b/scripts/install-openshell.sh index 4e3cd2ac627..c0fcdd3d68c 100755 --- a/scripts/install-openshell.sh +++ b/scripts/install-openshell.sh @@ -39,8 +39,18 @@ MIN_VERSION="0.0.39" # Maximum version validated for this NemoClaw release. Newer OpenShell builds # may change sandbox semantics; upgrade NemoClaw before upgrading past this. MAX_VERSION="0.0.39" -# Pin fresh installs to this version instead of pulling "latest". +# Pin fresh installs to this version. The TS installer normally overrides this +# via NEMOCLAW_OPENSHELL_PIN_VERSION after resolving the highest published +# OpenShell release that satisfies the blueprint's max_openshell_version +# (see #3404). The hardcoded value is the fallback for offline runs. PIN_VERSION="$MAX_VERSION" +if [ -n "${NEMOCLAW_OPENSHELL_PIN_VERSION:-}" ]; then + if [[ "$NEMOCLAW_OPENSHELL_PIN_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + PIN_VERSION="$NEMOCLAW_OPENSHELL_PIN_VERSION" + else + fail "NEMOCLAW_OPENSHELL_PIN_VERSION='$NEMOCLAW_OPENSHELL_PIN_VERSION' is not a valid X.Y.Z version." + fi +fi DEV_MIN_VERSION="0.0.39" CHANNEL="${NEMOCLAW_OPENSHELL_CHANNEL:-auto}" diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 222dfbfb4cb..b5391e0a0eb 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -2767,14 +2767,111 @@ function getPortConflictServiceHints(platform = process.platform): string[] { ]; } +/** + * Query published OpenShell release tags from GitHub. Returns `null` on any + * fetch failure (gh missing, curl failure, network down) so callers can fall + * back to the legacy install behaviour instead of hard-failing. Returns + * `string[]` (possibly empty) on a successful query. + */ +function listOpenshellReleaseTags(): string[] | null { + const ghResult = spawnSync( + "gh", + [ + "release", + "list", + "--repo", + "NVIDIA/OpenShell", + "--limit", + "100", + "--json", + "tagName", + ], + { + env: { + ...process.env, + GH_PROMPT_DISABLED: "1", + GH_TOKEN: process.env.GH_TOKEN || process.env.GITHUB_TOKEN || "", + }, + encoding: "utf-8", + stdio: ["ignore", "pipe", "pipe"], + timeout: 30_000, + }, + ); + if (ghResult.status === 0 && typeof ghResult.stdout === "string") { + try { + const parsed = JSON.parse(ghResult.stdout); + if (Array.isArray(parsed)) { + return parsed + .map((entry) => (entry && typeof entry.tagName === "string" ? entry.tagName : null)) + .filter((tag): tag is string => tag !== null); + } + } catch { + // fall through to curl + } + } + const curlResult = spawnSync( + "curl", + [ + "-fsSL", + "-H", + "Accept: application/vnd.github+json", + "https://api.github.com/repos/NVIDIA/OpenShell/releases?per_page=100", + ], + { + encoding: "utf-8", + stdio: ["ignore", "pipe", "pipe"], + timeout: 30_000, + }, + ); + if (curlResult.status === 0 && typeof curlResult.stdout === "string") { + try { + const parsed = JSON.parse(curlResult.stdout); + if (Array.isArray(parsed)) { + return parsed + .map((entry) => (entry && typeof entry.tag_name === "string" ? entry.tag_name : null)) + .filter((tag): tag is string => tag !== null); + } + } catch { + return null; + } + } + return null; +} + function installOpenshell(): { installed: boolean; localBin: string | null; futureShellPathHint: string | null; } { + const installEnv: NodeJS.ProcessEnv = { ...process.env }; + const maxVersion = getBlueprintMaxOpenshellVersion(); + if (maxVersion) { + const releases = listOpenshellReleaseTags(); + if (releases !== null && releases.length > 0) { + const resolution = openshellInstallFlow.resolveOpenshellInstallVersion( + releases, + { max: maxVersion }, + { versionGte }, + ); + if (resolution.kind === "incompatible") { + console.error(""); + console.error(` ✗ ${resolution.message}`); + console.error(""); + return { installed: false, localBin: null, futureShellPathHint: null }; + } + if (resolution.kind === "pin") { + installEnv.NEMOCLAW_OPENSHELL_PIN_VERSION = resolution.version; + if (resolution.reason === "max-cap") { + console.log( + ` Pinning OpenShell to ${resolution.version} (latest ${resolution.latest ?? "unknown"} exceeds blueprint max ${maxVersion})`, + ); + } + } + } + } const result = spawnSync("bash", [path.join(SCRIPTS, "install-openshell.sh")], { cwd: ROOT, - env: process.env, + env: installEnv, stdio: ["ignore", "pipe", "pipe"], encoding: "utf-8", timeout: 300_000, @@ -11010,6 +11107,8 @@ module.exports = { getFutureShellPathHint, areRequiredDockerDriverBinariesPresent, ensureOpenshellForOnboard, + parseOpenshellReleaseTag: openshellInstallFlow.parseOpenshellReleaseTag, + resolveOpenshellInstallVersion: openshellInstallFlow.resolveOpenshellInstallVersion, shouldRequireDockerDriverEnv, getGatewayBootstrapRepairPlan, getGatewayLocalEndpoint, diff --git a/src/lib/onboard/openshell-install.ts b/src/lib/onboard/openshell-install.ts index 2b04a0b5fb8..010b106ca57 100644 --- a/src/lib/onboard/openshell-install.ts +++ b/src/lib/onboard/openshell-install.ts @@ -7,6 +7,77 @@ export type OpenShellInstallResult = { futureShellPathHint: string | null; }; +export type OpenshellInstallVersionResolution = + | { kind: "pin"; version: string; latest: string | null; reason: "latest" | "max-cap" } + | { kind: "no-max"; latest: string | null } + | { kind: "incompatible"; latest: string | null; max: string; message: string }; + +const SEMVER_TRIPLE = /^[0-9]+\.[0-9]+\.[0-9]+$/; + +/** + * Sanitize an OpenShell release tag from an external source (GitHub release tag + * name, blueprint field, env var) into a plain `X.Y.Z` version string. Returns + * null for empty, leading-`-`, or non-semver-triple inputs so callers never + * pass malformed strings into version comparison logic. + */ +export function parseOpenshellReleaseTag(tag: unknown): string | null { + if (typeof tag !== "string") return null; + const trimmed = tag.trim(); + if (!trimmed) return null; + if (trimmed.startsWith("-")) return null; + const stripped = trimmed.startsWith("v") ? trimmed.slice(1) : trimmed; + if (!SEMVER_TRIPLE.test(stripped)) return null; + return stripped; +} + +/** + * Pure resolver for which OpenShell release the installer should fetch. + * + * - If `options.max` is null, returns `kind: "no-max"` so callers leave the + * install path alone (legacy behaviour — script picks its own pin / latest). + * - Otherwise, picks the highest entry of `available` that is `<= max`. + * Returns `kind: "incompatible"` with a message naming both latest and max + * when no such release exists. + * + * Malformed entries in `available` (empty string, leading `-`, non-semver) are + * silently dropped. The shipped blueprint guarantees `max` is valid before it + * reaches this function, but a defensive parse is also applied here. + */ +export function resolveOpenshellInstallVersion( + available: readonly string[], + options: { max: string | null }, + helpers: { versionGte: (a: string, b: string) => boolean }, +): OpenshellInstallVersionResolution { + const sanitized = (available ?? []) + .map((entry) => parseOpenshellReleaseTag(entry)) + .filter((entry): entry is string => entry !== null); + sanitized.sort((a, b) => (helpers.versionGte(a, b) ? (a === b ? 0 : -1) : 1)); + const latest = sanitized[0] ?? null; + + const max = parseOpenshellReleaseTag(options.max); + if (!max) { + return { kind: "no-max", latest }; + } + + if (latest && helpers.versionGte(max, latest)) { + return { kind: "pin", version: latest, latest, reason: "latest" }; + } + + const capped = sanitized.find((entry) => helpers.versionGte(max, entry)); + if (capped) { + return { kind: "pin", version: capped, latest, reason: "max-cap" }; + } + + return { + kind: "incompatible", + latest, + max, + message: + `No OpenShell release ≤ ${max} is available (latest published: ${latest ?? "unknown"}). ` + + "Upgrade NemoClaw or raise max_openshell_version in nemoclaw-blueprint/blueprint.yaml.", + }; +} + export type DockerDriverBinaryOverrides = { gatewayBin?: string | null; sandboxBin?: string | null; diff --git a/test/onboard.test.ts b/test/onboard.test.ts index c72fc523218..9b4f8047674 100644 --- a/test/onboard.test.ts +++ b/test/onboard.test.ts @@ -2503,6 +2503,115 @@ const { loadAgent } = require(${agentDefsPath}); expect(versionGte(max, min)).toBe(true); }); + describe("resolveOpenshellInstallVersion (#3404)", () => { + const onboardModule = require("../dist/lib/onboard") as { + parseOpenshellReleaseTag: (tag: unknown) => string | null; + resolveOpenshellInstallVersion: ( + available: readonly string[], + options: { max: string | null }, + helpers: { versionGte: (a: string, b: string) => boolean }, + ) => { + kind: "pin" | "no-max" | "incompatible"; + version?: string; + latest?: string | null; + max?: string; + message?: string; + reason?: "latest" | "max-cap"; + }; + }; + const helpers = { versionGte }; + + it("picks the highest available release ≤ max when latest exceeds max", () => { + const result = onboardModule.resolveOpenshellInstallVersion( + ["v0.0.34", "0.0.35", "v0.0.38"], + { max: "0.0.36" }, + helpers, + ); + expect(result.kind).toBe("pin"); + expect(result.version).toBe("0.0.35"); + expect(result.reason).toBe("max-cap"); + expect(result.latest).toBe("0.0.38"); + }); + + it("picks latest unchanged when latest is ≤ max", () => { + const result = onboardModule.resolveOpenshellInstallVersion( + ["v0.0.34", "0.0.35", "v0.0.36"], + { max: "0.0.39" }, + helpers, + ); + expect(result.kind).toBe("pin"); + expect(result.version).toBe("0.0.36"); + expect(result.reason).toBe("latest"); + expect(result.latest).toBe("0.0.36"); + }); + + it("returns an incompatible resolution when no release ≤ max exists", () => { + const result = onboardModule.resolveOpenshellInstallVersion( + ["v0.0.38", "0.0.39"], + { max: "0.0.36" }, + helpers, + ); + expect(result.kind).toBe("incompatible"); + expect(result.latest).toBe("0.0.39"); + expect(result.max).toBe("0.0.36"); + expect(result.message).toContain("0.0.39"); + expect(result.message).toContain("0.0.36"); + }); + + it("falls back to legacy fetch behaviour when max is missing", () => { + const result = onboardModule.resolveOpenshellInstallVersion( + ["v0.0.38", "0.0.39"], + { max: null }, + helpers, + ); + expect(result.kind).toBe("no-max"); + expect(result.latest).toBe("0.0.39"); + }); + + it("falls back to legacy fetch when max is malformed", () => { + for (const max of ["", "-1.0.0", "not-a-version", "v"] as const) { + const result = onboardModule.resolveOpenshellInstallVersion( + ["v0.0.38"], + { max }, + helpers, + ); + expect(result.kind).toBe("no-max"); + } + }); + + it("silently drops malformed entries from the available list", () => { + const result = onboardModule.resolveOpenshellInstallVersion( + ["", "v0.0.35", "-1.0.0", "not-a-version", "v0.0.34"], + { max: "0.0.36" }, + helpers, + ); + expect(result.kind).toBe("pin"); + expect(result.version).toBe("0.0.35"); + }); + + it("parseOpenshellReleaseTag strips leading v and rejects malformed input", () => { + expect(onboardModule.parseOpenshellReleaseTag("v0.0.39")).toBe("0.0.39"); + expect(onboardModule.parseOpenshellReleaseTag("0.0.39")).toBe("0.0.39"); + expect(onboardModule.parseOpenshellReleaseTag("")).toBe(null); + expect(onboardModule.parseOpenshellReleaseTag(" ")).toBe(null); + expect(onboardModule.parseOpenshellReleaseTag("-1.0.0")).toBe(null); + expect(onboardModule.parseOpenshellReleaseTag("0.0")).toBe(null); + expect(onboardModule.parseOpenshellReleaseTag(42)).toBe(null); + expect(onboardModule.parseOpenshellReleaseTag(null)).toBe(null); + }); + + it("matches the DGX Spark repro: latest=0.0.38 max=0.0.36 picks 0.0.36", () => { + const result = onboardModule.resolveOpenshellInstallVersion( + ["v0.0.36", "v0.0.37", "v0.0.38"], + { max: "0.0.36" }, + helpers, + ); + expect(result.kind).toBe("pin"); + expect(result.version).toBe("0.0.36"); + expect(result.reason).toBe("max-cap"); + }); + }); + it("pins the gateway image to the installed OpenShell release version", () => { expect(getInstalledOpenshellVersion("openshell 0.0.12")).toBe("0.0.12"); expect(getInstalledOpenshellVersion("openshell 0.0.13-dev.8+gbbcaed2ea")).toBe("0.0.13"); From bf351020fa433c75bacc78209a50dee71bc924d5 Mon Sep 17 00:00:00 2001 From: Dongni Yang Date: Wed, 13 May 2026 16:47:34 +0800 Subject: [PATCH 2/7] fix(onboard): wire blueprint MIN/MAX through install script (#3404) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses CodeRabbit review feedback and the onboard-entrypoint-budget check on #3446, and widens scope so the shell installer's MIN/MAX constants are no longer a second source of truth: - Move installOpenshell() body to src/lib/onboard/openshell-pin.ts so src/lib/onboard.ts shrinks by 19 net lines (entrypoint budget passes). - Paginate published OpenShell release lookups: gh release list uses --limit 1000; the curl fallback walks per_page=100&page=N until the page is short. Older compatible versions are reachable once NVIDIA/OpenShell exceeds one page. - Skip NEMOCLAW_OPENSHELL_PIN_VERSION (and now MIN/MAX) validation on the dev channel — that path installs from RELEASE_TAG=dev and never consults PIN_VERSION, so a malformed override should not abort a dev install. - TS pin module overlays NEMOCLAW_OPENSHELL_MIN_VERSION and NEMOCLAW_OPENSHELL_MAX_VERSION from the blueprint onto spawn env; install-openshell.sh validates each against ^X.Y.Z$ and overrides its hardcoded MIN_VERSION / MAX_VERSION when the env is present and the channel is stable. - Unit tests cover the new MIN/MAX/PIN overlay and the orchestrator fallback paths (offline, no max, no release ≤ max). Signed-off-by: Dongni Yang Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/install-openshell.sh | 36 ++++- src/lib/onboard.ts | 148 ++------------------ src/lib/onboard/openshell-pin.ts | 230 +++++++++++++++++++++++++++++++ test/onboard.test.ts | 166 +++++++++++++++++++--- 4 files changed, 424 insertions(+), 156 deletions(-) create mode 100644 src/lib/onboard/openshell-pin.ts diff --git a/scripts/install-openshell.sh b/scripts/install-openshell.sh index c0fcdd3d68c..4b6ad9fabcc 100755 --- a/scripts/install-openshell.sh +++ b/scripts/install-openshell.sh @@ -44,13 +44,6 @@ MAX_VERSION="0.0.39" # OpenShell release that satisfies the blueprint's max_openshell_version # (see #3404). The hardcoded value is the fallback for offline runs. PIN_VERSION="$MAX_VERSION" -if [ -n "${NEMOCLAW_OPENSHELL_PIN_VERSION:-}" ]; then - if [[ "$NEMOCLAW_OPENSHELL_PIN_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then - PIN_VERSION="$NEMOCLAW_OPENSHELL_PIN_VERSION" - else - fail "NEMOCLAW_OPENSHELL_PIN_VERSION='$NEMOCLAW_OPENSHELL_PIN_VERSION' is not a valid X.Y.Z version." - fi -fi DEV_MIN_VERSION="0.0.39" CHANNEL="${NEMOCLAW_OPENSHELL_CHANNEL:-auto}" @@ -65,6 +58,35 @@ else RESOLVED_CHANNEL="$CHANNEL" fi +# Honour the TS installer's blueprint-derived env overrides only on the stable +# channel — the dev channel installs from the `dev` tag and uses DEV_MIN_VERSION +# instead, so a malformed override should not abort a dev install (#3446 review). +# The TS layer passes MIN/MAX/PIN from the blueprint so a single source of truth +# (nemoclaw-blueprint/blueprint.yaml) drives the install (#3404). +apply_blueprint_override() { + local name="$1" value="$2" + if [[ "$value" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + printf '%s' "$value" + else + fail "${name}='${value}' is not a valid X.Y.Z version." + fi +} +if [ "$RESOLVED_CHANNEL" != "dev" ]; then + if [ -n "${NEMOCLAW_OPENSHELL_MIN_VERSION:-}" ]; then + MIN_VERSION="$(apply_blueprint_override NEMOCLAW_OPENSHELL_MIN_VERSION "$NEMOCLAW_OPENSHELL_MIN_VERSION")" + fi + if [ -n "${NEMOCLAW_OPENSHELL_MAX_VERSION:-}" ]; then + MAX_VERSION="$(apply_blueprint_override NEMOCLAW_OPENSHELL_MAX_VERSION "$NEMOCLAW_OPENSHELL_MAX_VERSION")" + # Default the pin to the (possibly overridden) MAX_VERSION before applying + # the explicit PIN override so a bumped blueprint without a resolver result + # still pins to the new max. + PIN_VERSION="$MAX_VERSION" + fi + if [ -n "${NEMOCLAW_OPENSHELL_PIN_VERSION:-}" ]; then + PIN_VERSION="$(apply_blueprint_override NEMOCLAW_OPENSHELL_PIN_VERSION "$NEMOCLAW_OPENSHELL_PIN_VERSION")" + fi +fi + if [ "$RESOLVED_CHANNEL" = "dev" ]; then RELEASE_TAG="dev" else diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index b5391e0a0eb..bce19698589 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -309,6 +309,8 @@ const validationRecovery: typeof import("./validation-recovery") = require("./va const webSearch: typeof import("./inference/web-search") = require("./inference/web-search"); const openshellInstallFlow: typeof import("./onboard/openshell-install") = require("./onboard/openshell-install"); +const openshellPinFlow: typeof import("./onboard/openshell-pin") = + require("./onboard/openshell-pin"); const sandboxCreateFailureDiagnostics: typeof import("./onboard/sandbox-create-failure") = require("./onboard/sandbox-create-failure"); @@ -345,6 +347,7 @@ import type { WebSearchConfig } from "./inference/web-search"; import type { DockerDriverBinaryOverrides, OpenShellInstallDeps, + OpenShellInstallResult, } from "./onboard/openshell-install"; import type { SelectionDrift } from "./onboard/selection-drift"; @@ -2767,139 +2770,20 @@ function getPortConflictServiceHints(platform = process.platform): string[] { ]; } -/** - * Query published OpenShell release tags from GitHub. Returns `null` on any - * fetch failure (gh missing, curl failure, network down) so callers can fall - * back to the legacy install behaviour instead of hard-failing. Returns - * `string[]` (possibly empty) on a successful query. - */ -function listOpenshellReleaseTags(): string[] | null { - const ghResult = spawnSync( - "gh", - [ - "release", - "list", - "--repo", - "NVIDIA/OpenShell", - "--limit", - "100", - "--json", - "tagName", - ], - { - env: { - ...process.env, - GH_PROMPT_DISABLED: "1", - GH_TOKEN: process.env.GH_TOKEN || process.env.GITHUB_TOKEN || "", - }, - encoding: "utf-8", - stdio: ["ignore", "pipe", "pipe"], - timeout: 30_000, - }, - ); - if (ghResult.status === 0 && typeof ghResult.stdout === "string") { - try { - const parsed = JSON.parse(ghResult.stdout); - if (Array.isArray(parsed)) { - return parsed - .map((entry) => (entry && typeof entry.tagName === "string" ? entry.tagName : null)) - .filter((tag): tag is string => tag !== null); - } - } catch { - // fall through to curl - } - } - const curlResult = spawnSync( - "curl", - [ - "-fsSL", - "-H", - "Accept: application/vnd.github+json", - "https://api.github.com/repos/NVIDIA/OpenShell/releases?per_page=100", - ], - { - encoding: "utf-8", - stdio: ["ignore", "pipe", "pipe"], - timeout: 30_000, - }, - ); - if (curlResult.status === 0 && typeof curlResult.stdout === "string") { - try { - const parsed = JSON.parse(curlResult.stdout); - if (Array.isArray(parsed)) { - return parsed - .map((entry) => (entry && typeof entry.tag_name === "string" ? entry.tag_name : null)) - .filter((tag): tag is string => tag !== null); - } - } catch { - return null; - } - } - return null; -} - -function installOpenshell(): { - installed: boolean; - localBin: string | null; - futureShellPathHint: string | null; -} { - const installEnv: NodeJS.ProcessEnv = { ...process.env }; - const maxVersion = getBlueprintMaxOpenshellVersion(); - if (maxVersion) { - const releases = listOpenshellReleaseTags(); - if (releases !== null && releases.length > 0) { - const resolution = openshellInstallFlow.resolveOpenshellInstallVersion( - releases, - { max: maxVersion }, - { versionGte }, - ); - if (resolution.kind === "incompatible") { - console.error(""); - console.error(` ✗ ${resolution.message}`); - console.error(""); - return { installed: false, localBin: null, futureShellPathHint: null }; - } - if (resolution.kind === "pin") { - installEnv.NEMOCLAW_OPENSHELL_PIN_VERSION = resolution.version; - if (resolution.reason === "max-cap") { - console.log( - ` Pinning OpenShell to ${resolution.version} (latest ${resolution.latest ?? "unknown"} exceeds blueprint max ${maxVersion})`, - ); - } - } - } - } - const result = spawnSync("bash", [path.join(SCRIPTS, "install-openshell.sh")], { +function installOpenshell(): OpenShellInstallResult { + return openshellPinFlow.runOpenshellInstall({ + scriptsDir: SCRIPTS, cwd: ROOT, - env: installEnv, - stdio: ["ignore", "pipe", "pipe"], - encoding: "utf-8", - timeout: 300_000, + resolveOpenshell, + getFutureShellPathHint, + setOpenshellBin: (bin) => { + OPENSHELL_BIN = bin; + }, + getBlueprintMinOpenshellVersion, + getBlueprintMaxOpenshellVersion, + versionGte, + log: console.log, }); - if (result.status !== 0) { - const output = `${result.stdout || ""}${result.stderr || ""}`.trim(); - if (output) { - console.error(output); - } - return { installed: false, localBin: null, futureShellPathHint: null }; - } - const localBin = process.env.XDG_BIN_HOME || path.join(process.env.HOME || "", ".local", "bin"); - const openshellPath = path.join(localBin, "openshell"); - const futureShellPathHint = fs.existsSync(openshellPath) - ? getFutureShellPathHint(localBin, process.env.PATH) - : null; - if (fs.existsSync(openshellPath) && futureShellPathHint) { - process.env.PATH = `${localBin}${path.delimiter}${process.env.PATH}`; - } - OPENSHELL_BIN = resolveOpenshell(); - if (OPENSHELL_BIN) { - process.env.NEMOCLAW_OPENSHELL_BIN = OPENSHELL_BIN; - } - return { - installed: OPENSHELL_BIN !== null, - localBin, - futureShellPathHint, - }; } function areRequiredDockerDriverBinariesPresent( @@ -11107,8 +10991,6 @@ module.exports = { getFutureShellPathHint, areRequiredDockerDriverBinariesPresent, ensureOpenshellForOnboard, - parseOpenshellReleaseTag: openshellInstallFlow.parseOpenshellReleaseTag, - resolveOpenshellInstallVersion: openshellInstallFlow.resolveOpenshellInstallVersion, shouldRequireDockerDriverEnv, getGatewayBootstrapRepairPlan, getGatewayLocalEndpoint, diff --git a/src/lib/onboard/openshell-pin.ts b/src/lib/onboard/openshell-pin.ts new file mode 100644 index 00000000000..d8832f3fb1e --- /dev/null +++ b/src/lib/onboard/openshell-pin.ts @@ -0,0 +1,230 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { spawnSync, type SpawnSyncOptionsWithStringEncoding } from "node:child_process"; +import * as fs from "node:fs"; +import * as path from "node:path"; + +import { + type OpenShellInstallResult, + type OpenshellInstallVersionResolution, + resolveOpenshellInstallVersion, +} from "./openshell-install"; + +const GH_LIMIT = 1000; +const PER_PAGE = 100; +const PAGE_BUDGET = 10; + +type ReleaseFetcher = () => string[] | null; + +export type OpenshellInstallPinDeps = { + getBlueprintMinOpenshellVersion?: () => string | null; + getBlueprintMaxOpenshellVersion: () => string | null; + versionGte: (a: string, b: string) => boolean; + listReleases?: ReleaseFetcher; + log?: (message: string) => void; + error?: (message: string) => void; +}; + +export type OpenshellInstallEnvDirective = + | { env: NodeJS.ProcessEnv } + | { env: null }; + +export type OpenshellInstallPinResult = + | { kind: "pin"; version: string; latest: string | null; reason: "latest" | "max-cap" } + | { kind: "no-max" } + | { kind: "incompatible"; message: string }; + +/** + * List published OpenShell release tags. Returns `null` on any fetch failure + * (gh missing, curl failure, network down) so callers fall back to the legacy + * install path. Pages beyond `PER_PAGE` results so the resolver does not miss + * older compatible releases once the repo exceeds one page (#3446 review). + */ +export function listOpenshellReleaseTags(): string[] | null { + const ghTags = listOpenshellReleaseTagsViaGh(); + if (ghTags !== null) return ghTags; + return listOpenshellReleaseTagsViaCurl(); +} + +function listOpenshellReleaseTagsViaGh(): string[] | null { + const options: SpawnSyncOptionsWithStringEncoding = { + env: { + ...process.env, + GH_PROMPT_DISABLED: "1", + GH_TOKEN: process.env.GH_TOKEN || process.env.GITHUB_TOKEN || "", + }, + encoding: "utf-8", + stdio: ["ignore", "pipe", "pipe"], + timeout: 30_000, + }; + const result = spawnSync( + "gh", + [ + "release", + "list", + "--repo", + "NVIDIA/OpenShell", + "--limit", + String(GH_LIMIT), + "--json", + "tagName", + ], + options, + ); + if (result.status !== 0 || typeof result.stdout !== "string") return null; + try { + const parsed = JSON.parse(result.stdout); + if (!Array.isArray(parsed)) return null; + return parsed + .map((entry) => (entry && typeof entry.tagName === "string" ? entry.tagName : null)) + .filter((tag): tag is string => tag !== null); + } catch { + return null; + } +} + +function listOpenshellReleaseTagsViaCurl(): string[] | null { + const tags: string[] = []; + for (let page = 1; page <= PAGE_BUDGET; page += 1) { + const result = spawnSync( + "curl", + [ + "-fsSL", + "-H", + "Accept: application/vnd.github+json", + `https://api.github.com/repos/NVIDIA/OpenShell/releases?per_page=${PER_PAGE}&page=${page}`, + ], + { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], timeout: 30_000 }, + ); + if (result.status !== 0 || typeof result.stdout !== "string") { + return page === 1 ? null : tags; + } + let parsed: unknown; + try { + parsed = JSON.parse(result.stdout); + } catch { + return page === 1 ? null : tags; + } + if (!Array.isArray(parsed) || parsed.length === 0) break; + for (const entry of parsed) { + if (entry && typeof (entry as { tag_name?: unknown }).tag_name === "string") { + tags.push((entry as { tag_name: string }).tag_name); + } + } + if (parsed.length < PER_PAGE) break; + } + return tags; +} + +/** + * Resolve which OpenShell release to pin the installer to. Orchestrates the + * GitHub fetch and the pure resolver. Returns a result that the caller can map + * to env var / error / no-op without itself touching the network. + * + * When the GitHub query fails (offline, gh missing), returns `kind: "no-max"` + * so the shell installer falls back to its hardcoded pin. The blueprint's + * upper-bound is still enforced post-install by the existing gate, so a stale + * fetch never silently raises the cap. + */ +export function resolveOpenshellInstallPin( + deps: OpenshellInstallPinDeps, +): OpenshellInstallPinResult { + const maxVersion = deps.getBlueprintMaxOpenshellVersion(); + if (!maxVersion) return { kind: "no-max" }; + const releases = (deps.listReleases ?? listOpenshellReleaseTags)(); + if (releases === null || releases.length === 0) return { kind: "no-max" }; + const resolution: OpenshellInstallVersionResolution = resolveOpenshellInstallVersion( + releases, + { max: maxVersion }, + { versionGte: deps.versionGte }, + ); + if (resolution.kind === "pin") { + if (resolution.reason === "max-cap" && deps.log) { + deps.log( + ` Pinning OpenShell to ${resolution.version} (latest ${resolution.latest ?? "unknown"} exceeds blueprint max ${maxVersion})`, + ); + } + return { + kind: "pin", + version: resolution.version, + latest: resolution.latest, + reason: resolution.reason, + }; + } + if (resolution.kind === "incompatible") { + return { kind: "incompatible", message: resolution.message }; + } + return { kind: "no-max" }; +} + +/** + * Compose the resolution with side-effects so the caller can map a single + * value into the `spawnSync` call: `env` is the env to pass through, or + * `null` to abort (the helper has already logged a clear error in that case). + */ +export function computeOpenshellInstallEnv( + baseEnv: NodeJS.ProcessEnv, + deps: OpenshellInstallPinDeps, +): OpenshellInstallEnvDirective { + const pin = resolveOpenshellInstallPin(deps); + if (pin.kind === "incompatible") { + const error = deps.error ?? ((m: string) => console.error(m)); + error(""); + error(` ✗ ${pin.message}`); + error(""); + return { env: null }; + } + const overlay: NodeJS.ProcessEnv = {}; + const blueprintMin = deps.getBlueprintMinOpenshellVersion?.() ?? null; + const blueprintMax = deps.getBlueprintMaxOpenshellVersion(); + if (blueprintMin) overlay.NEMOCLAW_OPENSHELL_MIN_VERSION = blueprintMin; + if (blueprintMax) overlay.NEMOCLAW_OPENSHELL_MAX_VERSION = blueprintMax; + if (pin.kind === "pin") overlay.NEMOCLAW_OPENSHELL_PIN_VERSION = pin.version; + return Object.keys(overlay).length === 0 + ? { env: baseEnv } + : { env: { ...baseEnv, ...overlay } }; +} + +export type RunOpenshellInstallDeps = OpenshellInstallPinDeps & { + scriptsDir: string; + cwd: string; + resolveOpenshell: () => string | null; + getFutureShellPathHint: (binDir: string, pathValue?: string) => string | null; + setOpenshellBin: (binPath: string | null) => void; +}; + +/** + * Execute `scripts/install-openshell.sh`, wiring in the blueprint-driven pin + * resolution and the host-side state updates onboard.ts cares about (binary + * path, PATH augmentation, `OPENSHELL_BIN` cache). Lives in this submodule so + * the top-level onboard entrypoint stays net-neutral (#3404 follow-up). + */ +export function runOpenshellInstall(deps: RunOpenshellInstallDeps): OpenShellInstallResult { + const { env } = computeOpenshellInstallEnv(process.env, deps); + if (env === null) return { installed: false, localBin: null, futureShellPathHint: null }; + const result = spawnSync("bash", [path.join(deps.scriptsDir, "install-openshell.sh")], { + cwd: deps.cwd, + env, + stdio: ["ignore", "pipe", "pipe"], + encoding: "utf-8", + timeout: 300_000, + }); + if (result.status !== 0) { + const output = `${result.stdout || ""}${result.stderr || ""}`.trim(); + if (output) console.error(output); + return { installed: false, localBin: null, futureShellPathHint: null }; + } + const localBin = process.env.XDG_BIN_HOME || path.join(process.env.HOME || "", ".local", "bin"); + const openshellPath = path.join(localBin, "openshell"); + const futureShellPathHint = fs.existsSync(openshellPath) + ? deps.getFutureShellPathHint(localBin, process.env.PATH) + : null; + if (fs.existsSync(openshellPath) && futureShellPathHint) { + process.env.PATH = `${localBin}${path.delimiter}${process.env.PATH}`; + } + const bin = deps.resolveOpenshell(); + deps.setOpenshellBin(bin); + if (bin) process.env.NEMOCLAW_OPENSHELL_BIN = bin; + return { installed: bin !== null, localBin, futureShellPathHint }; +} diff --git a/test/onboard.test.ts b/test/onboard.test.ts index 9b4f8047674..66b4c72f5f6 100644 --- a/test/onboard.test.ts +++ b/test/onboard.test.ts @@ -2504,7 +2504,7 @@ const { loadAgent } = require(${agentDefsPath}); }); describe("resolveOpenshellInstallVersion (#3404)", () => { - const onboardModule = require("../dist/lib/onboard") as { + const installModule = require("../dist/lib/onboard/openshell-install") as { parseOpenshellReleaseTag: (tag: unknown) => string | null; resolveOpenshellInstallVersion: ( available: readonly string[], @@ -2522,7 +2522,7 @@ const { loadAgent } = require(${agentDefsPath}); const helpers = { versionGte }; it("picks the highest available release ≤ max when latest exceeds max", () => { - const result = onboardModule.resolveOpenshellInstallVersion( + const result = installModule.resolveOpenshellInstallVersion( ["v0.0.34", "0.0.35", "v0.0.38"], { max: "0.0.36" }, helpers, @@ -2534,7 +2534,7 @@ const { loadAgent } = require(${agentDefsPath}); }); it("picks latest unchanged when latest is ≤ max", () => { - const result = onboardModule.resolveOpenshellInstallVersion( + const result = installModule.resolveOpenshellInstallVersion( ["v0.0.34", "0.0.35", "v0.0.36"], { max: "0.0.39" }, helpers, @@ -2546,7 +2546,7 @@ const { loadAgent } = require(${agentDefsPath}); }); it("returns an incompatible resolution when no release ≤ max exists", () => { - const result = onboardModule.resolveOpenshellInstallVersion( + const result = installModule.resolveOpenshellInstallVersion( ["v0.0.38", "0.0.39"], { max: "0.0.36" }, helpers, @@ -2559,7 +2559,7 @@ const { loadAgent } = require(${agentDefsPath}); }); it("falls back to legacy fetch behaviour when max is missing", () => { - const result = onboardModule.resolveOpenshellInstallVersion( + const result = installModule.resolveOpenshellInstallVersion( ["v0.0.38", "0.0.39"], { max: null }, helpers, @@ -2570,7 +2570,7 @@ const { loadAgent } = require(${agentDefsPath}); it("falls back to legacy fetch when max is malformed", () => { for (const max of ["", "-1.0.0", "not-a-version", "v"] as const) { - const result = onboardModule.resolveOpenshellInstallVersion( + const result = installModule.resolveOpenshellInstallVersion( ["v0.0.38"], { max }, helpers, @@ -2580,7 +2580,7 @@ const { loadAgent } = require(${agentDefsPath}); }); it("silently drops malformed entries from the available list", () => { - const result = onboardModule.resolveOpenshellInstallVersion( + const result = installModule.resolveOpenshellInstallVersion( ["", "v0.0.35", "-1.0.0", "not-a-version", "v0.0.34"], { max: "0.0.36" }, helpers, @@ -2590,18 +2590,18 @@ const { loadAgent } = require(${agentDefsPath}); }); it("parseOpenshellReleaseTag strips leading v and rejects malformed input", () => { - expect(onboardModule.parseOpenshellReleaseTag("v0.0.39")).toBe("0.0.39"); - expect(onboardModule.parseOpenshellReleaseTag("0.0.39")).toBe("0.0.39"); - expect(onboardModule.parseOpenshellReleaseTag("")).toBe(null); - expect(onboardModule.parseOpenshellReleaseTag(" ")).toBe(null); - expect(onboardModule.parseOpenshellReleaseTag("-1.0.0")).toBe(null); - expect(onboardModule.parseOpenshellReleaseTag("0.0")).toBe(null); - expect(onboardModule.parseOpenshellReleaseTag(42)).toBe(null); - expect(onboardModule.parseOpenshellReleaseTag(null)).toBe(null); + expect(installModule.parseOpenshellReleaseTag("v0.0.39")).toBe("0.0.39"); + expect(installModule.parseOpenshellReleaseTag("0.0.39")).toBe("0.0.39"); + expect(installModule.parseOpenshellReleaseTag("")).toBe(null); + expect(installModule.parseOpenshellReleaseTag(" ")).toBe(null); + expect(installModule.parseOpenshellReleaseTag("-1.0.0")).toBe(null); + expect(installModule.parseOpenshellReleaseTag("0.0")).toBe(null); + expect(installModule.parseOpenshellReleaseTag(42)).toBe(null); + expect(installModule.parseOpenshellReleaseTag(null)).toBe(null); }); it("matches the DGX Spark repro: latest=0.0.38 max=0.0.36 picks 0.0.36", () => { - const result = onboardModule.resolveOpenshellInstallVersion( + const result = installModule.resolveOpenshellInstallVersion( ["v0.0.36", "v0.0.37", "v0.0.38"], { max: "0.0.36" }, helpers, @@ -2612,6 +2612,140 @@ const { loadAgent } = require(${agentDefsPath}); }); }); + describe("resolveOpenshellInstallPin (#3404 orchestrator)", () => { + const pinModule = require("../dist/lib/onboard/openshell-pin") as { + resolveOpenshellInstallPin: (deps: { + getBlueprintMaxOpenshellVersion: () => string | null; + versionGte: (a: string, b: string) => boolean; + listReleases?: () => string[] | null; + log?: (m: string) => void; + }) => { kind: "pin" | "no-max" | "incompatible"; version?: string; message?: string }; + }; + + it("returns no-max when the blueprint has no max_openshell_version", () => { + const result = pinModule.resolveOpenshellInstallPin({ + getBlueprintMaxOpenshellVersion: () => null, + versionGte, + listReleases: () => ["v0.0.38"], + }); + expect(result.kind).toBe("no-max"); + }); + + it("falls back to no-max when GitHub fetch fails (offline)", () => { + const result = pinModule.resolveOpenshellInstallPin({ + getBlueprintMaxOpenshellVersion: () => "0.0.36", + versionGte, + listReleases: () => null, + }); + expect(result.kind).toBe("no-max"); + }); + + it("falls back to no-max when GitHub returns an empty list", () => { + const result = pinModule.resolveOpenshellInstallPin({ + getBlueprintMaxOpenshellVersion: () => "0.0.36", + versionGte, + listReleases: () => [], + }); + expect(result.kind).toBe("no-max"); + }); + + it("pins to highest ≤ max when releases exceed the cap (QA repro)", () => { + const logged: string[] = []; + const result = pinModule.resolveOpenshellInstallPin({ + getBlueprintMaxOpenshellVersion: () => "0.0.36", + versionGte, + listReleases: () => ["v0.0.36", "v0.0.37", "v0.0.38"], + log: (m) => logged.push(m), + }); + expect(result.kind).toBe("pin"); + expect(result.version).toBe("0.0.36"); + expect(logged.join("\n")).toContain("0.0.36"); + expect(logged.join("\n")).toContain("0.0.38"); + }); + + it("surfaces incompatible when no published release ≤ max exists", () => { + const result = pinModule.resolveOpenshellInstallPin({ + getBlueprintMaxOpenshellVersion: () => "0.0.36", + versionGte, + listReleases: () => ["v0.0.38", "v0.0.39"], + }); + expect(result.kind).toBe("incompatible"); + expect(result.message ?? "").toContain("0.0.36"); + expect(result.message ?? "").toContain("0.0.39"); + }); + }); + + describe("computeOpenshellInstallEnv overlays MIN/MAX/PIN (#3404 widening)", () => { + const pinModule = require("../dist/lib/onboard/openshell-pin") as { + computeOpenshellInstallEnv: ( + baseEnv: Record, + deps: { + getBlueprintMinOpenshellVersion?: () => string | null; + getBlueprintMaxOpenshellVersion: () => string | null; + versionGte: (a: string, b: string) => boolean; + listReleases?: () => string[] | null; + log?: (m: string) => void; + }, + ) => { env: Record | null }; + }; + + it("overlays MIN/MAX/PIN env vars from blueprint when latest exceeds max", () => { + const result = pinModule.computeOpenshellInstallEnv( + { EXISTING: "preserved" }, + { + getBlueprintMinOpenshellVersion: () => "0.0.39", + getBlueprintMaxOpenshellVersion: () => "0.0.39", + versionGte, + listReleases: () => ["v0.0.38", "v0.0.39", "v0.0.42"], + }, + ); + expect(result.env).not.toBe(null); + expect(result.env?.EXISTING).toBe("preserved"); + expect(result.env?.NEMOCLAW_OPENSHELL_MIN_VERSION).toBe("0.0.39"); + expect(result.env?.NEMOCLAW_OPENSHELL_MAX_VERSION).toBe("0.0.39"); + expect(result.env?.NEMOCLAW_OPENSHELL_PIN_VERSION).toBe("0.0.39"); + }); + + it("overlays MIN/MAX but no PIN when GitHub fetch fails (offline)", () => { + const result = pinModule.computeOpenshellInstallEnv( + {}, + { + getBlueprintMinOpenshellVersion: () => "0.0.39", + getBlueprintMaxOpenshellVersion: () => "0.0.39", + versionGte, + listReleases: () => null, + }, + ); + expect(result.env).not.toBe(null); + expect(result.env?.NEMOCLAW_OPENSHELL_MIN_VERSION).toBe("0.0.39"); + expect(result.env?.NEMOCLAW_OPENSHELL_MAX_VERSION).toBe("0.0.39"); + expect(result.env?.NEMOCLAW_OPENSHELL_PIN_VERSION).toBeUndefined(); + }); + + it("returns the base env unchanged when blueprint exposes no min/max", () => { + const baseEnv = { ONLY_THIS: "value" }; + const result = pinModule.computeOpenshellInstallEnv(baseEnv, { + getBlueprintMinOpenshellVersion: () => null, + getBlueprintMaxOpenshellVersion: () => null, + versionGte, + listReleases: () => ["v0.0.38"], + }); + expect(result.env).toBe(baseEnv); + }); + + it("aborts (env=null) when no release ≤ max exists", () => { + const result = pinModule.computeOpenshellInstallEnv( + {}, + { + getBlueprintMaxOpenshellVersion: () => "0.0.36", + versionGte, + listReleases: () => ["v0.0.38", "v0.0.39"], + }, + ); + expect(result.env).toBe(null); + }); + }); + it("pins the gateway image to the installed OpenShell release version", () => { expect(getInstalledOpenshellVersion("openshell 0.0.12")).toBe("0.0.12"); expect(getInstalledOpenshellVersion("openshell 0.0.13-dev.8+gbbcaed2ea")).toBe("0.0.13"); From cde2d707d0346e69baceb37d0c5d8c7d41aa7498 Mon Sep 17 00:00:00 2001 From: Dongni Yang Date: Wed, 13 May 2026 17:04:34 +0800 Subject: [PATCH 3/7] fix(onboard): preserve legacy fallback paths on partial release fetch (#3404) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address two CodeRabbit major findings on #3446: - scripts/install-openshell.sh no longer defaults PIN_VERSION to a blueprint MAX env override. When the TS resolver couldn't reach GitHub it only sets MIN/MAX (never PIN), and the script's hardcoded PIN_VERSION is the known-good safe fallback. Letting the blueprint MAX silently drive the pin meant a bumped blueprint + rate-limited install would probe an unreleased tag. - src/lib/onboard/openshell-pin.ts paginated curl fetch returns null on any per-page failure rather than the partial accumulated list. A page-2+ failure can hide older compatible releases, which would cause the resolver to falsely return "incompatible" and abort the install — null preserves the legacy fallback. Signed-off-by: Dongni Yang Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/install-openshell.sh | 8 ++++---- src/lib/onboard/openshell-pin.ts | 11 +++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/scripts/install-openshell.sh b/scripts/install-openshell.sh index 4b6ad9fabcc..1dd7d339218 100755 --- a/scripts/install-openshell.sh +++ b/scripts/install-openshell.sh @@ -77,10 +77,10 @@ if [ "$RESOLVED_CHANNEL" != "dev" ]; then fi if [ -n "${NEMOCLAW_OPENSHELL_MAX_VERSION:-}" ]; then MAX_VERSION="$(apply_blueprint_override NEMOCLAW_OPENSHELL_MAX_VERSION "$NEMOCLAW_OPENSHELL_MAX_VERSION")" - # Default the pin to the (possibly overridden) MAX_VERSION before applying - # the explicit PIN override so a bumped blueprint without a resolver result - # still pins to the new max. - PIN_VERSION="$MAX_VERSION" + # Intentionally do NOT default PIN_VERSION to the overridden MAX here. + # If the TS resolver couldn't reach GitHub (rate-limited / offline) it + # only sets MIN/MAX, never PIN — falling through to the script's + # hardcoded PIN_VERSION is the known-good safe path (#3446 CodeRabbit). fi if [ -n "${NEMOCLAW_OPENSHELL_PIN_VERSION:-}" ]; then PIN_VERSION="$(apply_blueprint_override NEMOCLAW_OPENSHELL_PIN_VERSION "$NEMOCLAW_OPENSHELL_PIN_VERSION")" diff --git a/src/lib/onboard/openshell-pin.ts b/src/lib/onboard/openshell-pin.ts index d8832f3fb1e..cbed863b7f3 100644 --- a/src/lib/onboard/openshell-pin.ts +++ b/src/lib/onboard/openshell-pin.ts @@ -97,14 +97,17 @@ function listOpenshellReleaseTagsViaCurl(): string[] | null { ], { encoding: "utf-8", stdio: ["ignore", "pipe", "pipe"], timeout: 30_000 }, ); - if (result.status !== 0 || typeof result.stdout !== "string") { - return page === 1 ? null : tags; - } + // Any per-page failure invalidates the whole list — a partial result + // could let the resolver wrongly return `incompatible` because an older + // compatible release on a missing page is invisible to it. Returning + // null lets the caller fall back to the script's legacy behaviour + // (#3446 CodeRabbit). + if (result.status !== 0 || typeof result.stdout !== "string") return null; let parsed: unknown; try { parsed = JSON.parse(result.stdout); } catch { - return page === 1 ? null : tags; + return null; } if (!Array.isArray(parsed) || parsed.length === 0) break; for (const entry of parsed) { From 18fe70508789aceaf06ea93b6b301fdcfb116464 Mon Sep 17 00:00:00 2001 From: Dongni Yang Date: Wed, 13 May 2026 17:14:41 +0800 Subject: [PATCH 4/7] fix(onboard): surface install-openshell.sh validation errors to stderr (#3404) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While running the issue #3404 repro locally with NEMOCLAW_OPENSHELL_PIN_VERSION set to a malformed value, the script exited 1 with no visible error: the `apply_blueprint_override` helper called `fail` inside a `$(...)` substitution, which captured the stderr-less error message into the variable assignment instead of letting it reach the user. Inline the regex check at each call site so `fail`'s message reaches stderr when an override is malformed. Behaviour for valid overrides is unchanged. Verified end-to-end against the real install-openshell.sh: - A. issue #3404 repro (resolver picked 0.0.36) → "Installing OpenShell from release 'v0.0.36'..." - B. today (resolver picked 0.0.39) → "Installing OpenShell from release 'v0.0.39'..." - C. offline (TS only set MIN/MAX, no PIN) → script keeps hardcoded 'v0.0.39' - D. dev channel with garbage overrides → "Installing OpenShell from release 'dev'..." - E. malformed PIN on stable → fails loudly with the env-name in the message Signed-off-by: Dongni Yang Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/install-openshell.sh | 38 ++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/scripts/install-openshell.sh b/scripts/install-openshell.sh index 1dd7d339218..c2a98f294d7 100755 --- a/scripts/install-openshell.sh +++ b/scripts/install-openshell.sh @@ -63,27 +63,35 @@ fi # instead, so a malformed override should not abort a dev install (#3446 review). # The TS layer passes MIN/MAX/PIN from the blueprint so a single source of truth # (nemoclaw-blueprint/blueprint.yaml) drives the install (#3404). -apply_blueprint_override() { - local name="$1" value="$2" - if [[ "$value" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then - printf '%s' "$value" - else - fail "${name}='${value}' is not a valid X.Y.Z version." - fi -} +# +# Validation is inlined (rather than wrapped in a helper that returns via +# $(...)) so that `fail`'s error message reaches the user's stderr instead of +# being captured into the variable assignment. if [ "$RESOLVED_CHANNEL" != "dev" ]; then if [ -n "${NEMOCLAW_OPENSHELL_MIN_VERSION:-}" ]; then - MIN_VERSION="$(apply_blueprint_override NEMOCLAW_OPENSHELL_MIN_VERSION "$NEMOCLAW_OPENSHELL_MIN_VERSION")" + if [[ "$NEMOCLAW_OPENSHELL_MIN_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + MIN_VERSION="$NEMOCLAW_OPENSHELL_MIN_VERSION" + else + fail "NEMOCLAW_OPENSHELL_MIN_VERSION='$NEMOCLAW_OPENSHELL_MIN_VERSION' is not a valid X.Y.Z version." + fi fi if [ -n "${NEMOCLAW_OPENSHELL_MAX_VERSION:-}" ]; then - MAX_VERSION="$(apply_blueprint_override NEMOCLAW_OPENSHELL_MAX_VERSION "$NEMOCLAW_OPENSHELL_MAX_VERSION")" - # Intentionally do NOT default PIN_VERSION to the overridden MAX here. - # If the TS resolver couldn't reach GitHub (rate-limited / offline) it - # only sets MIN/MAX, never PIN — falling through to the script's - # hardcoded PIN_VERSION is the known-good safe path (#3446 CodeRabbit). + if [[ "$NEMOCLAW_OPENSHELL_MAX_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + MAX_VERSION="$NEMOCLAW_OPENSHELL_MAX_VERSION" + # Intentionally do NOT default PIN_VERSION to the overridden MAX here. + # If the TS resolver couldn't reach GitHub (rate-limited / offline) it + # only sets MIN/MAX, never PIN — falling through to the script's + # hardcoded PIN_VERSION is the known-good safe path (#3446 CodeRabbit). + else + fail "NEMOCLAW_OPENSHELL_MAX_VERSION='$NEMOCLAW_OPENSHELL_MAX_VERSION' is not a valid X.Y.Z version." + fi fi if [ -n "${NEMOCLAW_OPENSHELL_PIN_VERSION:-}" ]; then - PIN_VERSION="$(apply_blueprint_override NEMOCLAW_OPENSHELL_PIN_VERSION "$NEMOCLAW_OPENSHELL_PIN_VERSION")" + if [[ "$NEMOCLAW_OPENSHELL_PIN_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + PIN_VERSION="$NEMOCLAW_OPENSHELL_PIN_VERSION" + else + fail "NEMOCLAW_OPENSHELL_PIN_VERSION='$NEMOCLAW_OPENSHELL_PIN_VERSION' is not a valid X.Y.Z version." + fi fi fi From 6c4187cf6f86ef062079f244afedd4838c9ebcea Mon Sep 17 00:00:00 2001 From: Dongni Yang Date: Wed, 13 May 2026 17:24:36 +0800 Subject: [PATCH 5/7] fix(onboard): redirect install-openshell.sh fail() output to stderr (#3404) CodeRabbit minor on #3446: the comment introduced in 18fe70508 claimed fail()'s output reaches stderr, but fail() (Line 14-17) still echoed to stdout. Today the inlined validation avoids $(...) capture, so the user does see the message, but the stderr claim itself was false. Redirect fail()'s echo to stderr (>&2) so it matches both the comment and conventional shell error reporting. The non-error path (info/warn) is untouched. The validation continues to live outside $(...) so a future caller cannot accidentally capture it. Signed-off-by: Dongni Yang Co-Authored-By: Claude Opus 4.7 (1M context) --- scripts/install-openshell.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scripts/install-openshell.sh b/scripts/install-openshell.sh index c2a98f294d7..8f3ec3ae248 100755 --- a/scripts/install-openshell.sh +++ b/scripts/install-openshell.sh @@ -12,7 +12,7 @@ NC='\033[0m' info() { echo -e "${GREEN}[install]${NC} $1"; } warn() { echo -e "${YELLOW}[install]${NC} $1"; } fail() { - echo -e "${RED}[install]${NC} $1" + echo -e "${RED}[install]${NC} $1" >&2 exit 1 } @@ -65,8 +65,9 @@ fi # (nemoclaw-blueprint/blueprint.yaml) drives the install (#3404). # # Validation is inlined (rather than wrapped in a helper that returns via -# $(...)) so that `fail`'s error message reaches the user's stderr instead of -# being captured into the variable assignment. +# $(...)) so a `fail` triggered here is not captured into the variable +# assignment. `fail` now writes to stderr (#3446 CodeRabbit), but keeping +# the validation outside of $(...) avoids relying on that. if [ "$RESOLVED_CHANNEL" != "dev" ]; then if [ -n "${NEMOCLAW_OPENSHELL_MIN_VERSION:-}" ]; then if [[ "$NEMOCLAW_OPENSHELL_MIN_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then From 2ede7d712b72667480f1f8d773d2d3ae89c0f97c Mon Sep 17 00:00:00 2001 From: Dongni Yang Date: Wed, 13 May 2026 17:44:38 +0800 Subject: [PATCH 6/7] test(onboard): assert fail() output on stderr after #3446 redirect (#3404) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI fixup. The three pre-existing version-check tests asserted that fail()'s error string appeared on stdout. Commit 6c4187cf6 redirected fail() to stderr (per CodeRabbit minor on #3446), so the assertions have to move with it. Behaviour is unchanged — only the stream the error lands on. Signed-off-by: Dongni Yang Co-Authored-By: Claude Opus 4.7 (1M context) --- test/install-openshell-version-check.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/install-openshell-version-check.test.ts b/test/install-openshell-version-check.test.ts index 87c8be6f5bb..f0c68bbd82c 100644 --- a/test/install-openshell-version-check.test.ts +++ b/test/install-openshell-version-check.test.ts @@ -140,7 +140,8 @@ describe("install-openshell.sh version check", { timeout: 15_000 }, () => { it("fails closed when openshell 0.0.39 lacks required messaging rewrite support", () => { const result = runWithInstalledVersion("0.0.39", {}, { capability: false }); expect(result.status).toBe(1); - expect(result.stdout).toMatch(/missing request-body-credential-rewrite support/); + // `fail()` writes to stderr as of #3446; previously stdout. + expect(result.stderr).toMatch(/missing request-body-credential-rewrite support/); }); it("accepts macOS openshell 0.0.39 when the gateway and VM driver binaries are installed", () => { @@ -447,13 +448,15 @@ exit 0`, it("fails with a clear error when openshell is above MAX_VERSION", () => { const result = runWithInstalledVersion("0.0.40"); expect(result.status).toBe(1); - expect(result.stdout).toMatch(/above the maximum/); + // `fail()` writes to stderr as of #3446; previously stdout. + expect(result.stderr).toMatch(/above the maximum/); }); it("fails with a clear error when openshell is at a much newer version", () => { const result = runWithInstalledVersion("0.1.0"); expect(result.status).toBe(1); - expect(result.stdout).toMatch(/above the maximum/); + // `fail()` writes to stderr as of #3446; previously stdout. + expect(result.stderr).toMatch(/above the maximum/); }); it("accepts an installed OpenShell dev-channel Docker-driver build", () => { From f0b8e38b5712641587d27710436027cfd6023672 Mon Sep 17 00:00:00 2001 From: Julie Yaunches Date: Wed, 13 May 2026 16:49:27 -0400 Subject: [PATCH 7/7] fix(onboard): reinstall pinned OpenShell above max If a host already has an OpenShell release newer than this NemoClaw release supports, reinstall the pinned compatible release instead of hard-failing before the download path. This lets the #3474 regression guard pass while preserving the post-install blueprint max gate. Related: #3474 --- scripts/install-openshell.sh | 5 ++--- test/install-openshell-version-check.test.ts | 22 +++++++++++--------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/scripts/install-openshell.sh b/scripts/install-openshell.sh index 8f3ec3ae248..4f68f5a4244 100755 --- a/scripts/install-openshell.sh +++ b/scripts/install-openshell.sh @@ -270,9 +270,8 @@ if command -v openshell >/dev/null 2>&1; then else if version_gte "$INSTALLED_VERSION" "$MIN_VERSION"; then if ! version_gte "$MAX_VERSION" "$INSTALLED_VERSION"; then - fail "openshell $INSTALLED_VERSION is above the maximum ($MAX_VERSION) supported by this NemoClaw release. Upgrade NemoClaw first." - fi - if ! required_driver_bins_present; then + warn "openshell $INSTALLED_VERSION is above the maximum ($MAX_VERSION) supported by this NemoClaw release — reinstalling pinned OpenShell ${PIN_VERSION}..." + elif ! required_driver_bins_present; then warn "openshell $INSTALLED_VERSION is missing Docker-driver binaries — reinstalling pinned OpenShell ${PIN_VERSION}..." elif ! openshell_has_required_messaging_features; then fail "${OPENSHELL_FEATURE_CHECK_ERROR:-openshell $INSTALLED_VERSION is missing required messaging credential rewrite support. Install an OpenShell build that includes provider aliases, WebSocket text rewrite, and request-body credential rewrite.}" diff --git a/test/install-openshell-version-check.test.ts b/test/install-openshell-version-check.test.ts index f0c68bbd82c..7a849905641 100644 --- a/test/install-openshell-version-check.test.ts +++ b/test/install-openshell-version-check.test.ts @@ -16,8 +16,8 @@ function writeExecutable(target: string, contents: string) { /** * Run install-openshell.sh with a fake `openshell` binary that reports the * given version. The download/install code path is never reached because we - * either exit early (version + capability ok / too high / missing capability) - * or hit the upgrade warn and then the script tries to download — so we stub + * either exit early (version + capability ok / missing capability) + * or hit an upgrade/reinstall warn and then the script tries to download — so we stub * curl and gh to fail fast. */ function runWithInstalledVersion( @@ -445,18 +445,20 @@ exit 0`, expect(result.stdout).toMatch(/below minimum.*upgrading/); }); - it("fails with a clear error when openshell is above MAX_VERSION", () => { + it("reinstalls the pinned release when openshell is above MAX_VERSION", () => { const result = runWithInstalledVersion("0.0.40"); - expect(result.status).toBe(1); - // `fail()` writes to stderr as of #3446; previously stdout. - expect(result.stderr).toMatch(/above the maximum/); + expect(result.status).not.toBe(0); + expect(result.stdout).toMatch(/above the maximum.*reinstalling pinned OpenShell 0\.0\.39/); + expect(result.stdout).toMatch(/Installing OpenShell from release 'v0\.0\.39'/); + expect(result.stderr).not.toMatch(/Upgrade NemoClaw first/); }); - it("fails with a clear error when openshell is at a much newer version", () => { + it("reinstalls the pinned release when openshell is at a much newer version", () => { const result = runWithInstalledVersion("0.1.0"); - expect(result.status).toBe(1); - // `fail()` writes to stderr as of #3446; previously stdout. - expect(result.stderr).toMatch(/above the maximum/); + expect(result.status).not.toBe(0); + expect(result.stdout).toMatch(/above the maximum.*reinstalling pinned OpenShell 0\.0\.39/); + expect(result.stdout).toMatch(/Installing OpenShell from release 'v0\.0\.39'/); + expect(result.stderr).not.toMatch(/Upgrade NemoClaw first/); }); it("accepts an installed OpenShell dev-channel Docker-driver build", () => {