fix(adapter-utils): answer 413, not a retryable 503, when a body-read reserve can never fit

reserveBodyRead answered 503 bridge_busy for two different conditions: the
bound is transiently full (a retry can succeed) and the read's own charge
is larger than the whole bound (no retry ever succeeds). The second case
invites the same endless-retry defect the earlier 502-on-oversize-body fix
closed.

Split the two. reserveBodyRead now returns a RESERVE_NEVER_FITS marker when
reserveBytes alone exceeds maxInflightBodyBytes, and both gateways answer a
non-retryable 413 request_too_large for that marker, before they take any
reserve. They keep the existing 503 bridge_busy for the transient case.

In-spec traffic at the production defaults does not reach the never-fits
branch (the file gateway's worst-case charge is about 42 MiB against a
64 MiB bound); this is a backstop, so a future factor change that breaks
that property fails loudly with a clean 413 instead of turning legal
traffic into a retry loop.

Also add explicit coverage for the three real no-declared-length request
shapes: a DELETE with no body sends no Content-Length header; a POST with a
streamed body sends chunked transfer encoding and no Content-Length; a POST
with an explicit empty body sends Content-Length: 0. All three now reach
the host worker at the production inflight default, not a 503.

Verified: 45 tests in sandbox-callback-bridge.test.ts (2 new), 19 in
duplex-bridge-local-e2e.test.ts, 136 in execution-target-sandbox.test.ts,
and 2 in execution-target-ssh-buffer.test.ts pass. npx tsc --noEmit on
packages/adapter-utils is clean.

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

packages/adapter-utils/src/sandbox-callback-bridge.test.ts

@@ -1244,6 +1244,183 @@ describe("sandbox callback bridge", () => {
     expect(receivedBody).toBe(body);
   }, 20000);
 
