perf(test): reduce onboarding subprocess isolation - #6276
Merged
Conversation
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
📝 WalkthroughWalkthroughThis PR refactors onboarding inference provider setup (Bedrock Runtime, Hermes auth/provider, Ollama, remote, routed, vLLM-local, local-inference-route, Windows host Ollama detection) to use injected ChangesOnboarding inference DI boundary refactor
Responses-fallback curl probe test support
service-env.test.ts filesystem API refactor
In-process messaging build phase for config test
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant OnboardFlow
participant setupInferenceWithDeps
participant ProviderSetup
participant exitProcess
OnboardFlow->>setupInferenceWithDeps: setupInference(sandbox, model, provider)
setupInferenceWithDeps->>ProviderSetup: dispatch (Hermes/remote/vllm/ollama/routed/bedrock)
alt failure
ProviderSetup->>ProviderSetup: error(message)
ProviderSetup->>exitProcess: exitProcess(status)
exitProcess-->>OnboardFlow: terminate / reject
else success
ProviderSetup-->>setupInferenceWithDeps: outcome.done
setupInferenceWithDeps-->>OnboardFlow: { ok: true }
end
Possibly related issues
Suggested labels: Suggested reviewers: PoemA rabbit hopped through onboarding code, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in the Show a code coverage summary of the most covered files.
TypeScript / code-coverage/cliThe overall coverage in the Show a code coverage summary of the most covered files.
Updated |
Contributor
PR Review Advisor (Nemotron Ultra) — Changes requestedMerge posture: Do not merge yet Action checklist
Findings index
🚨 Required before mergeAddress these before merging unless a maintainer explicitly overrides the advisor with rationale.
|
Contributor
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
Contributor
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
Contributor
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Collaborator
Author
|
Final-head advisor disposition (
There are no required findings on the final head. We are freezing this PR after the batched fixes and treating these warnings as follow-up work to avoid another full push/CI cycle. |
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Make provider dependency ownership explicit so new requirements must be wired deliberately. Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Collaborator
Author
|
Final-head advisor disposition (
No unresolved final-head advisor item requires another code change. The branch remains frozen while required CI completes. |
cv
marked this pull request as ready for review
July 4, 2026 19:28
![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: 1
🧹 Nitpick comments (4)
src/lib/onboard/inference-providers/types.ts (1)
71-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider tightening
classification/ClassifyApplyFailuretyping.
classification: anyandClassifyApplyFailure'sanyreturn weaken the otherwise-improved type safety in this same edit. Not blocking, but since the surrounding types were just hardened, it'd be a small consistency win to typeclassificationas the actual classification shape.🤖 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/inference-providers/types.ts` around lines 71 - 77, Tighten the typings in the shared inference-provider types by replacing the loose `any` usage in `ClassifyApplyFailure` and the `classification` parameter with the actual classification shape used by this flow. Update the relevant type alias and any related function signatures in `types.ts` so `classification` is explicitly typed instead of `any`, and make `ClassifyApplyFailure` return the concrete failure/classification result type rather than `any`.test/service-env.test.ts (1)
71-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider consolidating the four near-identical snippet extractors.
extractToolRedirectsSnippetandextractProxyVarsSnippetrepeat the same read/indexOf/throw scaffolding as the pre-existingextractRuntimeShellEnvSnippet/extractRuntimeShellEnvShimSnippet. A sharedextractScriptSnippet(startMarker, endMarker, errorContext)helper (handling the 2-marker case, with the 3-marker tool-redirects case as a thin wrapper) would remove the duplication without changing behavior.♻️ Example shared helper
+function extractScriptSnippet(startMarker: string, endMarker: string, errorContext: string, from = 0) { + const src = readFileSync(NEMOCLAW_START_SCRIPT, "utf-8"); + const start = src.indexOf(startMarker, from); + const end = src.indexOf(endMarker, start); + if (start === -1 || end === -1 || end <= start) { + throw new Error(`Failed to extract ${errorContext} from scripts/nemoclaw-start.sh`); + } + return { src, start, end }; +} + function extractProxyVarsSnippet() { - const src = readFileSync(NEMOCLAW_START_SCRIPT, "utf-8"); - const start = src.indexOf("PROXY_HOST="); - const endMarker = 'export no_proxy="$_NO_PROXY_VAL"'; - const end = src.indexOf(endMarker, start); - if (start === -1 || end === -1 || end <= start) { - throw new Error( - "Failed to extract proxy configuration from scripts/nemoclaw-start.sh — " + - "the PROXY_HOST..no_proxy block may have been moved or renamed", - ); - } - return src.slice(start, end + endMarker.length); + const endMarker = 'export no_proxy="$_NO_PROXY_VAL"'; + const { src, start, end } = extractScriptSnippet("PROXY_HOST=", endMarker, "proxy configuration"); + return src.slice(start, end + endMarker.length); }🤖 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/service-env.test.ts` around lines 71 - 98, The two new snippet extractors duplicate the same read/indexOf/error scaffolding already used by extractRuntimeShellEnvSnippet and extractRuntimeShellEnvShimSnippet. Refactor these helpers in test/service-env.test.ts to use a shared extractScriptSnippet-style utility that takes start/end markers and an error context, with a thin wrapper for the three-marker _TOOL_REDIRECTS case in extractToolRedirectsSnippet. Keep the existing behavior and error messages equivalent while removing the repeated logic in extractProxyVarsSnippet and the other snippet extractors.src/lib/onboard/inference-providers/vllm-local.ts (1)
35-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winconsole.warn left un-migrated in this DI boundary.
The failure branch (lines 45-49) was migrated to injected
error/exitProcess, but the healthy-host warning branch still callsconsole.warndirectly andlogisn't part of this function's destructured deps. This leaves a gap in the DI boundary this PR is establishing, and this warning path can't be asserted/redirected by the new test harness.♻️ Proposed fix
- const { upsertProvider, validateLocalProvider, getLocalProviderHealthCheck, getLocalProviderBaseUrl, applyLocalInferenceRoute, run, VLLM_LOCAL_CREDENTIAL_ENV, exitProcess, error } = deps; + const { upsertProvider, validateLocalProvider, getLocalProviderHealthCheck, getLocalProviderBaseUrl, applyLocalInferenceRoute, run, VLLM_LOCAL_CREDENTIAL_ENV, exitProcess, error, log } = deps; ... if (hostResponding) { - console.warn(` ⚠ ${validation.message}`); + log(` ⚠ ${validation.message}`); if (validation.diagnostic) { - console.warn(` Diagnostic: ${validation.diagnostic}`); + log(` Diagnostic: ${validation.diagnostic}`); } - console.warn( + log( " The server is healthy on the host — continuing. " + "The sandbox uses a different network path and may work correctly.", );🤖 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/inference-providers/vllm-local.ts` around lines 35 - 44, The healthy-host warning path in vllm-local’s validation flow still uses console.warn directly, leaving the new dependency-injected boundary incomplete. Update the same branch that handles hostResponding to use the injected logging dependency instead of console.warn, and make sure the function’s destructured deps include the logger used there so the warning can be captured in tests alongside the existing injected error/exitProcess path.test/onboard-inference-failure-paths.test.ts (1)
12-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor:
bedrockRuntimeOnboardcould use a plain ESM import.Unlike
../src/lib/onboard(documented to needrequire()due to its bottom-of-filemodule.exportspattern),bedrock-runtime.tsdoesn't have that constraint, soimport * as bedrockRuntimeOnboard from "../src/lib/onboard/bedrock-runtime.js"would align with the coding guideline for root-level integration tests to "use ESM imports." Not functionally impactful under the current TS/vitest interop config.🤖 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/onboard-inference-failure-paths.test.ts` around lines 12 - 16, `bedrockRuntimeOnboard` is using `require()` even though `bedrock-runtime.ts` does not need CommonJS interop. Update the root-level test setup in the onboarding failure-paths spec to use a plain ESM namespace import for `../src/lib/onboard/bedrock-runtime.js`, while keeping the existing `onboard` require only if needed for its `module.exports` pattern. This keeps the import style aligned with the guideline and the existing `createSetupInference`/`bedrockRuntimeOnboard` references should make the change easy to locate.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 `@src/lib/onboard/inference-providers/routed.ts`:
- Around line 47-57: Add a failure-path test for setupRoutedInference covering
the non-zero runOpenshell path in routed.ts. Exercise the branch where the
"inference set" command returns a non-zero status, then assert that the error
handling path logs the failure message and calls exitProcess with the returned
status (or 1 fallback). Use setupRoutedInference, runOpenshell, and exitProcess
as the key symbols to locate and cover this behavior.
---
Nitpick comments:
In `@src/lib/onboard/inference-providers/types.ts`:
- Around line 71-77: Tighten the typings in the shared inference-provider types
by replacing the loose `any` usage in `ClassifyApplyFailure` and the
`classification` parameter with the actual classification shape used by this
flow. Update the relevant type alias and any related function signatures in
`types.ts` so `classification` is explicitly typed instead of `any`, and make
`ClassifyApplyFailure` return the concrete failure/classification result type
rather than `any`.
In `@src/lib/onboard/inference-providers/vllm-local.ts`:
- Around line 35-44: The healthy-host warning path in vllm-local’s validation
flow still uses console.warn directly, leaving the new dependency-injected
boundary incomplete. Update the same branch that handles hostResponding to use
the injected logging dependency instead of console.warn, and make sure the
function’s destructured deps include the logger used there so the warning can be
captured in tests alongside the existing injected error/exitProcess path.
In `@test/onboard-inference-failure-paths.test.ts`:
- Around line 12-16: `bedrockRuntimeOnboard` is using `require()` even though
`bedrock-runtime.ts` does not need CommonJS interop. Update the root-level test
setup in the onboarding failure-paths spec to use a plain ESM namespace import
for `../src/lib/onboard/bedrock-runtime.js`, while keeping the existing
`onboard` require only if needed for its `module.exports` pattern. This keeps
the import style aligned with the guideline and the existing
`createSetupInference`/`bedrockRuntimeOnboard` references should make the change
easy to locate.
In `@test/service-env.test.ts`:
- Around line 71-98: The two new snippet extractors duplicate the same
read/indexOf/error scaffolding already used by extractRuntimeShellEnvSnippet and
extractRuntimeShellEnvShimSnippet. Refactor these helpers in
test/service-env.test.ts to use a shared extractScriptSnippet-style utility that
takes start/end markers and an error context, with a thin wrapper for the
three-marker _TOOL_REDIRECTS case in extractToolRedirectsSnippet. Keep the
existing behavior and error messages equivalent while removing the repeated
logic in extractProxyVarsSnippet and the other snippet extractors.
🪄 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: a93554a8-ec09-4358-82b2-6dc978ba2545
📒 Files selected for processing (28)
ci/test-file-size-budget.jsonsrc/lib/actions/sandbox/rebuild-local-provider-recreate.test.tssrc/lib/inference/onboard-probes-curl-harness.tssrc/lib/inference/onboard-probes-responses-fallback.test.tssrc/lib/onboard.tssrc/lib/onboard/bedrock-runtime.test.tssrc/lib/onboard/bedrock-runtime.tssrc/lib/onboard/hermes-auth.test.tssrc/lib/onboard/hermes-auth.tssrc/lib/onboard/inference-providers/hermes.test.tssrc/lib/onboard/inference-providers/hermes.tssrc/lib/onboard/inference-providers/ollama-local.tssrc/lib/onboard/inference-providers/remote.tssrc/lib/onboard/inference-providers/routed.tssrc/lib/onboard/inference-providers/types.tssrc/lib/onboard/inference-providers/vllm-local.tssrc/lib/onboard/local-inference-route.test.tssrc/lib/onboard/local-inference-route.tssrc/lib/onboard/setup-inference.tssrc/lib/onboard/windows-host-ollama.test.tssrc/lib/onboard/windows-host-ollama.tstest/generate-openclaw-config.test.tstest/onboard-inference-failure-paths.test.tstest/onboard-selection.test.tstest/onboard.test.tstest/service-env.test.tstest/support/onboard-selection-test-helpers.tstest/support/setup-inference-test-harness.ts
This was referenced Jul 6, 2026
cv
added a commit
that referenced
this pull request
Jul 7, 2026
<!-- markdownlint-disable MD041 --> ## Summary Run the integration project as a bounded four-worker phase during the canonical local `npm test`, while keeping CI, coverage, focused integration, and direct Vitest runs serialized. Isolate two onboarding fixtures from host-global dashboard ports so the parallel suite remains deterministic. This is the final cumulative #6245 step after the named onboarding conversions, representative process-contract work, and sequenced loader cleanup already merged; the final clean-build Node 22 suite passes in 3:52.03. ## Related Issue Closes #6245. ## Changes - Replace the dashboard-exhaustion fixture's real host listeners with a fake `lsof` while retaining the real CLI, preflight, diagnostic, and non-zero exit contract. - Give the restore-intent fixture an explicit existing dashboard forward so unrelated host port occupancy cannot divert the behavior under test. - Resolve integration scheduling from npm lifecycle, CI, coverage, and worker-cap inputs: local `npm test` uses at most four workers in group 1, while every safety-sensitive route stays serial. - Add a behavior matrix covering local, CI, coverage, focused, direct, and explicit worker-throttle modes. - Complete the cumulative #6245 acceptance path after #6276/#6336/#6383 converted the named onboarding hotspots, #6285/#6417 retained representative process contracts, and #6286/#6299/#6388/#6415 sequenced loader cleanup after process removal. - Record the final host-specific timings, hotspot disposition, and retained process-contract inventory in `test/README.md` as an advisory acceptance snapshot rather than a permanent CI budget. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: Test fixtures and local test-runner scheduling changed; NemoClaw commands, configuration, runtime behavior, and CI/coverage workflows are unchanged. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Independent final-diff review confirmed that the fake `lsof` preserves the real CLI/preflight/exit contract, the restore-intent assertions remain intact, and resolved CI/coverage configurations remain serialized. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Real CLI exhaustion contract passed; restore-intent passed with all 11 dashboard ports deliberately occupied; scheduling matrix passed 14/14 through the lifecycle-triggered config; `npm run test:projects:check` reported 1,327 files disjoint across 8 projects. - [x] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Clean-build Node 22 `npm test -- --reporter=blob` under the normal `umask 022` passed 1,251 files and 13,879 tests with 39 skipped, 1 todo, and zero failures in 3:52.03, down 73% from the issue's 14:19.65 baseline despite a larger suite. The matching diff-scoped routine pre-commit stage passed in 13.95s. #6270 separately removed full coverage from routine pre-commit while preserving manual and authoritative CI gates. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Integration test runs now use adaptive scheduling to speed up local execution while keeping CI/focused runs serialized. * **Bug Fixes** * Improved reliability of onboarding regression coverage by simulating dashboard port exhaustion in a hermetic way. * Updated onboarding-related fixtures to better match the intended readiness/exit behavior. * **Tests** * Added coverage for integration scheduling behavior (local caps, invalid inputs, and CI/coverage scenarios). * **Documentation** * Added test-suite documentation with a local performance snapshot and key test hotspots. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Merged
21 tasks
jyaunches
added a commit
that referenced
this pull request
Jul 9, 2026
…e step into modules (#6444) <!-- markdownlint-disable MD041 --> ## Summary Extracts cohesive units of the sandbox create/register orchestration out of the ~4,900-line `src/lib/onboard.ts` entrypoint into focused modules under `src/lib/onboard/`, following the injected-deps boundary style established by `created-sandbox-finalization.ts` (#6332). The primary goal is maintainability and independent unit-test coverage for the create path, with intentional safety/behavior refinements discovered during review: redacted create-output failure echoing, preservation of non-zero create-stream status when readiness fails, direct argv spawning for trusted create paths, fail-closed Docker-GPU create-poll side-effect handling, and redacted trace reporting for poll/readiness errors. Note on the issue's premise: #6258 cites a `+27 net lines` growth-guard violation from #6166. That premise is stale — the merged #6166 left `onboard.ts` net-smaller, and #6276/#6332 shrank it further, so the guard (`.github/workflows/codebase-growth-guardrails.yaml`, a per-PR net-neutral diff gate) is not currently red. This PR is therefore incremental maintainability work; it keeps `onboard.ts` net-neutral-or-smaller so the gate stays green. ## Related Issue Refs #6258 <!-- Refs (not Fixes): this PR lands two increments of a larger extraction; the remaining create/finalize wiring is left for follow-ups, so the issue should stay open. --> ## Changes - Add `src/lib/onboard/created-sandbox-failure.ts`: `reportSandboxCreateFailure` (warns-and-continues on an incomplete create; otherwise prints diagnostics + recovery hints and exits) and `reportSandboxReadinessFailure` (prints the readiness failure, defers cleanup to the Docker-GPU patch or deletes the failed sandbox, then exits). `onboard.ts` replaces the two inline blocks with module calls — net −2 lines. - Add `src/lib/onboard/sandbox-create-step.ts`: `runSandboxCreateStep` encapsulates the BuildKit prebuild handoff → Docker-GPU create-patch provisioning → create-stream behind a context + injected-deps boundary. This move is **line-neutral** on `onboard.ts`; its value is a named, unit-testable boundary (the `prepare → patch → stream` sequence is now testable without standing up the entrypoint), not a size reduction. Build-context and exit-listener cleanup stay with the caller that armed them. - Add focused unit tests across `created-sandbox-failure.test.ts`, `sandbox-create-step.test.ts`, `sandbox-create-launch.test.ts`, `create-stream.test.ts`, `create-stream-argv.test.ts`, and `create-stream-ready-gate.test.ts` covering failure branches, redaction, exit-code preservation, GPU vs non-GPU readiness cleanup, prebuild/patch/stream wiring, direct argv spawn boundaries, terminal-agent/default-driver ready-check gating, fail-closed `onPoll` error handling, and redacted poll/readiness trace behavior. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: no CLI flags, commands, configuration, or documented user workflow changed. The user-visible differences are limited to safer failure-path diagnostics (credential redaction), more accurate readiness-failure exit status, and fail-closed create-poll error handling. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: onboarding/sandbox path. Extraction boundaries were verified against the pre-extraction source; intentional safety refinements are explicitly covered by tests: create output is redacted before failure logging, readiness failure preserves a non-zero create-stream status instead of flattening to `1`, Docker-GPU during-create polling is isolated from readiness detection via `onPoll`, escaping poll errors abort create with classified/generic failure text plus redacted trace emission, and ready-check exceptions emit redacted trace evidence without falsely forcing Ready. Requesting maintainer sensitive-path review. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result: `npx vitest run --project cli src/lib/actions/sandbox/snapshot.test.ts src/lib/onboard/created-sandbox-failure.test.ts src/lib/onboard/sandbox-create-launch.test.ts src/lib/onboard/sandbox-create-step.test.ts src/lib/sandbox/create-stream.test.ts src/lib/sandbox/create-stream-ready-gate.test.ts src/lib/sandbox/create-stream-argv.test.ts` → 107 passed; `npx tsc --noEmit --pretty false --project tsconfig.cli.json` → clean; `npm run test-size:check` → passed; `npx prek run --all-files --stage pre-commit --skip source-shape-test-budget --skip test-skills-yaml` → passed; `npm run test-conditionals:scan -- --top 25` → no new changed-file conditional failures; `git diff --check` → clean. - [x] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Required live E2E run [29021019725](https://github.com/NVIDIA/NemoClaw/actions/runs/29021019725) passed on exact head `82518f7e6f69c39d9bc04f63a365753aac7b1b6d`: `cloud-onboard`, `onboard-repair`, `onboard-resume`, `state-backup-restore`, `upgrade-stale-sandbox`, and `snapshot-commands` all succeeded. PR CI checks also passed on the same head after rerunning flaky `policy-channel-list.test.ts` shard timeout (`cli-test-shards (3)`). - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Dongni Yang <dongniy@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved sandbox onboarding using a dedicated creation step that centralizes prebuild handoff, streaming create execution, GPU patch wiring, and readiness capture. * **Bug Fixes** * More consistent, centralized handling for both create failures and readiness failures, including redacted output, clearer diagnostics, and reliable retry guidance. * Safer cleanup on readiness failures to avoid same-name collisions, with correct behavior for GPU-enabled flows. * **Tests** * Added coverage for create/readiness failure reporting, exit-code fallback, cleanup/command messaging, and early-detach behavior in create-stream. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Dongni Yang <dongniy@nvidia.com> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Julie Yaunches <jyaunches@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
<!-- markdownlint-disable MD041 --> ## Summary Replace unit-shaped test subprocesses with direct typed seams while retaining meaningful real-process contracts. The final onboarding pilot is 20.5% faster, and the OpenClaw config target is 40.5% faster, without changing production defaults or CLI behavior. ## Related Issue Part of NVIDIA#6245. ## Changes - Extract a typed `createSetupInference` test seam into a focused module while preserving the production dependency wiring and reducing `src/lib/onboard.ts` by 65 net lines. - Rewrite unit-shaped subprocess fixtures in the onboarding, remote-provider-selection, and service-environment suites while retaining representative process-boundary, fail-closed, and production Bash coverage. - Replace 46 service-environment harness child calls with equivalent Node filesystem operations and cache the three-proxy fixture input. - Expand the direct dependency-failure suite to 25 cases, including remote/Bedrock exit boundaries, credential/upsert/apply failures, falsey status fallbacks, local providers, Ollama proxy recovery, routed reconciliation/upsert/route-apply failures, Hermes provider-store/credential/lookup failures, and a focused real Responses-to-Chat-Completions probe fallback test. - Complete injected exit, error, and log wiring across remote, Bedrock, Hermes, Ollama, vLLM, routed-provider, local-route-application, and Hermes-auth paths while retaining production defaults and real `setupNim` boundaries for all five native-Docker Windows-provider rejection scenarios. - Make provider dependency ownership explicit, document the local route recovery source boundary and removal condition, and add three focused local-route recovery tests. - Require explicit Bedrock and Hermes auth failure boundaries, cover positive Hermes auth navigation, and use scanner-safe runtime redaction canaries. - Retain a production-exported `setupInference`/OpenShell process boundary proving raw credentials never enter argv and only the provider update child receives the explicitly scoped credential environment. - Invoke the exported messaging post-install phase directly in OpenClaw config tests, eliminating 98 redundant outer applier launches while preserving all 20 real `openclaw doctor` launches and generator/applier executable contracts. - Restore checked-JavaScript validation for `test/generate-openclaw-config.test.ts` by removing its file-wide `@ts-nocheck` directive. - Tighten legacy test-file size ratchets to 6,146 lines for `test/onboard-selection.test.ts`, 4,057 for `test/onboard.test.ts`, and 1,945 for `test/generate-openclaw-config.test.ts`. - Record a final-head onboarding median improvement from 44.34s to 35.24s (20.5%) and reduce aggregate `execve` attempts from 7,654 to 6,444 (15.8%). - Record an OpenClaw config median improvement from 19.83s to 11.80s (40.5%), with successful `execve` calls reduced from 294 to 196. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: Internal test seams and test-harness rewrites only; production CLI behavior, output, configuration, and public interfaces are unchanged. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Independent focused reviews confirmed production dependency wiring, secret containment, environment restoration, and retained process boundaries; automated advisor findings were addressed or explicitly dispositioned. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: current head: onboarding (65/65), dependency failures (25/25), local-route/rebuild source tests (5/5), Hermes auth (6/6), Bedrock source (4/4), OpenClaw config (128/128), and provider/source targets (26/26); earlier focused probe/provider targets (28/28), selection (69/69), final onboarding benchmark target (134/134 in each run), messaging-applier support (33/33), and service environment (39/39); CLI and checked-JavaScript type-checks plus project-membership, title/size/conditional/source-shape/Biome/diff checks pass. - [x] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: broad pilot gate: `npm test` (1,128 files, 12,621 tests); `npm run test:coverage:cli` (1,004 files, 11,063 tests, all ratchets); current-head pre-commit and pre-push repository/type-check gates passed. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Expanded onboarding support for multiple inference providers, including improved handling for remote, local, routed, Bedrock Runtime, Hermes, Ollama, and Windows host detection flows. * Added clearer fallback behavior when OpenAI-compatible endpoints need to switch from `/responses` to chat completions. * **Bug Fixes** * Improved failure handling and recovery messaging during onboarding, including better exit behavior in non-interactive flows. * Reduced the chance of leaking sensitive values in error output. * **Tests** * Added broader coverage for onboarding, provider selection, and inference-route fallback scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
<!-- markdownlint-disable MD041 --> ## Summary Run the integration project as a bounded four-worker phase during the canonical local `npm test`, while keeping CI, coverage, focused integration, and direct Vitest runs serialized. Isolate two onboarding fixtures from host-global dashboard ports so the parallel suite remains deterministic. This is the final cumulative NVIDIA#6245 step after the named onboarding conversions, representative process-contract work, and sequenced loader cleanup already merged; the final clean-build Node 22 suite passes in 3:52.03. ## Related Issue Closes NVIDIA#6245. ## Changes - Replace the dashboard-exhaustion fixture's real host listeners with a fake `lsof` while retaining the real CLI, preflight, diagnostic, and non-zero exit contract. - Give the restore-intent fixture an explicit existing dashboard forward so unrelated host port occupancy cannot divert the behavior under test. - Resolve integration scheduling from npm lifecycle, CI, coverage, and worker-cap inputs: local `npm test` uses at most four workers in group 1, while every safety-sensitive route stays serial. - Add a behavior matrix covering local, CI, coverage, focused, direct, and explicit worker-throttle modes. - Complete the cumulative NVIDIA#6245 acceptance path after NVIDIA#6276/NVIDIA#6336/NVIDIA#6383 converted the named onboarding hotspots, NVIDIA#6285/NVIDIA#6417 retained representative process contracts, and NVIDIA#6286/NVIDIA#6299/NVIDIA#6388/NVIDIA#6415 sequenced loader cleanup after process removal. - Record the final host-specific timings, hotspot disposition, and retained process-contract inventory in `test/README.md` as an advisory acceptance snapshot rather than a permanent CI budget. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: Test fixtures and local test-runner scheduling changed; NemoClaw commands, configuration, runtime behavior, and CI/coverage workflows are unchanged. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Independent final-diff review confirmed that the fake `lsof` preserves the real CLI/preflight/exit contract, the restore-intent assertions remain intact, and resolved CI/coverage configurations remain serialized. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: Real CLI exhaustion contract passed; restore-intent passed with all 11 dashboard ports deliberately occupied; scheduling matrix passed 14/14 through the lifecycle-triggered config; `npm run test:projects:check` reported 1,327 files disjoint across 8 projects. - [x] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Clean-build Node 22 `npm test -- --reporter=blob` under the normal `umask 022` passed 1,251 files and 13,879 tests with 39 skipped, 1 todo, and zero failures in 3:52.03, down 73% from the issue's 14:19.65 baseline despite a larger suite. The matching diff-scoped routine pre-commit stage passed in 13.95s. NVIDIA#6270 separately removed full coverage from routine pre-commit while preserving manual and authoritative CI gates. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Integration test runs now use adaptive scheduling to speed up local execution while keeping CI/focused runs serialized. * **Bug Fixes** * Improved reliability of onboarding regression coverage by simulating dashboard port exhaustion in a hermetic way. * Updated onboarding-related fixtures to better match the intended readiness/exit behavior. * **Tests** * Added coverage for integration scheduling behavior (local caps, invalid inputs, and CI/coverage scenarios). * **Documentation** * Added test-suite documentation with a local performance snapshot and key test hotspots. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Hadar301
pushed a commit
to Hadar301/NemoClaw-OpenShift
that referenced
this pull request
Jul 12, 2026
…e step into modules (NVIDIA#6444) <!-- markdownlint-disable MD041 --> ## Summary Extracts cohesive units of the sandbox create/register orchestration out of the ~4,900-line `src/lib/onboard.ts` entrypoint into focused modules under `src/lib/onboard/`, following the injected-deps boundary style established by `created-sandbox-finalization.ts` (NVIDIA#6332). The primary goal is maintainability and independent unit-test coverage for the create path, with intentional safety/behavior refinements discovered during review: redacted create-output failure echoing, preservation of non-zero create-stream status when readiness fails, direct argv spawning for trusted create paths, fail-closed Docker-GPU create-poll side-effect handling, and redacted trace reporting for poll/readiness errors. Note on the issue's premise: NVIDIA#6258 cites a `+27 net lines` growth-guard violation from NVIDIA#6166. That premise is stale — the merged NVIDIA#6166 left `onboard.ts` net-smaller, and NVIDIA#6276/NVIDIA#6332 shrank it further, so the guard (`.github/workflows/codebase-growth-guardrails.yaml`, a per-PR net-neutral diff gate) is not currently red. This PR is therefore incremental maintainability work; it keeps `onboard.ts` net-neutral-or-smaller so the gate stays green. ## Related Issue Refs NVIDIA#6258 <!-- Refs (not Fixes): this PR lands two increments of a larger extraction; the remaining create/finalize wiring is left for follow-ups, so the issue should stay open. --> ## Changes - Add `src/lib/onboard/created-sandbox-failure.ts`: `reportSandboxCreateFailure` (warns-and-continues on an incomplete create; otherwise prints diagnostics + recovery hints and exits) and `reportSandboxReadinessFailure` (prints the readiness failure, defers cleanup to the Docker-GPU patch or deletes the failed sandbox, then exits). `onboard.ts` replaces the two inline blocks with module calls — net −2 lines. - Add `src/lib/onboard/sandbox-create-step.ts`: `runSandboxCreateStep` encapsulates the BuildKit prebuild handoff → Docker-GPU create-patch provisioning → create-stream behind a context + injected-deps boundary. This move is **line-neutral** on `onboard.ts`; its value is a named, unit-testable boundary (the `prepare → patch → stream` sequence is now testable without standing up the entrypoint), not a size reduction. Build-context and exit-listener cleanup stay with the caller that armed them. - Add focused unit tests across `created-sandbox-failure.test.ts`, `sandbox-create-step.test.ts`, `sandbox-create-launch.test.ts`, `create-stream.test.ts`, `create-stream-argv.test.ts`, and `create-stream-ready-gate.test.ts` covering failure branches, redaction, exit-code preservation, GPU vs non-GPU readiness cleanup, prebuild/patch/stream wiring, direct argv spawn boundaries, terminal-agent/default-driver ready-check gating, fail-closed `onPoll` error handling, and redacted poll/readiness trace behavior. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: no CLI flags, commands, configuration, or documented user workflow changed. The user-visible differences are limited to safer failure-path diagnostics (credential redaction), more accurate readiness-failure exit status, and fail-closed create-poll error handling. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: onboarding/sandbox path. Extraction boundaries were verified against the pre-extraction source; intentional safety refinements are explicitly covered by tests: create output is redacted before failure logging, readiness failure preserves a non-zero create-stream status instead of flattening to `1`, Docker-GPU during-create polling is isolated from readiness detection via `onPoll`, escaping poll errors abort create with classified/generic failure text plus redacted trace emission, and ready-check exceptions emit redacted trace evidence without falsely forcing Ready. Requesting maintainer sensitive-path review. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result: `npx vitest run --project cli src/lib/actions/sandbox/snapshot.test.ts src/lib/onboard/created-sandbox-failure.test.ts src/lib/onboard/sandbox-create-launch.test.ts src/lib/onboard/sandbox-create-step.test.ts src/lib/sandbox/create-stream.test.ts src/lib/sandbox/create-stream-ready-gate.test.ts src/lib/sandbox/create-stream-argv.test.ts` → 107 passed; `npx tsc --noEmit --pretty false --project tsconfig.cli.json` → clean; `npm run test-size:check` → passed; `npx prek run --all-files --stage pre-commit --skip source-shape-test-budget --skip test-skills-yaml` → passed; `npm run test-conditionals:scan -- --top 25` → no new changed-file conditional failures; `git diff --check` → clean. - [x] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Required live E2E run [29021019725](https://github.com/NVIDIA/NemoClaw/actions/runs/29021019725) passed on exact head `82518f7e6f69c39d9bc04f63a365753aac7b1b6d`: `cloud-onboard`, `onboard-repair`, `onboard-resume`, `state-backup-restore`, `upgrade-stale-sandbox`, and `snapshot-commands` all succeeded. PR CI checks also passed on the same head after rerunning flaky `policy-channel-list.test.ts` shard timeout (`cli-test-shards (3)`). - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Dongni Yang <dongniy@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved sandbox onboarding using a dedicated creation step that centralizes prebuild handoff, streaming create execution, GPU patch wiring, and readiness capture. * **Bug Fixes** * More consistent, centralized handling for both create failures and readiness failures, including redacted output, clearer diagnostics, and reliable retry guidance. * Safer cleanup on readiness failures to avoid same-name collisions, with correct behavior for GPU-enabled flows. * **Tests** * Added coverage for create/readiness failure reporting, exit-code fallback, cleanup/command messaging, and early-detach behavior in create-stream. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Dongni Yang <dongniy@nvidia.com> Signed-off-by: Julie Yaunches <jyaunches@nvidia.com> Signed-off-by: Carlos Villela <cvillela@nvidia.com> Co-authored-by: Julie Yaunches <jyaunches@nvidia.com> Co-authored-by: Carlos Villela <cvillela@nvidia.com>
wscurran
added
area: ci
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
3 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
Replace unit-shaped test subprocesses with direct typed seams while retaining meaningful real-process contracts. The final onboarding pilot is 20.5% faster, and the OpenClaw config target is 40.5% faster, without changing production defaults or CLI behavior.
Related Issue
Part of #6245.
Changes
createSetupInferencetest seam into a focused module while preserving the production dependency wiring and reducingsrc/lib/onboard.tsby 65 net lines.setupNimboundaries for all five native-Docker Windows-provider rejection scenarios.setupInference/OpenShell process boundary proving raw credentials never enter argv and only the provider update child receives the explicitly scoped credential environment.openclaw doctorlaunches and generator/applier executable contracts.test/generate-openclaw-config.test.tsby removing its file-wide@ts-nocheckdirective.test/onboard-selection.test.ts, 4,057 fortest/onboard.test.ts, and 1,945 fortest/generate-openclaw-config.test.ts.execveattempts from 7,654 to 6,444 (15.8%).execvecalls reduced from 294 to 196.Type of Change
Quality Gates
Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: broad pilot gate:npm test(1,128 files, 12,621 tests);npm run test:coverage:cli(1,004 files, 11,063 tests, all ratchets); current-head pre-commit and pre-push repository/type-check gates passed.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com
Summary by CodeRabbit
New Features
/responsesto chat completions.Bug Fixes
Tests