fix: remove dead code, fix __del__ safety, make agents.update version explicit

- Remove unused _new_request_id() and import uuid from transport.py
- Fix __del__ to wrap is_closed check in try/except for interpreter shutdown safety
- Make version a required kwarg in agents.update() instead of auto-retrieving
- Move stream error handling from session_events into transport retry loop
- Add unit tests for agents.update version contract

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: foleydang

dashscope/agentstudio/resources/agents.py

@@ -6,23 +6,15 @@
 
 from typing import Any, Mapping, Optional, Sequence
 
-from dashscope.agentstudio.pagination import (
-    AsyncCursorPage,
-    CursorPage,
-    build_page,
-)
-from dashscope.agentstudio.resources._helpers import (
-    _coerce_agent,
-    _coerce_agent_version,
-)
+from dashscope.agentstudio.pagination import (AsyncCursorPage, CursorPage,
+                                              build_page)
+from dashscope.agentstudio.resources._helpers import (_coerce_agent,
+                                                      _coerce_agent_version)
 from dashscope.agentstudio.types import Agent, AgentVersion
-from dashscope.agentstudio.types.params import (
-    AgentCreateParams,
-    AgentListParams,
-    AgentUpdateParams,
-    AgentVersionListParams,
-)
-
+from dashscope.agentstudio.types.params import (AgentCreateParams,
+                                                AgentListParams,
+                                                AgentUpdateParams,
+                                                AgentVersionListParams)
 
 _PATH_AGENTS = "/agents"
 
