Skip to content

Commit dbd4d80

Browse files
PR Feedback
- 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 Fix 3 subagent-found bugs: trimmedPattern in search, Razor HTML escaping, whitespace-only type guard - BookListingTool.cs: use trimmedPattern (not pattern) in Contains() search call - Index.cshtml: fix Razor HTML escaping of optional badge via @if block - BookGuidelinesTool.cs: use IsNullOrWhiteSpace guard for type param - BookGuidelinesTool.cs: fix description to say substring match not exact text Use MCP SDK JsonRpcError/JsonRpcErrorDetail instead of custom records Replace McpRateLimitErrorEnvelope and McpRateLimitErrorDetail private records with the SDK's built-in ModelContextProtocol.Protocol types, as suggested in PR review.
1 parent 19c6015 commit dbd4d80

8 files changed

Lines changed: 146 additions & 426 deletions

File tree

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);
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: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.Threading.RateLimiting;
2+
using ModelContextProtocol.Protocol;
23
using EssentialCSharp.Chat.Common.Extensions;
34
using EssentialCSharp.Web.Areas.Identity.Data;
45
using EssentialCSharp.Web.Areas.Identity.Services.PasswordValidators;
@@ -324,15 +325,19 @@ private static void Main(string[] args)
324325
if (context.HttpContext.Request.Path.StartsWithSegments("/mcp"))
325326
{
326327
context.HttpContext.Response.ContentType = "application/json";
327-
var mcpErrorResponse = new
328+
var mcpErrorResponse = new JsonRpcError
328329
{
329-
jsonrpc = "2.0",
330-
error = new { code = -32000, message = "Rate limit exceeded. Please wait before sending another request." },
331-
id = (object?)null
330+
JsonRpc = "2.0",
331+
Error = new JsonRpcErrorDetail
332+
{
333+
Code = -32000,
334+
Message = "Rate limit exceeded. Please wait before sending another request."
335+
}
332336
};
333-
await context.HttpContext.Response.WriteAsync(
334-
System.Text.Json.JsonSerializer.Serialize(mcpErrorResponse),
335-
cancellationToken);
337+
await System.Text.Json.JsonSerializer.SerializeAsync(
338+
context.HttpContext.Response.Body,
339+
mcpErrorResponse,
340+
cancellationToken: cancellationToken);
336341
return;
337342
}
338343

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
}
Lines changed: 40 additions & 59 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,17 +18,18 @@ 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 (case-insensitive substring 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 substring 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 case-insensitive substring 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 substring 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);
2931

30-
if (type is not null && typeFilter is null)
32+
if (!string.IsNullOrWhiteSpace(type) && typeFilter is null)
3133
{
3234
return "Invalid guideline type. Valid values: 'do', 'consider', 'avoid', 'donot' (also accepts 'do not', 'dont').";
3335
}
@@ -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: 9 additions & 2 deletions
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
@@ -66,7 +73,7 @@ public async Task<string> SearchListingsByCode(
6673
foreach (var listing in listings)
6774
{
6875
if (found >= maxResults) break;
69-
if (listing.Content.Contains(pattern, StringComparison.OrdinalIgnoreCase))
76+
if (listing.Content.Contains(trimmedPattern, StringComparison.OrdinalIgnoreCase))
7077
{
7178
string langHint = listing.FileExtension == "cs" ? "csharp" : listing.FileExtension;
7279
sb.AppendLine(CultureInfo.InvariantCulture, $"### Listing {listing.ChapterNumber}.{listing.ListingNumber}");

0 commit comments

Comments
 (0)