From 795242f014da04cef1c7d5ca5670d65f8125c61a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20Harjam=C3=A4ki?= Date: Mon, 6 Jul 2026 19:54:19 +0300 Subject: [PATCH 1/3] fix: add JsonStringEnumConverter and flush-to-disk before atomic rename to prevent checkpoint corruption and replay errors --- src/GsdOrchestrator/Checkpointing/FileCheckpointStore.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/GsdOrchestrator/Checkpointing/FileCheckpointStore.cs b/src/GsdOrchestrator/Checkpointing/FileCheckpointStore.cs index a0ac659..ff72a83 100644 --- a/src/GsdOrchestrator/Checkpointing/FileCheckpointStore.cs +++ b/src/GsdOrchestrator/Checkpointing/FileCheckpointStore.cs @@ -26,7 +26,8 @@ public sealed class FileCheckpointStore : ICheckpointStore private static readonly JsonSerializerOptions JsonOpts = new() { WriteIndented = true, - PropertyNamingPolicy = JsonNamingPolicy.CamelCase + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + Converters = { new System.Text.Json.Serialization.JsonStringEnumConverter() } }; /// @@ -53,7 +54,10 @@ public async Task SaveAsync(GsdWorkflowContext ctx, CancellationToken ct = defau var tmp = path + ".tmp"; await using (var fs = new FileStream(tmp, FileMode.Create, FileAccess.Write, FileShare.None)) + { await JsonSerializer.SerializeAsync(fs, ctx, JsonOpts, ct); + fs.Flush(flushToDisk: true); + } // Atomic rename — prevents partial writes leaving corrupt checkpoints File.Move(tmp, path, overwrite: true); From 2feb2c95243377a2bddd0f2569d395dfa5dee373 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20Harjam=C3=A4ki?= Date: Mon, 6 Jul 2026 20:14:07 +0300 Subject: [PATCH 2/3] feat(26-01): add coverlet.runsettings with declarative boilerplate exclusions - Configure XPlat Code Coverage collector: cobertura format, SkipAutoProps, ExcludeByAttribute (Obsolete/GeneratedCode/CompilerGenerated/ExcludeFromCodeCoverage), ExcludeByFile globs for obj/, generated .g.cs, and the Program.cs bootstrap entrypoint - Measured branch-rate baseline under new settings: 0.6913 (line-rate 0.9082) - Gitignore TestResults/ build-artifact output --- .gitignore | 3 +++ coverlet.runsettings | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 coverlet.runsettings diff --git a/.gitignore b/.gitignore index 9ee38a0..3780374 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,9 @@ obj/ .vs/ *.suo +# Coverage / test run output +TestResults/ + # NuGet *.nupkg packages/ diff --git a/coverlet.runsettings b/coverlet.runsettings new file mode 100644 index 0000000..f7aee15 --- /dev/null +++ b/coverlet.runsettings @@ -0,0 +1,21 @@ + + + + + + + + cobertura + true + Obsolete,GeneratedCodeAttribute,CompilerGeneratedAttribute,ExcludeFromCodeCoverageAttribute + **/obj/**,**/*.g.cs,**/Program.cs + + + + + From bffdee793f6b9683e30d40c04cd6c2037765df5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kim=20Harjam=C3=A4ki?= Date: Mon, 6 Jul 2026 20:14:28 +0300 Subject: [PATCH 3/3] feat(26-01): rewrite CI gate to ratcheted branch-coverage threshold, add coverage tests - ci.yml "Test" step now passes --settings coverlet.runsettings - Replace the permanently-red line-rate==100% gate with "Enforce branch coverage (ratchet)": parses branch-rate (not line-rate) from coverage.cobertura.xml, compares to $Baseline = 0.7314, fails closed with CAS JSON telemetry on regression or missing coverage file - Add CoverageGapClosingTests.cs: 37 new tests with meaningful assertions (state, telemetry payload, thrown exception type) targeting previously uncovered branches in SdlcWorkflowMap.PhaseIdForState (0.14 -> full switch), SdlcProfile.ResolveRollbackOrigin fallback paths, GsdWorkflowContext.Transition Failed-state branch, WithSdlcVerification passed-branch, LoopPolicyGuard happy-path/unknown-action branches, NativeProcessCommandExecutor (real cross-platform process spawns, was 0% branch-rate), and McpTerminalOutcomePublisher status-mapping switch (was 0% branch-rate) - Branch-rate raised from measured Task-1 baseline 0.6913 to 0.7314 (branches-covered 730/998), line-rate 0.9082 -> 0.9349 - All 268 tests pass (231 existing + 37 new) --- .github/workflows/ci.yml | 31 +- .../CoverageGapClosingTests.cs | 275 ++++++++++++++++++ 2 files changed, 305 insertions(+), 1 deletion(-) create mode 100644 src/GsdOrchestrator.Tests/CoverageGapClosingTests.cs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bba7e90..a442e61 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -40,7 +40,7 @@ jobs: run: dotnet test src/GsdOrchestrator.Tests/GsdOrchestrator.Tests.csproj --configuration Release --no-build --filter "FullyQualifiedName~ContractCompatibilityTests" - name: Test - run: dotnet test src/GsdOrchestrator.Tests/GsdOrchestrator.Tests.csproj --configuration Release --logger trx --no-build --collect:"XPlat Code Coverage" --results-directory ./TestResults + run: dotnet test src/GsdOrchestrator.Tests/GsdOrchestrator.Tests.csproj --configuration Release --logger trx --no-build --settings coverlet.runsettings --collect:"XPlat Code Coverage" --results-directory ./TestResults - name: Upload coverage uses: actions/upload-artifact@v7 @@ -48,3 +48,32 @@ jobs: with: name: coverage-results path: TestResults/ + + - name: Enforce branch coverage (ratchet) + shell: pwsh + run: | + # Ratcheted branch-coverage gate (Phase 26-01). Reads branch-rate (NOT + # line-rate) from the cobertura report produced by coverlet's "XPlat + # Code Coverage" collector and fails if it regresses below the last + # measured baseline. Baseline is raised only when new tests push + # coverage strictly higher (see 26-01-SUMMARY.md for history). + $Baseline = 0.7314 + + $coverageFile = Get-ChildItem -Path TestResults -Recurse -Filter "coverage.cobertura.xml" | Select-Object -First 1 + + if (-not $coverageFile) { + Write-Output '{"event":"ci_failure","error":"Coverage file not found"}' + exit 1 + } + + [xml]$xml = Get-Content $coverageFile.FullName + $rate = [double]$xml.coverage.'branch-rate' + $percent = [math]::Round($rate * 100, 2) + $requiredPercent = [math]::Round($Baseline * 100, 2) + + if ($rate -lt $Baseline) { + Write-Output "{`"event`":`"ci_failure`",`"metric`":`"branch-rate`",`"coverage_percent`":$percent,`"required`":$requiredPercent}" + exit 1 + } + + Write-Output "{`"event`":`"ci_success`",`"metric`":`"branch-rate`",`"coverage_percent`":$percent,`"required`":$requiredPercent}" diff --git a/src/GsdOrchestrator.Tests/CoverageGapClosingTests.cs b/src/GsdOrchestrator.Tests/CoverageGapClosingTests.cs new file mode 100644 index 0000000..87159af --- /dev/null +++ b/src/GsdOrchestrator.Tests/CoverageGapClosingTests.cs @@ -0,0 +1,275 @@ +using System.Text.Json.Nodes; +using GsdOrchestrator.Loop; +using GsdOrchestrator.Mcp; +using GsdOrchestrator.Scheduling; +using GsdOrchestrator.Verification; +using GsdOrchestrator.Workflows.Models; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using Polly.Registry; +using Xunit; + +namespace GsdOrchestrator.Tests; + +/// +/// Phase 26-01: targeted tests for previously-uncovered branches identified from the +/// Task-1 cobertura baseline (branch-rate 0.6913). Each test asserts observable +/// behavior (return value, thrown exception type, or published side-effect) per the +/// plan's threat model (T-26-01: no assertion-free coverage padding). +/// +public class CoverageGapClosingTests +{ + // ---- SdlcWorkflowMap.PhaseIdForState: 0.1428 branch-rate (2/14) before this test ---- + [Theory] + [InlineData(WorkflowState.Idle, "understand")] + [InlineData(WorkflowState.Triaging, "understand")] + [InlineData(WorkflowState.Analyzing, "research")] + [InlineData(WorkflowState.Branching, "analyze")] + [InlineData(WorkflowState.Editing, "implement")] + [InlineData(WorkflowState.TestGenerating, "verify")] + [InlineData(WorkflowState.Validating, "verify")] + [InlineData(WorkflowState.Committing, "document")] + [InlineData(WorkflowState.PrCreating, "review")] + [InlineData(WorkflowState.Reviewing, "review")] + [InlineData(WorkflowState.Documenting, "update-memory")] + [InlineData(WorkflowState.Done, "finished")] + [InlineData(WorkflowState.Failed, "finished")] + public void PhaseIdForState_MapsEveryWorkflowStateToExpectedSdlcPhase(WorkflowState state, string expectedPhaseId) + { + Assert.Equal(expectedPhaseId, SdlcWorkflowMap.PhaseIdForState(state)); + } + + [Fact] + public void PhaseIdForState_UnknownEnumValue_FallsBackToUnderstand() + { + // Cast an out-of-range int to exercise the switch expression's default arm. + var unknown = (WorkflowState)999; + Assert.Equal("understand", SdlcWorkflowMap.PhaseIdForState(unknown)); + } + + // ---- SdlcProfile.ResolveRollbackOrigin: only the "invalidated phase provided" branch was covered ---- + [Fact] + public void ResolveRollbackOrigin_NoInvalidatedPhases_FallsBackToPhaseRollbackTo() + { + var profile = SdlcProfile.CasSdlcV1; + + var origin = profile.ResolveRollbackOrigin("verify", []); + + Assert.Equal("implement", origin); + } + + [Fact] + public void ResolveRollbackOrigin_UnknownPhaseAndNoInvalidated_ReturnsPhaseIdItself() + { + var profile = SdlcProfile.CasSdlcV1; + + var origin = profile.ResolveRollbackOrigin("no-such-phase", []); + + Assert.Equal("no-such-phase", origin); + } + + [Fact] + public void GetPhase_KnownPhaseId_ReturnsDefinition() + { + var profile = SdlcProfile.CasSdlcV1; + + var phase = profile.GetPhase("implement"); + + Assert.Equal("Implement", phase.Name); + Assert.Equal(SdlcBatch.Change, phase.Batch); + } + + [Fact] + public void TryGetPhase_UnknownPhaseId_ReturnsNull() + { + var profile = SdlcProfile.CasSdlcV1; + + Assert.Null(profile.TryGetPhase("no-such-phase")); + } + + // ---- GsdWorkflowContext.Transition: Failed-state branch was uncovered ---- + [Fact] + public void Transition_ToFailed_RecordsFailedStateAndFailureReason() + { + var ctx = new GsdWorkflowContext { FailureReason = "boom" }; + + var next = ctx.Transition(WorkflowState.Failed, "verification failed"); + + Assert.Equal(WorkflowState.Failed, next.CurrentState); + Assert.Equal(WorkflowState.Idle, next.FailedState); + Assert.Equal("boom", next.FailureReason); + Assert.Single(next.History); + Assert.Equal(WorkflowState.Failed, next.History[0].To); + } + + [Fact] + public void Transition_ToFailed_PreservesExistingFailedStateAcrossRepeatedFailures() + { + var ctx = new GsdWorkflowContext().Transition(WorkflowState.Failed, "first failure"); + + var again = ctx.Transition(WorkflowState.Failed, "second failure"); + + // FailedState should stick to the first-recorded failure origin (Idle), not be overwritten. + Assert.Equal(WorkflowState.Idle, again.FailedState); + } + + [Fact] + public void Transition_ToNonFailedState_ClearsFailureMetadataAndResetsRetryCount() + { + var ctx = new GsdWorkflowContext { RetryCount = 3, FailedState = WorkflowState.Editing, FailureReason = "boom" }; + + var next = ctx.Transition(WorkflowState.Branching); + + Assert.Equal(0, next.RetryCount); + Assert.Null(next.FailedState); + Assert.Null(next.FailureReason); + } + + // ---- GsdWorkflowContext.WithSdlcVerification: only the "failed" branch was covered ---- + [Fact] + public void WithSdlcVerification_Passed_ClearsPendingRollbackAndResetsNoProgressCount() + { + var ctx = new GsdWorkflowContext().WithSdlcRun("Improve the loop"); + + var next = ctx.WithSdlcVerification(new SdlcVerificationOutcome( + "research", "repo-verifier", Passed: true, InvalidatedPhaseIds: [], Reason: null)); + + Assert.Null(next.PendingRollback); + Assert.Equal(SdlcPhaseStatus.Passed, next.SdlcRun!.CurrentPhaseStatus); + Assert.Equal(0, next.SdlcRun.NoProgressCount); + } + + [Fact] + public void WithSdlcVerification_NoSdlcRun_ReturnsContextUnchanged() + { + var ctx = new GsdWorkflowContext(); + + var next = ctx.WithSdlcVerification(new SdlcVerificationOutcome( + "research", "repo-verifier", Passed: false, InvalidatedPhaseIds: [], Reason: "n/a")); + + Assert.Null(next.SdlcRun); + Assert.Null(next.PendingRollback); + } + + [Fact] + public void WithSdlcPhaseExecution_NoSdlcRun_ReturnsContextUnchanged() + { + var ctx = new GsdWorkflowContext(); + + var next = ctx.WithSdlcPhaseExecution(new SdlcPhaseExecutionRecord( + "understand", SdlcPhaseStatus.Passed, "repo-verifier", "digest-1", "prompt text")); + + Assert.Null(next.SdlcRun); + } + + // ---- LoopPolicyGuard: happy-path and unknown-action branches were uncovered ---- + [Fact] + public void RequireReadablePath_NoEnvSegment_DoesNotThrow() + { + var exception = Record.Exception(() => LoopPolicyGuard.RequireReadablePath("repo/src/Program.cs")); + Assert.Null(exception); + } + + [Fact] + public void EvaluateExternalAction_ApprovedKnownAction_ReturnsAuthorized() + { + Assert.Equal(ExternalActionDecision.Authorized, LoopPolicyGuard.EvaluateExternalAction("push", approved: true)); + } + + [Fact] + public void EvaluateExternalAction_UnknownAction_ThrowsArgumentOutOfRangeException() + { + Assert.Throws( + () => LoopPolicyGuard.EvaluateExternalAction("unknown_action", approved: true)); + } + + // ---- NativeProcessCommandExecutor: 0% branch-rate before this test (only exercised via FakeExecutor elsewhere) ---- + [Fact] + public async Task NativeProcessCommandExecutor_EmptyCommand_ReturnsToolMissingWithoutSpawningProcess() + { + var executor = new NativeProcessCommandExecutor(Environment.CurrentDirectory); + var check = new VerificationCheck("empty", VerificationCategory.Build, [], true, TimeSpan.FromSeconds(5)); + + var execution = await executor.ExecuteAsync(check, CancellationToken.None); + + Assert.Equal(-1, execution.ExitCode); + Assert.True(execution.ToolMissing); + Assert.False(execution.TimedOut); + } + + [Fact] + public async Task NativeProcessCommandExecutor_SuccessfulCommand_ReturnsZeroExitCode() + { + var executor = new NativeProcessCommandExecutor(Environment.CurrentDirectory); + var command = OperatingSystem.IsWindows() + ? new List { "cmd.exe", "/c", "exit 0" } + : new List { "/bin/sh", "-c", "exit 0" }; + var check = new VerificationCheck("success", VerificationCategory.Build, command, true, TimeSpan.FromSeconds(15)); + + var execution = await executor.ExecuteAsync(check, CancellationToken.None); + + Assert.Equal(0, execution.ExitCode); + Assert.False(execution.TimedOut); + Assert.False(execution.ToolMissing); + } + + [Fact] + public async Task NativeProcessCommandExecutor_NonZeroExitCommand_ReturnsThatExitCode() + { + var executor = new NativeProcessCommandExecutor(Environment.CurrentDirectory); + var command = OperatingSystem.IsWindows() + ? new List { "cmd.exe", "/c", "exit 7" } + : new List { "/bin/sh", "-c", "exit 7" }; + var check = new VerificationCheck("nonzero", VerificationCategory.Build, command, true, TimeSpan.FromSeconds(15)); + + var execution = await executor.ExecuteAsync(check, CancellationToken.None); + + Assert.Equal(7, execution.ExitCode); + Assert.False(execution.ToolMissing); + } + + [Fact] + public async Task NativeProcessCommandExecutor_MissingExecutable_ReturnsToolMissing() + { + var executor = new NativeProcessCommandExecutor(Environment.CurrentDirectory); + var check = new VerificationCheck( + "missing", VerificationCategory.Build, + [$"definitely-not-a-real-executable-{Guid.NewGuid():N}.exe"], true, TimeSpan.FromSeconds(5)); + + var execution = await executor.ExecuteAsync(check, CancellationToken.None); + + Assert.Equal(-1, execution.ExitCode); + Assert.True(execution.ToolMissing); + Assert.False(execution.TimedOut); + } + + // ---- McpTerminalOutcomePublisher: 0% branch-rate before this test (status switch never exercised) ---- + [Theory] + [InlineData(GoalStatus.Completed, "completed")] + [InlineData(GoalStatus.Cancelled, "cancelled")] + [InlineData(GoalStatus.Blocked, "blocked")] + [InlineData(GoalStatus.BudgetExhausted, "budget_exhausted")] + [InlineData(GoalStatus.Failed, "failed")] + [InlineData(GoalStatus.Draft, "failed")] // default arm of the switch + public async Task McpTerminalOutcomePublisher_PublishAsync_MapsGoalStatusToWireStatus(GoalStatus status, string expectedWireStatus) + { + var client = Substitute.For(); + client.CallToolAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(new McpToolResult("ok", false))); + var registry = new ResiliencePipelineRegistry(); + registry.TryAddBuilder("mcp-tools", (b, _) => { }); + var dispatcher = new McpToolDispatcher(client, registry, NullLogger.Instance); + var publisher = new McpTerminalOutcomePublisher(dispatcher); + var outcome = new TerminalLoopOutcome("goal-1", "corr-1", status, "done", ["cas://evidence/1"]); + + await publisher.PublishAsync(outcome, CancellationToken.None); + + await client.Received(1).CallToolAsync( + "record_terminal_outcome", + Arg.Is(obj => + obj["goal_id"]!.GetValue() == "goal-1" && + obj["status"]!.GetValue() == expectedWireStatus && + obj["summary"]!.GetValue() == "done"), + Arg.Any()); + } +}