Skip to content

DI Refactor: Azure.Mcp.Tools.Authorization#1901

Open
Copilot wants to merge 5 commits intomainfrom
copilot/refactor-authorization-di
Open

DI Refactor: Azure.Mcp.Tools.Authorization#1901
Copilot wants to merge 5 commits intomainfrom
copilot/refactor-authorization-di

Conversation

Copy link
Contributor

Copilot AI commented Mar 2, 2026

  • Understand the DI refactor pattern from AppConfig example
  • Update RoleAssignmentListCommand to inject IAuthorizationService via constructor instead of context.GetService<T>()
  • Update RoleAssignmentListCommandTests to pass service via constructor and follow updated test pattern
  • All 3 unit tests pass
  • Add changelog entry using New-ChangelogEntry.ps1 script (Step 5.3 from di-refactor-plan.md)
Original prompt

This section details on the original issue you should resolve

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

Refactor Azure.Mcp.Tools.Authorization 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} = Authorization.

An example PR is: #1815</issue_description>

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


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…gnmentListCommand

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

conniey commented Mar 2, 2026

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

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

Copilot AI commented Mar 2, 2026

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

Step 5 requires adding a changelog entry (Step 5.3 in the refactor plan). This was missing from the initial commit. Added the changelog entry in commit 6eeb75a at servers/Azure.Mcp.Server/changelog-entries/copilot-authorization-di-refactor.yaml.

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)

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

Co-authored-by: conniey <10136526+conniey@users.noreply.github.com>
@conniey conniey marked this pull request as ready for review March 3, 2026 19:37
@conniey conniey requested review from jongio and vurhanau as code owners March 3, 2026 19:37
Copilot AI review requested due to automatic review settings March 3, 2026 19:37
@conniey conniey requested review from a team and xiangyan99 as code owners March 3, 2026 19:37
@conniey conniey requested a review from alzimmermsft March 3, 2026 19:37
Copy link
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 refactors RoleAssignmentListCommand to use constructor dependency injection for IAuthorizationService instead of resolving it through context.GetService<T>() inside ExecuteAsync, following the established DI refactor pattern.

Changes:

  • RoleAssignmentListCommand now accepts IAuthorizationService via constructor injection
  • Tests updated to inject the service via constructor and share _command, _context, and _commandDefinition as fields
  • Changelog entry added for the change

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
RoleAssignmentListCommand.cs Adds IAuthorizationService constructor parameter; removes context.GetService<T>() call
RoleAssignmentListCommandTests.cs Passes IAuthorizationService to command constructor; promotes shared state to fields
1772484566722.yaml Changelog entry for the DI refactor

Co-authored-by: Copilot <175728472+Copilot@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.Authorization

3 participants