Skip to content

refactor(cli): remove TS dependency on onboard-session shim - #1543

Merged
cv merged 1 commit into
mainfrom
cv/remove-ts-onboard-session-backedge
Apr 7, 2026
Merged

cv merged 1 commit into
mainfrom
cv/remove-ts-onboard-session-backedge

Conversation

@cv

@cv cv commented Apr 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Remove the src/lib/runtime-recovery.ts dependency on the legacy bin/lib/onboard-session shim and import the TypeScript module directly.

Why

This is the first small cleanup PR in the #924 follow-through: reduce TS -> legacy JS backedges so the CLI TS modules form cleaner islands.

Changes

  • replace require("../../bin/lib/onboard-session") with a local TS import
  • keep runtime behavior unchanged

Testing

  • npm run build:cli
  • npx vitest run src/lib/runtime-recovery.test.ts

Relates to #924.

Summary by CodeRabbit

  • Refactor
    • Updated internal module loading system to improve code maintainability and consistency.

@coderabbitai

coderabbitai Bot commented Apr 6, 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: Pro

Run ID: 9c62f48c-9ef7-4878-89a8-93de1a6515ef

📥 Commits

Reviewing files that changed from the base of the PR and between 7d05229 and 4ae6cb0.

📒 Files selected for processing (1)
  • src/lib/runtime-recovery.ts

📝 Walkthrough

Walkthrough

A module import migration in runtime-recovery.ts converts a CommonJS require statement to an ES module import for the loadSession function from onboard-session. The function call syntax is updated accordingly while preserving the existing logic.

Changes

Cohort / File(s) Summary
Module System Migration
src/lib/runtime-recovery.ts
Changed CommonJS require('onboard-session') to ES module import of loadSession. Updated the function call from onboardSession.loadSession() to loadSession(). Logic remains unchanged.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐰 From CommonJS to ES with a hop and a bound,
No logic was harmed, just imports unwound,
loadSession now flows in the module's sweet breeze,
A rabbit's delight—such refactoring ease! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically identifies the main change: removing a TypeScript dependency on a legacy JavaScript shim module, which aligns with the primary refactoring objective of reducing backedges.

✏️ 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 cv/remove-ts-onboard-session-backedge

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

@cv
cv enabled auto-merge (squash) April 6, 2026 21:54
@cv
cv requested review from brandonpelfrey and kjw3 April 6, 2026 23:56
@cv
cv merged commit 9fe2b78 into main Apr 7, 2026
11 checks passed
gemini2026 pushed a commit to gemini2026/NemoClaw that referenced this pull request Apr 14, 2026
)

## Summary

Remove the `src/lib/runtime-recovery.ts` dependency on the legacy
`bin/lib/onboard-session` shim and import the TypeScript module
directly.

## Why

This is the first small cleanup PR in the NVIDIA#924 follow-through: reduce TS
-> legacy JS backedges so the CLI TS modules form cleaner islands.

## Changes

- replace `require("../../bin/lib/onboard-session")` with a local TS
import
- keep runtime behavior unchanged

## Testing

- `npm run build:cli`
- `npx vitest run src/lib/runtime-recovery.test.ts`

Relates to NVIDIA#924.

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

## Summary by CodeRabbit

* **Refactor**
* Updated internal module loading system to improve code maintainability
and consistency.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@cv
cv deleted the cv/remove-ts-onboard-session-backedge branch May 27, 2026 21:16
@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

refactor PR restructures code without intended behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants