Skip to content

fix: ensure correct task/contextId in emitted Messages#976

Merged
jmesnil merged 1 commit into
a2aproject:mainfrom
kabir:checkEmittedIds
Jul 9, 2026
Merged

fix: ensure correct task/contextId in emitted Messages#976
jmesnil merged 1 commit into
a2aproject:mainfrom
kabir:checkEmittedIds

Conversation

@kabir

@kabir kabir commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

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.

Suggested change
EventQueueItem item = eventQueue.dequeueEventItem(1000);
EventQueueItem item = eventQueue.dequeueEventItem(5000);

Comment on lines +456 to +459
EventQueueItem item = eventQueue.dequeueEventItem(1000);
assertNotNull(item);
assertInstanceOf(Message.class, item.getEvent());
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

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());
    }

Comment thread server-common/src/main/java/org/a2aproject/sdk/server/tasks/AgentEmitter.java Outdated
Comment thread server-common/src/main/java/org/a2aproject/sdk/server/tasks/AgentEmitter.java Outdated
@kabir kabir force-pushed the checkEmittedIds branch from 3386952 to d7cd22b Compare July 9, 2026 11:43
@jmesnil jmesnil merged commit b6e2b87 into a2aproject:main Jul 9, 2026
12 checks passed
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.

2 participants