Skip to content

feat(lib): expose data_converter kwarg on AgentexWorker and Temporal client APIs#372

Open
matteolibrizzi-scale wants to merge 1 commit into
nextfrom
feat/expose-data-converter-kwarg
Open

feat(lib): expose data_converter kwarg on AgentexWorker and Temporal client APIs#372
matteolibrizzi-scale wants to merge 1 commit into
nextfrom
feat/expose-data-converter-kwarg

Conversation

@matteolibrizzi-scale
Copy link
Copy Markdown

@matteolibrizzi-scale matteolibrizzi-scale commented May 27, 2026

Summary

Adds a data_converter kwarg to the AgentexWorker / TemporalClient / TemporalACP API surface so callers can compose OpenAIAgentsPlugin with a payload codec (e.g. AES-GCM at-rest encryption).

Why

Today, passing payload_codec=... to AgentexWorker(...) together with OpenAIAgentsPlugin raises with a hard guard. That guard is correct for the API surface currently exposed: without it, the plugin's _data_converter(None) transformer builds a fresh DataConverter and the user's codec is silently dropped — payloads land in Temporal in plain text.

But the composition does work if you pre-build a DataConverter with both payload_converter_class=OpenAIPayloadConverter AND payload_codec=... and pass it via Client.connect(data_converter=...). The plugin's transformer has a fourth branch that passes through any DataConverter whose payload converter is an OpenAIPayloadConverter instance (or subclass) unchanged. This PR exposes that working path through the agentex API.

The first downstream user is the Emu Deep Research agent, which needs both OpenAIAgentsPlugin (to dispatch model calls as invoke_model_activity under Temporal) and the shared emu_shared.encryption.codec.get_payload_codec() already used by OneEdge / FDD / VDR.

What changed

New data_converter kwarg on:

  • agentex.lib.core.temporal.workers.worker.get_temporal_client and AgentexWorker.__init__ (worker side)
  • agentex.lib.core.clients.temporal.utils.get_temporal_client (client side)
  • TemporalClient.__init__ / TemporalClient.create
  • TemporalACP.__init__ / TemporalACP.create
  • TemporalACPConfig (forwarded by FastACP.create_async_acp)

Guards in both get_temporal_client implementations:

  1. Ambiguitypayload_codec AND data_converter together raise ValueError (the codec belongs inside the data converter).
  2. Silent-drop guard refined — fires only when OpenAIAgentsPlugin is present AND payload_codec is the standalone kwarg AND no data_converter was supplied. The error message now points callers at the working composition path (DataConverter(payload_converter_class=OpenAIPayloadConverter, payload_codec=...)).

No behavior change for existing callers — none currently pass data_converter.

Usage

from temporalio.converter import DataConverter
from temporalio.contrib.openai_agents import OpenAIAgentsPlugin, OpenAIPayloadConverter

worker = AgentexWorker(
    task_queue=...,
    plugins=[OpenAIAgentsPlugin(...)],
    data_converter=DataConverter(
        payload_converter_class=OpenAIPayloadConverter,
        payload_codec=my_aes_gcm_codec,
    ),
)

Test plan

  • All 355 existing tests in tests/lib/ pass
  • New tests added in tests/lib/test_payload_codec.py for both get_temporal_client paths:
    • data_converter pass-through with OpenAIAgentsPlugin
    • data_converter pass-through without plugin
    • payload_codec + data_converter together raises ambiguity error
    • Silent-drop guard error message references data_converter
  • New tests for AgentexWorker, TemporalClient, TemporalACP, TemporalACPConfig, FastACP storing/forwarding data_converter
  • Signature smoke check confirms data_converter appears on all five public surfaces

🤖 Generated with Claude Code

Greptile Summary

This PR exposes a data_converter kwarg across the AgentexWorker, TemporalClient, TemporalACP, TemporalACPConfig, and FastACP API surfaces so callers can compose OpenAIAgentsPlugin with an encrypted payload codec by pre-building a DataConverter(payload_converter_class=OpenAIPayloadConverter, payload_codec=...) and passing it directly.

  • New mutual-exclusivity guard (payload_codec + data_converter → ValueError) and a refined silent-drop guard (payload_codec + OpenAIAgentsPlugin → ValueError only when data_converter is absent) are added to both get_temporal_client implementations.
  • The data_converter field is threaded through all five public API surfaces with correct forwarding; TemporalACPConfig gains the field with a good docstring but no Pydantic cross-field validator for mutual exclusivity with payload_codec.
  • The new data_converter path with OpenAIAgentsPlugin has no validation that the supplied converter uses OpenAIPayloadConverter as its payload_converter_class; passing DataConverter(payload_codec=my_codec) (which defaults to DefaultPayloadConverter) alongside the plugin will still silently drop the codec via the plugin's transformer, and the new tests do not catch this because Client.connect is mocked.

Confidence Score: 3/5

The PR is safe for callers who follow the documented pattern (DataConverter(payload_converter_class=OpenAIPayloadConverter, payload_codec=...)), but callers who pass a DataConverter with any other payload_converter_class alongside OpenAIAgentsPlugin will have their codec silently dropped — the same encryption failure the PR was written to prevent.

The core forwarding plumbing is correct and the two new guards catch the most obvious misuse patterns. The remaining gap is that data_converter + OpenAIAgentsPlugin + wrong payload_converter_class still silently drops encryption, and the new tests pass the wrong converter class themselves but don't catch it because Client.connect is mocked.

Both get_temporal_client implementations (src/agentex/lib/core/clients/temporal/utils.py and src/agentex/lib/core/temporal/workers/worker.py) need a guard for data_converter.payload_converter_class when OpenAIAgentsPlugin is present. The test_data_converter_passthrough_with_openai_plugin tests in tests/lib/test_payload_codec.py should also be updated to use OpenAIPayloadConverter as the converter class.

Important Files Changed

Filename Overview
src/agentex/lib/core/clients/temporal/utils.py Adds data_converter kwarg to get_temporal_client; guards for payload_codec+data_converter ambiguity and OpenAI-plugin+payload_codec silent-drop are correct, but the new data_converter path lacks a guard that validates payload_converter_class=OpenAIPayloadConverter when OpenAIAgentsPlugin is present — leaving the original silent-drop risk reachable via the new kwarg.
src/agentex/lib/core/temporal/workers/worker.py Mirrors the utils.py changes for the worker-side get_temporal_client; carries the same missing validation for data_converter.payload_converter_class when OpenAIAgentsPlugin is present.
src/agentex/lib/core/clients/temporal/temporal_client.py Threads data_converter through TemporalClient.__init__, create, and _get_temporal_client correctly; no issues.
src/agentex/lib/types/fastacp.py Adds data_converter: Any = Field(default=None, frozen=True) and good docstring; missing a cross-field model_validator to enforce mutual exclusivity with payload_codec at construction time.
tests/lib/test_payload_codec.py New tests cover the main cases but test_data_converter_passthrough_with_openai_plugin creates DataConverter(payload_codec=_NoopCodec()) (with DefaultPayloadConverter) — the wrong class for use with OpenAIAgentsPlugin — and the bug is masked because Client.connect is mocked.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Caller: AgentexWorker / TemporalClient / TemporalACP"] -->|payload_codec AND data_converter| B["❌ ValueError: ambiguous — put codec inside DataConverter"]
    A -->|payload_codec + OpenAIAgentsPlugin, no data_converter| C["❌ ValueError: codec would be silently dropped"]
    A -->|data_converter, no plugin| D["connect_kwargs[data_converter] = caller's DataConverter"]
    A -->|data_converter + OpenAIAgentsPlugin| E{"data_converter.payload_converter_class == OpenAIPayloadConverter?"}
    E -->|Yes ✓| F["Plugin transformer passes DataConverter through intact → codec preserved"]
    E -->|No ✗ — NOT checked| G["⚠️ Plugin rebuilds DataConverter with OpenAIPayloadConverter → codec silently dropped"]
    A -->|no data_converter, no plugin| H["connect_kwargs[data_converter] = pydantic/custom_data_converter ± payload_codec"]
    A -->|no data_converter, OpenAIAgentsPlugin, no payload_codec| I["No data_converter set → plugin installs its own"]
Loading

Fix All in Cursor Fix All in Claude Code Fix All in Codex

Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/agentex/lib/core/clients/temporal/utils.py:147-157
**Silent codec-drop still reachable via `data_converter` + `OpenAIAgentsPlugin`**

The PR description explains that the plugin's `_data_converter` transformer only passes a caller-supplied `DataConverter` through unchanged when its `payload_converter_class` is `OpenAIPayloadConverter` (or a subclass). If the caller passes e.g. `DataConverter(payload_codec=my_aes_codec)` — which defaults to `DefaultPayloadConverter` — alongside `OpenAIAgentsPlugin`, the plugin rebuilds a fresh converter (using `OpenAIPayloadConverter`) and the codec is silently dropped, landing payloads in Temporal in plain text. This is the exact failure mode the existing guard was added to prevent.

The same gap exists in `src/agentex/lib/core/temporal/workers/worker.py` at the equivalent block. The tests in `test_payload_codec.py` (`test_data_converter_passthrough_with_openai_plugin`) also pass `DataConverter(payload_codec=_NoopCodec())` — which carries `DefaultPayloadConverter` — but they mock `Client.connect`, so they do not exercise the actual plugin transformer and provide false confidence.

### Issue 2 of 2
src/agentex/lib/types/fastacp.py:75-76
**`TemporalACPConfig` has no mutual-exclusivity guard for `payload_codec` + `data_converter`**

The model already uses `@field_validator` for `plugins` and `interceptors` to surface errors at construction time. Passing both `payload_codec` and `data_converter` to `TemporalACPConfig(...)` today creates a valid model, but the `ValueError` from `get_temporal_client` is only raised later when `TemporalACP` tries to connect. Adding a `model_validator(mode="after")` that mirrors the guard in `get_temporal_client` would catch the misconfiguration at config creation time, consistent with the existing validation pattern in this class.

Reviews (1): Last reviewed commit: "feat(lib): expose data_converter kwarg o..." | Re-trigger Greptile

Greptile also left 2 inline comments on this PR.

…client APIs

Adds a `data_converter` kwarg to:
- `AgentexWorker.__init__` and `worker.get_temporal_client`
- `clients.temporal.utils.get_temporal_client`
- `TemporalClient` / `TemporalACP` / `TemporalACPConfig` / `FastACP`

This unlocks composing `OpenAIAgentsPlugin` with a payload codec by passing
a pre-built `DataConverter(payload_converter_class=OpenAIPayloadConverter,
payload_codec=...)`. Previously the plugin would silently drop a standalone
`payload_codec` kwarg because its `_data_converter(None)` transformer builds
a fresh converter without any codec — the existing guard rejected this
combination outright. The guard is refined: it now fires only when both
the plugin is present AND `payload_codec` is the standalone kwarg AND no
`data_converter` was supplied, and the error message points callers at the
working composition path. An additional guard rejects passing `payload_codec`
and `data_converter` together as ambiguous.

No behavior change for existing callers (none currently pass `data_converter`).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Comment on lines +147 to +157
if data_converter is not None:
# Caller supplied a pre-built converter. With the OpenAI plugin present
# and `payload_converter_class=OpenAIPayloadConverter` (or subclass),
# the plugin's `_data_converter` transformer passes it through intact,
# preserving any payload_codec.
connect_kwargs["data_converter"] = data_converter
elif not has_openai_plugin:
dc = pydantic_data_converter
if payload_codec:
dc = dataclasses.replace(dc, payload_codec=payload_codec)
connect_kwargs["data_converter"] = dc
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Silent codec-drop still reachable via data_converter + OpenAIAgentsPlugin

The PR description explains that the plugin's _data_converter transformer only passes a caller-supplied DataConverter through unchanged when its payload_converter_class is OpenAIPayloadConverter (or a subclass). If the caller passes e.g. DataConverter(payload_codec=my_aes_codec) — which defaults to DefaultPayloadConverter — alongside OpenAIAgentsPlugin, the plugin rebuilds a fresh converter (using OpenAIPayloadConverter) and the codec is silently dropped, landing payloads in Temporal in plain text. This is the exact failure mode the existing guard was added to prevent.

The same gap exists in src/agentex/lib/core/temporal/workers/worker.py at the equivalent block. The tests in test_payload_codec.py (test_data_converter_passthrough_with_openai_plugin) also pass DataConverter(payload_codec=_NoopCodec()) — which carries DefaultPayloadConverter — but they mock Client.connect, so they do not exercise the actual plugin transformer and provide false confidence.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agentex/lib/core/clients/temporal/utils.py
Line: 147-157

Comment:
**Silent codec-drop still reachable via `data_converter` + `OpenAIAgentsPlugin`**

The PR description explains that the plugin's `_data_converter` transformer only passes a caller-supplied `DataConverter` through unchanged when its `payload_converter_class` is `OpenAIPayloadConverter` (or a subclass). If the caller passes e.g. `DataConverter(payload_codec=my_aes_codec)` — which defaults to `DefaultPayloadConverter` — alongside `OpenAIAgentsPlugin`, the plugin rebuilds a fresh converter (using `OpenAIPayloadConverter`) and the codec is silently dropped, landing payloads in Temporal in plain text. This is the exact failure mode the existing guard was added to prevent.

The same gap exists in `src/agentex/lib/core/temporal/workers/worker.py` at the equivalent block. The tests in `test_payload_codec.py` (`test_data_converter_passthrough_with_openai_plugin`) also pass `DataConverter(payload_codec=_NoopCodec())` — which carries `DefaultPayloadConverter` — but they mock `Client.connect`, so they do not exercise the actual plugin transformer and provide false confidence.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Cursor Fix in Claude Code Fix in Codex

Comment on lines 75 to +76
payload_codec: Any = Field(default=None, frozen=True)
data_converter: Any = Field(default=None, frozen=True)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 TemporalACPConfig has no mutual-exclusivity guard for payload_codec + data_converter

The model already uses @field_validator for plugins and interceptors to surface errors at construction time. Passing both payload_codec and data_converter to TemporalACPConfig(...) today creates a valid model, but the ValueError from get_temporal_client is only raised later when TemporalACP tries to connect. Adding a model_validator(mode="after") that mirrors the guard in get_temporal_client would catch the misconfiguration at config creation time, consistent with the existing validation pattern in this class.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agentex/lib/types/fastacp.py
Line: 75-76

Comment:
**`TemporalACPConfig` has no mutual-exclusivity guard for `payload_codec` + `data_converter`**

The model already uses `@field_validator` for `plugins` and `interceptors` to surface errors at construction time. Passing both `payload_codec` and `data_converter` to `TemporalACPConfig(...)` today creates a valid model, but the `ValueError` from `get_temporal_client` is only raised later when `TemporalACP` tries to connect. Adding a `model_validator(mode="after")` that mirrors the guard in `get_temporal_client` would catch the misconfiguration at config creation time, consistent with the existing validation pattern in this class.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Cursor Fix in Claude Code Fix in Codex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant