fix: restore NVIDIA_API_KEY inference wiring#5390
Open
jyaunches wants to merge 3 commits into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughStandardize NVIDIA credential to ChangesNVIDIA API key migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Contributor
|
🌿 Preview your docs: https://nvidia-preview-pr-5390.docs.buildwithfern.com/nemoclaw |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the branch is 96%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the branch is 44%. Coverage data for the branch is not yet available. Show a code coverage summary of the most covered files.
Updated |
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
Vitest E2E Scenario RecommendationRequired Vitest E2E scenarios: Dispatch required Vitest E2E scenarios:
Full Vitest E2E advisor summaryVitest E2E Scenario AdvisorBase: Required Vitest E2E scenarios
Optional Vitest E2E scenarios
Relevant changed files
|
Contributor
Selective E2E Results — ❌ Some jobs failedRun: 27472338696
|
Contributor
PR Review AdvisorFindings: 2 needs attention, 3 worth checking, 0 nice ideas Review findings🛠️ Needs attention
🔎 Worth checking
🌱 Nice ideas
Consider writing more tests for
Since last review detailsCurrent findings:
This is an automated advisory review. A human maintainer must make the final merge decision. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/lib/credentials/store.ts (1)
204-213:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
NVIDIA_INFERENCE_API_KEYas a read-only compatibility alias during the revert.
resolveProviderCredential("NVIDIA_API_KEY")now only checksNVIDIA_API_KEY, and the legacy-file staging path only rehydrates allowlisted current names. Any resumed or non-interactive install that still has onlyNVIDIA_INFERENCE_API_KEYstaged in its environment or legacycredentials.jsonnow resolves as missing instead of transparently recovering, even though the secret is still present under the just-reverted name.Please preserve the old name as a temporary read-only alias in this resolver and in the legacy staging allowlist until the rollback window is over.
src/lib/onboard.ts (1)
3999-4016:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not use
NVIDIA_API_KEYas the fallback NGC credential.This branch is still resolving
ngcApiKey, and the surrounding flow explicitly treats that as the secret used to pull NIM model artifacts. Falling back toNVIDIA_API_KEYhere skips the interactive NGC prompt when a cloud inference key is present, then passes the wrong credential intostartNimContainerByName(), so the user-selected local NIM path can fail and silently degrade to cloud inference.Suggested fix
- ngcApiKey = - hydrateCredentialEnv("NGC_API_KEY") || hydrateCredentialEnv("NVIDIA_API_KEY"); + ngcApiKey = hydrateCredentialEnv("NGC_API_KEY");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/onboard.ts` around lines 3999 - 4016, The code incorrectly falls back to hydrateCredentialEnv("NVIDIA_API_KEY") when resolving ngcApiKey, which can cause the wrong credential to be used for NIM pulls; change the resolution to only use hydrateCredentialEnv("NGC_API_KEY") (i.e., set ngcApiKey = hydrateCredentialEnv("NGC_API_KEY") without the || fallback), leaving the interactive prompt logic (credentialPrompt.readValue, credentialPrompt.returningToProviderSelection, isNonInteractive) and subsequent use in startNimContainerByName unchanged so that only a true NGC key is accepted and passed through.test/e2e/test-model-router-provider-routed-inference.sh (1)
68-75:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the standardized NVIDIA redaction placeholder and token-pattern redaction.
The redaction path still writes a generic
<REDACTED>and only replaces exact env-secret matches. The documented contract requires[REDACTED_NVIDIA_API_KEY]andnvapi-...token-pattern redaction in failure logs.Suggested patch
- python3 - "$file" <<'PY' -import os, sys + python3 - "$file" <<'PY' +import os, re, sys path = sys.argv[1] secrets = [os.environ.get("NVIDIA_API_KEY", ""), os.environ.get("NEMOCLAW_PROVIDER_KEY", "")] text = open(path, "r", errors="replace").read() for secret in filter(None, secrets): - text = text.replace(secret, "<REDACTED>") + text = text.replace(secret, "[REDACTED_NVIDIA_API_KEY]") +text = re.sub(r"nvapi-[A-Za-z0-9_-]+", "[REDACTED_NVIDIA_API_KEY]", text) open(path, "w").write(text) PYAs per coding guidelines, failure-log redaction must use
NVIDIA_API_KEYwith[REDACTED_NVIDIA_API_KEY], including regex redaction fornvapi-....🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/test-model-router-provider-routed-inference.sh` around lines 68 - 75, The script currently replaces exact env-secret values and writes a generic "<REDACTED>" placeholder; change it so NVIDIA_API_KEY is redacted using the standardized placeholder "[REDACTED_NVIDIA_API_KEY]" and also redact any token patterns matching the nvapi prefix (e.g. regex like r"nvapi-[A-Za-z0-9_-]+") anywhere in the file; keep replacing exact NEMOCLAW_PROVIDER_KEY value as before (or use a distinct placeholder if required) and ensure the code updates the variable secrets/text replacement logic to perform both exact-value replacement for env vars and regex-based replacement for nvapi tokens before writing the file.Source: Coding guidelines
🧹 Nitpick comments (2)
test/e2e-scenario/live/sandbox-operations.test.ts (1)
101-104: ⚡ Quick winThread the required secret value explicitly instead of re-reading ambient
process.env.You already gate on
secrets.required("NVIDIA_API_KEY")at Line 531, butonboardSandboxstill pulls fromprocess.env. Passing the returned secret intoonboardSandboxkeeps this test hermetic and avoids hidden env coupling.Also applies to: 531-531
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e-scenario/live/sandbox-operations.test.ts` around lines 101 - 104, The test currently re-reads process.env for the NVIDIA API key inside the onboardSandbox call and redactionValues, creating hidden env coupling; instead, capture the secret returned by secrets.required("NVIDIA_API_KEY") (e.g., const nvidiaApiKey = await secrets.required("NVIDIA_API_KEY")) and pass that value into onboardSandbox's env (set NVIDIA_API_KEY: nvidiaApiKey) and into redactionValues ([nvidiaApiKey]) so the test is hermetic and no longer reads process.env directly.scripts/nemoclaw-start.sh (1)
1744-1767: Run the sandbox entrypoint E2E lanes for this credential-wiring revert.Please run
sandbox-survival-e2e,sandbox-operations-e2e,cloud-e2e, andopenclaw-slack-pairing-e2eon this branch to validate startup, recovery, and channel behavior after restoringNVIDIA_API_KEYwiring.As per coding guidelines, changes in
scripts/nemoclaw-start.shaffect every sandbox boot and should be validated with the listed E2E jobs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/nemoclaw-start.sh` around lines 1744 - 1767, Summary: This change restores NVIDIA_API_KEY credential wiring in the startup script and needs full sandbox E2E validation. Run the specified E2E pipelines—sandbox-survival-e2e, sandbox-operations-e2e, cloud-e2e, and openclaw-slack-pairing-e2e—against this branch, focusing on startup, recovery, and channel behavior after restoring the NVIDIA_API_KEY/NEMOCLAW_PROVIDER_KEY wiring (verify the provider_key / auth-profiles.json creation logic in the startup script and the Python snippet that writes the {provider_key}:manual api_key entry); report any failures or deviations.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/inference/inference-options.mdx`:
- Around line 112-117: Router pool examples in the docs use the wrong api_base;
update the router pool entries that set api_base:
"https://inference-api.nvidia.com" to use "https://integrate.api.nvidia.com/v1"
so they match the NemoClaw endpoint described earlier and the repo's router pool
config (pool-config.yaml) and allowlist in scripts/validate-configs.ts;
specifically change the api_base value in the router pool examples (the blocks
that include name: super and the surrounding router pool entries) to the
integrate.api.nvidia.com/v1 URL.
In `@scripts/walkthrough.sh`:
- Around line 39-41: Remove the stale in-sandbox API key instruction by deleting
the line that exports the NVIDIA_API_KEY environment variable (the line
containing export NVIDIA_API_KEY=nvapi-...) so the walkthrough no longer tells
operators to set a secret inside the sandbox; leave the surrounding sandbox
commands (e.g., openshell sandbox connect nemoclaw and nemoclaw-start) intact
and do not add any other secret-handling steps.
In `@test/e2e/test-telegram-injection.sh`:
- Around line 290-297: The test currently looks for the literal substring
"nvapi-" in t4_result which can false-negative; update the leak-detection to
check for the actual expanded secret value and the literal placeholder: if echo
"$t4_result" | grep -qF "$NVIDIA_API_KEY"; then fail "T4: \${NVIDIA_API_KEY}
expanded to actual key value — secret leaked!"; elif echo "$t4_result" | grep
-qF '${NVIDIA_API_KEY}'; then pass "T4: \${NVIDIA_API_KEY} treated as literal
string (not expanded)"; else pass "T4: \${NVIDIA_API_KEY} did not expand to key
value (result: ${t4_result:0:100})" — ensure both grep uses are quoted and use
-qF for fixed-string matching.
---
Outside diff comments:
In `@src/lib/onboard.ts`:
- Around line 3999-4016: The code incorrectly falls back to
hydrateCredentialEnv("NVIDIA_API_KEY") when resolving ngcApiKey, which can cause
the wrong credential to be used for NIM pulls; change the resolution to only use
hydrateCredentialEnv("NGC_API_KEY") (i.e., set ngcApiKey =
hydrateCredentialEnv("NGC_API_KEY") without the || fallback), leaving the
interactive prompt logic (credentialPrompt.readValue,
credentialPrompt.returningToProviderSelection, isNonInteractive) and subsequent
use in startNimContainerByName unchanged so that only a true NGC key is accepted
and passed through.
In `@test/e2e/test-model-router-provider-routed-inference.sh`:
- Around line 68-75: The script currently replaces exact env-secret values and
writes a generic "<REDACTED>" placeholder; change it so NVIDIA_API_KEY is
redacted using the standardized placeholder "[REDACTED_NVIDIA_API_KEY]" and also
redact any token patterns matching the nvapi prefix (e.g. regex like
r"nvapi-[A-Za-z0-9_-]+") anywhere in the file; keep replacing exact
NEMOCLAW_PROVIDER_KEY value as before (or use a distinct placeholder if
required) and ensure the code updates the variable secrets/text replacement
logic to perform both exact-value replacement for env vars and regex-based
replacement for nvapi tokens before writing the file.
---
Nitpick comments:
In `@scripts/nemoclaw-start.sh`:
- Around line 1744-1767: Summary: This change restores NVIDIA_API_KEY credential
wiring in the startup script and needs full sandbox E2E validation. Run the
specified E2E pipelines—sandbox-survival-e2e, sandbox-operations-e2e, cloud-e2e,
and openclaw-slack-pairing-e2e—against this branch, focusing on startup,
recovery, and channel behavior after restoring the
NVIDIA_API_KEY/NEMOCLAW_PROVIDER_KEY wiring (verify the provider_key /
auth-profiles.json creation logic in the startup script and the Python snippet
that writes the {provider_key}:manual api_key entry); report any failures or
deviations.
In `@test/e2e-scenario/live/sandbox-operations.test.ts`:
- Around line 101-104: The test currently re-reads process.env for the NVIDIA
API key inside the onboardSandbox call and redactionValues, creating hidden env
coupling; instead, capture the secret returned by
secrets.required("NVIDIA_API_KEY") (e.g., const nvidiaApiKey = await
secrets.required("NVIDIA_API_KEY")) and pass that value into onboardSandbox's
env (set NVIDIA_API_KEY: nvidiaApiKey) and into redactionValues ([nvidiaApiKey])
so the test is hermetic and no longer reads process.env directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b2e40073-e8dd-459a-8c4b-414faa4f851c
📒 Files selected for processing (217)
.github/workflows/brev-nightly-e2e.yaml.github/workflows/e2e-branch-validation.yaml.github/workflows/e2e-script.yaml.github/workflows/e2e-vitest-scenarios.yaml.github/workflows/macos-e2e.yaml.github/workflows/nightly-e2e.yaml.github/workflows/regression-e2e.yaml.github/workflows/wsl-e2e.yamlagents/hermes/policy-additions.yamlagents/hermes/policy-permissive.yamlagents/openclaw/policy-permissive.yamldocs/_components/StarterPromptButton.tsxdocs/about/release-notes.mdxdocs/get-started/quickstart-hermes.mdxdocs/get-started/quickstart.mdxdocs/inference/inference-options.mdxdocs/network-policy/approve-network-requests.mdxdocs/reference/network-policies.mdxdocs/security/credential-storage.mdxnemoclaw-blueprint/blueprint.yamlnemoclaw-blueprint/policies/openclaw-sandbox-permissive.yamlnemoclaw-blueprint/policies/openclaw-sandbox.yamlnemoclaw/src/blueprint/runner.test.tsnemoclaw/src/commands/config-show.test.tsnemoclaw/src/commands/slash.test.tsnemoclaw/src/index.tsnemoclaw/src/lib/subprocess-env.tsnemoclaw/src/onboard/config.test.tsnemoclaw/src/register.test.tsnemoclaw/src/security/secret-scanner.test.tsscripts/checks/direct-credential-env.tsscripts/install.shscripts/nemoclaw-start.shscripts/smoke-macos-install.shscripts/walkthrough.shsrc/commands/sandbox/config/rotate-token.tssrc/lib/actions/dev/npm-link-or-shim.test.tssrc/lib/credentials/store.tssrc/lib/deploy/index.test.tssrc/lib/deploy/index.tssrc/lib/diagnostics/debug.test.tssrc/lib/inference/health.test.tssrc/lib/inference/health.tssrc/lib/inference/model-prompts.test.tssrc/lib/inference/model-prompts.tssrc/lib/inference/nim.test.tssrc/lib/inference/nim.tssrc/lib/messaging-channel-config.test.tssrc/lib/onboard.tssrc/lib/onboard/docker-gpu-patch.test.tssrc/lib/onboard/machine/core-flow-phases.test.tssrc/lib/onboard/machine/flow-phases/provider-sandbox.test.tssrc/lib/onboard/machine/handlers/finalization.test.tssrc/lib/onboard/machine/handlers/policies.test.tssrc/lib/onboard/machine/handlers/provider-inference.test.tssrc/lib/onboard/machine/runtime.test.tssrc/lib/onboard/missing-credential-hints.tssrc/lib/onboard/model-router.tssrc/lib/onboard/providers.test.tssrc/lib/onboard/providers.tssrc/lib/onboard/routed-inference.test.tssrc/lib/onboard/routed-inference.tssrc/lib/onboard/summary.test.tssrc/lib/onboard/validation-recovery-prompt.tssrc/lib/security/redact.test.tssrc/lib/security/redact.tssrc/lib/state/onboard-session.test.tssrc/lib/state/onboard-step-mutation.test.tssrc/lib/subprocess-env.tssrc/lib/trace.test.tssrc/lib/validation.test.tssrc/lib/validation.tstest/canonical-credential-resolution.test.tstest/check-env-var-docs.test.tstest/cli/dispatch-basics.test.tstest/config-set-nested-ssrf.test.tstest/credential-exposure.test.tstest/credentials-cli-command.test.tstest/credentials-shim.test.tstest/credentials.test.tstest/e2e-runtime/4851-ultra-toolless-validation.mdtest/e2e-scenario/fixtures/hosted-inference.tstest/e2e-scenario/fixtures/phases/onboarding.tstest/e2e-scenario/live/credential-migration.test.tstest/e2e-scenario/live/credential-sanitization.test.tstest/e2e-scenario/live/gateway-guard-recovery.test.tstest/e2e-scenario/live/hermes-e2e.test.tstest/e2e-scenario/live/inference-routing.test.tstest/e2e-scenario/live/issue-4434-tui-unreachable-inference.test.tstest/e2e-scenario/live/launchable-smoke.test.tstest/e2e-scenario/live/model-router-provider-routed-inference.test.tstest/e2e-scenario/live/network-policy.test.tstest/e2e-scenario/live/onboard-negative-paths.test.tstest/e2e-scenario/live/onboard-resume.test.tstest/e2e-scenario/live/openclaw-tui-chat-correlation.test.tstest/e2e-scenario/live/rebuild-openclaw.test.tstest/e2e-scenario/live/sandbox-operations.test.tstest/e2e-scenario/live/sandbox-rebuild.test.tstest/e2e-scenario/live/sandbox-survival.test.tstest/e2e-scenario/live/shields-config.test.tstest/e2e-scenario/live/skill-agent.test.tstest/e2e-scenario/live/token-rotation.test.tstest/e2e-scenario/live/whatsapp-qr-compact.test.tstest/e2e-scenario/manifests/hermes-nvidia-discord.yamltest/e2e-scenario/manifests/hermes-nvidia-slack.yamltest/e2e-scenario/manifests/hermes-nvidia.yamltest/e2e-scenario/manifests/openclaw-nvidia-brave.yamltest/e2e-scenario/manifests/openclaw-nvidia-brev-launchable.yamltest/e2e-scenario/manifests/openclaw-nvidia-custom-policies.yamltest/e2e-scenario/manifests/openclaw-nvidia-discord.yamltest/e2e-scenario/manifests/openclaw-nvidia-double-provider-switch.yamltest/e2e-scenario/manifests/openclaw-nvidia-double-same-provider.yamltest/e2e-scenario/manifests/openclaw-nvidia-gateway-port-conflict.yamltest/e2e-scenario/manifests/openclaw-nvidia-invalid-key.yamltest/e2e-scenario/manifests/openclaw-nvidia-macos.yamltest/e2e-scenario/manifests/openclaw-nvidia-no-docker-negative.yamltest/e2e-scenario/manifests/openclaw-nvidia-post-reboot-recovery.yamltest/e2e-scenario/manifests/openclaw-nvidia-rebuild.yamltest/e2e-scenario/manifests/openclaw-nvidia-repair.yamltest/e2e-scenario/manifests/openclaw-nvidia-resume.yamltest/e2e-scenario/manifests/openclaw-nvidia-slack.yamltest/e2e-scenario/manifests/openclaw-nvidia-telegram.yamltest/e2e-scenario/manifests/openclaw-nvidia-token-rotation.yamltest/e2e-scenario/manifests/openclaw-nvidia-wsl.yamltest/e2e-scenario/manifests/openclaw-nvidia.yamltest/e2e-scenario/scenarios/scenarios/baseline.tstest/e2e-scenario/scenarios/types.tstest/e2e-scenario/support-tests/docker-probe.test.tstest/e2e-scenario/support-tests/e2e-fixture-context.test.tstest/e2e-scenario/support-tests/e2e-manifests.test.tstest/e2e-scenario/support-tests/e2e-phase-environment.test.tstest/e2e-scenario/support-tests/e2e-phase-onboarding.test.tstest/e2e-scenario/support-tests/e2e-phase-state-validation.test.tstest/e2e-scenario/support-tests/e2e-scenario-matrix.test.tstest/e2e-scenario/support-tests/e2e-scenarios-workflow.test.tstest/e2e-scenario/support-tests/hosted-inference.test.tstest/e2e-scenario/support-tests/network-policy-transient-provider.test.tstest/e2e-script-workflow.test.tstest/e2e/brev-e2e.test.tstest/e2e/e2e-cloud-experimental/checks/03-security-checks.shtest/e2e/e2e-cloud-experimental/expect-interactive-install.shtest/e2e/e2e-cloud-experimental/features/skill/add-sandbox-skill.shtest/e2e/e2e-cloud-experimental/features/skill/verify-sandbox-skill-via-agent.shtest/e2e/e2e-cloud-experimental/test-port8080-conflict.shtest/e2e/lib/ci-compatible-inference.shtest/e2e/test-agent-turn-latency-e2e.shtest/e2e/test-bedrock-runtime-compatible-anthropic.shtest/e2e/test-brave-search-e2e.shtest/e2e/test-channels-add-remove.shtest/e2e/test-channels-stop-start.shtest/e2e/test-cloud-inference-e2e.shtest/e2e/test-cloud-onboard-e2e.shtest/e2e/test-common-egress-agent-e2e.shtest/e2e/test-credential-migration.shtest/e2e/test-credential-sanitization.shtest/e2e/test-cron-preflight-inference-local-e2e.shtest/e2e/test-device-auth-health.shtest/e2e/test-diagnostics.shtest/e2e/test-double-onboard.shtest/e2e/test-full-e2e.shtest/e2e/test-hermes-discord-e2e.shtest/e2e/test-hermes-e2e.shtest/e2e/test-hermes-inference-switch.shtest/e2e/test-hermes-slack-e2e.shtest/e2e/test-inference-routing.shtest/e2e/test-issue-2478-crash-loop-recovery.shtest/e2e/test-issue-4434-tui-unreachable-inference.shtest/e2e/test-issue-4462-scope-upgrade-approval.shtest/e2e/test-kimi-inference-compat.shtest/e2e/test-launchable-smoke.shtest/e2e/test-messaging-providers.shtest/e2e/test-model-router-provider-routed-inference.shtest/e2e/test-network-policy.shtest/e2e/test-onboard-negative-paths.shtest/e2e/test-onboard-repair.shtest/e2e/test-onboard-resume.shtest/e2e/test-openclaw-discord-pairing.shtest/e2e/test-openclaw-inference-switch.shtest/e2e/test-openclaw-plugin-runtime-exdev.shtest/e2e/test-openclaw-skill-cli-e2e.shtest/e2e/test-openclaw-slack-pairing.shtest/e2e/test-overlayfs-autofix.shtest/e2e/test-rebuild-hermes.shtest/e2e/test-rebuild-openclaw.shtest/e2e/test-sandbox-operations.shtest/e2e/test-sandbox-rebuild.shtest/e2e/test-sandbox-survival.shtest/e2e/test-sessions-agents-cli.shtest/e2e/test-shields-config.shtest/e2e/test-skill-agent-e2e.shtest/e2e/test-snapshot-commands.shtest/e2e/test-state-backup-restore.shtest/e2e/test-telegram-injection.shtest/e2e/test-token-rotation.shtest/e2e/test-tunnel-lifecycle.shtest/e2e/test-upgrade-stale-sandbox.shtest/gateway-state-reconcile-2276.test.tstest/helpers/onboard-final-flow-phases.tstest/host-artifact-cleanup.test.tstest/nemoclaw-start.test.tstest/no-direct-credential-env.test.tstest/ollama-proxy-recovery.test.tstest/onboard-messaging.test.tstest/onboard-model-router.test.tstest/onboard-selection-vllm.test.tstest/onboard-selection.test.tstest/onboard.test.tstest/rebuild-credential-hydration.test.tstest/rebuild-credential-preflight.test.tstest/rebuild-shields-auto-unlock.test.tstest/rebuild-stale-recovery.test.tstest/regression-e2e-workflow.test.tstest/runner.test.tstest/secret-redaction.test.tstest/smoke-macos-install.test.tstest/validate-blueprint.test.tstools/e2e-scenarios/workflow-boundary.mts
💤 Files with no reviewable changes (3)
- test/e2e-scenario/support-tests/hosted-inference.test.ts
- test/e2e-scenario/fixtures/hosted-inference.ts
- test/e2e/lib/ci-compatible-inference.sh
Comment on lines
+112
to
+117
| api_base: "https://inference-api.nvidia.com" | ||
|
|
||
| - name: super | ||
| litellm_model: "openai/nvidia/nvidia/nemotron-3-super-v3" | ||
| cost_per_m_input_tokens: 0.10 | ||
| api_base: "https://integrate.api.nvidia.com" | ||
| api_base: "https://inference-api.nvidia.com" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if both NVIDIA endpoints are consistently referenced across docs and config
# Search for all references to NVIDIA API endpoints in docs and config
rg -n --type=md --type=mdx --type=yaml -C2 'integrate\.api\.nvidia\.com|inference-api\.nvidia\.com'Repository: NVIDIA/NemoClaw
Length of output: 87
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="docs/inference/inference-options.mdx"
echo "---- Inspect target file around lines 70-130 ----"
nl -ba "$FILE" | sed -n '70,130p'
echo
echo "---- Repo-wide occurrences of both endpoints (explicit globs) ----"
rg -n -C2 "integrate\.api\.nvidia\.com|inference-api\.nvidia\.com" \
-g'*.md' -g'*.mdx' -g'*.yml' -g'*.yaml'Repository: NVIDIA/NemoClaw
Length of output: 170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="docs/inference/inference-options.mdx"
echo "---- Inspect target file around lines 70-130 ----"
# Line-numbered output without `nl`
awk 'NR>=70 && NR<=130 {printf "%4d:%s\n", NR, $0}' "$FILE"
echo
echo "---- Repo-wide occurrences of both endpoints (explicit globs) ----"
rg -n -C2 "integrate\.api\.nvidia\.com|inference-api\.nvidia\.com" \
-g'*.md' -g'*.mdx' -g'*.yml' -g'*.yaml'Repository: NVIDIA/NemoClaw
Length of output: 24942
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- Show router pool-config.yaml ----"
sed -n '1,120p' nemocl…/router/pool-config.yaml 2>/dev/null || true
# safer: print with actual path from earlier results
sed -n '1,120p' nemoclaw-blueprint/router/pool-config.yaml
echo
echo "---- Find where 'api_base' is used for router/nemotron ----"
rg -n --hidden --glob '!.git' "api_base" nemoclaw-blueprint docs agents src test tools 2>/dev/null || true
echo
echo "---- Find litellm router/provider config usage (nvidia-router, routed) ----"
rg -n --hidden --glob '!.git' "nvidia-router|provider_type: \"nvidia\"|routed\"|nemoclaw-blueprint/router" . 2>/dev/null || trueRepository: NVIDIA/NemoClaw
Length of output: 14824
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- Inspect src/lib/onboard/routed-inference.ts ----"
sed -n '1,220p' src/lib/onboard/routed-inference.ts
echo
echo "---- Find how router pool 'api_base' is interpreted ----"
rg -n "pool-config.yaml|api_base|inference-api\.nvidia\.com|integrate\.api\.nvidia\.com|v1" src/lib/onboard src/lib/inference src/lib/actions src/lib/domain 2>/dev/null || true
echo
echo "---- Inspect router config tests mentioning api_base ----"
sed -n '180,320p' test/validate-config-schemas.test.ts
sed -n '1,140p' test/validate-configs-dangerous-hosts.test.tsRepository: NVIDIA/NemoClaw
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' scripts/validate-configs.ts
echo
echo "---- show ROUTER_API_BASE_HOST_ALLOWLIST definition ----"
rg -n "ROUTER_API_BASE_HOST_ALLOWLIST" scripts/validate-configs.ts
sed -n '220,360p' scripts/validate-configs.tsRepository: NVIDIA/NemoClaw
Length of output: 13336
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,260p' src/lib/inference/provider-models.tsRepository: NVIDIA/NemoClaw
Length of output: 7638
Fix router pool api_base endpoint mismatch in NVIDIA inference options docs.
In docs/inference/inference-options.mdx, the NVIDIA Endpoints description (line 83) says NemoClaw uses https://integrate.api.nvidia.com/v1, but the router pool examples (lines 112 and 117) use https://inference-api.nvidia.com.
The repo’s actual router pool config nemoclaw-blueprint/router/pool-config.yaml uses api_base: "https://integrate.api.nvidia.com/v1", and scripts/validate-configs.ts only allowlists integrate.api.nvidia.com as a safe router pool api_base host.
Update the router pool examples to https://integrate.api.nvidia.com/v1 (or update the router implementation + config validation + docs to justify https://inference-api.nvidia.com).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/inference/inference-options.mdx` around lines 112 - 117, Router pool
examples in the docs use the wrong api_base; update the router pool entries that
set api_base: "https://inference-api.nvidia.com" to use
"https://integrate.api.nvidia.com/v1" so they match the NemoClaw endpoint
described earlier and the repo's router pool config (pool-config.yaml) and
allowlist in scripts/validate-configs.ts; specifically change the api_base value
in the router pool examples (the blocks that include name: super and the
surrounding router pool entries) to the integrate.api.nvidia.com/v1 URL.
Comment on lines
39
to
41
| # openshell sandbox connect nemoclaw | ||
| # export NVIDIA_INFERENCE_API_KEY=nvapi-... | ||
| # export NVIDIA_API_KEY=nvapi-... | ||
| # nemoclaw-start |
Contributor
There was a problem hiding this comment.
Remove the stale in-sandbox API key export instruction.
Line 40 conflicts with the behavior explained on Line 88-89 (“not needed inside the sandbox”). Keeping both instructions can confuse operators and promotes unnecessary secret handling in the sandbox shell.
Suggested update
Terminal 2 (Agent):
openshell sandbox connect nemoclaw
- export NVIDIA_API_KEY=nvapi-...
nemoclaw-start
openclaw agent --agent main --local --session-id live🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/walkthrough.sh` around lines 39 - 41, Remove the stale in-sandbox API
key instruction by deleting the line that exports the NVIDIA_API_KEY environment
variable (the line containing export NVIDIA_API_KEY=nvapi-...) so the
walkthrough no longer tells operators to set a secret inside the sandbox; leave
the surrounding sandbox commands (e.g., openshell sandbox connect nemoclaw and
nemoclaw-start) intact and do not add any other secret-handling steps.
Comment on lines
+290
to
+297
| # The result should contain the literal string ${NVIDIA_API_KEY}, not a nvapi- value | ||
| if echo "$t4_result" | grep -q "nvapi-"; then | ||
| fail "T4: \${NVIDIA_INFERENCE_API_KEY} expanded to actual key value — secret leaked!" | ||
| elif echo "$t4_result" | grep -qF '${NVIDIA_INFERENCE_API_KEY}'; then | ||
| pass "T4: \${NVIDIA_INFERENCE_API_KEY} treated as literal string (not expanded)" | ||
| fail "T4: \${NVIDIA_API_KEY} expanded to actual key value — secret leaked!" | ||
| elif echo "$t4_result" | grep -qF '${NVIDIA_API_KEY}'; then | ||
| pass "T4: \${NVIDIA_API_KEY} treated as literal string (not expanded)" | ||
| else | ||
| # Empty or other result — still safe as long as key not leaked | ||
| pass "T4: \${NVIDIA_INFERENCE_API_KEY} did not expand to key value (result: ${t4_result:0:100})" | ||
| pass "T4: \${NVIDIA_API_KEY} did not expand to key value (result: ${t4_result:0:100})" |
Contributor
There was a problem hiding this comment.
T4 leak detection can false-negative by matching only nvapi- instead of the actual secret value.
Line 291 should check for ${NVIDIA_API_KEY} directly. Right now, if the key format changes, the test can pass even when expansion leaked the real key.
Suggested fix
-# The result should contain the literal string ${NVIDIA_API_KEY}, not a nvapi- value
-if echo "$t4_result" | grep -q "nvapi-"; then
+# The result should contain the literal string ${NVIDIA_API_KEY}, not the real key value
+if echo "$t4_result" | grep -qF "$NVIDIA_API_KEY"; then
fail "T4: \${NVIDIA_API_KEY} expanded to actual key value — secret leaked!"
elif echo "$t4_result" | grep -qF '${NVIDIA_API_KEY}'; then
pass "T4: \${NVIDIA_API_KEY} treated as literal string (not expanded)"
else🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/e2e/test-telegram-injection.sh` around lines 290 - 297, The test
currently looks for the literal substring "nvapi-" in t4_result which can
false-negative; update the leak-detection to check for the actual expanded
secret value and the literal placeholder: if echo "$t4_result" | grep -qF
"$NVIDIA_API_KEY"; then fail "T4: \${NVIDIA_API_KEY} expanded to actual key
value — secret leaked!"; elif echo "$t4_result" | grep -qF '${NVIDIA_API_KEY}';
then pass "T4: \${NVIDIA_API_KEY} treated as literal string (not expanded)";
else pass "T4: \${NVIDIA_API_KEY} did not expand to key value (result:
${t4_result:0:100})" — ensure both grep uses are quoted and use -qF for
fixed-string matching.
Contributor
Vitest E2E Scenario Results — ❌ Some jobs failedRun: 27472269478
|
Contributor
Selective E2E Results — ❌ Some jobs failedRun: 27472268756
|
Contributor
Selective E2E Results — ❌ Some jobs failedRun: 27472268756
|
12 tasks
Collaborator
|
Let's hold on merging this one. I still have hope that I can get E2Es pointing at the custom inference API with a few tweaks, and then we can further replace the inference API calls entirely for a fake server where it makes sense to do so. |
jyaunches
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Reverts the NVIDIA_INFERENCE_API_KEY migration stack so E2E can validate the old NVIDIA_API_KEY-backed path while weekend traffic is lower.
Reverted PRs/commits:
fix(e2e): support compatible credential migrationfix(inference): use NVIDIA inference credential envValidation
git diff --check origin/main..HEADNVIDIA_INFERENCE_API_KEY/ compatible migration toggles:.github/workflows/nightly-e2e.yaml.github/workflows/e2e-vitest-scenarios.yaml.github/workflows/e2e-script.yamlFull E2E runs will be dispatched on this branch after PR creation:
Note: local pre-push hooks were bypassed because this worktree has no installed npm dependencies; hook failures were missing local modules/tools (
typescript,tsx,vitest,ajv, biome transitiveklaw), not validation failures from the revert.Summary by CodeRabbit
Bug Fixes
Documentation