fix: version check logic and validation

[utkarsh232005]

This PR improves the version check utility by adding runtime validation for the npm registry response and cleaning up duplicate function declarations.

Key changes:

• Added runtime validation for the JSON response from the registry.
• Fixed duplicate and nested getUpdateType declarations.
• Added comprehensive unit tests in src/__tests__/version-check.test.ts.
• Exported compareSemver and getUpdateType for better testability.

Summary by CodeRabbit

• Tests

  • Added comprehensive test coverage for version comparison and update-checking logic, including semantic-version ordering, update-type classification, error scenarios, and messaging when a newer version is found.

• Bug Fixes

  • Strengthened update-checking robustness by validating registry responses at runtime, logging invalid-response errors, and avoiding crashes or false reports on malformed or failed fetches.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Exports semantic-version helpers (compareSemver, getUpdateType), adds logger and runtime validation for checkForUpdates to ensure fetched registry JSON contains a string version, logs on invalid payloads, handles network errors silently, and adds Vitest coverage for all behaviors.

Changes

Version Check Utilities with Registry Validation

Layer / File(s)	Summary
Version comparison helpers src/utils/version-check.ts, src/__tests__/version-check.test.ts	Exported compareSemver(a, b) and getUpdateType(installed, latest); tests cover lt/gt/eq, leading-v equality, and major/minor/patch classification (empty string for equal/downgrade).
Registry response validation and logging src/utils/version-check.ts, src/__tests__/version-check.test.ts	Added logger import; checkForUpdates treats fetched JSON as unknown, validates presence of a string version, logs Invalid registry response: version not found and returns early on invalid shape, silently handles fetch rejections, and logs available-update messages to console when applicable. Tests stub fetch and assert error logging, silent failures, and console output for updates.

Sequence Diagram(s)

sequenceDiagram
  participant checkForUpdates
  participant fetch
  participant RegistryAPI
  participant logger
  participant console
  checkForUpdates->>fetch: request latest package metadata
  fetch->>RegistryAPI: GET /package
  RegistryAPI-->>fetch: response JSON (unknown)
  fetch-->>checkForUpdates: parsed JSON
  checkForUpdates->>checkForUpdates: validate { version: string }
  alt invalid structure
    checkForUpdates->>logger: error("Invalid registry response: version not found")
    checkForUpdates-->>checkForUpdates: return
  else network error
    fetch-->>checkForUpdates: rejection
    checkForUpdates-->>checkForUpdates: return (silent)
  else valid version
    checkForUpdates->>checkForUpdates: compareSemver(installed, latest)
    alt newer available
      checkForUpdates->>console: log("update available!", latest)
    end
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• KDM-cli/kdm-cli#29: Earlier version-check changes touching compareSemver/getUpdateType and checkForUpdates behavior; closely related to these stricter validations and exports.

Poem

🛠️ Versions now checked with care and light,

exports revealed, tests guarding the night.
Bad JSON is logged, network woes stay mute,
New releases shout loud — console prints the truth.
Small helpers, strong checks; the CLI sleeps tight.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 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: adding runtime validation for version check logic and exporting utilities for testability.
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 unit tests (beta)

• Create PR with unit tests

---

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

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/utils/version-check.ts (1)

33-40: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Handle downgrade inputs in exported getUpdateType.

When installed is newer than latest, compareSemver returns 'gt', but this logic falls through and returns 'patch'. For an exported helper, downgrades should not be classified as updates.

Proposed fix

 export function getUpdateType(installed: string, latest: string): string {
   const cmp = compareSemver(installed, latest);
-  if (cmp === 'eq') return '';
+  if (cmp !== 'lt') return '';
   const [i1 = 0, i2 = 0] = installed.replace(/^v/, '').split('.').map(Number);
   const [l1 = 0, l2 = 0] = latest.replace(/^v/, '').split('.').map(Number);
   if (l1 > i1) return 'major';
   if (l2 > i2) return 'minor';
   return 'patch';
 }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/utils/version-check.ts` around lines 33 - 40, The exported getUpdateType
currently treats a newer installed version as a 'patch' because it ignores
compareSemver's 'gt' result; update the function (getUpdateType) to explicitly
handle the 'gt' case (in addition to 'eq') and return '' for downgrades so they
are not classified as updates, keeping the existing major/minor/patch logic for
actual newer latest versions.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/__tests__/version-check.test.ts`:
- Around line 57-60: The teardown currently calls vi.unstubAllGlobals() and
vi.clearAllMocks() but does not restore spy implementations (the console.log spy
created near line 54), leaving spies active across tests; update the afterEach
to call vi.restoreAllMocks() (either in addition to or instead of
vi.clearAllMocks()) so spies and mocked implementations created during tests are
restored to their originals, ensuring test isolation for the afterEach block and
functions like the console.log spy.

---

Outside diff comments:
In `@src/utils/version-check.ts`:
- Around line 33-40: The exported getUpdateType currently treats a newer
installed version as a 'patch' because it ignores compareSemver's 'gt' result;
update the function (getUpdateType) to explicitly handle the 'gt' case (in
addition to 'eq') and return '' for downgrades so they are not classified as
updates, keeping the existing major/minor/patch logic for actual newer latest
versions.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2700e711-1928-4cc7-a322-9cf497398304

📥 Commits

Reviewing files that changed from the base of the PR and between 6530e83 and a3b084f.

📒 Files selected for processing (2)

• src/__tests__/version-check.test.ts
• src/utils/version-check.ts