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 `@nemoclaw-blueprint/router/pool-config.yaml`:
- Around line 24-29: The pool entry currently has a mismatched identity: the
fields name: gpt-5.2 and litellm_model:
"openai/nvidia/nvidia/Nemotron-3-Nano-30B-A3B" must be aligned; either change
the name to match the routed model (e.g., name: nemotron-3-nano) or change
litellm_model to the correct GPT-5.2 model identifier so that the router’s
checkpoint (which uses the name field) maps accuracy predictions to the actual
model; update the first pool entry’s name or litellm_model to mirror the pattern
used in the working entry (nemotron-3-super → nemotron-3-super-v3).

In `@nemoclaw/src/blueprint/runner.ts`:
- Around line 149-161: The router validation currently allows components.router
= { enabled: true } without a pool_config_path; update the validation in
runner.ts (the block handling components.router in isBlueprint / validation) so
that when router.enabled === true you require a present, non-empty
pool_config_path (use the existing string validation helper rather than
isOptionalString) and keep the existing port checks; ensure the condition
rejects router objects with enabled true and pool_config_path missing/invalid so
broken routed blueprints fail validation instead of at startup.

In `@README.md`:
- Line 145: Replace the GitHub repo link in the sentence that mentions "NVIDIA
LLM Router v3" with either the official documentation URL for the NVIDIA LLM
Router (if available) or remove the hyperlink and keep the plain text name
"NVIDIA LLM Router v3"; ensure the README.md line referencing the router does
not link to a third-party code repo per guidelines and that the rest of the
sentence ("prefill routing engine" / "runs on the host as a LiteLLM proxy" /
"sandbox reaches it through the OpenShell gateway") remains unchanged.

In `@scripts/install.sh`:
- Around line 1341-1343: The routed install path silently degrades: instead of
no-op when pip3 is missing or swallowing failures with "|| true", make routed
installs fail fast by checking command_exists pip3 and testing the pip3 install
return code (the spin "Installing model router" pip3 install ... call) and
exiting non‑zero with an error message if pip3 is missing or the install fails;
likewise remove or replace any "|| true" used around submodule fetches for the
llm-router submodule so git fetch/checkout failures are checked and cause an
immediate error/exit; apply the same fail-fast checks to the other occurrences
around the llm-router installs mentioned (the blocks referencing
NEMOCLAW_SOURCE_ROOT/nemoclaw-blueprint/router/llm-router and any uses of
command_exists pip3, spin, and NEMOCLAW_PROVIDER=routed).

