Skip to content

refactor(onboard): run final phases through FSM slice#4507

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

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

Conversation

@cv
Copy link
Copy Markdown
Collaborator

@cv cv commented May 29, 2026

Summary

Move the fresh-run agent/openclaw setup, policies, and finalization live call sites onto the final FSM flow slice. Resume remains on the compatibility path for now so branch setup, policy, and final verification checks continue to run for ahead-state sessions.

Changes

  • Build a final OnboardFlowContext from the core flow output in src/lib/onboard.ts.
  • Wrap branch setup, policy setup, and finalization as sequence phases.
  • Use runFinalOnboardFlowSequence(...) for fresh runs starting at openclaw or agent_setup.
  • 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: 6f3df188-99bd-4cef-a638-5b6ce52e420b

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-final-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, hermes-e2e, onboard-resume-e2e
Optional E2E: onboard-repair-e2e, openclaw-onboard-security-posture-e2e

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

Workflow run

Full advisor summary

E2E Recommendation Advisor

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

Required E2E

  • cloud-onboard-e2e (high): Required because this change affects the default OpenClaw onboarding path after sandbox creation, including policy selection and final deployment verification. This job runs a real cloud onboard with custom policy presets and validates sandbox health/security checks.
  • hermes-e2e (high): Required because the new final flow has an agent_setup branch for agent-based assistants. Hermes E2E validates install, onboard --agent hermes, health probing, and live inference through the affected final agent setup/policies/finalization sequence.
  • onboard-resume-e2e (high): Required because the diff changes the final onboard control flow while explicitly special-casing resume compatibility. This job validates interrupted onboarding followed by --resume through cached state and completion.

Optional E2E

  • onboard-repair-e2e (high): Useful adjacent coverage for repair/re-run behavior around onboard state and sandbox recovery, but less directly targeted than the default onboard, Hermes agent, and resume paths.
  • openclaw-onboard-security-posture-e2e (high): Optional extra confidence because the refactor wraps policy and final verification steps that also enforce the OpenClaw security posture, but cloud-onboard-e2e already exercises the primary OpenClaw final flow.

New E2E recommendations

  • onboarding state machine (medium): Existing E2E validates end-state success, but there does not appear to be a dedicated E2E assertion that the new final flow-slice path records and advances each final machine state in order for both openclaw and agent_setup branches.
    • Suggested test: Add a targeted final-flow-slice E2E that starts from a saved machine state of openclaw and agent_setup, runs non-resume onboarding, and asserts recorded machine transitions through policies and finalizing/post-verify without falling back to the compatibility path.

Dispatch hint

  • Workflow: .github/workflows/nightly-e2e.yaml
  • jobs input: cloud-onboard-e2e,hermes-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-core-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, 1 worth checking, 0 nice ideas
Top item: Add live final-FSM wiring tests for policy/finalization ordering

Review findings

🛠️ Needs attention

  • Add live final-FSM wiring tests for policy/finalization ordering (src/lib/onboard.ts:7373): The PR moves high-risk final onboarding host glue onto `runFinalOnboardFlowSequence`, but the diff changes only `src/lib/onboard.ts` and does not add or update tests for this live wiring. Existing generic runner/slice/handler tests do not prove that the actual `onboard.ts` integration preserves both branch states, policy application before final verification, the side-effectful `recordPostVerifyStarted()` plus `completeOnboardMachine()` interaction, or resume/ahead-state compatibility behavior. This also leaves the PR checklist clause 'Tests added or updated for new or changed behavior' unsupported by the diff and leaves security-sensitive ordering assurance weak.
    • Recommendation: Add targeted tests or clearly identify existing tests that exercise the changed `onboard.ts` wiring for fresh OpenClaw, fresh agent, resume/ahead compatibility, verification failure without completion, and policy/recovery/web-search/deployment verification ordering. For the compatibility path, also document why the source state cannot be normalized in this PR and when the workaround can be removed.
    • Evidence: The changed path calls `runFinalOnboardFlowSequence(...)` at line 7373 for fresh runs and falls back to manual phase execution for resume/ahead-state sessions at lines 7370-7389. The only changed file is `src/lib/onboard.ts`; nearby tests cover generic `flow-slices`, `runner`, and individual handlers, not this new live wiring.
  • Avoid growing the onboard.ts monolith for phase wiring (src/lib/onboard.ts:7221): `src/lib/onboard.ts` is already a large onboarding monolith and this PR adds net new inline phase wiring instead of extracting it. The deterministic drift check reports the file grew from 7468 to 7535 lines (+67), exceeding the monolith-growth threshold. There is already a reusable phase-factory area under `src/lib/onboard/machine/flow-phases/agent-policy-finalization.ts`, so adding more inline host glue increases review and merge-risk in an actively changing file.
    • Recommendation: Extract the final phase construction/dependency assembly into a focused helper/module, or otherwise offset the line growth in `src/lib/onboard.ts`. Prefer using or extending the existing `onboard/machine/flow-phases` abstractions so the main onboard function only composes phases.
    • Evidence: Diff stat: `src/lib/onboard.ts | 305 ++++++++++++++++++++++++++++++++---------------------`, 186 insertions and 119 deletions. Deterministic monolith delta: baseLines 7468, headLines 7535, delta +67.

🔎 Worth checking

  • Source-of-truth review needed: Resume/ahead-state final-phase 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: `src/lib/onboard.ts` lines 7370-7389 manually run branch setup, policies, and finalization outside `runFinalOnboardFlowSequence` for resume/ahead-state sessions.

🌱 Nice ideas

  • None.

Workflow run details

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

@wscurran wscurran added enhancement: feature Use this label to identify requests for new capabilities in NemoClaw. refactor This is a refactor of the code and/or architecture. labels 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

enhancement: feature Use this label to identify requests for new capabilities in NemoClaw. 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