+  it("verifies the real no-declared-length request shapes are all admitted at production defaults: DELETE, chunked POST, and empty-body POST", async () => {
+    // Coverage note from the PAP-5041 scope amendment: verify the real
+    // header shapes a client sends, do not assume them. A DELETE with no
+    // body sends no Content-Length header at all. A POST with a streamed
+    // body sends chunked transfer encoding and no Content-Length (covered by
+    // the test above). A POST with an explicit empty body sends
+    // "Content-Length: 0" -- a known, zero, declared length, so it takes the
+    // exact-size readBody path rather than the unknown-length one. All three
+    // must reach the host worker, not a 503, at the production inflight
+    // default.
+    const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-bridge-header-shapes-"));
+    cleanupDirs.push(rootDir);
+
+    const localWorkspaceDir = path.join(rootDir, "local-workspace");
+    const remoteWorkspaceDir = path.join(rootDir, "remote-workspace");
+    await mkdir(localWorkspaceDir, { recursive: true });
+    await mkdir(remoteWorkspaceDir, { recursive: true });
+    await writeFile(path.join(localWorkspaceDir, "README.md"), "bridge header shapes test\n", "utf8");
+
+    const runner = createExecRunner();
+    const bridgeAsset = await createSandboxCallbackBridgeAsset();
+    cleanupFns.push(bridgeAsset.cleanup);
+    const prepared = await prepareCommandManagedRuntime({
+      runner,
+      spec: {
+        remoteCwd: remoteWorkspaceDir,
+        timeoutMs: 30_000,
+      },
+      adapterKey: "codex",
+      workspaceLocalDir: localWorkspaceDir,
+      assets: [{ key: "bridge", localDir: bridgeAsset.localDir }],
+    });
+
+    const queueDir = path.posix.join(prepared.runtimeRootDir, "paperclip-bridge");
+    const bridgeToken = createSandboxCallbackBridgeToken();
+
+    const seenMethods: string[] = [];
+    const worker = await startSandboxCallbackBridgeWorker({
+      client: createFileSystemSandboxCallbackBridgeQueueClient(),
+      queueDir,
+      authorizeRequest: async () => null,
+      handleRequest: async (request) => {
+        seenMethods.push(request.method);
+        return {
+          status: 200,
+          headers: { "content-type": "application/json" },
+          body: JSON.stringify({ ok: true }),
+        };
+      },
+    });
+    cleanupFns.push(async () => {
+      await worker.stop();
+    });
+
+    const bridge = await startSandboxCallbackBridgeServer({
+      runner,
+      remoteCwd: remoteWorkspaceDir,
+      assetRemoteDir: prepared.assetDirs.bridge,
+      queueDir,
+      bridgeToken,
+      timeoutMs: 30_000,
+      pollIntervalMs: 10,
+      // maxInflightBodyBytes stays at the production default (64 MiB).
+    });
+    cleanupFns.push(async () => {
+      await bridge.stop();
+    });
+
+    const deleteResponse = await fetch(`${bridge.baseUrl}/api/issues/issue-1`, {
+      method: "DELETE",
+      headers: {
+        authorization: `Bearer ${bridgeToken}`,
+        "content-type": "application/json",
+      },
+    });
+    expect(deleteResponse.status).not.toBe(503);
+
+    const emptyBodyResponse = await fetch(`${bridge.baseUrl}/api/issues/issue-1/comments`, {
+      method: "POST",
+      headers: {
+        authorization: `Bearer ${bridgeToken}`,
+        "content-type": "application/json",
+      },
+      body: "",
+    });
+    expect(emptyBodyResponse.status).not.toBe(503);
+
+    const chunkedResponse = await postWithoutContentLength(
+      `${bridge.baseUrl}/api/issues/issue-1/comments`,
+      {
+        authorization: `Bearer ${bridgeToken}`,
+        "content-type": "application/json",
+      },
+      JSON.stringify({ body: "a chunked comment" }),
+    );
+    expect(chunkedResponse.status).not.toBe(503);
+
+    expect(seenMethods).toEqual(["DELETE", "POST", "POST"]);
+  }, 20000);
+
+  it("answers a non-retryable 413, not a retryable 503, when a single read's charge alone can never fit the inflight bound", async () => {
+    // Regression coverage for the PAP-5041 scope amendment's criterion 9:
+    // reserveBodyRead must not answer a retryable 503 for a read that can
+    // never fit the bound on its own -- no amount of waiting admits it, so a
+    // caller that retries loops forever. This pins maxInflightBodyBytes well
+    // under a single read's charge, on an otherwise idle gateway, so the
+    // bound is never transiently full -- the charge alone is the only reason
+    // admission fails.
+    const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-bridge-never-fits-"));
+    cleanupDirs.push(rootDir);
+
+    const localWorkspaceDir = path.join(rootDir, "local-workspace");
+    const remoteWorkspaceDir = path.join(rootDir, "remote-workspace");
+    await mkdir(localWorkspaceDir, { recursive: true });
+    await mkdir(remoteWorkspaceDir, { recursive: true });
+    await writeFile(path.join(localWorkspaceDir, "README.md"), "bridge never-fits test\n", "utf8");
+
+    const runner = createExecRunner();
+    const bridgeAsset = await createSandboxCallbackBridgeAsset();
+    cleanupFns.push(bridgeAsset.cleanup);
+    const prepared = await prepareCommandManagedRuntime({
+      runner,
+      spec: {
+        remoteCwd: remoteWorkspaceDir,
+        timeoutMs: 30_000,
+      },
+      adapterKey: "codex",
+      workspaceLocalDir: localWorkspaceDir,
+      assets: [{ key: "bridge", localDir: bridgeAsset.localDir }],
+    });
+
+    const queueDir = path.posix.join(prepared.runtimeRootDir, "paperclip-bridge");
+    const bridgeToken = createSandboxCallbackBridgeToken();
+
+    const maxBodyBytes = 4096;
+    const bodySize = 1024;
+    // reserveCharge (1024) * FACTOR (4) + EXTRA_BYTES (393216) is about
+    // 397 KiB. Pin maxInflightBodyBytes far under that, so the charge alone
+    // -- not contention from another read -- is why admission fails.
+    const maxInflightBodyBytes = 1024;
+    const bridge = await startSandboxCallbackBridgeServer({
+      runner,
+      remoteCwd: remoteWorkspaceDir,
+      assetRemoteDir: prepared.assetDirs.bridge,
+      queueDir,
+      bridgeToken,
+      timeoutMs: 30_000,
+      pollIntervalMs: 10,
+      maxBodyBytes,
+      maxInflightBodyBytes,
+    });
+    cleanupFns.push(async () => {
+      await bridge.stop();
+    });
+
+    const body = "a".repeat(bodySize);
+    const post = () =>
+      fetch(`${bridge.baseUrl}/api/issues/issue-1/comments`, {
+        method: "POST",
+        headers: {
+          authorization: `Bearer ${bridgeToken}`,
+          "content-type": "application/json",
+        },
+        body,
+      });
+
+    const first = await post();
+    expect(first.status).toBe(413);
+    await expect(first.json()).resolves.toEqual({ error: "request_too_large" });
+
+    // A never-fits read takes no reserve, so a second, otherwise-identical
+    // request also gets an immediate 413, not a transient 503 from a
+    // phantom reservation the first request supposedly left behind.
+    const second = await post();
+    expect(second.status).toBe(413);
+  }, 20000);
+
   it("streams the escaped request body correctly across a write-slice boundary, including a split surrogate pair", async () => {
     // Regression coverage for the streamed write in runFileGateway: it must
     // reassemble a body that crosses FILE_GATEWAY_BODY_WRITE_SLICE_CHARS

packages/adapter-utils/src/sandbox-callback-bridge.ts

@@ -2365,15 +2365,32 @@ const DUPLEX_GATEWAY_BODY_READ_RESERVE_EXTRA_BYTES = DEFAULT_MAX_DUPLEX_FRAME_BY
 let reservedBodyBytes = 0;
 const noopRelease = () => {};
 
-// Reserve reserveBytes against the local admission bound for one active read.
-// Return a release function on success, or null when the bound is full. Each
-// caller releases on every terminal path: success, size rejection, other
-// error, timeout, and connection close.
+// reserveBodyRead returns this marker when reserveBytes alone -- with no
+// other read competing for the bound -- is larger than the whole inflight
+// bound. No amount of waiting admits that read, so a caller must answer a
+// non-retryable 413, not a retryable 503: the same reasoning that turned the
+// old retryable 502 on an oversize body into a clean 413. In-spec traffic
+// never reaches this branch once the reserve charge matches the true
+// retained peak; it exists as a backstop, so a future factor change that
+// breaks that property fails loudly instead of turning legal traffic into
+// an endless retry loop.
+const RESERVE_NEVER_FITS = Symbol("reserve_never_fits");
+
+// Reserve reserveBytes against the local admission bound for one active
+// read. Return a release function on success, RESERVE_NEVER_FITS when
+// reserveBytes alone can never fit the bound (see its comment), or null when
+// the bound is only transiently full -- other active reads hold it, and a
+// retry can succeed, so 503 is the right answer for that case. Each caller
+// releases the returned function on every terminal path: success, size
+// rejection, other error, timeout, and connection close.
 //
 // A caller passes the bytes the read will actually retain -- the declared
 // body size times the calling gateway's representation factor, plus that
 // gateway's fixed per-read term -- not a flat per-request charge.
 function reserveBodyRead(reserveBytes) {
+  if (reserveBytes > maxInflightBodyBytes) {
+    return RESERVE_NEVER_FITS;
+  }
   if (reservedBodyBytes + reserveBytes > maxInflightBodyBytes) {
     return null;
   }
@@ -2562,13 +2579,18 @@ async function runFileGateway() {
       // the same worst-case charge the old flat model always used -- but
       // readBody itself never preallocates that worst case; see its comment.
       const reserveCharge = hasRequestBody ? (declaredLength !== null ? declaredLength : maxBodyBytes) : 0;
-      const releaseBodyRead = hasRequestBody
+      const reserved = hasRequestBody
         ? reserveBodyRead(reserveCharge * FILE_GATEWAY_BODY_RESERVE_FACTOR + FILE_GATEWAY_BODY_RESERVE_EXTRA_BYTES)
         : noopRelease;
-      if (!releaseBodyRead) {
+      if (reserved === RESERVE_NEVER_FITS) {
+        writeJsonResponse(res, 413, { error: "request_too_large" });
+        return;
+      }
+      if (!reserved) {
         writeJsonResponse(res, 503, { error: "bridge_busy" });
         return;
       }
+      const releaseBodyRead = reserved;
       try {
       const requestBody = decodeUtf8Body(await readBody(req, declaredLength, maxBodyBytes));
       const requestPath = path.posix.join(requestsDir, \`\${requestId}.json\`);
@@ -2928,13 +2950,18 @@ function runDuplexGateway() {
       // the same worst-case charge as before -- but readBody itself never
       // preallocates that worst case; see its comment.
       const reserveCharge = hasRequestBody ? (declaredLength !== null ? declaredLength : maxBodyBytes) : 0;
-      const releaseBodyRead = hasRequestBody
+      const reserved = hasRequestBody
         ? reserveBodyRead(reserveCharge + DUPLEX_GATEWAY_BODY_READ_RESERVE_EXTRA_BYTES)
         : noopRelease;
-      if (!releaseBodyRead) {
+      if (reserved === RESERVE_NEVER_FITS) {
+        writeJsonResponse(res, 413, { error: "request_too_large" });
+        return;
+      }
+      if (!reserved) {
         writeJsonResponse(res, 503, { error: "bridge_busy" });
         return;
       }
+      const releaseBodyRead = reserved;
       // Once the send below starts, releaseReserveWhenBothDone owns the
       // reserve release. This flag stays false only while a return above the
       // send can still happen, so the outer finally releases directly for