feat: support multiple concurrent Claude Code instances

[nicksenap]

Summary

• Server: MCP server auto-discovers a free port in range 9222–9231 instead of crashing with EADDRINUSE when port 9222 is taken
• Extension: New ConnectionManager connects to all ports in the range simultaneously, so it talks to every running MCP server instance
• Shared: Port range constants (DEFAULT_WS_PORT, WS_PORT_RANGE_SIZE) in @paparazzi/shared so both sides agree on the range

Test plan

• pnpm build — all packages compile
• pnpm test:run — 59/59 tests pass (3 new port discovery tests)
• pnpm lint — clean
• pnpm typecheck — clean
• pnpm knip — no new unused exports

🤖 Generated with Claude Code

[claude]

PR Review: Support Multiple Concurrent Claude Code Instances

Summary

This PR adds support for running multiple Claude Code instances concurrently by implementing port range discovery (9222-9231) on the server side and multi-port connection management on the extension side. Overall, this is a well-implemented feature with good test coverage.

✅ Strengths

1. Code Quality

• Clean, readable code with good separation of concerns
• Excellent use of TypeScript types and interfaces
• Consistent error handling patterns
• Well-structured classes with clear responsibilities

2. Architecture

• Smart design: servers auto-discover ports, extensions connect to all
• Shared constants in @paparazzi/shared ensure consistency
• New ConnectionManager class provides good abstraction
• Backward compatible - no breaking changes

3. Test Coverage

• Comprehensive test suite with 3 new port discovery tests
• Tests cover both success and failure scenarios
• Good edge case testing (port exhaustion, concurrent bridges)
• All 59 tests passing

4. Documentation

• Clear JSDoc comments on all public methods
• Helpful inline comments explaining behavior
• Good error messages for debugging

🔍 Areas for Consideration

1. Resource Management - ConnectionManager (Medium Priority)

Issue: The ConnectionManager creates WebSocket clients for all 10 ports immediately, even if only 1-2 servers are running.

Location: packages/extension/src/background/connection-manager.ts:18-25

Impact:

• 10 concurrent connection attempts on every reconnect
• 10 active reconnection timers (every 3 seconds)
• Console spam from 8-9 failed connections
• Unnecessary resource consumption

Suggestion:
Consider a "lazy discovery" approach:

1. Ping each port sequentially with a short timeout (100-200ms)
2. Only create persistent WebSocket clients for ports that respond
3. Periodically re-scan for new instances

Alternative: If you want to keep the current approach, document this behavior and consider:

• Increasing reconnection intervals for unused ports
• Adding exponential backoff for failed connections

2. Request Broadcasting Logic Missing (High Priority)

Issue: The extension connects to multiple servers, but I don't see logic for which server should handle a request.

Questions:

• Does ConnectionManager need a method to send requests to a specific server?
• Should requests go to all servers or just one?
• How does the MCP server know which extension connection corresponds to which Claude Code instance?

Current state: ConnectionManager only has connectAll(), pingAll(), and isAnyConnected() - no request routing.

Suggestion: Clarify the request routing strategy:

// Option 1: Broadcast to all
async requestAll(action, params): Promise<unknown[]>

// Option 2: Route to first available
async request(action, params): Promise<unknown>

// Option 3: Route by criteria
async requestToServer(serverId, action, params): Promise<unknown>

3. Error Handling in Port Discovery (Low Priority)

Issue: In websocket-server.ts:74-90, non-EADDRINUSE errors immediately throw and stop port scanning.

Scenario: If port 9222 has a transient permission error, the entire startup fails instead of trying 9223-9231.

Suggestion:

} catch (err) {
  if ((err as NodeJS.ErrnoException).code === 'EADDRINUSE') {
    console.error(`[Paparazzi] Port ${port} is already in use, trying next...`);
    continue;
  }
  // Log but continue trying other ports for other errors
  console.error(`[Paparazzi] Port ${port} failed with error, trying next...`, err);
  continue; // Instead of: throw err;
}

Only throw if all ports fail.

4. Port Range Size Configuration (Low Priority)

Issue: WS_PORT_RANGE_SIZE = 10 is hardcoded in shared constants.

Consideration:

• Is 10 a reasonable limit? (supports 10 concurrent instances)
• Should this be configurable via environment variable?
• Consider documenting the rationale for 10

Suggestion:

export const WS_PORT_RANGE_SIZE = Number(process.env.PAPARAZZI_PORT_RANGE_SIZE) || 10;

5. Logging Improvements (Low Priority)

Current: Each client logs with its port: [Paparazzi:9222]

Enhancement: Consider adding timestamps and log levels for production debugging:

console.error(`${new Date().toISOString()} ${this.logPrefix} Connected to MCP server`);

6. Race Condition in tryPort (Very Low Priority)

Location: websocket-server.ts:56-68

Issue: The error handler calls wss.close() but doesn't wait for it to complete before rejecting.

Impact: Minimal - port will be released shortly, but technically not immediately.

Suggestion:

wss.on('error', (err) => {
  wss.close(() => reject(err));
});

🔒 Security Review

✅ No security concerns identified

• All connections are localhost only (127.0.0.1)
• No external network exposure
• No user input validation needed for port numbers
• Standard WebSocket security practices followed

🚀 Performance Considerations

1. Connection Attempts: 10 simultaneous connection attempts may cause brief CPU spike on startup
2. Memory: 10 WebSocket client instances = minimal overhead (~10KB each)
3. Network: All local, negligible impact

Verdict: Performance impact is acceptable for the use case.

📝 Minor Suggestions

1. TypeScript: Consider using readonly for class properties that don't change:

   private readonly clients: WebSocketClient[];

2. Constants: Export getPort() result when starting server for better debugging:

   console.error(`[Paparazzi] Server started on port ${bridge.getPort()}`);

   ✅ Already done in index.ts:23 - nice!

3. Testing: Consider adding a test for the ConnectionManager.pingAll() method to ensure it handles mixed connected/disconnected states.

🎯 Recommendations

Must Address Before Merge:

1. Clarify request routing logic - How do requests get sent to specific servers?

Should Address:

1. Resource management - Consider lazy connection strategy or document current behavior
2. Error handling - Don't fail startup on first non-EADDRINUSE error

Nice to Have:

1. Environment variable for port range size
2. Better logging with timestamps
3. Add tests for ConnectionManager.pingAll()

✨ Overall Assessment

Rating: 8/10 - Strong implementation with excellent test coverage. The architecture is sound, and the code quality is high. The main gap is clarity around request routing between multiple servers. Address the "must fix" item and this will be ready to merge!

Great work on this feature! 🎉