fix: ensure correct task/contextId in emitted Messages#976
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds validation checks to AgentEmitter.sendMessage to ensure that non-null message task and context IDs match those of the emitter, throwing an IllegalArgumentException on mismatch. Corresponding unit tests have been added to verify these scenarios. The review feedback suggests increasing the dequeue timeout in the tests from 1000ms to 5000ms to avoid flakiness in asynchronous environments, and adding explicit assertions to verify that the dequeued message has null IDs in the null-ID test case.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| public void sendMessageWithMatchingIdsSucceeds() throws Exception { | ||
| agentEmitter.sendMessage(SAMPLE_MESSAGE); | ||
|
|
||
| EventQueueItem item = eventQueue.dequeueEventItem(1000); |
There was a problem hiding this comment.
The test uses a timeout of 1000 ms for dequeueEventItem. Since event distribution via MainEventBusProcessor is asynchronous, a 1-second timeout can lead to flaky test failures in slower CI/CD environments. Consider using 5000 ms to match the timeout used in other tests in this class.
| EventQueueItem item = eventQueue.dequeueEventItem(1000); | |
| EventQueueItem item = eventQueue.dequeueEventItem(5000); |
| EventQueueItem item = eventQueue.dequeueEventItem(1000); | ||
| assertNotNull(item); | ||
| assertInstanceOf(Message.class, item.getEvent()); | ||
| } |
There was a problem hiding this comment.
The test uses a timeout of 1000 ms for dequeueEventItem and only asserts that the event is a Message. Since this test specifically verifies sending a message with null IDs, we should also assert that the enqueued message indeed has null task and context IDs to make the test more robust. Additionally, increasing the timeout to 5000 ms prevents flakiness in asynchronous event distribution.
EventQueueItem item = eventQueue.dequeueEventItem(5000);
assertNotNull(item);
assertInstanceOf(Message.class, item.getEvent());
Message dequeuedMessage = (Message) item.getEvent();
assertNull(dequeuedMessage.taskId());
assertNull(dequeuedMessage.contextId());
}
No description provided.