Skip to content

fix(mcp): stop HTTP status detection from matching data in exception text - #1913

Merged
Aaronontheweb merged 2 commits into
netclaw-dev:devfrom
Aaronontheweb:fix/mcp-status-sniff-embedded-data
Aug 13, 2026
Merged

fix(mcp): stop HTTP status detection from matching data in exception text#1913
Aaronontheweb merged 2 commits into
netclaw-dev:devfrom
Aaronontheweb:fix/mcp-status-sniff-embedded-data

Conversation

@Aaronontheweb

Copy link
Copy Markdown
Collaborator

Problem

McpClientManager.FindHttpStatus walked an exception chain and read an
HTTP status from bare text: Contains("401") and Contains("403"). An
exception message can hold other data, like a command name or a temp
path. That data can hold digits that look like a status code by
chance.

This defect caused two problems.

  • A flaky test. FailedStdioStartup_IsReportedBeforeLeaseAssertions
    builds a fake stdio command name from a random GUID. When the GUID
    holds 401 or 403, the stdio spawn failure misreports as an HTTP
    auth failure, so the test asserts the wrong message. CI run
    31659263924 shows the failure with GUID 632401b4.... The odds of a
    hit are about 1.4% per run: a :N-formatted GUID has 32 hex digits
    and 30 three-digit start positions, each with a 1-in-4096 chance of
    spelling 401, and the same again for 403, so the combined odds
    round to 1.4%. Root cause was verified by two independent specialist
    analyses; see the comment trail on fix(providers): detect a mid-stream LLM stall within seconds #1888.
  • A wrong report in production. A real stdio MCP server with 401
    or 403 in its command or file path shows as an HTTP auth failure in
    netclaw mcp list and netclaw doctor, hiding the real spawn error.

A sibling method, CreateSafeOAuthError, calls the same
FindHttpStatus and carried the same risk for messages that embed
GUID-bearing temp paths.

Fix

Two layers, applied together.

  • Skip status detection for stdio transport failures. A stdio
    server is a local child process. No HTTP request ever occurs for it,
    so no HTTP status can be genuine. BuildConnectionFailureStatus
    already receives the server's Transport through McpServerEntry,
    so this needed no new parameter — just a check at the top of the
    method, threaded through CreateUnreachableStatus and
    GetSafeConnectionFailure.
  • Remove the bare Contains("401")/Contains("403") checks in
    FindHttpStatus.
    The method now trusts only two anchored shapes:
    a typed HttpRequestException.StatusCode, or literal text like
    "HTTP 401" / "status Unauthorized". CreateSafeOAuthError
    inherits the fix automatically, since it calls the same method.

No genuine 401/403 detection test needed the bare substring pattern.
Every existing HTTP-failure test constructs its exception with a typed
HttpStatusCode (caught by the first branch), and the one message-only
case (DynamicRegistrationBadRequestIsNotMisreportedAsOuterUnauthorizedChallenge)
already used the anchored "status BadRequest" shape. Decompiling the
MCP SDK confirmed HttpResponseMessageExtensions.CreateHttpRequestException
and McpOAuthClientRegistrar both set the typed HttpRequestException.StatusCode
whenever they report a real HTTP failure, so the anchored/typed checks
already cover every real-world shape.

Tests

  • New unit test:
    BuildConnectionFailureStatus_ForStdioSpawnFailureWithEmbeddedStatusLikeDigits_ReturnsUnreachable.
    Feeds a stdio spawn failure whose command name embeds 401 (mirrors
    the real GUID from CI run 31659263924). Fails against the old code
    (asserts the HTTP 401 string), passes against the fix.
  • Added a regression-note comment at the flaky test's GUID-based
    command builder, pointing at this fix and the new unit test. The GUID
    generation itself is unchanged, so the flaky test still proves the
    fix on its own terms.

Validation

  • dotnet build Netclaw.slnx -c Release — 0 warnings, 0 errors.
  • dotnet test src/Netclaw.Daemon.Tests — 1021 passed, 0 failed.
  • dotnet slopwatch analyze -d . — 0 new violations (1 pre-existing
    baselined warning, unrelated file).
  • ./scripts/Add-FileHeaders.ps1 -Verify — all files have headers.
  • FailedStdioStartup_IsReportedBeforeLeaseAssertions run 25 times in
    a loop — 25/25 passed.

McpClientManager.FindHttpStatus checked an exception message for the
bare text "401" and "403". An exception message can hold other data,
like a command name or a GUID. A GUID that held "401" made the method
report the wrong HTTP status.

This defect caused two problems.

- A flaky test. The test builds a stdio command name from a random
  GUID. The test fails when the GUID holds "401" or "403".
- A wrong report. A stdio server with "401" or "403" in its command
  path shows as an HTTP auth failure in `netclaw mcp list` and
  `netclaw doctor`.

The fix has two parts.

- FindHttpStatus now skips status detection for stdio transport
  failures. A stdio server is a local process. No HTTP request occurs
  for it.
- FindHttpStatus no longer checks the bare text "401" or "403". It
  trusts only a typed `HttpRequestException.StatusCode`, or an
  anchored text pattern like "HTTP 401" or "status Unauthorized".

CreateSafeOAuthError calls FindHttpStatus too, so it gets the same fix.

A new test proves the fix. A stdio spawn failure with "401" in the
command name now reports as "Failed to reach MCP server. Check daemon
logs for details." It does not report as an HTTP 401 failure. The
test fails on the old code. The test passes on the new code.
@Aaronontheweb Aaronontheweb added bug Something isn't working mcp Model context protocol server / client issues. tests All issues related to testing, quality assurance, and smoke testing. labels Aug 13, 2026
@Aaronontheweb
Aaronontheweb enabled auto-merge (squash) August 13, 2026 03:25

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Aaronontheweb
Aaronontheweb disabled auto-merge August 13, 2026 12:52
@Aaronontheweb
Aaronontheweb merged commit 4c1c53a into netclaw-dev:dev Aug 13, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working mcp Model context protocol server / client issues. tests All issues related to testing, quality assurance, and smoke testing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant