fix(tools): validate cancellation timeout bounds

Author: elkaix

packages/pythinker-core/src/pythinker_core/__init__.py

@@ -285,13 +285,9 @@ def completed_tool_results(self) -> dict[str, ToolResult]:
 
         completed: dict[str, ToolResult] = {}
         for tool_call_id, future in self._tool_result_futures.items():
-            if not future.done() or future.cancelled():
+            if not future.done() or future.cancelled() or future.exception() is not None:
                 continue
-            try:
-                result = future.result()
-            except BaseException:
-                continue
-            completed[tool_call_id] = result
+            completed[tool_call_id] = future.result()
         return completed
 
     @property

packages/pythinker-core/src/pythinker_core/tooling/__init__.py

@@ -338,13 +338,17 @@ class ToolCancellationTimeoutError(RuntimeError):
 
 @dataclass(frozen=True, slots=True)
 class ToolBatchContext:
+    """Immutable cross-step execution metadata supplied when a batch is created."""
+
     turn_id: str = ""
     step_no: int = 0
     prior_call_fingerprints: tuple[ToolCallFingerprint, ...] = ()
 
 
 @dataclass(frozen=True, slots=True)
 class ToolBatchSummary:
+    """Final normalized call and deduplication state exposed by a batch handle."""
+
     current_call_fingerprints: tuple[ToolCallFingerprint, ...] = ()
     dedup_triggered: bool = False
     consecutive_identical_call_count: int = 0
@@ -353,6 +357,8 @@ class ToolBatchSummary:
 
 @runtime_checkable
 class ToolBatchHandle(Protocol):
+    """Supervises one exception-atomic terminal batch and its ordered results."""
+
     @property
     def tool_calls(self) -> Sequence[ToolCall]: ...
 
@@ -369,6 +375,8 @@ async def cancel_and_settle(self, *, timeout: float | None = None) -> None: ...
 
 @runtime_checkable
 class BatchToolset(Protocol):
+    """Optional additive Toolset protocol for terminal batch dispatch."""
+
     def handle_batch(
         self,
         tool_calls: Sequence[ToolCall],

src/pythinker_code/soul/tool_execution.py

@@ -6,6 +6,7 @@
 import difflib
 import hashlib
 import json
+import math
 import time
 from collections.abc import AsyncGenerator, Awaitable, Callable, Mapping, Sequence
 from contextvars import ContextVar
@@ -891,8 +892,8 @@ async def _bounded_settlement(self, timeout: float) -> None:
 
     async def cancel_and_settle(self, *, timeout: float | None = None) -> None:
         effective_timeout = TOOL_CANCELLATION_TIMEOUT_SECONDS if timeout is None else timeout
-        if effective_timeout < 0:
-            raise ValueError("tool cancellation timeout cannot be negative")
+        if not math.isfinite(effective_timeout) or effective_timeout < 0:
+            raise ValueError("tool cancellation timeout must be finite and non-negative")
 
         settlement = self._settlement_task
         if settlement is None:

tests/core/test_tool_execution_cancellation.py

@@ -87,6 +87,22 @@ def test_cancellation_timeout_default_is_five_seconds() -> None:
     assert tool_execution.TOOL_CANCELLATION_TIMEOUT_SECONDS == 5.0
 
 
+@pytest.mark.parametrize("timeout", [-1.0, float("nan"), float("inf")])
+async def test_invalid_cancellation_timeout_does_not_cancel_work(timeout: float) -> None:
+    stubborn = CancellationIgnoringTool()
+    toolset = PythinkerToolset()
+    toolset.add(stubborn)
+    batch = toolset.handle_batch([_call("stubborn", "Stubborn")], ToolBatchContext())
+    await stubborn.started.wait()
+
+    with pytest.raises(ValueError, match="finite and non-negative"):
+        await batch.cancel_and_settle(timeout=timeout)
+
+    assert not stubborn.cancel_seen.is_set()
+    stubborn.release.set()
+    assert [result.tool_call_id for result in await batch.results()] == ["stubborn"]
+
+
 async def test_timeout_poisons_new_batches_until_late_task_is_drained() -> None:
     stubborn = CancellationIgnoringTool()
     immediate = ImmediateTool()