Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Sep 30, 2025

Summary

This PR implements the IOptionsFormatter interface on the McpOptions class, following the pattern established by other Azure WebJobs SDK extensions (e.g., HttpOptions) to improve serviceability and diagnostics.

Changes

McpOptions.cs

  • Implements IOptionsFormatter interface
  • Adds Format() method that returns a formatted JSON string containing configuration values
  • Includes only non-sensitive configuration properties:
    • ServerName
    • ServerVersion
    • Instructions
    • EncryptClientState
    • MessageOptions.UseAbsoluteUriForEndpoint

Testing

  • Added two new tests in McpOptionsTests.cs to verify the Format() implementation
  • Format_ReturnsExpectedJson() validates correct JSON output with custom values
  • Format_IncludesAllNonSensitiveProperties() ensures all expected properties are present
  • All 56 tests pass successfully (54 existing + 2 new)

Benefits

This implementation allows the Azure Functions runtime to log configuration values for diagnostic purposes, making it easier to troubleshoot configuration-related issues in production environments. The formatted output follows the same pattern as other extensions, providing a consistent experience across the WebJobs SDK ecosystem.

Fixes #<issue_number>

Original prompt

This section details on the original issue you should resolve

<issue_title>Implement IOptionsFormatter on McpOptions</issue_title>
<issue_description>### Description

This follows the pattern of all other extensions, improving serviceability. For example, HttpOptions here https://github.com/Azure/azure-webjobs-sdk-extensions/blob/fab754723aec94be723b489e1cc42f247b199394/src/WebJobs.Extensions.Http/Config/HttpOptions.cs#L16. Only non-sensitive config values should be logged.</issue_description>

Comments on the Issue (you are @copilot in this section)

Fixes #110

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot AI changed the title [WIP] Implement IOptionsFormatter on McpOptions Implement IOptionsFormatter on McpOptions for improved serviceability Sep 30, 2025
Copilot AI requested a review from mathewc September 30, 2025 18:00
Copilot AI requested a review from mathewc September 30, 2025 18:21
@mathewc
Copy link
Member

mathewc commented Sep 30, 2025

@copilot Please address the PR comments I've made up to this point

Copy link
Member

@mathewc mathewc left a comment

Choose a reason for hiding this comment

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

Please address the requested changes

@mathewc mathewc marked this pull request as ready for review September 30, 2025 21:16
@fabiocav
Copy link
Member

@copilot act on the feedback provided.

[EditorBrowsable(EditorBrowsableState.Never)]
string IOptionsFormatter.Format()
{
JObject options = new JObject
Copy link
Member

@mathewc mathewc Sep 30, 2025

Choose a reason for hiding this comment

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

Use System.Text.Json rather than Newtonsoft

{
{ nameof(ServerName), ServerName },
{ nameof(ServerVersion), ServerVersion },
{ nameof(Instructions), Instructions },
Copy link
Member

@mathewc mathewc Sep 30, 2025

Choose a reason for hiding this comment

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

Let's omit Instructions, since this may contain customer sensitive info


Assert.NotNull(formatted);

var json = JObject.Parse(formatted);
Copy link
Member

@mathewc mathewc Sep 30, 2025

Choose a reason for hiding this comment

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

Also do an assertion on the number of properties to make the test stronger


var json = JObject.Parse(formatted);

Assert.Contains(nameof(options.ServerName), json.Properties().Select(p => p.Name));
Copy link
Member

@mathewc mathewc Sep 30, 2025

Choose a reason for hiding this comment

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

Same here - do an assertion on the number of properties logged

Copy link
Member

@mathewc mathewc left a comment

Choose a reason for hiding this comment

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

Please fix

@mathewc mathewc self-requested a review September 30, 2025 22:28
Copy link
Member

@mathewc mathewc left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@mathewc mathewc left a comment

Choose a reason for hiding this comment

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

@copilot See the comments I left and please address

@microsoft-github-policy-service

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

@mathewc mathewc closed this Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement IOptionsFormatter on McpOptions

3 participants