[TEST] Combined eval troubleshooting guide + MCP C# skills - #442
Closed
danmoseley wants to merge 41 commits into
Closed
[TEST] Combined eval troubleshooting guide + MCP C# skills#442danmoseley wants to merge 41 commits into
danmoseley wants to merge 41 commits into
Conversation
Four new skills for the C# MCP server development lifecycle: - mcp-csharp-create: Scaffolding with dotnet new mcpserver, tools/prompts/resources, transport config - mcp-csharp-debug: MCP Inspector, VS Code integration, breakpoint debugging, logging - mcp-csharp-test: Unit tests with ClientServerTestBase, integration with WebApplicationFactory, evals - mcp-csharp-publish: NuGet packaging, Docker/Azure deployment, MCP Registry publishing Each skill includes SKILL.md with progressive disclosure references/ and eval.yaml tests.
…te syntax Replace scaffolding-heavy scenarios with implementation-focused ones that test MCP-specific features (resources, prompts, logging). Fix assertion patterns to match combined C# attribute syntax [McpServerTool, Description()] instead of requiring standalone [McpServerTool]. Increase timeouts to 180s to account for skill-reading overhead. Validator result: passed=True, improvement=44.6% (threshold=10%)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…aging.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Cross-skill references used ../sibling/SKILL.md paths which the validator rejects. Replace with backtick-quoted skill names to match the convention used by other skills in the repo.
Add domains referenced by the mcp-csharp-* skills: - github.com/modelcontextprotocol/registry - github.com/open-telemetry/semantic-conventions - npmjs.com, xunit.net, fluentassertions.com, nugettest.org Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
The URL regex truncates at angle brackets (invalid in URIs), so placeholder URLs like https://github.com/<yourusername>/<repo> become https://github.com/ which didn't match any path-scoped entry. Adding github.com/ (with trailing slash) covers only this case without broadly allowing all github.com URLs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
McpServerToolType is only needed with WithToolsFromAssembly(), which is discouraged because it is not Native AOT compatible. Updated eval assertions and rubrics to prefer WithTools<T>() instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Increase mcp-csharp-create timeouts from 180s (or default 120s) to 360s for all 3 scenarios. These scenarios consistently time out because the model spends time on bash exploration/scaffolding before writing code. - Add explicit timeout to scenario 3 which was relying on the 120s default. - Broaden mcp-csharp-debug Inspector rubric to accept both the concise single CLI command style and the step-by-step UI configuration walkthrough, since both are correct approaches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Add InvestigatingResults.md with: - How to find and download evaluation artifacts from workflow runs - Full results.json schema documentation - Common failure patterns (timeouts, empty output, variance, rubric issues, skill not activated) with diagnosis steps and fixes - Example analysis script for AI agents to parse results Link the doc from: - Reporter.cs: show a link in the PR comment when any skill fails - skill-validator README.md: Output section - CONTRIBUTING.md: CI evaluation section Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…oaches The rubric criterion 'Shows how to attach a debugger' was too narrow. The skilled answer correctly focused on the #1 cause (stdout pollution) but scored low because it didn't show a specific 'attach to process' flow. Broadened to accept any valid debugging approach: attaching, Debugger.Launch(), or launch.json configuration. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…to danmose/eval-combined-test
danmoseley
requested review from
JanKrivanek,
ViktorHofer,
dbreshears and
timheuer
as code owners
March 25, 2026 03:15
Contributor
|
Note This PR is from a fork and modifies infrastructure files ( Changes to infrastructure typically need to be submitted from a branch in Please consider recreating this PR from an upstream branch. If you don't have push access to |
danmoseley
added
the
DO NOT MERGE
Contributor
Author
|
/evaluate |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR combines two efforts: (1) adding an evaluation troubleshooting guide and surfacing it in CI/markdown summaries, and (2) adding MCP C# skill documentation plus associated eval scenarios under tests/dotnet-ai/.
Changes:
- Adds an evaluation results troubleshooting guide (
InvestigatingResults.md) and links/prompts to it from validator output and the evaluation workflow. - Introduces MCP C# skill docs (
mcp-csharp-create/debug/test/publish) with supporting reference docs. - Adds new eval scenarios for the MCP C# skills and updates CODEOWNERS/known-domains to support the new documentation.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/dotnet-ai/mcp-csharp-test/eval.yaml | Adds eval scenarios for MCP server testing guidance. |
| tests/dotnet-ai/mcp-csharp-publish/eval.yaml | Adds eval scenarios for MCP server packaging/deployment/publishing. |
| tests/dotnet-ai/mcp-csharp-debug/eval.yaml | Adds eval scenarios for MCP server debugging and IDE/Inspector setup. |
| tests/dotnet-ai/mcp-csharp-create/eval.yaml | Adds eval scenarios for creating MCP servers and implementing tools/prompts/resources. |
| plugins/dotnet-ai/skills/mcp-csharp-test/references/test-patterns.md | Adds reference patterns for unit/integration testing MCP servers. |
| plugins/dotnet-ai/skills/mcp-csharp-test/SKILL.md | Adds the MCP C# testing skill content. |
| plugins/dotnet-ai/skills/mcp-csharp-publish/references/nuget-packaging.md | Adds detailed NuGet tool packaging guidance for MCP servers. |
| plugins/dotnet-ai/skills/mcp-csharp-publish/references/mcp-registry.md | Adds MCP Registry publishing guidance and server.json schema notes. |
| plugins/dotnet-ai/skills/mcp-csharp-publish/references/docker-azure.md | Adds Docker/Azure deployment guidance for HTTP MCP servers. |
| plugins/dotnet-ai/skills/mcp-csharp-publish/SKILL.md | Adds the MCP C# publishing/deployment skill content. |
| plugins/dotnet-ai/skills/mcp-csharp-debug/references/mcp-inspector.md | Adds MCP Inspector usage and troubleshooting reference. |
| plugins/dotnet-ai/skills/mcp-csharp-debug/references/ide-config.md | Adds VS Code/Visual Studio config and debugging reference. |
| plugins/dotnet-ai/skills/mcp-csharp-debug/SKILL.md | Adds the MCP C# debugging skill content. |
| plugins/dotnet-ai/skills/mcp-csharp-create/references/transport-config.md | Adds stdio/HTTP transport configuration reference. |
| plugins/dotnet-ai/skills/mcp-csharp-create/references/api-patterns.md | Adds C# MCP SDK API/attribute/DI patterns reference. |
| plugins/dotnet-ai/skills/mcp-csharp-create/SKILL.md | Adds the MCP C# server creation skill content. |
| eng/skill-validator/src/Evaluate/Reporter.cs | Adds a troubleshooting-guide link to markdown summaries when failures occur. |
| eng/skill-validator/README.md | Links to the new troubleshooting guide from validator docs. |
| eng/skill-validator/InvestigatingResults.md | Adds the new evaluation troubleshooting guide and results.json schema/patterns. |
| eng/known-domains.txt | Expands allowlist for new documentation links and adjusts inline format guidance. |
| CONTRIBUTING.md | Links to troubleshooting guide from contributing/evaluation section. |
| .github/workflows/evaluation.yml | Appends a copy/paste investigation prompt to PR comments when failures occur. |
| .github/CODEOWNERS | Adds ownership entries for the new MCP C# skill/test paths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| stdio JSON-RPC protocol. | ||
| assertions: | ||
| - type: "output_matches" | ||
| pattern: "\\[McpServerTool[\\]T,\\(]" |
There was a problem hiding this comment.
The output_matches regex looks malformed: \\[McpServerTool[\\]T,\\(] includes an unexpected T inside the character class, so it may match [McpServerToolT instead of the intended [McpServerTool]/[McpServerTool( patterns. This will make the assertion flaky or always-failing depending on the agent output.
Update the pattern to match the attribute name consistently (e.g., mirror the earlier scenario’s pattern) and ensure it matches the intended bracket/parenthesis forms.
Suggested change
| pattern: "\\[McpServerTool[\\]T,\\(]" | |
| pattern: "\\[McpServerTool[\\],\\(]" |
Comment on lines
+646
to
+652
| if grep -q '❌' summary-body.md; then | ||
| RUN_ID="${{ github.run_id }}" | ||
| echo "" | ||
| echo "> **To investigate failures**, paste this to your AI coding agent:" | ||
| echo ">" | ||
| echo "> _Download eval artifacts with \`gh run download ${RUN_ID} --repo ${{ github.repository }} --dir /tmp/eval-results\`, then fetch https://raw.githubusercontent.com/${{ github.repository }}/main/eng/skill-validator/InvestigatingResults.md and follow it to analyze the results.json files. Diagnose each failure, suggest fixes to the eval.yaml and skill content, and tell me what to fix first._" | ||
| fi |
There was a problem hiding this comment.
The investigation prompt fetches InvestigatingResults.md from the main branch. In PRs where the guide is being introduced/updated (including this one), that URL can 404 or be out of sync with the evaluator output, making the prompt ineffective.
Consider fetching the guide at the evaluated commit SHA (e.g., ${{ needs.gate.outputs.head_sha }} or ${{ github.sha }}) so the link always matches the run’s content.
Comment on lines
+56
to
+57
| github.com/yourusername | ||
| github.com/username |
There was a problem hiding this comment.
Allowlisting github.com/yourusername and github.com/username weakens reference validation because these are real, potentially user-controlled GitHub accounts. This effectively permits arbitrary external content under those owners without explicit review.
Safer alternatives: update the docs to use non-resolvable placeholders (so they aren’t detected as URLs), or use an org/repo that’s already allowlisted (or a GitHub-owned example org) and remove these placeholder allowlist entries.
Suggested change
| github.com/yourusername | |
| github.com/username | |
| # Use non-resolvable placeholders in docs instead of real GitHub usernames. |
|
|
||
| bool anyFailure = verdicts.Any(v => !v.Passed); | ||
| if (anyFailure) | ||
| sb.AppendLine("\n> 📖 See [InvestigatingResults.md](https://github.com/dotnet/skills/blob/main/eng/skill-validator/InvestigatingResults.md) for how to diagnose failures — or use the copy-paste prompt below."); |
There was a problem hiding this comment.
This new line says “or use the copy-paste prompt below,” but summary.md generated by skill-validator doesn’t include any prompt text—only the GitHub Actions workflow appends it. This makes local runs or artifact-only reads confusing.
Suggest rewording to avoid referencing a prompt below (or conditionally emit this only in the PR comment generation path).
Suggested change
| sb.AppendLine("\n> 📖 See [InvestigatingResults.md](https://github.com/dotnet/skills/blob/main/eng/skill-validator/InvestigatingResults.md) for how to diagnose failures — or use the copy-paste prompt below."); | |
| sb.AppendLine("\n> 📖 See [InvestigatingResults.md](https://github.com/dotnet/skills/blob/main/eng/skill-validator/InvestigatingResults.md) for how to diagnose failures."); |
Contributor
Skill Validation Results
[1] (Isolated) Quality unchanged but weighted score is -4.5% due to: judgment
Model: claude-opus-4.6 | Judge: claude-opus-4.6 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Throwaway PR to test evaluation troubleshooting guide (#441) with MCP C# skills (#317).
Combines both PRs to see if the troubleshooting output is useful when there are real eval failures.
This PR should be closed after testing -- do not merge.