@@ -82,6 +74,7 @@ def update(
         self,
         agent_id: str,
         *,
+        version: int,
         name: Optional[str] = None,
         description: Optional[str] = None,
         model: Optional[str] = None,
@@ -91,9 +84,14 @@ def update(
         skills: Optional[Sequence[Mapping[str, Any]]] = None,
         metadata: Optional[Mapping[str, Any]] = None,
     ) -> Agent:
-        current = self.retrieve(agent_id)
+        """Update the latest version of an agent.
+
+        ``version`` must equal the server's current version; the server rejects mismatches with
+        HTTP 409. Call ``retrieve(agent_id)`` first to obtain the current version — the SDK does
+        not auto-resolve it.
+        """
         body = AgentUpdateParams(
-            version=current.version,
+            version=version,
             name=name,
             description=description,
             model=model,
@@ -229,6 +227,7 @@ async def update(
         self,
         agent_id: str,
         *,
+        version: int,
         name: Optional[str] = None,
         description: Optional[str] = None,
         model: Optional[str] = None,
@@ -238,9 +237,14 @@ async def update(
         skills: Optional[Sequence[Mapping[str, Any]]] = None,
         metadata: Optional[Mapping[str, Any]] = None,
     ) -> Agent:
-        current = await self.retrieve(agent_id)
+        """Update the latest version of an agent.
+
+        ``version`` must equal the server's current version; the server rejects mismatches with
+        HTTP 409. Call ``retrieve(agent_id)`` first to obtain the current version — the SDK does
+        not auto-resolve it.
+        """
         body = AgentUpdateParams(
-            version=current.version,
+            version=version,
             name=name,
             description=description,
             model=model,

dashscope/agentstudio/resources/session_events.py

@@ -14,7 +14,6 @@
     Sequence,
 )
 
-from dashscope.agentstudio import exceptions
 from dashscope.agentstudio.pagination import (
     AsyncCursorPage,
     CursorPage,
@@ -132,22 +131,6 @@ def stream(
             stream=True,
             timeout=timeout or AGENTSTUDIO_DEFAULT_TIMEOUT,
         )
-        if resp.status_code >= 400:
-            try:
-                resp.read()
-                body = resp.json()
-            except ValueError:
-                body = {"raw": resp.text}
-            try:
-                raise exceptions.from_response(
-                    status_code=resp.status_code,
-                    body=body,
-                    headers=resp.headers,
-                )
-            finally:
-                resp.close()
-        # Transport is shared -- stream close only closes
-        # the response, NOT the transport.
         return _TypedEventStream(
             EventStream(response=resp),
         )
@@ -294,22 +277,6 @@ async def stream(
             stream=True,
             timeout=timeout or AGENTSTUDIO_DEFAULT_TIMEOUT,
         )
-        if resp.status_code >= 400:
-            try:
-                await resp.aread()
-                body = resp.json()
-            except ValueError:
-                body = {"raw": resp.text}
-            try:
-                raise exceptions.from_response(
-                    status_code=resp.status_code,
-                    body=body,
-                    headers=resp.headers,
-                )
-            finally:
-                await resp.aclose()
-        # Transport is shared -- stream close only closes
-        # the response, NOT the transport.
         return _AioTypedEventStream(
             AsyncEventStream(response=resp),
         )

dashscope/agentstudio/transport.py

@@ -6,7 +6,7 @@
 (async) so the rest of the SDK can speak in plain Python data structures.
 The transport layer is responsible for:
 
-* injecting auth / workspace / uid / x-request-id headers,
+* injecting auth / workspace / uid headers,
 * JSON encoding / decoding the request and response bodies,
 * converting non-2xx responses into :class:`AgentStudioError`,
 * retry-on-network-error with bounded exponential backoff.
@@ -22,7 +22,6 @@
 import random
 import socket
 import time
-import uuid
 from dataclasses import dataclass
 from typing import Any, Dict, IO, Mapping, Optional, Tuple, Union
 
@@ -132,15 +131,6 @@ def is_error_payload(payload: Any) -> bool:
     return False
 
 
-def _new_request_id() -> str:
-    """Generate ``req_<26-char ULID-ish>``.
-
-    We use a UUID4 substring rather than an actual ULID to avoid an extra
-    runtime dependency – the backend treats this header opaquely.
-    """
-
-    return "req_" + uuid.uuid4().hex[:26]
-
 
 def _resolve_timeout(
     timeout: Union[float, httpx.Timeout, Tuple[float, float], None],
@@ -381,31 +371,31 @@ def request(  # pylint: disable=too-many-branches
                 if attempt >= self.max_retries or not should_retry:
                     raise exceptions.APIConnectionError(str(exc)) from exc
             else:
-                if not stream and _should_retry(
-                    resp.status_code,
-                    resp.headers,
+                if stream:
+                    if resp.status_code >= 400:
+                        resp.read()
+                    else:
+                        return resp
+                if _should_retry(
+                    resp.status_code, resp.headers,
                 ):
                     last_exc = exceptions.APIStatusError(
                         f"HTTP {resp.status_code}",
                         status_code=resp.status_code,
                     )
-                    if attempt >= self.max_retries:
-                        try:
-                            return self._parse(resp)
-                        finally:
-                            resp.close()
-                    wait = _get_retry_after(resp.headers) or _backoff(attempt)
-                    resp.close()
-                    time.sleep(wait)
-                    attempt += 1
-                    continue
-                if stream:
-                    return resp
+                    if attempt < self.max_retries:
+                        wait = (
+                            _get_retry_after(resp.headers)
+                            or _backoff(attempt)
+                        )
+                        resp.close()
+                        time.sleep(wait)
+                        attempt += 1
+                        continue
                 try:
                     return self._parse(resp)
                 finally:
-                    if not stream:
-                        resp.close()
+                    resp.close()
             time.sleep(_backoff(attempt))
             attempt += 1
         raise exceptions.APIConnectionError(
@@ -470,9 +460,9 @@ def __exit__(
         self.close()
 
     def __del__(self) -> None:
-        if self.is_closed:
-            return
         try:
+            if self.is_closed:
+                return
             self.close()
         except Exception:
             pass
@@ -605,31 +595,31 @@ async def request(  # pylint: disable=too-many-branches
                 if attempt >= self.max_retries or not should_retry:
                     raise exceptions.APIConnectionError(str(exc)) from exc
             else:
-                if not stream and _should_retry(
-                    resp.status_code,
-                    resp.headers,
+                if stream:
+                    if resp.status_code >= 400:
+                        await resp.aread()
+                    else:
+                        return resp
+                if _should_retry(
+                    resp.status_code, resp.headers,
                 ):
                     last_exc = exceptions.APIStatusError(
                         f"HTTP {resp.status_code}",
                         status_code=resp.status_code,
                     )
-                    if attempt >= self.max_retries:
-                        try:
-                            return await self._parse(resp)
-                        finally:
-                            await resp.aclose()
-                    wait = _get_retry_after(resp.headers) or _backoff(attempt)
-                    await resp.aclose()
-                    await asyncio.sleep(wait)
-                    attempt += 1
-                    continue
-                if stream:
-                    return resp
+                    if attempt < self.max_retries:
+                        wait = (
+                            _get_retry_after(resp.headers)
+                            or _backoff(attempt)
+                        )
+                        await resp.aclose()
+                        await asyncio.sleep(wait)
+                        attempt += 1
+                        continue
                 try:
                     return await self._parse(resp)
                 finally:
-                    if not stream:
-                        await resp.aclose()
+                    await resp.aclose()
             await asyncio.sleep(_backoff(attempt))
             attempt += 1
         raise exceptions.APIConnectionError(

tests/unit/test_agentstudio_protocol.py

@@ -20,14 +20,9 @@
 
 from dashscope.agentstudio import exceptions
 from dashscope.agentstudio.transport import is_error_payload, unwrap
-from dashscope.agentstudio.types import (
-    user_message,
-    user_interrupt,
-    user_tool_confirmation,
-    user_custom_tool_result,
-    user_define_outcome,
-)
-
+from dashscope.agentstudio.types import (user_custom_tool_result,
+                                         user_define_outcome, user_interrupt,
+                                         user_message, user_tool_confirmation)
 
 # ---------------------------------------------------------------------------
 # 1. error.{code, message}
@@ -276,3 +271,66 @@ def test_from_response_spring_default():
     }
     err = exceptions.from_response(status_code=404, body=body)
     assert isinstance(err, exceptions.NotFoundError)
+
+
+# ---------------------------------------------------------------------------
+# 4. agents.update version contract (no auto-retrieve)
+# ---------------------------------------------------------------------------
+
+
+class _RecordingTransport:
+    """Minimal transport that records requests and returns a canned agent."""
+
+    def __init__(self):
+        self.calls = []
+
+    def request(self, method, path, **kwargs):
+        self.calls.append({"method": method, "path": path, **kwargs})
+        from dashscope.agentstudio.transport import APIResponse
+
+        return APIResponse(
+            data={"id": "agent_1", "version": 3, "name": "demo"},
+            request_id="req_1",
+        )
+
+
+def _client_with_recording_transport():
+    from dashscope.agentstudio import Client
+
+    c = Client(api_key="test-key", base_url="http://test")
+    c.transport = _RecordingTransport()
+    return c
+
+
+def test_agents_update_requires_version_kwarg():
+    """version is a required keyword-only argument — omitting it is a TypeError."""
+    client = _client_with_recording_transport()
+    with pytest.raises(TypeError):
+        client.agents.update("agent_1", name="new-name")  # type: ignore[call-arg]
+
+
+def test_agents_update_with_version_sends_body():
+    """update(agent_id, version=...) sends POST /agents/{id} with version in body."""
+    client = _client_with_recording_transport()
+    client.agents.update("agent_1", version=3, name="new-name")
+    assert len(client.transport.calls) == 1
+    call = client.transport.calls[0]
+    assert call["method"] == "POST"
+    assert call["path"] == "/agents/agent_1"
+    body = call["json"]
+    assert body["version"] == 3
+    assert body["name"] == "new-name"
+
+
+def test_agents_update_does_not_auto_retrieve():
+    """SDK must NOT call retrieve() internally — caller manages version now.
+
+    Mirrors the Anthropic Managed Agents SDK: the server only accepts updates
+    against the latest version (409 on mismatch), so the caller is responsible
+    for retrieving the current version first.
+    """
+    client = _client_with_recording_transport()
+    client.agents.update("agent_1", version=3, name="new-name")
+    # Only the POST should be recorded, no preceding GET.
+    assert len(client.transport.calls) == 1
+    assert client.transport.calls[0]["method"] == "POST"