Skip to content

refactor(onboard): derive progress labels from FSM metadata#4364

Draft
cv wants to merge 1 commit into
stack/onboard-fsm-step-mappingfrom
stack/onboard-fsm-progress-metadata
Draft

refactor(onboard): derive progress labels from FSM metadata#4364
cv wants to merge 1 commit into
stack/onboard-fsm-step-mappingfrom
stack/onboard-fsm-progress-metadata

Conversation

@cv
Copy link
Copy Markdown
Collaborator

@cv cv commented May 27, 2026

Summary

Move onboard progress label lookup behind machine metadata. The skip/resume banner path now reads progress labels from a small FSM progress helper instead of a hand-maintained table in src/lib/onboard.ts.

Changes

  • Add src/lib/onboard/machine/progress.ts to expose onboarding progress-step metadata derived from state definitions plus the existing messaging pseudo-step.
  • Replace ONBOARD_STEP_INDEX in src/lib/onboard.ts with getOnboardProgressStep().
  • Add progress metadata tests that preserve the existing eight-step labels and verify state-backed labels come from FSM definitions.

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

Summary by CodeRabbit

  • Refactor

    • Improved onboarding progress tracking and step messaging to enhance consistency and reliability throughout the entire onboarding workflow.
  • Tests

    • Added comprehensive test coverage for onboarding progress functionality to ensure consistency and reliability of step information across all onboarding stages.

Review Change Stack

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

copy-pr-bot Bot commented May 27, 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 27, 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: 7887f97e-45b0-4772-96e8-a799a3d9c647

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:

  • ✅ Review completed - (🔄 Check again to review again)
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch stack/onboard-fsm-progress-metadata

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: onboard-resume-e2e
Optional E2E: cloud-onboard-e2e, onboard-repair-e2e

Dispatch hint: onboard-resume-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/stack/onboard-fsm-step-mapping
Head: HEAD
Confidence: high

Required E2E

  • onboard-resume-e2e (high): This is the most targeted existing E2E for the changed skip/progress path: it interrupts onboard, resumes, verifies skipped preflight/gateway/sandbox/inference behavior, and confirms the session reaches complete. The changed skippedStepMessage() path is exercised directly during resume.

Optional E2E

  • cloud-onboard-e2e (high): Useful baseline confidence that a normal non-interactive cloud onboarding still reaches completion after moving progress metadata out of onboard.ts and changing machine definitions, but less targeted than the resume E2E.
  • onboard-repair-e2e (high): Adjacent coverage for onboard recovery/repair state handling that may share skip/reuse messaging paths, but the diff is most directly covered by resume.

New E2E recommendations

  • onboarding progress labels (medium): Existing resume E2E checks skip messages and completion but does not appear to assert the canonical visible progress labels/totals such as [1/8] Preflight checks or [7/8] Setting up OpenClaw inside sandbox. A regression in step totals could pass unless the flow otherwise breaks.
    • Suggested test: Extend onboard-resume-e2e to assert resumed skipped steps print the canonical [n/8] labels and that openclaw uses the branded agent title.

Dispatch hint

  • Workflow: nightly-e2e.yaml
  • jobs input: 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-step-mapping
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: 0 needs attention, 1 worth checking, 1 nice ideas
Top item: Document the messaging pseudo-step exception

Review findings

🛠️ Needs attention

  • None.

🔎 Worth checking

  • Source-of-truth review needed: src/lib/onboard/machine/progress.ts `EXTRA_PROGRESS_STEPS`: 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: `ONBOARD_PROGRESS_STEPS` is derived from machine definitions and then appends `EXTRA_PROGRESS_STEPS` with `stepName: "messaging"`.

🌱 Nice ideas

  • Document the messaging pseudo-step exception (src/lib/onboard/machine/progress.ts:17): The new progress helper intentionally derives most labels from FSM metadata but keeps `messaging` in a separate `EXTRA_PROGRESS_STEPS` list because messaging is still a user-visible step without a session/FSM state. That is compatible with today's code, but the exception has no inline note explaining why it remains outside `ONBOARD_MACHINE_STATE_DEFINITIONS`, when it should be removed, or whether `ONBOARD_PROGRESS_STEPS` is lookup-only.
    • Recommendation: Add a short comment near `EXTRA_PROGRESS_STEPS` explaining that messaging is currently emitted inside the sandbox flow rather than represented as a session/FSM state, and note the removal condition: either messaging becomes a real FSM/session step or the progress API stops preserving legacy pseudo-step labels.
    • Evidence: `ONBOARD_PROGRESS_STEPS` is derived from `ONBOARD_MACHINE_STATE_DEFINITIONS` plus `EXTRA_PROGRESS_STEPS`; `setupMessagingChannels()` still prints `step(5, 8, "Messaging channels")` directly, and `defaultSteps()`/FSM transitions do not define a `messaging` state.

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 27, 2026
@cv cv added the v0.0.55 Release target label May 27, 2026
@cv cv requested a review from ericksoa May 27, 2026 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor This is a refactor of the code and/or architecture. v0.0.55 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants