Skip to content

Fix npx MCP entrypoint - #2

Merged
ElliotPadfield merged 1 commit into
mainfrom
codex/fix-npx-mcp-entrypoint
Jun 6, 2026
Merged

Fix npx MCP entrypoint#2
ElliotPadfield merged 1 commit into
mainfrom
codex/fix-npx-mcp-entrypoint

Conversation

@ElliotPadfield

Copy link
Copy Markdown
Member

Summary

  • Fix CLI entrypoint detection for npm bin symlinks so npx actually starts the MCP bridge
  • Add a regression test for symlinked npm bin invocation
  • Bump package/server metadata to 0.1.2 and include pnpm lockfile

Verification

  • pnpm test
  • pnpm type-check
  • pnpm lint
  • pnpm build
  • pnpm dlx ajv-cli validate --strict=false -s /tmp/mcp-server.schema.json -d server.json
  • Live smoke: npx -y -p ./influship-mcp-0.1.2.tgz influship-mcp initialized over stdio, connected to https://mcp.influship.com/mcp, and listed 14 tools

Publish note

npm publish is blocked from this local shell by npm scope permissions: registry returned 404 / no permission for @influship/mcp@0.1.2.

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Enhanced CLI entrypoint detection mechanism.
  • Tests

    • Added test coverage for CLI entrypoint detection.
  • Chores

    • Bumped version from 0.1.1 to 0.1.2.

Walkthrough

This PR introduces an isCliEntrypoint utility function that detects whether a module URL matches the CLI entrypoint by normalizing filesystem paths using resolve() and realpathSync(). The implementation is tested against a simulated npm bin-style symlink, and the release version is bumped from 0.1.1 to 0.1.2 across all manifests.

Changes

CLI Entrypoint Detection Release

Layer / File(s) Summary
isCliEntrypoint implementation
src/cli.ts
Added realpathSync import and exported isCliEntrypoint(moduleUrl, argvPath) function that normalizes both path arguments via resolve() and realpathSync.native() (with fallback to resolved path on error), returning false when argvPath is absent.
isCliEntrypoint test
src/cli.test.ts
Added Node.js utilities imports (fs, os, path, url) and new Vitest suite that creates a temp directory with a CLI file and symlink, verifying isCliEntrypoint returns true when comparing the module URL against the bin symlink path.
Release version updates
glama.json, package.json, server.json
Version incremented from 0.1.1 to 0.1.2 in package manifest and server manifest (top-level and packages array); maintainers array in glama.json reformatted to single-line.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.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
Title check ✅ Passed The title 'Fix npx MCP entrypoint' directly describes the primary change—resolving CLI entrypoint detection for npm bin symlinks to enable npx to start the MCP bridge correctly.
Description check ✅ Passed The description is directly related to the changeset, detailing the bug fix, test additions, version bumps, verification steps, and publication status.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-npx-mcp-entrypoint

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/cli.test.ts (1)

1-1: ⚡ Quick win

Clean up temporary test artifacts in a finally block.

The test creates a temp directory/symlink but never removes it. Add deterministic cleanup to keep local and CI runs hygienic.

Proposed patch
-import { mkdtempSync, symlinkSync, writeFileSync } from 'node:fs';
+import { mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
@@
   it('recognizes npm bin symlinks as the CLI entrypoint', () => {
     const tempDir = mkdtempSync(join(tmpdir(), 'influship-mcp-'));
     const cliPath = join(tempDir, 'cli.js');
     const binPath = join(tempDir, 'influship-mcp');
-    writeFileSync(cliPath, '');
-    symlinkSync(cliPath, binPath);
-
-    expect(isCliEntrypoint(pathToFileURL(cliPath).href, binPath)).toBe(true);
+    try {
+      writeFileSync(cliPath, '');
+      symlinkSync(cliPath, binPath);
+      expect(isCliEntrypoint(pathToFileURL(cliPath).href, binPath)).toBe(true);
+    } finally {
+      rmSync(tempDir, { recursive: true, force: true });
+    }
   });
 });

Also applies to: 43-51

🤖 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/cli.test.ts` at line 1, The test in src/cli.test.ts creates temporary
artifacts using mkdtempSync, symlinkSync, and writeFileSync but never removes
them—wrap the setup and assertions inside a try/finally and in the finally block
synchronously remove the symlink, files, and temp directory (use appropriate fs
unlinkSync/rmdirSync or rmSync calls) to guarantee deterministic cleanup after
the test; reference the temp dir variable returned by mkdtempSync and the path
used with symlinkSync/writeFileSync when performing the removals.
🤖 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/cli.test.ts`:
- Line 1: The test in src/cli.test.ts creates temporary artifacts using
mkdtempSync, symlinkSync, and writeFileSync but never removes them—wrap the
setup and assertions inside a try/finally and in the finally block synchronously
remove the symlink, files, and temp directory (use appropriate fs
unlinkSync/rmdirSync or rmSync calls) to guarantee deterministic cleanup after
the test; reference the temp dir variable returned by mkdtempSync and the path
used with symlinkSync/writeFileSync when performing the removals.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 01da1933-60c1-45e3-a7d5-c71571a45319

📥 Commits

Reviewing files that changed from the base of the PR and between 46414d7 and 1a5ad0d.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • glama.json
  • package.json
  • server.json
  • src/cli.test.ts
  • src/cli.ts

@ElliotPadfield
ElliotPadfield merged commit d630385 into main Jun 6, 2026
2 checks passed
@ElliotPadfield
ElliotPadfield deleted the codex/fix-npx-mcp-entrypoint branch June 6, 2026 14:31
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