Skip to content

refactor(onboard): let FSM handlers return result sequences - #4473

Merged
cv merged 25 commits into
mainfrom
stack/onboard-fsm-runner-multi-result
Jun 8, 2026
Merged

refactor(onboard): let FSM handlers return result sequences#4473
cv merged 25 commits into
mainfrom
stack/onboard-fsm-runner-multi-result

Conversation

@cv

@cv cv commented May 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

Allow FSM runner handlers to return ordered result sequences as well as single results. Sequence handlers now have an explicit source-state and ownership contract so composite flows, such as provider/inference retry, can apply multiple transitions without stale context updates or silent state ownership gaps.

Changes

  • Add OnboardStateHandlerResult as OnboardStateResult | readonly OnboardStateResult[].
  • Update runOnboardMachine() to apply result arrays in order, reject empty result lists, validate metadata.state, enforce composite-handler sequence ownership, count every result toward the transition limit, and stop cleanly after terminal results.
  • Document the result-sequence handler contract and ownership allowlist in src/lib/onboard/machine/README.md.
  • Extend runner tests to cover multi-result handlers, source-state mismatches, unowned cross-state sequences, terminal results inside arrays, invalid mid-sequence transitions, retry, branch, failure, and missing-handler paths.

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

cv added 22 commits May 27, 2026 15:18
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Carlos Villela <cvillela@nvidia.com>
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-pr-bot Bot commented May 29, 2026

Copy link
Copy Markdown

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

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@cv, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 6 minutes and 35 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 291a2e3c-c038-4e35-8b9e-6b8fe1ceabd7

📥 Commits

Reviewing files that changed from the base of the PR and between 4307f43 and 7c379a1.

📒 Files selected for processing (3)
  • src/lib/onboard/machine/README.md
  • src/lib/onboard/machine/runner-sequence.test.ts
  • src/lib/onboard/machine/runner.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch stack/onboard-fsm-runner-multi-result

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

@github-actions

github-actions Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: cloud-onboard-e2e, onboard-resume-e2e
Optional E2E: onboard-negative-paths-e2e, double-onboard-e2e, cloud-inference-e2e

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

Auto-dispatched E2E: cloud-onboard-e2e, onboard-resume-e2e via nightly-e2e.yaml at 7c379a1a6a5103d686339538586ede61ffab7486nightly run

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: medium

Required E2E

  • cloud-onboard-e2e (medium; Ubuntu runner with real NVIDIA_API_KEY and Docker/OpenShell sandbox lifecycle): Best existing merge-blocking smoke for clean non-interactive install and onboarding through provider selection, inference configuration, sandbox creation, policy setup, and runtime health. This directly guards the user-visible onboarding flow that the FSM runner is intended to drive.
  • onboard-resume-e2e (medium; Ubuntu runner with install plus live onboarding resume flow): Exercises resumable onboarding/session state after interrupted progress. The runner changes source-state validation and ordered application of multiple results, so resume coverage is important for catching partially applied machine-state regressions.

Optional E2E

  • onboard-negative-paths-e2e (medium; Ubuntu runner with install and negative onboarding scenarios): Useful adjacent confidence for newly introduced runner error paths and failure-state behavior, especially cases where a sequence item fails validation after an earlier transition has already advanced the session.
  • double-onboard-e2e (medium-high; Ubuntu runner with install and repeated onboarding): Optional regression check for repeated onboarding/provider-selection behavior. The changed runner logic is adjacent to provider_selection and inference retries, which are commonly stressed by re-onboarding flows.
  • cloud-inference-e2e (medium; Ubuntu runner with real NVIDIA_API_KEY and live inference): Optional confidence that the provider_selection to inference transition still results in a working live inference route after onboarding.

New E2E recommendations

  • onboarding-fsm-result-sequences (high): Existing E2E jobs validate broad onboarding behavior but do not appear to force a live composite handler to return a multi-result sequence and then assert each persisted machine transition, metadata.state source validation, ownership allowlist behavior, and terminal short-circuit behavior.
    • Suggested test: Add an E2E or scenario fixture that drives provider_selection -> inference retry -> provider_selection and then returns a provider_selection-owned sequence through inference to sandbox, asserting session.machine.state/revision and redacted machine events after each applied result.

Dispatch hint

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

@github-actions

github-actions Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

E2E Scenario Advisor Recommendation

Required scenario E2E: ubuntu-repo-cloud-openclaw-double-provider-switch
Optional scenario E2E: ubuntu-repo-cloud-openclaw, ubuntu-repo-cloud-openclaw-double-same-provider

Dispatch required scenario E2E:

  • gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw-double-provider-switch

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

  • ubuntu-repo-cloud-openclaw-double-provider-switch: The PR changes the onboarding machine runner to support multi-result handler sequences and default sequence ownership for provider_selection -> inference. The double-provider-switch onboarding lifecycle is the smallest routed scenario that directly exercises provider switching/retry behavior across provider selection and inference while still completing a real repo-current onboarding flow.
    • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw-double-provider-switch

Optional scenario E2E

  • ubuntu-repo-cloud-openclaw: Baseline Ubuntu repo-current cloud OpenClaw onboarding is an adjacent sanity check for the standard single-result runner path, but the primary changed surface is covered by the provider-switch lifecycle scenario.
    • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw
  • ubuntu-repo-cloud-openclaw-double-same-provider: Adjacent lifecycle scenario for repeated provider selection without switching; useful for additional confidence in onboarding runner sequencing but not the smallest primary target for this sequence-ownership change.
    • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw-double-same-provider

Relevant changed files

  • src/lib/onboard/machine/runner.ts

@github-actions

github-actions Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

Findings: 0 needs attention, 0 worth checking, 0 nice ideas
Since last review: 1 prior item resolved, 0 still apply, 0 new items found

Consider writing more tests for
  • **Mocked behavioral coverage** — `runOnboardMachineWithRecordOnlySteps` applies a provider_selection/inference multi-result sequence while record-only step helpers do not mutate `session.machine`.. The core runner contract has strong unit coverage, including negative paths. One adapter-level behavioral mock would further prove that the new sequence options behave correctly through the record-only runner boundary used by the migration path.
  • **Mocked behavioral coverage** — `runOnboardMachineWithRecordOnlySteps` forwards custom `sequenceOwnership` and rejects an unlisted cross-state result through the boundary runtime.. The core runner contract has strong unit coverage, including negative paths. One adapter-level behavioral mock would further prove that the new sequence options behave correctly through the record-only runner boundary used by the migration path.

Workflow run details

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

@wscurran wscurran added area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow and removed onboarding labels Jun 3, 2026
@cv cv added the v0.0.61 label Jun 5, 2026
Base automatically changed from stack/onboard-fsm-live-record-only-sequence to main June 8, 2026 00:54
Resolve #4473 against current main and keep the tested result-sequence runner contract.

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Jun 8, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@cv
cv marked this pull request as ready for review June 8, 2026 02:11
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27112562359
Target ref: 1ff11057a0fcb98c383b00b1ae4852decd0db37e
Workflow ref: main
Requested jobs: cloud-e2e,onboard-resume-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job Result
cloud-e2e ✅ success
onboard-resume-e2e ✅ success

@cv
cv merged commit 86358c3 into main Jun 8, 2026
25 checks passed
@cv
cv deleted the stack/onboard-fsm-runner-multi-result branch June 8, 2026 02:41
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27112859278
Target ref: 7c379a1a6a5103d686339538586ede61ffab7486
Workflow ref: main
Requested jobs: cloud-onboard-e2e,onboard-resume-e2e
Summary: 2 passed, 0 failed, 0 skipped

Job Result
cloud-onboard-e2e ✅ success
onboard-resume-e2e ✅ success

cv added a commit that referenced this pull request Jun 8, 2026
## Summary
Add a sequence-runner adapter that can migrate the existing rich
onboarding phases onto the strict FSM runner. Each phase can return an
updated context plus one or more explicit FSM results, while
`runOnboardMachine()` remains responsible for applying and validating
transitions.

## Changes
- Add `runOnboardSequenceWithRunner()` and
`buildOnboardSequenceHandlers()`.
- Add duplicate phase detection for state-to-handler adapter
construction.
- Pass runner guardrails such as `maxTransitions` and
`sequenceOwnership` through the sequence adapter.
- Align multi-result phase tests with the strict `metadata.state`
sequence contract from #4473.
- Add sequence-runner tests covering rich context updates, multi-result
provider-style phases, custom sequence ownership, retry, branch, and
terminal completion.

## 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)

## Verification
- [x] `npx prek run --all-files` passes
- [x] `npm test` passes
- [x] `npm run typecheck:cli` passes
- [x] Tests added or updated for new or changed behavior
- [x] 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](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

---
<!-- DCO sign-off required by CI. Run: git config user.name && git
config user.email -->
Signed-off-by: Carlos Villela <cvillela@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Tests**
* Added comprehensive test coverage for onboard sequence execution,
validating state transitions, phase ordering, and error handling.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
@wscurran wscurran added the refactor PR restructures code without intended behavior change label Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants