feat(lib): expose data_converter kwarg on AgentexWorker and Temporal client APIs#372
feat(lib): expose data_converter kwarg on AgentexWorker and Temporal client APIs#372matteolibrizzi-scale wants to merge 1 commit into
Conversation
…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>
| 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 |
There was a problem hiding this 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.
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.| payload_codec: Any = Field(default=None, frozen=True) | ||
| data_converter: Any = Field(default=None, frozen=True) |
There was a problem hiding this 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.
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.
Summary
Adds a
data_converterkwarg to the AgentexWorker / TemporalClient / TemporalACP API surface so callers can composeOpenAIAgentsPluginwith a payload codec (e.g. AES-GCM at-rest encryption).Why
Today, passing
payload_codec=...toAgentexWorker(...)together withOpenAIAgentsPluginraises 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 freshDataConverterand the user's codec is silently dropped — payloads land in Temporal in plain text.But the composition does work if you pre-build a
DataConverterwith bothpayload_converter_class=OpenAIPayloadConverterANDpayload_codec=...and pass it viaClient.connect(data_converter=...). The plugin's transformer has a fourth branch that passes through anyDataConverterwhose payload converter is anOpenAIPayloadConverterinstance (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 asinvoke_model_activityunder Temporal) and the sharedemu_shared.encryption.codec.get_payload_codec()already used by OneEdge / FDD / VDR.What changed
New
data_converterkwarg on:agentex.lib.core.temporal.workers.worker.get_temporal_clientandAgentexWorker.__init__(worker side)agentex.lib.core.clients.temporal.utils.get_temporal_client(client side)TemporalClient.__init__/TemporalClient.createTemporalACP.__init__/TemporalACP.createTemporalACPConfig(forwarded byFastACP.create_async_acp)Guards in both
get_temporal_clientimplementations:payload_codecANDdata_convertertogether raiseValueError(the codec belongs inside the data converter).OpenAIAgentsPluginis present ANDpayload_codecis the standalone kwarg AND nodata_converterwas 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
Test plan
tests/lib/passtests/lib/test_payload_codec.pyfor bothget_temporal_clientpaths:data_converterpass-through withOpenAIAgentsPlugindata_converterpass-through without pluginpayload_codec+data_convertertogether raises ambiguity errordata_converterAgentexWorker,TemporalClient,TemporalACP,TemporalACPConfig,FastACPstoring/forwardingdata_converterdata_converterappears on all five public surfaces🤖 Generated with Claude Code
Greptile Summary
This PR exposes a
data_converterkwarg across theAgentexWorker,TemporalClient,TemporalACP,TemporalACPConfig, andFastACPAPI surfaces so callers can composeOpenAIAgentsPluginwith an encrypted payload codec by pre-building aDataConverter(payload_converter_class=OpenAIPayloadConverter, payload_codec=...)and passing it directly.payload_codec + data_converter → ValueError) and a refined silent-drop guard (payload_codec + OpenAIAgentsPlugin → ValueErroronly whendata_converteris absent) are added to bothget_temporal_clientimplementations.data_converterfield is threaded through all five public API surfaces with correct forwarding;TemporalACPConfiggains the field with a good docstring but no Pydantic cross-field validator for mutual exclusivity withpayload_codec.data_converterpath withOpenAIAgentsPluginhas no validation that the supplied converter usesOpenAIPayloadConverteras itspayload_converter_class; passingDataConverter(payload_codec=my_codec)(which defaults toDefaultPayloadConverter) alongside the plugin will still silently drop the codec via the plugin's transformer, and the new tests do not catch this becauseClient.connectis 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 aDataConverterwith any otherpayload_converter_classalongsideOpenAIAgentsPluginwill 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_classstill silently drops encryption, and the new tests pass the wrong converter class themselves but don't catch it becauseClient.connectis mocked.Both
get_temporal_clientimplementations (src/agentex/lib/core/clients/temporal/utils.pyandsrc/agentex/lib/core/temporal/workers/worker.py) need a guard fordata_converter.payload_converter_classwhenOpenAIAgentsPluginis present. Thetest_data_converter_passthrough_with_openai_plugintests intests/lib/test_payload_codec.pyshould also be updated to useOpenAIPayloadConverteras the converter class.Important Files Changed
data_converterkwarg toget_temporal_client; guards for payload_codec+data_converter ambiguity and OpenAI-plugin+payload_codec silent-drop are correct, but the newdata_converterpath lacks a guard that validatespayload_converter_class=OpenAIPayloadConverterwhenOpenAIAgentsPluginis present — leaving the original silent-drop risk reachable via the new kwarg.utils.pychanges for the worker-sideget_temporal_client; carries the same missing validation fordata_converter.payload_converter_classwhenOpenAIAgentsPluginis present.data_converterthroughTemporalClient.__init__,create, and_get_temporal_clientcorrectly; no issues.data_converter: Any = Field(default=None, frozen=True)and good docstring; missing a cross-fieldmodel_validatorto enforce mutual exclusivity withpayload_codecat construction time.test_data_converter_passthrough_with_openai_plugincreatesDataConverter(payload_codec=_NoopCodec())(withDefaultPayloadConverter) — the wrong class for use withOpenAIAgentsPlugin— and the bug is masked becauseClient.connectis 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"]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(lib): expose data_converter kwarg o..." | Re-trigger Greptile