From 3cca5194fb076f5a46e7c18993d6187c2a948066 Mon Sep 17 00:00:00 2001 From: Yanyun Liao Date: Thu, 30 Jul 2026 16:08:56 +0800 Subject: [PATCH 1/2] fix(onboard): fall back on a negative timeout or poll override `envInt` backs the poll counts, poll intervals and readiness budgets used across onboarding and gateway recovery. It clamped a negative override to 0 while falling back to the caller's default for every other unusable value, so the same function answered two kinds of invalid input in opposite ways -- and picked the most damaging reading for one of them: 0 empties a poll loop, zeroes a readiness budget, and turns NEMOCLAW_GATEWAY_START_TIMEOUT into a timeout that expires before its first attempt. Treat a negative the way the sibling readNonNegativeNumberEnv already does and fall back. An explicit 0 is unchanged, so callers that read it as "disabled" or clamp it upward themselves keep their meaning; only input that was never valid changes. One caller already compensated with a local Math.max(1, ...); the rest inherited the hazard. Refs #7881 Signed-off-by: Yanyun Liao --- .../configure-inference-timeouts.mdx | 3 + src/lib/onboard/env-int.test.ts | 63 +++++++++++++++++++ src/lib/onboard/env.ts | 9 ++- .../onboard/sandbox-readiness-tracing.test.ts | 10 ++- 4 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 src/lib/onboard/env-int.test.ts diff --git a/docs/inference/configure-inference-timeouts.mdx b/docs/inference/configure-inference-timeouts.mdx index 643b65efc1..ef08e3bdeb 100644 --- a/docs/inference/configure-inference-timeouts.mdx +++ b/docs/inference/configure-inference-timeouts.mdx @@ -24,6 +24,9 @@ Use the error location to select the correct setting. The readiness timeout does not govern inference requests or provider validation. +Set each value to a whole number of seconds; a fractional value is rounded. +NemoClaw falls back to the default shown above for any value it cannot use, including a negative one — a negative budget is treated as an invalid setting rather than as a request to give up immediately. + ## Increase the OpenClaw Request Timeout diff --git a/src/lib/onboard/env-int.test.ts b/src/lib/onboard/env-int.test.ts new file mode 100644 index 0000000000..0f24f08c7c --- /dev/null +++ b/src/lib/onboard/env-int.test.ts @@ -0,0 +1,63 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// `envInt` backs the poll counts, poll intervals and readiness budgets used +// across onboarding and gateway recovery. A negative override used to be +// clamped to 0, which is the most damaging reading available for every one of +// those knobs, while any other invalid value already fell back (#7881). + +import { describe, expect, it } from "vitest"; + +import { envInt } from "./env"; + +const FALLBACK = 30; + +function read(value: string | undefined): number { + return envInt("NEMOCLAW_TEST_KNOB", FALLBACK, { NEMOCLAW_TEST_KNOB: value }); +} + +describe("envInt override parsing", () => { + it.each([ + "-1", + "-30", + "-0.4", + "-1e3", + ])("falls back instead of collapsing a negative override (%s) to zero", (value) => { + expect(read(value)).toBe(FALLBACK); + }); + + it.each([ + "abc", + "NaN", + "Infinity", + "-Infinity", + ])("keeps falling back for a non-finite override (%s)", (value) => { + expect(read(value)).toBe(FALLBACK); + }); + + it.each([ + ["unset", undefined], + ["empty", ""], + ])("keeps falling back for an %s override", (_label, value) => { + expect(read(value)).toBe(FALLBACK); + }); + + it("still accepts an explicit zero", () => { + // Regression lock: callers that read 0 as "disabled" or clamp it upward + // themselves must keep seeing 0, so this fix cannot change their meaning. + expect(read("0")).toBe(0); + }); + + it.each([ + ["3", 3], + ["0.4", 0], + ["2.6", 3], + ["600", 600], + ])("keeps rounding a valid override (%s)", (value, expected) => { + expect(read(value)).toBe(expected); + }); + + it("uses the supplied env map rather than the process environment", () => { + expect(envInt("NEMOCLAW_TEST_KNOB", FALLBACK, {})).toBe(FALLBACK); + }); +}); diff --git a/src/lib/onboard/env.ts b/src/lib/onboard/env.ts index d59bbb3fda..0466ce4039 100644 --- a/src/lib/onboard/env.ts +++ b/src/lib/onboard/env.ts @@ -10,7 +10,14 @@ export function envInt( const raw = env[name]; if (raw === undefined || raw === "") return fallback; const n = Number(raw); - return Number.isFinite(n) ? Math.max(0, Math.round(n)) : fallback; + // A negative override is invalid input, not a request for zero. Clamping it + // to 0 picked the most damaging reading available for these knobs -- an + // empty poll loop, a zero-second readiness budget, a timeout that expires + // before its first attempt -- while any other unparseable value already fell + // back to the caller's default. Treat both the same way, which is what the + // sibling `readNonNegativeNumberEnv` has always done (#7881). + if (!Number.isFinite(n) || n < 0) return fallback; + return Math.round(n); } /** Inference timeout (seconds) for local providers (Ollama, vLLM, NIM). */ diff --git a/src/lib/onboard/sandbox-readiness-tracing.test.ts b/src/lib/onboard/sandbox-readiness-tracing.test.ts index 3c74ffaa53..452ef12bc6 100644 --- a/src/lib/onboard/sandbox-readiness-tracing.test.ts +++ b/src/lib/onboard/sandbox-readiness-tracing.test.ts @@ -404,13 +404,21 @@ describe("getSandboxReadyErrorDebouncePolls env contract", () => { it("clamps to a minimum of 1 poll", () => { expect(getSandboxReadyErrorDebouncePolls({ [SANDBOX_READY_ERROR_DEBOUNCE_ENV]: "0" })).toBe(1); - expect(getSandboxReadyErrorDebouncePolls({ [SANDBOX_READY_ERROR_DEBOUNCE_ENV]: "-5" })).toBe(1); // envInt rounds 0.4 -> 0, then the clamp lifts it to 1. expect(getSandboxReadyErrorDebouncePolls({ [SANDBOX_READY_ERROR_DEBOUNCE_ENV]: "0.4" })).toBe( 1, ); }); + it("falls back for a negative override instead of clamping it to the minimum", () => { + // A negative is invalid input, so it reaches the documented default the + // same way "abc" does above, rather than silently becoming the smallest + // legal debounce (#7881). + expect(getSandboxReadyErrorDebouncePolls({ [SANDBOX_READY_ERROR_DEBOUNCE_ENV]: "-5" })).toBe( + 30, + ); + }); + it("rounds fractional env values (envInt semantics)", () => { expect(getSandboxReadyErrorDebouncePolls({ [SANDBOX_READY_ERROR_DEBOUNCE_ENV]: "2.6" })).toBe( 3, From b7055880323b0a352537bd0f567d0a74ed4e3ee2 Mon Sep 17 00:00:00 2001 From: Yanyun Liao Date: Thu, 30 Jul 2026 16:55:46 +0800 Subject: [PATCH 2/2] test(onboard): prove the supplied env map wins over the process environment The custom-environment case asserted the fallback while passing an empty map. That passes even if `envInt` ignores the supplied map and reads `process.env`, because the key is absent there too, so the assertion proved nothing about the precedence its title claims. Stub a conflicting process value, then assert both directions: a supplied value is read, and an absent key falls back rather than picking up the process value. Teardown runs in a `finally` so a failing expectation cannot leak the stub. Verified by mutation: rewriting `envInt` to read `process.env[name]` now fails this test, which it did not before. Signed-off-by: Yanyun Liao --- src/lib/onboard/env-int.test.ts | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/lib/onboard/env-int.test.ts b/src/lib/onboard/env-int.test.ts index 0f24f08c7c..4ffce04e15 100644 --- a/src/lib/onboard/env-int.test.ts +++ b/src/lib/onboard/env-int.test.ts @@ -6,7 +6,7 @@ // clamped to 0, which is the most damaging reading available for every one of // those knobs, while any other invalid value already fell back (#7881). -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { envInt } from "./env"; @@ -58,6 +58,15 @@ describe("envInt override parsing", () => { }); it("uses the supplied env map rather than the process environment", () => { - expect(envInt("NEMOCLAW_TEST_KNOB", FALLBACK, {})).toBe(FALLBACK); + // Both assertions need a conflicting process value to have any teeth: with + // `process.env` unset, an implementation that ignored the supplied map + // would return the fallback here and still pass. + vi.stubEnv("NEMOCLAW_TEST_KNOB", "999"); + try { + expect(envInt("NEMOCLAW_TEST_KNOB", FALLBACK, { NEMOCLAW_TEST_KNOB: "7" })).toBe(7); + expect(envInt("NEMOCLAW_TEST_KNOB", FALLBACK, {})).toBe(FALLBACK); + } finally { + vi.unstubAllEnvs(); + } }); });