refactor(onboard): extract modules from onboard.ts (WIP)#2087
Closed
refactor(onboard): extract modules from onboard.ts (WIP)#2087
Conversation
Move provider metadata constants (REMOTE_PROVIDER_CONFIG, endpoint URLs, LOCAL_INFERENCE_PROVIDERS, DISCORD_SNOWFLAKE_RE), lookup helpers (getEffectiveProviderName, getNonInteractive*, getRequested*Hint), gateway CRUD (upsertProvider, providerExistsInGateway, upsertMessagingProviders, buildProviderArgs), and getSandboxInferenceConfig into a dedicated onboard-providers module. Add getProviderLabel() to consolidate the scattered if/else provider name→label chains (used in printDashboard). Functions that need runOpenshell accept it as a last parameter (dependency injection) to avoid circular requires. onboard.ts retains thin wrappers with the original signatures. Update source-scanning tests to read onboard-providers.ts where the code now lives.
Move the entire Ollama auth-proxy subsystem into a dedicated module: token/PID persistence, proxy spawn/kill lifecycle, model prompt/pull, and the exposure warning. Zero coupling to other onboard logic — all dependencies (runner, ports, local-inference, credentials, model-prompts) are imported directly.
Move inference endpoint probe functions (parseJsonObject, hasResponsesToolCall, shouldRequireResponsesToolCalling, getProbeAuthMode, getValidationProbeCurlArgs, probeResponsesToolCalling, probeOpenAiLikeEndpoint, probeAnthropicEndpoint) into a dedicated module. All dependencies (http-probe, credentials, platform, validation) are imported directly — no circular deps. The 4 validate* wrapper functions remain in onboard.ts for now as they depend on promptValidationRecovery; they'll consolidate when setupNim moves in a later step. Update source-scanning tests to read from the new compiled file.
Move dashboard URL management, port forwarding, token retrieval, access info, guidance lines, and the summary printer into a dedicated module. Functions needing runOpenshell/openshellShellCommand accept them as DI parameters; onboard.ts provides thin wrappers. printDashboard now uses getProviderLabel() from onboard-providers.
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
5 tasks
jyaunches
added a commit
that referenced
this pull request
Apr 27, 2026
…d) (#2495) ## ⚡ Interim PR — Unblocks #2306 This is an **interim extraction PR** that lands the foundational module decomposition needed by #2306 (extract rebuild/recreate path + canonicalize credential resolution). It is intentionally scoped to pure code movement with no behavior changes. Rebased onto current main. Supersedes #2087 and #2489. --- ## Summary Steps 1–3 of the original extraction plan (authored by @ericksoa in #2087): - Extract `onboard-providers.ts` — provider metadata, CRUD, labels, inference config - Extract `onboard-ollama-proxy.ts` — proxy lifecycle, model pull/validate - Extract `onboard-inference-probes.ts` — endpoint validation probes (OpenAI, Anthropic, Gemini) Reduces `onboard.ts` by ~1,000 lines. All 3 new modules are under 400 lines. No behavior changes — all existing exports remain accessible via re-exports and thin wrappers. **Step 4 (dashboard extraction) was dropped** — main already landed `dashboard-contract.ts`, `dashboard-health.ts`, and `dashboard-recover.ts` (#2398) which supersede that extraction. Steps 5–10 (gateway, preflight, messaging, policies, inference-setup, sandbox) remain deferred per the original plan. ## Downstream: #2306 `#2306` (extract rebuild/recreate path + canonicalize credential resolution) builds directly on top of this PR. It imports `REMOTE_PROVIDER_CONFIG`, `upsertProvider`, and `ensureOllamaAuthProxy` from the new extracted modules via onboard.ts re-exports. Merging this PR first gives #2306 a clean module surface to build on. ## CodeRabbit comments All 6 comments addressed: - 1 resolved (deleted planning doc) - 4 are valid improvements on **pre-existing code** that was moved as-is — intentionally out of scope for a pure extraction PR, flagged as follow-up material - 1 is incorrect (`upsertMessagingProviders` was never exported on main) ## Rebase notes - Ported all main-side refinements into the extracted modules (e.g., `startOllamaAuthProxy` now returns `boolean`, `getProbeAuthMode` always returns `undefined`) - Added type annotations for the CJS `require()` imports to satisfy strict TypeScript - Re-imported endpoint URL constants that `onboard.ts` still references after extraction ## Test plan - [x] `npx tsc -p tsconfig.src.json --noEmit` compiles cleanly - [x] `npx tsc -p tsconfig.cli.json --noEmit` compiles cleanly (pre-existing failures only, same as main) - [x] Affected tests pass: gemini-probe-auth, credential-exposure, wsl2-probe-timeout - [x] CI: all 15 checks green - [ ] Manual smoke test: `nemoclaw onboard` completes successfully Co-authored-by: Aaron Erickson <aerickson@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Reorganized onboarding logic into modular utilities for improved code maintainability and separation of concerns. * Separated provider management, inference endpoint validation, and Ollama proxy lifecycle into dedicated modules. * **Tests** * Updated tests to reflect new code organization structure. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Aaron Erickson <aerickson@nvidia.com>
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
Rebased onto current main (post-#2441 merge). Steps 1–3 of the original plan:
onboard-providers.ts— provider metadata, CRUD, labels, inference configonboard-ollama-proxy.ts— proxy lifecycle, model pull/validateonboard-inference-probes.ts— endpoint validation probes (OpenAI, Anthropic, Gemini)Reduces
onboard.tsby ~1,000 lines. All 3 new modules are under 400 lines. No behavior changes — all existing exports remain accessible via re-exports and thin wrappers.Step 4 (dashboard extraction) was dropped — main already landed
dashboard-contract.ts,dashboard-health.ts, anddashboard-recover.ts(#2398) which supersede that extraction.Steps 5–10 (gateway, preflight, messaging, policies, inference-setup, sandbox) remain deferred per the original plan.
Downstream: #2306
#2306(extract rebuild/recreate path + canonicalize credential resolution) builds directly on top of this PR. It importsREMOTE_PROVIDER_CONFIG,upsertProvider, andensureOllamaAuthProxyfrom the new extracted modules via onboard.ts re-exports. Merging this PR first gives #2306 a clean module surface to build on.Rebase notes
startOllamaAuthProxynow returnsboolean,getProbeAuthModealways returnsundefined)require()imports to satisfy strict TypeScriptTest plan
npx tsc -p tsconfig.src.json --noEmitcompiles cleanlynpm test(CI)nemoclaw onboardcompletes successfullyCo-authored-by: Aaron Erickson aerickson@nvidia.com
Co-authored-by: jyaunches jyaunches@nvidia.com