In `@src/lib/onboard.ts`:
- Around line 6620-6673: The provider recovery logic needs a reverse mapping for
the routed provider: update the providerNameToOptionKey function so that when
name === "nvidia-router" it returns "routed" (respecting the existing signature
providerNameToOptionKey(name, opts) and any opts.hasNimContainer logic); this
ensures the persisted provider set in onboard.ts (provider = bp.provider_name ||
"nvidia-router") is correctly mapped back to the "routed" selection during
non-interactive recovery rather than falling through to "build".
- Around line 854-887: The current health-check loop can mistakenly accept a
pre-existing /health on the same port while the spawned child has already
exited; capture the spawned child's lifecycle and only treat /health as success
if the child is still running. After calling spawn("model-router", ... ) and
before returning pid, attach an 'exit' handler or poll child.exitCode (or use
process.kill(pid, 0) safely) to detect early termination and save that state; in
the health loop, when a healthy HTTP response is seen, verify the child has not
exited (child.exitCode === null/undefined and not killed) and only then return
pid; if the child exited with a non-zero code or is not alive, throw an error
and continue retries. Ensure to reference the existing symbols: spawn call,
child, child.pid, child.on('exit') / child.exitCode, ROUTER_HEALTH_RETRIES,
ROUTER_HEALTH_INTERVAL_MS, litellmConfigPath, poolConfigPath, port,
blueprintDir, and buildSubprocessEnv when implementing this check.
- Around line 6627-6644: Resolve the routed credential by calling
hydrateCredentialEnv(routerCredentialEnv) (or equivalent) before the
non-interactive/interactive gate and use its returned value to populate
process.env[routerCredentialEnv]; ensure that hydrateCredentialEnv performs
legacy staging fallback so saved/staged credentials count in non-interactive
mode. Update the conditional checks that reference
process.env[routerCredentialEnv] (and bp.credential_default) to instead use the
hydrated result so callers like startModelRouter() / upsertProvider() receive
the credential, and keep the ensureNamedCredential(routerCredentialEnv, "Model
Router API key", null) call only when the hydrated value is absent and
interactive.

---

Nitpick comments:
In `@nemoclaw-blueprint/router/llm-router`:
- Line 1: Add a short governance and update policy for the llm-router submodule:
document an owner responsible for tracking upstream llm-router releases, require
pinning to tagged releases (e.g., v3.x.x) rather than branch HEAD for
predictable behavior, and define a cadence/process for evaluating and applying
upstream commits; include a security monitoring step (subscribe to upstream
advisories or use Dependabot/Snyk) and a CI change to run git submodule update
--init --recursive (ensure workflows that build or test reference the llm-router
submodule perform this initialization) so builds won’t fail when submodules are
uninitialized.
- Line 1: The submodule is pinned to an untagged commit
(2bd8dfaa751efb60aa4e7e49b270490dfbc0a68a) from the
NVIDIA-AI-Blueprints/llm-router v3 branch; update .gitmodules to point to a
released tag (e.g., v3.0.0 or v3.1.0) instead of the commit SHA, and update any
corresponding submodule init/update steps (git submodule set-url or the
submodule "branch"/"tag" reference) so the repo uses the stable release tag for
llm-router rather than the arbitrary commit.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@scripts/install.sh`:
- Around line 1334-1338: install_model_router_if_present() currently returns
success when prerequisites are missing (checks like command_exists pip3, [[ -d
"$router_dir" ]], and existence of "$router_dir/pyproject.toml" or
"$router_dir/setup.py"), allowing routed provider flows to continue without a
router; change those early-return paths to propagate failure (non-zero exit)
when NEMOCLAW_PROVIDER is "routed" so installs fail fast: replace the
noop/return 0 behavior in the checks for pip3, router_dir, and missing
pyproject/setup with a non-zero return (or exit) when the environment expects a
routed provider, and update the places that call
install_model_router_if_present() (the call sites that currently downgrade
errors to warnings) to treat the function’s non-zero return as a fatal error
instead of a warning.

In `@src/lib/onboard.ts`:
- Around line 6631-6647: The code validates router credentials by only checking
process.env and bp.credential_default, which skips staged or saved credentials;
call hydrateCredentialEnv(routerCredentialEnv) (or the existing
hydrateCredentialEnv function) before the interactive/non-interactive checks so
any stored/staged key is resolved into the env and prevents stale fallbacks from
overriding explicit env values; specifically, before using
routerCredentialEnv/_providerKeyHint and before the isNonInteractive() branch,
invoke hydrateCredentialEnv(routerCredentialEnv), then proceed to set
credentialEnv and run the existing logic (including ensureNamedCredential) so
downstream router/provider setup sees the resolved credential.
- Around line 6666-6667: The persisted provider value "nvidia-router" is not
mapped back to the routed option, causing recovery to fall back to "build";
update the reverse mapping in the providerNameToOptionKey function to include an
entry mapping "nvidia-router" (and any equivalent persisted keys) to "routed" so
that when provider is read (e.g., provider variable set from bp.provider_name)
it correctly resolves to the routed option; locate providerNameToOptionKey and
add the "nvidia-router" -> "routed" case along with any related string variants
to ensure non-interactive reruns recover the routed selection.
- Around line 857-891: The health-check loop can return a false positive if
another process already serves /health or if the spawned child exited; update
the logic around spawn/child and the loop (symbols: spawn call that creates
child, child, pid, ROUTER_HEALTH_RETRIES, ROUTER_HEALTH_INTERVAL_MS) so that
after receiving a healthy response you also verify the spawned process is still
running (e.g., ensure child.exitCode === null and !child.killed or use
process.kill(pid, 0) for existence) before returning pid; if the child has
exited, treat it as not healthy and continue retries (and surface/log the
child.exitCode/error), and consider failing after retries by killing/stopping
the child if necessary.

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 `@scripts/install.sh`:
- Around line 1336-1354: install_model_router_if_present currently calls pip3
install directly which can cause the whole script to exit on failure even when a
routed provider isn't requested; change the final step so the pip3 install (the
spin "Installing model router" pip3 install --quiet --user
"${router_dir}[prefill,proxy]") is executed and its exit status is checked, and
if it fails call error "..." only when is_routed_provider_requested returns
true, otherwise call warn "Skipping model router install — pip install failed"
(and do not hard-exit); implement this by replacing the direct spin+pip3 line
with an if ! spin ...; then if is_routed_provider_requested; error "pip3 install
of llm-router failed"; else warn "Skipping model router install — pip3 install
failed"; fi; fi so the script only hard-fails for routed installs.
- Around line 1330-1334: The is_routed_provider_requested function uses
Bash-4-only lowercase expansion (${provider,,}) which fails on macOS Bash 3.2;
change the normalization to a POSIX-compatible approach (e.g., pipe provider
through tr '[:upper:]' '[:lower:]' or use printf + tr) before comparing, so
replace the ${provider,,} usage in is_routed_provider_requested with a
POSIX-safe lowercase conversion and then test [[ "$provider" == "routed" ]] as
before.

In `@src/lib/onboard.ts`:
- Around line 6653-6692: The router is being started prematurely inside the
selected.key === "routed" provider-selection branch (see
loadBlueprintProfile("routed"), ensureNamedCredential, and startModelRouter
usage); move the actual startModelRouter(...) call and the onboarding state
mutation (onboardSession.updateSession(...).routerPid) out of this branch and
instead invoke a reconcile/start step at a committed lifecycle point for routed
sandboxes (e.g., after sandbox creation completes or in the resume/reuse path),
and add a verification/restart routine that runs on resume/reuse (and before
marking routed inference ready) to ensure the router process is present and
restarted if needed; keep credential collection
(hydrateCredentialEnv/ensureNamedCredential) in the selection branch but defer
starting or persisting the router PID until the centralized reconcile/start
logic (and call startModelRouter from there).
- Around line 835-843: The isRouterHealthy probe lacks a request timeout so
http.get can hang indefinitely; update the isRouterHealthy function to abort or
timeout the request after a short period (e.g., a few seconds) and
resolve(false) on timeout or error. Locate isRouterHealthy and the http.get call
(using the port variable) and attach a timeout handler—either call
request.setTimeout(...) to destroy/abort the socket or use an AbortController
with a timer—to ensure the promise resolves false on timeout and cleans up
listeners so await isRouterHealthy() cannot block forever.

In `@test/onboard.test.ts`:
- Line 109: Add a runtime guard check inside isOnboardTestInternals to validate
the newly exposed member providerNameToOptionKey: ensure the internals object
has own property "providerNameToOptionKey" and that typeof
internals.providerNameToOptionKey === "function" (or allow null/undefined if
your type permits) so the upfront type guard fails clearly if a stale/dist drift
removes or changes this helper; update the guard logic used by any callers of
providerNameToOptionKey (e.g., tests referenced in onboard.test.ts) to rely on
this enhanced check.

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 `@src/lib/onboard.ts`:
- Around line 805-808: When constructing the router object (variable router
built from parsed?.components?.router and profile), avoid unconditionally
overwriting an existing parsed.components.router.credential_env with
profile.credential_env; instead only apply profile.credential_env when it is
explicitly set (not undefined/null), and otherwise preserve
parsed?.components?.router.credential_env. Update the router creation logic (the
spread of parsed?.components?.router and the assignment of credential_env) so
that credential_env is conditionally included from profile (or falls back to the
existing parsed value) to prevent silently clobbering a valid router-level
credential_env.
- Around line 961-971: reconcileModelRouter currently calls
hydrateCredentialEnv(routerCredentialEnv) but proceeds to startModelRouter even
if the resolved credential is absent; update reconcileModelRouter to read the
resolved credential name (routerCredentialEnv or bp.router.credential_env) after
hydrateCredentialEnv and assert the corresponding value is present (e.g.,
process.env[routerCredentialEnv] or the module's credential getter) and if
missing log an explicit error and fail fast (return/throw) before calling
startModelRouter(bp.router); this check should reference hydrateCredentialEnv,
routerCredentialEnv, and startModelRouter to locate the code to change.

---

Outside diff comments:
In `@src/lib/onboard-session.ts`:
- Around line 118-134: SessionUpdates includes routerPid but filterSafeUpdates
currently omits routerPid so any routerPid passed into update flows (e.g.,
markStepComplete, completeSession) gets dropped; either add routerPid to the
allowed fields in filterSafeUpdates so it is propagated through update flows, or
if routerPid should only be set at creation remove it from the SessionUpdates
interface to avoid misuse—update the filterSafeUpdates implementation and adjust
call sites (markStepComplete, completeSession) accordingly to ensure routerPid
is preserved when intended.

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 `@src/lib/onboard.ts`:
- Around line 869-878: The code builds credEnvVars (using buildSubprocessEnv,
routerCfg.credential_env, and environment keys like
NVIDIA_API_KEY/OPENAI_API_KEY/_providerKey) and injects it when starting a
router in startModelRouter(), but reconcileModelRouter() may reuse an existing
healthy listener without verifying those credentials, allowing stale API keys to
remain after rotations; update reconcileModelRouter() to detect changes to the
effective routed credential (compare the current process.env-derived credential
value for routerCfg.credential_env or the resolved provider key against the
listener's last-started credential) and force-restart or replace the child
listener if the credential differs, or alternatively ensure the child is always
started with the new credEnvVars (propagate the new buildSubprocessEnv output)
so listeners never continue with revoked keys; apply the same check/fix to the
analogous logic around the other block noted (lines ~958-973).
- Around line 6714-6736: The code selects routerCredentialEnv from
bp.credential_env causing mismatch when a blueprint sets
components.router.credential_env; change the selection to prefer
bp.router?.credential_env first (e.g., compute routerCredentialEnv =
bp.router?.credential_env || bp.credential_env || "OPENAI_API_KEY") so all
subsequent uses (hydrateCredentialEnv, process.env assignment, the
_providerKeyHint check, isNonInteractive handling and ensureNamedCredential
calls) consistently use the router-specific env key; also update the other
occurrence (the similar block referenced around reconcileModelRouter) to use the
same router-first lookup so loadBlueprintProfile/reconcileModelRouter paths are
aligned.
- Around line 886-913: The spawn() child process may emit an 'error' before pid
is available; add a child.once('error', (err) => { ... }) listener immediately
after creating child (and before calling child.unref() and before checking const
pid) to capture spawn failures and convert them to a thrown or logged onboarding
error (include err.message), then remove or ignore the listener if the process
starts successfully; update the block around the spawn call, child.unref(),
const pid check, and the existing child.once('exit', ...) so the 'error' event
is handled deterministically for the model-router child.