Resolve text approvals against the oldest pending approval

openspec/changes/consolidate-binding-actor-engines/specs/channel-binding-parity/spec.md

@@ -32,7 +32,7 @@ The shared engine SHALL store cursors as strings, which matches the persisted `C
 
 ### Requirement: Shared approval-response flow
 
-The system SHALL implement text-approval parsing, cold-spawn approval forwarding, and pending-prompt resolution in a single shared flow. The requester identity check SHALL execute inside the shared flow. Per-channel hooks SHALL be limited to prompt rendering, the pending-approval match order, and, for Mattermost only, the synchronous webhook reply.
+The system SHALL implement text-approval parsing, cold-spawn approval forwarding, and pending-prompt resolution in a single shared flow. The requester identity check SHALL execute inside the shared flow. The pending-approval match order SHALL be the same on every channel. Per-channel hooks SHALL be limited to prompt rendering and, for Mattermost only, the synchronous webhook reply.
 
 #### Scenario: Wrong requester is rejected on every channel
 
@@ -48,20 +48,19 @@ The system SHALL implement text-approval parsing, cold-spawn approval forwarding
 - **THEN** the Mattermost hook sends the synchronous HTTP reply
 - **AND** Discord and Slack register no such hook
 
-#### Scenario: Channel match order picks the same candidate as before
+#### Scenario: Text approval resolves the earliest pending approval
 
 - **GIVEN** two pending approvals that the same sender may approve
 - **WHEN** that sender sends a text approval reply
-- **THEN** Slack resolves the earliest pending approval
-- **AND** Discord and Mattermost resolve the most recent pending approval
+- **THEN** Slack, Discord, and Mattermost each resolve the earliest pending approval
+- **AND** the next text approval reply resolves the second pending approval
 
-> Note: this match order is the one real difference the step-3 stop rule found
+> Note: the step-3 stop rule found this match order as the one real difference
 > between the three copies. Slack selected its candidate with `FindIndex`
 > (earliest match); Discord and Mattermost selected it with `LastOrDefault`
-> (most recent match). The shared lookup keeps one requester check and takes the
-> order as a required `ApprovalMatchOrder` input, so each channel keeps the
-> selection it had. Which order is correct is a separate product question,
-> tracked outside this change.
+> (most recent match). The maintainer resolved the difference: the earliest
+> pending approval wins on every channel. The shared lookup keeps one requester
+> check and hard-codes the earliest match, so no channel supplies an order.
 
 ### Requirement: Shared output-completion bookkeeping
 

openspec/specs/channel-binding-parity/spec.md

@@ -36,7 +36,7 @@ The shared engine SHALL store cursors as strings, which matches the persisted `C
 
 ### Requirement: Shared approval-response flow
 
-The system SHALL implement text-approval parsing, cold-spawn approval forwarding, and pending-prompt resolution in a single shared flow. The requester identity check SHALL execute inside the shared flow. Per-channel hooks SHALL be limited to prompt rendering, the pending-approval match order, and, for Mattermost only, the synchronous webhook reply.
+The system SHALL implement text-approval parsing, cold-spawn approval forwarding, and pending-prompt resolution in a single shared flow. The requester identity check SHALL execute inside the shared flow. The pending-approval match order SHALL be the same on every channel. Per-channel hooks SHALL be limited to prompt rendering and, for Mattermost only, the synchronous webhook reply.
 
 #### Scenario: Wrong requester is rejected on every channel
 
@@ -52,20 +52,19 @@ The system SHALL implement text-approval parsing, cold-spawn approval forwarding
 - **THEN** the Mattermost hook sends the synchronous HTTP reply
 - **AND** Discord and Slack register no such hook
 
-#### Scenario: Channel match order picks the same candidate as before
+#### Scenario: Text approval resolves the earliest pending approval
 
 - **GIVEN** two pending approvals that the same sender may approve
 - **WHEN** that sender sends a text approval reply
-- **THEN** Slack resolves the earliest pending approval
-- **AND** Discord and Mattermost resolve the most recent pending approval
+- **THEN** Slack, Discord, and Mattermost each resolve the earliest pending approval
+- **AND** the next text approval reply resolves the second pending approval
 
-> Note: this match order is a real difference between the pre-consolidation
+> Note: the consolidation found a real difference between the pre-consolidation
 > copies. Slack selected its candidate with `FindIndex` (earliest match);
 > Discord and Mattermost selected it with `LastOrDefault` (most recent match).
