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 955-957: ensureModelRouterCommand currently calls
resolveHostCommandPath("model-router") first which lets a PATH binary shadow the
repo-managed virtualenv and forces repeated installs; change it to prefer the
managed router binary (the repo-managed ~/.nemoclaw/.../bin/model-router path
used by installModelRouterCommand) by checking that managed path exists and is
executable first, then fall back to resolveHostCommandPath("model-router"), and
only call installModelRouterCommand() if neither the managed path nor a PATH
binary is found; use the same helper functions (ensureModelRouterCommand,
installModelRouterCommand, resolveHostCommandPath) and perform a file-exists +
executable permission check on the managed binary before returning it.

---

Outside diff comments:
In `@src/lib/onboard.ts`:
- Around line 6889-6900: The routed credential fallback should use a single
shared default constant instead of hardcoding "OPENAI_API_KEY"; replace the
literal fallback in the onboard initialization (where routerCredentialEnv is
computed) to use DEFAULT_MODEL_ROUTER_CREDENTIAL_ENV so the same default is used
by hydrateCredentialEnv/saveCredential/resolveProviderCredential and matches the
other routed startup paths (e.g., reconcileModelRouter); update the computation
of routerCredentialEnv in the block that sets credentialEnv, routedCredential,
and the provider hint handling to reference DEFAULT_MODEL_ROUTER_CREDENTIAL_ENV.

---

