Skip to content

refactor(onboard): extract selection drift helpers#3298

Closed
cv wants to merge 8 commits into
refactor/onboard-compatible-smokefrom
refactor/onboard-selection-drift
Closed

refactor(onboard): extract selection drift helpers#3298
cv wants to merge 8 commits into
refactor/onboard-compatible-smokefrom
refactor/onboard-selection-drift

Conversation

@cv
Copy link
Copy Markdown
Collaborator

@cv cv commented May 8, 2026

Summary

Extract sandbox provider/model selection drift detection out of the large onboarding module. This continues the stacked onboarding cleanup by isolating the sandbox selection-config download/parsing path and adding direct unit coverage.

Changes

  • Add src/lib/onboard/selection-drift.ts for finding downloaded selection configs, reading sandbox selection metadata, and computing provider/model drift.
  • Update src/lib/onboard.ts to call the extracted drift helper with the existing OpenShell runner dependency.
  • Add unit tests for selection-config discovery, failed download handling, temp cleanup, unknown drift, and provider/model drift.
  • Update the existing source-shape regression test to follow the extracted helper file.

Type of Change

  • 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)

Verification

  • npx prek run --all-files passes
  • npm test passes
  • Tests added or updated for new or changed behavior
  • No secrets, API keys, or credentials committed
  • Docs updated for user-facing behavior changes
  • make docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: Carlos Villela cvillela@nvidia.com

@cv cv self-assigned this May 8, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 8, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 49141517-7efa-4404-96b7-cbf22e911106

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/onboard-selection-drift

Comment @coderabbitai help to get the list of available commands and usage tips.

@cv cv marked this pull request as draft May 9, 2026 01:23
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 9, 2026

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.

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 9, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@cv cv added the v0.0.39 Release target label May 9, 2026
@wscurran wscurran added enhancement: testing Use this label to identify requests to improve NemoClaw test coverage. refactor This is a refactor of the code and/or architecture. and removed v0.0.39 Release target labels May 11, 2026
@cv cv added the v0.0.39 Release target label May 11, 2026
@cv cv added v0.0.40 Release target and removed v0.0.39 Release target labels May 12, 2026
@github-actions
Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: None
Optional E2E: double-onboard-e2e, openclaw-inference-switch-e2e, cloud-onboard-e2e, onboard-resume-e2e

Dispatch hint: double-onboard-e2e,openclaw-inference-switch-e2e,cloud-onboard-e2e,onboard-resume-e2e

Workflow run

Full advisor summary

Pi Semantic E2E Advisor

Base: origin/refactor/onboard-compatible-smoke
Head: HEAD
Confidence: high

Required E2E

  • None. This PR is a mechanical extraction of selection-drift helpers out of src/lib/onboard.ts into src/lib/onboard/selection-drift.ts with dependency injection of runOpenshell. No change in drift-detection semantics, prompt wording, or createSandbox reuse/recreate branching. New vitest coverage plus the existing test/onboard.test.ts source-regex assertions validate the wiring. Nightly onboarding E2E jobs already cover drift-driven recreation on schedule; running them on this PR would be a confidence check rather than a merge gate.

Optional E2E

  • double-onboard-e2e (medium): test/e2e/test-double-onboard.sh re-onboards the same sandbox name with varying provider/model configs, which is exactly the drift-detection code path refactored here. Confirms no behavioral regression when reusing vs. recreating an existing sandbox.
  • openclaw-inference-switch-e2e (medium): Exercises switching provider/model on a running OpenClaw sandbox and validates registry + session + openclaw.json state — closest live coverage of the selection configuration semantics that getSelectionDrift reads from the sandbox.
  • cloud-onboard-e2e (medium): End-to-end non-interactive onboard happy path covering the createSandbox code path that now wires runOpenshell into getSelectionDrift. Guards against any accidental breakage in the dependency-injection refactor under real gateway I/O.
  • onboard-resume-e2e (medium): The resume path interacts with sandbox reuse decisions; worth a confidence check after modifying the sandbox-reuse drift gate.

New E2E recommendations

  • onboarding (low): No existing E2E explicitly asserts that a second nemoclaw onboard run with a different --provider/--model on an existing ready sandbox triggers the selection-drift prompt/non-interactive auto-recreate path. The current refactor is covered only by unit-level assertions on source-code regexes. A targeted E2E would catch regressions in getSelectionDrift's interaction with openshell sandbox download under real gateway conditions.
    • Suggested test: Add a test/e2e/test-selection-drift-recreate.sh scenario: onboard with provider=A/model=X, then re-onboard the same sandbox name with provider=B/model=Y under NEMOCLAW_NON_INTERACTIVE=1 and assert (a) the sandbox is recreated, (b) /sandbox/.nemoclaw/config.json reflects provider=B/model=Y, and (c) the log contains 'Recreating sandbox due to provider/model drift.'

Dispatch hint

  • Workflow: nightly-e2e.yaml
  • jobs input: double-onboard-e2e,openclaw-inference-switch-e2e,cloud-onboard-e2e,onboard-resume-e2e

@ericksoa ericksoa deleted the branch refactor/onboard-compatible-smoke May 12, 2026 17:35
@ericksoa ericksoa closed this May 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement: testing Use this label to identify requests to improve NemoClaw test coverage. refactor This is a refactor of the code and/or architecture. v0.0.40 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants