Skip to content

.NET: fix: resolve CA1873 in GitHubCopilotAgent by using LoggerMessage sour…#6814

Merged
alliscode merged 1 commit into
microsoft:mainfrom
alliscode:bentho/fix-ca1873-github-copilot-logger
Jun 30, 2026
Merged

.NET: fix: resolve CA1873 in GitHubCopilotAgent by using LoggerMessage sour…#6814
alliscode merged 1 commit into
microsoft:mainfrom
alliscode:bentho/fix-ca1873-github-copilot-logger

Conversation

@alliscode

Copy link
Copy Markdown
Member

This pull request refactors how warnings are logged when a custom OnPreToolUse hook is present in the SessionConfig. Instead of using a raw logger.LogWarning call, it introduces a strongly-typed logging extension using LoggerMessageAttribute for improved performance and maintainability.

Logging improvements:

  • Added a new file GitHubCopilotAgentLogMessages.cs that defines a LogApprovalGatingSkippedDueToCustomHook extension method using LoggerMessageAttribute for compile-time optimized logging.
  • Updated the logging call in GitHubCopilotAgent.cs to use the new LogApprovalGatingSkippedDueToCustomHook method instead of a direct LogWarning call.

…ce generator

Replace the direct logger.LogWarning() call (which eagerly evaluates
string.Join()) with a [LoggerMessage]-generated extension method in
GitHubCopilotAgentLogMessages.cs.

Fixes build error:
  GitHubCopilotAgent.cs(580,13): error CA1873: Evaluation of this argument
  may be expensive and unnecessary if logging is disabled

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 30, 2026 01:10
@alliscode alliscode enabled auto-merge June 30, 2026 01:11
@moonbox3 moonbox3 added the .NET Usage: [Issues, PRs], Target: .Net label Jun 30, 2026
@github-actions github-actions Bot changed the title fix: resolve CA1873 in GitHubCopilotAgent by using LoggerMessage sour… .NET: fix: resolve CA1873 in GitHubCopilotAgent by using LoggerMessage sour… Jun 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 a warning log in the .NET GitHub Copilot agent to use source-generated LoggerMessageAttribute logging, addressing CA1873 and aligning the log site with the repository’s existing strongly-typed logging patterns.

Changes:

  • Added a GitHubCopilotAgentLogMessages extension class with a LoggerMessageAttribute-based warning log method.
  • Updated GitHubCopilotAgent to call the new strongly-typed log method instead of logger.LogWarning(...).
Show a summary per file
File Description
dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgentLogMessages.cs Introduces the source-generated logging extension for the approval-gating warning.
dotnet/src/Microsoft.Agents.AI.GitHub.Copilot/GitHubCopilotAgent.cs Switches the warning log call site to use the new generated logging method.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Low

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Automated Code Review

Reviewers: 5 | Confidence: 95% | Result: All clear

Reviewed: Correctness, Security Reliability, Test Coverage, Failure Modes, Design Approach


Automated review by alliscode's agents

@alliscode alliscode added this pull request to the merge queue Jun 30, 2026
@alliscode alliscode removed this pull request from the merge queue due to a manual request Jun 30, 2026
@alliscode alliscode merged commit e59a31c into microsoft:main Jun 30, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

.NET Usage: [Issues, PRs], Target: .Net

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants