Skip to content

refactor(onboard): run live sequence with record-only steps#4472

Draft
cv wants to merge 1 commit into
stack/onboard-fsm-provider-result-sequencefrom
stack/onboard-fsm-live-record-only-sequence
Draft

refactor(onboard): run live sequence with record-only steps#4472
cv wants to merge 1 commit into
stack/onboard-fsm-provider-result-sequencefrom
stack/onboard-fsm-live-record-only-sequence

Conversation

@cv
Copy link
Copy Markdown
Collaborator

@cv cv commented May 28, 2026

Summary

Switch the live manual onboarding sequence to record-only step helpers while preserving compatibility for resume/ahead states. Machine transitions now come from returned FSM results rather than implicit step-helper movement.

Changes

  • Configure the live OnboardRuntimeBoundary with stepMutationOptions: { updateMachine: false }.
  • Explicitly enter preflight from init before invoking the first handler when applicable.
  • Apply ordered provider/inference FSM results through the compatibility boundary instead of only the final result.
  • Keep stale/ahead result no-op behavior for existing resume flows and tests.

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
  • npm run 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

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@cv cv self-assigned this May 28, 2026
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 28, 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.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 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: d20e01b4-543b-4435-922b-84094b44149c

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 stack/onboard-fsm-live-record-only-sequence

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

@github-actions
Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: cloud-onboard-e2e, onboard-resume-e2e, double-onboard-e2e
Optional E2E: inference-routing-e2e, ubuntu-repo-cloud-openclaw-resume, ubuntu-repo-cloud-openclaw-double-provider-switch

Dispatch hint: cloud-onboard-e2e,onboard-resume-e2e,double-onboard-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/stack/onboard-fsm-provider-result-sequence
Head: HEAD
Confidence: high

Required E2E

  • cloud-onboard-e2e (high): Exercises the main non-interactive cloud onboarding path through preflight, provider inference setup, sandbox creation, policy setup, health, and security checks; this is the broadest required guard for changed onboard.ts state/result sequencing.
  • onboard-resume-e2e (high): Directly validates interrupted onboard state, session compatibility, and nemoclaw onboard --resume; required because this PR changes state-result recording and runtime-boundary machine mutation behavior.
  • double-onboard-e2e (high): Covers repeated onboarding/reuse behavior and provider setup across multiple onboard runs, which is relevant to the provider-inference result sequence change from a single result to multiple ordered results.

Optional E2E

  • inference-routing-e2e (medium): Useful adjacent confidence for provider route health, credential isolation, and inference error classification after provider-inference onboarding changes, but less directly tied to the FSM/session recording change than the required onboard tests.
  • ubuntu-repo-cloud-openclaw-resume (high): Typed scenario coverage for cloud OpenClaw resume-after-interrupt behavior would complement the legacy onboard-resume script and validate scenario-runner artifacts.
  • ubuntu-repo-cloud-openclaw-double-provider-switch (high): Typed scenario coverage for provider switching is adjacent to the provider-inference stateResults sequence and can catch result-ordering regressions across repeated provider configuration.

New E2E recommendations

  • onboarding FSM result-order assertions (high): Existing E2E coverage validates successful/resumed user flows, but does not appear to explicitly assert the exact ordered machine results introduced here, such as init -> preflight and multiple provider-inference sub-results while legacy step compatibility remains stable.
    • Suggested test: Add an E2E assertion to the typed scenario runner or onboard-resume script that inspects onboarding/session artifacts and verifies ordered state results for init, preflight, provider inference, sandbox, and finalization without premature legacy step mutation.

Dispatch hint

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

@github-actions
Copy link
Copy Markdown
Contributor

E2E Scenario Advisor Recommendation

Required scenario E2E: None
Optional scenario E2E: None

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/stack/onboard-fsm-provider-result-sequence
Head: HEAD
Confidence: high

Required scenario E2E

  • None. No scenario workflow, scenario metadata, scenario runtime, or validation-suite files changed.

Optional scenario E2E

  • None.

Relevant changed files

  • None.

@github-actions
Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 1 needs attention, 1 worth checking, 0 nice ideas
Top item: Add live onboarding FSM sequencing coverage

Review findings

🛠️ Needs attention

  • Add live onboarding FSM sequencing coverage (src/lib/onboard.ts:6384): This PR changes the live onboarding composition to make legacy step helpers record-only, explicitly enter preflight, and replay all provider/inference FSM results. The PR body also checks 'Tests added or updated for new or changed behavior', but the diff changes only src/lib/onboard.ts and adds no test coverage for the live glue path. Existing lower-level tests cover the runtime boundary and provider handler result sequences, but not the changed integration point where updateMachine:false, init->preflight, resume/ahead no-op compatibility, and provider retry replay interact.
    • Recommendation: Add or point to a targeted integration/unit test around the live onboard boundary wiring that verifies a fresh run records init->preflight with record-only step mutations, a provider-selection retry applies the ordered stateResults through sandbox, and an ahead/resume machine snapshot no-ops stale results safely.
    • Evidence: DiffStat shows one changed file: src/lib/onboard.ts. The changed code sets stepMutationOptions: { updateMachine: false }, calls advanceTo('preflight', { metadata: { state: 'init' } }), and loops over providerInferenceResult.stateResults, but no test file is changed despite the PR verification checkbox claiming tests were added or updated.

🔎 Worth checking

  • Source-of-truth review needed: Live onboarding FSM compatibility boundary: The advisor marked localized patch analysis as needs_followup.
    • Recommendation: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
    • Evidence: The PR adds updateMachine:false, explicit init->preflight, and ordered provider stateResults replay in src/lib/onboard.ts while retaining recordStateResultWithStepCompatibility no-op semantics.

🌱 Nice ideas

  • None.

Workflow run details

This is an automated advisory review. A human maintainer must make the final merge decision.

@cv cv added the onboarding Making the onboarding experience better label May 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

onboarding Making the onboarding experience better

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant