Skip to content

Commit 8e3629c

Browse files
BenjaminMichaelisCopilot
BenjaminMichaelis
and
Copilot
committed
Fix PR #876 review comments
- Issue 1: Replace anonymous rate-limit body with typed McpRateLimitErrorEnvelope records; use JsonSerializer.SerializeAsync instead of WriteAsync(Serialize(...)) - Issue 2: Strip prose default values from all tool Description attributes - Issue 3: Merge GetGuidelinesByTopic into GetCSharpGuidelines via optional 'topic' param; delete redundant GetGuidelinesByTopic tool - Issue 4: Make /mcp-setup page dynamic — inject IEnumerable<McpServerTool> into McpSetupController; replace 330+ lines of static HTML cards with Razor @foreach loop - Issue 5: Add listing search pattern minimum validation (>=2 alphanumeric chars or recognized C# operator) - Issues 6+7: Extract duplicated FormatGuidelineType/FormatType to shared GuidelineTypeExtensions.ToDisplayString() extension method; delete private copies Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 19c6015 commit 8e3629c

8 files changed

Lines changed: 144 additions & 426 deletions

File tree

‎EssentialCSharp.Web/Controllers/McpSetupController.cs‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,55 @@
11
using EssentialCSharp.Web.Services;
22
using Microsoft.AspNetCore.Authorization;
33
using Microsoft.AspNetCore.Mvc;
4+
using ModelContextProtocol.Server;
5+
using System.Text.Json;
46

57
namespace EssentialCSharp.Web.Controllers;
68

79
[AllowAnonymous]
810
public class McpSetupController : BaseController
911
{
10-
public McpSetupController(IRouteConfigurationService routeConfigurationService, IHttpContextAccessor httpContextAccessor)
12+
private readonly IEnumerable<McpServerTool> _tools;
13+
14+
public McpSetupController(IRouteConfigurationService routeConfigurationService, IHttpContextAccessor httpContextAccessor, IEnumerable<McpServerTool> tools)
1115
: base(routeConfigurationService, httpContextAccessor)
1216
{
17+
_tools = tools;
1318
}
1419

1520
[Route("/mcp-setup")]
1621
public IActionResult Index()
1722
{
1823
ViewBag.PageTitle = "MCP Setup";
19-
return View();
24+
var toolInfos = _tools
25+
.OrderBy(t => t.ProtocolTool.Name)
26+
.Select(t =>
27+
{
28+
var parameters = new List<McpParamInfo>();
29+
if (t.ProtocolTool.InputSchema.ValueKind == JsonValueKind.Object
30+
&& t.ProtocolTool.InputSchema.TryGetProperty("properties", out JsonElement props)
31+
&& props.ValueKind == JsonValueKind.Object)
32+
{
33+
t.ProtocolTool.InputSchema.TryGetProperty("required", out JsonElement requiredEl);
34+
foreach (JsonProperty prop in props.EnumerateObject())
35+
{
36+
string desc = prop.Value.TryGetProperty("description", out JsonElement d) ? d.GetString() ?? "" : "";
37+
bool required = requiredEl.ValueKind == JsonValueKind.Array
38+
&& requiredEl.EnumerateArray().Any(r => r.GetString() == prop.Name);
39+
parameters.Add(new McpParamInfo(prop.Name, desc, required));
40+
}
41+
}
42+
return new McpToolInfo(
43+
t.ProtocolTool.Name ?? "",
44+
t.ProtocolTool.Title ?? t.ProtocolTool.Name ?? "",
45+
t.ProtocolTool.Description ?? "",
46+
parameters);
47+
})
48+
.ToList();
49+
50+
return View(toolInfos);
2051
}
2152
}
53+
54+
public sealed record McpToolInfo(string Name, string Title, string Description, IReadOnlyList<McpParamInfo> Parameters);
55+
public sealed record McpParamInfo(string Name, string Description, bool Required);

‎EssentialCSharp.Web/Extensions/GuidelineTypeExtensions.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
namespace EssentialCSharp.Web.Extensions;
2+
3+
public static class GuidelineTypeExtensions
4+
{
5+
public static string ToDisplayString(this GuidelineType type) => type switch
6+
{
7+
GuidelineType.Do => "DO",
8+
GuidelineType.Consider => "CONSIDER",
9+
GuidelineType.Avoid => "AVOID",
10+
GuidelineType.DoNot => "DO NOT",
11+
_ => "NOTE"
12+
};
13+
}

‎EssentialCSharp.Web/Program.cs‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -324,15 +324,12 @@ private static void Main(string[] args)
324324
if (context.HttpContext.Request.Path.StartsWithSegments("/mcp"))
325325
{
326326
context.HttpContext.Response.ContentType = "application/json";
327-
var mcpErrorResponse = new
328-
{
329-
jsonrpc = "2.0",
330-
error = new { code = -32000, message = "Rate limit exceeded. Please wait before sending another request." },
331-
id = (object?)null
332-
};
333-
await context.HttpContext.Response.WriteAsync(
334-
System.Text.Json.JsonSerializer.Serialize(mcpErrorResponse),
335-
cancellationToken);
327+
var mcpErrorResponse = new McpRateLimitErrorEnvelope("2.0", null,
328+
new McpRateLimitErrorDetail(-32000, "Rate limit exceeded. Please wait before sending another request."));
329+
await System.Text.Json.JsonSerializer.SerializeAsync(
330+
context.HttpContext.Response.Body,
331+
mcpErrorResponse,
332+
cancellationToken: cancellationToken);
336333
return;
337334
}
338335

@@ -522,4 +519,13 @@ await context.HttpContext.Response.WriteAsync(
522519

523520
app.Run();
524521
}
522+
523+
private sealed record McpRateLimitErrorEnvelope(
524+
[property: System.Text.Json.Serialization.JsonPropertyName("jsonrpc")] string JsonRpc,
525+
[property: System.Text.Json.Serialization.JsonPropertyName("id")] object? Id,
526+
[property: System.Text.Json.Serialization.JsonPropertyName("error")] McpRateLimitErrorDetail Error);
527+
528+
private sealed record McpRateLimitErrorDetail(
529+
[property: System.Text.Json.Serialization.JsonPropertyName("code")] int Code,
530+
[property: System.Text.Json.Serialization.JsonPropertyName("message")] string Message);
525531
}

