fix: resolve ESLint error and speed up gateway recovery tests by cv · Pull Request #1223 · NVIDIA/NemoClaw

cv · 2026-04-01T03:17:26Z

Problem

After pulling latest main, prek run -a fails with:

ESLint error in snapshot.test.ts — @typescript-eslint/consistent-type-imports forbids import() type annotations in generic parameters
Slow tests — gateway recovery tests sleep 8–19s each waiting for health polls that will never succeed against stub openshell scripts, adding ~40s to the suite

Changes

ESLint fix

snapshot.test.ts: replace importOriginal<typeof import("node:fs")>() with a top-level import type fs from "node:fs" and use importOriginal<typeof fs>()

Test performance (70s → 28s)

onboard.js: make health-poll count and interval configurable via NEMOCLAW_HEALTH_POLL_COUNT and NEMOCLAW_HEALTH_POLL_INTERVAL env vars (production defaults unchanged)
cli.test.js: set POLL_COUNT=1, POLL_INTERVAL=0 in the runWithEnv test helper so gateway recovery tests skip unnecessary sleep loops
cli.test.js: reduce test timeouts from 25s → 10s accordingly

Formatting (pre-commit hooks)

Prettier reformatting in runner.test.ts, state.test.ts, uninstall.test.js

Verification

All 740 tests pass
All pre-commit hooks pass (ESLint, Prettier, TypeScript, tests)
No production behavior change (env vars default to existing values)

Summary by CodeRabbit

Chores
- Made gateway health polling configurable via environment variables (poll count and interval).
Tests
- Reduced CLI test timeouts and injected health-check env vars to speed CI runs.
- Improved test typing/mocking and cleaned up test formatting for stability and readability.

- snapshot.test.ts: replace forbidden import() type annotation in importOriginal generic with a top-level type-only import - onboard.js: make health-poll count and interval configurable via NEMOCLAW_HEALTH_POLL_COUNT and NEMOCLAW_HEALTH_POLL_INTERVAL env vars (defaults unchanged for production) - cli.test.js: set poll count=1 and interval=0 in test helper to eliminate ~40s of unnecessary sleep in gateway recovery tests - cli.test.js: reduce test timeouts from 25s to 10s accordingly - Apply Prettier formatting fixes from pre-commit hooks

coderabbitai · 2026-04-01T03:17:39Z

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: 6505e2aa-fe96-4afe-a884-5fd1ccf1d296

📥 Commits

Reviewing files that changed from the base of the PR and between 878b2cc and a0170a6.

📒 Files selected for processing (1)

bin/lib/onboard.js

🚧 Files skipped from review as they are similar to previous changes (1)

bin/lib/onboard.js

📝 Walkthrough

Walkthrough

Gateway health-check polling loops were made configurable via environment variables; Vitest fs mocks were re-typed to use import type fs with importOriginal<typeof fs>(); CLI tests inject the new env vars and shorten timeouts; some tests were reformatted to single-line calls.

Changes

Cohort / File(s)	Summary
Gateway Health Configuration `bin/lib/onboard.js`	Added `envInt(name, fallback)` helper; replaced hardcoded health poll counts and fixed 2s sleeps with `NEMOCLAW_HEALTH_POLL_COUNT` (defaults: 5/10 in respective flows) and `NEMOCLAW_HEALTH_POLL_INTERVAL` (default: 2), and wired polling loops to these env-driven values.
TypeScript Mock Typing Updates `nemoclaw/src/blueprint/runner.test.ts`, `nemoclaw/src/blueprint/snapshot.test.ts`, `nemoclaw/src/blueprint/state.test.ts`	Changed Vitest `node:fs` mock factories to import the `fs` type (`import type fs from "node:fs"`) and call `importOriginal<typeof fs>()` for stronger typing; mock implementations and behavior unchanged.
CLI Test Changes `test/cli.test.js`	Injected `NEMOCLAW_HEALTH_POLL_COUNT="1"` and `NEMOCLAW_HEALTH_POLL_INTERVAL="0"` into CLI subprocess env; reduced several command timeouts from `25000` ms to `10000` ms.
Test Formatting / Minor Cleanup `test/uninstall.test.js`	Collapsed multi-line `fs` calls, `spawnSync` argument arrays, and some assertions into single-line statements; no behavioral changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I twitch my nose at polling time,
Env keys tell seconds, counts, and rhyme.
Tests trimmed neat, types hopped in place,
Shorter waits speed up the chase—
I nibble logs and bound away.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.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 accurately summarizes the main changes: fixing an ESLint error (type annotation in generics) and optimizing gateway recovery test performance through configurable polling and reduced timeouts.

