Skip to content

fix(mcp): use a more generic folder to put mcp filesystem root #1413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mfuntowicz
Copy link
Member

@mfuntowicz mfuntowicz commented Apr 30, 2025

Currently the MCP server attempts to access a folder under ${HOMEDIR}/Desktop which is not portable accross different OSes.

This PR proposes to change the Desktop part to .mcp which is OS agnostic and ensure the folder exists when spawning.

@mfuntowicz mfuntowicz requested a review from julien-c April 30, 2025 20:40
Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

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

👍 about not defaulting to ~/Desktop but not sure ~/.mcp is really relevant either (it would only be able to work on new files, not existing ones).

Another possibility is to default to ~/.mcp and make it a optional parameter of the CLI.


const SERVERS: StdioServerParameters[] = [
{
// Filesystem "official" mcp-server with access to your Desktop
command: "npx",
args: ["-y", "@modelcontextprotocol/server-filesystem", join(homedir(), "Desktop")],
args: ["-y", "@modelcontextprotocol/server-filesystem", MCP_LOCAL_FOLDER],
Copy link
Member

Choose a reason for hiding this comment

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

i would rather still use Desktop on mac OS and just homedir() on Win32, can you switch to this?

You can use process.platform === "darwin" to figure out

Thanks!

Comment on lines +54 to +57
if (!existsSync(MCP_LOCAL_FOLDER)) {
mkdirSync(MCP_LOCAL_FOLDER);
console.info("created folder", MCP_LOCAL_FOLDER);
}
Copy link
Member

Choose a reason for hiding this comment

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

note (unimportant given we'll not need that code) you're inside an async block so you can use the promise-based variants and just await them

@julien-c
Copy link
Member

julien-c commented May 2, 2025

i can finish your PR if you want @mfuntowicz? let me know

@mfuntowicz
Copy link
Member Author

I've put ~/.mcp to have something which can be easily implemented without branching on plateform.
@julien-c if we are fine with the branching, I totally think we can do something closer to the original for all platform.

Happy to take a snab at it 👍🏻

@julien-c
Copy link
Member

julien-c commented May 2, 2025

yes, branching depending on OS is completely fine

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