Skip to content

Commit bb8ba98

Browse files
authored
Merge pull request #2753 from Widthdom/fix-issue1417-1469-1470
2 parents 1acf9e8 + f195323 commit bb8ba98

6 files changed

Lines changed: 309 additions & 15 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
category: fixed
3+
issues:
4+
- 1417
5+
affected:
6+
- src/CodeIndex/Mcp/McpServer.cs
7+
- src/CodeIndex/Mcp/McpToolHandlers.cs
8+
- tests/CodeIndex.Tests/McpServerTests.cs
9+
---
10+
11+
## English
12+
13+
- **MCP tool argument type mismatches now return JSON-RPC invalid params (#1417)** — wrong JSON types such as a string `limit` now produce `-32602` with structured parameter details instead of falling through to an internal/tool failure.
14+
15+
## 日本語
16+
17+
- **MCP ツール引数の型不一致が JSON-RPC invalid params を返すようになりました (#1417)** — 文字列の `limit` など誤った JSON 型は、internal/tool failure に落ちず `-32602` と構造化されたパラメータ詳細を返します。
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
category: fixed
3+
issues:
4+
- 1469
5+
affected:
6+
- src/CodeIndex/Mcp/McpServer.cs
7+
- tests/CodeIndex.Tests/McpServerTests.cs
8+
---
9+
10+
## English
11+
12+
- **MCP startup logs no longer expose the full DB path by default (#1469)** — the startup banner now logs only a sanitized DB filename unless `CDIDX_DEBUG=unsafe` is set.
13+
14+
## 日本語
15+
16+
- **MCP 起動ログが既定で完全な DB パスを公開しないようになりました (#1469)** — 起動バナーは `CDIDX_DEBUG=unsafe` が設定されていない限り、サニタイズ済みの DB ファイル名だけを記録します。
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
category: fixed
3+
issues:
4+
- 1470
5+
affected:
6+
- src/CodeIndex/Mcp/McpServer.cs
7+
- tests/CodeIndex.Tests/McpServerTests.cs
8+
---
9+
10+
## English
11+
12+
- **MCP catch-all error responses now hide exception details by default (#1470)** — unexpected tool and loop failures return generic wire messages while preserving detailed diagnostics in stderr, with verbose responses limited to `CDIDX_DEBUG=unsafe`.
13+
14+
## 日本語
15+
16+
- **MCP catch-all エラー応答が既定で例外詳細を隠すようになりました (#1470)** — 予期しないツール/ループ失敗は wire 上では汎用メッセージを返し、詳細診断は stderr に残します。詳細応答は `CDIDX_DEBUG=unsafe` の場合だけ有効です。

src/CodeIndex/Mcp/McpServer.cs

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ public partial class McpServer : IDisposable
157157
internal const int DefaultMaxResponseBytes = 10 * 1024 * 1024;
158158
private const string MaxResponseBytesEnvVar = "CDIDX_MCP_RESPONSE_MAX_BYTES";
159159
private const string KeepAliveIntervalEnvironmentVariable = "CDIDX_MCP_KEEP_ALIVE_INTERVAL_S";
160+
internal const string DebugEnvironmentVariable = "CDIDX_DEBUG";
160161
internal const int MaxJsonDepth = 32;
161162
internal const int MaxBatchRequestCount = 100;
162163
// Stdio buffer for the JSON-RPC loop. Sized to fit typical large MCP payloads (e.g. batch_query)
@@ -436,7 +437,7 @@ internal async Task RunAsync(IMcpTransport transport, CancellationToken cancella
436437

437438
// Use stderr for logging so stdout stays clean for JSON-RPC
438439
// stdoutをJSON-RPC用にクリーンに保つため、ログはstderrに出力
439-
ConsoleUi.TryWriteErrorLine($"[cdidx-mcp] Starting MCP server v{_version} (db: {_dbPath}, transport: {transport.Name} @ {transport.Endpoint}, max in-flight: {MaxConcurrency})");
440+
ConsoleUi.TryWriteErrorLine($"[cdidx-mcp] Starting MCP server v{_version} (db: {FormatDbPathForLog(_dbPath)}, transport: {transport.Name} @ {transport.Endpoint}, max in-flight: {MaxConcurrency})");
440441

441442
if (transport is HttpMcpTransport httpTransport)
442443
{
@@ -2143,11 +2144,25 @@ private async Task<JsonNode> HandleToolsCallAsync(JsonNode? id, JsonNode? callPa
21432144
if (ValidateToolArguments(toolName, args) is JsonObject argumentError)
21442145
{
21452146
metricsError = "invalid_argument";
2146-
response = CreateToolErrorResponse(id, argumentError["message"]!.GetValue<string>(),
2147-
category: McpErrorEnvelope.CategoryInvalidArgument,
2148-
suggestion: "Use exactly the argument names advertised by tools/list for this tool.",
2149-
retrySafe: false,
2150-
extraData: argumentError);
2147+
if (argumentError["jsonrpc_invalid_params"] is JsonValue invalidParamsMarker
2148+
&& invalidParamsMarker.TryGetValue<bool>(out var invalidParams)
2149+
&& invalidParams)
2150+
{
2151+
argumentError.Remove("jsonrpc_invalid_params");
2152+
response = CreateErrorResponse(hasId: true, id: id, code: -32602, message: argumentError["message"]!.GetValue<string>(),
2153+
category: McpErrorEnvelope.CategoryInvalidArgument,
2154+
suggestion: "Use the JSON types advertised by tools/list for this tool.",
2155+
retrySafe: false,
2156+
extraData: argumentError);
2157+
}
2158+
else
2159+
{
2160+
response = CreateToolErrorResponse(id, argumentError["message"]!.GetValue<string>(),
2161+
category: McpErrorEnvelope.CategoryInvalidArgument,
2162+
suggestion: "Use exactly the argument names advertised by tools/list for this tool.",
2163+
retrySafe: false,
2164+
extraData: argumentError);
2165+
}
21512166
}
21522167
else if (ValidateCommonListArguments(args) is JsonObject listArgumentError)
21532168
{
@@ -2556,6 +2571,28 @@ internal static string BuildUnknownNotificationLog(string method) =>
25562571
internal static bool IsSupportedMcpLogLevel(string? level)
25572572
=> level is "debug" or "info" or "notice" or "warning" or "error" or "critical" or "alert" or "emergency";
25582573

2574+
internal static bool IsUnsafeDebugEnabled()
2575+
=> string.Equals(Environment.GetEnvironmentVariable(DebugEnvironmentVariable), "unsafe", StringComparison.OrdinalIgnoreCase);
2576+
2577+
internal static string FormatDbPathForLog(string dbPath)
2578+
{
2579+
if (IsUnsafeDebugEnabled())
2580+
return dbPath;
2581+
2582+
try
2583+
{
2584+
var path = dbPath;
2585+
if (Uri.TryCreate(dbPath, UriKind.Absolute, out var uri) && uri.IsFile)
2586+
path = uri.LocalPath;
2587+
var fileName = Path.GetFileName(path);
2588+
return string.IsNullOrWhiteSpace(fileName) ? "(configured db)" : fileName;
2589+
}
2590+
catch
2591+
{
2592+
return "(configured db)";
2593+
}
2594+
}
2595+
25592596
// Wire-safe error body for the tool catch-all. Mentions the tool and the
25602597
// exception type so the client can branch (retry vs. surface to user)
25612598
// while keeping bound values or matched content out of the response (#1530).
@@ -2569,6 +2606,8 @@ internal static bool IsSupportedMcpLogLevel(string? level)
25692606
// #1530 で封じた ex.Message 漏れを再現させずに失敗詳細をクライアントへ届ける。
25702607
internal static string BuildSanitizedToolErrorMessage(string toolName, Exception ex)
25712608
{
2609+
if (!IsUnsafeDebugEnabled())
2610+
return $"Tool '{toolName}' failed. See cdidx server stderr for details.";
25722611
if (ex is CodeIndexException codeIndexEx)
25732612
return $"Error executing {toolName} ({ex.GetType().Name}) [{codeIndexEx.Code}/{codeIndexEx.Category}]{BuildPathFragment(codeIndexEx)}{BuildHintFragment(codeIndexEx)}. See cdidx server stderr for details.";
25742613
return $"Error executing {toolName} ({ex.GetType().Name}). See cdidx server stderr for details.";
@@ -2579,6 +2618,8 @@ internal static string BuildSanitizedToolErrorMessage(string toolName, Exception
25792618
// JSON-RPC ループ catch-all のワイヤー向け本文。理由はツール catch-all と同じ(#1530, #1580)。
25802619
internal static string BuildSanitizedLoopErrorMessage(Exception ex)
25812620
{
2621+
if (!IsUnsafeDebugEnabled())
2622+
return "Internal MCP error. See cdidx server stderr for details.";
25822623
if (ex is CodeIndexException codeIndexEx)
25832624
return $"Internal error ({ex.GetType().Name}) [{codeIndexEx.Code}/{codeIndexEx.Category}]{BuildPathFragment(codeIndexEx)}{BuildHintFragment(codeIndexEx)}. See cdidx server stderr for details.";
25842625
return $"Internal error ({ex.GetType().Name}). See cdidx server stderr for details.";

src/CodeIndex/Mcp/McpToolHandlers.cs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,9 +386,88 @@ private static List<string> ReadStringList(JsonNode? args, string propertyName)
386386
}
387387
}
388388

389+
if (ValidateToolArgumentTypes(toolName, obj) is JsonObject typeError)
390+
return typeError;
391+
389392
return null;
390393
}
391394

395+
private static JsonObject? ValidateToolArgumentTypes(string toolName, JsonObject args)
396+
{
397+
foreach (var property in args)
398+
{
399+
if (TryGetExpectedJsonType(toolName, property.Key, out var expected)
400+
&& !MatchesExpectedJsonType(property.Value, expected))
401+
{
402+
return new JsonObject
403+
{
404+
["message"] = $"Invalid type for argument '{property.Key}' on tool '{toolName}'. Expected {expected}.",
405+
["tool"] = toolName,
406+
["parameter"] = property.Key,
407+
["expected"] = expected,
408+
["actual"] = DescribeJsonType(property.Value),
409+
["jsonrpc_invalid_params"] = true,
410+
};
411+
}
412+
}
413+
414+
return null;
415+
}
416+
417+
private static bool TryGetExpectedJsonType(string toolName, string argumentName, out string expected)
418+
{
419+
if (argumentName is "path" or "project" or "excludePaths" or "names")
420+
{
421+
expected = string.Empty;
422+
return false;
423+
}
424+
425+
expected = argumentName switch
426+
{
427+
"limit" or "offset" or "snippetLines" or "maxLineWidth" or "before" or "after" or
428+
"focusLine" or "focusColumn" or "focusLength" or "startLine" or "endLine" or
429+
"maxHops" or "maxDepth" or "depth" or "parallelism" => "integer",
430+
"excludeTests" or "includeGenerated" or "rawQuery" or "noDedup" or "exactSubstring" or
431+
"exactName" or "exact" or "prefix" or "countOnly" or "includeBody" or "lsp_compatible" or
432+
"regex" or "withPaths" or "rebuild" or "dryRun" or "dry_run" or "force" or "optimize" => "boolean",
433+
"query" or "lang" or "kind" or "format" or "rankBy" or "since" or "path" or "project" or
434+
"solution" or "symbol" or "direction" or "groupBy" or "category" or "language" or
435+
"description" or "context" or "toolInvocationContext" or "db" => "string",
436+
"queries" => "array",
437+
_ => string.Empty,
438+
};
439+
440+
if (expected.Length == 0)
441+
return false;
442+
443+
return true;
444+
}
445+
446+
private static bool MatchesExpectedJsonType(JsonNode? node, string expected) => expected switch
447+
{
448+
"integer" => node is JsonValue value && value.TryGetValue<int>(out _),
449+
"boolean" => node is JsonValue value && value.TryGetValue<bool>(out _),
450+
"string" => node is JsonValue value && value.TryGetValue<string>(out _),
451+
"array" => node is JsonArray,
452+
_ => true,
453+
};
454+
455+
private static string DescribeJsonType(JsonNode? node)
456+
{
457+
if (node is null)
458+
return "null";
459+
return node.GetValueKind() switch
460+
{
461+
JsonValueKind.String => "string",
462+
JsonValueKind.Number => "number",
463+
JsonValueKind.True or JsonValueKind.False => "boolean",
464+
JsonValueKind.Array => "array",
465+
JsonValueKind.Object => "object",
466+
JsonValueKind.Null => "null",
467+
_ => "unknown",
468+
};
469+
}
470+
392471
private static bool IsKnownToolName(string toolName) => toolName switch
393472
{
394473
"search" or "definition" or "references" or "callers" or "callees" or "symbols" or

0 commit comments

Comments
 (0)