Skip to content

refactor(onboard): drop unused record-only runner - #5647

Merged
cv merged 15 commits into
mainfrom
refactor/onboard-fsm-drop-record-only-runner
Jun 24, 2026
Merged

refactor(onboard): drop unused record-only runner#5647
cv merged 15 commits into
mainfrom
refactor/onboard-fsm-drop-record-only-runner

Conversation

@cv

@cv cv commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Remove the unused record-only onboarding machine runner adapter. The live onboarding flow now uses the normal runtime boundary directly, so this speculative adapter and its dedicated tests no longer have production callers.

Changes

  • Delete src/lib/onboard/machine/record-only-runner.ts.
  • Delete the co-located tests for the unused adapter.
  • Verify the remaining runner and runtime-boundary record-only coverage still passes.

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

Summary by CodeRabbit

  • Chores
    • Removed the record-only onboard runner mode and associated test coverage.

@cv cv self-assigned this Jun 23, 2026
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

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: 7d6ab34e-b10e-4715-a067-119f6cdc5fe3

📥 Commits

Reviewing files that changed from the base of the PR and between 169b884 and 626135d.

📒 Files selected for processing (2)
  • src/lib/onboard/machine/record-only-runner.test.ts
  • src/lib/onboard/machine/record-only-runner.ts
💤 Files with no reviewable changes (2)
  • src/lib/onboard/machine/record-only-runner.test.ts
  • src/lib/onboard/machine/record-only-runner.ts

📝 Walkthrough

Walkthrough

Two files are deleted: src/lib/onboard/machine/record-only-runner.ts, which exported the RecordOnlyOnboardRuntimeBoundary types and the runOnboardMachineWithRecordOnlySteps adapter function, and src/lib/onboard/machine/record-only-runner.test.ts, which contained 296 lines of test coverage for that module.

Changes

Cohort / File(s) Summary
Delete record-only onboard runner
src/lib/onboard/machine/record-only-runner.ts, src/lib/onboard/machine/record-only-runner.test.ts
Removes the record-only runner implementation (exported types RecordOnlyOnboardRuntimeBoundaryOptions, RecordOnlyStepRecorders, interfaces RecordOnlyOnboardRuntimeBoundary, RecordOnlyOnboardMachineRunnerOptions, and functions createRecordOnlyOnboardRuntimeBoundary, runOnboardMachineWithRecordOnlySteps) along with all associated test coverage for step transitions, mutation enforcement, lifecycle events, context updates, and failure propagation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Suggested reviewers

  • jyaunches

Poem

🐇 Hippity-hop, two files are gone,
The record-only runner has moved along.
No more transitions, no lifecycle cheers,
The tests have vanished, the code disappears.
A tidy burrow, clean and bare —
Sometimes less code is the best kind of care! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'refactor(onboard): drop unused record-only runner' accurately and concisely describes the main change: removal of the unused record-only runner module and its tests from the onboarding system.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 refactor/onboard-fsm-drop-record-only-runner

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

@github-code-quality

github-code-quality Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

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

Show a code coverage summary of the most covered files.
File main refactor/onboard-fsm... 626135d +/-
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-fsm... branch is 46%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main refactor/onboard-fsm... 626135d +/-
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 67%
src/lib/onboard/preflight.ts 64%
src/lib/actions...licy-channel.ts 56%
src/lib/state/sandbox.ts 55%
src/lib/onboard...er-gpu-patch.ts 50%
src/lib/policy/index.ts 49%
src/lib/onboard.ts 18%

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

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: onboard-resume-e2e, onboard-repair-e2e, cloud-onboard-e2e
Optional E2E: None

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

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • onboard-resume-e2e (medium): Required by the onboarding resume compatibility rule: this PR changes onboarding machine transition/resume-related source behavior, so resume from persisted machine state must be validated end-to-end.
  • onboard-repair-e2e (medium): Required by the onboarding resume compatibility rule: record-only machine transition ownership is adjacent to resume repair semantics and must be validated against real repair/resume flows.
  • cloud-onboard-e2e (high): Validates that full hosted onboarding still completes with the current onboarding FSM/runtime behavior after deleting the record-only runner adapter.

Optional E2E

  • None.

New E2E recommendations

  • None.

Dispatch hint

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

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Vitest E2E Scenario Recommendation

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

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/main
Head: HEAD
Confidence: high

Required Vitest E2E scenarios

  • onboard-resume-vitest: Deletion of src/lib/onboard/machine record-only runner code can affect onboarding state-machine live-slice orchestration and resume compatibility paths, so the focused onboarding resume Vitest job is required.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-resume-vitest
  • onboard-repair-vitest: The same state-machine/resume-path change can affect repair or backstop execution from persisted onboarding sessions, so repair coverage is required rather than optional.
    • Dispatch: gh workflow run e2e-vitest-scenarios.yaml --ref <pr-head-ref> --field jobs=onboard-repair-vitest

