fix(sandbox): make a pre-response duplex forward failure non-retryable for a mutating method

The duplex broker forwarded each request and mapped a forward rejection to a
retryable 502 when its controller was not aborted. A fetch can reject after the
request bytes reach the host and the host commits a mutation, but before the
response headers arrive. A retry then sent a new request id outside the broker
deduplication set, so the host applied the mutation twice.

The broker now classifies a non-aborted forward rejection by method safety. A
safe method (GET, HEAD, OPTIONS, TRACE) never changes host state, so its
rejection stays a retryable 502. Any other method may have committed, so the
broker returns a non-retryable 504 and marks the outcome indeterminate. The
in-sandbox gateway maps the indeterminate outcome to a terminal 409, so a caller
never repeats a possibly-committed mutation. Add broker coverage for both paths.

Co-authored-by: Paperclip <noreply@paperclip.ing>

packages/adapter-utils/src/duplex-bridge-broker.test.ts

@@ -57,12 +57,12 @@ interface FakeChannelHarness {
 }
 
 /** Build one valid request frame with distinctive, secret-looking fields. */
-function requestFrame(id: string): DuplexRequestFrame {
+function requestFrame(id: string, method = "POST"): DuplexRequestFrame {
   return {
     version: DUPLEX_FRAME_VERSION,
     type: "request",
     id,
-    method: "POST",
+    method,
     path: `/api/issues/${id}`,
     query: "?secret-query=leak",
     headers: { authorization: "Bearer super-secret-provider-token" },
@@ -419,4 +419,74 @@ describe("duplex bridge broker request limits", () => {
     expect(harness.forwards.filter((entry) => entry.id === "commit-1")).toHaveLength(1);
     expect(broker.state).toBe("open");
   });
+
+  it("maps a forward rejection for a mutating method to a non-retryable indeterminate response and retains the id", async () => {
+    const harness = createFakeChannelHarness();
+    const broker = createDuplexBridgeBroker({
+      channel: harness.channel,
+      forwardRequest: controllableForward(harness),
+    });
+    brokers.push(broker);
+    broker.start();
+
+    // The forward rejects before the host delivers a response. A fetch can
+    // reject after the request reaches the host and the host commits, but
+    // before the response headers arrive. The broker did not abort the
+    // forward, so a POST must return a non-retryable indeterminate response.
+    // A retryable status would let a caller repeat a committed mutation.
+    harness.feed(requestFrame("mutate-1", "POST"));
+    const forward = harness.forwards.find((entry) => entry.id === "mutate-1" && !entry.settled);
+    if (!forward) throw new Error("The broker did not forward the request.");
+    forward.settled = true;
+    forward.reject(new Error("fetch failed before the response headers arrived"));
+    // The broker answers inside the forward rejection microtask, so let it settle.
+    await new Promise((resolve) => setTimeout(resolve, 0));
+
+    expect(harness.responses).toHaveLength(1);
+    const response = harness.responses[0]!;
+    expect(response.status).toBe(504);
+    expect(response.outcome).toBe("indeterminate");
+    expect(response.headers["x-paperclip-bridge-outcome"]).toBe("indeterminate");
+    expect(JSON.parse(response.body)).toEqual({
+      error: "fetch failed before the response headers arrived",
+      outcome: "indeterminate",
+      retryable: false,
+    });
+
+    // The broker delivered a response, so it retained the id. A resend never
+    // reaches the forward a second time, so a caller that ignores the
+    // non-retryable status still cannot repeat the mutation through the broker.
+    harness.feed(requestFrame("mutate-1", "POST"));
+    expect(harness.forwards.filter((entry) => entry.id === "mutate-1")).toHaveLength(1);
+    expect(broker.state).toBe("open");
+  });
+
+  it("maps a forward rejection for a safe method to a retryable response", async () => {
+    const harness = createFakeChannelHarness();
+    const broker = createDuplexBridgeBroker({
+      channel: harness.channel,
+      forwardRequest: controllableForward(harness),
+    });
+    brokers.push(broker);
+    broker.start();
+
+    // A safe method never changes host state, so a retry cannot double-apply a
+    // mutation. A forward rejection for a GET stays retryable: the broker
+    // returns a 502 with the completed outcome, so the gateway passes it through.
+    harness.feed(requestFrame("read-1", "GET"));
+    const forward = harness.forwards.find((entry) => entry.id === "read-1" && !entry.settled);
+    if (!forward) throw new Error("The broker did not forward the request.");
+    forward.settled = true;
+    forward.reject(new Error("fetch failed before the response headers arrived"));
+    await new Promise((resolve) => setTimeout(resolve, 0));
+
+    expect(harness.responses).toHaveLength(1);
+    const response = harness.responses[0]!;
+    expect(response.status).toBe(502);
+    expect(response.outcome).toBe("completed");
+    expect(response.headers["x-paperclip-bridge-outcome"]).toBeUndefined();
+    expect(JSON.parse(response.body)).toEqual({
+      error: "fetch failed before the response headers arrived",
+    });
+  });
 });

packages/adapter-utils/src/duplex-bridge-broker.ts

@@ -258,6 +258,18 @@ function errorMessage(error: unknown): string {
   return error instanceof Error ? error.message : String(error);
 }
 
+/**
+ * The safe HTTP methods. RFC 7231 section 4.2.1 defines this set. A safe method
+ * does not change host state, so the host applies no mutation for it. A caller
+ * can retry a safe method after a forward failure without a double-apply risk.
+ */
+const SAFE_BRIDGE_METHODS = new Set(["GET", "HEAD", "OPTIONS", "TRACE"]);
+
+/** Report whether the method is safe, so a forward failure stays retryable. */
+function isSafeBridgeMethod(method: string): boolean {
+  return SAFE_BRIDGE_METHODS.has(method.trim().toUpperCase());
+}
+
 /** The internal bookkeeping for one in-flight request. */
 interface PendingRequest {
   controller: AbortController;
@@ -535,20 +547,46 @@ export function createDuplexBridgeBroker(options: DuplexBrokerOptions): DuplexBr
           );
           return;
         }
-        // A rejection here means the forward never received a host response, so
-        // the host did not start a mutation and a retry is safe. The forward
-        // handler owns the post-response case: it converts a response-read
-        // failure into a resolved indeterminate result, so a possibly-committed
-        // mutation never reaches this branch. Return a 502 with the completed
-        // outcome, so the gateway passes it through as a retryable status.
+        // The forward rejected before the host delivered a response. This
+        // rejection does not prove that the host applied no mutation. A fetch
+        // can reject after the request bytes reach the host and the host
+        // commits, but before the response headers arrive. A safe method never
+        // changes host state, so a retry stays safe for it. For any other
+        // method the host may have committed, so the outcome is indeterminate.
+        if (isSafeBridgeMethod(frame.method)) {
+          // The method is safe, so a retry cannot double-apply a mutation.
+          // Return a 502 with the completed outcome, so the gateway passes it
+          // through as a retryable status.
+          respond(
+            frame.id,
+            {
+              status: 502,
+              headers: { "content-type": "application/json" },
+              body: JSON.stringify({ error: errorMessage(error) }),
+            },
+            "completed",
+            "error",
+          );
+          return;
+        }
+        // The method may mutate host state, so a retry with a new request id
+        // could apply the mutation twice. Return a non-retryable 504 and mark
+        // the outcome indeterminate, so the gateway maps it to a terminal 409.
         respond(
           frame.id,
           {
-            status: 502,
-            headers: { "content-type": "application/json" },
-            body: JSON.stringify({ error: errorMessage(error) }),
+            status: 504,
+            headers: {
+              "content-type": "application/json",
+              "x-paperclip-bridge-outcome": "indeterminate",
+            },
+            body: JSON.stringify({
+              error: errorMessage(error),
+              outcome: "indeterminate",
+              retryable: false,
+            }),
           },
-          "completed",
+          "indeterminate",
           "error",
         );
       },