Update server version handling and add tests - #63
Merged
wpak-ai merged 4 commits intoMay 5, 2026
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR replaces a hardcoded SERVER_VERSION with runtime resolution from package.json (with DEFAULT_SERVER_VERSION fallback and logging), re-exports SERVER_VERSION from the resolver, adds tests for parsing/resolution behavior, and adds a ChangesServer Version Alignment
Repository Ignore Update
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/server-version.ts (1)
16-19: ⚡ Quick winHarden
versionvalidation to reject whitespace-only values.Line 16 accepts
" "as valid because it only checks.length. Trimming before validation avoids emitting effectively empty server metadata.Proposed diff
export function parsePackageJsonVersion(raw: string, pathForErrors = 'package.json'): string { const parsed = JSON.parse(raw) as { version?: unknown }; - if (typeof parsed.version !== 'string' || parsed.version.length === 0) { + if (typeof parsed.version !== 'string' || parsed.version.trim().length === 0) { throw new Error(`Invalid or missing "version" in ${pathForErrors}`); } - return parsed.version; + return parsed.version.trim(); }🤖 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/server-version.ts` around lines 16 - 19, The current validation in server-version.ts checks only parsed.version.length, which accepts whitespace-only strings; update the validation in the code that reads parsed.version to trim the value and reject empty/whitespace-only strings (e.g., check typeof parsed.version === 'string' and parsed.version.trim().length > 0), and return the trimmed version instead of the original so downstream consumers get a normalized version; reference parsed.version and pathForErrors in your change.src/server-version.test.ts (1)
28-35: ⚡ Quick winAdd parser negative-path tests to lock validation behavior.
Current tests validate happy paths only. Add cases for malformed JSON, missing
version, and empty/whitespaceversionso parser guarantees are regression-safe.Proposed diff
describe('parsePackageJsonVersion', () => { it('extracts version from several package.json shapes', () => { expect(parsePackageJsonVersion(PACKAGE_JSON_FIXTURES[0])).toBe('0.1.0'); expect(parsePackageJsonVersion(PACKAGE_JSON_FIXTURES[1])).toBe('1.0.0'); expect(parsePackageJsonVersion(PACKAGE_JSON_FIXTURES[2])).toBe('0.1.6'); expect(parsePackageJsonVersion(PACKAGE_JSON_FIXTURES[3])).toBe('2.3.4'); }); + + it('throws on malformed or invalid version fields', () => { + expect(() => parsePackageJsonVersion('{')).toThrow(); + expect(() => parsePackageJsonVersion(JSON.stringify({ name: 'x' }))).toThrow(); + expect(() => parsePackageJsonVersion(JSON.stringify({ version: '' }))).toThrow(); + expect(() => parsePackageJsonVersion(JSON.stringify({ version: ' ' }))).toThrow(); + }); });🤖 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/server-version.test.ts` around lines 28 - 35, Add negative-path unit tests for parsePackageJsonVersion to cover malformed JSON, missing version fields, and empty/whitespace version strings so behavior is regression-safe: update the test suite in src/server-version.test.ts (the describe('parsePackageJsonVersion') block) to include new it cases that call parsePackageJsonVersion with fixtures representing (1) invalid JSON input (should throw or return the defined error/undefined behavior your parser uses), (2) an object missing the version property (should return undefined or throw per current contract), and (3) version set to '' or ' ' (should be treated as invalid); reuse PACKAGE_JSON_FIXTURES or add new fixture entries and assert the expected error/return values consistent with parsePackageJsonVersion's specified behavior.
🤖 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.
Nitpick comments:
In `@src/server-version.test.ts`:
- Around line 28-35: Add negative-path unit tests for parsePackageJsonVersion to
cover malformed JSON, missing version fields, and empty/whitespace version
strings so behavior is regression-safe: update the test suite in
src/server-version.test.ts (the describe('parsePackageJsonVersion') block) to
include new it cases that call parsePackageJsonVersion with fixtures
representing (1) invalid JSON input (should throw or return the defined
error/undefined behavior your parser uses), (2) an object missing the version
property (should return undefined or throw per current contract), and (3)
version set to '' or ' ' (should be treated as invalid); reuse
PACKAGE_JSON_FIXTURES or add new fixture entries and assert the expected
error/return values consistent with parsePackageJsonVersion's specified
behavior.
In `@src/server-version.ts`:
- Around line 16-19: The current validation in server-version.ts checks only
parsed.version.length, which accepts whitespace-only strings; update the
validation in the code that reads parsed.version to trim the value and reject
empty/whitespace-only strings (e.g., check typeof parsed.version === 'string'
and parsed.version.trim().length > 0), and return the trimmed version instead of
the original so downstream consumers get a normalized version; reference
parsed.version and pathForErrors in your change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9f41149a-0356-43cd-a924-c447305bd90e
📒 Files selected for processing (4)
.gitignoresrc/constants.tssrc/server-version.test.tssrc/server-version.ts
Introduce custom error classes for handling invalid or missing version fields in package.json. Update the version parsing logic to utilize these errors and add corresponding tests to ensure proper functionality. This enhances the robustness of server version management.
…t default fallback
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/server-version.test.ts (1)
29-59: ⚡ Quick winAdd explicit tests for malformed JSON and missing
version.Current coverage is good, but Line 20 (missing
version) and Line 34 (JSON parse failure) in the production parser are still untested. Adding those two cases will lock in fallback behavior.Suggested test additions
describe('parsePackageJsonVersion', () => { + it('returns default when version field is missing', () => { + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + try { + expect(parsePackageJsonVersion(JSON.stringify({ name: 'pkg' }))).toBe('0.0.1'); + expect(logSpy).toHaveBeenCalled(); + } finally { + logSpy.mockRestore(); + } + }); + + it('returns default when package.json is malformed JSON', () => { + const logSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + try { + expect(parsePackageJsonVersion('{')).toBe('0.0.1'); + expect(logSpy).toHaveBeenCalled(); + } finally { + logSpy.mockRestore(); + } + }); + it('extracts version from several package.json shapes', () => {🤖 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/server-version.test.ts` around lines 29 - 59, Add two tests for parsePackageJsonVersion: one that passes malformed JSON (e.g. a non-JSON string) and asserts it returns the default '0.0.1' and that console.log was called, and another that passes JSON with the version property missing (e.g. JSON.stringify({})) and asserts the same default return and a console.log call; use vi.spyOn(console, 'log').mockImplementation(() => {}) around each assertion and restore the spy in finally blocks to mirror the existing tests.
🤖 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/server-version.ts`:
- Around line 21-63: The diagnostics in parsePackageJsonVersion and
resolveServerVersion are written to stdout via console.log and will corrupt the
stdio protocol; change all diagnostic console.log calls in
parsePackageJsonVersion (the messages for missing/invalid "version", empty
"version", and parse error) and in resolveServerVersion (the "package.json not
found" and "could not read" messages) to use console.error instead so
diagnostics go to stderr; update the console.log -> console.error for those five
messages and keep the same message text and error-detail construction.
---
Nitpick comments:
In `@src/server-version.test.ts`:
- Around line 29-59: Add two tests for parsePackageJsonVersion: one that passes
malformed JSON (e.g. a non-JSON string) and asserts it returns the default
'0.0.1' and that console.log was called, and another that passes JSON with the
version property missing (e.g. JSON.stringify({})) and asserts the same default
return and a console.log call; use vi.spyOn(console,
'log').mockImplementation(() => {}) around each assertion and restore the spy in
finally blocks to mirror the existing tests.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31740c02-fd4c-4a5e-8070-6857bf4e80fe
📒 Files selected for processing (2)
src/server-version.test.tssrc/server-version.ts
leostar0412
requested review from
jonathanMLDev
and removed request for
jonathanMLDev
May 5, 2026 05:04
…ror instead of console.log for invalid version cases
jonathanMLDev
approved these changes
May 5, 2026
wpak-ai
approved these changes
May 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Description
The MCP server advertised
SERVER_VERSIONas a separate string in source, so it could fall behind the real npm package version. Clients read the version from the MCPinitializehandshake, which made diagnostics and tooling misleading compared topackage.json/npm ls.This PR derives
SERVER_VERSIONat load time from the rootpackage.jsonnext to the built entrypoint (same layout as a published install undernode_modules/.../package.json).parsePackageJsonVersioncentralizes validation (non-empty stringversionfield).constants.tsre-exportsSERVER_VERSIONfromserver-version.jssoserver.tsbehavior is unchanged.Fixes #62
Type of Change
Changes Made
src/server-version.ts: read rootpackage.json, exportSERVER_VERSIONandparsePackageJsonVersion.SERVER_VERSIONfromsrc/constants.tsinstead of a hard-coded literal.src/server-version.test.ts: parsing fixtures, alignment helper tests, and a live check thatSERVER_VERSIONmatches the repo’s rootpackage.json.Testing
Describe the tests you ran to verify your changes:
Test Configuration
node -v)Suggested verification: after
npm run build, start the server and confirm the reported MCP server version matches the installed package’sversioninpackage.json.Checklist
Summary by CodeRabbit
New Features
Tests
Chores