Skip to content

refactor(onboard): return ordered provider FSM results#4471

Merged
cv merged 25 commits into
mainfrom
stack/onboard-fsm-provider-result-sequence
Jun 7, 2026
Merged

refactor(onboard): return ordered provider FSM results#4471
cv merged 25 commits into
mainfrom
stack/onboard-fsm-provider-result-sequence

Conversation

@cv

@cv cv commented May 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Return the provider/inference handler's FSM transitions in the order they occur. This gives the live sequence enough information to drive intermediate provider-selection, inference retry, and final sandbox transitions explicitly.

Changes

  • Add ordered stateResults to ProviderInferenceStateResult.
  • Include provider_selection -> inference, inference -> provider_selection retry, and final inference -> sandbox results as applicable.
  • Extend provider/inference tests to assert ordered state results for fresh and retry flows.

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

    • Onboarding now records and returns the full sequence of state transitions across provider selection, inference, retry, and sandbox advancement for more consistent state tracking.
  • Tests

    • Provider-inference tests expanded to assert complete state transition sequences, including retry flows and progression into sandbox, ensuring behavior is validated end-to-end.

cv added 20 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>
@cv cv self-assigned this May 28, 2026
@copy-pr-bot

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

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 6253bd93-e2aa-4c14-ac9f-09c7874e6bb6

📥 Commits

Reviewing files that changed from the base of the PR and between 5db7c65 and ba670ec.

📒 Files selected for processing (1)
  • src/lib/onboard.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/onboard.ts

📝 Walkthrough

Walkthrough

The provider-inference handler now accumulates and returns an ordered stateResults sequence; tests assert the full transition sequence; the runtime consumer records those stateResults without mutating the outer session and passes the providerInferenceResult.session explicitly to sandbox handling.

Changes

Provider-inference state transitions

Layer / File(s) Summary
Contract and imports for state transitions
src/lib/onboard/machine/handlers/provider-inference.ts
ProviderInferenceStateResult now includes stateResults: OnboardStateTransitionResult[]; advanceTo is imported to build transitions.
State transitions recording in handleProviderInferenceState
src/lib/onboard/machine/handlers/provider-inference.ts
Handler initializes stateResults, appends advanceTo("inference") after provider_selection, records retryTo("provider_selection") into stateResults on retries for both inference paths, appends final advanceTo("sandbox"), and returns stateResults.
Test assertions for state transitions sequence
src/lib/onboard/machine/handlers/provider-inference.test.ts
“Fresh flow” test asserts provider_selection → inference order in result.stateResults; “retry” test asserts inference → retry(provider_selection) → inference → sandbox sequence.
Consumer integration in onboard.ts
src/lib/onboard.ts
Call recordStateResultsWithStepCompatibility(providerInferenceResult.stateResults) without reassigning the outer session; pass session: providerInferenceResult.session explicitly to handleSandboxState.

Sequence Diagram(s)

sequenceDiagram
  participant Handler as handleProviderInferenceState
  participant StateArray as stateResults
  participant Onboard as onboard.ts
  participant Runtime as onboardRuntimeBoundary
  participant Sandbox as handleSandboxState

  Handler->>StateArray: initialize and append transitions (advanceTo/retryTo)
  Handler->>Onboard: return providerInferenceResult { stateResults, session, ... }
  Onboard->>Runtime: recordStateResultsWithStepCompatibility(providerInferenceResult.stateResults)
  Onboard->>Sandbox: handleSandboxState(session: providerInferenceResult.session, ...)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

Possibly related PRs

Suggested labels

refactor, onboarding, feature

Suggested reviewers

  • prekshivyas
  • cjagwani
  • jyaunches

Poem

🐰 I hopped through states both near and far,
I counted advances, each retry and spar,
From selection to inference, then sandbox at last,
I stored every step so no session would be past,
A tiny trail of transitions — neat and fast.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor(onboard): return ordered provider FSM results' directly and clearly summarizes the main change—adding an ordered stateResults field to track provider/inference FSM transitions in sequence.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch stack/onboard-fsm-provider-result-sequence

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

