refactor: improve custom hook URI handling and error reporting

Author: bekossy

sdks/python/agenta/sdk/engines/running/handlers.py

@@ -2275,9 +2275,13 @@ async def remote_forward_v0(
             got=webhook_url,
         ) from exc
 
-    # The stored url is the service base (pre-/invoke); the invoke surface lives
-    # at /invoke and is always appended.
-    target_url = f"{webhook_url.rstrip('/')}/invoke"
+    # The stored url is the service base; /invoke is always the invoke surface.
+    # Strip a trailing /invoke first so existing hooks that stored the full
+    # endpoint URL don't get /invoke/invoke after this PR's behaviour change.
+    base_url = webhook_url.rstrip("/")
+    if base_url.endswith("/invoke"):
+        base_url = base_url[: -len("/invoke")]
+    target_url = f"{base_url}/invoke"
 
     log.info("remote_forward_v0 POST", url=target_url)
 

sdks/python/agenta/sdk/engines/running/runners/restricted.py

@@ -9,13 +9,12 @@
     full_write_guard,
 )
 
-from agenta.sdk.engines.running.runners.base import CodeRunner
+from agenta.sdk.engines.running.runners.base import CodeRunner, normalize_result
 
 
 # Pure data/iteration builtins that RestrictedPython's safe_builtins omits but
 # evaluators routinely need. All operate on data only — none reach the host or
-# the class graph, so adding them does not widen the sandbox (escapes go through
-# attribute access, which safer_getattr blocks).
+# the class graph.
 _SAFE_EXTRA_BUILTINS = (
     "dict",
     "list",
@@ -38,16 +37,21 @@
 # pathlib, socket, importlib, io, shutil, ...) or the network (httpx, urllib,
 # requests, ...) is excluded. Operators who need unrestricted execution must opt
 # into the `local` runner; hostile multi-tenant should use `daytona`.
+#
+# EXCLUDED intentionally even though they look safe:
+#   - `typing`    — exposes `typing.sys`, giving access to `sys.modules` and
+#                   therefore every already-loaded module (including `os`).
+#   - `datetime`  — same escape: `datetime.sys.modules`.
+#   - `statistics`— same escape: `statistics.sys.modules`.
+# safer_getattr only blocks underscore-prefixed names; plain public attributes
+# like `sys` and `modules` on imported module objects are not blocked.
 _ALLOWED_IMPORTS = frozenset(
     {
         "math",
-        "statistics",
-        "datetime",
         "json",
         "re",
         "random",
         "string",
-        "typing",
         "collections",
         "itertools",
         "functools",
@@ -129,16 +133,21 @@ def run(
             trace: Full trace data (v2 only)
 
         Returns:
-            Float score between 0 and 1, or None if execution fails
+            Versions "1"/"2": float score between 0 and 1.
+            Version "3": any JSON-serializable value (dict, list, str, float, bool).
         """
         # Normalize runtime: None means python
         runtime = runtime or "python"
 
         # The restricted sandbox runs in-process and only supports Python.
+        # JavaScript and TypeScript require the Daytona runner
+        # (AGENTA_SERVICES_CODE_SANDBOX_RUNNER=daytona).
         if runtime != "python":
             raise ValueError(
-                f"RestrictedRunner only supports 'python' runtime, got: {runtime}. "
-                "Use the Daytona runner for javascript/typescript."
+                f"Runtime '{runtime}' is not supported by the default sandbox. "
+                "JavaScript and TypeScript evaluators require the Daytona runner. "
+                "Set AGENTA_SERVICES_CODE_SANDBOX_RUNNER=daytona, or change the "
+                "runtime to 'python'."
             )
 
         try:
@@ -153,24 +162,12 @@ def run(
 
             fn = environment["evaluate"]
 
-            if version == "2":
+            if version in ("2", "3"):
                 result = fn(inputs, output, trace)
             else:
                 result = fn(app_params, inputs, output, correct_answer)
 
-            # Attempt to convert result to float
-            if isinstance(result, (float, int, str)):
-                try:
-                    result = float(result)
-                except ValueError as e:
-                    raise ValueError(f"Result cannot be converted to float: {e}")
-
-            if not isinstance(result, float):
-                raise TypeError(
-                    f"Result is not a float after conversion: {type(result)}"
-                )
-
-            return result
+            return normalize_result(result, version)
 
         except KeyError as e:
             raise KeyError(f"Missing expected key in environment: {e}")

sdks/python/agenta/sdk/middlewares/running/resolver.py

@@ -16,7 +16,10 @@
     parse_uri,
 )
 from agenta.sdk.engines.running.handlers import remote_forward_v0
-from agenta.sdk.engines.running.errors import InvalidInterfaceURIV0Error
+from agenta.sdk.engines.running.errors import (
+    InvalidInterfaceURIV0Error,
+    MissingConfigurationParameterV0Error,
+)
 
 # Internal embeds resolution defaults (not user-configurable)
 _EMBEDS_MAX_CHECKS = 20
@@ -28,17 +31,22 @@
 _AG_EMBED_MARKER = "@ag.embed"
 
 
-def _is_url_backed_custom_hook(revision: Optional[WorkflowRevisionData]) -> bool:
-    """True for a custom-hook revision that carries a url (forwards remotely)."""
-    if not revision or not revision.uri or not revision.url:
+def _is_custom_hook_uri(uri: Optional[str]) -> bool:
+    """True for any custom:hook URI regardless of whether a url is set."""
+    if not uri:
         return False
     try:
-        _provider, kind, key, _version = parse_uri(revision.uri)
+        _provider, kind, key, _version = parse_uri(uri)
     except Exception:
         return False
     return kind == "custom" and key == "hook"
 
 
+def _is_url_backed_custom_hook(revision: Optional[WorkflowRevisionData]) -> bool:
+    """True for a custom-hook revision that carries a url (forwards remotely)."""
+    return bool(revision and revision.url and _is_custom_hook_uri(revision.uri))
+
+
 def _raise_bad_request(message: str) -> None:
     error = ValueError(message)
     error.status_code = 400  # type: ignore[attr-defined]
@@ -604,12 +612,17 @@ async def __call__(
         # forwarder); only resolve from the URI registry for pure URI dispatch.
         # A URL-backed custom hook has no local handler, so its URI would resolve
         # to the raising registry stub — forward it remotely instead.
-        if ctx.handler is None and _is_url_backed_custom_hook(revision):
+        # A custom hook with no URL is a user misconfiguration: raise a clear
+        # error rather than letting it fall through to the internal stub.
+        uri = revision.uri if revision else None
+        if ctx.handler is None and _is_custom_hook_uri(uri):
+            if not (revision and revision.url):
+                raise MissingConfigurationParameterV0Error(
+                    path="url",
+                )
             handler = remote_forward_v0
         else:
-            handler = ctx.handler or await resolve_handler(
-                uri=(revision.uri if revision else None)
-            )
+            handler = ctx.handler or await resolve_handler(uri=uri)
 
         ctx.revision = (
             {"data": revision.model_dump(mode="json", exclude_none=True)}