Address Python goal review feedback

sdk/python/src/openai_codex/api.py

@@ -78,6 +78,8 @@ from .generated.v2_all import (
 )
 from .models import InitializeResponse, JsonObject, Notification
 
+_MAX_GOAL_OBJECTIVE_CHARS = 4_000
+
 
 def _normalize_goal_objective(objective: str) -> str:
     if not isinstance(objective, str):
@@ -85,6 +87,8 @@ def _normalize_goal_objective(objective: str) -> str:
     objective = objective.strip()
     if not objective:
         raise ValueError("goal objective must not be empty")
+    if len(objective) > _MAX_GOAL_OBJECTIVE_CHARS:
+        raise ValueError(f"goal objective must be at most {_MAX_GOAL_OBJECTIVE_CHARS} characters")
     return objective
 
 

sdk/python/src/openai_codex/async_client.py

@@ -53,6 +53,7 @@ class AsyncCodexClient:
     def __init__(self, config: CodexConfig | None = None) -> None:
         """Create the wrapped sync client that owns the transport process."""
         self._sync = CodexClient(config=config)
+        self._background_tasks: set[asyncio.Task[None]] = set()
 
     async def __aenter__(self) -> "AsyncCodexClient":
         """Start the Codex process when entering an async context."""
@@ -90,6 +91,9 @@ class AsyncCodexClient:
     async def close(self) -> None:
         """Close the wrapped sync client in a worker thread."""
         await self._call_sync(self._sync.close)
+        if self._background_tasks:
+            await asyncio.gather(*tuple(self._background_tasks), return_exceptions=True)
+            self._background_tasks.clear()
 
     async def initialize(self) -> InitializeResponse:
         """Initialize the Codex session."""
@@ -247,17 +251,23 @@ class AsyncCodexClient:
         try:
             return await asyncio.shield(operation)
         except asyncio.CancelledError:
-            try:
-                state, _ = await operation
-            except BaseException:
-                pass
-            else:
+
+            async def cleanup_cancelled_start() -> None:
+                try:
+                    state, _ = await operation
+                except BaseException:
+                    return
                 try:
                     await self.pause_goal(thread_id)
                 except Exception:
                     pass
-                state.finish()
-                self.unregister_goal_operation(state)
+                finally:
+                    state.finish()
+                    self.unregister_goal_operation(state)
+
+            cleanup = asyncio.create_task(cleanup_cancelled_start())
+            self._background_tasks.add(cleanup)
+            cleanup.add_done_callback(self._background_tasks.discard)
             raise
 
     async def turn_start(

sdk/python/tests/test_app_server_goal_turns.py

@@ -576,6 +576,19 @@ def test_failed_goal_starts_release_routing_without_model_requests(tmp_path) ->
             with pytest.raises(TypeError) as type_error:
                 thread.start_goal(123)  # type: ignore[arg-type]
 
+            existing = codex._client.thread_goal_set(
+                thread.id,
+                objective="Keep this valid stored goal",
+                status=ThreadGoalStatus.paused,
+            ).goal
+            with pytest.raises(ValueError) as long_error:
+                thread.start_goal("x" * 4_001)
+            persisted = codex._client.request(
+                "thread/goal/get",
+                {"threadId": thread.id},
+                response_model=ThreadGoalGetResponse,
+            ).goal
+
             ephemeral = codex.thread_start(ephemeral=True)
             with pytest.raises(InvalidRequestError) as ephemeral_error:
                 ephemeral.start_goal("Persist this goal")
@@ -585,7 +598,17 @@ def test_failed_goal_starts_release_routing_without_model_requests(tmp_path) ->
             registered_goals = dict(codex._client._router._goal_operations)
 
     assert {
-        "errors": [str(empty_error.value), str(type_error.value), ephemeral_error.value.message],
+        "errors": [
+            str(empty_error.value),
+            str(type_error.value),
+            str(long_error.value),
+            ephemeral_error.value.message,
+        ],
+        "stored_goal": (
+            existing.objective,
+            persisted.objective if persisted else None,
+            persisted.status if persisted else None,
+        ),
         "follow_up": (follow_up.status, follow_up.final_response),
         "request_count": len(requests),
         "registered_goals": registered_goals,
@@ -593,8 +616,14 @@ def test_failed_goal_starts_release_routing_without_model_requests(tmp_path) ->
         "errors": [
             "goal objective must not be empty",
             "goal objective must be a string",
+            "goal objective must be at most 4000 characters",
             f"thread must be persisted before starting a goal: {ephemeral.id}",
         ],
+        "stored_goal": (
+            "Keep this valid stored goal",
+            "Keep this valid stored goal",
+            ThreadGoalStatus.paused,
+        ),
         "follow_up": (TurnStatus.completed, "Ordinary turn complete."),
         "request_count": 1,
         "registered_goals": {},