Optional Vitest E2E scenarios

  • None.

Relevant changed files

  • src/lib/onboard/machine/record-only-runner.test.ts
  • src/lib/onboard/machine/record-only-runner.ts

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings

Merge posture: No blocking advisor findings
Primary next action: Add or justify PRA-T1 and any related test follow-ups.
Open items: 0 required · 0 warnings · 0 suggestions · 1 test follow-up
Since last review: 0 prior items resolved · 0 still apply · 0 new items found

Action checklist

  • PRA-T1 Add or justify test follow-up: Acceptance clause
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Acceptance clause — Verify the remaining runner and runtime-boundary record-only coverage still passes. — add test evidence or identify existing coverage. Read-only review did not execute tests. Static evidence shows remaining record-only coverage in `src/lib/onboard/runtime-boundary-record-only.test.ts`, `src/lib/onboard/runtime-boundary.test.ts`, `src/lib/onboard/machine/runtime.test.ts`, and `src/lib/onboard/resume-machine-repair.test.ts`.

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 commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator Author

PR Review Advisor follow-up:

  • PRA-T1: test evidence is in the PR verification and local run: npm run build:cli plus npx vitest run --project cli src/lib/onboard/machine/runner.test.ts src/lib/onboard/runtime-boundary-record-only.test.ts passed before opening this PR. CI also passed the full relevant CLI/static/typecheck gates after push. The deleted record-only-runner adapter had no production import sites (rg found only the deleted file and test), while the remaining active runner/runtime-boundary behavior stays covered by machine/runner.test.ts, runtime-boundary.test.ts, runtime-boundary-record-only.test.ts, machine/runtime.test.ts, and resume-machine-repair.test.ts.

@cv cv added the v0.0.67 label Jun 23, 2026
@wscurran wscurran added the refactor PR restructures code without intended behavior change label Jun 23, 2026
@jyaunches jyaunches added v0.0.68 and removed v0.0.67 labels Jun 24, 2026
Base automatically changed from refactor/onboard-fsm-compat-result-loop to main June 24, 2026 03:18
@cv
cv merged commit 3ce5930 into main Jun 24, 2026
44 checks passed
@cv
cv deleted the refactor/onboard-fsm-drop-record-only-runner branch June 24, 2026 03:44
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary
Remove the unused record-only onboarding machine runner adapter. The
live onboarding flow now uses the normal runtime boundary directly, so
this speculative adapter and its dedicated tests no longer have
production callers.

## Changes
- Delete `src/lib/onboard/machine/record-only-runner.ts`.
- Delete the co-located tests for the unused adapter.
- Verify the remaining runner and runtime-boundary record-only coverage
still passes.

## 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] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [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)

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

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

## Summary by CodeRabbit

* **Chores**
* Removed the record-only onboard runner mode and associated test
coverage.

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

Signed-off-by: Hadar Cohen <hacohen@redhat.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary
Remove the unused record-only onboarding machine runner adapter. The
live onboarding flow now uses the normal runtime boundary directly, so
this speculative adapter and its dedicated tests no longer have
production callers.

## Changes
- Delete `src/lib/onboard/machine/record-only-runner.ts`.
- Delete the co-located tests for the unused adapter.
- Verify the remaining runner and runtime-boundary record-only coverage
still passes.

## 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] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [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)

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

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

## Summary by CodeRabbit

* **Chores**
* Removed the record-only onboard runner mode and associated test
coverage.

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

Signed-off-by: Hadar Cohen <hacohen@redhat.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
## Summary
Remove the unused record-only onboarding machine runner adapter. The
live onboarding flow now uses the normal runtime boundary directly, so
this speculative adapter and its dedicated tests no longer have
production callers.

## Changes
- Delete `src/lib/onboard/machine/record-only-runner.ts`.
- Delete the co-located tests for the unused adapter.
- Verify the remaining runner and runtime-boundary record-only coverage
still passes.

## 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] PR description includes the DCO sign-off declaration and every
commit appears as `Verified` in GitHub
- [x] Git hooks passed during commit and push, or `npx prek run
--from-ref main --to-ref HEAD` passes
- [x] Targeted tests pass for changed behavior
- [ ] Full `npm test` passes (broad runtime changes only)
- [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)

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

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

## Summary by CodeRabbit

* **Chores**
* Removed the record-only onboard runner mode and associated test
coverage.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants