Skip to content

refactor(onboard): run core phases through FSM slice#4500

Draft
cv wants to merge 1 commit into
stack/onboard-fsm-use-initial-slicefrom
stack/onboard-fsm-use-core-slice
Draft

refactor(onboard): run core phases through FSM slice#4500
cv wants to merge 1 commit into
stack/onboard-fsm-use-initial-slicefrom
stack/onboard-fsm-use-core-slice

Conversation

@cv
Copy link
Copy Markdown
Collaborator

@cv cv commented May 29, 2026

Summary

Move the fresh-run provider/inference and sandbox live call sites onto the core FSM flow slice. Resume remains on the compatibility path for now so provider and sandbox repair checks still run even when saved machine state is already ahead.

Changes

  • Build a core OnboardFlowContext from the initial flow output in src/lib/onboard.ts.
  • Wrap the existing provider/inference and sandbox handler calls as sequence phases.
  • Use runCoreOnboardFlowSequence(...) for fresh runs that start at provider_selection.
  • Preserve compatibility execution for resume/ahead-state sessions.

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 29, 2026
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 29, 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 29, 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: 558d24f2-755d-44a8-a71a-ffc5f72ce4a3

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-use-core-slice

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
Optional E2E: onboard-repair-e2e, double-onboard-e2e, onboard-inference-smoke-e2e, ubuntu-repo-cloud-openclaw

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

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/stack/onboard-fsm-use-initial-slice
Head: HEAD
Confidence: high

Required E2E

  • cloud-onboard-e2e (high): Required because src/lib/onboard.ts changed the non-resume core onboarding path from provider selection through sandbox setup. This job performs a real cloud OpenClaw onboard and validates sandbox health, credential/security posture, and inference.local behavior.
  • onboard-resume-e2e (high): Required because the refactor keeps resume on a compatibility path but now runs provider and sandbox handling through phase wrappers. This job validates interrupted onboard state, resume completion, credential hydration, and cached preflight/gateway behavior.

Optional E2E

  • onboard-repair-e2e (high): Useful adjacent coverage for the same resume/sandbox phase refactor: verifies missing recorded sandbox repair and rejection of conflicting resumed provider/model/name state.
  • double-onboard-e2e (high): Optional confidence for repeated onboard and gateway/sandbox reuse after the provider_selection and sandbox phases were moved into the core flow sequence.
  • onboard-inference-smoke-e2e (medium): Optional regression guard that onboard does not report success until the configured provider/model route serves a real completion; relevant because provider inference output propagation changed.
  • ubuntu-repo-cloud-openclaw (medium): Optional typed scenario coverage for the canonical Ubuntu repo cloud OpenClaw setup plan with smoke, inference, credentials, and baseline-onboarding suites.

New E2E recommendations

  • onboarding state-machine flow (high): Existing resume E2E interrupts after sandbox/OpenClaw setup at the policies step. This PR introduces a new core sequence boundary around provider_selection and sandbox, but there is no targeted E2E that interrupts between those two states and resumes from the saved machine state.
    • Suggested test: Add an E2E that forces interruption immediately after provider_selection before sandbox creation, then runs onboard --resume and asserts the saved machine state advances through sandbox exactly once with provider/model/credential fields preserved.

Dispatch hint

  • Workflow: .github/workflows/nightly-e2e.yaml
  • jobs input: cloud-onboard-e2e,onboard-resume-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-use-initial-slice
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: 2 needs attention, 3 worth checking, 0 nice ideas
Top item: Missing provider/sandbox FSM parity tests

Review findings

🛠️ Needs attention

  • Missing tests for the new provider/sandbox FSM wiring (src/lib/onboard.ts:7158): The PR body checks “Tests added or updated for new or changed behavior,” but the deterministic diff changes only src/lib/onboard.ts and no test files. This refactor changes the execution path for provider selection, inference, sandbox creation, state-result application, and branch continuation on fresh runs, so generic flow-slice tests are not enough to prove parity with the previous manual sequencing.
    • Recommendation: Add targeted tests or an equivalent checked-in runtime/integration harness for the src/lib/onboard.ts wiring: fresh machine state provider_selection should run provider then sandbox through runCoreOnboardFlowSequence, apply provider retry result arrays in order, preserve sandboxName/model/provider/webSearchConfig/selectedMessagingChannels/hermesToolGateways/nimContainer for downstream phases, and branch correctly to agent_setup or openclaw.
    • Evidence: Diff stat shows only src/lib/onboard.ts changed. The new code at the core flow conditional calls runCoreOnboardFlowSequence for !resume && machine.state === "provider_selection" and otherwise uses the manual compatibility path; no changed test file exercises that integration.
  • Onboarding monolith grows while adding inline phase wrappers (src/lib/onboard.ts:6999): The current onboarding monolith grows by 69 lines in a file already around 7.4k lines. The new provider and sandbox phase wrapper construction is embedded inline inside onboard(), which increases the size and coupling of a high-risk host/sandbox orchestration function during an active FSM migration.
    • Recommendation: Extract the core phase construction into a focused helper/module or offset the growth by moving existing phase construction out of src/lib/onboard.ts. Keep dependency injection explicit, but avoid further expanding the monolithic onboard() body.
    • Evidence: Deterministic monolith delta reports src/lib/onboard.ts baseLines=7399, headLines=7468, delta=69, severity=blocker. The diff adds CoreOnboardFlowContext plus providerInferencePhase and sandboxPhase inline in onboard().

🔎 Worth checking

  • Source-of-truth review needed: src/lib/onboard.ts core-flow resume compatibility path: 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: Lines around the new core-flow conditional state: “Keep resume on the compatibility path for now: resume can intentionally re-run provider/sandbox repair checks even when saved machine state is ahead.”
  • Compatibility fallback lacks source-of-truth exit criteria (src/lib/onboard.ts:7158): The new compatibility-path comment explains that resume intentionally reruns provider/sandbox repair checks even when saved machine state is ahead, but it does not document why the source state cannot be modeled directly in this PR, what regression test protects this behavior, or when the workaround can be removed.
    • Recommendation: Document the invalid persisted-state/source boundary and add a regression test for resume/ahead-state sessions that verifies provider and sandbox repair checks still run. Include a removal condition, such as when those repair checks are represented as first-class resumable FSM states.
    • Evidence: Localized patch signal: “Keep resume on the compatibility path for now: resume can intentionally re-run provider/sandbox repair checks even when saved machine state is ahead.” No corresponding changed test or removal condition is present in the diff.
  • Security-relevant onboarding path needs parity validation (src/lib/onboard.ts:7158): No direct sandbox escape, SSRF bypass, policy bypass, credential leak, blueprint tampering, installer-trust, or workflow-boundary vulnerability was found. However, this is high-risk sandbox lifecycle glue: a sequencing regression could skip or reorder credential cleanup, model-router reconciliation, sandbox drift checks, messaging/web-search config propagation, or branch-to-policy setup.
    • Recommendation: Cover the new FSM-driven fresh path with tests that verify provider cleanup and sandbox validation side effects are preserved, and that resume/ahead-state compatibility still reruns repair checks rather than trusting stale machine state.
    • Evidence: The new core path delegates provider_selection and sandbox to runCoreOnboardFlowSequence; existing flow-slice tests cover generic runner behavior, not this src/lib/onboard.ts integration.

🌱 Nice ideas

  • None.

Workflow run details

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

@wscurran wscurran added the refactor This is a refactor of the code and/or architecture. label May 29, 2026
@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 refactor This is a refactor of the code and/or architecture.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants