From cc94b3a68c204f7b93c04ea43eeea546931b66e6 Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Mon, 13 Jul 2026 09:51:30 -0400
Subject: [PATCH] fix(esg): park buffered calls that fail deterministically
 with ArgumentException (path template/verb) instead of retrying to exhaustion
 (plan R2-06 T3)

---
 .../ExternalSystemClient.cs                   | 21 ++++++++++-
 .../ExternalSystemClientTests.cs              | 36 +++++++++++++++++++
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ExternalSystemClient.cs b/src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ExternalSystemClient.cs
index 9144b0e7..c70920ce 100644
--- a/src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ExternalSystemClient.cs
+++ b/src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ExternalSystemClient.cs
@@ -166,7 +166,11 @@ public class ExternalSystemClient : IExternalSystemClient
     /// Delivers a buffered ExternalSystem call during a store-and-forward
     /// retry sweep. Returns true on success, false on permanent failure (the message
     /// is parked); throws <see cref="TransientExternalSystemException"/> on a
-    /// transient failure so the engine retries.
+    /// transient failure so the engine retries. Deterministic-poison failures against
+    /// the CURRENT method definition — malformed JSON, or an
+    /// <see cref="ArgumentException"/> from an unresolvable <c>{param}</c> path
+    /// template or an unsupported HTTP verb — return false (park immediately) rather
+    /// than retry, because re-running the same stored payload can never succeed.
     /// </summary>
     /// <param name="message">The buffered message to deliver.</param>
     /// <param name="cancellationToken">Cancellation token.</param>
@@ -221,6 +225,21 @@ public class ExternalSystemClient : IExternalSystemClient
             _logger.LogError(ex, "Buffered call to '{System}' failed permanently; parking.", payload.SystemName);
             return false;
         }
+        catch (ArgumentException ex)
+        {
+            // N2: same poison-message shape as the JsonException catch above. BuildUrl's
+            // {param} path-template substitution (placeholder with no matching/non-null
+            // parameter) and ValidateHttpMethod both throw ArgumentException
+            // deterministically for the SAME stored payload — the method definition
+            // changed underneath the buffered call, and retrying can never succeed.
+            // Return false so the S&F engine parks the message instead of counting the
+            // throw as transient and burning MaxRetries.
+            _logger.LogError(
+                ex,
+                "Buffered call to '{System}'/'{Method}' fails deterministically against the current method definition; parking.",
+                payload.SystemName, payload.MethodName);
+            return false;
+        }
         // TransientExternalSystemException propagates — the S&F engine retries.
     }
 
diff --git a/tests/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.Tests/ExternalSystemClientTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.Tests/ExternalSystemClientTests.cs
index 514380df..fcbfe7c2 100644
--- a/tests/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.Tests/ExternalSystemClientTests.cs
+++ b/tests/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.Tests/ExternalSystemClientTests.cs
@@ -1341,4 +1341,40 @@ public class ExternalSystemClientTests
             () => client.CallAsync("TestAPI", "getRecipe"));
         Assert.Contains("id", ex.Message);
     }
+
+    [Fact]
+    public async Task DeliverBuffered_PathTemplateNowUnresolvable_ReturnsFalseSoMessageParks()
+    {
+        // Arch-review R2 N2: a method whose path gained a {param} placeholder AFTER
+        // calls were buffered throws ArgumentException deterministically for every
+        // already-buffered message — it must park immediately (permanent), not burn
+        // MaxRetries "transient" attempts (the S&F sweep's catch-all retries any throw).
+        var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
+        var method = new ExternalSystemMethod("getRecipe", "GET", "/recipes/{id}") { Id = 1, ExternalSystemDefinitionId = 1 };
+        StubResolution(system, method);
+        _httpClientFactory.CreateClient(Arg.Any<string>())
+            .Returns(new HttpClient(new MockHttpMessageHandler(HttpStatusCode.OK, "{}")));
+        var client = new ExternalSystemClient(
+            _httpClientFactory, _repository, NullLogger<ExternalSystemClient>.Instance);
+
+        // BufferedCall carries Parameters:null — the {id} placeholder has no value.
+        var delivered = await client.DeliverBufferedAsync(BufferedCall("TestAPI", "getRecipe"));
+
+        Assert.False(delivered);
+    }
+
+    [Fact]
+    public async Task DeliverBuffered_UnsupportedVerbOnStoredMethod_ReturnsFalseSoMessageParks()
+    {
+        // ValidateHttpMethod throws the same deterministic ArgumentException shape.
+        var system = new ExternalSystemDefinition("TestAPI", "https://api.example.com", "none") { Id = 1 };
+        var method = new ExternalSystemMethod("oddVerb", "FOO", "/p") { Id = 1, ExternalSystemDefinitionId = 1 };
+        StubResolution(system, method);
+        var client = new ExternalSystemClient(
+            _httpClientFactory, _repository, NullLogger<ExternalSystemClient>.Instance);
+
+        var delivered = await client.DeliverBufferedAsync(BufferedCall("TestAPI", "oddVerb"));
+
+        Assert.False(delivered);
+    }
 }