Skip to content

Conversation

@totalolage
Copy link

Summary

This PR fixes a race condition where server.address() could return null if called before the Express server finished binding to its port, causing a TypeError when trying to access the port property.

Problem

When using mcp-remote with OAuth-enabled servers (like Atlassian), the application would crash with:

TypeError: Cannot read properties of null (reading 'port')
    at coordinateAuth (file://.../dist/chunk-MNGQEN7H.js:14570:30)

This occurred because:

  1. app.listen() is asynchronous and returns immediately
  2. The callback is only called once the server is actually listening
  3. server.address() returns null until the server has started listening

Solution

Modified setupOAuthCallbackServerWithLongPoll to return a Promise that resolves only after the server has successfully started listening. This ensures server.address() will always return a valid address object.

Changes

  • Made setupOAuthCallbackServerWithLongPoll async and wrap the server creation in a Promise
  • Updated setupOAuthCallbackServer to await the async function
  • Updated coordinateAuth to await the setup function

Testing

Tested locally by:

  1. Building the project with npm run build
  2. Linking locally with npm link
  3. Running mcp-remote https://mcp.atlassian.com/v1/sse
  4. Verified that the server now starts without the null port error

Related Issues

This fixes issues reported by users when connecting to OAuth-enabled MCP servers, particularly Atlassian's MCP implementation.

fkalny-groupon and others added 4 commits August 29, 2025 00:08
The setupOAuthCallbackServerWithLongPoll function now returns a Promise that resolves only after the server has started listening. This prevents server.address() from returning null, which was causing a TypeError when trying to access the port property.

Fixes the issue where mcp-remote would crash with:
'TypeError: Cannot read properties of null (reading 'port')'
when initializing OAuth authentication, particularly with services like Atlassian.
When the configured OAuth callback port is already in use, the system now automatically finds an alternative available port. This prevents EADDRINUSE errors and allows multiple instances to coexist.

- Added findAvailablePort support to coordinateAuth and createLazyAuthCoordinator
- Clear old OAuth client registration when port changes to ensure correct redirect URL
- Return actual port used for OAuth callback server
- Handle port conflicts gracefully with informative error messages
Copy link
Contributor

@clouatre clouatre left a comment

Choose a reason for hiding this comment

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

Reviewed and tested locally on macOS.

Testing:

  • ✅ All unit tests pass (44/44)
  • ✅ Build succeeds without errors

Issues:

This PR contains 4 commits mixing multiple concerns:

  1. Commit cefec7a - Bug fix: Race condition in OAuth server setup
  2. Commit 5e34e03 - Feature: Dynamic port assignment for port conflicts
  3. Commit ebce393 - Adds contributor field to package.json
  4. Commit cb3daea - Updates .gitignore

Concerns:

  1. Mixed scope: Bug fix + feature + metadata changes make this difficult to review atomically
  2. Contributors field: This is the first time anyone has added themselves to package.json. Contributors with 8-12 merged commits haven't done this. Typically, git history serves as the contributor record, or the maintainer manages this field
  3. One logical change per PR: These should be separate PRs

Recommendation:

Split into focused PRs:

  • PR A: Race condition fix only (commit cefec7a)
  • PR B: Port conflict feature (commit 5e34e03)
  • Remove contributor/gitignore commits (not standard practice for self-nomination)

The bug fix is valuable and should merge quickly. The feature deserves separate discussion.

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.

3 participants