Skip to content

fix: MCPServerCheck false negative in subdirectory projects#297

Merged
bguidolim merged 1 commit intomcs-cli:mainfrom
anettodev:fix/MCPServerCheck
Mar 26, 2026
Merged

fix: MCPServerCheck false negative in subdirectory projects#297
bguidolim merged 1 commit intomcs-cli:mainfrom
anettodev:fix/MCPServerCheck

Conversation

@anettodev
Copy link
Contributor

@anettodev anettodev commented Mar 26, 2026

Summary

mcs doctor permanently reported docs-mcp-server: not registered in subdirectory projects

ProjectDetector resolves the mcs project root to the subdirectory, but claude mcp add -s local keys entries in ~/.claude.json by the git root — causing MCPServerCheck to look at the wrong path.

Why is important?

Using mcs in a git subdirectory with CLAUDE.local.md is a supported use case, not an edge case.

mcs doctor is the primary health signal users rely on to know their configuration is correct — a permanent false negative on MCP server registration erodes trust in the tool and leaves users with no actionable fix path.

Changes

  • MCPServerCheck.check() now walks up from the mcs project root to the git root when looking up projects[path].mcpServers in ~/.claude.json, matching how the Claude CLI determines the local scope key

Test plan

  • swift test passes locally
  • swiftformat --lint . and swiftlint pass without violations
  • Affected commands verified with a real pack (e.g. mcs sync, mcs doctor)
Checklist for engine changes
  • Any fix() implementation does cleanup/migration only — never installs or registers resources

Copy link
Collaborator

@bguidolim bguidolim left a comment

Choose a reason for hiding this comment

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

Review Summary

The bug is valid — verified empirically and through Claude Code docs. Claude CLI keys local-scope MCP servers by the git root, while ProjectDetector.findProjectRoot() can return a subdirectory (when CLAUDE.local.md is found before .git/). This causes MCPServerCheck to look up the wrong path in ~/.claude.json.

What needs attention

1. Same bug exists in ConfigurationDiscovery.discoverMCPServers() (line 166)

mcs export from a subdirectory project will silently miss project-scoped MCP servers — same root cause, same direct projects[projectRoot.path] lookup:

// Export/ConfigurationDiscovery.swift:165-167
let projectEntry = projects[projectRoot.path] as? [String: Any]

Suggestion: Extract the walk-up logic into a shared utility (e.g., a static method on ProjectDetector like findGitRoot(from:), or a small helper that probes ~/.claude.json project keys walking up to .git/). Both MCPServerCheck and ConfigurationDiscovery should use it.

2. Missing tests

The walk-up loop has non-trivial termination logic (.git/ sentinel + / guard). CLAUDE.md requires integration tests for doctor check changes. Minimum needed:

  • Subdirectory project root finds server keyed at git root → .pass
  • Walk-up stops at .git/ boundary → doesn't escape the repo
  • Regression: projectRoot == gitRoot still works

3. Minor simplification

candidate: URL? can be plain var candidate = rootdeletingLastPathComponent() always returns a non-nil URL, so the optional wrapper is unnecessary.

What looks good

  • Algorithm is correct: checks each ancestor before the .git/ break, so the git root itself is probed
  • Well-bounded loop: terminates at both .git/ and /
  • Edge cases handled: nil projectRoot, root == /, root == git root
  • Minimal and focused change

@bguidolim bguidolim merged commit b01f658 into mcs-cli:main Mar 26, 2026
4 checks passed
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.

2 participants