Skip to content

refactor(onboard): apply strict step results directly - #5710

Merged
cv merged 14 commits into
test/onboard-record-only-boundary-stale-resultsfrom
refactor/onboard-strict-step-results
Jun 24, 2026
Merged

refactor(onboard): apply strict step results directly#5710
cv merged 14 commits into
test/onboard-record-only-boundary-stale-resultsfrom
refactor/onboard-strict-step-results

Conversation

@cv

@cv cv commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

Route record-only step/result pairing through the strict runtime result path. This keeps the stale-result compatibility bridge reserved for explicit legacy or resume replay paths.

Changes

  • Change recordStepCompleteWithStateResult to call recordStateResult after its preflight validation.
  • Leave legacy/resume compatible result handling unchanged.
  • Preserve existing runtime-boundary coverage.

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

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior
  • Full npm test passes (broad runtime changes only)
  • 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 cv self-assigned this Jun 24, 2026
@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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: 5648f3a0-97bc-4129-9ce3-99f18b1e9401

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 refactor/onboard-strict-step-results

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

@github-code-quality

github-code-quality Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the refactor/onboard-str... branch is 96%. Coverage data for the test/onboard-record-... branch is not yet available.

Show a code coverage summary of the most covered files.
File test/onboard-record-... refactor/onboard-str... f9af87b +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the refactor/onboard-str... branch is 47%. Coverage data for the test/onboard-record-... branch is not yet available.

Show a code coverage summary of the most covered files.
File test/onboard-record-... refactor/onboard-str... f9af87b +/-
src/lib/state/o...oard-session.ts 91%
src/lib/inference/local.ts 76%
src/lib/sandbox/config.ts 72%
src/lib/actions...dbox/rebuild.ts 70%
src/lib/onboard/preflight.ts 64%
src/lib/actions...licy-channel.ts 58%
src/lib/state/sandbox.ts 55%
src/lib/policy/index.ts 49%
src/lib/onboard...er-gpu-patch.ts 44%
src/lib/onboard.ts 18%

Updated June 24, 2026 20:15 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

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

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

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/test/onboard-record-only-boundary-stale-results
Head: HEAD
Confidence: high

Required E2E

  • onboard-resume-e2e (medium): Required because the source change is adjacent to onboarding resume compatibility and state-result replay semantics. Validates a real resumed onboarding flow against persisted machine/session state.
  • onboard-repair-e2e (medium): Required because the runtime boundary compatibility path is explicitly used for repaired resume/ahead-state replay. Validates the live repair path instead of relying only on unit coverage.
  • cloud-onboard-e2e (high): Required because the changed onboarding runtime boundary can affect normal hosted onboarding state transitions and step/result persistence in a real sandbox flow.

Optional E2E

  • onboard-negative-paths-e2e (medium): Optional confidence for invalid/stale transition handling, since this PR changes the strict record-only result path and tests rejection behavior in unit coverage.

New E2E recommendations

  • None.

Dispatch hint

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

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

Required Vitest E2E scenarios: onboard-resume-vitest, onboard-repair-vitest
Optional Vitest E2E scenarios: cloud-onboard-vitest

Dispatch required Vitest E2E scenarios:

  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-resume-vitest
  • gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-repair-vitest

Workflow run

Full Vitest E2E advisor summary

Vitest E2E Scenario Advisor

Base: origin/test/onboard-record-only-boundary-stale-results
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • onboard-resume-vitest: The PR changes OnboardRuntimeBoundary record-only step/result compatibility behavior, which is part of onboarding state-result application and can affect persisted-session resume compatibility paths. The onboarding resume compatibility rule requires the onboard resume live Vitest job.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-resume-vitest
  • onboard-repair-vitest: The changed boundary logic touches the compatibility bridge used for repaired resume/persisted ahead-state sessions, so repair/backstop execution from persisted sessions must be covered alongside resume.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-repair-vitest

Optional Vitest E2E scenarios

  • cloud-onboard-vitest: Adjacent baseline coverage for the standard cloud onboarding path using the same runtime boundary, useful if maintainers want a non-resume/non-repair smoke of normal record-only onboarding transitions.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=cloud-onboard-vitest

Relevant changed files

  • src/lib/onboard/runtime-boundary-record-only.test.ts
  • src/lib/onboard/runtime-boundary.ts

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: No advisor follow-up required beyond maintainer review.
Open items: 0 required · 0 warnings · 0 suggestions · 0 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@cv cv added the v0.0.68 label Jun 24, 2026
@cv

cv commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

PRA-T1 test evidence: ran npx vitest run --project cli src/lib/onboard/runtime-boundary-record-only.test.ts src/lib/onboard/runtime-boundary.test.ts locally after adding the direct strict-path regression test; 2 files / 18 tests passed. Push hooks and CI cli-tests also passed on head 593b1cd.

@wscurran wscurran added area: onboarding Onboarding FSM, provider setup, sandbox launch, or first-run flow refactor PR restructures code without intended behavior change labels Jun 24, 2026
cv added 8 commits June 24, 2026 09:34
…y-stale-results' into refactor/onboard-compat-bridge-comments
…omments' into refactor/onboard-strict-step-results
…y-stale-results' into refactor/onboard-compat-bridge-comments
…omments' into refactor/onboard-strict-step-results
…y-stale-results' into refactor/onboard-compat-bridge-comments
…omments' into refactor/onboard-strict-step-results
Base automatically changed from refactor/onboard-compat-bridge-comments to test/onboard-record-only-boundary-stale-results June 24, 2026 20:10
@cv
cv merged commit decb592 into test/onboard-record-only-boundary-stale-results Jun 24, 2026
37 checks passed
@cv
cv deleted the refactor/onboard-strict-step-results branch June 24, 2026 20:19
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