‎EssentialCSharp.Web/Tools/BookContentTool.cs‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ public BookContentTool(
3737
Description("Retrieve the prose content of a specific book section identified by its slug/key (e.g., 'hello-world', 'creating-editing-compiling-and-running-c-source-code'). Returns the section text with code examples preserved. Use GetChapterSections to discover available slugs.")]
3838
public async Task<string> GetSectionContent(
3939
[Description("The section slug/key (e.g., 'hello-world'). Use GetChapterSections to get valid slugs.")] string sectionKey,
40-
[Description("Maximum number of characters to return (500–8000, default 4000). Long sections are truncated.")] int maxChars = 4000,
40+
[Description("Maximum number of characters to return (500–8000). Long sections are truncated.")] int maxChars = 4000,
4141
CancellationToken cancellationToken = default)
4242
{
4343
maxChars = Math.Clamp(maxChars, 500, 8000);
@@ -365,7 +365,7 @@ public string GetChapterSummary(
365365
sb.AppendLine("## Guidelines in this Chapter");
366366
foreach (var g in guidelines)
367367
{
368-
sb.AppendLine(CultureInfo.InvariantCulture, $"- **[{FormatGuidelineType(g.Type)}]** {g.Guideline}");
368+
sb.AppendLine(CultureInfo.InvariantCulture, $"- **[{g.Type.ToDisplayString()}]** {g.Guideline}");
369369
}
370370
}
371371

@@ -431,15 +431,6 @@ private static void ExtractNodeContent(HtmlNode node, StringBuilder sb)
431431
}
432432
}
433433

434-
private static string FormatGuidelineType(GuidelineType type) => type switch
435-
{
436-
GuidelineType.Do => "DO",
437-
GuidelineType.Consider => "CONSIDER",
438-
GuidelineType.Avoid => "AVOID",
439-
GuidelineType.DoNot => "DO NOT",
440-
_ => "NOTE"
441-
};
442-
443434
[GeneratedRegex(@"^[A-Za-z0-9_-]{1,128}$")]
444435
private static partial Regex AnchorIdRegex();
445436
}

‎EssentialCSharp.Web/Tools/BookGuidelinesTool.cs‎

Lines changed: 39 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System.ComponentModel;
22
using System.Globalization;
33
using System.Text;
4+
using EssentialCSharp.Web.Extensions;
45
using EssentialCSharp.Web.Services;
56
using ModelContextProtocol.Server;
67

@@ -17,12 +18,13 @@ public BookGuidelinesTool(IGuidelinesService guidelinesService)
1718
}
1819