@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: cloud-onboard-e2e, onboard-resume-e2e
Optional E2E: onboard-inference-smoke-e2e, double-onboard-e2e, ubuntu-repo-cloud-openclaw-resume

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

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

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • cloud-onboard-e2e (high): Validates the real non-interactive cloud onboarding path through provider selection, inference setup, session persistence, sandbox creation, and inference.local after the session handoff change.
  • onboard-resume-e2e (high): Directly exercises interrupted onboard and --resume behavior, which is the highest-risk area for the new batched stateResults recording and updated session propagation.

Optional E2E

  • onboard-inference-smoke-e2e (low): Focused confidence for inference setup failure/smoke-probe behavior adjacent to the provider-inference handler, though this PR mainly changes state recording rather than setupInference itself.
  • double-onboard-e2e (high): Useful broader coverage for repeated onboarding, gateway reuse, and stale registry/session reconciliation after the provider/inference-to-sandbox session handoff change.
  • ubuntu-repo-cloud-openclaw-resume (high): Typed scenario coverage for cloud OpenClaw resume-after-interrupt; overlaps onboard-resume-e2e but provides scenario-runner validation of the same user flow.

New E2E recommendations

  • provider inference retry state recording (high): Existing E2E coverage does not appear to force setupInference to return retry: selection and then verify that provider_selection, inference retry, second provider_selection, final inference, and sandbox transitions are persisted in order and remain resumable.
    • Suggested test: Add an E2E or typed scenario that uses a hermetic compatible endpoint/provider setup to trigger an inference-to-provider-selection retry, then completes onboarding and verifies onboard-session state compatibility before sandbox creation.

Dispatch hint

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

@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

E2E Scenario Advisor Recommendation

Required scenario E2E: ubuntu-repo-cloud-openclaw, ubuntu-repo-cloud-openclaw-resume
Optional scenario E2E: ubuntu-repo-cloud-hermes, ubuntu-repo-openai-compatible-openclaw

Dispatch required scenario E2E:

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

Workflow run

Full scenario advisor summary

E2E Scenario Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required scenario E2E

  • ubuntu-repo-cloud-openclaw: Core onboarding provider/inference state handling changed. This primary Ubuntu repo cloud OpenClaw scenario exercises fresh provider selection, inference setup, state recording, sandbox creation, and inference-local validation on the standard runner.
    • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw
  • ubuntu-repo-cloud-openclaw-resume: The change alters provider/inference transition result recording and session propagation around onboarding state boundaries; the resume lifecycle scenario is the smallest targeted coverage for resumed/interrupted onboarding state behavior.
    • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-openclaw-resume

Optional scenario E2E

  • ubuntu-repo-cloud-hermes: Optional adjacent coverage for the same provider/inference onboarding handler with the Hermes agent path, which has a specialized inference branch but is not the primary changed surface.
    • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-cloud-hermes
  • ubuntu-repo-openai-compatible-openclaw: Optional adjacent coverage for a non-NVIDIA compatible endpoint onboarding profile through the same provider inference handler.
    • Dispatch: gh workflow run e2e-scenarios.yaml --ref <pr-head-ref> --field scenarios=ubuntu-repo-openai-compatible-openclaw

Relevant changed files

  • src/lib/onboard.ts
  • src/lib/onboard/machine/handlers/provider-inference.ts

@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor

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

Review findings

🛠️ Needs attention

  • None.

