Refactor PowerForge Studio orchestration onto shared host services

[PrzemyslawKlys]

Summary

• move Studio build, publish, signing, verification, git, and queue orchestration onto shared PowerForge C# host services
• add reusable PowerForge process, git, dotnet nuget, project/module build, signing, repository, and verification service boundaries plus focused tests
• thin Studio queue/state/storage helpers and document the regroup direction for keeping cmdlets thin and Studio cmdlet-free

Verification

• dotnet build .\PowerForgeStudio.Orchestrator\PowerForgeStudio.Orchestrator.csproj -c Release --no-restore
• dotnet test .\PowerForgeStudio.Tests\PowerForgeStudio.Tests.csproj -c Release --filter "FullyQualifiedName~PowerForgeStudioStateDatabaseTests|FullyQualifiedName~PowerForgeStudioQueueCommandStateServiceTests|FullyQualifiedName~PowerForgeStudioReleaseQueueCommandServiceTests|FullyQualifiedName~PowerForgeStudioQueueSessionFactoryTests|FullyQualifiedName~PowerForgeStudioQueueTargetProjectionServiceTests" --no-restore

[claude]

PR 212 Review - Refactor PowerForge Studio orchestration onto shared host services

This is a substantial refactoring (13,530 additions / 3,475 deletions) that moves Studio build, signing, git, and housekeeping orchestration onto well-structured shared C# host services. The architecture is clean and the test coverage is solid. A few items worth addressing before merge.

BUGS / CORRECTNESS

1. Sync-over-async anti-pattern in GitHubActionsCacheCleanupService

TryGetUsage() and ListCaches() use .GetAwaiter().GetResult() to block on async HTTP calls. In a CLI context this will not deadlock, but it is a recognised anti-pattern that can cause thread-pool starvation. Since Prune() is already synchronous, prefer HttpClient.Send() (synchronous overload available since .NET 5) to keep the sync design without the blocking workaround.

2. Task.Run does not propagate cancellation in ModuleBuildHostService

RunCommandAsync wraps _powerShellRunner.Run() in Task.Run(..., cancellationToken). The token only prevents the task from starting; once Run is in-flight the 15-minute build cannot be cancelled. Either document this in the XML doc or plumb a CancellationToken into IPowerShellRunner.Run so the underlying process can be killed on cancellation.

3. RunnerHousekeepingService.Clean() has no CancellationToken

The service runs docker prune, directory walks, and dotnet nuget locals clear, but Clean() accepts no cancellation. GitClient already propagates CancellationToken consistently; adding it here keeps the API surface uniform.

CODE QUALITY

4. NormalizedSpec classes use mutable setters

Both inner NormalizedSpec classes are created once in NormalizeSpec() and never mutated. Using init-only properties makes the immutability intent explicit and catches accidental mutation at compile time.

5. GitHubActionsCacheCleanupItem mutated after construction

After building orderedPlanned, the loop mutates item.DeleteStatusCode and item.DeleteError in place. This is unexpected for result objects. Consider a separate DeleteFailureRecord wrapper rather than mutating plan items.

6. Duplicate constructor assignments in PowerShellRunRequest

The public file-based constructor and the private full constructor repeat the same property assignments. The public constructor could delegate to the private one.

SECURITY

7. -GitHubAccessToken plain string in Build-PowerForge.ps1

Confirm that $GitHubAccessToken is only ever forwarded as --token to the CLI process via structured arguments and is never interpolated into a log message or Write-Host call.

8. powerforge-ref defaults to main in the reusable workflow

Callers that omit powerforge-ref will always track the latest main. A future breaking change to the action would silently affect all callers. Pinning to a commit SHA or release tag is standard supply-chain hygiene, even for first-party workflows.

PERFORMANCE

9. CLI rebuilt on every housekeeping action run

action.yml runs dotnet build on every invocation. Consider caching the build output with actions/cache keyed on the global.json SDK version and a hash of PowerForge.Cli.csproj.

MINOR OBSERVATIONS

• QuoteLiteral in ModuleBuildHostService correctly doubles single-quotes for PowerShell string escaping. Good.
• ProcessRunner uses structured ArgumentList rather than a raw string, preventing shell injection.
• GitClient propagates CancellationToken throughout; worth following in the housekeeping services too.
• Test coverage is good across all new services with constructor-injected fakes.
• The aggressiveApplied flag being evaluated once before cleanup steps run is the right approach.

SUMMARY

The core abstractions (IProcessRunner, GitClient, service boundaries) are well-designed and most items above are polish rather than blockers. Items 1 (sync-over-async) and 8 (ref pinning) are the highest priority to address before merge.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.