diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e93973..d1f88e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,130 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed +- Protocol diagnostics now redact credential-shaped values in request and response + bodies as well as headers. Workflow failure and callback logs also redact + credential-shaped exception text; registry search no longer logs raw intent. +- Standard A2A HTTP and SSE error envelopes retain their status, reason, + retry hint and safe details. An empty response stream fails immediately with + `a2a.empty_stream` instead of returning an unattributed unsuccessful result. +- A malformed subtask `input`, an activated extension missing from AgentCard, + or an unavailable explicitly preferred protocol now fails before dispatch. + Explicit TLS/mTLS/CRL options are rejected for unsupported non-HTTP transports + instead of being silently ignored. +- Hosts may inject `credential_encryption_key` into `A2ATransport` rather than + depending on a process-wide `A2AT_CRED_KEY`; the environment remains the + compatibility fallback. +- `MessageContent.with_extension(uri)` offers an immutable, Java-aligned way + to activate one extension for a single request. +- Registry and PSOP HTTP helpers accept separate connect/read timeout settings + instead of hard-coding both to 30 seconds. +- A **notification stream that died was never released**, and a **healthy one could be + reported as dead**. Liveness was judged from decoded business events only, but an SSE + comment heartbeat never reaches the protocol layer, so a subscription receiving only + heartbeats looked identical to a dead one -- `is_healthy()` returned false and a caller + acting on it tore down a live stream, while a genuinely dead stream leaked its + connection and task forever. Transport activity is now observed per body chunk and + recorded separately from business events. +- **Sensitive header names were only redacted in the exact spelling the pattern + happened to list**: `api_key`, `x_api_key`, `access_session`, `accessSession`, `pwd` + and `passwd` all leaked into the opt-in protocol log, while `api-key` and `ApiKey` + were caught. Names are now matched after stripping `-` and `_`, and a header + registered through `register_sensitive_header` matches in any spelling. +- A **merge step could start before its own predecessor had run**. Step readiness was + judged from *direct* predecessors, and a predecessor that had not been activated yet + was filtered out entirely -- so a merge whose branches became ready in different + batches ran early, with only part of its inputs, and still reported success. Readiness + is now judged from *all active ancestors*, matching the Java engine + (`WorkflowExecutor#getAllPredecessors`): an ancestor that is activated but has not + produced output is the signal that a branch is still running. +- A Negotiation-T Propose carried on a **taskless bare message** (A2A-T pre-task + negotiation, where the remote proposes before creating any task) was returned to + the host as if it were the final business answer: `SendMessageResult.is_success` + reported success and `outputs()` exposed the negotiation prompt as the result. + Such a message now opens a negotiation, correlated by contextId, and the + follow-up send carries no taskId. A bare message whose Negotiation-T metadata is + malformed now fails loudly instead of silently passing as a result. +- When an intermediary (API gateway, load balancer) closed an idle SSE stream + while the remote task was still running, the engine cancelled that task. A + transport fault during stream consumption now falls back to polling `get_task` + every `task_poll_interval_seconds` until the task leaves a non-terminal state or + the deadline expires, so a healthy task is awaited instead of destroyed. + Cancellation for a genuinely local interaction failure is unchanged. +- A `Notification-T` subscription whose remote never acknowledged stayed pending + forever, pinning the caller's coroutine and the connection. The first + acknowledgement is now bounded by `notification_ack_timeout_seconds` (default + 300s); SSE idleness after the acknowledgement is deliberately not bounded. +- `sse_normalization` patched `json_format.Parse` with a duplicate keyword + argument, so every `StreamResponse` text parse raised `TypeError`. The patch + now covers only `ParseDict`, the entry point that actually performs the + normalization, which leaves `Parse` untouched for every other caller. +- A `task` stream event carries a full task snapshot, yet its artifact text was + appended on every update, duplicating the response text. Text is now assembled + once from the merged artifact map, which also gives a non-empty result to + streams that deliver content only through `artifact_update` events. +- `subscribe_to_task` replaced its accumulated message list on every `task` + event, silently dropping standalone messages seen earlier. It now merges, and + additionally reports status and artifact updates and terminal failure codes, + matching `consume_stream`. +- `create_ssl_context` silently discarded the CA, mTLS and CRL arguments when + `verify_server=False`. A client identity is now still presented, while a trust + store or CRL without verification is rejected rather than ignored. +- Protocol logging redacted headers by name pattern alone, so a credential in a + custom `auth_header` was logged in clear. Authentication interceptors now + register the exact header names they inject. +- `failure_mapping` looked the A2A error mapping up by exact type, so a raised + subclass fell through to the generic failure code. It now walks the MRO. +- `env_file_loader` wrote any key from a `.env` file into the process + environment; keys must now be valid environment-variable names. + +### Changed +- Remote A2A `error_details` now uses Java-compatible `httpStatus`, `code`, + `status`, `reason`, `domain`, `details` and `retryAfter` fields. Callers using + the former `http_status` or top-level SDK metadata fields must migrate. +- `NotificationHeartbeat.last_event_at` now reports the last **transport activity**, + including an SSE comment heartbeat, and `event_count` counts only decoded A2A business + events. The last business event is available through the new + `last_business_event_at`. This matches the Java engine's `NotificationSubscription`. +- `NotificationSubscription.is_healthy()` judges liveness from transport activity, so a + subscription that is only receiving heartbeats is no longer reported as dead. A + freshly opened subscription counts as healthy, since opening the stream is activity. +- A notification stream is now **released after `send_timeout_seconds` without any + transport activity** (configurable through + `ExtensionSender(notification_idle_timeout_seconds=...)`), mirroring the Java engine's + idle budget. `await subscription.completion` raises `TimeoutError` when that happens. + A stream that keeps receiving heartbeats is never released. **Hosts that relied on a + subscription staying open forever must be updated.** +- `A2ATransport` accepts `task_poll_interval_seconds` (default 20s, floor 0.1s) + and `notification_ack_timeout_seconds` (default 300s), and `execute_psop` + forwards the former. These mirror the Java engine's `taskPollIntervalMillis` + and `notificationAckTimeoutSeconds`; the poll interval is used *only* after a + stream interruption, never while the SSE stream is alive. +- `consume_stream` includes `task_id` in the `AGENT_STATUS_UPDATE` intermediate + event it forwards, so a caller can learn the remote task identity before a + stream ends and recover from a dropped connection. +- `A2ATransport` caches one A2A client per agent through the new + `client_for(agent_name)` instead of rebuilding it for every send, task poll + and cancellation. `update_agent_cards` drops the cache. +- `A2ATransport.get_extension_uris` is the public form of the former private + `_get_extensions`, which `ExtensionSender` reached into across modules. +- `WorkflowEngineClient.stream_message` accepts `timeout_seconds` and bounds the + whole stream rather than relying on the idle read timeout alone. +- `credential_crypto` accepts an optional `aad` that binds a ciphertext to its + context. It defaults to `None` to stay wire-compatible with the Java SDK. +- Registry helpers accept a custom CA, mTLS identity and CRL, and + `RegistryClient` accepts an access token, so the registry path no longer has + less TLS capability than the transport path. +- `workflow_engine.__version__` is the single source of the distribution version + (`pyproject.toml` reads it through `[tool.setuptools.dynamic]`), and the + `a2a-t-sdk` lower bound now matches what `verify_sdk.py` asserts. + +### Removed +- Dead `A2ATransport._text_from_metadata`, and a stale + `extension_handlers.cpython-312.pyc` whose source was deleted in 0.0.9. + ## [0.1.0] - 2026-09-18 ### Changed diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 1450b41..e40b429 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -4,7 +4,7 @@ - Python 3.12 or newer - `a2a-sdk>=1.1.2,<2` -- `a2a-t-sdk>=1.0.9,<2` (the engine uses its core metadata contract; host callbacks use its content APIs) +- `a2a-t-sdk>=1.1.0,<2` (the engine uses its core metadata contract; host callbacks use its content APIs) ```bash python -m pip install -e ".[dev]" @@ -243,16 +243,21 @@ from a2a_t.core.standard_templates import ( ) auth_transport = A2ATransport(agent_cards, auth_provider=provider) -sender = ExtensionSender(auth_transport) +notification_transport = A2ATransport(agent_cards, auth_provider=provider) +auth_sender = ExtensionSender(auth_transport) +notification_sender = ExtensionSender(notification_transport) generated_auth = a2at_client.generate_auth_prompt_from_text( authorization_text, AUTHORIZATION_POLICY_MANAGEMENT_URI, ) auth_content = A2atMessages.from_generated(generated_auth, [Part(text="authorize policy")]) -auth_result = await sender.send_authorization("domain-a", auth_content) -if not auth_result.is_success: - record_independent_failure(auth_result.failure_code, auth_result.failure_message) +try: + auth_result = await auth_sender.send_authorization("domain-a", auth_content) + if not auth_result.is_success: + record_independent_failure(auth_result.failure_code, auth_result.failure_message) +finally: + await auth_transport.close() # The one-shot channel is no longer needed. ``` Notification uses a long-lived handle: @@ -260,8 +265,7 @@ Notification uses a long-lived handle: ```python def on_notification(subscription, received): process(received) - if is_expected_final_result(received): - subscription.close() + # A single result does not end the subscription; later notifications may follow. generated_notification = a2at_client.generate_notification_prompt_from_text( subscription_text, @@ -271,15 +275,18 @@ notification_content = A2atMessages.from_generated( generated_notification, [Part(text="subscribe to business event")], ) -subscription = sender.open_notification("domain-a", notification_content, on_notification) -ack = await subscription.acknowledgement -if ack.is_failure: +subscription = notification_sender.open_notification( + "domain-a", notification_content, on_notification, +) +try: + await subscription.acknowledgement # A state-bearing ACK; stream remains open. + await subscription.completion # Ends on explicit close, EOF, idle timeout or error. +finally: subscription.close() - record_independent_failure(ack.failure_code, ack.failure_message) -await subscription.completion + await notification_transport.close() ``` -Use a transport instance separate from workflow task traffic. Host orchestration may log an independent-operation failure, but it must not make workflow execution depend on that result unless explicit business policy says so. +Use three transport instances for workflow tasks, Authorization-T, and Notification-T. Authorization-T is one-shot; release its transport after the result. Notification-T keeps its own transport until the subscription ends. A message or artifact without task state is not an ACK, and a failure state rejects the subscription. Host orchestration may log an independent-operation failure, but it must not make workflow execution depend on that result unless explicit business policy says so. ## 7. Existing task operations @@ -332,9 +339,15 @@ The provider owns token acquisition. The engine does not read provider-specific ## 9. TLS and diagnostics -Default TLS verifies the server with system trust. Optional parameters are `ca_certs_path`, `client_cert_path`, `client_key_path`, `client_key_password`, and `crl_path`. Missing or invalid configured files raise during construction. +Default TLS verifies the server with system trust. Optional parameters are `ca_certs_path`, `client_cert_path`, `client_key_path`, `client_key_password`, and `crl_path`. Missing or invalid configured files raise during construction. Explicit TLS/mTLS/CRL options are rejected when the selected protocol cannot apply the HTTP client configuration; select an HTTP protocol or configure that transport's TLS independently. An explicitly preferred protocol absent from the AgentCard fails instead of falling back silently. + +For encrypted agent credentials, pass `credential_encryption_key=<64-hex-character key>` to `A2ATransport` so each host instance can receive its key from a secret manager. `A2AT_CRED_KEY` remains a fallback when no key is injected. Neither key is logged. + +Set `WORKFLOW_ENGINE_PROTOCOL_LOGGING=true` to log pretty-printed final A2A request/response objects. Credential-shaped values in bodies are always redacted. Sensitive headers stay redacted unless the separate sensitive-header flag is explicitly enabled. Protocol logs are diagnostic representations; transport internals may still prevent observation of exact wire bytes. + +Remote A2A failures use `a2a.` or `a2a.http.` when no reason is present. `error_details` follows the Java SDK's `httpStatus`, `code`, `status`, `reason`, `domain`, `details`, and `retryAfter` names. A response stream with no protocol event fails as `a2a.empty_stream`. -Set `WORKFLOW_ENGINE_PROTOCOL_LOGGING=true` to log pretty-printed final A2A request/response objects. Sensitive headers stay redacted unless the separate sensitive-header flag is explicitly enabled. Protocol logs are diagnostic representations; transport internals may still prevent observation of exact wire bytes. +`RegistryClient`, `load_psop`, and `search_psop` accept `connect_timeout_seconds` and `read_timeout_seconds` (both default to 30 seconds). Both must be positive. ## 10. Workflow retrieval diff --git a/README.md b/README.md index 2cd1d3e..a17510e 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ 面向宿主智能体的嵌入式工作流执行引擎。引擎负责工作流 DAG、A2A 消息封装、任务与会话关联、远端任务等待、Negotiation-T 交互循环和生命周期；宿主负责业务输入解释、A2A-T 内容生成与语义校验，以及路由和协商决策。 -Python 与 Java 版本遵循同一业务契约。Python 当前要求 Python 3.12+、`a2a-sdk>=1.1.2,<2` 和 `a2a-t-sdk>=1.0.9,<2`。 +Python 与 Java 版本遵循同一业务契约。Python 当前要求 Python 3.12+、`a2a-sdk>=1.1.2,<2` 和 `a2a-t-sdk>=1.1.0,<2`。 ## 安装 @@ -80,20 +80,31 @@ await transport.close() ## 独立协议操作 -Authorization-T 和 Notification-T 不属于工作流因果链，应使用独立 `A2ATransport` 和 `ExtensionSender`： +Authorization-T 和 Notification-T 不属于工作流因果链。工作流任务、一次性授权和长订阅应分别持有 `A2ATransport`；协议处理能力可以共用，但 HTTP 通道和生命周期不能共用： ```python -sender = ExtensionSender(independent_transport) -authorization = await sender.send_authorization(agent_name, authorization_content) - -subscription = sender.open_notification(agent_name, notification_content, on_notification) -ack = await subscription.acknowledgement -# 收到业务结果后由集成方显式关闭： -subscription.close() -await subscription.completion +auth_transport = A2ATransport(agent_cards) +notification_transport = A2ATransport(agent_cards) +try: + authorization = await ExtensionSender(auth_transport).send_authorization( + agent_name, authorization_content, + ) +finally: + await auth_transport.close() # 授权操作结束即释放，不影响订阅 + +subscription = ExtensionSender(notification_transport).open_notification( + agent_name, notification_content, on_notification, +) +try: + ack = await subscription.acknowledgement # 仅表示订阅已接受，流继续保持 + # 单条通知结果不结束订阅；保持监听，直到明确取消、停机或流异常结束。 + await subscription.completion +finally: + subscription.close() + await notification_transport.close() ``` -授权或订阅失败不会自动阻断后续工作流。`send_authorization` 会等待授权任务到达终态，集成方通过 `is_success` 判断结果；订阅确认通过 `is_failure` 排除失败、拒绝或取消。订阅确认与长连接结束是两个不同 Future；`heartbeat` 和 `is_healthy()` 可用于本地存活性判断。 +授权或订阅失败不会自动阻断后续工作流。`send_authorization` 会等待授权任务到达终态，集成方通过 `is_success` 判断结果；订阅拒绝会让 `acknowledgement` 抛错，普通消息和 artifact 不算 ACK。订阅确认与长连接结束是两个不同 Future；业务任务终态不会自动关闭独立订阅，持续到达的 SSE 心跳可保持长连接存活。显式关闭、远端 EOF、空闲超时或传输异常会结束流。`heartbeat` 和 `is_healthy()` 可用于本地存活性判断。 ## 远端任务管理 diff --git a/README_en.md b/README_en.md index e2bb0f2..96a11c2 100644 --- a/README_en.md +++ b/README_en.md @@ -81,7 +81,8 @@ authorization = await sender.send_authorization(agent_name, authorization_conten subscription = sender.open_notification(agent_name, notification_content, on_notification) ack = await subscription.acknowledgement -subscription.close() +# Keep receiving results; one notification does not end the subscription. +# The host may call subscription.close() on explicit cancellation or shutdown. await subscription.completion ``` diff --git a/pyproject.toml b/pyproject.toml index 4a04882..552755c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "workflow-exec-engine" -version = "0.1.0" +dynamic = ["version"] description = "Standalone workflow execution SDK for A2A-T multi-agent orchestration" readme = "README.md" license = "Apache-2.0" @@ -24,7 +24,7 @@ classifiers = [ ] dependencies = [ "a2a-sdk>=1.1.2,<2", - "a2a-t-sdk>=1.0.9,<2", + "a2a-t-sdk>=1.1.0,<2", "httpx>=0.27.0", "loguru>=0.7.0", "protobuf>=4.25.0", @@ -49,6 +49,11 @@ Changelog = "https://github.com/project-openan/workflow-engine-sdk-python/blob/m [tool.setuptools.packages.find] include = ["workflow_engine*"] +# Single source of truth: the version lives in workflow_engine/__init__.py, so +# the distribution metadata and __version__ can no longer drift apart. +[tool.setuptools.dynamic] +version = { attr = "workflow_engine.__version__" } + [tool.pytest.ini_options] asyncio_mode = "auto" testpaths = ["."] diff --git a/requirements.txt b/requirements.txt index 7567311..02f047d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,5 @@ a2a-sdk>=1.1.2,<2 -a2a-t-sdk>=1.0.9,<2 +a2a-t-sdk>=1.1.0,<2 httpx>=0.27.0 loguru>=0.7.0 protobuf>=4.25.0 diff --git a/test_java_history_parity.py b/test_java_history_parity.py new file mode 100644 index 0000000..04a9c97 --- /dev/null +++ b/test_java_history_parity.py @@ -0,0 +1,494 @@ +# Copyright (c) 2026 Huawei Technologies Co., Ltd. +# All Rights Reserved. +# +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Regression tests derived from the Java engine's fix history. + +The Java engine's commit log records defects it already repaired. Each test here +locks down one of those defects on the Python side, so the two engines cannot +drift back apart. The mapping is recorded in the capability-alignment audit. +""" + +import asyncio + +import httpx +import pytest +from a2a.types import ( + AgentCapabilities, + AgentCard, + AgentExtension, + AgentInterface, + Part, +) +from a2a_t.core import MetadataContent + +from workflow_engine import A2ATExtension, A2atMessages, MessageContent +from workflow_engine.client.a2a_transport import A2ATransport +from workflow_engine.client.extension_sender import ExtensionSender +from workflow_engine.client.stub_engine_client import StubWorkflowEngineClient +from workflow_engine.client.transport_activity import ( + bind_activity_listener, + current_activity_listener, + install_activity_hook, +) +from workflow_engine.control.control_points import ControlPoint +from workflow_engine.core.executor import WorkflowExecutor +from workflow_engine.core.models import ( + JumpCondition, + StepType, + Task, + Workflow, + WorkflowStep, +) + + +class _OrderControlPoint(ControlPoint): + """Records the order in which steps actually dispatch.""" + + def __init__(self): + self.order = [] + + async def on_task(self, request): + self.order.append(request.step_name) + return MessageContent.text(f"out:{request.step_name}") + + +def _step(name, targets, layer): + return WorkflowStep( + name=name, + step_type=StepType.ALL_SUCCESS, + subtasks=[Task(agent=f"agent_{name}", description=f"t_{name}")], + next=[JumpCondition(step=target, condition="") for target in targets], + layer=layer, + ) + + +def _late_branch_workflow() -> Workflow: + """A merge whose two direct predecessors become ready in different batches. + + a -> b, c ; b -> d, q ; c -> d ; d -> s ; s -> p ; q -> m ; p -> m + + ``m`` merges ``p`` and ``q``. ``q`` is ready one batch before ``p``, because + ``p`` waits on ``s`` which waits on ``d``. Judging readiness from *direct* + predecessors alone hides ``p`` (not yet activated) and lets ``m`` run early. + """ + return Workflow(name="late_branch_join", steps=[ + _step("a", ["b", "c"], 0), + _step("b", ["d", "q"], 1), + _step("c", ["d"], 1), + _step("d", ["s"], 2), + _step("s", ["p"], 3), + _step("q", ["m"], 2), + _step("p", ["m"], 4), + _step("m", ["endNode"], 5), + ]) + + +@pytest.mark.asyncio +async def test_merge_waits_for_every_ancestor_not_just_direct_predecessors(): + """Java 366f221: a merge must not run while an ancestor branch is still live. + + Python judged readiness from direct predecessors, so a predecessor that had + not been activated yet was filtered out entirely and the merge ran with only + the other branch's output -- silently, with ``success=True``. + """ + control = _OrderControlPoint() + executor = WorkflowExecutor( + workflow=_late_branch_workflow(), + control_point=control, + engine_client=StubWorkflowEngineClient(), + ) + + result = await executor.run() + + assert result.success, f"Workflow failed: {result.error}" + assert set(control.order) == {"a", "b", "c", "d", "s", "q", "p", "m"} + assert control.order.index("m") > control.order.index("p"), ( + f"merge ran before its own predecessor: {control.order}" + ) + assert control.order.index("m") > control.order.index("q"), ( + f"merge ran before its own predecessor: {control.order}" + ) + assert control.order[-1] == "m", f"merge must run last: {control.order}" + + +@pytest.mark.asyncio +async def test_merge_receives_both_branches_in_its_upstream_context(): + """The premature merge also lost an input; the fix must restore both.""" + workflow = _late_branch_workflow() + control = _OrderControlPoint() + executor = WorkflowExecutor( + workflow=workflow, + control_point=control, + engine_client=StubWorkflowEngineClient(), + ) + + result = await executor.run() + + assert result.success + # context_from is unset, so the merge's upstream is its direct predecessors. + assert set(executor.step_outputs) == {"a", "b", "c", "d", "s", "q", "p", "m"} + assert executor.step_execution_results["m"], "merge produced no task results" + + +@pytest.mark.asyncio +async def test_plain_diamond_join_still_works(): + """The fix must not disturb the ordinary two-branch diamond.""" + control = _OrderControlPoint() + workflow = Workflow(name="diamond", steps=[ + _step("left", ["merge"], 0), + _step("right", ["merge"], 0), + _step("merge", ["endNode"], 1), + ]) + executor = WorkflowExecutor( + workflow=workflow, + control_point=control, + engine_client=StubWorkflowEngineClient(), + ) + + result = await executor.run() + + assert result.success + assert control.order[-1] == "merge" + assert set(control.order[:2]) == {"left", "right"} + + +# ====================================================================== +# H5: sensitive header names must match across separator spellings +# ====================================================================== + +_REDACTED = "***REDACTED***" + +_SENSITIVE_SPELLINGS = [ + "Authorization", + "X-Auth-Token", + "x_auth_token", + "client_secret", + "client-secret", + "api_key", + "api-key", + "X-API-KEY", + "ApiKey", + "x_api_key", + "access_session", + "accessSession", + "Access-Session", + "pwd", + "passwd", + "Password", + "Cookie", + "x_cookie", +] + +_NOT_SENSITIVE = ["Content-Type", "A2A-Extensions", "Accept", "X-Request-Id"] + + +def _render_request_headers(monkeypatch, headers) -> str: + from loguru import logger as loguru_logger + + from workflow_engine.client.protocol_logger import log_request + + messages = [] + sink = loguru_logger.add(messages.append, format="{message}", level="DEBUG") + try: + monkeypatch.setenv("WORKFLOW_ENGINE_PROTOCOL_LOGGING", "true") + monkeypatch.delenv( + "WORKFLOW_ENGINE_PROTOCOL_INCLUDE_SENSITIVE_HEADERS", raising=False, + ) + log_request("agent", "https://example.com", "payload", headers) + finally: + loguru_logger.remove(sink) + return "".join(messages) + + +@pytest.mark.parametrize("name", _SENSITIVE_SPELLINGS) +def test_sensitive_header_is_redacted_in_every_spelling(monkeypatch, name): + """Java 366f221: match after stripping '-' and '_'. + + Matching the raw lower-case name silently missed ``api_key``, + ``access_session``, ``pwd`` and ``passwd``. + """ + rendered = _render_request_headers(monkeypatch, {name: "top-secret-value"}) + assert "top-secret-value" not in rendered, f"{name} leaked into the log" + assert _REDACTED in rendered + + +@pytest.mark.parametrize("name", _NOT_SENSITIVE) +def test_non_sensitive_header_stays_readable(monkeypatch, name): + rendered = _render_request_headers(monkeypatch, {name: "visible-value"}) + assert "visible-value" in rendered + + +def test_registered_header_matches_any_spelling(monkeypatch): + """A custom ``auth_header`` is registered by one spelling, seen in another.""" + from workflow_engine.client.protocol_logger import register_sensitive_header + + register_sensitive_header("X-Corp-Cred") + rendered = _render_request_headers( + monkeypatch, {"x_corp_cred": "top-secret-value"}, + ) + assert "top-secret-value" not in rendered + assert _REDACTED in rendered + + +def test_url_shaped_name_is_not_treated_as_a_credential(monkeypatch): + """A name carrying ``://`` is an endpoint, not a credential header.""" + rendered = _render_request_headers( + monkeypatch, {"https://gateway.example/token": "visible-value"}, + ) + assert "visible-value" in rendered + + +# ====================================================================== +# H2: notification liveness must come from transport activity +# ====================================================================== + +def _notification_card() -> AgentCard: + return AgentCard( + name="agent", + capabilities=AgentCapabilities( + extensions=[AgentExtension(uri=A2ATExtension.NOTIFICATION_T.uri)], + ), + supported_interfaces=[ + AgentInterface(url="https://agent.example/a2a", protocol_binding="HTTP+JSON") + ], + ) + + +def _notification_content() -> MessageContent: + return A2atMessages.from_generated( + MetadataContent( + "Notification-T/information-negotiation/propose/v1", + "notification prompt", + A2ATExtension.NOTIFICATION_T.uri, + ), + [Part(text="notification prompt")], + ) + + +class _HeartbeatClient: + """Open forever, reporting transport activity but never yielding an event. + + Stands in for an SSE stream carrying only comment heartbeats: bytes keep + arriving, but the protocol layer never sees a decoded event. It reports + activity through the same public accessor the real httpx hook uses. + """ + + def __init__(self, interval: float = 0.02): + self._interval = interval + self.activity_reports = 0 + + async def send_message(self, request): + del request + while True: + await asyncio.sleep(self._interval) + listener = current_activity_listener() + if listener is not None: + listener() + self.activity_reports += 1 + yield None # unreachable; keeps this an async generator + + +class _QuietAfterFirstEventClient: + """Deliver one event, then hold the stream open with no further activity.""" + + async def send_message(self, request): + del request + from a2a.types.a2a_pb2 import StreamResponse + + response = StreamResponse() + response.message.message_id = "m1" + response.message.context_id = "ctx" + response.message.role = 1 + response.message.parts.add().text = "ack" + yield response + await asyncio.sleep(3600) + + +def _notification_transport(client) -> A2ATransport: + transport = A2ATransport(agent_cards=[_notification_card()]) + transport.create_a2a_client = lambda card: client + return transport + + +@pytest.mark.asyncio +async def test_idle_notification_stream_is_released(): + """A dead stream used to leak its connection and its task forever.""" + transport = _notification_transport(_QuietAfterFirstEventClient()) + sender = ExtensionSender(transport, notification_idle_timeout_seconds=0.15) + subscription = sender.open_notification( + "agent", _notification_content(), lambda subscription, message: None, + ) + try: + with pytest.raises(TimeoutError, match="saw no activity"): + await asyncio.wait_for(subscription.completion, timeout=5.0) + assert not subscription.is_active + finally: + subscription.close() + await transport.close() + + +@pytest.mark.asyncio +async def test_heartbeats_keep_a_notification_stream_alive(): + """The reaper must not tear down a healthy stream that is merely quiet.""" + client = _HeartbeatClient() + transport = _notification_transport(client) + sender = ExtensionSender(transport, notification_idle_timeout_seconds=0.2) + subscription = sender.open_notification( + "agent", _notification_content(), lambda subscription, message: None, + ) + try: + await asyncio.sleep(0.7) # more than three idle budgets + assert client.activity_reports > 0 + assert subscription.is_active, "a stream receiving heartbeats was reaped" + assert subscription.is_healthy(0.5) + finally: + subscription.close() + await transport.close() + + +@pytest.mark.asyncio +async def test_heartbeat_separates_transport_activity_from_business_events(): + """``last_event_at`` tracks transport activity; the event count does not move.""" + transport = _notification_transport(_HeartbeatClient()) + sender = ExtensionSender(transport, notification_idle_timeout_seconds=5.0) + subscription = sender.open_notification( + "agent", _notification_content(), lambda subscription, message: None, + ) + try: + await asyncio.sleep(0.2) + heartbeat = subscription.heartbeat + assert heartbeat.event_count == 0 + assert heartbeat.last_business_event_at is None + assert heartbeat.last_event_at is not None + assert heartbeat.last_event_at >= heartbeat.opened_at + finally: + subscription.close() + await transport.close() + + +@pytest.mark.asyncio +async def test_is_healthy_is_true_for_a_quiet_but_live_stream(): + """Judging liveness from business events reported a live stream as dead.""" + transport = _notification_transport(_HeartbeatClient()) + sender = ExtensionSender(transport, notification_idle_timeout_seconds=5.0) + subscription = sender.open_notification( + "agent", _notification_content(), lambda subscription, message: None, + ) + try: + await asyncio.sleep(0.15) + assert subscription.last_business_event_at is None + assert subscription.is_healthy(0.5) is True + finally: + subscription.close() + await transport.close() + + +@pytest.mark.asyncio +async def test_notification_idle_timeout_override_is_validated(): + transport = _notification_transport(_HeartbeatClient()) + try: + with pytest.raises(ValueError, match="notification_idle_timeout_seconds"): + ExtensionSender(transport, notification_idle_timeout_seconds=0) + finally: + await transport.close() + + +@pytest.mark.asyncio +async def test_host_callback_cannot_refresh_subscription_activity(): + """Protocol traffic issued by the host callback must not count as activity. + + Otherwise a callback that keeps calling the remote would keep a dead + notification stream alive. + """ + seen_inside_callback = [] + + class _OneEventClient: + async def send_message(self, request): + del request + from a2a.types.a2a_pb2 import StreamResponse + + response = StreamResponse() + response.message.message_id = "m1" + response.message.context_id = "ctx" + response.message.role = 1 + response.message.parts.add().text = "event" + yield response + await asyncio.sleep(3600) + + transport = _notification_transport(_OneEventClient()) + sender = ExtensionSender(transport, notification_idle_timeout_seconds=5.0) + + def listener(subscription, message): + del subscription, message + seen_inside_callback.append(current_activity_listener()) + + subscription = sender.open_notification( + "agent", _notification_content(), listener, + ) + try: + await asyncio.sleep(0.2) + assert seen_inside_callback, "the listener never ran" + assert seen_inside_callback[0] is None, ( + "the activity listener stayed bound while the host callback ran" + ) + finally: + subscription.close() + await transport.close() + + +@pytest.mark.asyncio +async def test_activity_hook_reports_body_chunks_and_is_scoped_to_the_call(): + """The mechanism the reaper relies on, at the httpx layer. + + A comment-only SSE chunk must count as activity, and traffic outside a bound + call must not report anything -- otherwise one agent's traffic would keep + another agent's dead subscription alive. + """ + chunks = [b": heartbeat\n\n", b'data: {"a":1}\n\n', b": heartbeat\n\n"] + reported = [] + + class _Stream(httpx.AsyncByteStream): + async def __aiter__(self): + for chunk in chunks: + yield chunk + + async def handler(request): + del request + return httpx.Response( + 200, headers={"content-type": "text/event-stream"}, stream=_Stream() + ) + + client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) + assert install_activity_hook(client) is True + assert install_activity_hook(client) is False, "hook must install only once" + async with client: + # No listener bound: nothing is reported. + async with client.stream("GET", "http://example/sse") as response: + async for _ in response.aiter_lines(): + pass + assert reported == [] + + # Bound for the call: every chunk is reported. + with bind_activity_listener(lambda: reported.append("activity")): + async with client.stream("GET", "http://example/sse") as response: + async for _ in response.aiter_lines(): + pass + + # Every body chunk, plus one report for the response arriving at all: a slow + # remote whose headers land late must not be reaped on the creation timestamp. + assert len(reported) == len(chunks) + 1 + assert current_activity_listener() is None, "binding must be reset" diff --git a/test_notification_lifecycle.py b/test_notification_lifecycle.py new file mode 100644 index 0000000..5faa7e0 --- /dev/null +++ b/test_notification_lifecycle.py @@ -0,0 +1,205 @@ +# Copyright (c) 2026 Huawei Technologies Co., Ltd. +# All Rights Reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Notification-T ACK and stream lifecycle regressions.""" + +import asyncio + +import pytest +from a2a.types.a2a_pb2 import StreamResponse, TaskState + +from test_p0_alignment import _notification_card, _notification_content +from workflow_engine.client.a2a_transport import A2ATransport +from workflow_engine.client.extension_sender import ExtensionSender + + +def _status(state): + response = StreamResponse() + response.status_update.task_id = "task-1" + response.status_update.status.state = state + return response + + +def _message(): + response = StreamResponse() + response.message.message_id = "message-1" + response.message.role = 1 + response.message.parts.add().text = "business event" + return response + + +def _artifact(): + response = StreamResponse() + response.artifact_update.task_id = "task-1" + response.artifact_update.artifact.artifact_id = "artifact-1" + response.artifact_update.artifact.parts.add().text = "business result" + return response + + +async def _open(events, *, linger=False): + class Client: + async def send_message(self, request): + for event in events: + yield event + if linger: + await asyncio.sleep(3600) + + transport = A2ATransport( + agent_cards=[_notification_card()], notification_ack_timeout_seconds=2, + ) + transport.create_a2a_client = lambda card: Client() + received = [] + subscription = ExtensionSender(transport).open_notification( + "agent", _notification_content(), lambda _, message: received.append(message), + ) + return transport, subscription, received + + +@pytest.mark.asyncio +async def test_empty_stream_fails_ack_immediately_and_cleans_up_timers(): + transport, subscription, _ = await _open([]) + try: + with pytest.raises(RuntimeError, match="ended before acknowledgement"): + await asyncio.wait_for(subscription.acknowledgement, 0.5) + await asyncio.wait_for(subscription.completion, 0.5) + await asyncio.sleep(0) + assert not subscription.is_active + assert subscription._ack_timer.done() + assert subscription._idle_timer.done() + finally: + subscription.close() + await transport.close() + + +@pytest.mark.asyncio +async def test_open_subscription_has_no_reported_transport_activity_yet(): + """Java exposes no heartbeat timestamp until bytes actually arrive.""" + transport, subscription, _ = await _open([], linger=True) + try: + assert subscription.heartbeat.last_event_at is None + assert not subscription.is_healthy(1) + finally: + subscription.close() + await asyncio.wait_for(subscription.completion, 0.5) + await transport.close() + + +@pytest.mark.asyncio +async def test_business_message_and_artifact_do_not_acknowledge_subscription(): + transport, subscription, received = await _open( + [_message(), _artifact(), _status(TaskState.TASK_STATE_WORKING)], linger=True, + ) + try: + result = await asyncio.wait_for(subscription.acknowledgement, 0.5) + assert result.task_state == "TASK_STATE_WORKING" + assert len(received) == 3 + assert subscription.is_active + await asyncio.sleep(0) + assert subscription._ack_timer.done() + finally: + subscription.close() + await asyncio.wait_for(subscription.completion, 0.5) + await transport.close() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("state", [TaskState.TASK_STATE_FAILED, TaskState.TASK_STATE_REJECTED]) +async def test_failure_state_rejects_ack_and_ends_stream(state): + transport, subscription, _ = await _open([_status(state)], linger=True) + try: + with pytest.raises(RuntimeError, match="subscription rejected"): + await asyncio.wait_for(subscription.acknowledgement, 0.5) + await asyncio.wait_for(subscription.completion, 0.5) + assert not subscription.is_active + finally: + subscription.close() + await transport.close() + + +@pytest.mark.asyncio +async def test_normal_eof_after_ack_releases_timers(): + transport, subscription, _ = await _open([_status(TaskState.TASK_STATE_WORKING)]) + try: + await asyncio.wait_for(subscription.acknowledgement, 0.5) + await asyncio.wait_for(subscription.completion, 0.5) + await asyncio.sleep(0) + assert subscription._ack_timer.done() + assert subscription._idle_timer.done() + finally: + subscription.close() + await transport.close() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("result_state", [ + TaskState.TASK_STATE_COMPLETED, + TaskState.TASK_STATE_FAILED, + TaskState.TASK_STATE_CANCELED, + TaskState.TASK_STATE_REJECTED, + TaskState.TASK_STATE_INPUT_REQUIRED, + TaskState.TASK_STATE_AUTH_REQUIRED, +]) +async def test_business_task_result_state_does_not_end_subscription(result_state): + """Result state cannot cancel the independent, already acknowledged stream.""" + transport, subscription, received = await _open([ + _status(TaskState.TASK_STATE_WORKING), + _status(result_state), + _message(), + ], linger=True) + try: + ack = await asyncio.wait_for(subscription.acknowledgement, 0.5) + assert ack.task_state == "TASK_STATE_WORKING" + await asyncio.wait_for(_wait_for_messages(received, 3), 0.5) + assert subscription.is_active + assert not subscription.completion.done() + finally: + subscription.close() + await asyncio.wait_for(subscription.completion, 0.5) + await transport.close() + + +async def _wait_for_messages(received, count): + while len(received) < count: + await asyncio.sleep(0) + + +@pytest.mark.asyncio +async def test_each_notification_has_its_own_a2a_client_and_closes_its_stream(): + clients = [] + + class Client: + def __init__(self): + self.stream_closed = False + + async def send_message(self, request): + try: + yield _status(TaskState.TASK_STATE_WORKING) + await asyncio.sleep(3600) + finally: + self.stream_closed = True + + def make_client(card): + client = Client() + clients.append(client) + return client + + transport = A2ATransport(agent_cards=[_notification_card()]) + transport.create_a2a_client = make_client + sender = ExtensionSender(transport) + first = sender.open_notification("agent", _notification_content(), lambda *_: None) + second = sender.open_notification("agent", _notification_content(), lambda *_: None) + try: + await asyncio.wait_for(asyncio.gather( + first.acknowledgement, second.acknowledgement, + ), 0.5) + assert len(clients) == 2 + assert clients[0] is not clients[1] + first.close() + await asyncio.wait_for(first.completion, 0.5) + assert clients[0].stream_closed + assert second.is_active + finally: + second.close() + await asyncio.wait_for(second.completion, 0.5) + await transport.close() diff --git a/test_p0_alignment.py b/test_p0_alignment.py new file mode 100644 index 0000000..dbd05cb --- /dev/null +++ b/test_p0_alignment.py @@ -0,0 +1,561 @@ +# Copyright (c) 2026 Huawei Technologies Co., Ltd. +# All Rights Reserved. +# +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Regression tests for the three P0 capability gaps against the Java engine. + +Each test names the gap it locks down, so a failure maps straight back to a +finding in the capability-alignment audit: + +- P0-1 pre-task negotiation: a Negotiation-T Propose on a *taskless* bare + message must enter negotiation instead of being returned as a result. +- P0-2 an interrupted response stream must fall back to polling the remote + task rather than abandoning (and cancelling) a task that still runs. +- P0-3 the first Notification-T acknowledgement must be bounded by a timeout. +""" + +import asyncio +from types import SimpleNamespace + +import pytest +from a2a.types import ( + AgentCapabilities, + AgentCard, + AgentExtension, + AgentInterface, + Part, + Task, + TaskState, + TaskStatus, +) +from a2a_t.core import MetadataContent, NegotiationContext, NegotiationPerformative +from a2a_t.core.standard_templates import INFORMATION_NEGOTIATION_ACCEPT_REJECT_URI + +from workflow_engine import ( + A2ATExtension, + A2atMessages, + BusinessInput, + ControlPoint, + MessageContent, + NegotiationReply, + ReceivedMessage, + SendMessageResult, + TaskRequest, + WorkflowEngineClient, +) +from workflow_engine.client.a2a_transport import A2ATransport +from workflow_engine.client.extension_sender import ExtensionSender +from workflow_engine.control.control_points import EventType + + +# ---------------------------------------------------------------------- +# Helpers +# ---------------------------------------------------------------------- + +def _proposal(performative=NegotiationPerformative.PROPOSE, round_number=1): + """A Negotiation-T message the remote would send to open a negotiation.""" + return A2atMessages.from_generated( + MetadataContent( + "Negotiation-T/information-negotiation/propose/v1", + "negotiation prompt", + A2ATExtension.NEGOTIATION_T.uri, + NegotiationContext( + "3dbc13b5-bd57-4c2b-b503-24e381b6c8d3", round_number, 5, performative, + ), + ), + [Part(text="negotiation prompt")], + ) + + +def _task_request() -> TaskRequest: + return TaskRequest( + execution_id="execution", task_id="logical", input=BusinessInput.from_text("task"), + agent_name="agent", skill="", instruction="task", step_name="step", + ) + + +class _RecordingControlPoint(ControlPoint): + """Accepts the first proposal and records every negotiation it is asked about.""" + + def __init__(self): + self.requests = [] + + async def on_negotiation(self, request): + self.requests.append(request) + proposal = A2atMessages.negotiation_context(request.received) + generated = MetadataContent( + INFORMATION_NEGOTIATION_ACCEPT_REJECT_URI, + "clarification accepted", + A2ATExtension.NEGOTIATION_T.uri, + proposal.with_performative(NegotiationPerformative.ACCEPT), + ) + return NegotiationReply.send( + A2atMessages.from_generated( + generated, [Part(text="clarification accepted")] + ) + ) + + +class _ScriptedTransport: + """Transport double with scripted per-send results and a breakable stream.""" + + _send_timeout_seconds = 5 + + def __init__( + self, + results=(), + *, + break_on_send=None, + stream_error=None, + reveal_task_id=True, + poll_states=(), + task_poll_interval_seconds=0.01, + ): + self._results = list(results) + self._break_on_send = break_on_send + self._stream_error = stream_error or RuntimeError("gateway closed the stream") + self._reveal_task_id = reveal_task_id + self._poll_states = list(poll_states) + self._task_poll_interval_seconds = task_poll_interval_seconds + self.sends = 0 + self.task_ids = [] + self.cancelled = [] + self.polls = 0 + self.card = SimpleNamespace( + name="agent", + supported_interfaces=[SimpleNamespace(url="https://agent.example/a2a")], + ) + + @property + def agent_names(self): + return ["agent"] + + @property + def send_timeout_seconds(self): + return self._send_timeout_seconds + + @property + def task_poll_interval_seconds(self): + return self._task_poll_interval_seconds + + def get_card(self, name): + return self.card if name == "agent" else None + + def create_a2a_client(self, card): + return object() + + def client_for(self, agent_name): + return self.create_a2a_client(self.get_card(agent_name)) + + def validate_content_extensions(self, card, content): + return None + + def build_send_request(self, content, context_id, task_id=None): + self.task_ids.append(task_id or "") + return SimpleNamespace( + message=SimpleNamespace(context_id=context_id, task_id=task_id or ""), + ) + + async def consume_stream(self, client, request, callback, agent_name): + self.sends += 1 + if self._break_on_send is not None and self.sends >= self._break_on_send: + # Emulate a stream that delivered a task snapshot and then died: the + # identity reaches the caller through the intermediate callback + # before the transport fault surfaces. + if self._reveal_task_id: + callback(EventType.AGENT_STATUS_UPDATE, { + "agent": agent_name, "task_id": "remote-1", + "state": "TASK_STATE_WORKING", "is_final": False, + "text": "", "metadata": {}, + }) + raise self._stream_error + result = self._results[self.sends - 1] + # A real remote echoes the caller's contextId; identity validation + # compares against it, so the double has to do the same. + if result.task is not None: + result.task.context_id = request.message.context_id + return result + + async def get_task(self, agent_name, task_id): + self.polls += 1 + state = self._poll_states.pop(0) if self._poll_states else "TASK_STATE_COMPLETED" + return SendMessageResult(task_state=state, text="done") + + async def cancel_task(self, agent_name, task_id): + self.cancelled.append((agent_name, task_id)) + return SendMessageResult(task_state="TASK_STATE_CANCELED") + + async def close(self): + pass + + +def _completed(task_id="remote-1", text="complete") -> SendMessageResult: + return SendMessageResult( + task=Task( + id=task_id, context_id="ctx", + status=TaskStatus(state=TaskState.TASK_STATE_COMPLETED), + ), + task_state="TASK_STATE_COMPLETED", + text=text, + received_messages=(ReceivedMessage(message=MessageContent.text(text)),), + ) + + +def _bare_message_result(content) -> SendMessageResult: + """A standalone message with no task at all -- A2A-T pre-task negotiation.""" + return SendMessageResult( + task=None, + task_state="", + text="negotiation prompt", + received_messages=(ReceivedMessage(message=content),), + ) + + +# ---------------------------------------------------------------------- +# P0-1: a Propose on a taskless bare message must open a negotiation +# ---------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_taskless_bare_message_propose_enters_negotiation(): + """A pre-task Propose used to be returned to the host as the final answer.""" + transport = _ScriptedTransport([ + _bare_message_result(_proposal()), + _completed(), + ]) + control = _RecordingControlPoint() + + result = await WorkflowEngineClient(transport).dispatch( + _task_request(), MessageContent.text("task"), control, + ) + + assert len(control.requests) == 1, "the host must be asked to negotiate" + assert result.task_state == "TASK_STATE_COMPLETED" + assert transport.sends == 2 + + +@pytest.mark.asyncio +async def test_taskless_negotiation_continuation_carries_no_task_id(): + """Pre-task negotiation is correlated by contextId; no task exists yet.""" + transport = _ScriptedTransport([ + _bare_message_result(_proposal()), + _completed(), + ]) + + await WorkflowEngineClient(transport).dispatch( + _task_request(), MessageContent.text("task"), _RecordingControlPoint(), + ) + + assert transport.task_ids == ["", ""], "no taskId may be sent before one exists" + + +@pytest.mark.asyncio +async def test_bare_message_without_negotiation_metadata_is_still_a_final_answer(): + """A plain standalone message must keep completing the interaction.""" + transport = _ScriptedTransport([_bare_message_result(MessageContent.text("here you go"))]) + control = _RecordingControlPoint() + + result = await WorkflowEngineClient(transport).dispatch( + _task_request(), MessageContent.text("task"), control, + ) + + assert control.requests == [], "a plain answer must not be treated as a proposal" + assert result.is_success + + +@pytest.mark.asyncio +async def test_invalid_negotiation_metadata_on_a_bare_message_fails_loudly(): + """Malformed Negotiation-T metadata must fail, never pass as a result.""" + transport = _ScriptedTransport([ + _bare_message_result(_proposal(NegotiationPerformative.ACCEPT)), + ]) + control = _RecordingControlPoint() + + with pytest.raises(ValueError, match="valid Negotiation-T Propose"): + await WorkflowEngineClient(transport).dispatch( + _task_request(), MessageContent.text("task"), control, + ) + + assert control.requests == [] + + +@pytest.mark.asyncio +async def test_repeated_taskless_round_is_reported_instead_of_looping(): + """Without a task there is nothing to poll, so a repeat must not spin.""" + transport = _ScriptedTransport([ + _bare_message_result(_proposal()), + _bare_message_result(_proposal()), + ]) + + with pytest.raises(RuntimeError, match="taskless negotiation round"): + await WorkflowEngineClient(transport).dispatch( + _task_request(), MessageContent.text("task"), _RecordingControlPoint(), + ) + + +# ---------------------------------------------------------------------- +# P0-2: an interrupted stream must poll, not cancel a still-running task +# ---------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_interrupted_stream_polls_instead_of_cancelling_the_remote_task(): + """The old path cancelled a healthy task merely because a gateway dropped SSE.""" + transport = _ScriptedTransport(break_on_send=1) + + result = await WorkflowEngineClient(transport).dispatch( + _task_request(), MessageContent.text("task"), _RecordingControlPoint(), + ) + + assert result.task_state == "TASK_STATE_COMPLETED" + assert transport.polls == 1 + assert transport.cancelled == [], "a transport fault must never cancel the task" + + +@pytest.mark.asyncio +async def test_interrupted_stream_polls_until_the_task_reaches_a_terminal_state(): + transport = _ScriptedTransport( + break_on_send=1, + poll_states=("TASK_STATE_WORKING", "TASK_STATE_WORKING", "TASK_STATE_COMPLETED"), + ) + + result = await WorkflowEngineClient(transport).dispatch( + _task_request(), MessageContent.text("task"), _RecordingControlPoint(), + ) + + assert result.task_state == "TASK_STATE_COMPLETED" + assert transport.polls == 3 + assert transport.cancelled == [] + + +@pytest.mark.asyncio +async def test_interrupted_stream_without_a_task_identity_still_propagates(): + """With no task identity there is nothing to poll, so the fault must surface.""" + transport = _ScriptedTransport(break_on_send=1, reveal_task_id=False) + + with pytest.raises(RuntimeError, match="gateway closed the stream"): + await WorkflowEngineClient(transport).dispatch( + _task_request(), MessageContent.text("task"), _RecordingControlPoint(), + ) + + assert transport.cancelled == [] + + +@pytest.mark.asyncio +async def test_remote_rejection_is_not_mistaken_for_a_recoverable_stream_break(): + from workflow_engine.client.remote_error import RemoteA2AError + + error = RemoteA2AError(403, 403, "denied", reason="ACCESS_DENIED") + transport = _ScriptedTransport(break_on_send=1, stream_error=error) + + with pytest.raises(RemoteA2AError, match="denied"): + await WorkflowEngineClient(transport).dispatch( + _task_request(), MessageContent.text("task"), _RecordingControlPoint(), + ) + + assert transport.polls == 0 + assert transport.cancelled == [] + + +@pytest.mark.asyncio +async def test_empty_stream_does_not_poll_or_cancel_a_known_task(): + from workflow_engine.client.remote_error import EmptyA2AStreamError + + transport = _ScriptedTransport( + break_on_send=1, + stream_error=EmptyA2AStreamError("A2A response stream ended without an event"), + ) + with pytest.raises(EmptyA2AStreamError): + await WorkflowEngineClient(transport).dispatch( + _task_request(), MessageContent.text("task"), _RecordingControlPoint(), + ) + assert transport.polls == 0 + assert transport.cancelled == [] + + +@pytest.mark.asyncio +async def test_local_negotiation_failure_still_cancels_the_remote_task(): + """The cancellation contract for local interaction failures must survive.""" + class RefusingControlPoint(ControlPoint): + async def on_negotiation(self, request): + raise ValueError("host cannot answer") + + transport = _ScriptedTransport([ + SendMessageResult( + task=Task( + id="remote-1", context_id="ctx", + status=TaskStatus(state=TaskState.TASK_STATE_INPUT_REQUIRED), + ), + task_state="TASK_STATE_INPUT_REQUIRED", + received_messages=(ReceivedMessage(message=_proposal()),), + ), + ]) + + with pytest.raises(ValueError, match="host cannot answer"): + await WorkflowEngineClient(transport).dispatch( + _task_request(), MessageContent.text("task"), RefusingControlPoint(), + ) + + assert transport.cancelled == [("agent", "remote-1")] + + +@pytest.mark.asyncio +async def test_status_update_events_carry_the_task_id(): + """The identity that makes stream recovery possible must reach the callback.""" + from a2a.types.a2a_pb2 import StreamResponse + + transport = A2ATransport(agent_cards=[AgentCard( + name="agent", + capabilities=AgentCapabilities(), + supported_interfaces=[ + AgentInterface(url="https://agent.example/a2a", protocol_binding="HTTP+JSON") + ], + )]) + response = StreamResponse() + response.task.id = "remote-9" + response.task.context_id = "ctx" + response.task.status.state = TaskState.TASK_STATE_WORKING + + class _Client: + async def send_message(self, request): + del request + yield response + + seen = [] + try: + await transport.consume_stream( + _Client(), object(), + lambda event_type, data: seen.append((event_type, data)), "agent", + ) + finally: + await transport.close() + + assert seen[0][0] == EventType.AGENT_STATUS_UPDATE + assert seen[0][1]["task_id"] == "remote-9" + + +def test_task_poll_interval_is_validated(): + """Mirrors the Java engine's 100ms floor for taskPollIntervalMillis.""" + with pytest.raises(ValueError, match="task_poll_interval_seconds"): + A2ATransport(agent_cards=[], task_poll_interval_seconds=0.05) + + transport = A2ATransport(agent_cards=[], task_poll_interval_seconds=0.1) + assert transport.task_poll_interval_seconds == 0.1 + + +# ---------------------------------------------------------------------- +# P0-3: the first Notification-T acknowledgement must be bounded +# ---------------------------------------------------------------------- + +def _notification_card() -> AgentCard: + return AgentCard( + name="agent", + capabilities=AgentCapabilities( + extensions=[AgentExtension(uri=A2ATExtension.NOTIFICATION_T.uri)], + ), + supported_interfaces=[ + AgentInterface(url="https://agent.example/a2a", protocol_binding="HTTP+JSON") + ], + ) + + +def _notification_content() -> MessageContent: + return A2atMessages.from_generated( + MetadataContent( + "Notification-T/information-negotiation/propose/v1", + "notification prompt", + A2ATExtension.NOTIFICATION_T.uri, + ), + [Part(text="notification prompt")], + ) + + +class _SilentStreamClient: + """Accepts the subscription and then never sends anything at all.""" + + async def send_message(self, request): + del request + await asyncio.sleep(3600) + yield None + + +class _ImmediateAckClient: + """Acknowledges at once, then stays open and idle.""" + + async def send_message(self, request): + del request + from a2a.types.a2a_pb2 import StreamResponse + + response = StreamResponse() + from a2a.types.a2a_pb2 import TaskState + response.status_update.task_id = "task-1" + response.status_update.status.state = TaskState.TASK_STATE_WORKING + yield response + await asyncio.sleep(3600) + + +def _notification_transport(client, **kwargs) -> A2ATransport: + transport = A2ATransport(agent_cards=[_notification_card()], **kwargs) + transport.create_a2a_client = lambda card: client + return transport + + +@pytest.mark.asyncio +async def test_notification_acknowledgement_times_out_when_the_remote_never_acks(): + """The acknowledgement future used to stay pending forever.""" + transport = _notification_transport( + _SilentStreamClient(), notification_ack_timeout_seconds=0.05, + ) + sender = ExtensionSender(transport) + subscription = sender.open_notification( + "agent", _notification_content(), lambda subscription, message: None, + ) + try: + with pytest.raises(TimeoutError, match="acknowledgement timed out"): + await asyncio.wait_for(subscription.acknowledgement, timeout=5.0) + assert not subscription.is_active + finally: + subscription.close() + await transport.close() + + +@pytest.mark.asyncio +async def test_notification_ack_timeout_does_not_bound_sse_idle_after_the_ack(): + """Only the first ACK is bounded; a quiet-but-open stream must survive.""" + transport = _notification_transport( + _ImmediateAckClient(), notification_ack_timeout_seconds=0.05, + ) + sender = ExtensionSender(transport) + subscription = sender.open_notification( + "agent", _notification_content(), lambda subscription, message: None, + ) + try: + result = await asyncio.wait_for(subscription.acknowledgement, timeout=5.0) + assert result.received_messages + + # Idle for well past the ACK window: the subscription must stay open. + await asyncio.sleep(0.2) + assert subscription.is_active + finally: + subscription.close() + await transport.close() + + +def test_notification_ack_timeout_override_is_validated(): + transport = _notification_transport(_SilentStreamClient()) + with pytest.raises(ValueError, match="notification_ack_timeout_seconds"): + ExtensionSender(transport, notification_ack_timeout_seconds=0) + + sender = ExtensionSender(transport, notification_ack_timeout_seconds=0.5) + assert sender is not None diff --git a/test_protocol_parity.py b/test_protocol_parity.py index 2ae98a9..9960e07 100644 --- a/test_protocol_parity.py +++ b/test_protocol_parity.py @@ -211,8 +211,9 @@ def test_standard_a2a_error_maps_to_stable_business_failure(): assert not result.success assert result.error_code == "a2a.task_not_found" assert result.error == "Task unavailable" - assert result.error_details["http_status"] == 404 - assert result.error_details["taskId"] == "task-1" + assert result.error_details["httpStatus"] == 404 + assert result.error_details["code"] == 404 + assert result.error_details["details"][0]["metadata"]["taskId"] == "task-1" def test_generic_failure_preserves_actionable_message(): @@ -322,6 +323,9 @@ def get_card(self, name): def create_a2a_client(self, card): return object() + def client_for(self, agent_name): + return self.create_a2a_client(self.get_card(agent_name)) + def validate_content_extensions(self, card, content): return None @@ -537,7 +541,7 @@ def __init__(self): def get_card(self, agent_name): return SimpleNamespace(name=agent_name) - def _get_extensions(self, card): + def get_extension_uris(self, card): del card return {A2ATExtension.AUTHORIZATION_T.uri} @@ -547,6 +551,9 @@ def validate_content_extensions(self, card, content): def create_a2a_client(self, card): return card + def client_for(self, agent_name): + return self.create_a2a_client(self.get_card(agent_name)) + def build_send_request(self, content, context_id): del content self.context_id = context_id diff --git a/test_review_fixes.py b/test_review_fixes.py new file mode 100644 index 0000000..f0e4cd6 --- /dev/null +++ b/test_review_fixes.py @@ -0,0 +1,792 @@ +# Copyright (c) 2026 Huawei Technologies Co., Ltd. +# All Rights Reserved. +# +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Regression tests for the defects found in the SDK review. + +Each test names the defect it locks down so a reviewer can map a failure back +to a specific finding. Rationale and blast radius for every change are recorded +in the accompanying change note. +""" + +import asyncio +import datetime +import json +import os +from pathlib import Path +from types import SimpleNamespace + +import pytest +from loguru import logger +from a2a.client.interceptors import BeforeArgs +from a2a.types import ( + AgentCapabilities, + AgentCard, + AgentInterface, + APIKeySecurityScheme, + SecurityRequirement, + SecurityScheme, + StringList, + TaskState, +) +from a2a.types.a2a_pb2 import StreamResponse +from google.protobuf import json_format +from google.protobuf.json_format import Parse, ParseDict + +from workflow_engine import MessageContent, WorkflowEngineClient +from workflow_engine.client.a2a_transport import A2ATransport +from workflow_engine.client.credential_crypto import decrypt_if_needed, encrypt +from workflow_engine.client.credential_service import CustomAuthInterceptor +from workflow_engine.client.env_file_loader import load_to_environ +from workflow_engine.client.protocol_logger import log_request +from workflow_engine.client.ssl_context import create_ssl_context +from workflow_engine.core.failure_mapping import failure_to_task_result + + +# ---------------------------------------------------------------------- +# Helpers +# ---------------------------------------------------------------------- + +def _card(name: str = "agent") -> AgentCard: + return AgentCard( + name=name, + capabilities=AgentCapabilities(), + supported_interfaces=[ + AgentInterface( + url="https://agent.example/a2a", protocol_binding="HTTP+JSON", + ) + ], + ) + + +def _task_snapshot(task_id: str, text: str, artifact_id: str = "a1") -> StreamResponse: + response = StreamResponse() + response.task.id = task_id + response.task.context_id = "ctx" + response.task.status.state = TaskState.TASK_STATE_WORKING + artifact = response.task.artifacts.add() + artifact.artifact_id = artifact_id + artifact.parts.add().text = text + return response + + +def _message_event(message_id: str, text: str) -> StreamResponse: + response = StreamResponse() + response.message.message_id = message_id + response.message.context_id = "ctx" + response.message.role = 1 # ROLE_USER + response.message.parts.add().text = text + return response + + +def _artifact_update(task_id: str, text: str, append: bool = False) -> StreamResponse: + response = StreamResponse() + response.artifact_update.task_id = task_id + response.artifact_update.context_id = "ctx" + response.artifact_update.artifact.artifact_id = "a1" + response.artifact_update.artifact.parts.add().text = text + response.artifact_update.append = append + return response + + +class _FakeStreamClient: + """Minimal stand-in for the a2a client used by the transport.""" + + def __init__(self, events): + self._events = events + + async def send_message(self, request): + del request + for event in self._events: + yield event + + async def subscribe(self, request): + del request + for event in self._events: + yield event + + +class _NeverEndingClient: + """A stream that stays open forever without ever going idle.""" + + async def send_message(self, request): + del request + while True: + await asyncio.sleep(3600) + yield None + + +def _transport_with(events): + """A transport plus the fake client that consume_stream must be handed.""" + transport = A2ATransport(agent_cards=[_card()]) + return transport, _FakeStreamClient(events) + + +def _transport_subscribing_to(events): + """A transport whose client lookup is redirected to the fake stream.""" + transport = A2ATransport(agent_cards=[_card()]) + client = _FakeStreamClient(events) + transport.client_for = lambda agent_name: client + return transport + + +def _write_self_signed_cert(tmp_path): + from cryptography import x509 + from cryptography.hazmat.primitives import hashes, serialization + from cryptography.hazmat.primitives.asymmetric import rsa + from cryptography.x509.oid import NameOID + + key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + name = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, "sdk-test-client")]) + now = datetime.datetime.now(datetime.timezone.utc) + certificate = ( + x509.CertificateBuilder() + .subject_name(name) + .issuer_name(name) + .public_key(key.public_key()) + .serial_number(x509.random_serial_number()) + .not_valid_before(now - datetime.timedelta(days=1)) + .not_valid_after(now + datetime.timedelta(days=1)) + .sign(key, hashes.SHA256()) + ) + cert_path = tmp_path / "client.pem" + key_path = tmp_path / "client.key" + cert_path.write_bytes(certificate.public_bytes(serialization.Encoding.PEM)) + key_path.write_bytes(key.private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.NoEncryption(), + )) + return str(cert_path), str(key_path) + + +# ---------------------------------------------------------------------- +# Finding 1: the json_format.Parse patch raised TypeError +# ---------------------------------------------------------------------- + +def test_json_format_parse_of_a_stream_response_no_longer_raises(): + """The patch passed ignore_unknown_fields twice, breaking every Parse call.""" + bare_task = json.dumps({ + "id": "t1", "contextId": "c1", + "status": {"state": "TASK_STATE_WORKING"}, + }) + parsed = json_format.Parse(bare_task, StreamResponse()) + assert parsed.HasField("task") + assert parsed.task.id == "t1" + + +def test_bare_task_json_is_normalized_into_a_stream_response(): + """ParseDict is the entry point that must keep normalizing bare task payloads.""" + parsed = json_format.ParseDict( + {"id": "t1", "contextId": "c1", "status": {"state": "TASK_STATE_WORKING"}}, + StreamResponse(), + ) + assert parsed.HasField("task") + assert parsed.task.id == "t1" + + +def test_parse_dict_still_honours_the_callers_unknown_field_policy(): + """Non-stream targets must keep the caller's own argument, not a forced True.""" + assert ParseDict({"name": "agent"}, AgentCard()).name == "agent" + with pytest.raises(json_format.ParseError): + ParseDict({"name": "agent", "unknownField": 1}, AgentCard(), False) + + +def test_parse_is_not_replaced_by_the_normalization_patch(): + """Guard against re-introducing an attribute patch on json_format.Parse.""" + from a2a.client.transports import rest + + assert json_format.Parse is Parse + assert rest.Parse is Parse + + +# ---------------------------------------------------------------------- +# Finding 2: response text was duplicated by full task snapshots +# ---------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_repeated_task_snapshots_do_not_duplicate_response_text(): + transport, client = _transport_with([ + _task_snapshot("t1", "hello"), + _task_snapshot("t1", "hello"), + _task_snapshot("t1", "hello"), + ]) + try: + result = await transport.consume_stream(client, object(), agent_name="agent") + finally: + await transport.close() + assert result.text == "hello" + + +@pytest.mark.asyncio +async def test_artifact_only_stream_still_yields_text(): + """Content delivered solely through artifact_update used to produce ''.""" + transport, client = _transport_with([_artifact_update("t1", "chunk")]) + try: + result = await transport.consume_stream(client, object(), agent_name="agent") + finally: + await transport.close() + assert result.text == "chunk" + + +@pytest.mark.asyncio +async def test_artifact_text_is_concatenated_in_first_seen_order(): + transport, client = _transport_with([ + _artifact_update("t1", "one", append=True), + _artifact_update("t1", "two", append=True), + ]) + try: + result = await transport.consume_stream(client, object(), agent_name="agent") + finally: + await transport.close() + assert result.text == "onetwo" + + +# ---------------------------------------------------------------------- +# Finding 3: subscribe_to_task dropped standalone messages +# ---------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_subscribe_to_task_keeps_messages_seen_before_a_task_snapshot(): + transport = _transport_subscribing_to([ + _message_event("m1", "earlier message"), + _task_snapshot("t1", "hello"), + ]) + events = [] + try: + result = await transport.subscribe_to_task( + "agent", "t1", event_callback=events.append, + ) + finally: + await transport.close() + + # The standalone message must survive the later full task snapshot. + assert len(result.received_messages) == 2 + assert result.text == "hello" + assert [event["type"] for event in events] == ["message", "task"] + + +@pytest.mark.asyncio +async def test_subscribe_to_task_reports_status_and_artifact_updates(): + transport = _transport_subscribing_to([ + _artifact_update("t1", "body"), + _task_snapshot("t1", "body"), + ]) + events = [] + try: + result = await transport.subscribe_to_task( + "agent", "t1", event_callback=events.append, + ) + finally: + await transport.close() + assert [event["type"] for event in events] == ["artifact", "task"] + assert result.text == "body" + + +# ---------------------------------------------------------------------- +# Finding 4: TLS material was silently dropped when verification was off +# ---------------------------------------------------------------------- + +def test_disabling_verification_still_presents_a_client_identity(tmp_path): + cert_path, key_path = _write_self_signed_cert(tmp_path) + context = create_ssl_context( + False, cert_path=cert_path, key_path=key_path, + ) + assert context is not False + assert context.verify_mode.name == "CERT_NONE" + assert context.check_hostname is False + + +def test_trust_store_without_verification_is_rejected_not_ignored(tmp_path): + with pytest.raises(ValueError, match="require verify_server=True"): + create_ssl_context(False, ca_certs_path=str(tmp_path / "ca.pem")) + with pytest.raises(ValueError, match="require verify_server=True"): + create_ssl_context(False, crl_path=str(tmp_path / "crl.pem")) + + +def test_incomplete_client_identity_is_rejected_without_verification(tmp_path): + with pytest.raises(ValueError, match="Both client certificate and private key"): + create_ssl_context(False, cert_path=str(tmp_path / "client.pem")) + with pytest.raises(FileNotFoundError, match="not found"): + create_ssl_context( + False, + cert_path=str(tmp_path / "missing.pem"), + key_path=str(tmp_path / "missing.key"), + ) + + +def test_plain_unverified_mode_still_returns_false(): + """The fast path for callers that genuinely want no TLS setup is unchanged.""" + assert create_ssl_context(False) is False + + +# ---------------------------------------------------------------------- +# Finding 5: a custom auth_header was logged in clear +# ---------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_custom_auth_header_is_registered_so_it_is_redacted(monkeypatch): + from loguru import logger + + class _Credentials: + async def get_credentials(self, scheme_name, context=None): + del scheme_name, context + return "top-secret-value" + + card = AgentCard( + name="agent", + security_schemes={ + "corp": SecurityScheme( + api_key_security_scheme=APIKeySecurityScheme( + location="header", name="X-Ignored", + ) + ), + }, + security_requirements=[SecurityRequirement(schemes={"corp": StringList()})], + ) + interceptor = CustomAuthInterceptor( + _Credentials(), {"corp": {"auth_header": "X-Corp-Cred"}}, + ) + args = BeforeArgs(input=None, method="send_message", agent_card=card) + await interceptor.before(args) + assert args.context.service_parameters["X-Corp-Cred"] == "top-secret-value" + + messages = [] + sink = logger.add(messages.append, format="{message}", level="DEBUG") + try: + monkeypatch.setenv("WORKFLOW_ENGINE_PROTOCOL_LOGGING", "true") + monkeypatch.delenv( + "WORKFLOW_ENGINE_PROTOCOL_INCLUDE_SENSITIVE_HEADERS", raising=False, + ) + log_request( + "agent", "https://example.com", "payload", + dict(args.context.service_parameters), + ) + finally: + logger.remove(sink) + + rendered = "".join(messages) + assert "top-secret-value" not in rendered + assert "***REDACTED***" in rendered + + +# ---------------------------------------------------------------------- +# Finding 6: error mapping missed subclasses +# ---------------------------------------------------------------------- + +def test_failure_mapping_walks_the_mro_for_a2a_error_subclasses(): + from a2a.utils.errors import A2A_ERROR_MAPPING, TaskNotFoundError + + class DerivedTaskNotFound(TaskNotFoundError): + pass + + expected = A2A_ERROR_MAPPING[TaskNotFoundError] + result = failure_to_task_result(DerivedTaskNotFound("missing")) + assert result.success is False + assert result.error_code == f"a2a.{expected.reason.lower()}" + assert result.error_details["reason"] == expected.reason + assert result.error_details["domain"] == "a2a-protocol.org" + + +def test_protocol_logs_redact_credentials_in_request_and_response_bodies(monkeypatch): + from workflow_engine.client.protocol_logger import log_response + + monkeypatch.setenv("WORKFLOW_ENGINE_PROTOCOL_LOGGING", "true") + messages = [] + sink = logger.add(messages.append, format="{message}", level="DEBUG") + try: + log_request( + "agent", "https://example.com", + {"nested": {"access_token": "body-secret", "note": "safe"}}, + ) + log_response( + "agent", "Task", '{"password":"reply-secret","note":"safe"}', + ) + log_response("agent", "Message", "Bearer opaque-token password=form-secret") + finally: + logger.remove(sink) + rendered = "".join(messages) + for secret in ("body-secret", "reply-secret", "opaque-token", "form-secret"): + assert secret not in rendered + assert "safe" in rendered + assert "***" in rendered + + +def test_activated_extension_must_be_declared_by_agent_card(): + content = MessageContent( + parts=MessageContent.text("task").parts, + extensions=frozenset({"urn:example:unknown"}), + ) + with pytest.raises(ValueError, match="not declared"): + A2ATransport.validate_content_extensions(_card(), content) + + +def test_message_content_with_extension_returns_independent_snapshot(): + original = MessageContent.text("task") + activated = original.with_extension("urn:example:task") + assert original.extensions == frozenset() + assert activated.extensions == frozenset({"urn:example:task"}) + assert activated.with_extension("urn:example:task").extensions == activated.extensions + with pytest.raises(ValueError, match="Extension URI"): + original.with_extension(" ") + + +def test_registry_and_psop_http_timeouts_are_configurable(): + from workflow_engine.registry.registry_client import RegistryClient, _http_timeout + + timeout = _http_timeout(2.5, 45.0) + assert timeout.connect == 2.5 + assert timeout.read == 45.0 + assert RegistryClient( + "https://registry.example", connect_timeout_seconds=2.5, + read_timeout_seconds=45.0, + )._timeout == timeout + with pytest.raises(ValueError, match="timeouts must be positive"): + _http_timeout(0, 30) + + +@pytest.mark.parametrize("options", [ + {"crl_path": "unused.crl"}, + {"client_cert_path": "unused.crt", "client_key_path": "unused.key"}, + {"ca_certs_path": "unused-ca.crt"}, +]) +@pytest.mark.asyncio +async def test_explicit_tls_options_reject_unsupported_grpc_transport(options): + import httpx + + card = _card() + card.supported_interfaces[0].protocol_binding = "GRPC" + async with httpx.AsyncClient() as client: + transport = A2ATransport([card], httpx_client=client, **options) + try: + with pytest.raises(ValueError, match="cannot be applied to GRPC"): + transport.create_a2a_client(card) + finally: + await transport.close() + + +@pytest.mark.asyncio +async def test_explicit_preferred_protocol_does_not_silently_fall_back(): + transport = A2ATransport([_card()], preferred_protocol="GRPC") + try: + with pytest.raises(ValueError, match="not declared"): + transport.create_a2a_client(transport.get_card("agent")) + finally: + await transport.close() + + +def test_standard_remote_error_envelope_matches_java_error_contract(): + from workflow_engine.client.remote_error import from_payload + + remote = from_payload({ + "error": { + "code": 403, + "message": "Bearer raw-secret is denied", + "status": "PERMISSION_DENIED", + "details": [{ + "@type": "type.googleapis.com/google.rpc.ErrorInfo", + "reason": "AUTHORIZATION_DENIED", + "domain": "a2a-protocol.org", + "metadata": {"access_token": "nested-secret"}, + }], + }, + }, retry_after="30") + assert remote is not None + result = failure_to_task_result(remote) + assert result.error_code == "a2a.authorization_denied" + assert result.error_details["httpStatus"] == 403 + assert result.error_details["code"] == 403 + assert result.error_details["retryAfter"] == "30" + assert result.error_details["details"][0]["metadata"]["access_token"] == "***" + assert "raw-secret" not in result.error + assert "nested-secret" not in str(result.error_details) + + +def test_remote_error_without_reason_uses_http_status_fallback(): + from workflow_engine.client.remote_error import from_payload + + remote = from_payload('{"error":{"code":429,"message":"slow down"}}') + assert remote is not None + result = failure_to_task_result(remote) + assert result.error_code == "a2a.http.429" + assert result.error_details == {"httpStatus": 429, "code": 429} + + +def test_http_and_sse_wrappers_retain_structured_remote_error(): + import httpx + + from a2a.client.errors import A2AClientError + from workflow_engine.client.remote_error import find_in_exception + + envelope = '{"error":{"code":403,"message":"forbidden"}}' + request = httpx.Request("GET", "https://agent.example/a2a") + response = httpx.Response( + 403, request=request, text=envelope, headers={"Retry-After": "60"}, + ) + http_error = httpx.HTTPStatusError("remote failure", request=request, response=response) + mapped_http = failure_to_task_result(http_error) + assert mapped_http.error_code == "a2a.http.403" + assert mapped_http.error_details["retryAfter"] == "60" + + sse_error = A2AClientError(f"SSE stream error event received: {envelope}") + mapped_sse = find_in_exception(sse_error) + assert mapped_sse is not None + assert failure_to_task_result(sse_error).error_code == "a2a.http.403" + + +def test_sse_message_with_error_envelope_cannot_be_normalized_into_empty_event(): + from workflow_engine.client.remote_error import RemoteA2AError + from workflow_engine.client.sse_normalization import apply_sse_normalization + + apply_sse_normalization() + with pytest.raises(RemoteA2AError) as raised: + ParseDict( + {"error": {"code": 429, "message": "rate limited"}}, + StreamResponse(), + ) + assert raised.value.workflow_code == "a2a.http.429" + + +@pytest.mark.parametrize("bad_input", [ + {"text": "hello", "data": {"x": 1}}, + {"other": "hello"}, + 42, + ["hello"], +]) +def test_workflow_rejects_malformed_subtask_business_input(bad_input): + from workflow_engine import Workflow + + with pytest.raises(ValueError, match="subtask input"): + Workflow.from_dict({ + "steps": [{"name": "one", "subtasks": [{"agent": "agent", "input": bad_input}]}], + }) + + +@pytest.mark.asyncio +async def test_transport_accepts_explicit_credential_key_without_process_environment(monkeypatch): + monkeypatch.delenv("A2AT_CRED_KEY", raising=False) + key = "ab" * 32 + sealed = encrypt("client-secret", key_hex=key) + config = {"agent": {"bearer": {"password": sealed}}} + + with pytest.raises(RuntimeError, match="A2AT_CRED_KEY"): + A2ATransport([_card()], credentials_config=config) + + transport = A2ATransport( + [_card()], credentials_config=config, credential_encryption_key=key, + ) + try: + assert decrypt_if_needed(sealed, key_hex=key) == "client-secret" + assert transport._auth_manager._auth_manager.get_service("agent") is not None + finally: + await transport.close() + + +@pytest.mark.asyncio +async def test_empty_transport_stream_fails_with_stable_error_code(): + from workflow_engine.client.remote_error import EmptyA2AStreamError + + class EmptyClient: + async def send_message(self, request): + if False: + yield request + + transport = A2ATransport([_card()]) + try: + with pytest.raises(EmptyA2AStreamError) as raised: + await transport.consume_stream(EmptyClient(), object(), agent_name="agent") + result = failure_to_task_result(raised.value) + assert result.error_code == "a2a.empty_stream" + assert not result.success + finally: + await transport.close() + + +@pytest.mark.asyncio +async def test_empty_protocol_event_is_not_counted_as_a_valid_stream_event(): + from workflow_engine.client.remote_error import EmptyA2AStreamError + + class EmptyEventClient: + async def send_message(self, request): + del request + yield StreamResponse() + + transport = A2ATransport([_card()]) + try: + with pytest.raises(EmptyA2AStreamError, match="empty event"): + await transport.consume_stream(EmptyEventClient(), object(), agent_name="agent") + finally: + await transport.close() + + +# ---------------------------------------------------------------------- +# Finding 7: .env keys were written into the process environment unchecked +# ---------------------------------------------------------------------- + +def test_env_file_loader_rejects_invalid_key_names(tmp_path, monkeypatch): + monkeypatch.delenv("SDK_REVIEW_GOOD_KEY", raising=False) + monkeypatch.delenv("A2AT_CRED_KEY", raising=False) + env_file = tmp_path / ".env" + env_file.write_text( + "SDK_REVIEW_GOOD_KEY=value\n" + "A2AT_CRED_KEY=deadbeef\n" + "BAD-KEY=value\n" + "1NUMBERED=value\n" + "SPACED KEY=value\n" + "QUOTED='single'\n", + encoding="utf-8", + ) + loaded = load_to_environ(env_file) + + assert os.environ["SDK_REVIEW_GOOD_KEY"] == "value" + # A2AT_CRED_KEY is a valid name and stays loadable: bridging it is the + # documented purpose of this loader. + assert os.environ["A2AT_CRED_KEY"] == "deadbeef" + assert os.environ["QUOTED"] == "single" + assert "BAD-KEY" not in os.environ + assert "1NUMBERED" not in os.environ + assert "SPACED KEY" not in os.environ + assert loaded == 3 + + +# ---------------------------------------------------------------------- +# Finding 8: credential ciphertexts were not bound to their context +# ---------------------------------------------------------------------- + +def test_credential_crypto_binds_a_ciphertext_to_its_aad(monkeypatch): + monkeypatch.setenv("A2AT_CRED_KEY", "ab" * 32) + sealed = encrypt("secret", aad="agent-a/bearer") + assert decrypt_if_needed(sealed, aad="agent-a/bearer") == "secret" + with pytest.raises(RuntimeError, match="decryption failed"): + decrypt_if_needed(sealed, aad="agent-b/bearer") + + +def test_credential_crypto_without_aad_stays_wire_compatible(monkeypatch): + """The default must keep the format the Java SDK produces and consumes.""" + monkeypatch.setenv("A2AT_CRED_KEY", "ab" * 32) + sealed = encrypt("secret") + assert sealed.startswith("enc:") + assert len(sealed.split(":")) == 3 + assert decrypt_if_needed(sealed) == "secret" + + +# ---------------------------------------------------------------------- +# Finding 9: an A2A client was rebuilt for every operation +# ---------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_transport_caches_one_client_per_agent(): + transport = A2ATransport(agent_cards=[_card()]) + created = [] + + def _create(card): + created.append(card.name) + return SimpleNamespace(name=card.name) + + transport.create_a2a_client = _create + try: + first = transport.client_for("agent") + second = transport.client_for("agent") + assert first is second + assert created == ["agent"] + + # A card update invalidates the cache: the client captured old bindings. + transport.update_agent_cards([_card()]) + third = transport.client_for("agent") + assert third is not first + assert created == ["agent", "agent"] + finally: + await transport.close() + + +@pytest.mark.asyncio +async def test_transport_client_lookup_rejects_an_unknown_agent(): + transport = A2ATransport(agent_cards=[_card()]) + try: + with pytest.raises(RuntimeError, match="Agent not found"): + transport.client_for("nobody") + finally: + await transport.close() + + +# ---------------------------------------------------------------------- +# Finding 10: stream_message had no overall deadline +# ---------------------------------------------------------------------- + +@pytest.mark.asyncio +async def test_stream_message_bounds_the_whole_stream(): + transport = A2ATransport(agent_cards=[_card()]) + transport.client_for = lambda agent_name: _NeverEndingClient() + client = WorkflowEngineClient(transport) + try: + with pytest.raises(TimeoutError): + async for _ in client.stream_message( + "agent", MessageContent.text("hi"), timeout_seconds=0.05, + ): + pass + finally: + await transport.close() + + +@pytest.mark.asyncio +async def test_stream_message_rejects_a_non_positive_timeout(): + transport = A2ATransport(agent_cards=[_card()]) + transport.client_for = lambda agent_name: _FakeStreamClient([]) + client = WorkflowEngineClient(transport) + try: + with pytest.raises(ValueError, match="must be positive"): + async for _ in client.stream_message( + "agent", MessageContent.text("hi"), timeout_seconds=0, + ): + pass + finally: + await transport.close() + + +# ---------------------------------------------------------------------- +# Finding 11: three disagreeing version sources +# ---------------------------------------------------------------------- + +def test_distribution_version_has_a_single_source(): + import tomllib + + pyproject = Path(__file__).resolve().parent / "pyproject.toml" + data = tomllib.loads(pyproject.read_text(encoding="utf-8")) + project = data["project"] + + assert "version" not in project, "pyproject must not restate the version" + assert project["dynamic"] == ["version"] + assert ( + data["tool"]["setuptools"]["dynamic"]["version"]["attr"] + == "workflow_engine.__version__" + ) + + import workflow_engine + assert workflow_engine.__version__ == "0.1.0" + + +def test_declared_a2a_t_sdk_lower_bound_matches_the_verifier(): + """verify_sdk.py asserts >=1.1.0, so pyproject must not advertise less.""" + import tomllib + + root = Path(__file__).resolve().parent + data = tomllib.loads((root / "pyproject.toml").read_text(encoding="utf-8")) + declared = [ + dep for dep in data["project"]["dependencies"] + if dep.startswith("a2a-t-sdk") + ] + assert declared == ["a2a-t-sdk>=1.1.0,<2"] + assert "1.1.0" in (root / "verify_sdk.py").read_text(encoding="utf-8") diff --git a/workflow_engine/__init__.py b/workflow_engine/__init__.py index 5a3eba9..63b259c 100644 --- a/workflow_engine/__init__.py +++ b/workflow_engine/__init__.py @@ -1,4 +1,4 @@ -# Copyright (c) 2026 Huawei Technologies Co., Ltd. +# Copyright (c) 2026 Huawei Technologies Co., Ltd. # All Rights Reserved. # # SPDX-License-Identifier: Apache-2.0 @@ -99,4 +99,4 @@ async def on_route(self, request): "execute_psop", ] -__version__ = "0.0.3" +__version__ = "0.1.0" diff --git a/workflow_engine/client/__init__.py b/workflow_engine/client/__init__.py index ebac7ce..39df234 100644 --- a/workflow_engine/client/__init__.py +++ b/workflow_engine/client/__init__.py @@ -1,4 +1,4 @@ -# Copyright (c) 2026 Huawei Technologies Co., Ltd. +# Copyright (c) 2026 Huawei Technologies Co., Ltd. # All Rights Reserved. # # SPDX-License-Identifier: Apache-2.0 @@ -33,7 +33,9 @@ from workflow_engine.client.extension_interceptor import ExtensionInterceptor from workflow_engine.client.agentcard_normalizer import normalize_agent_dict -from workflow_engine.client.protocol_logger import log_request, log_response +from workflow_engine.client.protocol_logger import ( + log_request, log_response, register_sensitive_header, +) from workflow_engine.client.stub_engine_client import StubWorkflowEngineClient __all__ = [ @@ -43,5 +45,5 @@ "CustomAuthInterceptor", "ExtensionInterceptor", "normalize_agent_dict", "A2ATExtension", "A2atMessages", "encrypt", "decrypt_if_needed", "load_to_environ", "AuthProvider", "AuthProviderInterceptor", - "log_request", "log_response", "StubWorkflowEngineClient", + "log_request", "log_response", "register_sensitive_header", "StubWorkflowEngineClient", ] diff --git a/workflow_engine/client/a2a_transport.py b/workflow_engine/client/a2a_transport.py index f9c7e4d..4c881a2 100644 --- a/workflow_engine/client/a2a_transport.py +++ b/workflow_engine/client/a2a_transport.py @@ -1,4 +1,4 @@ -# Copyright (c) 2026 Huawei Technologies Co., Ltd. +# Copyright (c) 2026 Huawei Technologies Co., Ltd. # All Rights Reserved. # # SPDX-License-Identifier: Apache-2.0 @@ -11,8 +11,8 @@ * ``WorkflowEngineClient`` (engine_client.py) -- workflow task dispatch, task waiting, Negotiation-T lifecycle, event callback, and control point. -* ``ExtensionSender`` (extension_sender.py) -- one-shot pre-positioning - sends: Authorization-T / Notification-T. +* ``ExtensionSender`` (extension_sender.py) -- one-shot Authorization-T sends + and long-lived Notification-T subscriptions. Neither facade duplicates transport code; both delegate here. """ @@ -44,6 +44,7 @@ from workflow_engine.client.protocol_interceptor import ProtocolLoggingInterceptor from workflow_engine.client.sse_normalization import apply_sse_normalization from workflow_engine.client.agentcard_normalizer import normalize_agent_dict +from workflow_engine.client.transport_activity import install_activity_hook from workflow_engine.control.control_points import EventType from workflow_engine.core.models import ( A2AStreamEvent, MessageContent, ReceivedArtifact, ReceivedMessage, @@ -78,27 +79,54 @@ def __init__( auth_provider: Optional[AuthProvider] = None, preferred_protocol: Optional[str] = None, send_timeout_seconds: int = 600, + task_poll_interval_seconds: float = 20.0, + notification_ack_timeout_seconds: float = 300.0, + *, + credential_encryption_key: Optional[str] = None, ): if send_timeout_seconds <= 0: raise ValueError("send_timeout_seconds must be positive") + if task_poll_interval_seconds < 0.1: + # Mirrors the Java engine's 100ms floor for taskPollIntervalMillis. + raise ValueError("task_poll_interval_seconds must be at least 0.1") + if notification_ack_timeout_seconds <= 0: + raise ValueError("notification_ack_timeout_seconds must be positive") + self._task_poll_interval_seconds = float(task_poll_interval_seconds) + self._notification_ack_timeout_seconds = float( + notification_ack_timeout_seconds + ) normalized_cards = self._normalize_cards(agent_cards) self._card_map = self._build_card_map(normalized_cards) self._send_timeout_seconds = send_timeout_seconds self._owns_httpx_client = httpx_client is None self._closed = False + self._a2a_clients: Dict[str, Any] = {} self._httpx_client = httpx_client or self._create_httpx_client( ssl_verify, ca_certs_path, client_cert_path, client_key_path, client_key_password, crl_path, ) - self._auth_manager = AuthManager(normalized_cards, credentials_config) + # Observe body-chunk arrival so a long-lived notification stream can tell + # "quiet but alive" (SSE heartbeats) apart from "dead". The hook is a + # no-op unless a listener is bound for the call, so it is safe to install + # on a caller-provided client as well. + install_activity_hook(self._httpx_client) + self._auth_manager = AuthManager( + normalized_cards, credentials_config, + credential_encryption_key=credential_encryption_key, + ) self._auth_manager.set_httpx_client(self._httpx_client) self._context_id = str(uuid.uuid4()) self._auth_provider = auth_provider self._preferred_protocol = preferred_protocol + self._explicit_tls_options = bool( + ca_certs_path or client_cert_path or client_key_path + or client_key_password or crl_path + ) logger.info( f"[Transport] Initialized with {len(self._card_map)} agent(s), " f"ssl_verify={ssl_verify}, " - f"send_timeout={send_timeout_seconds}s" + f"send_timeout={send_timeout_seconds}s, " + f"task_poll_interval={self._task_poll_interval_seconds}s" ) # ------------------------------------------------------------------ @@ -195,6 +223,24 @@ def httpx_client(self) -> httpx.AsyncClient: def send_timeout_seconds(self) -> int: return self._send_timeout_seconds + @property + def task_poll_interval_seconds(self) -> float: + """Fallback poll interval used only after a response stream is interrupted. + + While the SSE stream is alive this interval is unused; it exists so a task + that is still running on the remote side is not abandoned (or cancelled) + merely because an intermediary dropped the connection. + """ + return self._task_poll_interval_seconds + + @property + def notification_ack_timeout_seconds(self) -> float: + """How long to wait for the first Notification-T acknowledgement. + + Governs only the initial ACK; it does not bound SSE idleness afterwards. + """ + return self._notification_ack_timeout_seconds + def get_card(self, agent_name: str): return self._card_map.get(agent_name) @@ -202,13 +248,20 @@ def update_agent_cards(self, agent_cards: List[Any]): normalized_cards = self._normalize_cards(agent_cards) self._card_map = self._build_card_map(normalized_cards) self._auth_manager.update_agent_cards(normalized_cards) + # Cached clients captured the previous cards, protocol bindings and + # interceptor set, so they must not outlive the update. + self._a2a_clients.clear() @staticmethod def validate_content_extensions(agent_card, content: MessageContent) -> None: - """Enforce AgentCard extensions marked as required for this request.""" + """Reject missing required and undeclared activated extensions.""" declared = getattr( getattr(agent_card, "capabilities", None), "extensions", () ) or () + declared_uris = {extension.uri for extension in declared} + undeclared = set(content.extensions) - declared_uris + if undeclared: + raise ValueError(f"Extension not declared by AgentCard: {sorted(undeclared)[0]}") missing = [ extension.uri for extension in declared if getattr(extension, "required", False) @@ -247,14 +300,28 @@ def create_a2a_client(self, agent_card): if matched: interfaces = matched else: - logger.warning( - f"[Transport] Preferred protocol {self._preferred_protocol} " - f"not in supportedInterfaces for {agent_card.name}, using first available" + raise ValueError( + f"Preferred protocol {self._preferred_protocol} is not declared " + f"by AgentCard for {agent_card.name}" + ) + elif self._preferred_protocol and not interfaces: + if self._preferred_protocol.upper() not in {"HTTP+JSON", "JSONRPC"}: + raise ValueError( + f"Preferred protocol {self._preferred_protocol} is not declared " + f"by AgentCard for {agent_card.name}" ) protocol_bindings = ( [iface.protocol_binding for iface in interfaces] or ["HTTP+JSON", "JSONRPC"] ) + if self._explicit_tls_options and protocol_bindings[0].upper() not in { + "HTTP+JSON", "JSONRPC", + }: + raise ValueError( + f"TLS/mTLS/CRL options cannot be applied to {protocol_bindings[0]} " + f"for {agent_card.name}; select an HTTP protocol or configure " + "that protocol's TLS transport explicitly" + ) streaming = ( agent_card.capabilities.streaming if agent_card.capabilities else False ) @@ -315,13 +382,20 @@ async def consume_stream( agent_name: str = "", ): """Reduce a response stream to its final task and structured evidence.""" - response_text = None + # A ``task`` stream event carries a FULL task snapshot, so its artifact + # text must replace -- never accumulate -- or every update would append + # the same artifact text again. Standalone ``message`` events are + # genuinely incremental, so their text accumulates separately. The final + # text is assembled from the merged artifact map, which also covers + # streams that deliver content only through ``artifact_update`` events. + message_text = None last_task_result = None last_metadata_dict: Dict[str, Any] = {} task_state = "" standalone_messages: Dict[str, ReceivedMessage] = {} task_artifacts: Dict[str, ReceivedArtifact] = {} task_status_message = None + received_any_event = False async for response in client.send_message(send_req): self.log_response_event(agent_name, response) @@ -329,13 +403,17 @@ async def consume_stream( has_message = response.HasField("message") has_status = response.HasField("status_update") has_artifact = response.HasField("artifact_update") + if not (has_task or has_message or has_status or has_artifact): + from workflow_engine.client.remote_error import EmptyA2AStreamError + + raise EmptyA2AStreamError("A2A response stream contained an empty event") + received_any_event = True if has_task: task = response.task state = self._extract_task_state(task) logger.info(f"[Transport] Received StreamResponse with task: state={state or None}") last_task_result = task - response_text = self._extract_task_text(task, response_text) task_state = state or task_state last_metadata_dict = self._merge_task_metadata(task, last_metadata_dict) task_status_message = self._message_content(task.status.message) @@ -350,9 +428,15 @@ async def consume_stream( ) on_intermediate(EventType.AGENT_STATUS_UPDATE, { "agent": agent_name, + # Carried so a caller can remember the remote task identity + # before the stream ends: if an intermediary drops the + # connection mid-flight, the task can still be polled + # instead of being abandoned or cancelled. + "task_id": getattr(task, "id", "") or "", "state": task_state, "is_final": is_final, - "text": response_text or "", + "text": self._text_from_artifacts(task_artifacts) + or message_text or "", "metadata": dict(last_metadata_dict) if last_metadata_dict else {}, }) # Emit artifact update events for each artifact in the task @@ -384,7 +468,7 @@ async def consume_stream( logger.info("[Transport] Received StreamResponse with message") msg = response.message msg_text = self._extract_message_text(msg, None) - response_text = self._extract_message_text(msg, response_text) + message_text = self._extract_message_text(msg, message_text) standalone_messages[msg.message_id] = ReceivedMessage( message=self._message_content(msg) ) @@ -450,6 +534,11 @@ async def consume_stream( "metadata": dict(artifact.metadata), }) + if not received_any_event: + from workflow_engine.client.remote_error import EmptyA2AStreamError + + raise EmptyA2AStreamError("A2A response stream ended without an event") + received = list(standalone_messages.values()) if last_task_result is not None or task_artifacts or task_status_message is not None: received.append(ReceivedMessage( @@ -462,7 +551,7 @@ async def consume_stream( task_state, task_status_message, ) return SendMessageResult( - text=response_text or "", + text=self._text_from_artifacts(task_artifacts) or message_text or "", task=last_task_result, metadata=last_metadata_dict, task_state=task_state, @@ -621,6 +710,21 @@ def _extract_task_text(task, current_text: Optional[str]) -> Optional[str]: current_text = (current_text or "") + part.text return current_text + @staticmethod + def _text_from_artifacts(task_artifacts) -> Optional[str]: + """Concatenate the text of a merged artifact map in first-seen order. + + Reading the merged map instead of one event's payload keeps snapshot and + ``artifact_update`` streams on the same code path, so an agent that only + streams artifacts still produces response text. + """ + text: Optional[str] = None + for artifact in task_artifacts.values(): + for part in artifact.parts or (): + if getattr(part, "text", ""): + text = (text or "") + part.text + return text + @staticmethod def _extract_task_state(task) -> str: if not (task.status and task.status.state): @@ -663,15 +767,6 @@ def _extract_task_metadata(task) -> Dict[str, Any]: return md return MessageToDict(md, preserving_proto_field_name=True) - @staticmethod - def _text_from_metadata(metadata: Dict[str, Any]) -> Optional[str]: - if not isinstance(metadata, dict): - return None - for val in metadata.values(): - if isinstance(val, str) and len(val) > 20: - return val - return None - @staticmethod def _extract_message_text(message, current_text: Optional[str]) -> Optional[str]: if not message.parts: @@ -682,7 +777,12 @@ def _extract_message_text(message, current_text: Optional[str]) -> Optional[str] return current_text @staticmethod - def _get_extensions(agent_card) -> List[str]: + def get_extension_uris(agent_card) -> List[str]: + """Return the extension URIs an AgentCard advertises. + + Public because the extension facades must validate against exactly the + same set the transport enforces for required extensions. + """ uris = [] exts = getattr( getattr(agent_card, "capabilities", None), "extensions", None @@ -693,11 +793,28 @@ def _get_extensions(agent_card) -> List[str]: uris.append(uri) return uris - def _client_for(self, agent_name: str): + # Backwards-compatible alias; prefer the public get_extension_uris(). + _get_extensions = get_extension_uris + + def client_for(self, agent_name: str): + """Return the A2A client for an agent, building it once and caching it. + + Client construction resolves protocol bindings, the interceptor chain and + authentication state, so rebuilding it for every send, task poll or + cancellation repeated that work on each call. The cache is dropped by + :meth:`update_agent_cards`. + """ card = self.get_card(agent_name) if card is None: raise RuntimeError(f"Agent not found: {agent_name}") - return self.create_a2a_client(card) + client = self._a2a_clients.get(agent_name) + if client is None: + client = self.create_a2a_client(card) + self._a2a_clients[agent_name] = client + return client + + def _client_for(self, agent_name: str): + return self.client_for(agent_name) @classmethod def _result_from_task(cls, task) -> SendMessageResult: @@ -743,32 +860,87 @@ async def subscribe_to_task( event_callback: Optional[Callable[[Dict[str, Any]], None]] = None, ) -> SendMessageResult: client = self._client_for(agent_name) - response_text = None task = None metadata: Dict[str, Any] = {} state = "" - received: list[ReceivedMessage] = [] + message_text = None + standalone: list[ReceivedMessage] = [] + task_artifacts: Dict[str, ReceivedArtifact] = {} + task_status_message = None async for response in client.subscribe(SubscribeToTaskRequest(id=task_id)): self.log_response_event(agent_name, response) if response.HasField("task"): + # Full task snapshot: rebuild rather than append, exactly as + # consume_stream does. The previous implementation replaced the + # whole `received` list here, silently dropping every standalone + # message observed before this event. task = response.task state = self._extract_task_state(task) - response_text = self._extract_task_text(task, response_text) metadata = self._merge_task_metadata(task, metadata) - received = [self._result_from_task(task).received_messages[0]] + task_status_message = self._message_content(task.status.message) + task_artifacts = { + artifact.artifact_id: self._received_artifact(artifact) + for artifact in task.artifacts + } event = {"agent": agent_name, "type": "task", "state": state} elif response.HasField("message"): message = response.message - response_text = self._extract_message_text(message, response_text) - received.append(ReceivedMessage(message=self._message_content(message))) - event = {"agent": agent_name, "type": "message", "text": response_text or ""} + message_text = self._extract_message_text(message, message_text) + standalone.append(ReceivedMessage(message=self._message_content(message))) + event = { + "agent": agent_name, "type": "message", + "text": self._extract_message_text(message, None) or "", + } + elif response.HasField("status_update"): + update = response.status_update + state = ( + TaskState.Name(update.status.state) + if update.status.state else state + ) + task_status_message = self._message_content(update.status.message) + event = {"agent": agent_name, "type": "status", "state": state} + elif response.HasField("artifact_update"): + update = response.artifact_update + artifact = self._received_artifact(update.artifact) + previous = task_artifacts.get(artifact.artifact_id) + if getattr(update, "append", False) and previous is not None: + artifact = ReceivedArtifact( + artifact_id=artifact.artifact_id, + name=artifact.name or previous.name, + description=artifact.description or previous.description, + parts=previous.parts + artifact.parts, + metadata={**dict(previous.metadata), **dict(artifact.metadata)}, + extensions=artifact.extensions or previous.extensions, + ) + task_artifacts[artifact.artifact_id] = artifact + event = { + "agent": agent_name, "type": "artifact", + "artifact_id": artifact.artifact_id, + } else: event = {"agent": agent_name, "type": "update"} if event_callback is not None: event_callback(event) + + received = list(standalone) + if task is not None or task_artifacts or task_status_message is not None: + received.append(ReceivedMessage( + message=task_status_message, + task_metadata=self._extract_task_metadata(task) + if task is not None else {}, + artifacts=tuple(task_artifacts.values()), + )) + failure_code, failure_message = self._failure_from_state( + state, task_status_message, + ) return SendMessageResult( - text=response_text or "", task=task, metadata=metadata, - task_state=state, received_messages=tuple(received), + text=self._text_from_artifacts(task_artifacts) or message_text or "", + task=task, + metadata=metadata, + task_state=state, + failure_code=failure_code, + failure_message=failure_message, + received_messages=tuple(received), ) # ------------------------------------------------------------------ diff --git a/workflow_engine/client/auth_manager.py b/workflow_engine/client/auth_manager.py index df2cc13..6f49fa0 100644 --- a/workflow_engine/client/auth_manager.py +++ b/workflow_engine/client/auth_manager.py @@ -1,4 +1,4 @@ -# Copyright (c) 2026 Huawei Technologies Co., Ltd. +# Copyright (c) 2026 Huawei Technologies Co., Ltd. # All Rights Reserved. # # SPDX-License-Identifier: Apache-2.0 @@ -33,12 +33,14 @@ from workflow_engine.client.credential_service import AgentAuthManager, CustomAuthInterceptor from workflow_engine.client.extension_interceptor import ExtensionInterceptor from workflow_engine.client.auth_provider import AuthProvider +from workflow_engine.client.protocol_logger import register_sensitive_header class AuthManager: """Builds auth/extension interceptors from AgentCard securitySchemes.""" - def __init__(self, agent_cards: List[Any], credentials_config: Optional[str | Dict] = None): + def __init__(self, agent_cards: List[Any], credentials_config: Optional[str | Dict] = None, + credential_encryption_key: Optional[str] = None): self._interceptors: Dict[str, List[Any]] = {} self._auth_manager: Optional[AgentAuthManager] = None @@ -49,13 +51,21 @@ def __init__(self, agent_cards: List[Any], credentials_config: Optional[str | Di if credentials_config is not None: if isinstance(credentials_config, str): - self._auth_manager = AgentAuthManager(config_path=credentials_config) + self._auth_manager = AgentAuthManager( + config_path=credentials_config, + credential_encryption_key=credential_encryption_key, + ) elif isinstance(credentials_config, dict): - self._auth_manager = AgentAuthManager(config=credentials_config) + self._auth_manager = AgentAuthManager( + config=credentials_config, + credential_encryption_key=credential_encryption_key, + ) else: raise TypeError("credentials_config must be a file path or mapping") else: - self._auth_manager = AgentAuthManager() + self._auth_manager = AgentAuthManager( + credential_encryption_key=credential_encryption_key, + ) self._build_interceptors(agent_cards) @@ -129,6 +139,8 @@ async def before(self, args: "BeforeArgs") -> None: raise RuntimeError( f"Authentication header conflict for agent {self._agent_name}: {name}" ) + # Everything an AuthProvider contributes is authentication material. + register_sensitive_header(name) args.context.service_parameters[name] = value if headers: logger.info(f"[AuthProvider] Injected {len(headers)} header(s) for {self._agent_name}") diff --git a/workflow_engine/client/credential_crypto.py b/workflow_engine/client/credential_crypto.py index 24dfffe..5a6e942 100644 --- a/workflow_engine/client/credential_crypto.py +++ b/workflow_engine/client/credential_crypto.py @@ -18,14 +18,22 @@ """AES-GCM credential encryption/decryption utility. Supports encrypted values in credential config files using the ``enc:`` -prefix. The encryption key is read from the ``A2AT_CRED_KEY`` environment -variable (32-byte hex string). Mirrors the Java SDK's ``CredentialCrypto``. +prefix. The encryption key is host-injected when provided, otherwise read +from ``A2AT_CRED_KEY`` (32-byte hex string). Mirrors the Java SDK's +``CredentialCrypto``. Usage in credentials JSON:: {"value": "enc::"} Plaintext values (no ``enc:`` prefix) are returned as-is for backward compat. + +Both entry points accept an optional ``aad`` (additional authenticated data) +that binds a ciphertext to its context -- pass the agent and scheme it belongs +to so a ciphertext cannot be replayed under a different identity. It defaults +to ``None`` because the wire format is shared with the Java SDK's +``CredentialCrypto``: enabling it on one side only would make every existing +ciphertext undecryptable, so binding must be a coordinated cross-SDK change. """ import os @@ -39,22 +47,27 @@ _TAG_LENGTH = 16 # 128-bit auth tag -def _resolve_key() -> Optional[str]: - """Resolve the encryption key from OS environment.""" +def _resolve_key(explicit_key: Optional[str] = None) -> Optional[str]: + """Prefer the host-provided key; retain the environment as a fallback.""" + if explicit_key is not None: + return explicit_key key = os.environ.get(_ENV_KEY) if key: return key return None -def decrypt_if_needed(value: Optional[str]) -> Optional[str]: +def decrypt_if_needed( + value: Optional[str], aad: Optional[str] = None, *, key_hex: Optional[str] = None, +) -> Optional[str]: """Decrypt a credential value if it has the ``enc:`` prefix. Values without the prefix are returned as-is (plaintext fallback). + ``aad`` must match the value used at encryption time. """ if not value or not value.startswith(_PREFIX): return value - key_hex = _resolve_key() + key_hex = _resolve_key(key_hex) if not key_hex or not key_hex.strip(): raise RuntimeError( f"Encrypted credential found but {_ENV_KEY} is not configured" @@ -72,7 +85,9 @@ def decrypt_if_needed(value: Optional[str]) -> Optional[str]: ciphertext = ciphertext_and_tag[:-_TAG_LENGTH] tag = ciphertext_and_tag[-_TAG_LENGTH:] from cryptography.hazmat.primitives.ciphers.aead import AESGCM - plaintext = AESGCM(key_bytes).decrypt(iv, ciphertext + tag, None) + plaintext = AESGCM(key_bytes).decrypt( + iv, ciphertext + tag, aad.encode("utf-8") if aad else None + ) return plaintext.decode("utf-8") except Exception as e: if isinstance(e, (RuntimeError, ValueError)): @@ -80,19 +95,23 @@ def decrypt_if_needed(value: Optional[str]) -> Optional[str]: raise RuntimeError("Credential decryption failed") from e -def encrypt(plaintext: str) -> str: - """Encrypt a plaintext value using AES-GCM with the key from A2AT_CRED_KEY. +def encrypt( + plaintext: str, aad: Optional[str] = None, *, key_hex: Optional[str] = None, +) -> str: + """Encrypt a plaintext value using AES-GCM with an explicit or environment key. Returns encrypted string in format ``enc::``. Raises RuntimeError if the key env var is not set. """ - key_hex = _resolve_key() + key_hex = _resolve_key(key_hex) if not key_hex or not key_hex.strip(): raise RuntimeError(f"{_ENV_KEY} environment variable not set") key_bytes = _decode_key(key_hex) iv = secrets.token_bytes(_IV_LENGTH) from cryptography.hazmat.primitives.ciphers.aead import AESGCM - ciphertext_and_tag = AESGCM(key_bytes).encrypt(iv, plaintext.encode("utf-8"), None) + ciphertext_and_tag = AESGCM(key_bytes).encrypt( + iv, plaintext.encode("utf-8"), aad.encode("utf-8") if aad else None + ) return ( _PREFIX + base64.b64encode(iv).decode("ascii") diff --git a/workflow_engine/client/credential_service.py b/workflow_engine/client/credential_service.py index e3d1cbd..6842479 100644 --- a/workflow_engine/client/credential_service.py +++ b/workflow_engine/client/credential_service.py @@ -1,4 +1,4 @@ -# Copyright (c) 2026 Huawei Technologies Co., Ltd. +# Copyright (c) 2026 Huawei Technologies Co., Ltd. # All Rights Reserved. # # SPDX-License-Identifier: Apache-2.0 @@ -31,6 +31,7 @@ from loguru import logger from workflow_engine.client.credential_crypto import decrypt_if_needed +from workflow_engine.client.protocol_logger import register_sensitive_header def _safe_url(value: str) -> str: @@ -58,10 +59,12 @@ class AgentCredentialService(CredentialService if _A2A_AVAILABLE else object): """Obtains tokens via login endpoint, caches with TTL.""" def __init__(self, agent_name: str, scheme_configs: Dict[str, dict], - httpx_client: Optional[httpx.AsyncClient] = None): + httpx_client: Optional[httpx.AsyncClient] = None, + credential_encryption_key: Optional[str] = None): self._agent_name = agent_name self._schemes = scheme_configs self._httpx_client = httpx_client + self._credential_encryption_key = credential_encryption_key self._tokens: Dict[str, tuple] = {} self._lock = None @@ -106,11 +109,13 @@ async def _login(self, scheme_cfg: dict) -> Optional[str]: token_field = scheme_cfg.get("token_field", "accessSession") request_fields = scheme_cfg.get("request_fields") if request_fields and isinstance(request_fields, dict): - body = {k: decrypt_if_needed(v) if isinstance(v, str) else v + body = {k: decrypt_if_needed(v, key_hex=self._credential_encryption_key) if isinstance(v, str) else v for k, v in request_fields.items()} else: username = scheme_cfg.get("username") - password = decrypt_if_needed(scheme_cfg.get("password")) + password = decrypt_if_needed( + scheme_cfg.get("password"), key_hex=self._credential_encryption_key, + ) if not username or not password: raise ValueError( f"Authentication username and password are required for {self._agent_name}" @@ -177,10 +182,13 @@ def _extract_nested_value(data: dict, path: str) -> Optional[str]: class AgentAuthManager: """Loads agent credentials from config, creates per-agent CredentialService.""" - def __init__(self, config: Optional[Dict[str, dict]] = None, config_path: Optional[str] = None): + def __init__(self, config: Optional[Dict[str, dict]] = None, + config_path: Optional[str] = None, + credential_encryption_key: Optional[str] = None): self._config: Dict[str, dict] = {} self._services: Dict[str, AgentCredentialService] = {} self._httpx_client: Optional[httpx.AsyncClient] = None + self._credential_encryption_key = credential_encryption_key if config is not None: self._config = self._resolve_config(copy.deepcopy(config)) elif config_path: @@ -247,13 +255,12 @@ def _merge_mapping(cls, target: dict, overrides: dict) -> None: else: target[key] = copy.deepcopy(value) - @classmethod - def _validate_encrypted_credentials(cls, value) -> None: + def _validate_encrypted_credentials(self, value) -> None: if isinstance(value, dict): for nested in value.values(): - cls._validate_encrypted_credentials(nested) + self._validate_encrypted_credentials(nested) elif isinstance(value, str) and value.startswith("enc:"): - decrypt_if_needed(value) + decrypt_if_needed(value, key_hex=self._credential_encryption_key) def get_service(self, agent_name: str) -> Optional[AgentCredentialService]: if agent_name in self._services: @@ -262,7 +269,8 @@ def get_service(self, agent_name: str) -> Optional[AgentCredentialService]: if not agent_creds: return None service = AgentCredentialService( - agent_name, agent_creds, httpx_client=self._httpx_client + agent_name, agent_creds, httpx_client=self._httpx_client, + credential_encryption_key=self._credential_encryption_key, ) self._services[agent_name] = service logger.info(f"[Auth] Created credential service for agent: {agent_name}") @@ -323,6 +331,9 @@ async def before(self, args: BeforeArgs) -> None: if name in candidate and candidate[name] != value: failure = f"conflicting values for header {name}" break + # `name` is configurable, so the logger cannot recognize it by + # pattern; declare it explicitly before it can reach a log line. + register_sensitive_header(name) candidate[name] = value accept_header = scheme_cfg.get("accept_header") if accept_header: diff --git a/workflow_engine/client/engine_client.py b/workflow_engine/client/engine_client.py index 8dcc81e..8ab4e6c 100644 --- a/workflow_engine/client/engine_client.py +++ b/workflow_engine/client/engine_client.py @@ -86,7 +86,9 @@ def _emit(self, event_type: str, data: Dict[str, Any]) -> None: try: self._event_callback.on_event(event_type, data) except Exception as exc: - logger.warning(f"Event callback failed for {event_type}: {exc}") + from workflow_engine.client.sensitive_data import redact + + logger.warning(f"Event callback failed for {event_type}: {redact(str(exc))}") def _forward_intermediate_event(self, event_type: str, data: Dict[str, Any]) -> None: self._emit(event_type, data) @@ -99,6 +101,11 @@ def agent_names(self) -> List[str]: def httpx_client(self) -> httpx.AsyncClient: return self._transport.httpx_client + @property + def task_poll_interval_seconds(self) -> float: + """Fallback poll interval used only after a response stream is interrupted.""" + return self._transport.task_poll_interval_seconds + def get_card(self, agent_name: str): return self._transport.get_card(agent_name) @@ -119,21 +126,42 @@ async def stream_message( content: MessageContent, context_id: Optional[str] = None, task_id: Optional[str] = None, + timeout_seconds: Optional[float] = None, ) -> AsyncIterator[A2AStreamEvent]: - """Stream normalized A2A events without exposing transport internals.""" + """Stream normalized A2A events without exposing transport internals. + + ``timeout_seconds`` bounds the WHOLE stream rather than a single read: + the transport's read timeout only fires on an idle socket, so a server + trickling a byte inside that window could otherwise hold the stream open + forever. It defaults to the same interaction deadline ``dispatch`` + already enforces, so both public paths expire together; pass a larger + value for a stream that legitimately runs longer. + """ if self._closed: raise RuntimeError("Workflow client closed") card = self._transport.get_card(agent_name) if card is None: raise RuntimeError(f"Agent not found: {agent_name}") self._transport.validate_content_extensions(card, content) - client = self._transport.create_a2a_client(card) + client = self._transport.client_for(agent_name) request = self._transport.build_send_request( content, context_id or str(uuid.uuid4()), task_id, ) - async for response in client.send_message(request): - self._transport.log_response_event(agent_name, response) - yield self._transport.parse_stream_event(response) + deadline = ( + self.callback_timeout_seconds if timeout_seconds is None else timeout_seconds + ) + if deadline <= 0: + raise ValueError("timeout_seconds must be positive") + async with asyncio.timeout(deadline): + received_any_event = False + async for response in client.send_message(request): + received_any_event = True + self._transport.log_response_event(agent_name, response) + yield self._transport.parse_stream_event(response) + if not received_any_event: + from workflow_engine.client.remote_error import EmptyA2AStreamError + + raise EmptyA2AStreamError("A2A response stream ended without an event") @staticmethod def _default_input(): @@ -160,7 +188,10 @@ async def dispatch( ) raise except Exception as exc: - await self._cancel_abandoned_interaction(request.agent_name, interaction) + from workflow_engine.client.remote_error import EmptyA2AStreamError, find_in_exception + + if not isinstance(exc, EmptyA2AStreamError) and find_in_exception(exc) is None: + await self._cancel_abandoned_interaction(request.agent_name, interaction) if interaction["negotiation_started"]: self._emit(EventType.NEGOTIATION_FAILED, { "agent": request.agent_name, @@ -194,9 +225,9 @@ async def _dispatch( remaining = deadline - time.monotonic() if remaining <= 0: raise TimeoutError("Task interaction timed out") - result = await asyncio.wait_for( - self._send_once(card, request, current_content, context_id, remote_task_id), - timeout=remaining, + result = await self._send_or_recover( + card, request, current_content, context_id, remote_task_id, + interaction, deadline, ) if result.task is not None and result.task.id: interaction["remote_task_id"] = result.task.id @@ -229,25 +260,42 @@ async def _dispatch( ) self._validate_remote_identity(result, context_id, remote_task_id) - if result.task_state != "TASK_STATE_INPUT_REQUIRED": - return await self._finish_remote_result( - request.agent_name, result, interaction, + # A taskless bare message may carry a Propose: A2A-T lets the remote + # negotiate *before* creating any task. Interpreting such a message as + # a final answer would hand the host a negotiation request as if it + # were the business result, with no error to notice. + candidate: Optional[ReceivedMessage] = None + taskless = result.task_state != "TASK_STATE_INPUT_REQUIRED" + if taskless: + candidate = ( + self._negotiation_candidate(result) if result.task is None else None ) + if candidate is None: + return await self._finish_remote_result( + request.agent_name, result, interaction, + ) interaction["negotiation_started"] = True if exchange_number >= self._max_negotiation_exchanges: raise RuntimeError("Negotiation exchange budget exhausted; no Abort generated") if callbacks is None: raise RuntimeError("on_negotiation handler is required") - if remote_task_id is None: + if not taskless and remote_task_id is None: raise ValueError("INPUT_REQUIRED has no remote task identity") - received = self._negotiation_response(result) + received = candidate if taskless else self._negotiation_response(result) context = self._negotiation_context(received) previous_context = contexts.get(context.id) self._validate_context_progression(previous_context, context) contexts[context.id] = context key = (remote_task_id, context.id, context.round) while key in answered: + if remote_task_id is None: + # Without a task there is nothing to poll, and re-sending the + # same reply for the same round would spin forever. + raise RuntimeError( + "Remote repeated a taskless negotiation round " + f"(context={context.id}, round={context.round})" + ) await asyncio.sleep(0.25) remaining = deadline - time.monotonic() if remaining <= 0: @@ -358,17 +406,96 @@ async def _cancel_abandoned_interaction( f"task_id={task_id}, error={type(exc).__name__}: {exc}" ) - async def _send_once( - self, card, request: TaskRequest, content: MessageContent, - context_id: str, remote_task_id: Optional[str], + async def _send_or_recover( + self, + card, + request: TaskRequest, + content: MessageContent, + context_id: str, + remote_task_id: Optional[str], + interaction: Dict[str, Any], + deadline: float, ) -> SendMessageResult: + """Send once, recovering from a dropped response stream by polling the task. + + An intermediary (API gateway, load balancer) may close an idle SSE stream + while the remote task is still running. That is a transport fault, not a + business failure: the task must be awaited rather than abandoned or + cancelled, so polling takes over until the task leaves a non-terminal + state or the deadline expires. + + The split is by *phase*, never by exception type: local preparation + (extension validation, agent lookup, request building) runs outside the + recoverable region, so a fault there still propagates and lets the caller + cancel the remote task. Everything raised while the stream is being + consumed is a recovery candidate, because a transport may surface a + dropped connection as any exception class at all. + """ self._emit(EventType.AGENT_REQUEST, {"agent": request.agent_name, "content": content}) self._transport.validate_content_extensions(card, content) - client = self._transport.create_a2a_client(card) + client = self._transport.client_for(request.agent_name) send_request = self._transport.build_send_request(content, context_id, remote_task_id) - return await self._transport.consume_stream( - client, send_request, self._forward_intermediate_event, request.agent_name, - ) + + def forward_intermediate(event_type: str, data: Dict[str, Any]) -> None: + # Remember the remote task identity the moment the stream reveals it. + # If an intermediary drops the connection afterwards, the task is still + # pollable instead of having to be abandoned or cancelled. + revealed = data.get("task_id") + if revealed: + interaction["remote_task_id"] = revealed + self._forward_intermediate_event(event_type, data) + + try: + return await asyncio.wait_for( + self._transport.consume_stream( + client, send_request, forward_intermediate, request.agent_name, + ), + timeout=max(0.0, deadline - time.monotonic()), + ) + except Exception as exc: + from workflow_engine.client.remote_error import EmptyA2AStreamError, find_in_exception + + # A structured remote rejection is a result, not an interrupted + # transport stream. Polling would obscure its reason and may wait + # on a task the remote has already rejected. + if isinstance(exc, EmptyA2AStreamError) or find_in_exception(exc) is not None: + raise + known_task_id = interaction.get("remote_task_id") + if not known_task_id or interaction.get("terminal"): + raise + if time.monotonic() >= deadline: + # Out of time: the interaction itself failed, so the caller keeps + # its existing contract of cancelling the remote task. + raise + logger.warning( + f"Response stream interrupted for agent={request.agent_name}, " + f"task_id={known_task_id}; the task may still be running, so " + f"falling back to polling every {self.task_poll_interval_seconds}s " + f"({type(exc).__name__}: {exc})" + ) + return await self._await_remote_task( + request.agent_name, known_task_id, deadline, + ) + + async def _await_remote_task( + self, agent_name: str, remote_task_id: str, deadline: float, + ) -> SendMessageResult: + """Poll ``get_task`` until the task leaves SUBMITTED/WORKING or time runs out.""" + interval = self.task_poll_interval_seconds + while True: + remaining = deadline - time.monotonic() + if remaining <= 0: + raise TimeoutError( + "Task interaction timed out while polling a remote task whose " + "response stream was interrupted" + ) + result = await asyncio.wait_for( + self._transport.get_task(agent_name, remote_task_id), + timeout=remaining, + ) + if result.task_state not in {"TASK_STATE_SUBMITTED", "TASK_STATE_WORKING"}: + return result + await asyncio.sleep(min(interval, max(0.0, deadline - time.monotonic()))) @staticmethod def _validate_remote_identity( @@ -394,12 +521,30 @@ def _metadata_views(received: ReceivedMessage): yield artifact.metadata @classmethod - def _negotiation_response(cls, result: SendMessageResult) -> ReceivedMessage: + def _negotiation_candidate(cls, result: SendMessageResult) -> Optional[ReceivedMessage]: + """Return the message carrying Negotiation-T metadata, or ``None`` if there is none. + + A2A-T allows a Propose to ride on a **taskless bare message** -- the remote + negotiates before creating any task. ``None`` means "no negotiation + metadata at all", so the message is an ordinary answer. A message that does + carry the Negotiation-T URI but holds a malformed context is rejected by + :meth:`_negotiation_context`, so invalid negotiation metadata fails loudly + instead of being silently mistaken for a business result. + """ uri = A2ATExtension.NEGOTIATION_T.uri for received in result.received_messages: if any(uri in metadata for metadata in cls._metadata_views(received)): return received - raise ValueError("Unsupported INPUT_REQUIRED interaction: no Negotiation-T proposal") + return None + + @classmethod + def _negotiation_response(cls, result: SendMessageResult) -> ReceivedMessage: + received = cls._negotiation_candidate(result) + if received is None: + raise ValueError( + "Unsupported INPUT_REQUIRED interaction: no Negotiation-T proposal" + ) + return received @classmethod def _negotiation_context(cls, received: ReceivedMessage) -> NegotiationContext: diff --git a/workflow_engine/client/env_file_loader.py b/workflow_engine/client/env_file_loader.py index 72d3e0f..a2a9086 100644 --- a/workflow_engine/client/env_file_loader.py +++ b/workflow_engine/client/env_file_loader.py @@ -24,11 +24,15 @@ """ import os +import re from pathlib import Path from typing import Optional, Union from loguru import logger +# A POSIX shell identifier, which is also what os.environ can actually hold. +_ENV_KEY_PATTERN = re.compile(r"[A-Za-z_][A-Za-z0-9_]*\Z") + def load_to_environ(env_file_path: Optional[Union[str, Path]]) -> int: """Parse a ``.env`` file and set each key as an env var unless already set. @@ -55,9 +59,16 @@ def load_to_environ(env_file_path: Optional[Union[str, Path]]) -> int: if eq <= 0: continue key = trimmed[:eq].strip() + # Reject anything that is not a valid environment-variable name instead + # of writing it into the process environment unchecked: a malformed or + # hostile .env must not be able to plant e.g. A2AT_CRED_KEY, which would + # then decide how credential ciphertext is decrypted. + if not _ENV_KEY_PATTERN.match(key): + logger.warning(f"[EnvLoader] Ignoring invalid key name: {key!r}") + continue value = trimmed[eq + 1:].strip() - # Strip surrounding quotes - if len(value) >= 2 and value.startswith('"') and value.endswith('"'): + # Strip one layer of matching surrounding quotes. + if len(value) >= 2 and value[0] == value[-1] and value[0] in "\"'": value = value[1:-1] if key in os.environ: continue diff --git a/workflow_engine/client/extension_sender.py b/workflow_engine/client/extension_sender.py index f74c704..8018353 100644 --- a/workflow_engine/client/extension_sender.py +++ b/workflow_engine/client/extension_sender.py @@ -11,20 +11,35 @@ import inspect import time import uuid +from contextlib import aclosing from dataclasses import dataclass from typing import Callable, Optional +from loguru import logger + from workflow_engine.client.a2a_transport import A2ATransport from workflow_engine.client.extensions import A2ATExtension +from workflow_engine.client.transport_activity import bind_activity_listener from workflow_engine.core.models import MessageContent, ReceivedMessage, SendMessageResult @dataclass(frozen=True) class NotificationHeartbeat: + """Local liveness snapshot of one Notification-T subscription. + + ``last_event_at`` reports the last **transport** activity, including an SSE + comment heartbeat, and ``event_count`` counts only decoded A2A business + events. ``last_business_event_at`` exposes the latter directly. That split + matches the Java engine's ``NotificationSubscription``: a long-lived + notification stream may legitimately carry only heartbeats for a long time, + so liveness cannot be judged from business events alone. + """ + opened_at: float last_event_at: Optional[float] event_count: int active: bool + last_business_event_at: Optional[float] = None class NotificationSubscription: @@ -37,9 +52,17 @@ def __init__(self, agent_name: str, context_id: str): self.acknowledgement: asyncio.Future[SendMessageResult] = loop.create_future() self.completion: asyncio.Future[None] = loop.create_future() self._opened_at = time.time() - self._last_event_at: Optional[float] = None + # The idle deadline starts at opening, even if no bytes arrive. Public + # heartbeat activity remains unset until real transport activity, + # matching Java's separate lastActivityNanos and lastActivityAt values. + self._last_activity_at: Optional[float] = None + self._last_activity_monotonic: Optional[float] = time.monotonic() + self._last_business_event_at: Optional[float] = None self._event_count = 0 self._task: Optional[asyncio.Task] = None + self._ack_timer: Optional[asyncio.Task] = None + self._idle_timer: Optional[asyncio.Task] = None + self._completion_error: Optional[Exception] = None self._closed = False @property @@ -49,36 +72,94 @@ def is_active(self) -> bool: @property def heartbeat(self) -> NotificationHeartbeat: return NotificationHeartbeat( - self._opened_at, self._last_event_at, self._event_count, self.is_active + self._opened_at, self._last_activity_at, self._event_count, + self.is_active, self._last_business_event_at, ) + @property + def last_business_event_at(self) -> Optional[float]: + """Last decoded A2A business event, excluding transport-only SSE comments.""" + return self._last_business_event_at + def is_healthy(self, maximum_idle_seconds: float) -> bool: + """Whether transport activity was seen within ``maximum_idle_seconds``. + + Judged from transport activity rather than business events: a healthy + subscription that is only receiving heartbeats would otherwise be + reported as dead, and a caller acting on that would tear down a live + stream. + """ if maximum_idle_seconds < 0: raise ValueError("maximum_idle_seconds must not be negative") return ( - self.is_active and self._last_event_at is not None - and time.time() - self._last_event_at <= maximum_idle_seconds + self.is_active + and self._last_activity_at is not None + and self._last_activity_monotonic is not None + and time.monotonic() - self._last_activity_monotonic <= maximum_idle_seconds ) + def _record_activity(self) -> None: + """Note that the transport delivered something (event or heartbeat).""" + self._last_activity_at = time.time() + self._last_activity_monotonic = time.monotonic() + def _record_event(self) -> None: self._event_count += 1 - self._last_event_at = time.time() + self._last_business_event_at = time.time() + self._record_activity() def _attach(self, task: asyncio.Task) -> None: self._task = task + task.add_done_callback(self._on_task_done) + + def _on_task_done(self, task: asyncio.Task) -> None: + """Completion means the streaming task has exited, not merely close requested.""" + self._closed = True + if not self.acknowledgement.done(): + self.acknowledgement.set_exception( + RuntimeError("Notification-T stream ended before acknowledgement") + ) + if not self.completion.done(): + if self._completion_error is not None: + self.completion.set_exception(self._completion_error) + elif task.cancelled(): + self.completion.set_result(None) + elif task.exception() is not None: + self.completion.set_exception(task.exception()) + else: + self.completion.set_result(None) + self._cancel_timers() + + def _attach_ack_timer(self, timer: asyncio.Task) -> None: + self._ack_timer = timer + + def _attach_idle_timer(self, timer: asyncio.Task) -> None: + self._idle_timer = timer + + def _cancel_timers(self) -> None: + try: + current = asyncio.current_task() + except RuntimeError: + current = None + for timer in (self._ack_timer, self._idle_timer): + if timer is not None and timer is not current and not timer.done(): + timer.cancel() def close(self) -> None: if self._closed: return self._closed = True - if self._task is not None and not self._task.done(): + try: + current = asyncio.current_task() + except RuntimeError: + current = None + if self._task is not None and self._task is not current and not self._task.done(): self._task.cancel() + self._cancel_timers() if not self.acknowledgement.done(): self.acknowledgement.cancel( "Notification-T subscription closed before acknowledgement" ) - if not self.completion.done(): - self.completion.set_result(None) async def __aenter__(self): return self @@ -94,8 +175,25 @@ async def __aexit__(self, exc_type, exc_val, exc_tb): class ExtensionSender: """Final-content sender on a caller-owned transport, outside workflow execution.""" - def __init__(self, transport: A2ATransport): + def __init__( + self, + transport: A2ATransport, + notification_ack_timeout_seconds: Optional[float] = None, + notification_idle_timeout_seconds: Optional[float] = None, + ): + if ( + notification_ack_timeout_seconds is not None + and notification_ack_timeout_seconds <= 0 + ): + raise ValueError("notification_ack_timeout_seconds must be positive") + if ( + notification_idle_timeout_seconds is not None + and notification_idle_timeout_seconds <= 0 + ): + raise ValueError("notification_idle_timeout_seconds must be positive") self._transport = transport + self._notification_ack_timeout_seconds = notification_ack_timeout_seconds + self._notification_idle_timeout_seconds = notification_idle_timeout_seconds @property def transport(self) -> A2ATransport: @@ -107,7 +205,7 @@ def _require_extension( card = self._transport.get_card(agent_name) if card is None: raise ValueError(f"Agent not found: {agent_name}") - advertised = self._transport._get_extensions(card) + advertised = self._transport.get_extension_uris(card) if ( extension.uri not in advertised or extension.uri not in content.extensions @@ -122,7 +220,7 @@ async def send_authorization( card = self._require_extension(agent_name, content, A2ATExtension.AUTHORIZATION_T) self._transport.validate_content_extensions(card, content) context_id = str(uuid.uuid4()) - client = self._transport.create_a2a_client(card) + client = self._transport.client_for(agent_name) request = self._transport.build_send_request(content, context_id) deadline = time.monotonic() + self._transport.send_timeout_seconds result = await asyncio.wait_for( @@ -173,36 +271,136 @@ def open_notification( async def consume() -> None: try: + # A subscription owns its A2A client rather than using the + # transport's per-agent client shared with one-shot traffic. client = self._transport.create_a2a_client(card) request = self._transport.build_send_request(content, context_id) - async for response in client.send_message(request): - self._transport.log_response_event(agent_name, response) - subscription._record_event() - result, received = self._incremental_result(response) - if not subscription.acknowledgement.done(): - subscription.acknowledgement.set_result(result) - if received is not None: - returned = listener(subscription, received) - if inspect.isawaitable(returned): - await returned + # Bound for the whole stream, not just the call: the request is + # issued on the first iteration, and every body chunk the HTTP + # layer reads afterwards -- including an SSE comment heartbeat + # that never becomes a decoded event -- reports activity here. + with bind_activity_listener(subscription._record_activity): + async with aclosing(client.send_message(request)) as stream: + async for response in stream: + self._transport.log_response_event(agent_name, response) + subscription._record_event() + result, received = self._incremental_result(response) + state = result.task_state + if state in { + "TASK_STATE_FAILED", "TASK_STATE_CANCELED", + "TASK_STATE_CANCELLED", "TASK_STATE_REJECTED", + } and not subscription.acknowledgement.done(): + subscription.acknowledgement.set_exception( + RuntimeError( + f"Notification-T subscription rejected by {agent_name}: " + f"state={state}, response={result.text or ''}" + ) + ) + subscription.close() + break + if state and not subscription.acknowledgement.done(): + subscription.acknowledgement.set_result(result) + if received is not None: + # Unbind while the host callback runs: any protocol + # traffic it issues belongs to the host, not to this + # subscription, and must not keep a dead stream alive. + with bind_activity_listener(None): + returned = listener(subscription, received) + if inspect.isawaitable(returned): + await returned + # Business task states on this channel do not own + # the subscription lifetime. Only an explicit + # close, transport EOF/error or idle timeout ends it. + if not subscription.is_active: + break except asyncio.CancelledError: - if not subscription.completion.done(): - subscription.completion.set_result(None) + pass except Exception as exc: if not subscription.acknowledgement.done(): subscription.acknowledgement.set_exception(exc) - if not subscription.completion.done(): - subscription.completion.set_exception(exc) + subscription._completion_error = exc else: - if not subscription.completion.done(): - subscription.completion.set_result(None) + if not subscription.acknowledgement.done(): + subscription.acknowledgement.set_exception( + RuntimeError("Notification-T stream ended before acknowledgement") + ) finally: subscription._closed = True - if not subscription.completion.done(): - subscription.completion.set_result(None) + subscription._cancel_timers() task = asyncio.create_task(consume(), name=f"notification-t-{agent_name}") subscription._attach(task) + + ack_timeout = ( + self._notification_ack_timeout_seconds + if self._notification_ack_timeout_seconds is not None + else self._transport.notification_ack_timeout_seconds + ) + + async def enforce_ack_deadline() -> None: + # Bounds only the *first* acknowledgement. A remote that accepts the + # stream but never acknowledges would otherwise leave the caller + # waiting forever. SSE idleness after the ACK is deliberately not + # bounded here, matching the Java engine's notificationAckTimeoutSeconds. + await asyncio.sleep(ack_timeout) + if subscription.acknowledgement.done(): + return + subscription.acknowledgement.set_exception( + TimeoutError( + f"Notification-T acknowledgement timed out after {ack_timeout}s" + ) + ) + logger.warning( + f"Notification-T subscription for agent={agent_name} was not " + f"acknowledged within {ack_timeout}s; closing the subscription" + ) + subscription.close() + + subscription._attach_ack_timer(asyncio.create_task( + enforce_ack_deadline(), name=f"notification-t-ack-{agent_name}", + )) + subscription.acknowledgement.add_done_callback( + lambda _: subscription._ack_timer.cancel() + if subscription._ack_timer is not None and not subscription._ack_timer.done() + else None + ) + + idle_timeout = ( + self._notification_idle_timeout_seconds + if self._notification_idle_timeout_seconds is not None + else float(self._transport.send_timeout_seconds) + ) + + async def enforce_idle_deadline() -> None: + # Mirrors the Java engine, which bounds a notification stream by + # sendTimeoutSeconds *without transport activity or a decoded event*. + # Because activity includes SSE heartbeats, a healthy but quiet + # subscription is never reaped -- only a genuinely dead one is + # released, instead of leaking its connection and task forever. + poll_seconds = min(1.0, max(0.05, idle_timeout / 10.0)) + while True: + await asyncio.sleep(poll_seconds) + if subscription.completion.done() or not subscription.is_active: + return + last_activity = subscription._last_activity_monotonic + if last_activity is None: + continue + if time.monotonic() - last_activity < idle_timeout: + continue + subscription._completion_error = TimeoutError( + f"Notification-T stream for agent={agent_name} saw no " + f"activity for {idle_timeout}s" + ) + logger.warning( + f"Notification-T subscription for agent={agent_name} saw no " + f"transport activity for {idle_timeout}s; releasing it" + ) + subscription.close() + return + + subscription._attach_idle_timer(asyncio.create_task( + enforce_idle_deadline(), name=f"notification-t-idle-{agent_name}", + )) return subscription def _incremental_result(self, response): diff --git a/workflow_engine/client/protocol_logger.py b/workflow_engine/client/protocol_logger.py index 3990469..f163f10 100644 --- a/workflow_engine/client/protocol_logger.py +++ b/workflow_engine/client/protocol_logger.py @@ -9,6 +9,17 @@ ``WORKFLOW_ENGINE_PROTOCOL_LOGGING=true`` is set. Sensitive headers remain redacted unless ``WORKFLOW_ENGINE_PROTOCOL_INCLUDE_SENSITIVE_HEADERS=true`` is also explicitly set. + +Redaction combines a name pattern with an explicit registry: authentication +interceptors call :func:`register_sensitive_header` for every header name they +actually inject. A pattern alone is not enough, because the credential +configuration accepts an arbitrary ``auth_header`` name and a custom name such +as ``X-Corp-Cred`` matches none of the built-in patterns. + +Header names are matched after stripping ``-`` and ``_``, so spelling variants +of the same credential header (``api_key``, ``api-key``, ``X-API-KEY``, +``ApiKey``) are all recognised. Matching the raw name alone silently missed +``api_key``, ``access_session``, ``pwd`` and ``passwd``. """ import json @@ -17,25 +28,58 @@ from loguru import logger +from workflow_engine.client.sensitive_data import redact + +# Substring patterns, matched against the separator-stripped lower-case name. _SENSITIVE_HEADER_PARTS = ( - "authorization", "token", "secret", "api-key", "apikey", "cookie" + "authorization", "token", "secret", "password", "cookie", "apikey", ) +# Exact names, matched against the separator-stripped lower-case name. +_SENSITIVE_HEADER_NAMES = frozenset({"pwd", "passwd", "accesssession"}) + +# Header names declared as credential-bearing by the interceptors that inject them. +_SENSITIVE_HEADERS: set[str] = set() + + +def _normalize_header_name(name: str) -> str: + """Lower-case and strip separators so spelling variants collapse together.""" + return name.strip().lower().replace("-", "").replace("_", "") + + +def register_sensitive_header(name: str) -> None: + """Declare one header name as carrying a credential (idempotent).""" + if name and isinstance(name, str): + _SENSITIVE_HEADERS.add(_normalize_header_name(name)) + + +def _is_sensitive(name: str) -> bool: + if not name or not isinstance(name, str): + return False + key = name.strip().lower() + if "://" in key: + # A URL is not a credential header name; redacting it would hide the + # endpoint being called, which is the point of the log. + return False + normalized = _normalize_header_name(name) + return ( + normalized in _SENSITIVE_HEADERS + or normalized in _SENSITIVE_HEADER_NAMES + or any(part in normalized for part in _SENSITIVE_HEADER_PARTS) + ) + def _enabled() -> bool: return os.getenv("WORKFLOW_ENGINE_PROTOCOL_LOGGING", "").lower() == "true" def _format_header(name: str, value: Any) -> str: - normalized = name.lower() include_sensitive = ( os.getenv("WORKFLOW_ENGINE_PROTOCOL_INCLUDE_SENSITIVE_HEADERS", "").lower() == "true" ) - if not include_sensitive and any( - part in normalized for part in _SENSITIVE_HEADER_PARTS - ): + if not include_sensitive and _is_sensitive(name): return "***REDACTED***" return value if isinstance(value, str) else str(value)[:200] @@ -63,14 +107,14 @@ def log_request( header_text = "\n".join(header_lines) if header_lines else " (none)" logger.debug( f">>> [{agent_name}] REQUEST to {endpoint}\n" - f"=== Headers ===\n{header_text}\n=== Body ===\n{body}" + f"=== Headers ===\n{header_text}\n=== Body ===\n{redact(body)}" ) def log_response(agent_name: str, event_type: str, body: str) -> None: """Log an incoming A2A response event.""" if _enabled(): - logger.debug(f"<<< [{agent_name}] RESPONSE [{event_type}]\n{body}") + logger.debug(f"<<< [{agent_name}] RESPONSE [{event_type}]\n{redact(body)}") -__all__ = ["log_request", "log_response"] +__all__ = ["log_request", "log_response", "register_sensitive_header"] diff --git a/workflow_engine/client/remote_error.py b/workflow_engine/client/remote_error.py new file mode 100644 index 0000000..057b9b0 --- /dev/null +++ b/workflow_engine/client/remote_error.py @@ -0,0 +1,145 @@ +# Copyright (c) 2026 Huawei Technologies Co., Ltd. +# All Rights Reserved. +# +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Stable projection of standard A2A error envelopes across HTTP and SSE.""" + +from __future__ import annotations + +import json +import re +from dataclasses import dataclass, field +from typing import Any + +import httpx + +from workflow_engine.client.sensitive_data import redact + + +class EmptyA2AStreamError(RuntimeError): + """The transport completed without a single protocol event.""" + + +@dataclass(frozen=True) +class RemoteA2AError(Exception): + http_status: int + code: int + message: str + status: str = "" + reason: str = "" + domain: str = "" + details: tuple[dict[str, Any], ...] = field(default_factory=tuple) + retry_after: str = "" + + @property + def workflow_code(self) -> str: + reason = re.sub(r"[^a-z0-9]+", "_", self.reason.lower()).strip("_") + return f"a2a.{reason}" if reason else f"a2a.http.{self.http_status}" + + @property + def error_details(self) -> dict[str, Any]: + result: dict[str, Any] = {"httpStatus": self.http_status, "code": self.code} + for key, value in ( + ("status", self.status), ("reason", self.reason), + ("domain", self.domain), ("retryAfter", self.retry_after), + ): + if value: + result[key] = value + if self.details: + result["details"] = list(self.details) + return result + + def __str__(self) -> str: + return self.message + + +def from_payload( + payload: str | dict[str, Any] | None, + *, + observed_http_status: int = 0, + retry_after: str = "", +) -> RemoteA2AError | None: + try: + value = json.loads(payload) if isinstance(payload, str) else payload + except ValueError: + return None + if not isinstance(value, dict) or not isinstance(value.get("error"), dict): + return None + error = value["error"] + code = error.get("code") + message = error.get("message") + if type(code) is not int or not 400 <= code <= 599 or not isinstance(message, str) or not message.strip(): + return None + status = observed_http_status if 400 <= observed_http_status <= 599 else code + raw_details = error.get("details") + details = tuple( + json.loads(redact(json.dumps(item, ensure_ascii=False))) + for item in raw_details + if isinstance(item, dict) + ) if isinstance(raw_details, list) else () + reason = "" + domain = "" + for detail in details: + if isinstance(detail.get("reason"), str): + reason = detail["reason"] + domain = detail.get("domain", "") if isinstance(detail.get("domain"), str) else "" + break + return RemoteA2AError( + http_status=status, + code=code, + message=redact(message), + status=redact(error.get("status", "")) if isinstance(error.get("status"), str) else "", + reason=reason, + domain=domain, + details=details, + retry_after=redact(retry_after), + ) + + +def find_in_exception(error: BaseException) -> RemoteA2AError | None: + """Recover a structured error from SDK wrappers without exposing raw bodies.""" + seen: set[int] = set() + current: BaseException | None = error + while current is not None and id(current) not in seen: + seen.add(id(current)) + if isinstance(current, RemoteA2AError): + return current + if isinstance(current, httpx.HTTPStatusError): + response = current.response + try: + parsed = from_payload( + response.text, + observed_http_status=response.status_code, + retry_after=response.headers.get("Retry-After", ""), + ) + except httpx.ResponseNotRead: + parsed = None + if parsed: + return parsed + if 400 <= response.status_code <= 599: + return RemoteA2AError( + response.status_code, response.status_code, + f"A2A request failed with HTTP {response.status_code}", + retry_after=redact(response.headers.get("Retry-After", "")), + ) + message = str(current) + for prefix in ("SSE stream error event received: ", "SSE stream error: "): + if message.startswith(prefix): + parsed = from_payload(message[len(prefix):]) + if parsed: + return parsed + current = current.__cause__ or current.__context__ + return None diff --git a/workflow_engine/client/sensitive_data.py b/workflow_engine/client/sensitive_data.py new file mode 100644 index 0000000..c889abd --- /dev/null +++ b/workflow_engine/client/sensitive_data.py @@ -0,0 +1,65 @@ +# Copyright (c) 2026 Huawei Technologies Co., Ltd. +# All Rights Reserved. +# +# SPDX-License-Identifier: Apache-2.0 +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Redact credential-shaped values before they enter diagnostics.""" + +from __future__ import annotations + +import json +import re +from typing import Any + +_FORM_SECRET = re.compile( + r"(?i)((?:password|passwd|pwd|[\w-]*token|[\w-]*secret|accessSession|api[-_]?key)=)[^&\s]*" +) +_AUTH_VALUE = re.compile(r"(?i)(\b(?:Bearer|Basic)[ \t]+)[^\s\"\\,;]+") +_JSON_SECRET = re.compile( + r'(?i)("(?:[^"\\]|\\.)*(?:password|passwd|pwd|token|secret|api[-_]?key|access[-_]?session)(?:[^"\\]|\\.)*"\s*:\s*)' + r'("(?:[^"\\]|\\.)*"|\{[^{}]*\}|\[[^\[\]]*\]|[^,}\]\s]+)' +) + + +def _sensitive_key(name: str) -> bool: + normalized = name.lower().replace("-", "").replace("_", "") + return ( + any(term in normalized for term in ("authorization", "password", "token", "secret", "apikey", "cookie")) + or normalized in {"pwd", "passwd", "accesssession"} + ) + + +def _redact_tree(value: Any) -> Any: + if isinstance(value, dict): + return { + key: "***" if _sensitive_key(str(key)) else _redact_tree(item) + for key, item in value.items() + } + if isinstance(value, list): + return [_redact_tree(item) for item in value] + return value + + +def redact(text: str | None) -> str: + """Hide standard credential forms while leaving other business text readable.""" + if text is None: + return "" + try: + parsed = json.loads(text) + except (TypeError, ValueError): + cleaned = _JSON_SECRET.sub(r'\1"***"', text) + else: + cleaned = json.dumps(_redact_tree(parsed), ensure_ascii=False) + return _AUTH_VALUE.sub(r"\1***", _FORM_SECRET.sub(r"\1***", cleaned)) diff --git a/workflow_engine/client/sse_normalization.py b/workflow_engine/client/sse_normalization.py index b07cd86..019544d 100644 --- a/workflow_engine/client/sse_normalization.py +++ b/workflow_engine/client/sse_normalization.py @@ -18,20 +18,28 @@ """SSE response normalization for non-standard agent responses. Some A2A agents return bare Task or Message objects instead of properly -wrapped StreamResponse envelopes. This module patches google.protobuf -json_format.Parse/ParseDict to coerce such responses into the expected -StreamResponse shape, mirroring the orchestration center's exec_engine. +wrapped StreamResponse envelopes. This module patches +``google.protobuf.json_format.ParseDict`` to coerce such responses into the +expected StreamResponse shape, mirroring the orchestration center's +exec_engine. + +Only ``ParseDict`` is patched. ``json_format.Parse`` resolves ``ParseDict`` +from its module globals at call time, so normalizing the dict entry point +already covers the text entry point -- and patching ``Parse`` as well would +override a caller's explicit ``ignore_unknown_fields`` argument instead of +honouring it. Import this module once at startup; the patch is process-global. """ import json as _json import google.protobuf.json_format as _json_format +from loguru import logger _STREAM_RESPONSE_KEYS = frozenset({"task", "message", "statusUpdate", "artifactUpdate"}) -_original_parse = _json_format.Parse _original_parse_dict = _json_format.ParseDict +_APPLIED = False def _normalize_stream_response(data: dict) -> dict: @@ -47,44 +55,41 @@ def _normalize_stream_response(data: dict) -> dict: return data -def _parse_with_unknown(text, message, ignore_unknown_fields=False, **kwargs): - from a2a.types.a2a_pb2 import StreamResponse - is_stream = isinstance(message, StreamResponse) - if is_stream: - try: - data = _json.loads(text) - if isinstance(data, dict): - if not _STREAM_RESPONSE_KEYS.intersection(data): - logger = __import__("loguru").logger - logger.warning( - f"[A2A] Non-SSE response from server: body_chars={len(text)}" - ) - logger.trace(f"[A2A] Non-SSE response body: {text[:2048]}") - data = _normalize_stream_response(data) - text = _json.dumps(data) - except Exception: - pass - kwargs["ignore_unknown_fields"] = True - return _original_parse(text, message, ignore_unknown_fields=ignore_unknown_fields, **kwargs) - - -def _parse_dict_with_unknown(js, message, *args, **kwargs): +def _parse_dict_with_unknown(js, message, ignore_unknown_fields=False, *args, **kwargs): + """Normalize a bare Task/Message payload before delegating to ParseDict. + + Non-``StreamResponse`` targets keep the caller's exact behaviour. A + ``StreamResponse`` target always parses with ``ignore_unknown_fields=True``: + the payload has already been recognized as non-standard, and a newer agent + adding a field must not turn a working stream into a hard failure. + """ from a2a.types.a2a_pb2 import StreamResponse - is_stream = isinstance(message, StreamResponse) - if is_stream and isinstance(js, dict): + if not isinstance(message, StreamResponse): + return _original_parse_dict(js, message, ignore_unknown_fields, *args, **kwargs) + if isinstance(js, dict): + from workflow_engine.client.remote_error import from_payload + + remote_error = from_payload(js) + if remote_error is not None: + raise remote_error + if not _STREAM_RESPONSE_KEYS.intersection(js): + logger.warning( + f"[A2A] Non-SSE response from server: keys={sorted(js)[:8]}" + ) + from workflow_engine.client.sensitive_data import redact + + logger.trace( + f"[A2A] Non-SSE response body: " + f"{redact(_json.dumps(js, ensure_ascii=False, default=str))[:2048]}" + ) js = _normalize_stream_response(js) - if not is_stream: - return _original_parse_dict(js, message, *args, **kwargs) - kwargs.pop("ignore_unknown_fields", None) - args = list(args) - if args: - args[0] = True - else: - kwargs["ignore_unknown_fields"] = True - return _original_parse_dict(js, message, *args, **kwargs) + return _original_parse_dict(js, message, True, *args, **kwargs) def apply_sse_normalization(): - """Apply the global Parse/ParseDict patches (idempotent).""" - _json_format.Parse = _parse_with_unknown + """Apply the global ``ParseDict`` patch (idempotent).""" + global _APPLIED + if _APPLIED: + return _json_format.ParseDict = _parse_dict_with_unknown + _APPLIED = True diff --git a/workflow_engine/client/ssl_context.py b/workflow_engine/client/ssl_context.py index 67d3b02..ae7ba1e 100644 --- a/workflow_engine/client/ssl_context.py +++ b/workflow_engine/client/ssl_context.py @@ -42,11 +42,40 @@ def create_ssl_context( crl_path: Path to CRL file. Returns: - ssl.SSLContext if verification enabled, False otherwise. + ssl.SSLContext if verification is enabled, or if verification is + disabled but a client identity must still be presented; ``False`` only + when verification is disabled and no client identity was supplied. """ if not verify_server: - logger.warning("Outbound TLS verification disabled. Insecure for production.") - return False + # A caller that disabled server verification may still need to present a + # client identity. Returning plain False silently discarded the mTLS + # material, so build an unverified context that keeps it instead. + if ca_certs_path or crl_path: + raise ValueError( + "ca_certs_path/crl_path require verify_server=True: a trust store " + "or CRL cannot be enforced while server verification is disabled" + ) + if not (cert_path or key_path): + logger.warning("Outbound TLS verification disabled. Insecure for production.") + return False + if bool(cert_path) != bool(key_path): + raise ValueError("Both client certificate and private key are required for mTLS") + if not os.path.isfile(cert_path) or not os.path.isfile(key_path): + raise FileNotFoundError("Client certificate or private key not found") + # check_hostname must be cleared before verify_mode, or the stdlib raises. + ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) + ctx.check_hostname = False + ctx.verify_mode = ssl.CERT_NONE + ctx.load_cert_chain( + certfile=cert_path, + keyfile=key_path, + password=key_password if key_password else None, + ) + logger.warning( + "Outbound TLS verification disabled but a client identity (mTLS) is " + "still presented. Insecure for production." + ) + return ctx ctx = ssl.create_default_context(ssl.Purpose.SERVER_AUTH) diff --git a/workflow_engine/client/transport_activity.py b/workflow_engine/client/transport_activity.py new file mode 100644 index 0000000..37ef161 --- /dev/null +++ b/workflow_engine/client/transport_activity.py @@ -0,0 +1,122 @@ +# Copyright (c) 2026 Huawei Technologies Co., Ltd. +# All Rights Reserved. +# +# SPDX-License-Identifier: Apache-2.0 + +"""Per-call transport activity observation for long-lived streams. + +Mirrors the Java engine's ``TransportActivityMonitor``. An activity listener is +bound for the duration of one protocol call, and the HTTP layer invokes it for +every body chunk it actually reads -- including an SSE comment heartbeat, which +never reaches the protocol layer as a decoded event. + +The binding is a :class:`contextvars.ContextVar` rather than a ``ThreadLocal`` +because that is the asyncio equivalent: a task inherits its creator's context, so +the response hook installed on the transport's httpx client attributes activity +to the call that opened the stream. Each subscription has its own A2A client; +separate Authorization-T and Notification-T transports own separate httpx clients. + +Why this matters: a notification stream that is merely quiet looks identical to a +dead one if liveness is judged only from decoded events. Judging it from observed +transport activity is what lets an idle subscription be reaped without tearing +down a healthy one that is receiving heartbeats. +""" + +from __future__ import annotations + +import contextlib +import contextvars +from typing import Callable, Iterator, Optional + +import httpx +from loguru import logger + + +ActivityListener = Callable[[], None] + +_ACTIVITY_LISTENER: contextvars.ContextVar[Optional[ActivityListener]] = ( + contextvars.ContextVar("workflow_engine_transport_activity", default=None) +) + +_HOOK_MARKER = "_workflow_engine_transport_activity_hook" + + +@contextlib.contextmanager +def bind_activity_listener(listener: Optional[ActivityListener]) -> Iterator[None]: + """Bind ``listener`` for the duration of one protocol call.""" + token = _ACTIVITY_LISTENER.set(listener) + try: + yield + finally: + _ACTIVITY_LISTENER.reset(token) + + +def current_activity_listener() -> Optional[ActivityListener]: + """Return the listener bound to the current call, if any.""" + return _ACTIVITY_LISTENER.get() + + +def _notify(listener: Optional[ActivityListener]) -> None: + if listener is None: + return + try: + listener() + except Exception as exc: # never let observation break the stream + logger.warning(f"Transport activity callback failed: {exc}") + + +class _ObservingStream(httpx.AsyncByteStream): + """Forwards body chunks while reporting each one as transport activity.""" + + def __init__(self, inner: httpx.AsyncByteStream, listener: ActivityListener): + self._inner = inner + self._listener = listener + + async def __aiter__(self): + async for chunk in self._inner: + _notify(self._listener) + yield chunk + + async def aclose(self) -> None: + await self._inner.aclose() + + +async def _observe_response(response: httpx.Response) -> None: + """Response hook: capture the current listener and wrap the unread body. + + httpx runs response hooks after the headers arrive and before the body is + read, so the stream can still be replaced here. + """ + listener = _ACTIVITY_LISTENER.get() + if listener is None: + return + stream = response.stream + if isinstance(stream, httpx.AsyncByteStream): + response.stream = _ObservingStream(stream, listener) + # A response that carries no body is still activity: the round trip completed. + _notify(listener) + + +setattr(_observe_response, _HOOK_MARKER, True) + + +def install_activity_hook(client: httpx.AsyncClient) -> bool: + """Install the observing response hook on ``client`` (idempotent). + + Returns True when the hook was newly installed. The hook is a no-op unless a + listener is bound, so installing it on a caller-provided client does not + change behaviour for traffic that never opens a notification stream. + """ + response_hooks = client.event_hooks.setdefault("response", []) + if any(getattr(hook, _HOOK_MARKER, False) for hook in response_hooks): + return False + response_hooks.append(_observe_response) + return True + + +__all__ = [ + "ActivityListener", + "bind_activity_listener", + "current_activity_listener", + "install_activity_hook", +] diff --git a/workflow_engine/core/executor.py b/workflow_engine/core/executor.py index 5f99237..83a3340 100644 --- a/workflow_engine/core/executor.py +++ b/workflow_engine/core/executor.py @@ -96,7 +96,7 @@ async def _run_bound(self) -> ExecutionResult: if not ready: missing = { self.workflow.steps[index].name: [ - name for name in self.context_builder.get_step_predecessors( + name for name in self.context_builder.get_all_predecessors( self.workflow.steps[index].name ) if (pred_index := self.context_builder.find_step_index(name)) in activated @@ -146,11 +146,17 @@ def _collect_ready(self, pending, activated, executed): if index >= len(self.workflow.steps) or index in executed: continue step = self.workflow.steps[index] - active_predecessors = [ - name for name in self.context_builder.get_step_predecessors(step.name) + # A direct predecessor can still be inactive while an earlier branch is + # running and may activate it later. Waiting on every active ancestor -- + # not just the direct predecessors -- is what blocks a merge from running + # before the branch that feeds it: an ancestor that is already activated + # but has not produced output is exactly the "still running" signal, and a + # direct predecessor that has not been activated yet is invisible here. + active_ancestors = [ + name for name in self.context_builder.get_all_predecessors(step.name) if (pred_index := self.context_builder.find_step_index(name)) in activated ] - (ready if all(name in self.step_outputs for name in active_predecessors) + (ready if all(name in self.step_outputs for name in active_ancestors) else deferred).append(index) return ready, deferred diff --git a/workflow_engine/core/failure_mapping.py b/workflow_engine/core/failure_mapping.py index 7eee592..9674794 100644 --- a/workflow_engine/core/failure_mapping.py +++ b/workflow_engine/core/failure_mapping.py @@ -8,7 +8,10 @@ from __future__ import annotations import asyncio +import json +from workflow_engine.client.remote_error import EmptyA2AStreamError, find_in_exception +from workflow_engine.client.sensitive_data import redact from workflow_engine.core.models import BusinessFailure, TaskResult @@ -27,24 +30,53 @@ def failure_to_task_result(error: BaseException) -> TaskResult: error_details=current.details, ) + if isinstance(current, EmptyA2AStreamError): + return TaskResult.failed("a2a.empty_stream", str(current)) + + remote = find_in_exception(error) + if remote is not None: + return TaskResult( + success=False, + error_code=remote.workflow_code, + error=remote.message, + error_details=remote.error_details, + ) + try: from a2a.utils.errors import A2AError, A2A_ERROR_MAPPING if isinstance(current, A2AError): - mapping = A2A_ERROR_MAPPING.get(type(current)) + # Walk the MRO: the mapping is keyed by exact type, so a subclass + # raised by the SDK would otherwise fall through to the generic code. + mapping = None + for klass in type(current).__mro__: + mapping = A2A_ERROR_MAPPING.get(klass) + if mapping is not None: + break reason = mapping.reason if mapping else "" - code = f"a2a.{reason.lower()}" if reason else "a2a.remote_error" - details = dict(getattr(current, "data", None) or {}) + http_status = mapping.http_code if mapping else 500 + code = f"a2a.{reason.lower()}" if reason else f"a2a.http.{http_status}" + metadata = dict(getattr(current, "data", None) or {}) + safe_metadata = json.loads(redact(json.dumps(metadata, ensure_ascii=False, default=str))) + detail = { + "@type": "type.googleapis.com/google.rpc.ErrorInfo", + "domain": "a2a-protocol.org", + } + if reason: + detail["reason"] = reason + if safe_metadata: + detail["metadata"] = safe_metadata + details = {"httpStatus": http_status, "code": http_status} if mapping: details.update({ - "http_status": mapping.http_code, "status": mapping.grpc_status, "reason": mapping.reason, "domain": "a2a-protocol.org", }) + details["details"] = [detail] return TaskResult( success=False, error_code=code, - error=getattr(current, "message", type(current).__name__), + error=redact(getattr(current, "message", type(current).__name__)), error_details=details, ) except ImportError: @@ -56,4 +88,4 @@ def failure_to_task_result(error: BaseException) -> TaskResult: code = "workflow.cancelled" else: code = "workflow.execution_failed" - return TaskResult.failed(code, str(current) or type(current).__name__) + return TaskResult.failed(code, redact(str(current)) or type(current).__name__) diff --git a/workflow_engine/core/models.py b/workflow_engine/core/models.py index 0177727..12b9b1e 100644 --- a/workflow_engine/core/models.py +++ b/workflow_engine/core/models.py @@ -124,6 +124,16 @@ def text(cls, text: str) -> "MessageContent": def from_parts(cls, parts: Sequence[Any]) -> "MessageContent": return cls(parts=tuple(parts)) + def with_extension(self, uri: str) -> "MessageContent": + """Return an immutable copy with one additional activated extension.""" + if not isinstance(uri, str) or not uri.strip(): + raise ValueError("Extension URI must not be blank") + return MessageContent( + parts=self.parts, + metadata=self.metadata, + extensions=self.extensions | {uri}, + ) + @dataclass(frozen=True) class ReceivedArtifact: @@ -264,12 +274,16 @@ def from_dict(cls, data: Mapping[str, Any]) -> "Workflow": raw_input = raw_task.get("input") business_input = None if isinstance(raw_input, Mapping): - if raw_input.get("text") is not None: + if len(raw_input) != 1 or ("text" in raw_input) == ("data" in raw_input): + raise ValueError("subtask input must contain exactly one of text or data") + if "text" in raw_input: business_input = BusinessInput.from_text(raw_input["text"]) - elif "data" in raw_input: + else: business_input = BusinessInput.from_data(raw_input["data"]) elif isinstance(raw_input, str): business_input = BusinessInput.from_text(raw_input) + elif raw_input is not None: + raise ValueError("subtask input must be an object with text or data") subtasks.append(Task( agent=raw_task.get("agent", ""), skill=raw_task.get("skill", ""), diff --git a/workflow_engine/registry/registry_client.py b/workflow_engine/registry/registry_client.py index 4666a24..c340726 100644 --- a/workflow_engine/registry/registry_client.py +++ b/workflow_engine/registry/registry_client.py @@ -1,4 +1,4 @@ -# Copyright (c) 2026 Huawei Technologies Co., Ltd. +# Copyright (c) 2026 Huawei Technologies Co., Ltd. # All Rights Reserved. # # SPDX-License-Identifier: Apache-2.0 @@ -19,14 +19,50 @@ Users can use this, or fetch AgentCards from any other source. The SDK does not depend on this module. + +TLS options mirror :class:`~workflow_engine.client.a2a_transport.A2ATransport` +(``ssl_verify`` plus an optional custom CA, mTLS client identity and CRL). A +boolean switch alone left a deployment behind a private CA with no option except +disabling verification outright, so the registry helpers accept the same +material the transport does. """ import json -from typing import List, Any +from typing import Any, Dict, List, Optional from loguru import logger from workflow_engine.client.agentcard_normalizer import normalize_agent_dict +from workflow_engine.client.ssl_context import create_ssl_context + + +def _tls_options( + ssl_verify: bool = True, + ca_certs_path: Optional[str] = None, + client_cert_path: Optional[str] = None, + client_key_path: Optional[str] = None, + client_key_password: Optional[str] = None, + crl_path: Optional[str] = None, +): + """Build the httpx ``verify`` argument, failing closed on bad TLS material.""" + return create_ssl_context( + verify_server=ssl_verify, + ca_certs_path=ca_certs_path, + cert_path=client_cert_path, + key_path=client_key_path, + key_password=client_key_password, + crl_path=crl_path, + ) + +def _http_timeout(connect_seconds: float, read_seconds: float): + import httpx + + if connect_seconds <= 0 or read_seconds <= 0: + raise ValueError("connect and read timeouts must be positive") + return httpx.Timeout( + connect=connect_seconds, read=read_seconds, + write=read_seconds, pool=connect_seconds, + ) async def load_psop( @@ -34,6 +70,14 @@ async def load_psop( psop_id: str, access_token: str = None, ssl_verify: bool = True, + ca_certs_path: Optional[str] = None, + client_cert_path: Optional[str] = None, + client_key_path: Optional[str] = None, + client_key_password: Optional[str] = None, + crl_path: Optional[str] = None, + *, + connect_timeout_seconds: float = 30.0, + read_timeout_seconds: float = 30.0, ) -> "Workflow": """Fetch a PSOP from the orchestration center external API. @@ -46,7 +90,14 @@ async def load_psop( url = f"{base_url}/api/v1/orchestrate/psop/{psop_id}" headers = {"Authorization": f"Bearer {access_token}"} if access_token else {} logger.info(f"[Registry] Loading PSOP from {url} (ssl_verify={ssl_verify})") - async with httpx.AsyncClient(verify=ssl_verify, timeout=30, follow_redirects=False) as client: + async with httpx.AsyncClient( + verify=_tls_options( + ssl_verify, ca_certs_path, client_cert_path, client_key_path, + client_key_password, crl_path, + ), + timeout=_http_timeout(connect_timeout_seconds, read_timeout_seconds), + follow_redirects=False, + ) as client: resp = await client.get(url, headers=headers) resp.raise_for_status() data = resp.json() @@ -61,6 +112,14 @@ async def search_psop( top_n: int = 5, access_token: str = None, ssl_verify: bool = True, + ca_certs_path: Optional[str] = None, + client_cert_path: Optional[str] = None, + client_key_path: Optional[str] = None, + client_key_password: Optional[str] = None, + crl_path: Optional[str] = None, + *, + connect_timeout_seconds: float = 30.0, + read_timeout_seconds: float = 30.0, ) -> List["WorkflowSearchResult"]: """Search for matching PSOP workflows from the orchestration center. @@ -76,8 +135,14 @@ async def search_psop( headers = {"Authorization": f"Bearer {access_token}"} if access_token else {} body = {"intent": intent, "top_n": top_n} logger.info(f"[Registry] Searching PSOP at {url} (intent_chars={len(intent)}, top_n={top_n})") - logger.trace(f"[Registry] Search intent={intent}") - async with httpx.AsyncClient(verify=ssl_verify, timeout=30, follow_redirects=False) as client: + async with httpx.AsyncClient( + verify=_tls_options( + ssl_verify, ca_certs_path, client_cert_path, client_key_path, + client_key_password, crl_path, + ), + timeout=_http_timeout(connect_timeout_seconds, read_timeout_seconds), + follow_redirects=False, + ) as client: resp = await client.post(url, json=body, headers=headers) resp.raise_for_status() data = resp.json() @@ -87,20 +152,53 @@ async def search_psop( return results - class RegistryClient: - """Fetches AgentCards from the Registry Center.""" + """Fetches AgentCards from the Registry Center. - def __init__(self, url: str, ssl_verify: bool = True): + ``access_token`` is attached to every request, so a registry with + authentication enabled is reachable through the same client the card + helpers use. + """ + + def __init__( + self, + url: str, + ssl_verify: bool = True, + access_token: Optional[str] = None, + ca_certs_path: Optional[str] = None, + client_cert_path: Optional[str] = None, + client_key_path: Optional[str] = None, + client_key_password: Optional[str] = None, + crl_path: Optional[str] = None, + *, + connect_timeout_seconds: float = 30.0, + read_timeout_seconds: float = 30.0, + ): self.url = url.rstrip("/") self.ssl_verify = ssl_verify + self.access_token = access_token + self._timeout = _http_timeout(connect_timeout_seconds, read_timeout_seconds) + # Built once so a missing or invalid TLS file fails at construction + # rather than halfway through a fetch. + self._verify = _tls_options( + ssl_verify, ca_certs_path, client_cert_path, client_key_path, + client_key_password, crl_path, + ) + + def _headers(self) -> Dict[str, str]: + if not self.access_token: + return {} + return {"Authorization": f"Bearer {self.access_token}"} async def fetch_agent_cards(self) -> List[Any]: """Fetch all AgentCards. Returns protobuf objects if a2a-sdk available, else dicts.""" import httpx logger.info(f"[Registry] Fetching all agent cards from {self.url}") - async with httpx.AsyncClient(verify=self.ssl_verify, timeout=30) as client: - resp = await client.get(f"{self.url}/rest/v1/registry-center/agent-cards") + async with httpx.AsyncClient(verify=self._verify, timeout=self._timeout) as client: + resp = await client.get( + f"{self.url}/rest/v1/registry-center/agent-cards", + headers=self._headers(), + ) resp.raise_for_status() data = resp.json() raw_cards = data.get("agentCards", data.get("data", [])) @@ -121,8 +219,12 @@ async def fetch_agent_card(self, name: str, organization: str = None) -> Any: params = {"name": name} if organization: params["organization"] = organization - async with httpx.AsyncClient(verify=self.ssl_verify, timeout=30) as client: - resp = await client.get(f"{self.url}/rest/v1/registry-center/agent-cards", params=params) + async with httpx.AsyncClient(verify=self._verify, timeout=self._timeout) as client: + resp = await client.get( + f"{self.url}/rest/v1/registry-center/agent-cards", + params=params, + headers=self._headers(), + ) resp.raise_for_status() data = resp.json() cards = data.get("agentCards", data.get("data", [])) @@ -153,8 +255,8 @@ async def register_agent_card(self, agent_card) -> dict: url = f"{self.url}/rest/v1/registry-center/agent-cards" payload = {"agentCards": [card_payload]} logger.info(f"[Registry] Registering agent card: name={card_payload.get('name', '?')}") - async with httpx.AsyncClient(verify=self.ssl_verify, timeout=30) as client: - resp = await client.post(url, json=payload) + async with httpx.AsyncClient(verify=self._verify, timeout=self._timeout) as client: + resp = await client.post(url, json=payload, headers=self._headers()) resp.raise_for_status() result = resp.json() logger.info(f"[Registry] Agent card registered: name={card_payload.get('name', '?')}") diff --git a/workflow_engine/runner.py b/workflow_engine/runner.py index 1415f37..a6b2a4f 100644 --- a/workflow_engine/runner.py +++ b/workflow_engine/runner.py @@ -1,4 +1,4 @@ -# Copyright (c) 2026 Huawei Technologies Co., Ltd. +# Copyright (c) 2026 Huawei Technologies Co., Ltd. # All Rights Reserved. # # SPDX-License-Identifier: Apache-2.0 @@ -122,6 +122,7 @@ async def execute_psop( auth_provider=None, preferred_protocol: Optional[str] = None, send_timeout_seconds: int = 600, + task_poll_interval_seconds: float = 20.0, max_negotiation_exchanges: int = 3, on_finish: Optional[Callable[[ExecutionResult, list], Awaitable[None]]] = None, on_event: Optional[Callable[[dict], Any]] = None, @@ -171,6 +172,7 @@ async def execute_psop( auth_provider=auth_provider, preferred_protocol=preferred_protocol, send_timeout_seconds=send_timeout_seconds, + task_poll_interval_seconds=task_poll_interval_seconds, ) engine_client = WorkflowEngineClient.owning( transport, @@ -196,8 +198,10 @@ async def _run_and_finalize(): except asyncio.CancelledError: holder["error"] = "Workflow cancelled (client disconnected)" except Exception as e: - logger.error(f"[execute_psop] Execution failed: {e}", exc_info=True) - holder["error"] = str(e) + from workflow_engine.client.sensitive_data import redact + + logger.error(f"[execute_psop] Execution failed: {redact(str(e))}") + holder["error"] = redact(str(e)) finally: if owns_client: try: @@ -226,7 +230,9 @@ async def _run_and_finalize(): if hasattr(ret, "__await__"): await ret except Exception as e: - logger.error(f"[execute_psop] on_finish failed: {e}", exc_info=True) + from workflow_engine.client.sensitive_data import redact + + logger.error(f"[execute_psop] on_finish failed: {redact(str(e))}") emitter.emit("close", {}) emitter.finish()