Nitpick comments:
In `@src/lib/onboard.ts`:
- Around line 829-1074: The onboarding router provisioning code was moved into
src/lib/onboard.ts and needs full E2E/preflight exercise before merge; run the
onboarding E2E matrix exercising the routed sandbox flow, resume path, and
host-side router startup to validate startModelRouter,
installModelRouterCommand, ensureModelRouterCommand,
initializeModelRouterSubmodule, isRouterHealthy and stopModelRouterProcess paths
(including submodule initialization, venv creation, pip install, proxy-config
generation, detached spawn and health polling) and fix any failures found
(missing env handling, permission/path issues, timeouts, or crash diagnostics)
before landing.

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 `@src/lib/onboard.ts`:
- Around line 925-957: ensureModelRouterCommand currently prefers a
PATH-installed "model-router" and only installs the managed venv when PATH lacks
it; change it to prefer the managed, NemoClaw-owned binary first: compute the
managed command path (the same logic used in installModelRouterCommand: venvDir,
routerCommand) and if fs.existsSync(routerCommand) return that; otherwise fall
back to resolveHostCommandPath("model-router") and if that returns a path return
it; finally call installModelRouterCommand() only if neither exists. Update
ensureModelRouterCommand to reference venvDir/routerCommand,
resolveHostCommandPath, and installModelRouterCommand accordingly so the managed
venv is reused and not shadowed by PATH.

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 `@test/onboard.test.ts`:
- Around line 2584-2773: The test creates a tmpDir and writes executables but
never removes it; wrap the test body for the "prepares Model Router dependencies
during routed inference setup when command is absent" it block (the function
containing tmpDir, fakeBin, scriptPath, fakeRouterSource, setupLog, venvDir,
etc.) in a try/finally and in the finally call fs.rmSync(tmpDir, { recursive:
true, force: true }) to clean up; do the same for the other newly added routed
Model Router test (the other it block in this diff range) so both tests remove
their tmpDir even on failure.
- Around line 2699-2707: The test runner currently lets git calls pass through
to originalRun (runner.run -> originalRun) when cmd.startsWith("python3 -m
venv") || cmd.includes("/bin/python -m pip") || cmd.includes("git -C"), which
allows real git to execute; change runner.run so any git invocation (e.g.,
cmd.includes("git -C") or cmd.startsWith("git ")) is handled deterministically
in the harness instead of calling originalRun — either stub the git call by
pushing the command into commands and returning a deterministic {status, stdout,
stderr} or explicitly throw/return a failure for unexpected git invocations;
update the condition around originalRun and the branch that checks for "provider
get" to ensure git never reaches originalRun.

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 1089-1108: ensureModelRouterCommand currently reinstalls the
router (via installModelRouterCommand) before checking for existing executables
or PATH binaries, causing failures when python3 is missing even though a host or
managed executable exists; modify ensureModelRouterCommand so that after
computing routerDir, venvDir and managedCommand it checks
isExecutableFile(managedCommand) and resolveHostCommandPath("model-router")
before calling installModelRouterCommand, and only call
initializeModelRouterSubmodule() if !isModelRouterPackageReady(routerDir) while
preserving the existing checks isModelRouterPackageReady(routerDir) and
isManagedModelRouterCurrent(routerDir, venvDir) to return managedCommand early —
use the existing helpers modelRouterPackageDir, modelRouterVenvDir,
modelRouterCommandPath, isModelRouterPackageReady, isManagedModelRouterCurrent,
initializeModelRouterSubmodule, installModelRouterCommand, isExecutableFile, and
resolveHostCommandPath to implement the new resolution order.

In `@test/onboard.test.ts`:
- Around line 2725-2736: The test harness' runner.run stub currently treats
unknown commands as success which masks regressions like a stray "pip3"
invocation; update the runner.run implementation(s) (the override around
runner.run in test/onboard.test.ts and the duplicate at the later block) to
detect "pip3" invocations (e.g., commands starting with or containing "pip3 " or
"/bin/pip3") and fail deterministically instead of returning a fake success —
either throw an Error or return a nonzero status for "pip3" so the test fails,
while keeping the existing special-cases for "python3 -m venv" and "/bin/python
-m pip".

---

Nitpick comments:
In `@src/lib/onboard.ts`:
- Around line 829-1287: The change moved host-side Model Router bootstrap into
onboard.ts but the full gateway/sandbox/rebuild flow (Model Router startup)
hasn't been exercised by unit tests; run the onboarding end-to-end slices that
exercise reconcileModelRouter, startModelRouter, ensureModelRouterCommand,
installModelRouterCommand and isRouterHealthy to validate venv creation,
fingerprinting, proxy-config/proxy startup, credential wiring, and stop/restart
behavior before merging, and add or update E2E tests that cover those flows if
any slice fails or is missing.

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 `@test/onboard.test.ts`:
- Around line 2731-2741: Tests currently treat unhandled commands as success, so
add a guard in runner.run to fail fast on unexpected pip invocations: inside the
runner.run implementation (the function that wraps originalRun and pushes into
commands, using _n and originalRun), detect commands invoking pip3 (e.g.
cmd.includes("pip3") or /^pip3(?:\s|$)/.test(cmd) or cmd.includes("/bin/pip3"))
and throw an Error like the existing git check; mirror the same check in the
other similar harness block referenced (lines ~3115-3125) so any unexpected pip3
invocation fails the test instead of silently succeeding.

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

Outside diff comments:
In `@src/lib/onboard.ts`:
- Around line 7367-7375: The code uses credentialEnv ||
DEFAULT_MODEL_ROUTER_CREDENTIAL_ENV when calling upsertProvider which can
register the provider against an out-of-date env var; instead obtain the
normalized credential env resolved by reconcileModelRouter (the
bp.router.credential_env || bp.credential_env ||
DEFAULT_MODEL_ROUTER_CREDENTIAL_ENV logic) and use that normalized value when
calling hydrateCredentialEnv and passing the env into upsertProvider; update the
call sites (referencing reconcileModelRouter, resolvedCredentialEnv,
credentialEnv, hydrateCredentialEnv, upsertProvider, and
DEFAULT_MODEL_ROUTER_CREDENTIAL_ENV) so the provider registration always uses
the blueprint-reconciled credential_env rather than a stale session value.

---

Duplicate comments:
In `@src/lib/onboard.ts`:
- Around line 1102-1108: The current logic returns
installModelRouterCommand(routerDir) as soon as
isModelRouterPackageReady(routerDir) is true but
isManagedModelRouterCurrent(...) is false, which forces a reinstall even if a
usable router binary exists; change the control flow in the block that checks
isModelRouterPackageReady to defer calling installModelRouterCommand(routerDir)
— instead, if isModelRouterPackageReady(routerDir) is true and
isManagedModelRouterCurrent(routerDir, venvDir) is false, first check if
isExecutableFile(managedCommand) or resolveHostCommandPath("model-router")
yields a usable command and return that, and only call
installModelRouterCommand(routerDir) as the last fallback.

In `@test/onboard.test.ts`:
- Around line 2731-2741: The test harness runner.run currently treats
unrecognized "pip3 ..." invocations as success; add an explicit guard in the
runner.run stub (the block that references originalRun, commands, and returns
{status:0,...}) to fail fast on any cmd that includes "pip3" or starts with
"pip3 " (either by returning a non-zero status result or throwing an Error), and
apply the same check to the other routed harness runner.run stub (the similar
block elsewhere that records commands and falls through to success) so
unexpected pip3 usage is detected immediately.

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 `@test/onboard.test.ts`:
- Around line 2915-2920: The test harness currently lets any unrecognized
command succeed in runner.run, masking regressions that re-run installs; update
the runner.run implementation so that it only returns success for explicitly
allowed commands and fails fast for any other/unhandled command (e.g., change
the default branch to return a non-zero status or throw), using the existing
command normalization (_n) and the commands array push to detect the command;
keep the special-case check for "provider get" (cmd.includes("provider get"))
but ensure all other commands (including pip, python3 -m venv, git, etc.)
produce a failure status so unexpected reinstall work will break the test.