Python: Fix A2AAgent to use session.service_session_id as authoritative context_id

[giles17]

Motivation and Context

A2AAgent extracted context_id from message.additional_properties instead of using AgentSession.service_session_id, which is the intended authoritative source for session identity. This broke session consistency when both were present, as the message property incorrectly took precedence.

Fixes #5353

Description

The root cause was in _prepare_message_for_a2a, which unconditionally read context_id from message.additional_properties with no way to pass in the session-level identifier. The fix adds a context_id keyword argument to _prepare_message_for_a2a (sourced from session.service_session_id at the call site in run()) and changes the resolution order so the session-level value takes precedence, falling back to additional_properties["context_id"] only when no session identifier is available. Tests cover the precedence behavior, the fallback path, and full integration through run().

Contribution Checklist

• The code builds clean without any errors or warnings
• The PR follows the Contribution Guidelines
• All unit tests pass, and I have added new tests where possible
• Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.

---

Note: PR autogenerated by giles17's agent

[giles17]

Automated Code Review

Reviewers: 4 | Confidence: 94%

✓ Correctness

The diff correctly flips the precedence of context_id in _prepare_message_for_a2a() so that the session-level identifier (passed via the context_id keyword argument, sourced from AgentSession.service_session_id at line 300) takes priority over the per-message additional_properties["context_id"]. The code change is a single-line swap from message.additional_properties.get("context_id") or context_id to context_id or message.additional_properties.get("context_id"), the docstring is updated to match, and the tests are correctly updated to assert the new precedence. A new test (test_run_message_context_id_used_when_no_session) covers the fallback path where no session is provided. The or short-circuit semantics are correct for this use case since both sides are either None or a non-empty string.

✓ Security Reliability

This is a clean, minimal, and correct fix. The change flips the precedence of context_id in _prepare_message_for_a2a() so that the session-derived context_id parameter (from AgentSession.service_session_id) takes priority over message.additional_properties["context_id"]. The call site at line 300 correctly passes session.service_session_id if session else None, so the session is now the authoritative source. The docstring update matches the new behavior. Tests are well-structured: they cover session-wins-over-message, fallback-when-no-session, and the existing fallback-when-no-additional-properties case. No security or reliability concerns — no new trust boundaries, no injection surface, no resource leaks.

✓ Test Coverage

The code change correctly flips the precedence so that context_id from the session takes priority over message.additional_properties. The existing tests were updated to match the new behavior, and a new integration test (test_run_message_context_id_used_when_no_session) covers the backward-compatible fallback path. However, one edge case is missing: when a session is provided but its service_session_id is None, the fallback to additional_properties["context_id"] should still work. This scenario is untested at the integration level.

✓ Design Approach

The change is correct, minimal, and addresses the right root cause. Flipping the precedence order from message.additional_properties.get('context_id') or context_id to context_id or message.additional_properties.get('context_id') makes the session-level service_session_id the authoritative source of truth for context_id, while retaining additional_properties as a backward-compatible fallback. The call site at line 300 already threads session.service_session_id in as context_id, so no further plumbing changes are needed. Tests are updated to match the new semantics and a new test covers the fallback path. No design issues found.

Suggestions

• Add a test for the case where a session is provided but service_session_id is None — this should still fall back to message.additional_properties["context_id"]. Currently test_run_message_context_id_used_when_no_session only tests the no-session path, but AgentSession(service_session_id=None) passed as session is a different and realistic scenario (line 300 of _agent.py evaluates to None when session.service_session_id is None).

---

Automated review by giles17's agents

[Copilot]

Pull request overview

This PR fixes A2AAgent session consistency by using AgentSession.service_session_id as the authoritative A2A context_id, rather than letting message.additional_properties["context_id"] override it.

Changes:

• Update A2AAgent.run() to pass session.service_session_id into _prepare_message_for_a2a() as the primary context_id.
• Change _prepare_message_for_a2a() to prefer the session-derived context_id and only fall back to message.additional_properties["context_id"] when no session context is available.
• Update/add tests to verify precedence (session wins), fallback behavior (message used when no session), and integration through run().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
python/packages/a2a/agent_framework_a2a/_agent.py	Ensures session-level service_session_id is used as the authoritative A2A context_id and adjusts precedence resolution.
python/packages/a2a/tests/test_a2a_agent.py	Updates/extends unit + integration tests to cover new precedence and fallback behavior.

[moonbox3]

Python Test Coverage Report •

File	Stmts	Miss	Cover	Missing
packages/a2a/agent_framework_a2a
_agent.py	238	16	93%	355, 360, 362, 521, 538, 546, 567, 588, 617, 637, 651, 665, 677–678, 718–719
TOTAL	29030	3474	88%

Python Unit Test Overview

Tests	Skipped	Failures	Errors	Time
5784	30 💤	0 ❌	0 🔥	1m 33s ⏱️

[giles17]

Automated Code Review

Reviewers: 4 | Confidence: 92%

✓ Correctness

The change correctly fixes the context_id resolution logic. Previously, context_id or message.additional_properties.get('context_id') would incorrectly fall through to the additional_properties fallback when context_id was an empty string ". The new context_id if context_id is not None else ... properly distinguishes between 'not provided' (None) and 'explicitly set' (including empty string). More importantly, it fixes the case where a session exists but has no service_session_id — the call site at line 300 passes session.service_session_id if session else None, which yields None when the session has no service_session_id, and the old truthy check would correctly fall through. However, the semantic intent is clearer with is not None, and the new test at line 955 properly covers the edge case. The change is minimal, correct, and well-tested.

