Skip to content

Commit 8552e01

Browse files
kjw3kjw3
committed
fix: address onboard review follow-ups
1 parent 9c73c7f commit 8552e01

7 files changed

Lines changed: 96 additions & 29 deletions

File tree

‎bin/lib/onboard-session.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ function createSession(overrides = {}) {
4545
sandboxName: overrides.sandboxName || null,
4646
provider: overrides.provider || null,
4747
model: overrides.model || null,
48+
endpointUrl: overrides.endpointUrl || null,
49+
credentialEnv: overrides.credentialEnv || null,
50+
preferredInferenceApi: overrides.preferredInferenceApi || null,
51+
nimContainer: overrides.nimContainer || null,
4852
metadata: {
4953
gatewayName: overrides.metadata?.gatewayName || "nemoclaw",
5054
},
@@ -94,6 +98,10 @@ function normalizeSession(data) {
9498
sandboxName: typeof data.sandboxName === "string" ? data.sandboxName : null,
9599
provider: typeof data.provider === "string" ? data.provider : null,
96100
model: typeof data.model === "string" ? data.model : null,
101+
endpointUrl: typeof data.endpointUrl === "string" ? data.endpointUrl : null,
102+
credentialEnv: typeof data.credentialEnv === "string" ? data.credentialEnv : null,
103+
preferredInferenceApi: typeof data.preferredInferenceApi === "string" ? data.preferredInferenceApi : null,
104+
nimContainer: typeof data.nimContainer === "string" ? data.nimContainer : null,
97105
lastStepStarted: typeof data.lastStepStarted === "string" ? data.lastStepStarted : null,
98106
lastCompletedStep: typeof data.lastCompletedStep === "string" ? data.lastCompletedStep : null,
99107
failure: sanitizeFailure(data.failure),
@@ -218,6 +226,10 @@ function filterSafeUpdates(updates) {
218226
if (typeof updates.sandboxName === "string") safe.sandboxName = updates.sandboxName;
219227
if (typeof updates.provider === "string") safe.provider = updates.provider;
220228
if (typeof updates.model === "string") safe.model = updates.model;
229+
if (typeof updates.endpointUrl === "string") safe.endpointUrl = updates.endpointUrl;
230+
if (typeof updates.credentialEnv === "string") safe.credentialEnv = updates.credentialEnv;
231+
if (typeof updates.preferredInferenceApi === "string") safe.preferredInferenceApi = updates.preferredInferenceApi;
232+
if (typeof updates.nimContainer === "string") safe.nimContainer = updates.nimContainer;
221233
if (isObject(updates.metadata)) {
222234
safe.metadata = {};
223235
if (typeof updates.metadata.gatewayName === "string") {
@@ -240,6 +252,10 @@ function summarizeForDebug(session = loadSession()) {
240252
sandboxName: session.sandboxName,
241253
provider: session.provider,
242254
model: session.model,
255+
endpointUrl: session.endpointUrl,
256+
credentialEnv: session.credentialEnv,
257+
preferredInferenceApi: session.preferredInferenceApi,
258+
nimContainer: session.nimContainer,
243259
lastStepStarted: session.lastStepStarted,
244260
lastCompletedStep: session.lastCompletedStep,
245261
failure: session.failure,

‎bin/lib/onboard.js‎

Lines changed: 50 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -183,20 +183,24 @@ function isSandboxReady(output, sandboxName) {
183183
* @returns {boolean}
184184
*/
185185
function hasStaleGateway(gwInfoOutput) {
186-
return typeof gwInfoOutput === "string" && gwInfoOutput.length > 0 && gwInfoOutput.includes(GATEWAY_NAME);
186+
return (
187+
typeof gwInfoOutput === "string" &&
188+
gwInfoOutput.length > 0 &&
189+
gwInfoOutput.includes(`Gateway: ${GATEWAY_NAME}`) &&
190+
!gwInfoOutput.includes("No gateway metadata found")
191+
);
192+
}
193+
194+
function isSelectedGateway(statusOutput = "", gatewayName = GATEWAY_NAME) {
195+
return typeof statusOutput === "string" && statusOutput.includes(`Gateway: ${gatewayName}`);
187196
}
188197

189-
function isGatewayHealthy(statusOutput = "", gwInfoOutput = "", activeGatewayInfoOutput = "") {
198+
function isGatewayHealthy(statusOutput = "", gwInfoOutput = "", _activeGatewayInfoOutput = "") {
190199
const connected = typeof statusOutput === "string" && statusOutput.includes("Connected");
191200
if (!connected) return false;
192201

193202
const namedGatewayKnown = hasStaleGateway(gwInfoOutput);
194-
const activeGatewayKnown =
195-
typeof activeGatewayInfoOutput === "string" &&
196-
activeGatewayInfoOutput.includes("Gateway endpoint:") &&
197-
!activeGatewayInfoOutput.includes("No gateway metadata found");
198-
199-
return namedGatewayKnown || activeGatewayKnown;
203+
return namedGatewayKnown || isSelectedGateway(statusOutput);
200204
}
201205

202206
function getGatewayReuseState(statusOutput = "", gwInfoOutput = "", activeGatewayInfoOutput = "") {
@@ -1223,6 +1227,24 @@ function getRequestedModelHint(nonInteractive = isNonInteractive()) {
12231227
return getNonInteractiveModel(providerKey);
12241228
}
12251229

1230+
function getEffectiveProviderName(providerKey) {
1231+
if (!providerKey) return null;
1232+
if (REMOTE_PROVIDER_CONFIG[providerKey]) {
1233+
return REMOTE_PROVIDER_CONFIG[providerKey].providerName;
1234+
}
1235+
1236+
switch (providerKey) {
1237+
case "nim-local":
1238+
return "nvidia-nim";
1239+
case "ollama":
1240+
return "ollama-local";
1241+
case "vllm":
1242+
return "vllm-local";
1243+
default:
1244+
return providerKey;
1245+
}
1246+
}
1247+
12261248
function getResumeConfigConflicts(session, opts = {}) {
12271249
const conflicts = [];
12281250
const nonInteractive = opts.nonInteractive ?? isNonInteractive();
@@ -1237,10 +1259,11 @@ function getResumeConfigConflicts(session, opts = {}) {
12371259
}
12381260

12391261
const requestedProvider = getRequestedProviderHint(nonInteractive);
1240-
if (requestedProvider && session?.provider && requestedProvider !== session.provider) {
1262+
const effectiveRequestedProvider = getEffectiveProviderName(requestedProvider);
1263+
if (effectiveRequestedProvider && session?.provider && effectiveRequestedProvider !== session.provider) {
12411264
conflicts.push({
12421265
field: "provider",
1243-
requested: requestedProvider,
1266+
requested: effectiveRequestedProvider,
12441267
recorded: session.provider,
12451268
});
12461269
}
@@ -1562,7 +1585,7 @@ function getGatewayStartEnv() {
15621585
async function recoverGatewayRuntime() {
15631586
runOpenshell(["gateway", "select", GATEWAY_NAME], { ignoreError: true });
15641587
let status = runCaptureOpenshell(["status"], { ignoreError: true });
1565-
if (status.includes("Connected")) {
1588+
if (status.includes("Connected") && isSelectedGateway(status)) {
15661589
process.env.OPENSHELL_GATEWAY = GATEWAY_NAME;
15671590
return true;
15681591
}
@@ -1575,7 +1598,7 @@ async function recoverGatewayRuntime() {
15751598

15761599
for (let i = 0; i < 5; i++) {
15771600
status = runCaptureOpenshell(["status"], { ignoreError: true });
1578-
if (status.includes("Connected")) {
1601+
if (status.includes("Connected") && isSelectedGateway(status)) {
15791602
process.env.OPENSHELL_GATEWAY = GATEWAY_NAME;
15801603
const runtime = getContainerRuntime();
15811604
if (shouldPatchCoredns(runtime)) {
@@ -1591,9 +1614,7 @@ async function recoverGatewayRuntime() {
15911614

15921615
// ── Step 3: Sandbox ──────────────────────────────────────────────
15931616

1594-
async function createSandbox(gpu, model, provider, preferredInferenceApi = null) {
1595-
step(5, 7, "Creating sandbox");
1596-
1617+
async function promptValidatedSandboxName() {
15971618
const nameAnswer = await promptOrDefault(
15981619
" Sandbox name (lowercase, numbers, hyphens) [my-assistant]: ",
15991620
"NEMOCLAW_SANDBOX_NAME", "my-assistant"
@@ -1609,6 +1630,14 @@ async function createSandbox(gpu, model, provider, preferredInferenceApi = null)
16091630
process.exit(1);
16101631
}
16111632

1633+
return sandboxName;
1634+
}
1635+
1636+
async function createSandbox(gpu, model, provider, preferredInferenceApi = null, sandboxNameOverride = null) {
1637+
step(5, 7, "Creating sandbox");
1638+
1639+
const sandboxName = sandboxNameOverride || (await promptValidatedSandboxName());
1640+
16121641
// Reconcile local registry state with the live OpenShell gateway state.
16131642
const liveExists = pruneStaleSandboxEntry(sandboxName);
16141643

@@ -2501,9 +2530,9 @@ async function onboard(opts = {}) {
25012530
onboardSession.markStepComplete("preflight");
25022531
}
25032532

2504-
const gatewayStatus = runCapture("openshell status 2>&1", { ignoreError: true });
2505-
const gatewayInfo = runCapture("openshell gateway info -g nemoclaw 2>/dev/null", { ignoreError: true });
2506-
const activeGatewayInfo = runCapture("openshell gateway info 2>&1", { ignoreError: true });
2533+
const gatewayStatus = runCaptureOpenshell(["status"], { ignoreError: true });
2534+
const gatewayInfo = runCaptureOpenshell(["gateway", "info", "-g", GATEWAY_NAME], { ignoreError: true });
2535+
const activeGatewayInfo = runCaptureOpenshell(["gateway", "info"], { ignoreError: true });
25072536
const gatewayReuseState = getGatewayReuseState(gatewayStatus, gatewayInfo, activeGatewayInfo);
25082537
const resumeGateway =
25092538
resume &&
@@ -2585,8 +2614,9 @@ async function onboard(opts = {}) {
25852614
}
25862615
}
25872616
}
2588-
startRecordedStep("sandbox", { provider, model });
2589-
sandboxName = await createSandbox(gpu, model, provider, preferredInferenceApi);
2617+
sandboxName = sandboxName || (await promptValidatedSandboxName());
2618+
startRecordedStep("sandbox", { sandboxName, provider, model });
2619+
sandboxName = await createSandbox(gpu, model, provider, preferredInferenceApi, sandboxName);
25902620
onboardSession.markStepComplete("sandbox", { sandboxName, provider, model, nimContainer });
25912621
}
25922622

‎install.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ ensure_nvm_loaded() {
246246

247247
detect_shell_profile() {
248248
local profile="$HOME/.bashrc"
249-
if [[ -n "${ZSH_VERSION:-}" ]] || [[ "$(basename "${SHELL:-}")" == "zsh" ]]; then
249+
if [[ "$(basename "${SHELL:-}")" == "zsh" ]]; then
250250
profile="$HOME/.zshrc"
251251
elif [[ ! -f "$HOME/.bashrc" && -f "$HOME/.profile" ]]; then
252252
profile="$HOME/.profile"

‎test/e2e/test-onboard-repair.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,13 +264,13 @@ else
264264
fail "Resume exited $provider_conflict_exit for conflicting provider/model (expected 1)"
265265
fi
266266

267-
if echo "$provider_conflict_output" | grep -q "Resumable state recorded provider 'nvidia-nim', not 'cloud'."; then
267+
if echo "$provider_conflict_output" | grep -Eq "Resumable state recorded provider '.*', not '.*'\."; then
268268
pass "Conflicting provider message is explicit"
269269
else
270270
fail "Conflicting provider message missing or incorrect"
271271
fi
272272

273-
if echo "$provider_conflict_output" | grep -q "Resumable state recorded model 'nvidia/nemotron-3-super-120b-a12b', not 'nvidia/conflicting-model'."; then
273+
if echo "$provider_conflict_output" | grep -Eq "Resumable state recorded model '.*', not 'nvidia/conflicting-model'\."; then
274274
pass "Conflicting model message is explicit"
275275
else
276276
fail "Conflicting model message missing or incorrect"

‎test/e2e/test-onboard-resume.sh‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,21 @@
2222

2323
set -uo pipefail
2424

25+
if [ "${NEMOCLAW_E2E_NO_TIMEOUT:-0}" != "1" ]; then
26+
TIMEOUT_SECONDS="${TIMEOUT_SECONDS:-600}"
27+
TIMEOUT_BIN=""
28+
if command -v timeout >/dev/null 2>&1; then
29+
TIMEOUT_BIN="timeout"
30+
elif command -v gtimeout >/dev/null 2>&1; then
31+
TIMEOUT_BIN="gtimeout"
32+
fi
33+
34+
if [ -n "$TIMEOUT_BIN" ]; then
35+
export NEMOCLAW_E2E_NO_TIMEOUT=1
36+
exec "$TIMEOUT_BIN" -s TERM "$TIMEOUT_SECONDS" "$0" "$@"
37+
fi
38+
fi
39+
2540
PASS=0
2641
FAIL=0
2742
SKIP=0

‎test/onboard-session.test.js‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,16 @@ describe("onboard session", () => {
5353
expect(loaded.failure.message).toMatch(/Sandbox creation failed/);
5454
});
5555

56-
it("filters unsafe updates instead of persisting secrets", () => {
56+
it("persists safe provider metadata without persisting secrets", () => {
5757
session.saveSession(session.createSession());
5858
session.markStepComplete("provider_selection", {
5959
provider: "nvidia-nim",
6060
model: "nvidia/test-model",
6161
sandboxName: "my-assistant",
62+
endpointUrl: "https://example.com/v1",
6263
credentialEnv: "NVIDIA_API_KEY",
64+
preferredInferenceApi: "openai-completions",
65+
nimContainer: "nim-123",
6366
apiKey: "nvapi-secret",
6467
metadata: {
6568
gatewayName: "nemoclaw",
@@ -71,7 +74,10 @@ describe("onboard session", () => {
7174
expect(loaded.provider).toBe("nvidia-nim");
7275
expect(loaded.model).toBe("nvidia/test-model");
7376
expect(loaded.sandboxName).toBe("my-assistant");
74-
expect(loaded.credentialEnv).toBeUndefined();
77+
expect(loaded.endpointUrl).toBe("https://example.com/v1");
78+
expect(loaded.credentialEnv).toBe("NVIDIA_API_KEY");
79+
expect(loaded.preferredInferenceApi).toBe("openai-completions");
80+
expect(loaded.nimContainer).toBe("nim-123");
7581
expect(loaded.apiKey).toBeUndefined();
7682
expect(loaded.metadata.gatewayName).toBe("nemoclaw");
7783
expect(loaded.metadata.token).toBeUndefined();

‎test/onboard.test.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ describe("onboard helpers", () => {
210210
"Error: × No gateway metadata found for 'nemoclaw'.",
211211
"Gateway Info\n\n Gateway: openshell\n Gateway endpoint: https://127.0.0.1:8080"
212212
)
213-
).toBe(true);
213+
).toBe(false);
214214
expect(isGatewayHealthy("Gateway status: Disconnected", "Gateway: nemoclaw")).toBe(false);
215215
expect(isGatewayHealthy("Gateway status: Connected", "Gateway: something-else")).toBe(false);
216216
});
@@ -228,7 +228,7 @@ describe("onboard helpers", () => {
228228
"Error: × No gateway metadata found for 'nemoclaw'.",
229229
"Gateway Info\n\n Gateway: openshell\n Gateway endpoint: https://127.0.0.1:8080"
230230
)
231-
).toBe("healthy");
231+
).toBe("active-unnamed");
232232
expect(
233233
getGatewayReuseState(
234234
"Gateway status: Disconnected",
@@ -241,7 +241,7 @@ describe("onboard helpers", () => {
241241
"",
242242
"Gateway Info\n\n Gateway: openshell\n Gateway endpoint: https://127.0.0.1:8080"
243243
)
244-
).toBe("healthy");
244+
).toBe("active-unnamed");
245245
expect(getGatewayReuseState("", "")).toBe("missing");
246246
});
247247

@@ -364,7 +364,7 @@ describe("onboard helpers", () => {
364364
).toEqual([
365365
{
366366
field: "provider",
367-
requested: "build",
367+
requested: "nvidia-prod",
368368
recorded: "nvidia-nim",
369369
},
370370
{

0 commit comments

Comments
 (0)