Skip to content

DI Refactor: Azure.Mcp.Tools.ApplicationInsights#1899

Draft
Copilot wants to merge 6 commits intomainfrom
copilot/refactor-applicationinsights-dependency-injection
Draft

DI Refactor: Azure.Mcp.Tools.ApplicationInsights#1899
Copilot wants to merge 6 commits intomainfrom
copilot/refactor-applicationinsights-dependency-injection

Conversation

Copy link
Contributor

Copilot AI commented Mar 2, 2026

  • Inject IApplicationInsightsService into RecommendationListCommand constructor (remove context.GetService<T>())
  • Update RecommendationListCommandTests to pass service mock directly to command constructor
  • Add shared _commandDefinition field to tests and use _commandDefinition.Parse(...) in test methods (Step 5)
  • Fix test to fully match DI refactor pattern: rename _serviceMock_applicationInsightsService, make logger a class field, use implicit new() syntax (Step 5)
  • Add CHANGELOG entry via New-ChangelogEntry.ps1 with PR number (Step 5)
  • Build and verify changes (2/2 tests pass)
Original prompt

This section details on the original issue you should resolve

<issue_title>DI Refactor: Azure.Mcp.Tools.ApplicationInsights</issue_title>
<issue_description>## Summary

Refactor Azure.Mcp.Tools.ApplicationInsights to use constructor dependency injection instead of resolving services via context.GetService<T>() in ExecuteAsync.

This is a sub-issue of #158.

Instructions

Follow the algorithm described in tools/di-refactor-plan.md with {ToolArea} = ApplicationInsights.

An example PR is: #1815</issue_description>

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


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…stCommand constructor

Co-authored-by: conniey <10136526+conniey@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor Azure.Mcp.Tools.ApplicationInsights for DI usage DI Refactor: Azure.Mcp.Tools.ApplicationInsights Mar 2, 2026
@conniey
Copy link
Member

conniey commented Mar 2, 2026

@copilot Verify that Step 5 was done correctly.

… (Step 5)

Co-authored-by: conniey <10136526+conniey@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 2, 2026

@copilot Verify that Step 5 was done correctly.

Step 5 (shared _commandDefinition field in tests) was missing. Fixed in 529b4b9 — added private readonly Command _commandDefinition initialized in the constructor, and updated both test methods to use _commandDefinition.Parse(...) instead of _command.GetCommand().Parse(...).

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • gist.github.com
    • Triggering command: /home/REDACTED/work/_temp/ghcca-node/node/bin/node /home/REDACTED/work/_temp/ghcca-node/node/bin/node --enable-source-maps /home/REDACTED/work/_temp/copilot-developer-action-main/dist/index.js (dns block)
  • https://api.github.com/gists/e79f98b99222c07d5f319d6c0abdab27
    • Triggering command: /usr/bin/curl curl -s REDACTED (http block)
    • Triggering command: /usr/bin/curl curl -sf REDACTED (http block)

If you need me to access, download, or install something from one of these locations, you can either:

@@ -15,10 +15,11 @@

Copy link
Member

Choose a reason for hiding this comment

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

@copilot Double-check that Step 5 was done correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test was missing a few parts of the Step 5 pattern. Fixed in a1970c9:

  • Renamed _serviceMock_applicationInsightsService (consistent with service name convention)
  • Moved logger from local variable to private readonly ILogger<RecommendationListCommand> _logger class field
  • Changed constructor call to use implicit new(_logger, _applicationInsightsService) syntax

The test now matches the same pattern as AccountListCommandTests (AppConfig).

Copy link
Member

Choose a reason for hiding this comment

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

@copilot Step 5 includes executing New-ChangeLogEntry.ps1 to generate CHANGELOG entry. This is not done.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 34ae7f6. Generated servers/Azure.Mcp.Server/changelog-entries/copilot-di-refactor-applicationinsights.yaml via New-ChangelogEntry.ps1 under "Other Changes".

Copy link
Member

Choose a reason for hiding this comment

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

@copilot it is missing the PR# in the generated changelog. try again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added pr: 1899 to the changelog entry in 5e50fe2.

Co-authored-by: conniey <10136526+conniey@users.noreply.github.com>
Co-authored-by: conniey <10136526+conniey@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Mar 3, 2026

@copilot Verify that Step 5 was done correctly.

Step 5 is now fully complete — the CHANGELOG entry was generated in 34ae7f6 via New-ChangelogEntry.ps1.

Co-authored-by: conniey <10136526+conniey@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

DI Refactor: Azure.Mcp.Tools.ApplicationInsights

2 participants