_{✏️ 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 fix/eslint-and-slow-tests

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

coderabbitai

🧹 Nitpick comments (2)

nemoclaw/src/blueprint/state.test.ts (1)
9-10: Consider using the same pattern as snapshot.test.ts for consistency.

This file uses type inference for importOriginal(), while snapshot.test.ts uses an explicit import type fs from "node:fs" followed by importOriginal<typeof fs>(). Using the same pattern across all three files would improve consistency and align with the approach described in the PR summary.
♻️ Align with snapshot.test.ts pattern

Add the type import near the top (after line 4):
 import { describe, it, expect, beforeEach, vi } from "vitest";
 import { loadState, saveState, clearState, type NemoClawState } from "./state.js";
+import type fs from "node:fs";
Then update the mock factory:
 vi.mock("node:fs", async (importOriginal) => {
-  const original = await importOriginal();
+  const original = await importOriginal<typeof fs>();
   return {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@nemoclaw/src/blueprint/state.test.ts` around lines 9 - 10, The test's mock
currently uses type inference for importOriginal(); update it to match
snapshot.test.ts by adding an explicit type import (import type fs from
"node:fs") and then call importOriginal<typeof fs>() inside the vi.mock factory
used in the vi.mock("node:fs", async (importOriginal) => { ... }) block so the
mocked "original" variable is strongly typed; adjust any related variable
typings in the mock factory (e.g., the `original` constant) to use the imported
fs type.
nemoclaw/src/blueprint/runner.test.ts (1)
34-35: Consider using the same pattern as snapshot.test.ts for consistency.

While relying on type inference works, snapshot.test.ts uses an explicit top-level type import (import type fs from "node:fs") followed by importOriginal<typeof fs>(). This approach aligns with the PR summary's stated fix and provides better consistency across files addressing the same ESLint issue.
♻️ Align with snapshot.test.ts pattern

Add the type import at the top of the file (after line 5):
 import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
 import YAML from "yaml";
+import type fs from "node:fs";
Then update the mock factory:
 vi.mock("node:fs", async (importOriginal) => {
-  const original = await importOriginal();
+  const original = await importOriginal<typeof fs>();
   return {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@nemoclaw/src/blueprint/runner.test.ts` around lines 34 - 35, Add a top-level
type import for the Node fs module (import type fs from "node:fs") and update
the vi.mock call to use the explicit generic when calling importOriginal by
changing importOriginal() to importOriginal<typeof fs>() so the mock factory
matches the pattern used in snapshot.test.ts and resolves the same ESLint/type
inference fix; keep the existing vi.mock("node:fs", async (importOriginal) => {
const original = await importOriginal<typeof fs>(); ... }) structure with the
new top-level type import.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@nemoclaw/src/blueprint/runner.test.ts`:
- Around line 34-35: Add a top-level type import for the Node fs module (import
type fs from "node:fs") and update the vi.mock call to use the explicit generic
when calling importOriginal by changing importOriginal() to
importOriginal<typeof fs>() so the mock factory matches the pattern used in
snapshot.test.ts and resolves the same ESLint/type inference fix; keep the
existing vi.mock("node:fs", async (importOriginal) => { const original = await
importOriginal<typeof fs>(); ... }) structure with the new top-level type
import.

In `@nemoclaw/src/blueprint/state.test.ts`:
- Around line 9-10: The test's mock currently uses type inference for
importOriginal(); update it to match snapshot.test.ts by adding an explicit type
import (import type fs from "node:fs") and then call importOriginal<typeof fs>()
inside the vi.mock factory used in the vi.mock("node:fs", async (importOriginal)
=> { ... }) block so the mocked "original" variable is strongly typed; adjust
any related variable typings in the mock factory (e.g., the `original` constant)
to use the imported fs type.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 981469ee-c84f-41db-8bf0-78b9c95bd491

📥 Commits

Reviewing files that changed from the base of the PR and between 71b4141 and be88f48.

📒 Files selected for processing (6)

bin/lib/onboard.js
nemoclaw/src/blueprint/runner.test.ts
nemoclaw/src/blueprint/snapshot.test.ts
nemoclaw/src/blueprint/state.test.ts
test/cli.test.js
test/uninstall.test.js

Apply the same import type fs + importOriginal<typeof fs>() pattern from snapshot.test.ts to runner.test.ts and state.test.ts for consistency.

Replace Number(env) || default with a dedicated envInt() helper that handles 0 correctly (Number('0') || 5 would silently fall back to 5), rejects non-finite values, and clamps to non-negative integers.

The recovery loop in recoverGatewayRuntime slept unconditionally after every iteration, including the last one. Guard with the same i < count - 1 check used in startGatewayWithOptions.

ericksoa

Clean and low risk. envInt() is well-written (NaN guard, floor at 0, rounding). Defaults preserve existing behavior. Test speedup is a nice win — no reason to sleep 20s in unit tests. ESLint fixes are the correct TS pattern.

Merge origin/main into feat/jetson-orin-nano-support to resolve conflicts from recent changes (NVIDIA#1208, NVIDIA#1200, NVIDIA#836, NVIDIA#1221, NVIDIA#1223). Jetson detection now leverages main's UNIFIED_MEMORY_GPU_TAGS with added jetson flag and /proc/device-tree fallback. All 116 tests pass.

## Problem After pulling latest main, `prek run -a` fails with: 1. **ESLint error** in `snapshot.test.ts` — `@typescript-eslint/consistent-type-imports` forbids `import()` type annotations in generic parameters 2. **Slow tests** — gateway recovery tests sleep 8–19s each waiting for health polls that will never succeed against stub `openshell` scripts, adding ~40s to the suite ## Changes ### ESLint fix - `snapshot.test.ts`: replace `importOriginal<typeof import("node:fs")>()` with a top-level `import type fs from "node:fs"` and use `importOriginal<typeof fs>()` ### Test performance (70s → 28s) - `onboard.js`: make health-poll count and interval configurable via `NEMOCLAW_HEALTH_POLL_COUNT` and `NEMOCLAW_HEALTH_POLL_INTERVAL` env vars (production defaults unchanged) - `cli.test.js`: set `POLL_COUNT=1`, `POLL_INTERVAL=0` in the `runWithEnv` test helper so gateway recovery tests skip unnecessary sleep loops - `cli.test.js`: reduce test timeouts from 25s → 10s accordingly ### Formatting (pre-commit hooks) - Prettier reformatting in `runner.test.ts`, `state.test.ts`, `uninstall.test.js` ## Verification - All 740 tests pass - All pre-commit hooks pass (ESLint, Prettier, TypeScript, tests) - No production behavior change (env vars default to existing values)  ## Summary by CodeRabbit * **Chores** * Made gateway health polling configurable via environment variables (poll count and interval). * **Tests** * Reduced CLI test timeouts and injected health-check env vars to speed CI runs. * Improved test typing/mocking and cleaned up test formatting for stability and readability.

…#1223) ## Problem After pulling latest main, `prek run -a` fails with: 1. **ESLint error** in `snapshot.test.ts` — `@typescript-eslint/consistent-type-imports` forbids `import()` type annotations in generic parameters 2. **Slow tests** — gateway recovery tests sleep 8–19s each waiting for health polls that will never succeed against stub `openshell` scripts, adding ~40s to the suite ## Changes ### ESLint fix - `snapshot.test.ts`: replace `importOriginal<typeof import("node:fs")>()` with a top-level `import type fs from "node:fs"` and use `importOriginal<typeof fs>()` ### Test performance (70s → 28s) - `onboard.js`: make health-poll count and interval configurable via `NEMOCLAW_HEALTH_POLL_COUNT` and `NEMOCLAW_HEALTH_POLL_INTERVAL` env vars (production defaults unchanged) - `cli.test.js`: set `POLL_COUNT=1`, `POLL_INTERVAL=0` in the `runWithEnv` test helper so gateway recovery tests skip unnecessary sleep loops - `cli.test.js`: reduce test timeouts from 25s → 10s accordingly ### Formatting (pre-commit hooks) - Prettier reformatting in `runner.test.ts`, `state.test.ts`, `uninstall.test.js` ## Verification - All 740 tests pass - All pre-commit hooks pass (ESLint, Prettier, TypeScript, tests) - No production behavior change (env vars default to existing values)  ## Summary by CodeRabbit * **Chores** * Made gateway health polling configurable via environment variables (poll count and interval). * **Tests** * Reduced CLI test timeouts and injected health-check env vars to speed CI runs. * Improved test typing/mocking and cleaned up test formatting for stability and readability.

coderabbitai Bot reviewed Apr 1, 2026

View reviewed changes

cv added 2 commits March 31, 2026 20:24

fix: align importOriginal type pattern across all test files

f7a8f74

Apply the same import type fs + importOriginal<typeof fs>() pattern from snapshot.test.ts to runner.test.ts and state.test.ts for consistency.

fix: use safe env var parsing for health-poll settings

878b2cc

Replace Number(env) || default with a dedicated envInt() helper that handles 0 correctly (Number('0') || 5 would silently fall back to 5), rejects non-finite values, and clamps to non-negative integers.

cv enabled auto-merge (squash) April 1, 2026 03:34

fix: skip sleep after final recovery poll attempt

a0170a6

The recovery loop in recoverGatewayRuntime slept unconditionally after every iteration, including the last one. Guard with the same i < count - 1 check used in startGatewayWithOptions.

ericksoa approved these changes Apr 1, 2026

View reviewed changes

cv merged commit 2c2b7b0 into main Apr 1, 2026
8 checks passed

wscurran added the bug-fix PR fixes a bug or regression label Jun 8, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: resolve ESLint error and speed up gateway recovery tests#1223

fix: resolve ESLint error and speed up gateway recovery tests#1223
cv merged 4 commits into
mainfrom
fix/eslint-and-slow-tests

cv commented Apr 1, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 1, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

ericksoa left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

cv commented Apr 1, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes

ESLint fix

Test performance (70s → 28s)

Formatting (pre-commit hooks)

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ericksoa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cv commented Apr 1, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 1, 2026 •

edited

Loading