🔎 Worth checking

  • Source-of-truth review needed: OnboardRuntimeBoundary recordStateResultsWithStepCompatibility consuming ordered provider/inference results: 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: runtime-boundary.ts documents and enforces metadata-only skipped transitions. onboard.ts now records providerInferenceResult.stateResults through recordStateResultsWithStepCompatibility, while provider-inference.ts returns the complete ordered stream.
  • Add runtime validation for the full provider/inference stateResults stream (src/lib/onboard.ts:6414): The PR now records the handler's complete ordered provider/inference transition stream through recordStateResultsWithStepCompatibility, but the added tests only assert handler output. Existing runtime-boundary coverage exercises a partial provider retry stream, not the exact new sequence after legacy step helpers have already advanced the machine. Because this is onboarding host glue around inference, credentials, resume, and sandbox handoff, the missing runtime coverage is also the remaining source-of-truth/security-posture warning for the compatibility bridge.
    • Recommendation: Add a focused runtime or caller test that feeds provider_selection→inference, inference→provider_selection retry, provider_selection→inference, and inference→sandbox through recordStateResultsWithStepCompatibility after legacy provider/inference step mutations, then asserts final sandbox state and expected skipped-result diagnostics. If practical, also assert onboard glue passes providerInferenceResult.stateResults verbatim and passes providerInferenceResult.session into handleSandboxState.
    • Evidence: src/lib/onboard.ts now calls recordStateResultsWithStepCompatibility(providerInferenceResult.stateResults). provider-inference.ts accumulates the full ordered stream. provider-inference.test.ts asserts fresh and retry handler ordering. Existing runtime-boundary.test.ts covers [retryResult, advanceTo("sandbox")] but not the complete new stream with both provider_selection→inference advances.

🌱 Nice ideas

  • None.
Consider writing more tests for
  • **Runtime validation** — recordStateResultsWithStepCompatibility consumes provider_selection→inference, inference→provider_selection retry, provider_selection→inference, and inference→sandbox after legacy provider/inference step mutations and leaves the machine at sandbox with expected skipped-result diagnostics.. The changed behavior crosses handler output, live onboard glue, legacy step mutation compatibility, runtime event sequencing, and sandbox handoff. Handler unit tests are useful but do not prove the full ordered stream behaves correctly through the runtime boundary.
  • **Runtime validation** — onboard provider/inference glue passes providerInferenceResult.stateResults verbatim to recordStateResultsWithStepCompatibility and passes providerInferenceResult.session into handleSandboxState.. The changed behavior crosses handler output, live onboard glue, legacy step mutation compatibility, runtime event sequencing, and sandbox handoff. Handler unit tests are useful but do not prove the full ordered stream behaves correctly through the runtime boundary.
  • **Runtime validation** — resume-ready provider inference returns provider_selection→inference and inference→sandbox stateResults without credentialEnv or endpointUrl metadata.. The changed behavior crosses handler output, live onboard glue, legacy step mutation compatibility, runtime event sequencing, and sandbox handoff. Handler unit tests are useful but do not prove the full ordered stream behaves correctly through the runtime boundary.
  • **Runtime validation** — forced resume provider recovery that reruns inference returns provider_selection→inference and inference→sandbox stateResults while preserving staged credential cleanup.. The changed behavior crosses handler output, live onboard glue, legacy step mutation compatibility, runtime event sequencing, and sandbox handoff. Handler unit tests are useful but do not prove the full ordered stream behaves correctly through the runtime boundary.
  • **Add runtime validation for the full provider/inference stateResults stream** — Add a focused runtime or caller test that feeds provider_selection→inference, inference→provider_selection retry, provider_selection→inference, and inference→sandbox through recordStateResultsWithStepCompatibility after legacy provider/inference step mutations, then asserts final sandbox state and expected skipped-result diagnostics. If practical, also assert onboard glue passes providerInferenceResult.stateResults verbatim and passes providerInferenceResult.session into handleSandboxState.
  • **Acceptance clause:** Return the provider/inference handler's FSM transitions in the order they occur. This gives the live sequence enough information to drive intermediate provider-selection, inference retry, and final sandbox transitions explicitly. — add test evidence or identify existing coverage. provider-inference.ts adds stateResults and pushes provider_selection→inference, retry, and inference→sandbox results in order; onboard.ts records stateResults. Handler tests cover fresh and retry ordering, but live runtime-boundary consumption of the exact full stream is not covered.
  • **Acceptance clause:** `npx prek run --all-files` passes — add test evidence or identify existing coverage. This read-only review did not execute commands or inspect external CI status.
  • **Acceptance clause:** `npm test` passes — add test evidence or identify existing coverage. This read-only review did not execute commands or inspect external CI status.
Since last review details

Current findings:

  • Source-of-truth review needed: OnboardRuntimeBoundary recordStateResultsWithStepCompatibility consuming ordered provider/inference results: 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: runtime-boundary.ts documents and enforces metadata-only skipped transitions. onboard.ts now records providerInferenceResult.stateResults through recordStateResultsWithStepCompatibility, while provider-inference.ts returns the complete ordered stream.
  • Add runtime validation for the full provider/inference stateResults stream (src/lib/onboard.ts:6414): The PR now records the handler's complete ordered provider/inference transition stream through recordStateResultsWithStepCompatibility, but the added tests only assert handler output. Existing runtime-boundary coverage exercises a partial provider retry stream, not the exact new sequence after legacy step helpers have already advanced the machine. Because this is onboarding host glue around inference, credentials, resume, and sandbox handoff, the missing runtime coverage is also the remaining source-of-truth/security-posture warning for the compatibility bridge.
    • Recommendation: Add a focused runtime or caller test that feeds provider_selection→inference, inference→provider_selection retry, provider_selection→inference, and inference→sandbox through recordStateResultsWithStepCompatibility after legacy provider/inference step mutations, then asserts final sandbox state and expected skipped-result diagnostics. If practical, also assert onboard glue passes providerInferenceResult.stateResults verbatim and passes providerInferenceResult.session into handleSandboxState.
    • Evidence: src/lib/onboard.ts now calls recordStateResultsWithStepCompatibility(providerInferenceResult.stateResults). provider-inference.ts accumulates the full ordered stream. provider-inference.test.ts asserts fresh and retry handler ordering. Existing runtime-boundary.test.ts covers [retryResult, advanceTo("sandbox")] but not the complete new stream with both provider_selection→inference advances.

Workflow run details

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

@cv cv added the onboarding label May 29, 2026
@wscurran wscurran added area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow and removed onboarding labels Jun 3, 2026
Base automatically changed from stack/onboard-fsm-record-only-runner to main June 5, 2026 17:24
@cv cv added the v0.0.61 Release target label Jun 5, 2026
Resolve PR #4471 against current main while preserving the ordered
provider/inference FSM result sequence.

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

copy-pr-bot Bot commented Jun 5, 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 6, 2026 07:02
@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27055809110
Target ref: 287fbf466d37468dbf4a7de2fe73812f36bf4fe9
Workflow ref: main
Requested jobs: onboard-resume-e2e,cloud-onboard-e2e
Summary: 2 passed, 0 failed, 0 skipped

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

@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Selective E2E Results — ❌ Some jobs failed

Run: 27085834419
Target ref: 5db7c65e314c61634ea3f54f525516611f85c50d
Workflow ref: main
Requested jobs: cloud-e2e,onboard-resume-e2e
Summary: 1 passed, 1 failed, 0 skipped

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

Failed jobs: onboard-resume-e2e. Check run artifacts for logs.

@github-actions

github-actions Bot commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Selective E2E Results — ✅ All requested jobs passed

Run: 27104483043
Target ref: ba670ec573e954c5860acf3b70224c3508a84254
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 cv merged commit df2b86a into main Jun 7, 2026
28 checks passed
@cv cv deleted the stack/onboard-fsm-provider-result-sequence branch June 7, 2026 21:14
@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 v0.0.61 Release target

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants