[TEST] Combined eval troubleshooting guide + MCP C# skills - #443
Closed
danmoseley wants to merge 43 commits into
Closed
[TEST] Combined eval troubleshooting guide + MCP C# skills#443danmoseley wants to merge 43 commits into
danmoseley wants to merge 43 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:17
Contributor
Author
|
/evaluate |
danmoseley
added
the
DO NOT MERGE
- Use skillActivationIsolated/skillActivationPlugin (not generic skillActivation) - Clarify timestamp is when results were written, not run start Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Combines an evaluation troubleshooting guide with new MCP C# skills and their eval scenarios, and wires CI reporting to provide investigation guidance when evals fail.
Changes:
- Add MCP C# skills (create/debug/test/publish) reference content plus new eval scenarios under
tests/dotnet-ai/. - Add
InvestigatingResults.mdand link it from skill-validator outputs, README, and CONTRIBUTING. - Update the evaluation workflow to append a copy/paste investigation prompt and expand
known-domains.txt/CODEOWNERS.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/dotnet-ai/mcp-csharp-create/eval.yaml | New eval scenarios for MCP server creation guidance. |
| tests/dotnet-ai/mcp-csharp-debug/eval.yaml | New eval scenarios for MCP server debugging/IDE/Inspector guidance. |
| tests/dotnet-ai/mcp-csharp-test/eval.yaml | New eval scenarios for unit/integration testing guidance. |
| tests/dotnet-ai/mcp-csharp-publish/eval.yaml | New eval scenarios for NuGet/Docker/Azure/Registry publishing guidance. |
| plugins/dotnet-ai/skills/mcp-csharp-create/SKILL.md | New skill content for creating MCP servers in C#. |
| plugins/dotnet-ai/skills/mcp-csharp-create/references/api-patterns.md | Reference doc for C# MCP SDK attributes/DI/return types/builder API. |
| plugins/dotnet-ai/skills/mcp-csharp-create/references/transport-config.md | Reference doc for stdio/HTTP transport config, auth, observability. |
| plugins/dotnet-ai/skills/mcp-csharp-debug/SKILL.md | New skill content for running/debugging MCP servers. |
| plugins/dotnet-ai/skills/mcp-csharp-debug/references/ide-config.md | Reference doc for VS Code/VS configuration (mcp.json/launch.json). |
| plugins/dotnet-ai/skills/mcp-csharp-debug/references/mcp-inspector.md | Reference doc for MCP Inspector usage/troubleshooting. |
| plugins/dotnet-ai/skills/mcp-csharp-test/SKILL.md | New skill content for testing MCP servers. |
| plugins/dotnet-ai/skills/mcp-csharp-test/references/test-patterns.md | Reference code patterns for mocking/in-memory/HTTP testing. |
| plugins/dotnet-ai/skills/mcp-csharp-publish/SKILL.md | New skill content for packaging/deployment/registry publishing. |
| plugins/dotnet-ai/skills/mcp-csharp-publish/references/nuget-packaging.md | Reference doc for NuGet tool packaging + server.json example. |
| plugins/dotnet-ai/skills/mcp-csharp-publish/references/docker-azure.md | Reference doc for Dockerfile + Azure deployment patterns. |
| plugins/dotnet-ai/skills/mcp-csharp-publish/references/mcp-registry.md | Reference doc for MCP Registry publishing workflows. |
| eng/skill-validator/InvestigatingResults.md | New troubleshooting guide describing results.json + failure patterns. |
| eng/skill-validator/README.md | Adds link to troubleshooting guide from output docs. |
| CONTRIBUTING.md | Adds link to troubleshooting guide in CI evaluation section. |
| eng/skill-validator/src/Evaluate/Reporter.cs | Appends troubleshooting link in generated markdown summaries on failures. |
| .github/workflows/evaluation.yml | Adds “copy/paste to AI agent” prompt to PR comment when failures detected. |
| eng/known-domains.txt | Expands allowlist and documents matching behavior. |
| .github/CODEOWNERS | Adds ownership entries for new skills/tests and an agent entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+56
to
+60
| await using var client = await CreateMcpClientForServer(); | ||
| var result = await client.CallToolAsync("my_tool", | ||
| new() { ["input"] = "test" }, | ||
| cancellationToken: TestContext.Current.CancellationToken); | ||
| Assert.NotNull(result); |
There was a problem hiding this comment.
This sample uses TestContext.Current.CancellationToken, which is not part of standard xUnit test projects and will be unfamiliar/unavailable to most users. Prefer CancellationToken.None (or omit the parameter) in documentation samples, or show how to pass a cancellation token via the test’s own setup if you want to demonstrate cancellation.
Comment on lines
+645
to
+646
| # If any skill failed, add a copy-paste prompt for AI-assisted investigation | ||
| if grep -q '❌' summary-body.md; then |
There was a problem hiding this comment.
The failure detection here keys off the presence of the '❌' glyph in the consolidated markdown. GenerateMarkdownSummary emits '❌' for any scenario with a negative improvement score, which can happen even when a skill passes overall (since pass/fail is based on average improvement vs --min-improvement). Consider detecting failures by inspecting the JSON results (e.g., any .verdicts[].passed == false) instead of grepping the markdown output.
Suggested change
| # If any skill failed, add a copy-paste prompt for AI-assisted investigation | |
| if grep -q '❌' summary-body.md; then | |
| # If any skill actually failed (based on JSON verdicts), add a copy-paste prompt for AI-assisted investigation | |
| if jq -e 'any(.verdicts[]?; .passed == false)' $JSON_FILES > /dev/null 2>&1; then |
Comment on lines
+612
to
+615
| 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.
The generated markdown links to InvestigatingResults.md on the main branch. During PR evaluation runs (before merge), that link can 404 or show stale content. Consider resolving the doc URL against the current commit when running in CI (e.g., using GITHUB_SHA/GITHUB_REPOSITORY env vars) and falling back to main for local runs.
| 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._" |
There was a problem hiding this comment.
The investigation prompt fetches InvestigatingResults.md from /main, which can be missing/outdated for the workflow run being diagnosed (especially within the PR that introduces the file). Use the workflow’s exact ref (e.g., ${{ github.sha }}) so the prompt always downloads the guide that matches the evaluated commit.
Suggested change
| 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._" | |
| 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 }}/${{ github.sha }}/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._" |
| # | ||
| # Format: one domain per line. Lines starting with # are comments. | ||
| # For GitHub, use github.com/<org>/<repo> to scope to a specific repo. | ||
| # Entries containing a slash match URLs with that exact prefix followed by |
There was a problem hiding this comment.
This comment describes path-scoped entries as matching the exact prefix, but the implementation also allows an optional www. prefix for slash entries. Please update the text to mention the optional www. so the file accurately documents matching behavior.
Suggested change
| # Entries containing a slash match URLs with that exact prefix followed by | |
| # Entries containing a slash match URLs with that prefix (optionally preceded by 'www.') followed by |
Comment on lines
+25
to
+26
| /plugins/dotnet/agents/optimizing-dotnet-performance.agent.md @dotnet/appmodel | ||
|
|
There was a problem hiding this comment.
This CODEOWNERS entry points to /plugins/dotnet/agents/optimizing-dotnet-performance.agent.md, but that path doesn’t exist in the repo (the agent file is under /plugins/dotnet-diag/agents/). As written it’s redundant (same entry already exists later) and can confuse ownership expectations—please remove or correct it.
Suggested change
| /plugins/dotnet/agents/optimizing-dotnet-performance.agent.md @dotnet/appmodel |
Comment on lines
+130
to
+134
| ENV ASPNETCORE_URLS=http://+:8080 | ||
| EXPOSE 8080 | ||
| HEALTHCHECK --interval=30s --timeout=3s --start-period=10s --retries=3 \ | ||
| CMD curl -f http://localhost:8080/health || exit 1 | ||
| ENTRYPOINT ["dotnet", "MyMcpServer.dll"] |
There was a problem hiding this comment.
The Dockerfile example uses curl in the HEALTHCHECK, but the mcr.microsoft.com/dotnet/aspnet runtime images don’t reliably include curl. Either install curl in the runtime stage or use a healthcheck approach that doesn’t depend on extra tools, otherwise users may copy this and end up with failing health checks.
Comment on lines
+33
to
+36
| # Health check | ||
| HEALTHCHECK --interval=30s --timeout=3s --start-period=10s --retries=3 \ | ||
| CMD curl -f http://localhost:8080/health || exit 1 | ||
|
|
There was a problem hiding this comment.
Same as in the SKILL.md example: this HEALTHCHECK relies on curl, which may not exist in the dotnet/aspnet runtime image. Please adjust the guidance to either install curl (and clean up package lists) or use an alternative healthcheck mechanism.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Skill Validation Results
[1] (Plugin) Quality unchanged but weighted score is -6.6% due to: tokens (11832 → 54340), tool calls (0 → 3), time (11.9s → 23.0s)
Model: claude-opus-4.6 | Judge: claude-opus-4.6
|
Contributor
Author
|
Closing — eval runs are expensive and the troubleshooting output changes only take effect after merge anyway. |
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.