-> The shared lookup keeps one requester check and takes the order as a
-> required `ApprovalMatchOrder` input, so each channel keeps the selection it
-> had. Which order is correct is a separate product question, tracked outside
-> the introducing change.
+> The maintainer resolved the difference: the earliest pending approval wins on
+> every channel. The shared lookup keeps one requester check and hard-codes the
+> earliest match, so no channel supplies an order.
 
 ### Requirement: Shared output-completion bookkeeping
 

src/Netclaw.Actors.Tests/Channels/Contracts/SessionBindingContractTests.cs

@@ -746,6 +746,70 @@ await AwaitAssertAsync(() =>
         }, cancellationToken: ct);
     }
 
+    // Cross-channel match-order contract. Slack resolved the earliest pending
+    // approval; Discord and Mattermost resolved the most recent one. The
+    // consolidation found the divergence and the maintainer chose one rule for
+    // every channel: a text reply answers the earliest pending approval, which
+    // is the first prompt the channel shows.
+    [Fact]
+    public async Task Text_approval_response_resolves_earliest_pending_approval()
+    {
+        var ct = TestContext.Current.CancellationToken;
+        var detector = new ConfigurablePromptInjectionDetector(PromptInjectionResult.Safe());
+        var sid = new SessionId("session-text-approve-order");
+        var pipeline = new RecordingSessionPipeline(_ =>
+        [
+            ApprovalRequest(sid, "call-order-1", "write_file"),
+            ApprovalRequest(sid, "call-order-2", "execute_shell")
+        ]);
+
+        var actor = CreateBindingActor(sid, pipeline, detector);
+
+        await AwaitAssertAsync(() =>
+        {
+            var texts = GetPostedTexts();
+            Assert.Contains(texts, t => t.Contains("write_file"));
+            Assert.Contains(texts, t => t.Contains("execute_shell"));
+        }, cancellationToken: ct);
+
+        // The same sender can approve both prompts, so only the order decides.
+        actor.Tell(CreateInboundMessage("A", "user-1"), TestActor);
+
+        await AwaitAssertAsync(() =>
+        {
+            var feedback = pipeline.RecordedFeedback.OfType<ToolInteractionResponse>().ToList();
+            Assert.Single(feedback);
+            Assert.Equal("call-order-1", feedback[0].CallId.Value);
+            Assert.Equal(ApprovalOptionKeys.ApproveOnce, feedback[0].SelectedKey.Value);
+        }, cancellationToken: ct);
+
+        // The second prompt stays pending and the next reply resolves it.
+        actor.Tell(CreateInboundMessage("A", "user-1"), TestActor);
+
+        await AwaitAssertAsync(() =>
+        {
+            var feedback = pipeline.RecordedFeedback.OfType<ToolInteractionResponse>().ToList();
+            Assert.Equal(2, feedback.Count);
+            Assert.Equal("call-order-2", feedback[1].CallId.Value);
+        }, cancellationToken: ct);
+    }
+
+    private static ToolInteractionRequest ApprovalRequest(SessionId sessionId, string callId, string toolName)
+        => new()
+        {
+            SessionId = sessionId,
+            Kind = "approval",
+            CallId = new Netclaw.Tools.ToolCallId(callId),
+            ToolName = new Netclaw.Tools.ToolName(toolName),
+            DisplayText = $"run {toolName}",
+            RequesterSenderId = new SenderId("user-1"),
+            Options =
+            [
+                new ToolInteractionOption(ApprovalOptionKeys.ApproveOnceKey, ApprovalOptionKeys.ApproveOnceLabel),
+                new ToolInteractionOption(ApprovalOptionKeys.DenyKey, ApprovalOptionKeys.DenyLabel)
+            ]
+        };
+
     [Fact]
     public async Task Approval_response_after_turn_completed_forwards_to_session()
     {

src/Netclaw.Actors/Reminders/ReminderManagerActor.cs

@@ -52,6 +52,12 @@ public sealed partial class ReminderManagerActor : ReceiveActor
     private readonly ActiveExecutionTracker _activeExecutions = new();
     private readonly Dictionary<ReminderId, int> _skipCounts = [];
 
+    // Uniqueness source for execution child actor names. A wall-clock
+    // millisecond suffix collided when two fires for one reminder landed in
+    // the same millisecond and threw InvalidActorNameException. The actor is
+    // single-threaded, so a plain counter is collision-free for its lifetime.
+    private long _executionSequence;
+
     public ReminderManagerActor(
         ISessionPipeline pipeline,
         EffectivePolicyDefaults defaults,
@@ -1251,7 +1257,7 @@ private void StartExecution(
         var startedAt = _timeProvider.GetUtcNow();
         _activeExecutions.Add(definition.Id, executionId, envelope, startedAt);
 
-        var actorName = $"exec-{SanitizeActorName(definition.Id.Value)}-{startedAt.ToUnixTimeMilliseconds()}";
+        var actorName = $"exec-{SanitizeActorName(definition.Id.Value)}-{++_executionSequence}";
         var executionActor = Context.ActorOf(
             ReminderExecutionActor.CreateProps(
                 executionId,

src/Netclaw.Channels.Discord/DiscordSessionBindingActor.cs

@@ -165,7 +165,6 @@ public DiscordSessionBindingActor(
             pipeline: _dependencies.Pipeline,
             operationTimeout: OperationTimeout,
             pendingRequests: _pendingApprovalRequests,
-            matchOrder: ApprovalMatchOrder.Newest,
             hasObservedApprovalRequest: () => _outputEngine.HasObservedApprovalRequest,
             postWrongRequesterWarningAsync: () => SafeReplyAsync(WrongRequesterWarning),
             persistPromptCleared: callId => Persist(

src/Netclaw.Channels.Mattermost/MattermostSessionBindingActor.cs

@@ -157,7 +157,6 @@ public MattermostSessionBindingActor(
             pipeline: _dependencies.Pipeline,
             operationTimeout: OperationTimeout,
             pendingRequests: _pendingApprovalRequests,
-            matchOrder: ApprovalMatchOrder.Newest,
             hasObservedApprovalRequest: () => _outputEngine.HasObservedApprovalRequest,
             postWrongRequesterWarningAsync: () => SafeReplyAsync(WrongRequesterWarning),
             persistPromptCleared: callId => Persist(

src/Netclaw.Channels.Slack/SlackThreadBindingActor.cs

@@ -156,10 +156,6 @@ public SlackThreadBindingActor(
             pipeline: _dependencies.Pipeline,
             operationTimeout: OperationTimeout,
             pendingRequests: _pendingApprovalRequests,
-            // Slack resolves the earliest matching prompt, which is the order its
-            // own lookup used before the extraction. Discord and Mattermost
-            // resolve the most recent one.
-            matchOrder: ApprovalMatchOrder.Oldest,
             hasObservedApprovalRequest: () => _outputEngine.HasObservedApprovalRequest,
             postWrongRequesterWarningAsync: () => SafePostAsync(WrongRequesterWarning),
             persistPromptCleared: callId => Persist(

src/Netclaw.Channels/ApprovalResponseFlow.cs

@@ -56,7 +56,6 @@ public sealed class ApprovalResponseFlow<TRequest, TPromptId>
     private readonly ISessionPipeline _pipeline;
     private readonly TimeSpan _operationTimeout;
     private readonly List<TRequest> _pendingRequests;
-    private readonly ApprovalMatchOrder _matchOrder;
     private readonly Func<bool> _hasObservedApprovalRequest;
     private readonly Func<Task> _postWrongRequesterWarningAsync;
     private readonly Action<ToolCallId> _persistPromptCleared;
@@ -77,7 +76,6 @@ public sealed class ApprovalResponseFlow<TRequest, TPromptId>
     /// The actor's own pending-approval list. The flow reads it and removes the
     /// entry it resolves; the actor keeps adding to it and replaying it.
     /// </param>
-    /// <param name="matchOrder">Which candidate wins when more than one matches.</param>
     /// <param name="hasObservedApprovalRequest">Reads the cold-path gate.</param>
     /// <param name="postWrongRequesterWarningAsync">Posts the channel's wrong-requester warning.</param>
     /// <param name="persistPromptCleared">
@@ -93,7 +91,6 @@ public ApprovalResponseFlow(
         ISessionPipeline pipeline,
         TimeSpan operationTimeout,
         List<TRequest> pendingRequests,
-        ApprovalMatchOrder matchOrder,
         Func<bool> hasObservedApprovalRequest,
         Func<Task> postWrongRequesterWarningAsync,
         Action<ToolCallId> persistPromptCleared,
@@ -106,7 +103,6 @@ public ApprovalResponseFlow(
         _pipeline = pipeline;
         _operationTimeout = operationTimeout;
         _pendingRequests = pendingRequests;
-        _matchOrder = matchOrder;
         _hasObservedApprovalRequest = hasObservedApprovalRequest;
         _postWrongRequesterWarningAsync = postWrongRequesterWarningAsync;
         _persistPromptCleared = persistPromptCleared;
@@ -125,7 +121,7 @@ public ApprovalResponseFlow(
     public async Task<bool> TryHandleTextApprovalResponseAsync(string? text, string senderId)
     {
         var (result, pending) = PendingApprovalLookup.Resolve<TRequest, TPromptId>(
-            _pendingRequests, senderId, callId: null, _matchOrder);
+            _pendingRequests, senderId, callId: null);
 
         if (result is ApprovalLookupResult.NotFound)
         {
@@ -240,7 +236,7 @@ public async Task HandleApprovalResponseAsync(
         Action<ISessionResponse>? respondSynchronously = null)
     {
         var (result, pending) = PendingApprovalLookup.Resolve<TRequest, TPromptId>(
-            _pendingRequests, senderId, callId, _matchOrder);
+            _pendingRequests, senderId, callId);
 
         // CanApprove fast-path: if the binding still holds the original request we can
         // post the wrong-requester warning locally without round-tripping through the

src/Netclaw.Channels/PendingApprovalLookup.cs

@@ -10,37 +10,30 @@ namespace Netclaw.Channels;
 
 public enum ApprovalLookupResult { Matched, WrongRequester, NotFound }
 
-/// <summary>
-/// Selects which candidate wins when more than one pending request matches.
-/// The requester check is the same for both orders; only the tie-break differs.
-/// </summary>
-public enum ApprovalMatchOrder
-{
-    /// <summary>The most recent match wins. Discord and Mattermost use this.</summary>
-    Newest,
-
-    /// <summary>The earliest match wins. Slack uses this.</summary>
-    Oldest
-}
-
 /// <summary>
 /// Finds the pending approval a channel binding actor should act on. A
-/// given call ID takes priority; otherwise the pending request the sender
-/// is allowed to approve wins, picked by the channel's match order.
+/// given call ID takes priority; otherwise the earliest pending request the
+/// sender is allowed to approve wins.
 /// </summary>
+/// <remarks>
+/// Slack, Discord, and Mattermost all resolve the earliest match. Discord and
+/// Mattermost resolved the most recent match before the binding-actor
+/// consolidation. The maintainer decided that one order applies to every
+/// channel: the earliest pending approval wins. A user who answers a queue of
+/// prompts answers them in the order that the channel shows them.
+/// </remarks>
 public static class PendingApprovalLookup
 {
     public static (ApprovalLookupResult Result, TRequest? Pending) Resolve<TRequest, TPromptId>(
         IReadOnlyList<TRequest> pendingRequests,
         string approvingSenderId,
-        ToolCallId? callId,
-        ApprovalMatchOrder matchOrder)
+        ToolCallId? callId)
         where TRequest : PendingApprovalRequest<TPromptId>
         where TPromptId : struct
     {
         if (callId is { } resolvedCallId)
         {
-            var byCallId = Select(pendingRequests, p => p.CallId == resolvedCallId, matchOrder);
+            var byCallId = pendingRequests.FirstOrDefault(p => p.CallId == resolvedCallId);
             if (byCallId is null)
                 return (ApprovalLookupResult.NotFound, null);
             if (!ApprovalButtonValueCodec.CanApprove(byCallId.RequesterPrincipal, byCallId.RequesterSenderId, approvingSenderId))
@@ -51,21 +44,10 @@ public static (ApprovalLookupResult Result, TRequest? Pending) Resolve<TRequest,
         if (pendingRequests.Count == 0)
             return (ApprovalLookupResult.NotFound, null);
 
-        var bySender = Select(
-            pendingRequests,
-            p => ApprovalButtonValueCodec.CanApprove(p.RequesterPrincipal, p.RequesterSenderId, approvingSenderId),
-            matchOrder);
+        var bySender = pendingRequests.FirstOrDefault(
+            p => ApprovalButtonValueCodec.CanApprove(p.RequesterPrincipal, p.RequesterSenderId, approvingSenderId));
         return bySender is not null
             ? (ApprovalLookupResult.Matched, bySender)
             : (ApprovalLookupResult.WrongRequester, null);
     }
-
-    private static TRequest? Select<TRequest>(
-        IReadOnlyList<TRequest> pendingRequests,
-        Func<TRequest, bool> predicate,
-        ApprovalMatchOrder matchOrder)
-        where TRequest : class
-        => matchOrder is ApprovalMatchOrder.Newest
-            ? pendingRequests.LastOrDefault(predicate)
-            : pendingRequests.FirstOrDefault(predicate);
 }