Skip to content

Refactor CopilotClientInfoLogger to minimize Reflection usage#38

Merged
SIkebe merged 8 commits intomainfrom
copilot/refactor-client-info-logger
Jan 23, 2026
Merged

Refactor CopilotClientInfoLogger to minimize Reflection usage#38
SIkebe merged 8 commits intomainfrom
copilot/refactor-client-info-logger

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 23, 2026 •
edited
Loading

Replaces Reflection-based access to SDK internals with public APIs from github/copilot-sdk#77 where possible.

Changes

CopilotClientInfoLogger Refactoring

  • Options access: Pass CopilotClientOptions as parameter instead of reflecting on _options field
  • Status info: Use GetStatusAsync() API for CLI version and protocol version (new output)
  • Process info: Retain Reflection for PID, command, arguments (SDK lacks public API)
// Before: Reflection
var options = GetFieldValue<object>(client, "_options");
var cliPath = GetPropertyValue<string>(options, "CliPath");

// After: Direct parameter access + public API
public async Task LogConnectionInfoAsync(CopilotClient client, CopilotClientOptions options)
{
    _logger.LogInformation("CLI Path: {CliPath}", options.CliPath ?? "(auto-detected)");
    var status = await client.GetStatusAsync();
    _logger.LogInformation("CLI Version: {Version}", status.Version);
}

E2E Test Visibility

  • Add --logger "console;verbosity=detailed" to workflow for GitHub Actions log output

Custom Instructions

  • Add convention: #region / #endregion not used
Original prompt

CopilotClientInfoLogger でなるべく Reflection を使わないように、github/copilot-sdk#77 で追加された API を中心に検討・修正してください。どうしても Reflection を使わなければいけないものだけ残してください。

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

@SIkebe
Refactor CopilotClientInfoLogger to minimize Reflection usage
a5bd1c1 
- Use CopilotClientOptions parameter instead of reflection to access options
- Use GetStatusAsync() API to get CLI version and protocol version info
- Removed all Reflection usage from the logger
- Simplified constructor to only require ILogger dependency
- Changed interface method from sync to async (LogConnectionInfoAsync)
- Updated App.cs to pass options and use async method
- Updated unit tests accordingly

Co-authored-by: SIkebe <17608272+SIkebe@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor CopilotClientInfoLogger to minimize Reflection usage Refactor CopilotClientInfoLogger to eliminate Reflection usage Jan 23, 2026
Copilot AI requested a review from SIkebe January 23, 2026 02:34
@SIkebe SIkebe marked this pull request as ready for review January 23, 2026 02:36
Copilot AI review requested due to automatic review settings January 23, 2026 02:36
@SIkebe
Copy link
Copy Markdown
Contributor

SIkebe commented Jan 23, 2026

@copilot Reflection では出力されていたが、Reflection を使わない新しい実装では出力されなくなっている情報がないかチェックしてください。

@SIkebe
Restore process info logging with Reflection (SDK lacks public API)
7aaf21c 
- Keep options logging via passed CopilotClientOptions (no Reflection)
- Keep GetStatusAsync() for CLI version info (no Reflection)
- Restore process info logging (Reflection required - no SDK API):
  - CLI Process ID
  - CLI Command
  - CLI Arguments
  - Resolved CLI Path
- Add comments documenting which Reflection is unavoidable

Co-authored-by: SIkebe <17608272+SIkebe@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 23, 2026

@copilot Reflection では出力されていたが、Reflection を使わない新しい実装では出力されなくなっている情報がないかチェックしてください。

確認しました。以下のプロセス関連情報が欠落していたため、復元しました (7aaf21c):

欠落していた情報（Reflection で復元）:

  • CLI Process ID
  • CLI Command
  • CLI Arguments
  • Resolved CLI Path

これらは SDK に公開 API がないため、Reflection を維持する必要があります。#region Reflection Helpers としてコメントを追加し、理由を明記しました。

最終的な出力情報:

