diff --git a/python/packages/declarative/agent_framework_declarative/_workflows/_executors_http.py b/python/packages/declarative/agent_framework_declarative/_workflows/_executors_http.py index e041a40c3b..3df7a2bca1 100644 --- a/python/packages/declarative/agent_framework_declarative/_workflows/_executors_http.py +++ b/python/packages/declarative/agent_framework_declarative/_workflows/_executors_http.py @@ -64,7 +64,7 @@ def _get_path(action_def: Mapping[str, Any], key: str) -> str | None: if isinstance(value, str): return value or None if isinstance(value, Mapping): - path = value.get("path") + path = value.get("path") # type: ignore[reportUnknownMemberType, reportUnknownVariableType] return path if isinstance(path, str) and path else None return None @@ -97,7 +97,7 @@ def _parse_response_body(body: str | None) -> Any: return None try: return json.loads(body) - except (ValueError, json.JSONDecodeError): + except json.JSONDecodeError: return body @@ -118,13 +118,14 @@ def _format_query_value(value: Any) -> str | None: def _get_messages_path(state: DeclarativeWorkflowState, conversation_id_expr: str | None) -> str | None: - """Mirror ``InvokeAzureAgentExecutor._get_conversation_messages_path`` semantics. + """Return the configured conversation messages path, if any. - Returns a single state path (no double-mirroring): ``Conversation.messages`` - when no conversation id is provided, otherwise - ``System.conversations.{evaluated_id}.messages``. Returns ``None`` if the - expression evaluates to an empty string (matches .NET ``GetConversationId`` - behaviour where empty becomes ``null`` and the response is not appended). + Returns ``System.conversations.{evaluated_id}.messages`` when a + ``conversation_id_expr`` is configured and evaluates to a non-empty value. + Returns ``None`` when no conversation id expression is configured or when + the expression evaluates to ``None`` or an empty string (matches .NET + ``GetConversationId`` behaviour where empty becomes ``null`` and the + response is not appended). """ if not conversation_id_expr: return None @@ -209,9 +210,7 @@ class HttpRequestActionExecutor(DeclarativeActionExecutor): except DeclarativeActionError: raise except httpx.HTTPError as exc: - raise DeclarativeActionError( - f"HTTP request to '{url}' failed: {type(exc).__name__}" - ) from exc + raise DeclarativeActionError(f"HTTP request to '{url}' failed: {type(exc).__name__}") from exc except Exception as exc: # Custom HttpRequestHandler implementations may raise arbitrary # exception types. Wrap them in DeclarativeActionError so workflow @@ -219,9 +218,7 @@ class HttpRequestActionExecutor(DeclarativeActionExecutor): # ``asyncio.CancelledError`` is a ``BaseException`` (not # ``Exception``) and so still propagates unmodified, preserving # workflow-cancellation semantics. - raise DeclarativeActionError( - f"HTTP request to '{url}' failed: {type(exc).__name__}" - ) from exc + raise DeclarativeActionError(f"HTTP request to '{url}' failed: {type(exc).__name__}") from exc if result.is_success_status_code: self._assign_response(state, result) @@ -234,10 +231,7 @@ class HttpRequestActionExecutor(DeclarativeActionExecutor): self._assign_response_headers(state, result) body_preview = _format_body_for_diagnostics(result.body) if body_preview: - message = ( - f"HTTP request to '{url}' failed with status code {result.status_code}. " - f"Body: '{body_preview}'" - ) + message = f"HTTP request to '{url}' failed with status code {result.status_code}. Body: '{body_preview}'" else: message = f"HTTP request to '{url}' failed with status code {result.status_code}." raise DeclarativeActionError(message) @@ -265,7 +259,7 @@ class HttpRequestActionExecutor(DeclarativeActionExecutor): if not isinstance(raw_headers, Mapping) or not raw_headers: return None result: dict[str, str] = {} - for key, value in raw_headers.items(): + for key, value in raw_headers.items(): # type: ignore[reportUnknownVariableType] if not isinstance(key, str) or not key: continue evaluated = state.eval_if_expression(value) @@ -282,7 +276,7 @@ class HttpRequestActionExecutor(DeclarativeActionExecutor): if not isinstance(raw_params, Mapping) or not raw_params: return None result: dict[str, str] = {} - for key, value in raw_params.items(): + for key, value in raw_params.items(): # type: ignore[reportUnknownVariableType] if not isinstance(key, str) or not key or value is None: continue evaluated = state.eval_if_expression(value) @@ -297,40 +291,34 @@ class HttpRequestActionExecutor(DeclarativeActionExecutor): return None, None if not isinstance(raw_body, Mapping): raise ValueError( - "HttpRequestAction 'body' must be a mapping with a 'kind' field " - "(json, raw) or omitted entirely." + "HttpRequestAction 'body' must be a mapping with a 'kind' field (json, raw) or omitted entirely." ) - kind_value = raw_body.get("kind") or raw_body.get("$kind") + kind_value: Any = raw_body.get("kind") or raw_body.get("$kind") # type: ignore[reportUnknownMemberType] if kind_value is None: raise ValueError( - "HttpRequestAction 'body' is missing 'kind'. Use 'json', 'raw', " - "or omit 'body' for no request body." + "HttpRequestAction 'body' is missing 'kind'. Use 'json', 'raw', or omit 'body' for no request body." ) if not isinstance(kind_value, str): - raise ValueError( - f"HttpRequestAction 'body.kind' must be a string, got {type(kind_value).__name__}." - ) + raise ValueError(f"HttpRequestAction 'body.kind' must be a string, got {kind_value!r}.") if kind_value in _BODY_KIND_NONE: return None, None if kind_value in _BODY_KIND_JSON: - content_expr = raw_body.get("content") + content_expr: Any = raw_body.get("content") # type: ignore[reportUnknownMemberType] if content_expr is None: return None, None evaluated = state.eval_if_expression(content_expr) try: body_text = json.dumps(evaluated, default=str) except (TypeError, ValueError) as exc: - raise ValueError( - f"HttpRequestAction 'body.content' could not be serialised as JSON: {exc}" - ) from exc + raise ValueError(f"HttpRequestAction 'body.content' could not be serialised as JSON: {exc}") from exc return body_text, "application/json" if kind_value in _BODY_KIND_RAW: - content_expr = raw_body.get("content") - content_type_expr = raw_body.get("contentType") + content_expr = raw_body.get("content") # type: ignore[reportUnknownMemberType] + content_type_expr: Any = raw_body.get("contentType") # type: ignore[reportUnknownMemberType] content: str | None = None if content_expr is not None: evaluated = state.eval_if_expression(content_expr) @@ -374,7 +362,7 @@ class HttpRequestActionExecutor(DeclarativeActionExecutor): connection = self._action_def.get("connection") if not isinstance(connection, Mapping): return None - name_expr = connection.get("name") + name_expr: Any = connection.get("name") # type: ignore[reportUnknownMemberType] if name_expr is None: return None evaluated = state.eval_if_expression(name_expr) diff --git a/python/packages/declarative/agent_framework_declarative/_workflows/_http_handler.py b/python/packages/declarative/agent_framework_declarative/_workflows/_http_handler.py index 75231fe91d..90ff5b87b4 100644 --- a/python/packages/declarative/agent_framework_declarative/_workflows/_http_handler.py +++ b/python/packages/declarative/agent_framework_declarative/_workflows/_http_handler.py @@ -57,8 +57,8 @@ class HttpRequestInfo: method: str url: str - headers: dict[str, str] = field(default_factory=dict) - query_parameters: dict[str, str] = field(default_factory=dict) + headers: dict[str, str] = field(default_factory=dict) # type: ignore[reportUnknownVariableType] + query_parameters: dict[str, str] = field(default_factory=dict) # type: ignore[reportUnknownVariableType] body: str | None = None body_content_type: str | None = None timeout_ms: int | None = None @@ -74,12 +74,17 @@ class HttpRequestResult: ``dict[str, list[str]]``. The executor folds duplicates into a single comma-joined string only at the point it assigns ``responseHeaders`` to workflow state. + + Header keys are normalized to lowercase so that lookups are consistent + regardless of the server's transmitted casing (HTTP headers are + case-insensitive per RFC 7230 §3.2). Custom :class:`HttpRequestHandler` + implementations should follow the same convention. """ status_code: int is_success_status_code: bool body: str - headers: dict[str, list[str]] = field(default_factory=dict) + headers: dict[str, list[str]] = field(default_factory=dict) # type: ignore[reportUnknownVariableType] @runtime_checkable @@ -132,7 +137,7 @@ class DefaultHttpRequestHandler: def __init__( self, *, - client: "httpx.AsyncClient | None" = None, + client: httpx.AsyncClient | None = None, client_provider: ClientProvider | None = None, ) -> None: self._owned_client: httpx.AsyncClient | None = None @@ -180,9 +185,12 @@ class DefaultHttpRequestHandler: ) # Preserve multi-value headers (e.g. multiple Set-Cookie) as list[str]. + # Normalize names to lowercase so lookups are consistent and case + # variations from the transport do not create duplicate logical keys + # (HTTP headers are case-insensitive per RFC 7230 §3.2). result_headers: dict[str, list[str]] = {} for key, value in response.headers.multi_items(): - result_headers.setdefault(key, []).append(value) + result_headers.setdefault(key.lower(), []).append(value) body_text = response.text @@ -216,7 +224,7 @@ class DefaultHttpRequestHandler: self._owned_client = httpx.AsyncClient() return self._owned_client - async def __aenter__(self) -> "DefaultHttpRequestHandler": + async def __aenter__(self) -> DefaultHttpRequestHandler: return self async def __aexit__(self, exc_type: Any, exc: Any, tb: Any) -> None: diff --git a/python/packages/declarative/tests/test_default_http_request_handler.py b/python/packages/declarative/tests/test_default_http_request_handler.py index 5391b90adf..ecdce3d7ff 100644 --- a/python/packages/declarative/tests/test_default_http_request_handler.py +++ b/python/packages/declarative/tests/test_default_http_request_handler.py @@ -181,9 +181,7 @@ class TestResponseHeaders: handler = _make_handler(httpx.MockTransport(respond)) try: - result = await handler.send( - HttpRequestInfo(method="GET", url="https://api.example.test/x") - ) + result = await handler.send(HttpRequestInfo(method="GET", url="https://api.example.test/x")) finally: await handler.aclose() @@ -199,9 +197,7 @@ class TestClientOwnership: handler = DefaultHttpRequestHandler() # Inject a MockTransport-backed client into the owned slot and verify # aclose() releases it. Avoids real network access. - owned = httpx.AsyncClient( - transport=httpx.MockTransport(lambda r: httpx.Response(200, text="ok")) - ) + owned = httpx.AsyncClient(transport=httpx.MockTransport(lambda r: httpx.Response(200, text="ok"))) handler._owned_client = owned assert not owned.is_closed await handler.aclose() @@ -209,13 +205,9 @@ class TestClientOwnership: @pytest.mark.asyncio async def test_caller_owned_client_is_not_closed(self) -> None: - client = httpx.AsyncClient( - transport=httpx.MockTransport(lambda r: httpx.Response(200, text="ok")) - ) + client = httpx.AsyncClient(transport=httpx.MockTransport(lambda r: httpx.Response(200, text="ok"))) handler = DefaultHttpRequestHandler(client=client) - await handler.send( - HttpRequestInfo(method="GET", url="https://api.example.test/x") - ) + await handler.send(HttpRequestInfo(method="GET", url="https://api.example.test/x")) await handler.aclose() assert not client.is_closed await client.aclose() # cleanup @@ -242,11 +234,7 @@ class TestClientOwnership: nonlocal construction_count if not args and not kwargs: construction_count += 1 - return original_ctor( - transport=httpx.MockTransport( - lambda r: httpx.Response(200, text="ok") - ) - ) + return original_ctor(transport=httpx.MockTransport(lambda r: httpx.Response(200, text="ok"))) return original_ctor(*args, **kwargs) import agent_framework_declarative._workflows._http_handler as hh @@ -256,10 +244,7 @@ class TestClientOwnership: handler = DefaultHttpRequestHandler() try: await asyncio.gather(*[ - handler.send( - HttpRequestInfo(method="GET", url="https://api.example.test/x") - ) - for _ in range(8) + handler.send(HttpRequestInfo(method="GET", url="https://api.example.test/x")) for _ in range(8) ]) finally: await handler.aclose() @@ -292,9 +277,7 @@ class TestClientProvider: handler = DefaultHttpRequestHandler(client=primary_client, client_provider=provider) try: - result = await handler.send( - HttpRequestInfo(method="GET", url="https://api.example.test/x") - ) + result = await handler.send(HttpRequestInfo(method="GET", url="https://api.example.test/x")) assert result.body == "provided" assert captured["transport"] == "provided" finally: @@ -316,9 +299,7 @@ class TestClientProvider: primary_client = httpx.AsyncClient(transport=httpx.MockTransport(primary)) handler = DefaultHttpRequestHandler(client=primary_client, client_provider=provider) try: - result = await handler.send( - HttpRequestInfo(method="GET", url="https://api.example.test/x") - ) + result = await handler.send(HttpRequestInfo(method="GET", url="https://api.example.test/x")) assert result.body == "primary" finally: await handler.aclose() @@ -343,8 +324,6 @@ class TestAsyncContextManager: @pytest.mark.asyncio async def test_context_manager_closes_owned_client(self) -> None: async with DefaultHttpRequestHandler() as handler: - owned = httpx.AsyncClient( - transport=httpx.MockTransport(lambda r: httpx.Response(200, text="ok")) - ) + owned = httpx.AsyncClient(transport=httpx.MockTransport(lambda r: httpx.Response(200, text="ok"))) handler._owned_client = owned assert owned.is_closed diff --git a/python/packages/declarative/tests/test_http_request_executor.py b/python/packages/declarative/tests/test_http_request_executor.py index effdc5b298..4030cf4294 100644 --- a/python/packages/declarative/tests/test_http_request_executor.py +++ b/python/packages/declarative/tests/test_http_request_executor.py @@ -72,9 +72,7 @@ def _ok(body: str = "", headers: dict[str, list[str]] | None = None) -> HttpRequ ) -def _err( - status: int = 500, body: str = "", headers: dict[str, list[str]] | None = None -) -> HttpRequestResult: +def _err(status: int = 500, body: str = "", headers: dict[str, list[str]] | None = None) -> HttpRequestResult: return HttpRequestResult( status_code=status, is_success_status_code=False, @@ -150,9 +148,7 @@ class TestSuccessPath: async def test_get_parses_json_object(self) -> None: handler = StubHandler(_ok('{"key":"value","number":42}')) factory = WorkflowFactory(http_request_handler=handler) - workflow = factory.create_workflow_from_definition( - _yaml(_action(method="GET", response="Local.Result")) - ) + workflow = factory.create_workflow_from_definition(_yaml(_action(method="GET", response="Local.Result"))) await workflow.run({}) decl = workflow._state.get(DECLARATIVE_STATE_KEY) @@ -165,9 +161,7 @@ class TestSuccessPath: async def test_get_parses_plain_string(self) -> None: handler = StubHandler(_ok("not-json content")) factory = WorkflowFactory(http_request_handler=handler) - workflow = factory.create_workflow_from_definition( - _yaml(_action(response="Local.Result")) - ) + workflow = factory.create_workflow_from_definition(_yaml(_action(response="Local.Result"))) await workflow.run({}) decl = workflow._state.get(DECLARATIVE_STATE_KEY) @@ -177,9 +171,7 @@ class TestSuccessPath: async def test_get_empty_body_yields_none(self) -> None: handler = StubHandler(_ok("")) factory = WorkflowFactory(http_request_handler=handler) - workflow = factory.create_workflow_from_definition( - _yaml(_action(response="Local.Result")) - ) + workflow = factory.create_workflow_from_definition(_yaml(_action(response="Local.Result"))) await workflow.run({}) decl = workflow._state.get(DECLARATIVE_STATE_KEY) @@ -189,9 +181,7 @@ class TestSuccessPath: async def test_response_object_form_path(self) -> None: handler = StubHandler(_ok('{"x":1}')) factory = WorkflowFactory(http_request_handler=handler) - workflow = factory.create_workflow_from_definition( - _yaml(_action(response={"path": "Local.Result"})) - ) + workflow = factory.create_workflow_from_definition(_yaml(_action(response={"path": "Local.Result"}))) await workflow.run({}) decl = workflow._state.get(DECLARATIVE_STATE_KEY) @@ -376,9 +366,7 @@ class TestBody: async def test_unknown_body_kind_raises(self) -> None: handler = StubHandler(_ok()) factory = WorkflowFactory(http_request_handler=handler) - workflow = factory.create_workflow_from_definition( - _yaml(_action(body={"kind": "weirdform", "content": "x"})) - ) + workflow = factory.create_workflow_from_definition(_yaml(_action(body={"kind": "weirdform", "content": "x"}))) with pytest.raises(Exception) as excinfo: await workflow.run({}) # Should surface as ValueError (potentially wrapped by runner) @@ -527,9 +515,7 @@ class TestResponseHeaders: ) ) factory = WorkflowFactory(http_request_handler=handler) - workflow = factory.create_workflow_from_definition( - _yaml(_action(response_headers="Local.H")) - ) + workflow = factory.create_workflow_from_definition(_yaml(_action(response_headers="Local.H"))) await workflow.run({}) decl = workflow._state.get(DECLARATIVE_STATE_KEY) h = decl["Local"]["H"] @@ -540,9 +526,7 @@ class TestResponseHeaders: async def test_response_headers_empty_assigned_none(self) -> None: handler = StubHandler(_ok("ok", headers={})) factory = WorkflowFactory(http_request_handler=handler) - workflow = factory.create_workflow_from_definition( - _yaml(_action(response_headers="Local.H")) - ) + workflow = factory.create_workflow_from_definition(_yaml(_action(response_headers="Local.H"))) await workflow.run({}) decl = workflow._state.get(DECLARATIVE_STATE_KEY) assert decl["Local"]["H"] is None @@ -551,9 +535,7 @@ class TestResponseHeaders: async def test_non_2xx_still_publishes_headers(self) -> None: handler = StubHandler(_err(status=500, body="boom", headers={"X-Trace": ["abc"]})) factory = WorkflowFactory(http_request_handler=handler) - workflow = factory.create_workflow_from_definition( - _yaml(_action(response_headers="Local.H")) - ) + workflow = factory.create_workflow_from_definition(_yaml(_action(response_headers="Local.H"))) with pytest.raises(DeclarativeActionError): await workflow.run({}) decl = workflow._state.get(DECLARATIVE_STATE_KEY) @@ -586,9 +568,7 @@ class TestConversationAppend: async def test_empty_conversation_id_does_not_append(self) -> None: handler = StubHandler(_ok('{"answer":"hello"}')) factory = WorkflowFactory(http_request_handler=handler) - workflow = factory.create_workflow_from_definition( - _yaml(_action(response="Local.Result", conversation_id="")) - ) + workflow = factory.create_workflow_from_definition(_yaml(_action(response="Local.Result", conversation_id=""))) await workflow.run({}) decl = workflow._state.get(DECLARATIVE_STATE_KEY) # Auto-init creates an entry for the System.ConversationId conversation, @@ -600,9 +580,7 @@ class TestConversationAppend: async def test_empty_body_skips_conversation_append(self) -> None: handler = StubHandler(_ok("")) factory = WorkflowFactory(http_request_handler=handler) - workflow = factory.create_workflow_from_definition( - _yaml(_action(conversation_id="conv-test-1")) - ) + workflow = factory.create_workflow_from_definition(_yaml(_action(conversation_id="conv-test-1"))) await workflow.run({}) decl = workflow._state.get(DECLARATIVE_STATE_KEY) # No conversation entry should have been created either. @@ -617,9 +595,7 @@ class TestConnection: async def test_connection_name_forwarded(self) -> None: handler = StubHandler(_ok()) factory = WorkflowFactory(http_request_handler=handler) - workflow = factory.create_workflow_from_definition( - _yaml(_action(connection={"name": "my-connection"})) - ) + workflow = factory.create_workflow_from_definition(_yaml(_action(connection={"name": "my-connection"}))) await workflow.run({}) assert handler.last_info is not None assert handler.last_info.connection_name == "my-connection" @@ -654,9 +630,7 @@ class TestTimeout: async def test_timeout_ms_forwarded(self) -> None: handler = StubHandler(_ok()) factory = WorkflowFactory(http_request_handler=handler) - workflow = factory.create_workflow_from_definition( - _yaml(_action(request_timeout_ms=2500)) - ) + workflow = factory.create_workflow_from_definition(_yaml(_action(request_timeout_ms=2500))) await workflow.run({}) assert handler.last_info is not None assert handler.last_info.timeout_ms == 2500 @@ -665,9 +639,7 @@ class TestTimeout: async def test_timeout_ms_zero_treated_as_unset(self) -> None: handler = StubHandler(_ok()) factory = WorkflowFactory(http_request_handler=handler) - workflow = factory.create_workflow_from_definition( - _yaml(_action(request_timeout_ms=0)) - ) + workflow = factory.create_workflow_from_definition(_yaml(_action(request_timeout_ms=0))) await workflow.run({}) assert handler.last_info is not None assert handler.last_info.timeout_ms is None