Skip to content

Refactor MCP text-result parsing and remove responseWriter StatusCode shadowing#7041

Merged
lpcox merged 7 commits into
mainfrom
copilot/refactor-semantic-function-clustering
Jun 5, 2026
Merged

Refactor MCP text-result parsing and remove responseWriter StatusCode shadowing#7041
lpcox merged 7 commits into
mainfrom
copilot/refactor-semantic-function-clustering

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 5, 2026

The codebase had the same MCP content/text parsing logic duplicated across mcp, server, and difc, plus a responseWriter.StatusCode() method that shadowed the promoted BaseResponseWriter.StatusCode field. This change consolidates MCP text extraction into one shared path and makes server-side status handling consistent with the base writer pattern used elsewhere.

  • Shared MCP text-content extraction

    • Added a shared helper for extracting concatenated text from raw MCP tool results.
    • Preserved compatibility with both decoded []interface{} content and []map[string]interface{} content produced by MCP response builders.
    • Kept internal/mcp as the package-facing entrypoint while moving the shared implementation into a cycle-free helper package.
  • Server rate-limit detection

    • Replaced bespoke content traversal in internal/server/rate_limit.go with the shared extractor.
    • Centralized the text source used by both:
      • rate-limit error message extraction
      • rate-limit detection / reset parsing
  • DIFC MCP response unwrapping

    • Updated unwrapMCPResponse in internal/difc/path_labels.go to reuse the same text extraction path before JSON unwrapping.
    • This also covers MCP responses built from []map[string]interface{} without duplicating traversal logic.
  • Response writer status handling

    • Removed responseWriter.StatusCode() from internal/server/response_writer.go.
    • Updated server logging and tests to use the embedded BaseResponseWriter.StatusCode field directly, eliminating the method/field shadowing ambiguity.
  • Focused test coverage

    • Added coverage for shared MCP text extraction across both MCP result shapes and malformed/non-text entries.
    • Added regression coverage for DIFC unwrapping and rate-limit text extraction using MCP text-response helpers.
    • Updated response-writer tests to validate direct field access.

Example of the status-code cleanup:

// before
logSDK.Printf("status=%d", lw.StatusCode())

// after
logSDK.Printf("status=%d", lw.StatusCode)

Copilot AI changed the title [WIP] Refactor MCP content parsing and address StatusCode shadowing Refactor MCP text-result parsing and remove responseWriter StatusCode shadowing Jun 5, 2026
Copilot finished work on behalf of lpcox June 5, 2026 14:00
Copilot AI requested a review from lpcox June 5, 2026 14:00
@lpcox lpcox marked this pull request as ready for review June 5, 2026 14:38
Copilot AI review requested due to automatic review settings June 5, 2026 14:38
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR consolidates MCP raw-result text extraction into a shared helper and removes responseWriter.StatusCode() to avoid method/field shadowing, updating server-side logging and tests accordingly.

Changes:

  • Introduces internal/mcpresult.ExtractTextContent and wires internal/mcp.ExtractTextContentFromResult to it for shared MCP text extraction.
  • Refactors rate-limit detection and DIFC MCP-response unwrapping to reuse the shared text extractor.
  • Removes responseWriter.StatusCode() and updates logging/tests to use the embedded BaseResponseWriter.StatusCode field directly.
Show a summary per file
File Description
internal/server/transport_test.go Updates tests to assert StatusCode field instead of removed accessor method.
internal/server/response_writer.go Removes StatusCode() method to eliminate method/field shadowing.
internal/server/rate_limit.go Replaces bespoke MCP content/text traversal with shared MCP text extraction helper.
internal/server/middleware.go Updates SDK logging to use lw.StatusCode field directly.
internal/server/http_helpers.go Updates response logging to use lw.StatusCode field directly.
internal/server/circuit_breaker_test.go Adds regression coverage for rate-limit text extraction using BuildMCPTextResponse.
internal/mcpresult/text_content.go Adds shared implementation for extracting concatenated text from raw MCP tool result maps.
internal/mcp/tool_result.go Adds ExtractTextContentFromResult wrapper to expose shared extraction via internal/mcp.
internal/mcp/tool_result_test.go Adds focused tests for ExtractTextContentFromResult across supported shapes and malformed entries.
internal/difc/path_labels.go Updates MCP response unwrapping to use shared text extractor before JSON unmarshal.
internal/difc/path_labels_test.go Adds coverage for unwrapping []map[string]interface{}-shaped MCP responses.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 11/11 changed files
  • Comments generated: 1

Comment thread internal/mcpresult/text_content.go
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 5, 2026

@copilot fix the failing ci check https://github.com/github/gh-aw-mcpg/actions/runs/27022607362/job/79754235458?pr=7041

Fixed in commit bb780f7.

I investigated the linked run and addressed the merge-ref lint failure (TestCopyTrimmedStringIntMap redeclared in this block) by updating internal/strutil/util_test.go to avoid the duplicate test symbol on the PR merge result. I also re-ran local lint/tests and final validation.

Copilot finished work on behalf of lpcox June 5, 2026 15:14
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Jun 5, 2026

@copilot resolve the merge conflicts in this pull request

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 5, 2026

@copilot resolve the merge conflicts in this pull request

Resolved by merging origin/main into this PR branch and fixing the conflict in internal/strutil/util_test.go. Included in commit 1958a157.

Copilot finished work on behalf of lpcox June 5, 2026 15:24
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented Jun 5, 2026

@copilot resolve the merge conflicts in this pull request

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 5, 2026

@copilot resolve the merge conflicts in this pull request

Resolved by merging origin/main again and fixing the recurring conflict in internal/strutil/util_test.go (removing the duplicate CopyTrimmedStringIntMap tests from that file). Included in commits c6dfcf1c and 474770e9.

Copilot finished work on behalf of lpcox June 5, 2026 15:36
@lpcox lpcox merged commit c80fb57 into main Jun 5, 2026
16 checks passed
@lpcox lpcox deleted the copilot/refactor-semantic-function-clustering branch June 5, 2026 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[refactor] Semantic function clustering analysis: MCP content parsing duplication and StatusCode shadowing

3 participants