情報 方法
Configured CLI Path Options パラメータ
Configured Port/UseStdio Options パラメータ
CLI Version GetStatusAsync() (新規)
Protocol Version GetStatusAsync() (新規)
CLI Process ID Reflection (SDK API なし)
CLI Command Reflection (SDK API なし)
CLI Arguments Reflection (SDK API なし)
Resolved CLI Path Reflection (SDK API なし)

@SIkebe
Copy link
Copy Markdown
Contributor

SIkebe commented Jan 23, 2026

@copilot E2E テストの CLI の出力結果が GitHub Actions のログでも確認できるようにしてください。

@SIkebe
Add Console output to E2E tests for GitHub Actions visibility
77e0f37 
Write CLI output to both ITestOutputHelper and Console so that
E2E test results are visible in GitHub Actions logs.

Co-authored-by: SIkebe <17608272+SIkebe@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 23, 2026

@copilot E2E テストの CLI の出力結果が GitHub Actions のログでも確認できるようにしてください。

ProcessResultExtensions.LogTo メソッドを更新し、ITestOutputHelper に加えて Console.WriteLine でも出力するようにしました (77e0f37)。これにより、E2E テストの CLI 出力が GitHub Actions のログに表示されます。

Copilot AI changed the title Refactor CopilotClientInfoLogger to eliminate Reflection usage Refactor CopilotClientInfoLogger to minimize Reflection usage Jan 23, 2026
@SIkebe
Copy link
Copy Markdown
Contributor

SIkebe commented Jan 23, 2026

@copilot 出ていませんよ

@SIkebe
Enable detailed console logging for E2E tests in GitHub Actions
481a6e5 
Add --logger "console;verbosity=detailed" to the E2E test command in
the workflow to ensure CLI output is visible in GitHub Actions logs.

Co-authored-by: SIkebe <17608272+SIkebe@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 23, 2026

@copilot 出ていませんよ

ワークフローの E2E テストコマンドに --logger "console;verbosity=detailed" を追加しました (481a6e5)。これにより、ITestOutputHelper に出力された内容が GitHub Actions のコンソールログに表示されます。

@SIkebe
Copy link
Copy Markdown
Contributor

SIkebe commented Jan 23, 2026

@copilot 出るようになりました。ならそもそも 77e0f37 の変更は不要だったんじゃない？

@SIkebe
Remove redundant Console.WriteLine from ProcessResultExtensions
8b59c6f 
The --logger "console;verbosity=detailed" in the workflow is sufficient
to make ITestOutputHelper output visible in GitHub Actions logs.

Co-authored-by: SIkebe <17608272+SIkebe@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 23, 2026

@copilot 出るようになりました。ならそもそも 77e0f37 の変更は不要だったんじゃない？

その通りです。Console.WriteLine の追加は不要でした。--logger "console;verbosity=detailed" だけで ITestOutputHelper の出力が GitHub Actions ログに表示されます。77e0f37 の変更を取り消しました (8b59c6f)。

@SIkebe
Copy link
Copy Markdown
Contributor

SIkebe commented Jan 23, 2026

@copilot #region　は使わないで

@SIkebe
Remove #region directives from CopilotClientInfoLogger
16cde26 
Co-authored-by: SIkebe <17608272+SIkebe@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jan 23, 2026

@copilot #region　は使わないで

#region と #endregion を削除しました (16cde26)。

@SIkebe
Add #region prohibition to Custom Instructions
49f0d76 
Document convention to not use #region/#endregion directives in C# code.

Co-authored-by: SIkebe <17608272+SIkebe@users.noreply.github.com>
@SIkebe SIkebe merged commit 3fba080 into main Jan 23, 2026
7 checks passed
@SIkebe SIkebe deleted the copilot/refactor-client-info-logger branch January 23, 2026 03:36
Copilot AI review requested due to automatic review settings March 23, 2026 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@SIkebe SIkebe SIkebe approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@SIkebe SIkebe

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@SIkebe