Add unit tests and extract startup form logic into testable services

[Copilot]

Addresses review feedback to add unit test coverage for startup form functionality and refactor logic into DI-injectable services for easier testing.

Changes

• New test project: ProjectCommander.UnitTests with xUnit 2.9.3 (v3 not yet stable) covering:

  • Startup form state management and completion
  • WaitForStartupFormAsync blocking behavior
  • Form validation and error handling
  • Resource name parsing (e.g., "datagenerator-abc123" → "datagenerator")

• Service extraction:

  • IStartupFormService: Encapsulates startup form state and operations
  • IResourceNameParser: Handles resource name string parsing logic

• DI integration: Updated AspireProjectCommanderClientWorker and ProjectCommanderHub to consume services via constructor injection

Example

// Before: Logic embedded in worker
_isStartupFormRequired = Environment.GetEnvironmentVariable("...") == "true";

// After: Service-based approach
startupFormService.SetStartupFormRequired(isRequired);

All 17 tests pass. No Aspire integration testing used.

---

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

[Copilot]

Pull request overview

Refactors startup form handling and resource-name parsing into injectable services to improve testability, and adds a new xUnit unit test project to cover the extracted logic.

Changes:

• Introduces IStartupFormService + StartupFormService and wires it into the client worker via DI.
• Introduces IResourceNameParser + ResourceNameParser and injects it into ProjectCommanderHub.
• Adds ProjectCommander.UnitTests with unit tests for startup form service behavior and resource name parsing.

Reviewed changes

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

Show a summary per file

File	Description
Src/Nivot.Aspire.ProjectCommander/StartupFormService.cs	New service encapsulating startup form state + async wait behavior.
Src/Nivot.Aspire.ProjectCommander/IStartupFormService.cs	Public interface for startup form operations/state.
Src/Nivot.Aspire.ProjectCommander/AspireProjectCommanderClientWorker.cs	Replaces embedded startup-form logic with injected service.
Src/Nivot.Aspire.ProjectCommander/ServiceCollectionAspireProjectCommanderExtensions.cs	Registers IStartupFormService in DI.
Src/Nivot.Aspire.ProjectCommander/Nivot.Aspire.ProjectCommander.csproj	Adds InternalsVisibleTo for unit testing and mocking.
Src/Nivot.Aspire.Hosting.ProjectCommander/ResourceNameParser.cs	New parser service for deriving “base” resource names.
Src/Nivot.Aspire.Hosting.ProjectCommander/IResourceNameParser.cs	Public interface for resource name parsing.
Src/Nivot.Aspire.Hosting.ProjectCommander/ProjectCommanderHub.cs	Injects and uses IResourceNameParser instead of inline splitting.
Src/Nivot.Aspire.Hosting.ProjectCommander/ProjectCommanderHubResource.cs	Registers IResourceNameParser in the hub host DI container.
Src/Nivot.Aspire.Hosting.ProjectCommander/Nivot.Aspire.Hosting.ProjectCommander.csproj	Adds InternalsVisibleTo for unit testing and mocking.
ProjectCommander.UnitTests/StartupFormServiceTests.cs	Adds unit tests for startup-form state and wait/cancel behavior.
ProjectCommander.UnitTests/ResourceNameParserTests.cs	Adds unit tests for resource name parsing behavior.
ProjectCommander.UnitTests/ProjectCommander.UnitTests.csproj	New unit test project with xUnit + Moq dependencies and project refs.
ProjectCommander.sln	Adds the new unit test project to the solution.

---

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

[Copilot]

StartupFormService is registered as a singleton and its state (_isStartupFormRequired, _isStartupFormCompleted, _startupFormData) can be read/written from different threads (SignalR callbacks vs. callers awaiting). Consider making the state updates thread-safe (e.g., locking or using Volatile/Interlocked) and creating the TaskCompletionSource with TaskCreationOptions.RunContinuationsAsynchronously to avoid running continuations inline on the completing thread.

[Copilot]

The Task.Delay(Timeout.Infinite, ...) + Task.WhenAny(...) pattern is heavier than needed and leaves an extra task pending until cancellation. Since this targets modern .NET, consider awaiting the completion task with cancellation directly (e.g., WaitAsync(cancellationToken)) to reduce allocations and simplify the flow.

Suggested change

	// Wait for the form to be completed
	using var cts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
	var completedTask = await Task.WhenAny(
	_completionSource.Task,
	Task.Delay(Timeout.Infinite, cts.Token));
	if (completedTask == _completionSource.Task)
	{
	return await _completionSource.Task;
	}
	// Cancelled
	cancellationToken.ThrowIfCancellationRequested();
	return null;
	// Wait for the form to be completed with cancellation support
	return await _completionSource.Task.WaitAsync(cancellationToken);

[Copilot]

GetBaseResourceName splits on the first hyphen, which breaks resource lookup when the base Aspire resource name itself contains hyphens. For example, a project named eventhub-datagenerator will report itself as eventhub-datagenerator-<suffix> and this implementation would incorrectly return eventhub. Consider stripping only the replica suffix by splitting on the last hyphen (or otherwise detecting/removing the OTEL suffix) so base names with hyphens remain intact.

Suggested change

	// Split on first hyphen to extract base name
	// Example: "datagenerator-abc123" -> "datagenerator"
	var parts = resourceName.Split('-', 2);
	return parts[0];
	// Strip only the replica/OTEL suffix by removing the segment after the last hyphen.
	// Examples:
	// "datagenerator-abc123" -> "datagenerator"
	// "eventhub-datagenerator-abc123" -> "eventhub-datagenerator"
	var lastHyphenIndex = resourceName.LastIndexOf('-');
	if (lastHyphenIndex <= 0)
	{
	// No hyphen or hyphen at the start; treat the whole string as the base name.
	return resourceName;
	}
	return resourceName[..lastHyphenIndex];

[Copilot]

These test expectations lock in the current “split on first hyphen” behavior, which will fail for valid base resource names containing hyphens (e.g., my-service-12345 would parse to my). Once the parser is updated to strip only the replica suffix (typically after the last hyphen), update these InlineData expectations accordingly.

Suggested change

	[InlineData("my-service-12345", "my")]
	[InlineData("singlename", "singlename")]
	[InlineData("resource-with-multiple-hyphens-123", "resource")]
	[InlineData("my-service-12345", "my-service")]
	[InlineData("singlename", "singlename")]
	[InlineData("resource-with-multiple-hyphens-123", "resource-with-multiple-hyphens")]

[Copilot]

Disposable 'CancellationTokenSource' is created but not disposed.

Suggested change

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