Ensure SSL_CERT_DIR messages are always shown and check for existing value

[tomerqodo]

Benchmark PR from agentic-review-benchmarks#3

[Copilot]

Pull request overview

This PR enhances the dotnet-dev-certs tool to ensure SSL_CERT_DIR configuration messages are always displayed (not just in verbose mode) and adds logic to check if SSL_CERT_DIR is already properly configured before suggesting changes.

Changes:

• Modified event listener initialization to enable LogAlways level events when not in verbose mode
• Added logic to check existing SSL_CERT_DIR environment variable and detect if the certificate directory is already included
• Added new event source methods for improved user messaging about SSL_CERT_DIR configuration
• Refactored NssDb class from primary constructor to explicit constructor

Reviewed changes

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

File	Description
src/Tools/dotnet-dev-certs/src/Program.cs	Creates event listener unconditionally and enables LogAlways level when not verbose; removes trailing whitespace
src/Shared/CertificateGeneration/UnixCertificateManager.cs	Adds SSL_CERT_DIR checking logic to detect existing configuration; refactors NssDb class constructor
src/Shared/CertificateGeneration/CertificateManager.cs	Removes outdated comment about verbose mode; improves event messages with proper quoting; adds new events for SSL_CERT_DIR status

[Copilot]

Using Path.GetFullPath(prettyCertDir) is incorrect because prettyCertDir may contain $HOME (a shell variable placeholder for display purposes), not an actual path. When certDir starts with the home directory, line 355 replaces it with $HOME, making prettyCertDir a display string like $HOME/.aspnet/dev-certs/trust. Calling Path.GetFullPath() on this will not work correctly as it doesn't expand shell variables.

Instead, use certDir (which is the actual full path) for comparison. The variable prettyCertDir should only be used for display in log messages.

Suggested change

	var certDirFullPath = Path.GetFullPath(prettyCertDir);
	var certDirFullPath = Path.GetFullPath(certDir);

[Copilot]

This change replaces a C# 12+ primary constructor with an explicit constructor and private fields. According to the coding guidelines, the repository uses C# 13 features (LangVersion is set to "preview" in eng/targets/CSharp.Common.props), and primary constructors are supported. The original primary constructor syntax private sealed class NssDb(string path, bool isFirefox) is more concise and idiomatic in modern C#. Unless there's a specific reason for this change (e.g., additional initialization logic needed), the primary constructor should be retained.

[Copilot]

The assignment sawTrustFailure = !hasValidSslCertDir; unconditionally overwrites any previous value of sawTrustFailure. This is problematic because if there was already a trust failure from earlier checks (e.g., NSS database trust failed at lines 322 or 328), but SSL_CERT_DIR is properly configured (hasValidSslCertDir = true), this line would reset sawTrustFailure to false, causing the method to incorrectly return TrustLevel.Full instead of TrustLevel.Partial.

To preserve previous failure state, use the OR-assignment operator: sawTrustFailure |= !hasValidSslCertDir;. This ensures that once a failure is detected, it remains true even if subsequent checks pass.

Suggested change

	sawTrustFailure = !hasValidSslCertDir;
	sawTrustFailure |= !hasValidSslCertDir;