fix(mcp): apply directory argument as the working directory#1600
fix(mcp): apply directory argument as the working directory#1600jmadren wants to merge 2 commits into
directory argument as the working directory#1600Conversation
The `directory` argument on the `test`, `packages_get`, and `packages_check_licenses` MCP tools was appended as a positional CLI argument. Those commands resolve their target package from `Directory.current` and treat positionals as test/target paths, so the value never changed where the command ran. In a monorepo the tools therefore executed from the MCP server's current directory (commonly a folder with no `pubspec.yaml`) and failed with exit code 66, regardless of whether a relative or absolute `directory` was supplied. Apply `directory` by switching `Directory.current` for the duration of the run and restoring it in a `finally` afterwards. Relative paths resolve against the server's current directory, matching the intent of the `path`/`directory` parameter from VeryGoodOpenSource/vgv-ai-flutter-plugin#70. A non-existent directory now surfaces a clear tool error instead of a confusing exit code. Refs VeryGoodOpenSource/vgv-ai-flutter-plugin#70 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thank you @jmadren for the fix and contribution! Let me take a look and test and i will get back to you 💙 |
ryzizub
left a comment
ryzizub
left a comment
There was a problem hiding this comment.
Hey, this is an awesome contribution and it actually solves the issue!
But I think we could add that option to the test command itself and not doing this override. That way both the MCP and the command can change the directory and also select the test (which is what's working right now).
Would you be open to making that update?
|
I'm not sure I follow. I thought the issue was the MCP command didn't use the directory parameter correctly, so how would adding (another) option to the test command fix this? |
marcossevilla
left a comment
marcossevilla
left a comment
There was a problem hiding this comment.
I'm good to approve/merge this, thoughts @ryzizub ?
|
Yeah @marcossevilla, I'm good too. I'll do a couple of tests, but I think it's great :) @jmadren just for context, the questions earlier were about the idea of adding a --directory option to the test command. But we agreed internally that your PR is good and less invasive ❤️ |
marcossevilla
changed the title
directory argument as the working directorydirectory argument as the working directory
3 participants
Description
The
directoryargument on thetest,packages_get, andpackages_check_licensesMCP tools was appended as a positional CLI argument. Those commands resolve their target package fromDirectory.currentand treat positionals as test/target paths, so the value never changed where the command ran. In a monorepo the tools therefore executed from the MCP server's current directory (commonly a folder with nopubspec.yaml) and failed with exit code 66, regardless of whether a relative or absolutedirectorywas supplied.Closes #1599
Type of Change
Fix
Apply
directoryby switchingDirectory.currentfor the duration of the run and restoring it in afinallyafterwards. Relative paths resolve against the server's current directory, matching the intent of thepath/directoryparameter from VeryGoodOpenSource/vgv-ai-flutter-plugin#70. A non-existent directory now surfaces a clear tool error instead of a confusing exit code.Refs VeryGoodOpenSource/vgv-ai-flutter-plugin#70