✓ Security Reliability

The diff changes the context_id fallback logic from truthiness-based (or) to an explicit None check (is not None), which is semantically more correct. With the old code, an empty string service_session_id would be silently overidden by the additional_properties fallback; the new code preserves any explicitly-set value. For the common case (None vs actual string), both old and new code behave identically. The new test correctly validates the scenario where a session exists but service_session_id is None, ensuring the fallback to additional_properties still works. No security, reliability, or resource leak issues found.

✗ Test Coverage

The code change from or to is not None is correct and prevents falsy-but-valid values (like empty string) from incorrectly falling through to the additional_properties fallback. However, the new test only covers the service_session_id=None case, which produces identical behavior under both the old and new code. The distinguishing edge case — where service_session_id is an empty string " — is not tested, meaning the new test doesn't actually validate the behavioral change introduced by this PR.

✓ Design Approach

The or → is not None change at line 686 is a correct precision fix: when service_session_id is an empty string ", the old or operator would incorrectly discard it and fall back to additional_properties, while is not None preserves it. The new test, however, only exercises the None path (an AgentSession() with no service_session_id), which behaved identically under both the old and new code. The actual changed behaviour — preserving an empty string as a valid context_id — is not covered by the new test.

Flagged Issues

• The new test test_run_message_context_id_used_when_session_has_no_service_id does not exercise the behavioral change. When service_session_id=None, both None or fallback and None if None is not None else fallback evaluate to fallback. A test with service_session_id="" is needed to distinguish the new is not None logic from the old or logic.

Suggestions

• Add a companion test where AgentSession(service_session_id="") is passed and verify that the empty string is preserved as the context_id (not overridden by additional_properties). This is the only scenario where the behavior actually differs between the old and new code, and it would have failed before this fix.

---

Automated review by giles17's agents

[giles17]

Automated Code Review

Reviewers: 4 | Confidence: 92%

✓ Correctness

The new test correctly validates that an empty string service_session_id is preserved (not overridden by additional_properties fallback). The implementation at _agent.py:686 uses context_id is not None, which correctly distinguishes empty string from None. The test follows the exact same pattern as the adjacent session context_id tests. No issues found.

✓ Security Reliability

This diff adds a single test case that validates an edge case: when AgentSession.service_session_id is an empty string, it should be preserved as the context_id rather than being overridden by additional_properties. The test is well-structured, consistent with neighboring tests, and correctly exercises the existing code path (empty string is not None, so it passes the is not None check in _prepare_message_for_a2a). No security or reliability concerns.

✓ Test Coverage

The new test test_run_empty_service_session_id_preserved is a well-constructed edge case test that validates the is not None check at _agent.py:686. It ensures an empty string service_session_id is preserved rather than falling back to additional_properties['context_id']. The assertions are meaningful (checking exact value " rather than just no-exception). The ``@mark.asyncio decorator is unnecessary given `asyncio_mode = "auto"` in `pyproject.toml:44`, but it is consistent with the four sibling tests in this section (lines 907, 920, 938, 955), so this is not a blocking concern. Overall the context_id test suite provides good coverage of the priority hierarchy: session id present > fallback to additional_properties > session exists but service_session_id is None > empty string preserved.

✗ Design Approach

The test documents and locks in a questionable design choice: treating AgentSession(service_session_id=") as an intentionally set, valid context_id that overides the additional_properties fallback. The implementation's is not None guard (line 686 of _agent.py) causes an empty string to be forwarded to the A2A server as context_id=", which is not a meaningful A2A identifier. The existing test at line 955–970 already covers the correct fallback path for the unset case (AgentSession() with service_session_id=None); the semantically equivalent case of service_session_id="" should behave identically. The ``@mark.asyncio decorator is also unnecessary given `asyncio_mode = "auto"` in pyproject.toml, consistent with the rest of the test file (the prior four tests in this region also carry the redundant decorator, so this is a pre-existing inconsistency rather than a new one).

Flagged Issues

• The test asserts that AgentSession(service_session_id="") produces context_id="" on the outgoing A2A message, even when a non-empty fallback exists in additional_properties. An empty string is not a valid A2A context identifier and service_session_id="" is semantically 'not set', identical to None. The implementation at _agent.py:686 uses is not None, so this test encodes and pins a bug: the A2A client will send context_id="" to the remote server. The fix should use a truthiness check instead (context_id if context_id else …) and this test should instead assert that service_session_id="" falls back to additional_properties["context_id"], matching the None behaviour already tested at line 955–970.

Suggestions

• The ``@mark.asyncio decorator on all five session context_id integration tests (lines 907, 920, 938, 955, 973) is redundant since `asyncio_mode = "auto"` is configured in `pyproject.toml:44`. Consider removing all five decorators in a follow-up for consistency with the rest of the test file, which relies on auto-detection.
• The ``@mark.asyncio decorator on line 973 is redundant because `asyncio_mode = "auto"` is configured in pyproject.toml. This is consistent with how the four preceding tests (lines 907, 920, 938, 955) were written, but consider cleaning up all five decorators in a follow-up.

---

Automated review by giles17's agents