fix: follow redirects, refuse a blank execution id, translate InvalidURL

A 3xx was returned as if it were the answer, which a poll loop reads as a
finished execution with no status; the previous transport followed redirects on
both verbs. A status endpoint carrying no execution id now fails instead of
polling for a blank one. InvalidURL joins the translation table, and the
docstring names the two httpx families that stay outside it.

Adds the multi-file upload comparison the parity suite never had, and lets the
drift gate see a file the generator newly creates.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014f9oEEYspPH4fmPULTnLkJ

Author: chandrasekharan-zipstack

.github/workflows/test.yml

@@ -59,5 +59,9 @@ jobs:
       - name: Regenerate from the committed spec
         run: ./tools/gen_sdk.sh
 
+      # `git add -N` first: a diff alone cannot see a file the generator has
+      # newly created, which is exactly what a spec growing an endpoint does.
       - name: Fail if the committed SDK is not what the spec generates
-        run: git diff --exit-code -- src/unstract/api_deployments/sdk_docstudio
+        run: |
+          git add -N -- src/unstract/api_deployments/sdk_docstudio
+          git diff --exit-code -- src/unstract/api_deployments/sdk_docstudio

src/unstract/api_deployments/client.py

@@ -24,6 +24,7 @@
     ConnectionError,
     ConnectTimeout,
     ContentDecodingError,
+    InvalidURL,
     MissingSchema,
     ProxyError,
     ReadTimeout,
@@ -52,9 +53,11 @@ def _translate_transport_errors(fn, *args, **kwargs):
 
     Callers document and catch the ``requests`` classes, and the retry policy
     keys off them too, so the class chosen here decides whether a failure is
-    retried. Every branch is ordered before the base class it derives from, and
-    ``RequestError`` is the catch-all that keeps a novel failure from escaping
-    untranslated.
+    retried. Every branch is ordered before the base class it derives from.
+    ``RequestError`` is the catch-all for the transport subtree, which is where
+    a novel failure appears. httpx puts three families outside it: ``InvalidURL``,
+    translated here because ``requests`` raised its own, and ``StreamError`` and
+    ``CookieConflict``, which propagate as themselves.
     """
     try:
         return fn(*args, **kwargs)
@@ -82,13 +85,25 @@ def _translate_transport_errors(fn, *args, **kwargs):
         raise TooManyRedirects(str(e)) from e
     except httpx.DecodingError as e:
         raise ContentDecodingError(str(e)) from e
+    except httpx.InvalidURL as e:
+        raise InvalidURL(str(e)) from e
     except httpx.RequestError as e:
         raise ConnectionError(str(e)) from e
 
 
 def _query_value(url: str, key: str) -> str:
-    """Read one query parameter out of a URL, absolute or relative."""
-    return parse_qs(urlparse(url).query).get(key, [""])[0]
+    """Read one required query parameter out of a URL, absolute or relative.
+
+    Empty is not a usable value here: it polls for an execution the service
+    cannot identify and reports whatever it makes of a blank id.
+    """
+    value = parse_qs(urlparse(url).query).get(key, [""])[0]
+    if not value:
+        raise APIDeploymentsClientException(
+            f"No {key} in {url!r}. The status endpoint the service returned "
+            "carries it; pass that endpoint unmodified."
+        )
+    return value
 
 
 class APIDeploymentsClientException(Exception):
@@ -266,6 +281,10 @@ def _transport(self):
                 verify_ssl=self.verify,
                 timeout=httpx.Timeout(self.transport_timeout),
                 raise_on_unexpected_status=False,
+                # The previous transport followed redirects. Without this a 30x
+                # from a load balancer is read as a terminal result with no
+                # status, which a poll loop reports as a finished-and-empty job.
+                follow_redirects=True,
             )
         return self._transport_client
 

tests/test_compat.py

@@ -13,6 +13,7 @@
 import inspect
 import io
 import json
+import re
 import socket
 import threading
 from pathlib import Path
@@ -38,6 +39,7 @@
     _EXECUTE_SEND_ONLY,
     _STATUS_SEND_ONLY,
     APIDeploymentsClient,
+    APIDeploymentsClientException,
 )
 
 BASELINE_VERSION = "1.5.3"
@@ -623,15 +625,15 @@ def test_execute_url_matches_the_deployment_url(api_url):
     assert args[1] == api_url == mock_requests.post.call_args[0][0]
 
 
-def _wire_heads(*calls):
-    """Run each call against a loopback server and return its request headers.
+def _wire_requests(*calls, reply=b'{"status":"COMPLETED","message":{}}'):
+    """Run each call against a loopback server and return the raw requests.
 
     Below the client, the transport adds headers of its own -- and drops none
     of them into any object the client can be asked for. A socket is the only
     place both clients can be compared on what they actually send. One server
     serves every call, so the ``Host`` header is the same for all of them.
     """
-    heads = []
+    raw = []
     server = socket.socket()
     server.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
     server.bind(("127.0.0.1", 0))
@@ -646,8 +648,17 @@ def serve():
                 if not chunk:
                     break
                 data += chunk
-            heads.append(data.split(b"\r\n\r\n")[0])
-            body = b'{"status":"COMPLETED","message":[]}'
+            # The body has to be drained too: a client whose upload is never
+            # read can block on the socket instead of returning.
+            head, _, rest = data.partition(b"\r\n\r\n")
+            declared = _header_value(head, "content-length")
+            while declared and len(rest) < int(declared):
+                chunk = conn.recv(65536)
+                if not chunk:
+                    break
+                rest += chunk
+            raw.append(head + b"\r\n\r\n" + rest)
+            body = reply
             conn.sendall(
                 b"HTTP/1.1 200 OK\r\nContent-Type: application/json\r\n"
                 b"Content-Length: %d\r\n\r\n%s" % (len(body), body)
@@ -664,15 +675,51 @@ def serve():
         thread.join(timeout=10)
         server.close()
 
-    return [
-        {
-            name.lower(): value.strip()
-            for name, _, value in (
-                line.partition(":") for line in head.decode().split("\r\n")[1:]
+    return raw
+
+
+def _headers(head: bytes) -> dict[str, str]:
+    return {
+        name.lower(): value.strip()
+        for name, _, value in (
+            line.partition(":") for line in head.decode().split("\r\n")[1:]
+        )
+    }
+
+
+def _header_value(head: bytes, name: str) -> str:
+    return _headers(head).get(name, "")
+
+
+def _wire_heads(*calls):
+    """The request headers each call put on the wire."""
+    return [_headers(raw.split(b"\r\n\r\n")[0]) for raw in _wire_requests(*calls)]
+
+
+def _multipart_parts(raw: bytes) -> list[tuple[str, str, bytes]]:
+    """``(field, filename, content)`` for every part of a multipart request.
+
+    The boundary itself is deliberately not compared: it is random per request
+    in both clients, so only what it delimits can be.
+    """
+    head, _, body = raw.partition(b"\r\n\r\n")
+    boundary = _header_value(head, "content-type").partition("boundary=")[2]
+    parts = []
+    for chunk in body.split(b"--" + boundary.encode()):
+        headers, _, content = chunk.partition(b"\r\n\r\n")
+        disposition = headers.decode("utf-8", errors="replace")
+        if "content-disposition" not in disposition.lower():
+            continue
+        field = re.search(r'name="([^"]*)"', disposition)
+        filename = re.search(r'filename="([^"]*)"', disposition)
+        parts.append(
+            (
+                field.group(1) if field else "",
+                filename.group(1) if filename else "",
+                content.removesuffix(b"\r\n"),
             )
-        }
-        for head in heads
-    ]
+        )
+    return parts
 
 
 def test_wire_headers_match_the_released_client():
@@ -697,6 +744,50 @@ def test_wire_headers_match_the_released_client():
     assert ours["user-agent"].startswith("python-httpx/")
 
 
+def test_a_multi_file_upload_matches_the_released_client(tmp_path):
+    """The method takes a list, and the second file is where a transport swap
+    diverges: one part written, one dropped, or two parts sharing a name the
+    server then reads as one."""
+    paths = []
+    for name, content in (("first.txt", b"one"), ("second.txt", b"two")):
+        path = tmp_path / name
+        path.write_bytes(content)
+        paths.append(str(path))
+
+    ours, theirs = _wire_requests(
+        lambda url: _client(api_url=url, api_timeout=300).structure_file(paths),
+        lambda url: _baseline_client(api_url=url, api_timeout=300).structure_file(
+            paths
+        ),
+    )
+
+    # Sorted: the two clients order the fields differently, which no multipart
+    # parser reads as meaning. The order of the files among themselves is the
+    # part that carries meaning, and it is pinned below.
+    assert sorted(_multipart_parts(ours)) == sorted(_multipart_parts(theirs))
+    uploaded = [part for part in _multipart_parts(ours) if part[0] == "files"]
+    assert [(filename, content) for _, filename, content in uploaded] == [
+        ("first.txt", b"one"),
+        ("second.txt", b"two"),
+    ]
+
+
+def test_redirects_are_followed():
+    """The released client followed them on both verbs.
+
+    Not following one turns a load balancer's 307 into a body the poll loop
+    reads as a finished execution with no status.
+    """
+    assert _client()._transport.get_httpx_client().follow_redirects is True
+
+
+def test_a_status_endpoint_without_an_execution_id_is_refused():
+    """Polling with a blank id asks the service about an execution nobody has;
+    what it answers is not this execution's state."""
+    with pytest.raises(APIDeploymentsClientException):
+        _client().check_execution_status("/deployment/api/testorg/testapi/")
+
+
 def test_the_transport_is_untimed_by_default():
     """A connection that stalls forever is what the released client did.
 

tests/test_retry.py

@@ -488,7 +488,7 @@ def test_503_after_exhaustion_sets_pending_true(
         mock_request.return_value = _mock_response(
             503, json_data={"status": "", "error": "Service Unavailable", "message": ""}
         )
-        result = client.check_execution_status("/api/v1/status/123")
+        result = client.check_execution_status("/api/v1/status/?execution_id=123")
         assert result["pending"] is True
         assert result["status_code"] == 503
 
@@ -497,7 +497,7 @@ def test_200_with_pending_status_sets_pending_true(self, mock_request, client):
         mock_request.return_value = _mock_response(
             200, json_data={"status": "EXECUTING", "error": "", "message": ""}
         )
-        result = client.check_execution_status("/api/v1/status/123")
+        result = client.check_execution_status("/api/v1/status/?execution_id=123")
         assert result["pending"] is True
         assert result["status_code"] == 200
 
@@ -511,7 +511,7 @@ def test_200_with_completed_status_sets_pending_false(self, mock_request, client
                 "message": '{"result": "data"}',
             },
         )
-        result = client.check_execution_status("/api/v1/status/123")
+        result = client.check_execution_status("/api/v1/status/?execution_id=123")
         assert result["pending"] is False
 
     @patch("unstract.api_deployments.client.APIDeploymentsClient._send")
@@ -526,7 +526,7 @@ def test_422_with_executing_status_sets_pending_true(self, mock_request, client)
         mock_request.return_value = _mock_response(
             422, json_data={"status": "EXECUTING", "error": "", "message": ""}
         )
-        result = client.check_execution_status("/api/v1/status/123")
+        result = client.check_execution_status("/api/v1/status/?execution_id=123")
         assert result["pending"] is True
         assert result["status_code"] == 422
 
@@ -537,7 +537,7 @@ def test_422_with_pending_status_sets_pending_true(self, mock_request, client):
         mock_request.return_value = _mock_response(
             422, json_data={"status": "PENDING", "error": "", "message": ""}
         )
-        result = client.check_execution_status("/api/v1/status/123")
+        result = client.check_execution_status("/api/v1/status/?execution_id=123")
         assert result["pending"] is True
         assert result["status_code"] == 422
 
@@ -546,7 +546,7 @@ def test_400_does_not_set_pending(self, mock_request, client):
         mock_request.return_value = _mock_response(
             400, json_data={"status": "", "error": "Bad request", "message": ""}
         )
-        result = client.check_execution_status("/api/v1/status/123")
+        result = client.check_execution_status("/api/v1/status/?execution_id=123")
         assert result["pending"] is False