Upgrade to .NET 10, update all dependencies, add xUnit v3 tests, fix bugs by alasdaircs · Pull Request #1 · alasdaircs/HybridConnectionClientProxy

alasdaircs · 2026-03-25T23:40:47Z

.NET and dependencies

Target framework: net9.0 → net10.0
All NuGet packages updated to latest stable versions (Azure.Core 1.51.1,
Azure.Identity 1.19.0, Microsoft.Azure.Relay 3.1.1, Microsoft.Extensions.*
10.0.x, Serilog 4.3.1/10.0.0/6.1.1, IdentityModel 8.16.0, etc.)
GitHub Actions: checkout/setup-dotnet updated to v4, dotnet-version 10.0.x

Bug fixes

CancellationTokenSource leak: _cts is now a local using var inside Run(),
so it is always disposed — previously the IDisposable was never called
because Create() discarded the instance.
TcpClient leak: ClientProxyConnection.Run() now disposes the TcpClient via
using var _ = tcpClient.
Orphaned pump: after Task.WhenAny one pump finished but the other was left
running indefinitely. Now CancelAsync() is called immediately after WhenAny
and both pumps are awaited, ensuring clean shutdown.
Console.WriteLine for errors replaced with Log.Error/Log.Debug (Serilog).
Case-sensitivity bug on Linux: ConfigurationLocator.DefaultsFile was
"appsettings.json" but the file on disk was "appSettings.json" (capital S).
Renamed to appsettings.json — would have crashed on the Ubuntu CI runner.
Program.cs: unnecessary else after throwing if removed; cts.Cancel() →
cts.CancelAsync(); exception type InvalidOperationException instead of Exception.
ConfigurationLocator.OverlayFile changed from static field (computed once at
class load) to a computed property, consistent with OverlayFilePath.

Testability / modernisation

New IHybridConnectionProvider interface and HybridConnectionClientProvider
implementation decouple the proxy from HybridConnectionClient, enabling
testing without live Azure credentials.
Internal overload ClientProxy.Create(IHybridConnectionProvider, TcpListener)
allows tests to supply their own started listener and inspect the actual port.
InternalsVisibleTo attribute added to main csproj for the test project.
Unused using directives removed; file-scoped namespaces; [] instead of
Array.Empty(); expression-bodied Proxy.ListenAddress; BCL keyword aliases.

Tests (new project: HybridConnectionClientProxy.Tests, xUnit v3)

ProxySettingsTests: Proxy.ListenAddress parsing (null, empty, invalid, IPv4,
IPv6, any).
ConfigurationLocatorTests: DefaultsFile constant, OverlayFileName/Path/File.
ClientProxyIntegrationTests (6 tests): full end-to-end proxy pipeline over
loopback TCP using TcpHybridConnectionProvider (no Azure needed):
client→backend, backend→client, bidirectional, multiple sequential
connections, graceful client disconnect, cancellation/shutdown.

https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

…bugs ## .NET and dependencies - Target framework: net9.0 → net10.0 - All NuGet packages updated to latest stable versions (Azure.Core 1.51.1, Azure.Identity 1.19.0, Microsoft.Azure.Relay 3.1.1, Microsoft.Extensions.* 10.0.x, Serilog 4.3.1/10.0.0/6.1.1, IdentityModel 8.16.0, etc.) - GitHub Actions: checkout/setup-dotnet updated to v4, dotnet-version 10.0.x ## Bug fixes - CancellationTokenSource leak: _cts is now a local `using var` inside Run(), so it is always disposed — previously the IDisposable was never called because Create() discarded the instance. - TcpClient leak: ClientProxyConnection.Run() now disposes the TcpClient via `using var _ = tcpClient`. - Orphaned pump: after Task.WhenAny one pump finished but the other was left running indefinitely. Now CancelAsync() is called immediately after WhenAny and both pumps are awaited, ensuring clean shutdown. - Console.WriteLine for errors replaced with Log.Error/Log.Debug (Serilog). - Case-sensitivity bug on Linux: ConfigurationLocator.DefaultsFile was "appsettings.json" but the file on disk was "appSettings.json" (capital S). Renamed to appsettings.json — would have crashed on the Ubuntu CI runner. - Program.cs: unnecessary else after throwing if removed; cts.Cancel() → cts.CancelAsync(); exception type InvalidOperationException instead of Exception. - ConfigurationLocator.OverlayFile changed from static field (computed once at class load) to a computed property, consistent with OverlayFilePath. ## Testability / modernisation - New IHybridConnectionProvider interface and HybridConnectionClientProvider implementation decouple the proxy from HybridConnectionClient, enabling testing without live Azure credentials. - Internal overload ClientProxy.Create(IHybridConnectionProvider, TcpListener) allows tests to supply their own started listener and inspect the actual port. - InternalsVisibleTo attribute added to main csproj for the test project. - Unused using directives removed; file-scoped namespaces; `[]` instead of Array.Empty<T>(); expression-bodied Proxy.ListenAddress; BCL keyword aliases. ## Tests (new project: HybridConnectionClientProxy.Tests, xUnit v3) - ProxySettingsTests: Proxy.ListenAddress parsing (null, empty, invalid, IPv4, IPv6, any). - ConfigurationLocatorTests: DefaultsFile constant, OverlayFileName/Path/File. - ClientProxyIntegrationTests (6 tests): full end-to-end proxy pipeline over loopback TCP using TcpHybridConnectionProvider (no Azure needed): client→backend, backend→client, bidirectional, multiple sequential connections, graceful client disconnect, cancellation/shutdown. https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

xUnit v3 does not inject a global 'using Xunit;' automatically. Adding <Using Include="Xunit" /> to the test csproj makes [Fact] and [Theory] available without requiring an explicit using in every test file. https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

Copilot

Pull request overview

This PR modernizes the HybridConnectionClientProxy app by upgrading to .NET 10 / latest dependencies, refactoring the proxy to be testable without Azure credentials, and fixing multiple shutdown/resource-leak issues.

Changes:

Upgrade to net10.0, update NuGet dependencies, and update GitHub Actions to install .NET 10.
Refactor proxy pipeline (provider abstraction, improved cancellation/disposal, improved logging) and fix connection/pump shutdown behavior.
Add a new xUnit v3 test project with unit + loopback integration tests.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
src/Settings/Proxy.cs	Modernize settings model; simplify `ListenAddress` parsing.
src/Settings/AppSettings.cs	Make `Proxies` non-null by default (`[]`).
src/Program.cs	Improve validation/logging and switch to `CancelAsync`.
src/IHybridConnectionProvider.cs	Introduce provider abstraction for testability.
src/HybridConnectionClientProvider.cs	Production provider backed by Azure Relay client.
src/ConfigurationLocator.cs	Fix filename casing and compute overlay file path dynamically.
src/ClientProxyConnection.cs	Fix CTS/TcpClient leaks; fix orphaned pump; use Serilog.
src/ClientProxy.cs	Refactor to provider + listener overload for tests; improve lifecycle handling.
appsettings.json	Add default Serilog configuration.
HybridConnectionClientProxy.sln	Add new test project to solution.
HybridConnectionClientProxy.csproj	Target `net10.0`, update packages, add `InternalsVisibleTo`, rename settings file item.
HybridConnectionClientProxy.Tests/TcpHybridConnectionProvider.cs	Test double that connects to local TCP backend.
HybridConnectionClientProxy.Tests/Settings/ProxySettingsTests.cs	Unit tests for listen-address parsing + defaults.
HybridConnectionClientProxy.Tests/HybridConnectionClientProxy.Tests.csproj	New test project with xUnit v3 + coverlet.
HybridConnectionClientProxy.Tests/ConfigurationLocatorTests.cs	Tests for config locator naming/path behavior.
HybridConnectionClientProxy.Tests/ClientProxyIntegrationTests.cs	Loopback end-to-end proxy pipeline integration tests.
.github/workflows/dotnet.yml	Update CI to actions v4 + .NET 10 SDK.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-25T23:45:06Z

+	}
+
+	public async Task<Stream> CreateConnectionAsync( CancellationToken cancellationToken = default )
+		=> await _client.CreateConnectionAsync();


CreateConnectionAsync accepts a CancellationToken but the token is currently unused, so cancelling the proxy won’t cancel an in-flight Azure Hybrid Connection connect/open. Consider either passing the token through to _client.CreateConnectionAsync(...) (if an overload exists) or explicitly honoring cancellation (e.g., ThrowIfCancellationRequested) before initiating the connection.

Suggested change

=> await _client.CreateConnectionAsync();

=> await _client.CreateConnectionAsync( cancellationToken );

Copilot · 2026-03-25T23:45:07Z

 var configurationSection = configurationManager.GetRequiredSection( AppSettings.Section );
 var appSettings = new AppSettings();
 ConfigurationBinder.Bind( configurationSection, appSettings );

 Log.Information( "Starting" );
+
+if( appSettings.Proxies.Length == 0 )
+	throw new InvalidOperationException( "You must specify at least one proxy in the configuration." );


ConfigurationManager.GetRequiredSection(AppSettings.Section) will throw if the defaults/overlay configuration doesn’t contain an AppSettings section. In this PR the new appsettings.json only configures Serilog, so the app will fail before reaching the clearer "must specify at least one proxy" validation. Either include an empty AppSettings section in the defaults file, or switch to GetSection and handle the missing section explicitly.

Copilot · 2026-03-25T23:45:07Z

