Skip to content

fix(onboard): emit machine events for resume conflicts#4374

Draft
cv wants to merge 1 commit into
stack/onboard-fsm-lifecycle-eventsfrom
stack/onboard-fsm-resume-conflict-events
Draft

fix(onboard): emit machine events for resume conflicts#4374
cv wants to merge 1 commit into
stack/onboard-fsm-lifecycle-eventsfrom
stack/onboard-fsm-resume-conflict-events

Conversation

@cv
Copy link
Copy Markdown
Collaborator

@cv cv commented May 28, 2026

Summary

Surface resume configuration conflicts on the onboard machine event stream before preserving the existing console error and exit behavior. This makes conflict diagnostics observable without changing user-facing output.

Changes

  • Add emitResumeConflict() to OnboardRuntime and a recordResumeConflict() runtime-boundary wrapper.
  • Emit resume.conflict for each resume config mismatch in src/lib/onboard.ts.
  • Extend runtime and boundary tests for redacted resume conflict event emission.

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 28, 2026
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented May 28, 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 28, 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: 5593aa42-7d2b-4b56-bf1f-b3f7350cedac

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-resume-conflict-events

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: onboard-negative-paths-e2e, cloud-onboard-e2e

Dispatch hint: onboard-resume-e2e

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/stack/onboard-fsm-lifecycle-events
Head: HEAD
Confidence: high

Required E2E

  • onboard-resume-e2e (medium): Directly exercises interrupted onboarding followed by nemoclaw onboard --resume; it is the closest existing E2E guard for the resume lifecycle path changed by this PR and should catch regressions from the new runtime-boundary call or event emission.

Optional E2E

  • onboard-negative-paths-e2e (medium): Useful adjacent coverage for non-interactive onboarding validation and friendly failure paths. It does not specifically cover resume conflicts, but it can catch regressions where diagnostic/event changes perturb negative onboard exits.
  • cloud-onboard-e2e (high): Broad happy-path install/onboard confidence that lifecycle event plumbing did not affect normal non-resume onboarding completion; optional because the PR is targeted at resume conflict diagnostics rather than fresh onboard setup.

New E2E recommendations

  • onboarding resume conflict diagnostics (high): No existing E2E appears to trigger a resumable session and then resume with conflicting sandbox/agent/--from settings while asserting that a resume.conflict machine event is emitted, redacted, and does not mutate durable session state.
    • Suggested test: Add an E2E resume-conflict diagnostic scenario that creates an interrupted onboard session, reruns nemoclaw onboard --resume with a conflicting NEMOCLAW_SANDBOX_NAME or --from, captures machine events via the JSONL onboard hook path, and asserts resume.conflict metadata is present and secrets/URL credentials are redacted.

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-lifecycle-events
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, 0 worth checking, 1 nice ideas
Top item: Add callsite coverage for resume-conflict emissions

Review findings

🛠️ Needs attention

  • None.

🔎 Worth checking

  • None.

🌱 Nice ideas

  • Add callsite coverage for resume-conflict emissions (src/lib/onboard.ts:6602): The runtime and boundary layers are covered, but the actual onboard resume-conflict branch is only verified by inspection. A narrow regression test around this callsite would prove that each conflict from getResumeConfigConflicts is emitted before preserving the existing console error and exit behavior, including multiple-conflict cases.
    • Recommendation: If the monolith test harness allows it, add a behavioral test with mocked session/runtime/process boundaries that drives a resume conflict through onboard() and asserts one resume.conflict event per mismatch before exit.
    • Evidence: src/lib/onboard.ts calls await recordResumeConflict(conflict) inside the resumeConflicts loop before existing console.error branches and process.exit(1). Added tests cover OnboardRuntime.emitResumeConflict and OnboardRuntimeBoundary.recordResumeConflict, but grep found no direct test for the onboard.ts resume-conflict branch.

Workflow run details

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant