fix(sandbox): make a post-commit response-read failure non-retryable

The bridge forward reads the host response body after the fetch resolves. A
mutating request may have committed on the host by then. A body over the size
limit, or a stream read error, made the forward reject. The duplex broker and
the file bridge both mapped that rejection to a retryable 502. A retry then sent
a new request id outside the broker deduplication set, so the host applied the
mutation twice.

The forward now catches a response-body read failure and returns a non-retryable
504 with the indeterminate outcome marker. The in-sandbox server maps the
indeterminate 504 to a non-retryable 409 for both transports, so a caller never
repeats a possibly-committed mutation. Add broker and integration coverage.

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

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

@@ -367,4 +367,56 @@ describe("duplex bridge broker request limits", () => {
     expect(harness.forwards.filter((forward) => forward.id === maxId)).toHaveLength(1);
     expect(broker.state).toBe("open");
   });
+
+  it("maps a forward that resolves with the indeterminate marker to a non-retryable 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 handler resolves with a 504 that carries the indeterminate
+    // marker. This is the shape the host forward returns for a response-read
+    // failure after the host commit. The broker must keep the marker and the
+    // non-retryable body, so the gateway maps it to a non-retryable status and a
+    // caller never repeats a possibly-committed mutation.
+    harness.feed(requestFrame("commit-1"));
+    const forward = harness.forwards.find((entry) => entry.id === "commit-1" && !entry.settled);
+    if (!forward) throw new Error("The broker did not forward the request.");
+    forward.settled = true;
+    forward.resolve({
+      status: 504,
+      headers: {
+        "content-type": "application/json",
+        "x-paperclip-bridge-outcome": "indeterminate",
+      },
+      body: JSON.stringify({
+        error: "Bridge response body exceeded the configured size limit of 32 bytes.",
+        outcome: "indeterminate",
+        retryable: false,
+      }),
+    });
+    // The broker answers inside the forward `then` microtask, so let it settle.
+    await Promise.resolve();
+
+    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: "Bridge response body exceeded the configured size limit of 32 bytes.",
+      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("commit-1"));
+    expect(harness.forwards.filter((entry) => entry.id === "commit-1")).toHaveLength(1);
+    expect(broker.state).toBe("open");
+  });
 });

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

@@ -535,9 +535,12 @@ export function createDuplexBridgeBroker(options: DuplexBrokerOptions): DuplexBr
           );
           return;
         }
-        // A plain forward failure did not start a host mutation, so a retry is
-        // safe. Return a 502 with the completed outcome, so the gateway passes it
-        // through as a retryable status.
+        // 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.
         respond(
           frame.id,
           {

packages/adapter-utils/src/execution-target-sandbox.test.ts

@@ -2495,7 +2495,14 @@ describe("sandbox adapter execution targets", () => {
     }
   });
 
-  it("fails oversized host responses with a 502 before returning them to the sandbox client", async () => {
+  it("fails an oversized host response with a non-retryable 409 so a committed mutation never repeats", async () => {
+    // The host receives the request and commits the mutation, then sends a
+    // response body over the size limit. The forward reads the body after the
+    // fetch resolves, so the read failure happens after the host commit. The
+    // forward must return a non-retryable 504 with the indeterminate outcome, not
+    // a retryable 502. The in-sandbox server maps the indeterminate 504 to a
+    // non-retryable 409. A retryable status would repeat the mutation with a new
+    // request id outside the broker deduplication set.
     const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-execution-target-bridge-limit-"));
     cleanupDirs.push(rootDir);
     const remoteCwd = path.join(rootDir, "workspace");
@@ -2503,7 +2510,10 @@ describe("sandbox adapter execution targets", () => {
     await mkdir(runtimeRootDir, { recursive: true });
 
     const requests: Array<{ method: string; url: string; auth: string | null; runId: string | null }> = [];
-    const largeBody = "x".repeat(64);
+    // The host body sits over the size limit, so the forward read fails. The
+    // limit stays above the small indeterminate marker the forward returns, so the
+    // marker still reaches the server for the 504-to-409 map.
+    const largeBody = "x".repeat(1024);
     const apiServer = createServer((req, res) => {
       requests.push({
         method: req.method ?? "GET",
@@ -2511,7 +2521,7 @@ describe("sandbox adapter execution targets", () => {
         auth: req.headers.authorization ?? null,
         runId: typeof req.headers["x-paperclip-run-id"] === "string" ? req.headers["x-paperclip-run-id"] : null,
       });
-      res.writeHead(200, {
+      res.writeHead(201, {
         "content-type": "application/json",
         "content-length": String(Buffer.byteLength(largeBody, "utf8")),
       });
@@ -2544,23 +2554,31 @@ describe("sandbox adapter execution targets", () => {
       adapterKey: "codex",
       hostApiToken: "real-run-jwt",
       hostApiUrl: `http://127.0.0.1:${address.port}`,
-      maxBodyBytes: 32,
+      maxBodyBytes: 512,
     });
     try {
-      const response = await fetch(`${bridge!.env.PAPERCLIP_API_URL}/api/agents/me`, {
+      const response = await fetch(`${bridge!.env.PAPERCLIP_API_URL}/api/issues/issue-1/comments`, {
+        method: "POST",
         headers: {
           authorization: `Bearer ${bridge!.env.PAPERCLIP_API_KEY}`,
-          accept: "application/json",
+          "content-type": "application/json",
         },
+        body: JSON.stringify({ body: "Status update." }),
       });
 
-      expect(response.status).toBe(502);
+      // The indeterminate 504 maps to a non-retryable 409, so the caller does not
+      // retry the committed mutation.
+      expect(response.status).toBe(409);
+      expect(response.headers.get("x-paperclip-bridge-outcome")).toBe("indeterminate");
       await expect(response.json()).resolves.toEqual({
-        error: "Bridge response body exceeded the configured size limit of 32 bytes.",
+        error: "Bridge response body exceeded the configured size limit of 512 bytes.",
+        outcome: "indeterminate",
+        retryable: false,
       });
+      // The host ran the mutation exactly once. It never receives a retry.
       expect(requests).toEqual([{
-        method: "GET",
-        url: "/api/agents/me",
+        method: "POST",
+        url: "/api/issues/issue-1/comments",
         auth: "Bearer real-run-jwt",
         runId: "run-bridge-limit",
       }]);

packages/adapter-utils/src/execution-target.ts

@@ -2631,7 +2631,32 @@ export async function startAdapterExecutionTargetPaperclipBridge(input: {
         `[paperclip] Bridge proxy response ${response.status} for ${method} ${request.path}${request.query ? `?${request.query}` : ""}\n`,
       );
     }
-    const responseBody = await readBridgeForwardResponseBody(response, maxBodyBytes);
+    // The host delivered response headers, so a mutating request may have
+    // committed on the host. A later response-body read failure (a body over the
+    // size limit, or a stream read error) must not surface as a retryable status.
+    // A retryable status makes the caller repeat the request with a new request
+    // id outside the broker deduplication set, so the host applies the mutation
+    // twice. Return a non-retryable 504 and mark the outcome indeterminate,
+    // exactly like an aborted in-flight forward. The in-sandbox server maps the
+    // indeterminate 504 to a non-retryable 409 for both the file bridge and the
+    // duplex broker.
+    let responseBody: string;
+    try {
+      responseBody = await readBridgeForwardResponseBody(response, maxBodyBytes);
+    } catch (error) {
+      return {
+        status: 504,
+        headers: {
+          "content-type": "application/json",
+          "x-paperclip-bridge-outcome": "indeterminate",
+        },
+        body: JSON.stringify({
+          error: error instanceof Error ? error.message : String(error),
+          outcome: "indeterminate",
+          retryable: false,
+        }),
+      };
+    }
     const commentMarker = postedIssueCommentLogMarker(method, request.path, response.status, responseBody);
     if (commentMarker) await onLog("stdout", commentMarker);
     return {