feat: add ExecApprovalsStore write path and wire coordinator side effects

src/OpenClaw.Shared/ExecApprovals/CanonicalCommandIdentity.cs

@@ -2,36 +2,36 @@
 
 namespace OpenClaw.Shared.ExecApprovals;
 
-// Architectural barrier produced by PR3.
+// Architectural barrier between raw requests and the evaluation pipeline.
 // Equivalent to ExecHostValidatedRequest in the macOS reference, extended with resolution outputs.
-// No module from PR4 onward may accept ValidatedRunRequest as direct input (research doc 05 line 439).
-// Rail 15: a single canonical representation reused across evaluation, logging, prompting, execution.
+// No evaluation module may accept ValidatedRunRequest as direct input — this is the canonical handoff type.
+// A single canonical representation reused across evaluation, logging, prompting, and execution.
 public sealed class CanonicalCommandIdentity
 {
     // ── Normalization outputs ─────────────────────────────────────────────────
 
-    // Argv exactly as produced by PR2 (no trimming; coding contract process-argv-semantics).
+    // Argv as received from the normalizer (no trimming; callers must not modify).
     public IReadOnlyList<string> Command { get; }
 
     // Canonical display form generated from argv. Never rawCommand from the agent.
-    // Used by logging and prompting. Research doc 05 decision 2.
+    // Used by logging and prompting.
     public string DisplayCommand { get; }
 
     // Safe rawCommand for executable resolution. Null in Windows v1 (rawCommand not in
-    // system.run protocol; research doc 05 OQ-V4 / decision 10).
+    // the system.run protocol).
     public string? EvaluationRawCommand { get; }
 
     // ── Resolution outputs ────────────────────────────────────────────────────
 
-    // Singular resolution for the state machine (PR5).
+    // Singular resolution for the state machine.
     // Null if the primary executable cannot be determined.
     public ExecCommandResolution? Resolution { get; }
 
-    // Per-segment resolutions for the allowlist matcher (PR4/PR5).
+    // Per-segment resolutions for the allowlist matcher.
     // Empty list means fail-closed — no allowlist satisfaction possible.
     public IReadOnlyList<ExecCommandResolution> AllowlistResolutions { get; }
 
-    // Suggested allowlist patterns for prompt/UI (PR6). Not a security decision.
+    // Suggested allowlist patterns for prompt/UI. Not a security decision.
     public IReadOnlyList<string> AllowAlwaysPatterns { get; }
 
     // ── Request context (carried from ValidatedRunRequest) ────────────────────

src/OpenClaw.Shared/ExecApprovals/ExecApprovalEvaluation.cs

@@ -4,9 +4,9 @@
 namespace OpenClaw.Shared.ExecApprovals;
 
 // Aggregated evaluation context passed to the stateless evaluator.
-// Shape mirrors macOS ExecApprovalEvaluation struct (research doc 06).
+// Shape mirrors macOS ExecApprovalEvaluation struct.
 // Derived fields are computed once in the constructor and must not be recomputed by callers.
-// Research doc 06 stable conclusion 3: construction belongs to the coordinator (PR7), not the evaluator.
+// Construction belongs to the coordinator, not the evaluator.
 public sealed class ExecApprovalEvaluation
 {
     public IReadOnlyList<string> Command { get; }
@@ -17,7 +17,7 @@ public sealed class ExecApprovalEvaluation
     public IReadOnlyDictionary<string, string>? Env { get; }
 
     // Singular resolution — AllowlistResolutions[0], or null if the list is empty.
-    // Research doc 06: "resolution = allowlistResolutions.first" (not an independent call).
+    // Always the first element of AllowlistResolutions, not an independent resolver call.
     public ExecCommandResolution? Resolution { get; }
 
     public IReadOnlyList<ExecCommandResolution> AllowlistResolutions { get; }
@@ -27,11 +27,11 @@ public sealed class ExecApprovalEvaluation
     public bool AllAllowlistResolutionsMatched { get; }
 
     // true iff security==allowlist && resolutions.Count>0 && matches.Count==resolutions.Count.
-    // Research doc 06 derivation rule — must not be re-derived outside the constructor.
+    // Derived once in the constructor — must not be re-derived outside it.
     public bool AllowlistSatisfied { get; }
 
     // First match when AllowlistSatisfied; null otherwise.
-    // Research doc 06 R5: AllowlistMatch must be null when AllowlistSatisfied is false.
+    // Must be null when AllowlistSatisfied is false.
     public ExecAllowlistEntry? AllowlistMatch { get; }
 
     // Always false in v1. Kept as part of the conceptual model; activation deferred.

src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2InputValidator.cs

@@ -4,7 +4,7 @@
 namespace OpenClaw.Shared.ExecApprovals;
 
 /// <summary>
-/// Phase 1 of the V2 exec approval pipeline: structural input validation (rail 18, step 1).
+/// Phase 1 of the V2 exec approval pipeline: structural input validation.
 /// Parses a raw NodeInvokeRequest into a ValidatedRunRequest or returns validation-failed.
 /// Does not resolve executables, detect shell wrappers, or evaluate policy.
 /// </summary>

src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2NormalizationStep.cs

@@ -3,7 +3,7 @@
 namespace OpenClaw.Shared.ExecApprovals;
 
 // Either a CanonicalCommandIdentity (IsResolved=true) or a typed denial (IsResolved=false).
-// Produced by ExecApprovalV2Normalizer; consumed by the coordinator pipeline (PR7).
+// Produced by ExecApprovalV2Normalizer; consumed by the coordinator pipeline.
 public sealed class ExecApprovalV2NormalizationOutcome
 {
     public bool IsResolved { get; }
@@ -29,7 +29,7 @@ public static ExecApprovalV2NormalizationOutcome Fail(ExecApprovalV2Result error
         => new(error);
 }
 
-// Rail 18 steps 2-4: normalize command form → resolve executable → build canonical identity.
+// Steps 2-4 of the approval pipeline: normalize command form → resolve executable → build canonical identity.
 // Stateless — safe to call concurrently.
 public static class ExecApprovalV2Normalizer
 {
@@ -39,26 +39,26 @@ public static ExecApprovalV2NormalizationOutcome Normalize(ValidatedRunRequest r
         var cwd = request.Cwd;
         var env = request.Env as IReadOnlyDictionary<string, string>;
 
-        // displayCommand is always derived from argv, never from rawCommand (research doc 05 decision 2).
+        // displayCommand is always derived from argv, never from rawCommand.
         var displayCommand = ShellQuoting.FormatExecCommand(argv);
 
-        // rawCommand is null in Windows v1 (system.run does not carry it; research doc 05 OQ-V4).
+        // rawCommand is null in Windows v1 (system.run does not carry it).
         // EvaluationRawCommand stays null — correct and documented conservative output.
         string? evaluationRawCommand = null;
 
         // Singular resolution for state machine.
         var resolution = ExecCommandResolver.Resolve(argv, cwd, env);
 
         // Multi-segment resolution for allowlist.
-        // Empty list is fail-closed: no allowlist satisfaction possible (research doc 04 R2).
+        // Empty list is fail-closed: no allowlist satisfaction possible.
         // An empty list is NOT itself a denial at this step — the evaluator decides.
         var allowlistResolutions = ExecCommandResolver.ResolveForAllowlist(
             argv, evaluationRawCommand, cwd, env);
 
         // UX patterns for prompting.
         var allowAlwaysPatterns = ExecCommandResolver.ResolveAllowAlwaysPatterns(argv, cwd, env);
 
-        // Rail 6: if argv is non-empty but resolution is entirely impossible, deny.
+        // If argv is non-empty but resolution is entirely impossible, deny.
         // "Ambiguous or inconsistent" → typed deny, not silent allow.
         if (resolution is null && allowlistResolutions.Count == 0)
             return Fail("executable-resolution-failed");

src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2NullHandler.cs

@@ -4,7 +4,7 @@ namespace OpenClaw.Shared.ExecApprovals;
 
 /// <summary>
 /// Default V2 handler: always returns <see cref="ExecApprovalV2Code.Unavailable"/>.
-/// Keeps the V2 path inert until a real handler is installed (rail 19).
+/// Keeps the V2 path inert until a real handler is installed.
 /// Never throws, never falls through to legacy.
 /// </summary>
 public sealed class ExecApprovalV2NullHandler : IExecApprovalV2Handler

src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2Result.cs

@@ -1,7 +1,7 @@
 namespace OpenClaw.Shared.ExecApprovals;
 
 /// <summary>
-/// Stable result codes for the V2 exec approval path (rail 7).
+/// Stable result codes for the V2 exec approval path.
 /// </summary>
 public enum ExecApprovalV2Code
 {

src/OpenClaw.Shared/ExecApprovals/ExecApprovalsContracts.cs

@@ -37,7 +37,6 @@ public sealed class ExecAllowlistEntry
     public Guid? Id { get; set; }
     public string? Pattern { get; set; }
     public double? LastUsedAt { get; set; }
-    public string? LastUsedCommand { get; set; }
     public string? LastResolvedPath { get; set; }
 }
 

src/OpenClaw.Shared/ExecApprovals/ExecApprovalsCoordinator.cs

@@ -8,18 +8,18 @@
 namespace OpenClaw.Shared.ExecApprovals;
 
 // Full coordinator pipeline: validate → normalize → buildContext → evaluate(pass1) →
-// prompt/fallback → [persistAllowlistEntry stub] → evaluate(pass2) → final decision.
-// Rail 10: no WinUI types. Rail 17: SemaphoreSlim serializes the prompt+pass2 block.
-// Rail 19: not wired in production src in PR7 — verified by test 15.
-// Must be registered as singleton when wired (PR8+): the SemaphoreSlim is per-instance.
+// prompt/fallback → evaluate(pass2) → side effects → final decision.
+// UI-free: no WinUI types. A SemaphoreSlim serializes the prompt+pass2 block.
+// Not wired in production src — verified by ProductionWiring_CoordinatorNotReferencedInSrc test.
+// Must be registered as singleton when wired: the SemaphoreSlim is per-instance.
 public sealed class ExecApprovalsCoordinator : IExecApprovalV2Handler
 {
     private readonly ExecApprovalsStore _store;
     private readonly ICanPresentEvaluator _canPresent;
     private readonly IExecApprovalV2PromptHandler _prompt;
     private readonly IOpenClawLogger _logger;
 
-    // Serializes the prompt call + second-pass block (rail 17).
+    // Serializes the prompt call + second-pass block.
     // Does NOT protect validate/normalize/buildContext — those are stateless.
     private readonly SemaphoreSlim _promptLock = new(1, 1);
 
@@ -92,24 +92,28 @@ public async Task<ExecApprovalV2Result> HandleAsync(NodeInvokeRequest request, s
             identity.AllowAlwaysPatterns,
             matches);
 
-        // Step 4: first pass (approvalDecision always null in PR7 — CVE #8682, ADR-0002 Phase 2)
+        // Step 4: first pass (approvalDecision always null — pass2 decides based on user response)
         var pass1 = ExecApprovalEvaluator.Evaluate(context, null);
         if (pass1 is ExecHostPolicyDecision.DenyOutcome denyPass1)
             return LogAndReturn(denyPass1.Error, correlationId,
                 promptAttempted: false, fallbackUsed: false, canonical: context.DisplayCommand);
         if (pass1 is ExecHostPolicyDecision.AllowOutcome)
         {
-            // Pre-approved path (security=Full, ask=Off or allowlist satisfied): skip prompt
+            // Pre-approved path (security=Full, ask=Off or allowlist satisfied): skip prompt.
+            // Side effects are best-effort: a metadata write failure must not flip an allow to a deny.
+            try { await RecordAllowlistUsageAsync(context).ConfigureAwait(false); }
+            catch (Exception ex) { _logger.Warn($"[EXEC-APPROVALS] [{correlationId}] side-effect: record-usage failed (non-fatal): {ex.Message}"); }
             _logger.Info($"[EXEC-APPROVALS] [{correlationId}] path=new " +
                 $"canonical=\"{SanitizeForLog(context.DisplayCommand)}\" decision=allow " +
                 $"reason=approved fallbackUsed=false promptAttempted=false");
             return ExecApprovalV2Result.Allow();
         }
         // RequiresPromptOutcome → continue to prompt/fallback block
 
-        // Steps 5-7: prompt/fallback + second pass (critical section)
+        // Steps 5-8: prompt/fallback + second pass (critical section) + side effect flag
         bool promptAttempted = false;
         bool fallbackUsed = false;
+        bool persistAllowlistEntry = false;
 
         await _promptLock.WaitAsync().ConfigureAwait(false);
         try
@@ -162,8 +166,6 @@ public async Task<ExecApprovalV2Result> HandleAsync(NodeInvokeRequest request, s
                 followupDecision = FallbackDecision(context, resolved.Defaults.AskFallback);
             }
 
-            // Step 6: AddAllowlistEntry stub (PR9 implements for AllowAlways + security==Allowlist)
-
             // Step 7: second pass — must never return RequiresPrompt
             var pass2 = ExecApprovalEvaluator.Evaluate(context, followupDecision);
             if (pass2 is ExecHostPolicyDecision.DenyOutcome denyPass2)
@@ -176,14 +178,23 @@ public async Task<ExecApprovalV2Result> HandleAsync(NodeInvokeRequest request, s
                 return LogAndReturn(ExecApprovalV2Result.InternalError("second-pass-requires-prompt"),
                     correlationId, promptAttempted, fallbackUsed, canonical: context.DisplayCommand);
             }
-            // AllowOutcome → fall through to steps 8-10
+            // pass2 is AllowOutcome — record whether AllowAlways was the prompt decision.
+            persistAllowlistEntry = followupDecision == ExecApprovalDecision.AllowAlways;
         }
         finally
         {
             _promptLock.Release();
         }
 
-        // Step 8: RecordAllowlistUse stub (PR9)
+        // Step 8: side effects — strictly after the final allow decision.
+        // Each side effect is independently best-effort so a failure in one does not skip the other.
+        if (persistAllowlistEntry && context.Security == ExecSecurity.Allowlist)
+        {
+            try { await PersistAllowlistEntriesAsync(context).ConfigureAwait(false); }
+            catch (Exception ex) { _logger.Warn($"[EXEC-APPROVALS] [{correlationId}] side-effect: persist-entry failed (non-fatal): {ex.Message}"); }
+        }
+        try { await RecordAllowlistUsageAsync(context).ConfigureAwait(false); }
+        catch (Exception ex) { _logger.Warn($"[EXEC-APPROVALS] [{correlationId}] side-effect: record-usage failed (non-fatal): {ex.Message}"); }
 
         // Step 9: final allow log
         _logger.Info($"[EXEC-APPROVALS] [{correlationId}] path=new " +
@@ -197,7 +208,7 @@ public async Task<ExecApprovalV2Result> HandleAsync(NodeInvokeRequest request, s
         {
             // Outer safety net: any unhandled exception in buildContext, CanPresent, FallbackDecision,
             // or an out-of-range prompt outcome produces a typed deny instead of escaping HandleAsync.
-            // Rail 1: failures in the new path must never be silent or untyped.
+            // Failures must never be silent or untyped.
             var msg = $"[EXEC-APPROVALS] [{correlationId}] path=new " +
                 $"canonical=\"\" decision=deny reason=unexpected-exception " +
                 $"fallbackUsed=false promptAttempted=false";
@@ -206,6 +217,37 @@ public async Task<ExecApprovalV2Result> HandleAsync(NodeInvokeRequest request, s
         }
     }
 
+    // Persists allowAlways patterns after an AllowAlways prompt decision (non-empty only).
+    // Caller guarantees Security == Allowlist (guard is in HandleAsync step 8).
+    private async Task PersistAllowlistEntriesAsync(ExecApprovalEvaluation context)
+    {
+        var seen = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+        foreach (var pattern in context.AllowAlwaysPatterns)
+        {
+            if (string.IsNullOrWhiteSpace(pattern) || !seen.Add(pattern)) continue;
+            await _store.AddAllowlistEntryAsync(context.AgentId, pattern).ConfigureAwait(false);
+        }
+    }
+
+    // Updates lastUsed* metadata for every matched allowlist entry after a final allow.
+    // Guard mirrors macOS recordAllowlistMatches: no-op unless security=allowlist and satisfied.
+    private async Task RecordAllowlistUsageAsync(ExecApprovalEvaluation context)
+    {
+        if (context.Security != ExecSecurity.Allowlist || !context.AllowlistSatisfied) return;
+        var seen = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+        for (var i = 0; i < context.AllowlistMatches.Count; i++)
+        {
+            var pattern = context.AllowlistMatches[i].Pattern;
+            if (string.IsNullOrEmpty(pattern) || !seen.Add(pattern)) continue;
+            var resolvedPath = i < context.AllowlistResolutions.Count
+                ? context.AllowlistResolutions[i].ResolvedPath
+                : null;
+            await _store.RecordAllowlistUseAsync(
+                context.AgentId, pattern, resolvedPath)
+                .ConfigureAwait(false);
+        }
+    }
+
     // Fail-safe defaults when no UI is available (Saltzer/Schroeder fail-safe defaults, OWASP ASVS 4.1.4).
     // ask=Always → Deny: human approval is a precondition; without UI the only safe outcome is deny.
     private static ExecApprovalDecision FallbackDecision(
@@ -230,15 +272,15 @@ private static ExecApprovalV2PromptRequest BuildPromptRequest(
         string correlationId)
         => new()
         {
-            DisplayCommand = context.DisplayCommand,  // NOT sanitized — presenter's responsibility (rail 11)
+            DisplayCommand = context.DisplayCommand,  // NOT sanitized — presenter's responsibility
             Cwd = identity.Cwd,
             Security = context.Security,
             Ask = context.Ask,
             AgentId = context.AgentId ?? "main",
             ResolvedPath = context.Resolution?.ResolvedPath,
             SessionKey = identity.SessionKey,
             CorrelationId = correlationId,
-            // Host omitted in PR7 (no gateway wiring yet)
+            // Host omitted (no gateway wiring yet)
         };
 
     // Anti log-injection: replaces control characters in DisplayCommand before writing to logs.