+public class ConfigurationLocatorTests
+{
+	[Fact]
+	public void DefaultsFile_IsLowercaseAppsettingsJson()
+	{
+		// Verifies the case-safe lowercase filename convention (capital S caused failures on Linux)
+		Assert.Equal( "appsettings.json", ConfigurationLocator.DefaultsFile );
+	}
+
+	[Fact]
+	public void OverlayFileName_ContainsMachineName()
+	{


This file uses [Fact]/Assert but doesn’t import xUnit (no using Xunit; and there is no global using Xunit; in the test project). As-is, the test project won’t compile. Add a Usings.cs with global using Xunit; (or add using Xunit; to each test file).

Copilot · 2026-03-25T23:45:08Z

+public class ProxySettingsTests
+{
+	[Fact]
+	public void ListenAddress_WhenListenIPAddressIsNull_ReturnsLoopback()
+	{
+		var proxy = new Proxy { ListenIPAddress = null };
+		Assert.Equal( IPAddress.Loopback, proxy.ListenAddress );


This file uses [Fact]/Assert but doesn’t import xUnit (no using Xunit; and there is no global using Xunit; in the test project). As-is, the test project won’t compile. Add a Usings.cs with global using Xunit; (or add using Xunit; to each test file).

Copilot · 2026-03-25T23:45:08Z

+public class ClientProxyIntegrationTests
+{
+	private const int TestTimeoutMs = 10_000;
+
+	/// <summary>
+	/// Binds to port 0, starts the listener, and returns it together with the OS-assigned port.
+	/// </summary>
+	private static (TcpListener listener, int port) StartListener()
+	{
+		var listener = new TcpListener( IPAddress.Loopback, 0 );
+		listener.Start();
+		var port = ( (IPEndPoint) listener.LocalEndpoint ).Port;
+		return (listener, port);
+	}
+
+	[Fact]
+	public async Task Proxy_ClientToBackend_DataForwardedCorrectly()
+	{
+		using var cts = new CancellationTokenSource( TestTimeoutMs );
+


This file uses [Fact]/Assert but doesn’t import xUnit (no using Xunit; and there is no global using Xunit; in the test project). As-is, the test project won’t compile. Add a Usings.cs with global using Xunit; (or add using Xunit; to each test file).

Copilot · 2026-03-25T23:45:08Z

+if( appSettings.Proxies.Length == 0 )
+	throw new InvalidOperationException( "You must specify at least one proxy in the configuration." );
+
 var cts = new CancellationTokenSource();


CancellationTokenSource implements IDisposable; in Main it’s created but never disposed. Consider using var cts = new CancellationTokenSource(); so the underlying timer/wait-handle resources are released promptly, especially since the app can run for long periods.

Suggested change

var cts = new CancellationTokenSource();

using var cts = new CancellationTokenSource();

Build fix: - Add GlobalUsings.cs with 'global using Xunit;' — the previous approach of <Using Include="Xunit" /> in the csproj was not being honoured by the SDK and the generated GlobalUsings.g.cs did not include the Xunit namespace. - Remove the now-redundant <Using Include="Xunit" /> from the test csproj. Copilot review fixes: - HybridConnectionClientProvider: pass cancellationToken through to _client.CreateConnectionAsync() so cancellation is honoured during in-flight Azure Relay connection setup. - appsettings.json: add empty AppSettings section so GetRequiredSection succeeds on a clean run; without it the app threw a cryptic InvalidOperationException before the friendly validation message. - Program.cs: use 'using var cts' so CancellationTokenSource is disposed promptly when the application exits. https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

xunit.v3 3.2.2 now depends on xunit.v3.mtp-v1 (Microsoft Testing Platform), which uses a completely different runner model from VSTest. The MTP integration requires EnableMicrosoftTestingPlatformRunner and does not provide compile-time assembly references through the standard dotnet test / VSTest pipeline, causing 'The type or namespace Xunit could not be found' at compile time. xunit v2 (2.9.3) is the latest stable release that works with the standard dotnet test workflow. The test API ([Fact], [Theory], Assert.*) is identical, so no test code changes are needed. GlobalUsings.cs (global using Xunit;) now resolves correctly because the xunit v2 assembly is a standard compile reference. https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

Microsoft.NET.Test.Sdk 18.x changed how compile-time assembly references are handled as part of its Microsoft Testing Platform (MTP) integration. This broke Xunit namespace resolution for both xunit.v2 and xunit.v3 — the package DLLs were not being added as compile references. Downgrading to 17.12.0 restores the standard VSTest / dotnet-test behaviour. Replace GlobalUsings.cs (global using Xunit;) with explicit 'using Xunit;' in each test file. The global-using file approach depended on the package correctly injecting the namespace, which was unreliable across SDK versions. Explicit usings are unconditional and don't rely on any package-level magic. CI: change 'dotnet test --no-build' to 'dotnet test --no-restore'. The --no-build flag skips the test-project build entirely, so if the Build step produced no test binary (e.g. due to a project-discovery issue), the Test step had nothing to run and failed silently. --no-restore keeps the packages from being downloaded twice while still ensuring the binary is always built. https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

Root cause of all xunit build failures: HybridConnectionClientProxy.csproj was at the repo root, so its default SDK glob (implicit **/*.cs) swept up every .cs file in the entire repository — including all the test files. The test files reference [Fact] from xunit, which is not a dependency of the main project, causing 'Xunit namespace not found' in the *main* project build regardless of how the test project was configured. Fix: each project now lives in its own subfolder so each project's implicit glob only covers its own source files. Layout after this commit: HybridConnectionClientProxy/ <- main project (was at root) HybridConnectionClientProxy.csproj appsettings.json config/ src/ HybridConnectionClientProxy.Tests/ HybridConnectionClientProxy.Tests.csproj src/ <- test sources (were at project root) ClientProxyIntegrationTests.cs ConfigurationLocatorTests.cs TcpHybridConnectionProvider.cs Settings/ProxySettingsTests.cs HybridConnectionClientProxy.slnx <- replaces HybridConnectionClientProxy.sln Other changes: - HybridConnectionClientProxy.sln removed; replaced by HybridConnectionClientProxy.slnx (Visual Studio XML solution format, much simpler than the old .sln format) - ProjectReference in test project updated to new relative path - .github/workflows reference in main csproj updated (../.github/...) - CI workflow: all dotnet commands now specify HybridConnectionClientProxy.slnx explicitly so there is no ambiguity about which project/solution to use https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

HybridConnectionClient.CreateConnectionAsync takes an optional IDictionary<string,string> of custom headers, not a CancellationToken. The Copilot suggestion to forward the token was incorrect. Reverted to the parameterless call (matches original behaviour). Restore xunit.v3 3.2.2 + xunit.runner.visualstudio 3.1.5. Every previous 'Xunit namespace not found' error was caused by the test .cs files being compiled as part of the main project (root-level csproj glob). Now that each project lives in its own folder that issue is gone and xunit.v3 can be used directly as intended. https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

Previously downgraded to 17.12.0 while debugging the xunit build failures. Those failures were caused by the project structure (root-level csproj glob picking up test files), not by this package. Now that CI is green, restore the latest version. https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

HybridConnectionStream.ReadAsync does not reliably unblock when its CancellationToken is cancelled. After signalling cancellation, explicitly close both the TCP stream and the hybrid connection stream so that any blocked ReadAsync call throws immediately (SocketException 995) rather than hanging until the Azure Relay times the connection out. https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

After WhenAny returns and both streams are explicitly closed, any exception from the pumps is caused by the close itself (RelayException wrapping SocketException 995) and is always expected. Silently catch both rather than logging misleading "Receive pump closed with error" messages. https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

- Write APPSETTINGS secret to appsettings.Test.json before build in CI - Copy that file to test output via conditional CopyToOutputDirectory - AzureIntegrationTests reads the connection string via System.Text.Json and calls Skip.If when the file is absent (fork PRs, local runs) - Three tests: connection established, disconnect cleanup, data flow https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

The backend behind the relay is smtp4dev, so replace the generic PING/echo test with a proper SMTP exchange: read the 220 greeting, send EHLO, assert a 250 response, then QUIT cleanly. ReadSmtpResponseAsync handles multi-line responses (NNN- continuation). https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

Pass the secret as an env var and guard the write with a shell if [ -n "$APPSETTINGS" ] test instead. https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

Skip is not in the Xunit namespace in xunit.v3 3.2.2; Assert.Skip() is the correct API from xunit.v3.assert. https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

alasdaircs requested a review from Copilot March 25, 2026 23:41

Copilot started reviewing on behalf of alasdaircs March 25, 2026 23:41 View session

Copilot AI reviewed Mar 25, 2026

View reviewed changes

claude added 14 commits March 25, 2026 23:56

Trigger CI with APPSETTINGS secret

44d87f5

https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

Fix workflow: secrets context not valid in step if condition

cb47e22

Pass the secret as an env var and guard the write with a shell if [ -n "$APPSETTINGS" ] test instead. https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

Fix: use Assert.Skip() instead of Skip.If() for xunit.v3

d806100

Skip is not in the Xunit namespace in xunit.v3 3.2.2; Assert.Skip() is the correct API from xunit.v3.assert. https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

Trigger CI re-run (SMTP server was down)

b767659

https://claude.ai/code/session_014Jr8HbUyCyqUmUguYgHVfF

alasdaircs merged commit 3b11898 into master Mar 26, 2026
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to .NET 10, update all dependencies, add xUnit v3 tests, fix bugs - #1

Upgrade to .NET 10, update all dependencies, add xUnit v3 tests, fix bugs#1
alasdaircs merged 16 commits into
masterfrom
claude/dotnet-10-upgrade-awoBh

alasdaircs commented Mar 25, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Copilot AI Mar 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	=> await _client.CreateConnectionAsync();
	=> await _client.CreateConnectionAsync( cancellationToken );

	var cts = new CancellationTokenSource();
	using var cts = new CancellationTokenSource();

Conversation

alasdaircs commented Mar 25, 2026

.NET and dependencies

Bug fixes

Testability / modernisation

Tests (new project: HybridConnectionClientProxy.Tests, xUnit v3)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants