Skip to content

refactor(cli): migrate preflight.js to TypeScript#1262

Merged
cv merged 4 commits intocv/extract-onboard-pure-fnsfrom
cv/migrate-preflight-ts
Apr 1, 2026
Merged

refactor(cli): migrate preflight.js to TypeScript#1262
cv merged 4 commits intocv/extract-onboard-pure-fnsfrom
cv/migrate-preflight-ts

Conversation

@cv
Copy link
Copy Markdown
Contributor

@cv cv commented Apr 1, 2026

Summary

  • Convert bin/lib/preflight.js (357 lines) to src/lib/preflight.ts with full type definitions
  • Typed interfaces for all opts objects and return types: PortProbeResult, MemoryInfo, SwapResult, CheckPortOpts, GetMemoryInfoOpts, EnsureSwapOpts
  • Extract parseLsofLines helper to reduce duplication in checkPortAvailable
  • Incorporate fix: use sudo -n for lsof retry in preflight to avoid password prompt #1227 fix: sudosudo -n (non-interactive) for lsof retry
  • bin/lib/preflight.js becomes a thin re-export shim — existing consumers unaffected
  • Co-locate tests: test/preflight.test.jssrc/lib/preflight.test.ts
  • Add real net probe tests (EADDRINUSE detection on occupied ports)
  • Fix all co-located test imports to use dist/ paths for coverage attribution
  • Add targeted dashboard/validation branch tests to maintain ratchet

Stacked on #1240. Not touched by any #924 blocker PR.

Test plan

  • 612 CLI tests pass (601 existing + 11 new)
  • tsc -p tsconfig.src.json compiles cleanly
  • tsc -p tsconfig.cli.json type-checks cleanly
  • tsc -p jsconfig.json type-checks cleanly (the pre-push check that caught the union issue)
  • Coverage ratchet passes

Relates to #924 (shell consolidation). Supersedes #1227.

🤖 Generated with Claude Code

cv and others added 2 commits April 1, 2026 10:05
Convert bin/lib/preflight.js (357 lines) to src/lib/preflight.ts with
full type definitions for all opts objects and return types. The old
file becomes a thin re-export shim so existing consumers are unaffected.

Changes:
- Typed interfaces: PortProbeResult, MemoryInfo, SwapResult, and all
  opts types (CheckPortOpts, GetMemoryInfoOpts, EnsureSwapOpts)
- Extract parseLsofLines helper to reduce duplication in checkPortAvailable
- Incorporate #1227 fix: sudo -> sudo -n (non-interactive) for lsof retry
- Co-locate tests: test/preflight.test.js -> src/lib/preflight.test.ts
  converted to expect-style with type narrowing
- Add real net probe tests (EADDRINUSE detection on occupied ports)
- Fix co-located test imports to go through dist/ for coverage attribution
- Add targeted dashboard and validation branch tests for ratchet

612 CLI tests pass. Coverage ratchet passes. No user-facing behavior changes.

Relates to #924 (shell consolidation). Supersedes #1227.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The discriminated union types (PortProbeOk | PortProbeConflict) caused
TS2339 errors in jsconfig.json type-checking of onboard.js, which
accesses .pid/.reason without narrowing. Flatten to a single interface
with optional properties.

Also fix co-located test imports to use dist/ paths for coverage
attribution.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 1, 2026

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: Pro

Run ID: 9c168359-14cd-4968-a806-36072f889686

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 cv/migrate-preflight-ts

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

cv and others added 2 commits April 1, 2026 11:12
The tsc-js pre-push hook (jsconfig.json) type-checks bin/lib/onboard.js
which requires dist/lib/ to exist. CI was not running npm run build:cli
before the prek checks, causing TS2307 "Cannot find module" errors.

- Add npm run build:cli step to .github/actions/basic-checks
- Update tsc-js hook to run build:cli before tsc

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@wscurran wscurran added NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). refactor This is a refactor of the code and/or architecture. labels Apr 1, 2026
@cv cv merged commit 2ac85ce into cv/extract-onboard-pure-fns Apr 1, 2026
8 checks passed
@cv cv deleted the cv/migrate-preflight-ts branch April 1, 2026 18:38
prekshivyas pushed a commit that referenced this pull request Apr 2, 2026
…eScript (#1298)

Move CJS implementations in `bin/lib/` to TypeScript in `src/lib/`,
compiled via `tsconfig.src.json` to `dist/lib/`. Replaces the inline
CJS with thin re-export shims following the pattern from #1262, #1265.

- Add `src/lib/resolve-openshell.ts` with typed DI options interface
- Add `src/lib/version.ts` preserving existing getVersion() string API
  (git describe → .version file → package.json fallback chain)
- Add `src/lib/chat-filter.ts` preserving existing array-based API
- Add co-located tests importing from `../../dist/lib/` for coverage
- Replace `bin/lib/` full implementations with thin shims
- Use platform-neutral paths in version tests (node:path join)

No API changes — all consumers (bin/nemoclaw.js, scripts/telegram-bridge.js)
continue to work without modification.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

NemoClaw CLI Use this label to identify issues with the NemoClaw command-line interface (CLI). refactor This is a refactor of the code and/or architecture.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants