Skip to content

Refactor: Extract duplicated JSON parsing in CursorMcp and fix ClaudecodeMcp.fromFile global flag #1285

@dyoshikawa

Description

@dyoshikawa

Background

During the review of PR #1273 (Add Cursor MCP support for global mode), two medium-severity issues were identified. These are not blockers for the PR itself but represent technical debt that should be addressed to improve code quality and prevent potential bugs.

Details

1. DRY Violation: Duplicated JSON parsing with error handling (Medium)

File: src/features/mcp/cursor-mcp.ts

The same JSON parse-with-descriptive-error pattern is repeated three times in CursorMcp: in the constructor, in fromFile, and in fromRulesyncMcp. Each uses an identical try/catch block:

try {
  json = JSON.parse(fileContent);
} catch (error) {
  throw new Error(
    `Failed to parse Cursor MCP config at ${join(paths.relativeDirPath, paths.relativeFilePath)}: ${formatError(error)}`,
    { cause: error },
  );
}

This should be extracted into a private static helper method to consolidate the error message format in one place and make future changes easier.

Note: The existing ClaudecodeMcp does not wrap its JSON.parse calls in try/catch at all (its constructor uses JSON.parse(this.fileContent || "{}") bare), so there is a divergence in error handling strategy between the two sibling classes. Ideally this should be resolved codebase-wide.

2. Pre-existing Bug: ClaudecodeMcp.fromFile does not pass global to constructor (Medium)

File: src/features/mcp/claudecode-mcp.ts

In ClaudecodeMcp.fromFile, the global flag is not passed to the constructor. Since isDeletable() depends on this.global, this means isDeletable() would always return true for instances created via ClaudecodeMcp.fromFile, even in global mode where it should return false.

The new CursorMcp.fromFile in PR #1273 correctly passes global to the constructor, highlighting the inconsistency in the existing ClaudecodeMcp implementation.

Solution / Next Steps

  1. Extract JSON parsing helper: Create a shared private static method (or a utility function) for JSON parsing with descriptive error messages, and use it across both CursorMcp and ClaudecodeMcp.
  2. Fix ClaudecodeMcp.fromFile: Pass the global parameter to the ClaudecodeMcp constructor in the fromFile method, matching the correct pattern used in CursorMcp.fromFile.
  3. Harmonize error handling: Consider adding consistent JSON parse error handling to ClaudecodeMcp as well, using the same shared helper.

Related PR: #1273

Metadata

Metadata

Assignees

No one assigned

    Labels

    maintainer-scrapRough notes for AI implementation. Not for human eyes.refactoring

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions