Skip to content

Conversation

@halter73
Copy link
Contributor

@halter73 halter73 commented Nov 21, 2025

This reverts commit 7080e3a.

With the update to MTP, dotnet test --filter no longer works which throws off AI agents. Instead you have to use something like --filter-class or --filter-query which takes time for agents to figure out. We could put something in AGENTS.md, but I'd rather have it work without special instructions. @mikekistler also reported problems when using agents with the new testing dependencies.

This does not revert all the changes in #990, since we might want the configuration changes the next time we update MTP, and it seems like it won't hurt in the meantime.

cc @Youssef1313

@stephentoub
Copy link
Contributor

You'll need to update the analyzers test project as well:

/home/runner/work/csharp-sdk/csharp-sdk/tests/ModelContextProtocol.Analyzers.Tests/ModelContextProtocol.Analyzers.Tests.csproj : error NU1010: The following PackageReference items do not define a corresponding PackageVersion item: Microsoft.Testing.Extensions.CodeCoverage, Microsoft.Testing.Extensions.HangDump, Microsoft.Testing.Extensions.CrashDump, Microsoft.Testing.Extensions.TrxReport, xunit.v3.mtp-v2. Projects using Central Package Management must declare PackageReference and PackageVersion items with matching names. For more information, visit https://aka.ms/nuget/cpm/gettingstarted [/home/runner/work/csharp-sdk/csharp-sdk/ModelContextProtocol.slnx]

- I left it in dependabot.yml config in case it gets added back as was done in #990
@Youssef1313
Copy link
Contributor

I'm honestly more in favor of updating AI instructions to understand this rather than reverting.
MTP is what we are pushing forward.

cc @Evangelink @nohwnd

@stephentoub
Copy link
Contributor

stephentoub commented Nov 21, 2025

I'm honestly more in favor of updating AI instructions to understand this rather than reverting. MTP is what we are pushing forward.

We can do that concurrently and then revert the revert the revert the revert.

We need folks to be productive working in the repo right now, especially with the push around the new MCP spec that's dropping next week, and we can't be blocked on desirable-but-unnecessary infrastructural changes. Changes that block copilot from helping with our velocity aren't viable right now.

@Youssef1313
Copy link
Contributor

@stephentoub My point is, isn't it easy enough to add the custom instructions? and can be already fast enough to be done without going into the churn of reverting and re-reverting back?

@Youssef1313
Copy link
Contributor

I opened #1012 for Copilot instructions.

@stephentoub
Copy link
Contributor

stephentoub commented Nov 21, 2025

My point is, isn't it easy enough to add the custom instructions? and can be already fast enough to be done without going into the churn of reverting and re-reverting back?

You would need to fully validate the end-to-end, ensuring that everything works locally with copilot CLI, that requests work to copilot with CCA in the repo, etc. We need to unblock the folks this evening that are currently struggling to make forward progress in their normal workflows.

We can't keep merging, reverting, merging, reverting. It needs to be fully correct the next time. Both times it's merged thus far it's almost immediately broken members of the team.

@halter73
Copy link
Contributor Author

My point is, isn't it easy enough to add the custom instructions? and can be already fast enough to be done without going into the churn of reverting and re-reverting back?

It's more than just teaching the agent to use --filter-query instead of --filer. When I try to specify a specific test project to run from the root of the repo, I get errors like the following:

$ cd /workspaces/csharp-sdk && dotnet test tests/ModelContextProtocol.Tests/ModelContextProtocol.Tests.csproj -f net8.0 -- --filter-not-trait Execution=Manual
Specifying a project for 'dotnet test' should be via '--project'.

This even throws me off. I understand that I need to put --project before tests/ModelContextProtocol.Tests/ModelContextProtocol.Tests.cspro, but why are we making all these breaking changes to the CLI interface? This makes it difficult for AI agents and humans alike.

@halter73 halter73 merged commit 707c083 into main Nov 21, 2025
8 of 9 checks passed
@halter73 halter73 deleted the halter73/revert-mtp branch November 21, 2025 22:53
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.

4 participants