Fix PR #81 review follow-ups

KSemenenko · KSemenenko · commit a6ca45329dc1 · 2026-03-14T02:14:05.000+01:00
diff --git a/DotPilot.Runtime.Host/Features/RuntimeFoundation/EmbeddedRuntimeTrafficPolicy.cs b/DotPilot.Runtime.Host/Features/RuntimeFoundation/EmbeddedRuntimeTrafficPolicy.cs
@@ -9,7 +9,6 @@ internal static class EmbeddedRuntimeTrafficPolicy
     private const string MermaidHeader = "flowchart LR";
     private const string MermaidArrow = " --> ";
     private const string MermaidActiveArrow = " ==> ";
-    private const string ClientTargetMethods = "GetAsync, UpsertAsync";
 
     public static string Summary => PolicySummary;
 
@@ -109,9 +108,6 @@ private static string CreateMermaidDiagramCore((string Source, string Target, st
                 transition.SourceMethods.Contains(activeTransition.Value.SourceMethod, StringComparer.Ordinal) &&
                 transition.TargetMethods.Contains(activeTransition.Value.TargetMethod, StringComparer.Ordinal);
             var arrow = isActive ? MermaidActiveArrow : MermaidArrow;
-            var targetMethods = transition.Target == EmbeddedRuntimeHostNames.ClientSourceName
-                ? ClientTargetMethods
-                : string.Join(", ", transition.TargetMethods);
             lines.Add(
                 string.Concat(
                     transition.Source,
@@ -120,7 +116,7 @@ private static string CreateMermaidDiagramCore((string Source, string Target, st
                     " : ",
                     string.Join(", ", transition.SourceMethods),
                     " -> ",
-                    targetMethods));
+                    string.Join(", ", transition.TargetMethods)));
         }
 
         return string.Join(Environment.NewLine, lines);
diff --git a/DotPilot.Runtime/Features/RuntimeFoundation/AgentFrameworkRuntimeClient.cs b/DotPilot.Runtime/Features/RuntimeFoundation/AgentFrameworkRuntimeClient.cs
@@ -18,11 +18,17 @@ public sealed class AgentFrameworkRuntimeClient : IAgentRuntimeClient
     private const string StartReplayKind = "run-started";
     private const string PauseReplayKind = "approval-pending";
     private const string ResumeReplayKind = "run-resumed";
+    private const string RejectedReplayKind = "approval-rejected";
     private const string CompletedReplayKind = "run-completed";
+    private const string ResumeNotAllowedDetailFormat =
+        "Session {0} is not paused with pending approval and cannot be resumed.";
+    private static readonly System.Text.CompositeFormat ResumeNotAllowedDetailCompositeFormat =
+        System.Text.CompositeFormat.Parse(ResumeNotAllowedDetailFormat);
     private readonly IGrainFactory _grainFactory;
     private readonly RuntimeSessionArchiveStore _archiveStore;
     private readonly DeterministicAgentTurnEngine _turnEngine;
     private readonly Workflow _workflow;
+    private readonly TimeProvider _timeProvider;
 
     public AgentFrameworkRuntimeClient(IGrainFactory grainFactory, RuntimeSessionArchiveStore archiveStore)
         : this(grainFactory, archiveStore, TimeProvider.System)
@@ -36,6 +42,7 @@ internal AgentFrameworkRuntimeClient(
     {
         _grainFactory = grainFactory;
         _archiveStore = archiveStore;
+        _timeProvider = timeProvider;
         _turnEngine = new DeterministicAgentTurnEngine(timeProvider);
         _workflow = BuildWorkflow();
     }
@@ -81,12 +88,26 @@ public async ValueTask<Result<AgentTurnResult>> ResumeAsync(AgentTurnResumeReque
     {
         cancellationToken.ThrowIfCancellationRequested();
 
-        var archive = await _archiveStore.LoadAsync(request.SessionId, cancellationToken);
+        StoredRuntimeSessionArchive? archive;
+        try
+        {
+            archive = await _archiveStore.LoadAsync(request.SessionId, cancellationToken);
+        }
+        catch (JsonException)
+        {
+            return Result<AgentTurnResult>.Fail(RuntimeCommunicationProblems.SessionArchiveCorrupted(request.SessionId));
+        }
+
         if (archive is null)
         {
             return Result<AgentTurnResult>.Fail(RuntimeCommunicationProblems.SessionArchiveMissing(request.SessionId));
         }
 
+        if (archive.Phase is not SessionPhase.Paused || archive.ApprovalState is not ApprovalState.Pending)
+        {
+            return Result<AgentTurnResult>.Fail(CreateResumeNotAllowedProblem(request.SessionId));
+        }
+
         if (string.IsNullOrWhiteSpace(archive.CheckpointId))
         {
             return Result<AgentTurnResult>.Fail(RuntimeCommunicationProblems.ResumeCheckpointMissing(request.SessionId));
@@ -115,7 +136,7 @@ await PersistRuntimeStateAsync(
             archive.OriginalRequest,
             result,
             resolvedCheckpoint,
-            request.ApprovalState is ApprovalState.Approved ? ResumeReplayKind : PauseReplayKind,
+            ResolveResumeReplayKind(request.ApprovalState),
             cancellationToken);
 
         return Result<AgentTurnResult>.Succeed(result);
@@ -232,13 +253,14 @@ private async ValueTask PersistRuntimeStateAsync(
     {
         var existingArchive = await _archiveStore.LoadAsync(originalRequest.SessionId, cancellationToken);
         var replay = existingArchive?.Replay.ToList() ?? [];
+        var recordedAt = _timeProvider.GetUtcNow();
         replay.Add(
             new RuntimeSessionReplayEntry(
                 replayKind,
                 result.Summary,
                 result.NextPhase,
                 result.ApprovalState,
-                DateTimeOffset.UtcNow));
+                recordedAt));
         if (result.NextPhase is SessionPhase.Execute or SessionPhase.Review or SessionPhase.Failed)
         {
             replay.Add(
@@ -247,7 +269,7 @@ private async ValueTask PersistRuntimeStateAsync(
                     result.Summary,
                     result.NextPhase,
                     result.ApprovalState,
-                    DateTimeOffset.UtcNow));
+                    recordedAt));
         }
 
         var archive = new StoredRuntimeSessionArchive(
@@ -257,16 +279,16 @@ private async ValueTask PersistRuntimeStateAsync(
             originalRequest,
             result.NextPhase,
             result.ApprovalState,
-            DateTimeOffset.UtcNow,
+            recordedAt,
             replay,
             result.ProducedArtifacts);
 
         await _archiveStore.SaveAsync(archive, cancellationToken);
-        await UpsertSessionStateAsync(originalRequest, result);
+        await UpsertSessionStateAsync(originalRequest, result, recordedAt);
         await UpsertArtifactsAsync(result.ProducedArtifacts);
     }
 
-    private async ValueTask UpsertSessionStateAsync(AgentTurnRequest request, AgentTurnResult result)
+    private async ValueTask UpsertSessionStateAsync(AgentTurnRequest request, AgentTurnResult result, DateTimeOffset timestamp)
     {
         var session = new SessionDescriptor
         {
@@ -276,8 +298,8 @@ private async ValueTask UpsertSessionStateAsync(AgentTurnRequest request, AgentT
             Phase = result.NextPhase,
             ApprovalState = result.ApprovalState,
             AgentProfileIds = [request.AgentProfileId],
-            CreatedAt = DateTimeOffset.UtcNow,
-            UpdatedAt = DateTimeOffset.UtcNow,
+            CreatedAt = timestamp,
+            UpdatedAt = timestamp,
         };
 
         await _grainFactory.GetGrain<ISessionGrain>(request.SessionId.ToString()).UpsertAsync(session);
@@ -352,6 +374,27 @@ await context.QueueStateUpdateAsync(
         await context.RequestHaltAsync();
     }
 
+    private static string ResolveResumeReplayKind(ApprovalState approvalState)
+    {
+        return approvalState switch
+        {
+            ApprovalState.Approved => ResumeReplayKind,
+            ApprovalState.Rejected => RejectedReplayKind,
+            _ => PauseReplayKind,
+        };
+    }
+
+    private static Problem CreateResumeNotAllowedProblem(SessionId sessionId)
+    {
+        return Problem.Create(
+            RuntimeCommunicationProblemCode.ResumeCheckpointMissing,
+            string.Format(
+                System.Globalization.CultureInfo.InvariantCulture,
+                ResumeNotAllowedDetailCompositeFormat,
+                sessionId),
+            (int)System.Net.HttpStatusCode.Conflict);
+    }
+
     private async ValueTask HandleResumeAsync(
         RuntimeWorkflowSignal signal,
         RuntimeWorkflowState state,
diff --git a/DotPilot.Runtime/Features/RuntimeFoundation/RuntimeFoundationCatalog.cs b/DotPilot.Runtime/Features/RuntimeFoundation/RuntimeFoundationCatalog.cs
@@ -25,7 +25,7 @@ public sealed class RuntimeFoundationCatalog : IRuntimeFoundationCatalog
         "Agent Framework orchestrates local runs, approvals, and checkpoints without moving execution logic into the Uno app.";
     private const string TrafficPolicyName = "Traffic policy";
     private const string TrafficPolicySummary =
-        "Allowed grain transitions are explicit, testable, and visualized through Orleans.Graph instead of hidden conventions.";
+        "Allowed grain transitions are explicit, testable, and surfaced through the embedded traffic-policy Mermaid catalog instead of hidden conventions.";
     private const string SessionPersistenceName = "Session persistence";
     private const string SessionPersistenceSummary =
         "Checkpoint, replay, and resume data survive host restarts in local session archives without changing the Orleans storage topology.";
diff --git a/DotPilot.Tests/Features/RuntimeFoundation/AgentFrameworkRuntimeClientTests.cs b/DotPilot.Tests/Features/RuntimeFoundation/AgentFrameworkRuntimeClientTests.cs
@@ -9,8 +9,10 @@ public sealed class AgentFrameworkRuntimeClientTests
     private const string PlanPrompt = "Plan the embedded runtime rollout.";
     private const string ApprovedResumeSummary = "Approved by the operator.";
     private const string RejectedResumeSummary = "Rejected by the operator.";
+    private const string ResumeRejectedKind = "approval-rejected";
     private const string ArchiveFileName = "archive.json";
     private const string ReplayFileName = "replay.md";
+    private static readonly DateTimeOffset FixedTimestamp = new(2026, 3, 14, 9, 30, 0, TimeSpan.Zero);
 
     [Test]
     public async Task ExecuteAsyncPersistsAReplayArchiveForPlanMode()
@@ -96,9 +98,41 @@ public async Task ResumeAsyncPersistsRejectedApprovalAsFailedReplay()
         rejectedResult.Value.ApprovalState.Should().Be(ApprovalState.Rejected);
         archiveResult.IsSuccess.Should().BeTrue();
         archiveResult.Value!.Phase.Should().Be(SessionPhase.Failed);
+        archiveResult.Value.Replay.Should().Contain(entry => entry.Kind == ResumeRejectedKind && entry.Phase == SessionPhase.Failed);
         archiveResult.Value.Replay.Should().Contain(entry => entry.Kind == "run-completed" && entry.Phase == SessionPhase.Failed);
     }
 
+    [Test]
+    public async Task ResumeAsyncRejectsArchivedSessionsThatAreNoLongerPausedForApproval()
+    {
+        using var runtimeDirectory = new TemporaryRuntimePersistenceDirectory();
+        var request = CreateRequest(ApprovalPrompt, AgentExecutionMode.Execute);
+
+        {
+            using var firstHost = CreateHost(runtimeDirectory.Root);
+            await firstHost.StartAsync();
+            var firstClient = firstHost.Services.GetRequiredService<IAgentRuntimeClient>();
+            _ = await firstClient.ExecuteAsync(request, CancellationToken.None);
+            _ = await firstClient.ResumeAsync(
+                new AgentTurnResumeRequest(request.SessionId, ApprovalState.Approved, ApprovedResumeSummary),
+                CancellationToken.None);
+        }
+
+        {
+            using var secondHost = CreateHost(runtimeDirectory.Root);
+            await secondHost.StartAsync();
+            var secondClient = secondHost.Services.GetRequiredService<IAgentRuntimeClient>();
+
+            var result = await secondClient.ResumeAsync(
+                new AgentTurnResumeRequest(request.SessionId, ApprovalState.Approved, ApprovedResumeSummary),
+                CancellationToken.None);
+
+            result.IsFailed.Should().BeTrue();
+            result.Problem!.HasErrorCode(RuntimeCommunicationProblemCode.ResumeCheckpointMissing).Should().BeTrue();
+            result.Problem.Detail.Should().Contain("cannot be resumed");
+        }
+    }
+
     [Test]
     public async Task GetSessionArchiveAsyncReturnsMissingProblemWhenNothingWasPersisted()
     {
@@ -133,6 +167,52 @@ public async Task GetSessionArchiveAsyncReturnsCorruptionProblemForInvalidArchiv
         result.Problem!.HasErrorCode(RuntimeCommunicationProblemCode.SessionArchiveCorrupted).Should().BeTrue();
     }
 
+    [Test]
+    public async Task ResumeAsyncReturnsCorruptionProblemForInvalidArchivePayload()
+    {
+        using var runtimeDirectory = new TemporaryRuntimePersistenceDirectory();
+        var sessionId = SessionId.New();
+        var sessionDirectory = Path.Combine(runtimeDirectory.Root, sessionId.ToString());
+        Directory.CreateDirectory(sessionDirectory);
+        await File.WriteAllTextAsync(Path.Combine(sessionDirectory, ArchiveFileName), "{ invalid json", CancellationToken.None);
+
+        using var host = CreateHost(runtimeDirectory.Root);
+        await host.StartAsync();
+        var client = host.Services.GetRequiredService<IAgentRuntimeClient>();
+
+        var result = await client.ResumeAsync(
+            new AgentTurnResumeRequest(sessionId, ApprovalState.Approved, ApprovedResumeSummary),
+            CancellationToken.None);
+
+        result.IsFailed.Should().BeTrue();
+        result.Problem!.HasErrorCode(RuntimeCommunicationProblemCode.SessionArchiveCorrupted).Should().BeTrue();
+    }
+
+    [Test]
+    public async Task AgentFrameworkRuntimeClientUsesTheInjectedTimeProviderForReplayArchiveAndSessionTimestamps()
+    {
+        using var runtimeDirectory = new TemporaryRuntimePersistenceDirectory();
+        using var host = CreateHost(runtimeDirectory.Root);
+        await host.StartAsync();
+        var client = CreateClient(host.Services, runtimeDirectory.Root, new FixedTimeProvider(FixedTimestamp));
+        var request = CreateRequest(PlanPrompt, AgentExecutionMode.Plan);
+
+        var result = await client.ExecuteAsync(request, CancellationToken.None);
+        var archiveResult = await client.GetSessionArchiveAsync(request.SessionId, CancellationToken.None);
+        var session = await host.Services
+            .GetRequiredService<IGrainFactory>()
+            .GetGrain<ISessionGrain>(request.SessionId.ToString())
+            .GetAsync();
+
+        result.IsSuccess.Should().BeTrue();
+        archiveResult.IsSuccess.Should().BeTrue();
+        archiveResult.Value!.UpdatedAt.Should().Be(FixedTimestamp);
+        archiveResult.Value.Replay.Should().OnlyContain(entry => entry.RecordedAt == FixedTimestamp);
+        session.Should().NotBeNull();
+        session!.CreatedAt.Should().Be(FixedTimestamp);
+        session.UpdatedAt.Should().Be(FixedTimestamp);
+    }
+
     [Test]
     public async Task ExtractCheckpointReturnsNullWhenRunHasNoCheckpointData()
     {
@@ -212,6 +292,24 @@ private static AgentTurnRequest CreateRequest(string prompt, AgentExecutionMode
         return new AgentTurnRequest(SessionId.New(), AgentProfileId.New(), prompt, mode, ProviderConnectionStatus.Available);
     }
 
+    private static AgentFrameworkRuntimeClient CreateClient(IServiceProvider services, string rootDirectory, TimeProvider timeProvider)
+    {
+        return (AgentFrameworkRuntimeClient)Activator.CreateInstance(
+            typeof(AgentFrameworkRuntimeClient),
+            BindingFlags.Instance | BindingFlags.NonPublic,
+            binder: null,
+            args:
+            [
+                services.GetRequiredService<IGrainFactory>(),
+                new RuntimeSessionArchiveStore(new RuntimePersistenceOptions
+                {
+                    RootDirectoryPath = rootDirectory,
+                }),
+                timeProvider,
+            ],
+            culture: null)!;
+    }
+
     private static Microsoft.Agents.AI.Workflows.Workflow CreateNoCheckpointWorkflow()
     {
         var executor = new Microsoft.Agents.AI.Workflows.FunctionExecutor<string>(
@@ -248,6 +346,11 @@ private static int GetFreeTcpPort()
     }
 }
 
+internal sealed class FixedTimeProvider(DateTimeOffset timestamp) : TimeProvider
+{
+    public override DateTimeOffset GetUtcNow() => timestamp;
+}
+
 internal sealed class TemporaryRuntimePersistenceDirectory : IDisposable
 {
     public TemporaryRuntimePersistenceDirectory()
diff --git a/DotPilot.Tests/RuntimeFoundationCatalogTests.cs b/DotPilot.Tests/RuntimeFoundationCatalogTests.cs
@@ -25,6 +25,11 @@ public void CatalogGroupsEpicTwelveIntoFourSequencedSlices()
             RuntimeFoundationIssues.AgentFrameworkRuntime,
             RuntimeFoundationIssues.GrainTrafficPolicy,
             RuntimeFoundationIssues.SessionPersistence);
+        snapshot.Slices.Single(slice => slice.IssueNumber == RuntimeFoundationIssues.GrainTrafficPolicy)
+            .Summary
+            .Should()
+            .Contain("Mermaid")
+            .And.NotContain("Orleans.Graph");
     }
 
     [Test]