1920
[McpServerTool(Title = "Get C# Guidelines", ReadOnly = true, Destructive = false, Idempotent = true, OpenWorld = false),
20-
Description("Retrieve C# coding guidelines from the Essential C# book. Optionally filter by keyword, chapter number, or guideline type (do/consider/avoid/donot). The book contains guidelines covering naming conventions, error handling, LINQ, async/await, generics, and many other topics. Each guideline includes its chapter and subsection context.")]
21+
Description("Retrieve C# coding guidelines from the Essential C# book. Filter by keyword (exact text match), chapter number, or guideline type. Use the 'topic' parameter for relevance-ranked discovery by concept (e.g., 'exception handling', 'naming', 'async'). Each guideline includes its chapter and subsection context. Tip: use 'topic' for broad discovery; use 'keyword' for precise text matching.")]
2122
public string GetCSharpGuidelines(
22-
[Description("Optional keyword to filter guidelines by (searched in guideline text and subsection name).")] string? keyword = null,
23+
[Description("Optional keyword for exact-text search in guideline text and subsection name.")] string? keyword = null,
2324
[Description("Optional chapter number to restrict results to a specific chapter.")] int? chapter = null,
2425
[Description("Optional guideline type: 'do', 'consider', 'avoid', or 'donot' (also accepts 'do not', 'dont').")] string? type = null,
25-
[Description("Maximum number of guidelines to return (1–50, default 20).")] int maxResults = 20)
26+
[Description("Optional topic or concept for relevance-ranked search (e.g., 'exception handling', 'naming', 'async'). Results are ordered by relevance. Use for broad discovery; use 'keyword' for exact text matching.")] string? topic = null,
27+
[Description("Maximum number of guidelines to return (1–50).")] int maxResults = 20)
2628
{
2729
maxResults = Math.Clamp(maxResults, 1, 50);
2830
GuidelineType? typeFilter = ParseGuidelineType(type);
@@ -46,66 +48,54 @@ public string GetCSharpGuidelines(
4648
g.SanitizedSubsection.Contains(keyword, StringComparison.OrdinalIgnoreCase) ||
4749
(g.ActualSubsection?.Contains(keyword, StringComparison.OrdinalIgnoreCase) == true));
4850

49-
var results = filtered.Take(maxResults).ToList();
50-
51-
if (results.Count == 0)
51+
if (!string.IsNullOrWhiteSpace(topic))
5252
{
53-
return "No guidelines found matching the specified filters.";
54-
}
55-
56-
var sb = new StringBuilder();
57-
sb.AppendLine(CultureInfo.InvariantCulture, $"# Essential C# Guidelines ({results.Count} result{(results.Count == 1 ? "" : "s")})");
58-
sb.AppendLine();
53+
var words = topic.Split(' ', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);
54+
var scored = filtered
55+
.Select(g =>
56+
{
57+
string combined = $"{g.Guideline} {g.SanitizedSubsection} {g.ActualSubsection} {g.ChapterTitle}";
58+
int score = words.Count(w => combined.Contains(w, StringComparison.OrdinalIgnoreCase));
59+
return (guideline: g, score);
60+
})
61+
.Where(x => x.score > 0)
62+
.OrderByDescending(x => x.score)
63+
.Take(maxResults)
64+
.ToList();
65+
66+
if (scored.Count == 0)
67+
{
68+
return $"No guidelines found related to '{topic}'.";
69+
}
5970

60-
foreach (var g in results)
61-
{
62-
sb.AppendLine(CultureInfo.InvariantCulture, $"**[{FormatType(g.Type)}]** {g.Guideline}");
63-
sb.AppendLine(CultureInfo.InvariantCulture, $" — Chapter {g.ChapterNumber}: {g.ChapterTitle} / {g.SanitizedSubsection}");
64-
sb.AppendLine();
65-
}
71+
var topicSb = new StringBuilder();
72+
topicSb.AppendLine(CultureInfo.InvariantCulture, $"# Essential C# Guidelines — Topic: {topic} ({scored.Count} result{(scored.Count == 1 ? "" : "s")})");
73+
topicSb.AppendLine();
6674

67-
return sb.ToString();
68-
}
75+
foreach (var (g, _) in scored)
76+
{
77+
topicSb.AppendLine(CultureInfo.InvariantCulture, $"**[{g.Type.ToDisplayString()}]** {g.Guideline}");
78+
topicSb.AppendLine(CultureInfo.InvariantCulture, $" — Chapter {g.ChapterNumber}: {g.ChapterTitle} / {g.SanitizedSubsection}");
79+
topicSb.AppendLine();
80+
}
6981

70-
[McpServerTool(Title = "Get Guidelines By Topic", ReadOnly = true, Destructive = false, Idempotent = true, OpenWorld = false),
71-
Description("Search C# coding guidelines from the Essential C# book by topic or concept. More discoverable than filtering by chapter — finds all guidelines related to exceptions, naming, async, LINQ, generics, interfaces, and more. Results are ordered by relevance to the topic.")]
72-
public string GetGuidelinesByTopic(
73-
[Description("The topic or concept to search guidelines for (e.g., 'exception handling', 'naming', 'async', 'LINQ', 'generics', 'interface').")] string topic,
74-
[Description("Maximum number of guidelines to return (1–30, default 15).")] int maxResults = 15)
75-
{
76-
if (string.IsNullOrWhiteSpace(topic))
77-
{
78-
return "Topic must not be empty.";
82+
return topicSb.ToString();
7983
}
8084

81-
maxResults = Math.Clamp(maxResults, 1, 30);
82-
83-
var words = topic.Split(' ', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries);
85+
var results = filtered.Take(maxResults).ToList();
8486

85-
var scored = _guidelinesService.Guidelines
86-
.Select(g =>
87-
{
88-
string combined = $"{g.Guideline} {g.SanitizedSubsection} {g.ActualSubsection} {g.ChapterTitle}";
89-
int score = words.Count(w => combined.Contains(w, StringComparison.OrdinalIgnoreCase));
90-
return (guideline: g, score);
91-
})
92-
.Where(x => x.score > 0)
93-
.OrderByDescending(x => x.score)
94-
.Take(maxResults)
95-
.ToList();
96-
97-
if (scored.Count == 0)
87+
if (results.Count == 0)
9888
{
99-
return $"No guidelines found related to '{topic}'.";
89+
return "No guidelines found matching the specified filters.";
10090
}
10191

10292
var sb = new StringBuilder();
103-
sb.AppendLine(CultureInfo.InvariantCulture, $"# Essential C# Guidelines — Topic: {topic} ({scored.Count} result{(scored.Count == 1 ? "" : "s")})");
93+
sb.AppendLine(CultureInfo.InvariantCulture, $"# Essential C# Guidelines ({results.Count} result{(results.Count == 1 ? "" : "s")})");
10494
sb.AppendLine();
10595

106-
foreach (var (g, _) in scored)
96+
foreach (var g in results)
10797
{
108-
sb.AppendLine(CultureInfo.InvariantCulture, $"**[{FormatType(g.Type)}]** {g.Guideline}");
98+
sb.AppendLine(CultureInfo.InvariantCulture, $"**[{g.Type.ToDisplayString()}]** {g.Guideline}");
10999
sb.AppendLine(CultureInfo.InvariantCulture, $" — Chapter {g.ChapterNumber}: {g.ChapterTitle} / {g.SanitizedSubsection}");
110100
sb.AppendLine();
111101
}
@@ -126,13 +116,4 @@ public string GetGuidelinesByTopic(
126116
_ => null
127117
};
128118
}
129-
130-
private static string FormatType(GuidelineType type) => type switch
131-
{
132-
GuidelineType.Do => "DO",
133-
GuidelineType.Consider => "CONSIDER",
134-
GuidelineType.Avoid => "AVOID",
135-
GuidelineType.DoNot => "DO NOT",
136-
_ => "NOTE"
137-
};
138119
}

‎EssentialCSharp.Web/Tools/BookListingTool.cs‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,21 @@ public async Task<string> GetListingSourceCode(
3939
Description("Search all code listings in the Essential C# book for a specific code pattern, keyword, or identifier. Searches actual C# source code (not prose). Useful for finding examples of Task.WhenAll, yield return, IDisposable, pattern matching, and similar code constructs.")]
4040
public async Task<string> SearchListingsByCode(
4141
[Description("The code pattern or keyword to search for in listing source code (case-insensitive substring match).")] string pattern,
42-
[Description("Maximum number of matching listings to return (1–20, default 10).")] int maxResults = 10,
42+
[Description("Maximum number of matching listings to return (1–20).")] int maxResults = 10,
4343
CancellationToken cancellationToken = default)
4444
{
4545
if (string.IsNullOrWhiteSpace(pattern))
4646
{
4747
return "Pattern must not be empty.";
4848
}
4949

50+
string trimmedPattern = pattern.Trim();
51+
bool isKnownOperator = trimmedPattern is "=>" or "??" or "?." or "::" or "??=" or "==" or "!=" or "<=" or ">=" or "&&" or "||";
52+
if (!isKnownOperator && trimmedPattern.Count(char.IsLetterOrDigit) < 2)
53+
{
54+
return "Pattern must contain at least two letters or digits, or be a recognized C# operator (=>, ??, ?., ::, ??=, ==, !=, <=, >=, &&, ||).";
55+
}
56+
5057
maxResults = Math.Clamp(maxResults, 1, 20);
5158

5259
var distinctChapters = _siteMappingService.SiteMappings

‎EssentialCSharp.Web/Tools/BookSearchTool.cs‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ public BookSearchTool(IServiceProvider serviceProvider, ISiteMappingService site
3232
Description("Search the Essential C# book content using semantic vector search. Returns relevant text chunks with chapter and heading context. Use this to find information about C# programming concepts covered in the book.")]
3333
public async Task<string> SearchBookContent(
3434
[Description("The search query describing the C# concept or topic to find in the book.")] string query,
35-
[Description("Number of results to return (1–10). Default is 5. Use a higher value for broad topics or comprehensive research; lower for quick lookups.")] int maxResults = AISearchService.DefaultSearchTop,
35+
[Description("Number of results to return (1–10). Use a higher value for broad topics or comprehensive research; lower for quick lookups.")] int maxResults = AISearchService.DefaultSearchTop,
3636
CancellationToken cancellationToken = default)
3737
{
3838
if (string.IsNullOrWhiteSpace(query))
@@ -173,7 +173,7 @@ public string GetDirectContentUrl(
173173
Description("Find all sections in the Essential C# book that cover a specific C# concept. Combines section heading search with semantic vector search (when available) to give broad coverage. Returns section slugs, chapter numbers, and direct links.")]
174174
public async Task<string> LookupConcept(
175175
[Description("The C# concept, feature, or topic to find in the book (e.g., 'LINQ', 'async/await', 'pattern matching', 'generics').")] string concept,
176-
[Description("Number of semantic search results to return (1–10). Default is 5.")] int maxResults = AISearchService.DefaultSearchTop,
176+
[Description("Number of semantic search results to return (1–10).")] int maxResults = AISearchService.DefaultSearchTop,
177177
CancellationToken cancellationToken = default)
178178
{
179179
if (string.IsNullOrWhiteSpace(concept))
@@ -404,7 +404,7 @@ public async Task<string> FindBookHelpForDiagnostic(
404404
sb.AppendLine("## Related Guidelines");
405405
foreach (var g in guidelineMatches)
406406
{
407-
sb.AppendLine(CultureInfo.InvariantCulture, $"**[{FormatGuidelineType(g.Type)}]** {g.Guideline}");
407+
sb.AppendLine(CultureInfo.InvariantCulture, $"**[{g.Type.ToDisplayString()}]** {g.Guideline}");
408408
sb.AppendLine(CultureInfo.InvariantCulture, $" — Chapter {g.ChapterNumber}: {g.ChapterTitle} / {g.SanitizedSubsection}");
409409
sb.AppendLine();
410410
}
@@ -417,7 +417,7 @@ public async Task<string> FindBookHelpForDiagnostic(
417417
Description("Find other sections in the Essential C# book that are semantically related to a given section. Uses the section heading as a search query to discover thematically connected content across the entire book. Requires AI services to be configured.")]
418418
public async Task<string> FindRelatedSections(
419419
[Description("The section slug/key to find related content for (e.g., 'async-await'). Use GetChapterSections to get valid slugs.")] string sectionKey,
420-
[Description("Number of related sections to return (1–10). Default is 5.")] int maxResults = AISearchService.DefaultSearchTop,
420+
[Description("Number of related sections to return (1–10).")] int maxResults = AISearchService.DefaultSearchTop,
421421
CancellationToken cancellationToken = default)
422422
{
423423
if (string.IsNullOrWhiteSpace(sectionKey))
@@ -474,16 +474,7 @@ public async Task<string> FindRelatedSections(
474474
return sb.ToString();
475475
}
476476

477-
private static string FormatGuidelineType(GuidelineType type) => type switch
478-
{
479-
GuidelineType.Do => "DO",
480-
GuidelineType.Consider => "CONSIDER",
481-
GuidelineType.Avoid => "AVOID",
482-
GuidelineType.DoNot => "DO NOT",
483-
_ => "NOTE"
484-
};
485-
486-
private static readonly Dictionary<string, string> DiagnosticMap = new(StringComparer.OrdinalIgnoreCase)
477+
private static readonly Dictionary<string, string> DiagnosticMap= new(StringComparer.OrdinalIgnoreCase)
487478
{
488479
// Nullable reference types
489480
["CS8600"] = "nullable reference types",

0 commit comments

Comments
 (0)