diff --git a/scripts/nemoclaw-start.sh b/scripts/nemoclaw-start.sh index 663a034ff99..6f145f246f0 100755 --- a/scripts/nemoclaw-start.sh +++ b/scripts/nemoclaw-start.sh @@ -844,11 +844,24 @@ PYOVERRIDE # ── Agent identity reconciliation with provider routing ─────────── # After the host-side `openshell inference set` swaps the gateway's -# inference provider entry, agents.defaults.model.primary in -# openclaw.json can drift from models.providers..models[0].name. -# When that happens the gateway routes requests to the new model but -# the agent self-reports the old one. Realign the two on every -# sandbox start so the next session boots with a consistent identity. +# inference provider entry, agents.defaults.model.primary AND the +# in-sandbox models.providers.inference.models[0] entry can both go +# stale: openshell only updates the gateway, not /sandbox/.openclaw/ +# openclaw.json. The gateway routes requests to the new model but +# the agent self-reports the old one, and on the next gateway +# reconciliation the file's stale entry can be pushed back, reverting +# the route. +# +# Probe the live gateway via `openshell inference get --json` and +# treat it as the source of truth: when the gateway model differs +# from the file, align both primary and the inference provider's +# first model entry so the agent identity and the gateway route stay +# consistent across the next reconcile cycle. +# +# When the gateway probe is unavailable (no openshell binary, gateway +# unreachable, malformed output), fall back to the legacy in-file +# reconcile so the function still closes primary↔models[0] drift. +# # Runs after apply_model_override so explicit NEMOCLAW_MODEL_OVERRIDE # values still win. No-op when already in sync. # Ref: https://github.com/NVIDIA/NemoClaw/issues/3175 @@ -867,32 +880,85 @@ reconcile_agent_model_with_provider() { return 0 fi + local gateway_model="" + if command -v openshell >/dev/null 2>&1; then + gateway_model="$( + python3 - <<'PYPROBE' +import json, subprocess +try: + result = subprocess.run( + ["openshell", "inference", "get", "--json"], + capture_output=True, + timeout=3, + check=False, + ) +except Exception: + raise SystemExit(0) +if result.returncode != 0: + raise SystemExit(0) +try: + data = json.loads(result.stdout) +except Exception: + raise SystemExit(0) +model = data.get("model") if isinstance(data, dict) else None +if isinstance(model, str) and model: + print(model) +PYPROBE + )" + fi + local provider_model_ref provider_model_ref="$( - python3 - "$config_file" <<'PYRECONCILE_READ' -import json, sys + GATEWAY_MODEL="${gateway_model:-}" python3 - "$config_file" <<'PYRECONCILE_READ' +import json, os, sys + try: with open(sys.argv[1]) as f: cfg = json.load(f) except Exception: sys.exit(0) + primary = cfg.get("agents", {}).get("defaults", {}).get("model", {}).get("primary") provider = cfg.get("models", {}).get("providers", {}).get("inference", {}) models = provider.get("models") if isinstance(provider, dict) else None -if not isinstance(models, list) or not models: +first = ( + models[0] + if isinstance(models, list) and models and isinstance(models[0], dict) + else None +) + + +def qualify(model_id): + if not isinstance(model_id, str) or not model_id: + return None + return model_id if model_id.startswith("inference/") else f"inference/{model_id}" + + +gateway_target = qualify(os.environ.get("GATEWAY_MODEL", "")) +if gateway_target is not None: + bare = gateway_target[len("inference/"):] + first_name = first.get("name") if first is not None else None + first_id = first.get("id") if first is not None else None + primary_ok = isinstance(primary, str) and primary == gateway_target + first_name_ok = isinstance(first_name, str) and first_name == gateway_target + first_id_ok = isinstance(first_id, str) and (first_id == bare or first_id == gateway_target) + if primary_ok and first_name_ok and first_id_ok: + sys.exit(0) + print(f"gateway\t{gateway_target}") sys.exit(0) -first = models[0] -if not isinstance(first, dict): + +# Legacy fallback: gateway probe is unavailable. Align primary with +# the in-file provider entry only (models[0] is treated as the +# source). Preserves pre-gateway-probe behavior for environments +# without openshell. +if first is None: sys.exit(0) -provider_ref = first.get("name") -if not isinstance(provider_ref, str) or not provider_ref: - provider_id = first.get("id") - if not isinstance(provider_id, str) or not provider_id: - sys.exit(0) - provider_ref = provider_id if provider_id.startswith("inference/") else f"inference/{provider_id}" -if not isinstance(primary, str) or primary == provider_ref: +legacy_target = qualify(first.get("name") or first.get("id")) +if legacy_target is None: sys.exit(0) -print(provider_ref) +if isinstance(primary, str) and primary == legacy_target: + sys.exit(0) +print(f"legacy\t{legacy_target}") PYRECONCILE_READ )" @@ -900,17 +966,40 @@ PYRECONCILE_READ return 0 fi - printf '[config] Reconciling agent identity with provider model: %s (#3175)\n' "$provider_model_ref" >&2 + local source_mode="${provider_model_ref%%$'\t'*}" + provider_model_ref="${provider_model_ref#*$'\t'}" + + printf '[config] Reconciling agent identity with provider model: %s (source=%s, #3175)\n' \ + "$provider_model_ref" "$source_mode" >&2 prepare_openclaw_config_for_write "$config_file" "$hash_file" local _write_rc=0 - python3 - "$config_file" "$provider_model_ref" <<'PYRECONCILE_WRITE' || _write_rc=$? -import json, sys + RECONCILE_SOURCE="$source_mode" python3 - "$config_file" "$provider_model_ref" <<'PYRECONCILE_WRITE' || _write_rc=$? +import json, os, sys config_file, provider_model = sys.argv[1], sys.argv[2] with open(config_file) as f: cfg = json.load(f) cfg.setdefault("agents", {}).setdefault("defaults", {}).setdefault("model", {})["primary"] = provider_model +if os.environ.get("RECONCILE_SOURCE") == "gateway": + bare = ( + provider_model[len("inference/"):] + if provider_model.startswith("inference/") + else provider_model + ) + models_root = cfg.setdefault("models", {}) + providers_root = models_root.setdefault("providers", {}) + inference = providers_root.setdefault("inference", {}) + models_list = inference.get("models") + if not isinstance(models_list, list) or not models_list: + models_list = [{}] + inference["models"] = models_list + first = models_list[0] + if not isinstance(first, dict): + first = {} + models_list[0] = first + first["id"] = bare + first["name"] = provider_model with open(config_file, "w") as f: json.dump(cfg, f, indent=2) PYRECONCILE_WRITE diff --git a/test/nemoclaw-start-reconcile.test.ts b/test/nemoclaw-start-reconcile.test.ts index 9d8347651fb..cd216f41bc9 100644 --- a/test/nemoclaw-start-reconcile.test.ts +++ b/test/nemoclaw-start-reconcile.test.ts @@ -9,6 +9,24 @@ import { describe, expect, it } from "vitest"; const START_SCRIPT = path.join(import.meta.dirname, "..", "scripts", "nemoclaw-start.sh"); +interface RunReconcileOptions { + /** + * Output the stubbed `openshell inference get --json` should print. + * - undefined → no openshell on PATH (probe falls back to in-file logic). + * - "" → openshell exists but returns empty JSON (probe yields no model). + * - non-empty string → openshell returns `{"model": }`. + * Ignored when `gatewayRawOutput` is set. + */ + gatewayModel?: string; + /** + * Raw stdout the stub emits instead of a JSON-formatted payload. Use to + * exercise malformed-JSON or unexpected-shape paths. Takes precedence + * over `gatewayModel` when both are set. + */ + gatewayRawOutput?: string; + env?: Record; +} + describe("agent identity reconciliation with provider (#3175)", () => { const src = fs.readFileSync(START_SCRIPT, "utf-8"); @@ -20,7 +38,7 @@ describe("agent identity reconciliation with provider (#3175)", () => { return `${name}() {${match[1]}\n}`; } - function runReconcile(initialConfig: unknown, env: Record = {}) { + function runReconcile(initialConfig: unknown, options: RunReconcileOptions = {}) { const root = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-reconcile-")); const openclawDir = path.join(root, ".openclaw"); fs.mkdirSync(openclawDir, { recursive: true }); @@ -32,6 +50,28 @@ describe("agent identity reconciliation with provider (#3175)", () => { fs.chmodSync(configPath, 0o660); fs.chmodSync(hashPath, 0o660); + const binDir = path.join(root, "bin"); + fs.mkdirSync(binDir); + const installStub = options.gatewayRawOutput !== undefined || options.gatewayModel !== undefined; + if (installStub) { + const payload = + options.gatewayRawOutput !== undefined + ? options.gatewayRawOutput + : options.gatewayModel === "" + ? "{}" + : JSON.stringify({ model: options.gatewayModel }); + const stub = [ + "#!/usr/bin/env bash", + 'if [ "$1" = "inference" ] && [ "$2" = "get" ]; then', + ` printf '%s' ${JSON.stringify(payload)}`, + " exit 0", + "fi", + "exit 1", + "", + ].join("\n"); + fs.writeFileSync(path.join(binDir, "openshell"), stub, { mode: 0o755 }); + } + const helperFns = [ extractShellFunction("openclaw_config_dir_owner"), extractShellFunction("prepare_openclaw_config_for_write"), @@ -57,9 +97,28 @@ describe("agent identity reconciliation with provider (#3175)", () => { ].join("\n"); const script = path.join(root, "run.sh"); fs.writeFileSync(script, wrapper, { mode: 0o700 }); + // Build PATH: when the test installs an openshell stub, prepend its + // bin dir; otherwise scrub openshell from the inherited PATH so the + // probe deterministically reports "not installed". + const inheritedPath = process.env.PATH ?? "/usr/bin:/bin"; + const scrubbedPath = inheritedPath + .split(path.delimiter) + .filter((dir) => { + if (!dir) return false; + try { + fs.accessSync(path.join(dir, "openshell"), fs.constants.X_OK); + return false; + } catch { + return true; + } + }) + .join(path.delimiter); + const pathValue = installStub + ? `${binDir}${path.delimiter}${scrubbedPath}` + : scrubbedPath; const result = spawnSync("bash", [script], { encoding: "utf-8", - env: { ...process.env, ...env }, + env: { ...process.env, ...options.env, PATH: pathValue }, }); const config = JSON.parse(fs.readFileSync(configPath, "utf-8")); const hash = fs.readFileSync(hashPath, "utf-8"); @@ -141,4 +200,132 @@ describe("agent identity reconciliation with provider (#3175)", () => { expect(config).toEqual({ unrelated: true }); expect(hash).toBe("oldhash\n"); }); + + // ── Gateway-as-source-of-truth path (the #3175 user-reported repro) ── + + it("patches primary AND models[0] to the live gateway model when both file fields are stale", () => { + const { result, config, hash } = runReconcile( + { + agents: { defaults: { model: { primary: "inference/nvidia-routed" } } }, + models: { + providers: { + inference: { + api: "openai-completions", + models: [{ id: "nvidia-routed", name: "inference/nvidia-routed" }], + }, + }, + }, + }, + { gatewayModel: "nvidia/nemotron-3-super-120b-a12b" }, + ); + + expect(result.status).toBe(0); + expect(config.agents.defaults.model.primary).toBe( + "inference/nvidia/nemotron-3-super-120b-a12b", + ); + expect(config.models.providers.inference.models[0].name).toBe( + "inference/nvidia/nemotron-3-super-120b-a12b", + ); + expect(config.models.providers.inference.models[0].id).toBe( + "nvidia/nemotron-3-super-120b-a12b", + ); + expect(hash).not.toBe("oldhash\n"); + expect(hash).toContain("openclaw.json"); + }); + + it("accepts an inference-qualified gateway model without double-prefixing", () => { + const { result, config } = runReconcile( + { + agents: { defaults: { model: { primary: "inference/nvidia-routed" } } }, + models: { + providers: { + inference: { + api: "openai-completions", + models: [{ id: "nvidia-routed", name: "inference/nvidia-routed" }], + }, + }, + }, + }, + { gatewayModel: "inference/nvidia/nemotron-3-super-120b-a12b" }, + ); + + expect(result.status).toBe(0); + expect(config.agents.defaults.model.primary).toBe( + "inference/nvidia/nemotron-3-super-120b-a12b", + ); + expect(config.models.providers.inference.models[0].id).toBe( + "nvidia/nemotron-3-super-120b-a12b", + ); + }); + + it("is a no-op when the live gateway model matches both file fields", () => { + const { result, config, hash } = runReconcile( + { + agents: { defaults: { model: { primary: "inference/nvidia/synced" } } }, + models: { + providers: { + inference: { + api: "openai-completions", + models: [{ id: "nvidia/synced", name: "inference/nvidia/synced" }], + }, + }, + }, + }, + { gatewayModel: "nvidia/synced" }, + ); + + expect(result.status).toBe(0); + expect(config.agents.defaults.model.primary).toBe("inference/nvidia/synced"); + expect(hash).toBe("oldhash\n"); + }); + + it("falls back to the in-file reconcile when the gateway probe returns no model", () => { + const { result, config } = runReconcile( + { + agents: { defaults: { model: { primary: "inference/old-model" } } }, + models: { + providers: { + inference: { + api: "openai-completions", + models: [{ id: "nvidia/new-model", name: "inference/nvidia/new-model" }], + }, + }, + }, + }, + { gatewayModel: "" }, + ); + + expect(result.status).toBe(0); + expect(config.agents.defaults.model.primary).toBe("inference/nvidia/new-model"); + // models[0] is untouched in legacy-fallback mode. + expect(config.models.providers.inference.models[0].id).toBe("nvidia/new-model"); + }); + + it("falls back to the in-file reconcile when the gateway probe emits malformed JSON", () => { + // A future packaging shift could ship an `openshell` shim that doesn't + // implement `inference get --json` and returns junk on stdout. The + // current absorb-via-SystemExit(0) path should still leave the user + // in the legacy in-file reconcile state — pinning this so a refactor + // of the probe parser can't silently degrade to "do nothing". + const { result, config } = runReconcile( + { + agents: { defaults: { model: { primary: "inference/old-model" } } }, + models: { + providers: { + inference: { + api: "openai-completions", + models: [{ id: "nvidia/new-model", name: "inference/nvidia/new-model" }], + }, + }, + }, + }, + { gatewayRawOutput: "not json at all" }, + ); + + expect(result.status).toBe(0); + // Legacy in-file path runs: primary is aligned to the file's first + // model, models[0] stays untouched (same shape as the empty-probe case). + expect(config.agents.defaults.model.primary).toBe("inference/nvidia/new-model"); + expect(config.models.providers.inference.models[0].id).toBe("nvidia/new-model"); + }); });