Skip to content

Fixing bug that was hiding error messages when publish and unpublish happen - #503

Open
deepaligargms wants to merge 1 commit into
mainfrom
u/deepaligarg/climessagefix
Open

deepaligargms wants to merge 1 commit into
mainfrom
u/deepaligarg/climessagefix

Conversation

@deepaligargms

Copy link
Copy Markdown
Contributor

What

a365 develop-mcp publish printed ERROR: Failed to publish ...: No response received on
every failure — a duplicate-instance rejection, a 400 validation error, or a downstream
5xx all looked identical, with the actual server message thrown away.

Root cause: Agent365ToolingService.PublishServerAsync did return null; on failure,
discarding the responseContent that ValidateResponseAsync had already extracted. The
executor then rendered publishResponse?.Message ?? "No response received" → the fallback
string, always.

Change

  • New internal static BuildPublishFailureResponse(responseContent, statusCode, logger) that
    returns a populated PublishMcpServerResponse (Status = "Failed") carrying the real message.
    The publish failure branch now calls it instead of return null;.
  • It handles the three body shapes the platform emits, in order:
    1. the { Status, Message } envelope — including double-serialized bodies, because the
      platform returns its already-JSON string via Ok(string), which re-serializes it
      (unwrapped by the existing DeserializeWithDoubleSerialization helper);
    2. ASP.NET { error[, details] } / { message } problem bodies (via ExtractErrorMessage,
      preferring details);
    3. fallback to Server returned {statusCode} when the body has no readable message.

This mirrors what AddServerAsync already does correctly — publish was the outlier.

Before / after

Before:
ERROR: Failed to publish MCP server msdyn_DataverseMCPServer: No response received

After (example — duplicate instance):
ERROR: Failed to publish MCP server msdyn_DataverseMCPServer: MCP server '...' is already
published in environment '...' under alias '...'. Only one published instance is allowed
per server.

Tests

7 new [Fact]s in Agent365ToolingServicePureFunctionTests covering every branch:
double-serialized envelope, single-serialized envelope, {error} (400), {error,details}
(500, prefers details), empty body, null body, non-JSON body. Full pure-function +
publish/develop/tooling suites green (175 tests).

Reviewer notes

  • No behavior change on the success path — PublishServerAsync still returns the deserialized
    success response; only the failure branch changed.
  • ValidateResponseAsync already flags both non-2xx responses and 200 bodies whose envelope
    Status != "Success", so this correctly surfaces the platform's 200-with-Status:"Failed"
    rejection as well.

Copilot AI lite review requested due to automatic review settings September 23, 2026 23:04
@deepaligargms
deepaligargms requested review from a team as code owners September 23, 2026 23:04
@github-actions github-actions Bot added the bug Something isn't working label Sep 23, 2026
@github-actions

Copy link
Copy Markdown

⚠️ Deprecation Warning: The deny-licenses option is deprecated for possible removal in the next major release. For more information, see issue 997.

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant