Skip to content

chore: Update dependencies and fix version strings#29

Merged
bearmug merged 1 commit into
mainfrom
chore/dependency-maintenance
Jan 5, 2026
Merged

chore: Update dependencies and fix version strings#29
bearmug merged 1 commit into
mainfrom
chore/dependency-maintenance

Conversation

@bearmug

@bearmug bearmug commented Jan 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix 3 security vulnerabilities (1 moderate, 2 high) in qs dependency via npm audit fix
  • Update outdated dependencies:
    • @modelcontextprotocol/sdk 1.20.1 → 1.25.1
    • @types/node 20.19.23 → 20.19.27
    • tsx 4.20.6 → 4.21.0
  • Read version from package.json dynamically instead of hardcoding 0.1.0 in source files

Test plan

  • npm audit shows 0 vulnerabilities
  • npm run build succeeds
  • All 82 tests pass

🤖 Generated with Claude Code

- Fix security vulnerabilities via npm audit fix (qs)
- Update @modelcontextprotocol/sdk 1.20.1 → 1.25.1
- Update @types/node 20.19.23 → 20.19.27
- Update tsx 4.20.6 → 4.21.0
- Read version from package.json instead of hardcoding in:
  - src/index.ts (MCP server version)
  - src/api/client.ts (User-Agent header)

🤖 Generated with [Claude Code](https://claude.ai/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@bearmug bearmug enabled auto-merge (squash) January 5, 2026 22:35
@bearmug bearmug merged commit 65bc00b into main Jan 5, 2026
5 checks passed
@claude

claude Bot commented Jan 5, 2026

Copy link
Copy Markdown

Pull Request Review

Thank you for this maintenance PR! Overall, this is a solid update that addresses security vulnerabilities and dependency updates. Here's my detailed feedback:

✅ Positive Aspects

  1. Security Fix: Excellent work addressing 3 security vulnerabilities in the qs dependency (1 moderate, 2 high). This is a critical improvement.

  2. Dynamic Version Reading: The change from hardcoded 0.1.0 to reading from package.json is a best practice. This eliminates version drift and ensures consistency.

  3. Dependency Updates: Updating to newer versions of core dependencies:

    • @modelcontextprotocol/sdk 1.20.1 → 1.25.1
    • @types/node 20.19.23 → 20.19.27
    • tsx 4.20.6 → 4.21.0
  4. Test Coverage: All 82 tests passing confirms backward compatibility.


🔍 Code Quality Review

src/api/client.ts (lines 6-10, 28)

import { createRequire } from "module";
const require = createRequire(import.meta.url);
const pkg = require("../../package.json");

Good: This approach works well for ESM modules. The version is now dynamically read at runtime.

Minor consideration: The createRequire pattern is correct for ESM, but consider that:

  • This reads package.json on every module load (acceptable for an MCP server)
  • The path ../../package.json is relative to the built dist/ directory (correct ✓)

src/index.ts (lines 23-26, 47)

const require = createRequire(import.meta.url);
const pkg = require("../package.json");

Good: Same pattern applied consistently. Path is ../package.json which is correct from dist/index.js.


🔒 Security Analysis

  1. qs vulnerability fix: Updated from 6.14.0 → 6.14.1

    • This is a patch release addressing security issues
    • ✅ No breaking changes expected
  2. ajv major version bump: 6.12.6 → 8.17.1

    • This is a major version change brought in by the MCP SDK update
    • The SDK now uses ajv@8 with ajv-formats for JSON schema validation
    • ✅ Validated via successful test suite
  3. New dependencies introduced by MCP SDK:

    • @hono/node-server - HTTP server (used internally by SDK)
    • ajv-formats - JSON schema format validators
    • jose - JWT/JWE/JWS implementation
    • json-schema-typed - TypeScript JSON schema types
    • ✅ All legitimate dependencies for the SDK's new features

⚠️ Potential Concerns

1. Breaking Changes in Dependencies

The MCP SDK update (1.20.1 → 1.25.1) is a minor version bump, but includes:

  • Major version change in ajv (6 → 8)
  • New peer dependency on express (marked as peer, not direct dependency)
  • New authentication-related dependencies (jose)

Recommendation: Since tests pass, this appears safe. However, consider checking the MCP SDK changelog for any behavioral changes in versions 1.21-1.25.

2. Package Lock File Size

The package-lock.json grew significantly due to:

  • esbuild platform-specific binaries updated (0.25.11 → 0.27.2)
  • New transitive dependencies from MCP SDK

Impact: Minimal - this is normal for dependency updates.

3. Version in package.json

"version": "0.0.0-dev"

Question: Is this intentional? The PR mentions reading version dynamically, but the package.json shows 0.0.0-dev. If this is a development version scheme, that's fine. Otherwise, consider if this should be a semantic version like 0.2.0.


🧪 Test Coverage

Coverage appears adequate:

  • 82 tests passing validates:
    • Cache functionality
    • Rate limiting
    • Field listing
    • Symbol lookup
    • Presets
    • Screening tools

Recommendation: Since the User-Agent header now uses the dynamic version, consider adding a test to verify the version is correctly read and not undefined or 0.0.0-dev in production builds.


🎯 Performance Considerations

User-Agent Header Construction (src/api/client.ts:28):

"User-Agent": `tradingview-mcp-server/${pkg.version}`
  • The pkg object is created once at module load time ✓
  • No performance impact from reading package.json on every request ✓

📝 Suggestions

  1. Add a test for version reading:

    // src/tests/client.test.ts (new file)
    import { describe, test } from 'node:test';
    import assert from 'node:assert';
    import { createRequire } from 'module';
    
    test('version should be read from package.json', () => {
      const require = createRequire(import.meta.url);
      const pkg = require('../../package.json');
      assert.ok(pkg.version);
      assert.notStrictEqual(pkg.version, '0.1.0');
    });
  2. Consider adding a version command:
    Could be useful for debugging:

    tradingview-mcp-server --version
  3. Document the dependency updates:
    Consider adding a note in the README or changelog about the MCP SDK update, especially if it brings new features.


✅ Final Verdict

LGTM with minor suggestions

This PR is well-executed and ready to merge. The security fixes are important, and the dynamic version reading is a solid improvement. The test suite passing provides confidence in backward compatibility.

Merge recommendation: ✅ Approve and merge


📋 Checklist Summary

  • ✅ Security vulnerabilities fixed
  • ✅ Tests pass (82/82)
  • ✅ No breaking API changes
  • ✅ Code quality is good
  • ✅ Follows project conventions (CLAUDE.md)
  • ⚠️ Minor: Consider adding version test
  • ℹ️ Info: Large package-lock diff is expected

Great work! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant