refactor(onboard): let the runner enter preflight - #7675
Conversation
|
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. |
📝 WalkthroughWalkthroughOnboarding core phases now use separate provider-inference and sandbox builders with keyed execution. Strict initial-flow handling owns ChangesOnboarding flow refactor
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant runInitialOnboardFlowSlice
participant flowSlices
participant runCoreOnboardFlowSlice
participant providerInference
participant sandbox
runInitialOnboardFlowSlice->>flowSlices: execute init or preflight entry state
flowSlices->>runCoreOnboardFlowSlice: continue with keyed core phases
runCoreOnboardFlowSlice->>providerInference: run provider inference
providerInference-->>runCoreOnboardFlowSlice: phase result
runCoreOnboardFlowSlice->>sandbox: run sandbox
sandbox-->>runCoreOnboardFlowSlice: phase result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit d91b20e in the TypeScript / code-coverage/cliThe overall coverage in commit d91b20e in the Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. Since last review: 0 prior items resolved · 0 still apply · 0 new items found E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
<!-- markdownlint-disable MD041 --> ## Summary This cumulative stack layer separates provider/inference and sandbox phase construction by effect boundary, then routes exact `provider_selection` entries through the strict FSM runner for fresh and resumed sessions. Compatibility replay remains for saved sessions already at `inference`, `sandbox`, a branch state, or a final-flow handoff state. Stack layers 5 and 6 of 6. Base: `codex/onboard-fsm-initial-entry`. ## Related Issue Refs #6224 ## Changes - Replace the combined core-phase factory with named provider/inference and sandbox factories. - Give each factory a phase-specific option and dependency contract. - Share endpoint-provenance inputs explicitly across the two phases. - Replace positional phase tuples with a named `{ providerInference, sandbox }` object. - Remove `provider_selection` from the resume compatibility state set. - Apply provider-selection, inference, sandbox, and branch transitions through the strict runner at exact entry. - Preserve provenance handoff, sandbox registry updates, and compatibility repair for downstream resume states. - Strengthen tests for context propagation, one sandbox effect, transition kinds, and the absence of compatibility recorders on strict entry. The strict-entry migration and live-slice probes now isolate provider/inference construction from sandbox effects. `runOnboard` and `runCoreOnboardFlowSlice` consume the split, while core-phase and live-slice tests protect ordering, provenance, compatibility scope, state transitions, and single-effect execution. ## 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: Commands, output, configuration, persisted contracts, and supported onboarding behavior 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 Codex review of the exact six-commit stack found no actionable findings and verified strict entry, compatibility bounds, state durability, recovery, rollback, and fail-closed behavior. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `no-docs-needed` - Evidence: The cumulative change splits provider/inference and sandbox construction, then routes exact `provider_selection` entry for fresh and resumed sessions through the strict runner. Downstream resume states retain compatibility replay. Commands, output, configuration, persisted contracts, and supported onboarding behavior are unchanged; existing documentation remains accurate. - Agent: Codex Desktop documentation-writer subagent <!-- docs-review-head-sha: be3764a --> <!-- docs-review-agents-blob-sha: be20a09 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: Not applicable - Station profile/scenario: Not applicable - Result: Not applicable; `scripts/prepare-dgx-station-host.sh` is unchanged. - Supporting evidence: Not applicable ## Verification - [x] PR description includes a `Signed-off-by:` line 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 — exact stack-head FSM selection: PASS (9 files, 112 tests); focused CLI Vitest: PASS (18 tests); focused integration Vitest: PASS (12 tests); CLI type-check: PASS. - [x] Applicable broad gate passed — exact-head GitHub CLI, platform, security, and selected live E2E gates passed; `cloud-onboard`, `onboard-repair`, and `onboard-resume` all succeeded. - [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: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Refined onboarding flow handling for provider inference and sandbox setup. * Improved endpoint provenance tracking during onboarding and sandbox resumption. * Updated resume compatibility behavior for more reliable session transitions. * **Bug Fixes** * Prevented invalid or unexpected onboarding states from triggering phase execution. * Improved handling of persisted sandbox registry information during resumed onboarding. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
@coderabbitai review |
✅ Action performedReview finished.
|
e5ba515
into
codex/onboard-fsm-final-recovery
Summary
This cumulative stack change makes the strict FSM runner the transition authority at exact
init,preflight, andprovider_selectionentries. It removes the entrypoint-ownedinit -> preflighttransition, separates provider/inference and sandbox phase construction by effect boundary, and preserves compatibility replay for saved sessions already downstream of the applicable exact entry. Commands, output, configuration, persisted contracts, and supported onboarding behavior are unchanged.Stack layers 4–6 of 6. Base:
codex/onboard-fsm-final-recovery.Related Issue
Refs #6224
Changes
recordInitialPreflightTransitionand its pre-slice call fromrunOnboard.initphase and strict runner owninit -> preflightfor fresh and resumed sessions.provider_selectionfrom the resume compatibility state set.Type of Change
Quality Gates
Documentation Writer Review
docs-updatedsrc/lib/onboard/lifecycle-contracts.mdnow assignsinitto the synthetic phase applied by the strict initial-flow runner. The cumulative implementation also separates provider/inference and sandbox construction and gives the strict runner exactprovider_selectionentry. Downstream resume compatibility and user-facing onboarding contracts remain unchanged.DGX Station Hardware Evidence
scripts/prepare-dgx-station-host.shis unchanged.Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablecloud-onboard,onboard-repair, andonboard-resumeall succeeded.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Carlos Villela cvillela@nvidia.com