wenjin272 commented on PR #631:
URL: https://github.com/apache/flink-agents/pull/631#issuecomment-4313615819
### Code review
Found 3 issues:
1. **`ChatRequestEvent.getMessages()` — unsafe cast after JSON roundtrip**:
The getter returns `(List<ChatMessage>) getAttr("messages")` with no
deserialization. After any JSON roundtrip (e.g., cross-language delivery via
`sendEventJson`, or `EventLogRecord` deserialization), the
`attributes["messages"]` entry is a `List<Map>`, not a `List<ChatMessage>`. Any
action that calls `event.getMessages()` on a deserialized event will throw
`ClassCastException`.
https://github.com/apache/flink-agents/blob/916518b02162a31e4f6cb97b26fd36ffa1db4aff/api/src/main/java/org/apache/flink/agents/api/event/ChatRequestEvent.java#L81-L85
2. **`ChatResponseEvent.getResponse()` — same unsafe cast**: `getResponse()`
returns `(ChatMessage) getAttr("response")` without checking whether the stored
object is already a `ChatMessage` or is a deserialized `Map`. The `fromEvent()`
factory handles this correctly with an `instanceof Map` branch, but the plain
getter does not, so it will throw `ClassCastException` on any deserialized
event (e.g., after a Python action sends the event and Java receives it via
`sendEventJson`).
https://github.com/apache/flink-agents/blob/916518b02162a31e4f6cb97b26fd36ffa1db4aff/api/src/main/java/org/apache/flink/agents/api/event/ChatResponseEvent.java#L83-L86
3. **Custom event subclasses store data in private fields, not
`attributes`** — field data is silently dropped during JSON serde:
`AsyncProcessedEvent.processedResult`,
`FlinkIntegrationAgent.ProcessedEvent.itemRef`, and
`MemoryObjectAgent.MyEvent.value` are stored in private final fields, not in
the `attributes` map. Jackson only serializes `id`/`type`/`attributes` from the
base `Event`. When these events pass through the EventLog JSON serde path or
cross the language boundary, the private fields serialize as missing and
deserialize as `null`, causing NPEs when `getProcessedResult()` /
`getItemRef()` / `getValue()` are called on the reconstructed object.
https://github.com/apache/flink-agents/blob/916518b02162a31e4f6cb97b26fd36ffa1db4aff/e2e-test/flink-agents-end-to-end-tests-integration/src/test/java/org/apache/flink/agents/integration/test/AsyncExecutionAgent.java#L74-L86
https://github.com/apache/flink-agents/blob/916518b02162a31e4f6cb97b26fd36ffa1db4aff/e2e-test/flink-agents-end-to-end-tests-integration/src/test/java/org/apache/flink/agents/integration/test/FlinkIntegrationAgent.java#L62-L76
🤖 Generated with [Claude Code](https://claude.ai/code)
<sub>- If this code review was useful, please react with 👍. Otherwise, react
with 👎.</sub>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
