Python: Fix A2AAgent context_providers to skip inactive history providers and suppress type errors

[eavanvalkenburg]

Motivation and Context

A2AAgent's _run_before_providers did not skip BaseHistoryProvider instances when load_messages=False, unlike the behavior in BaseAgent._prepare_session_and_messages and WorkflowAgent. This inconsistency caused unexpected errors when context providers were configured on A2AAgent. Additionally, pyright reported type errors on cross-package private attribute access and chained ResponseStream calls.

Fixes #4754

Description

The root cause was that A2AAgent._run_before_providers unconditionally invoked before_run on all context providers, including BaseHistoryProvider instances with load_messages=False that should be skipped—a pattern already established in the core agent implementations. The fix adds an isinstance check to skip those providers, matching the behavior in BaseAgent and WorkflowAgent. It also adds type suppression comments for SessionContext._response private attribute access and ResponseStream chained method calls, aligning with patterns used elsewhere in the codebase.

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 eavanvalkenburg's agent

[eavanvalkenburg]

Automated Code Review

Reviewers: 4 | Confidence: 90%

✓ Correctness

The diff makes two changes: (1) removes a REPRODUCTION_REPORT.md debugging artifact that should not be committed, and (2) adds a BaseHistoryProvider skip check in A2AAgent._run_before_providers to match the identical pattern in BaseAgent._prepare_session_and_messages. Both changes are correct. The import of BaseHistoryProvider is valid (exported from agent_framework.init), and the skip logic correctly prevents calling before_run on history providers with load_messages=False, consistent with BaseAgent's behavior. The existing tests cover context provider invocation for both streaming and non-streaming paths and should continue to pass since they use BaseContextProvider (not BaseHistoryProvider with load_messages=False).

✓ Security Reliability

The diff makes two changes: (1) removes a debug investigation artifact (REPRODUCTION_REPORT.md) from the repo root, and (2) adds a BaseHistoryProvider skip check in A2AAgent._run_before_providers to match the identical pattern in BaseAgent._prepare_session_and_messages (line 1318-1321 of _agents.py). The code change is correct and consistent with the base class behavior. No security issues (no injection risks, secrets, or unsafe deserialization). There is a pre-existing reliability concern where _run_after_providers is not called if the A2A stream or response processing throws an exception (no try/finally guard), but this is the same pattern used throughout the base class and is not introduced by this diff.

✗ Test Coverage

The diff adds a BaseHistoryProvider skip-when-load_messages=False check to A2AAgent._prepare_session_and_messages(), mirroring the same logic in BaseAgent._prepare_session_and_messages(). However, there are zero tests in the A2A test suite covering this new conditional branch. No test verifies that a BaseHistoryProvider with load_messages=False has its before_run skipped, nor that one with load_messages=True still has before_run called. The user also notes tests are failing on the PR, which should be investigated. Additionally, the REPRODUCTION_REPORT.md deletion is fine—it was a scratch artifact.

✓ Design Approach

The fix correctly mirrors the existing BaseAgent._prepare_session_and_messages pattern, which skips before_run for BaseHistoryProvider instances with load_messages=False while still allowing after_run (for storing). All 50 A2A tests pass locally. The one design concern is a pre-existing abstraction leak — agents encode the load_messages semantics as a skip condition rather than having BaseHistoryProvider.before_run be a no-op when load_messages=False. This leak now exists in three places (BaseAgent, A2AAgent, and the workflow agent). However, since this PR is fixing a consistency bug (A2AAgent not calling providers at all), matching the existing pattern is the right approach. No new blocking issues are introduced by this change.

Flagged Issues

• No test covers the new BaseHistoryProvider with load_messages=False skip logic in A2AAgent._prepare_session_and_messages(). Add at least two tests: (1) a BaseHistoryProvider subclass with load_messages=False should NOT have before_run called, and (2) a BaseHistoryProvider subclass with load_messages=True (the default) should still have before_run called.
• The user reports tests are failing on the PR. The new code path needs to be verified with passing tests before merge. All 50 A2A tests pass locally, so if CI is failing it may be due to the unrelated AgentFrameworkWorkflow import error in ag-ui tests — worth confirming the specific failing test names.

Suggestions

• Consider adding edge-case tests: (1) a mix of BaseContextProvider and BaseHistoryProvider(load_messages=False) verifying the regular provider's before_run is still called while the history provider's is skipped, and (2) verifying after_run is still called for a BaseHistoryProvider with load_messages=False.
• Consider wrapping the non-streaming and streaming paths in try/finally to ensure _run_after_providers is always called even when the A2A stream or response processing throws. This is a pre-existing issue shared with the base class, not introduced by this PR, but worth addressing in a follow-up.
• The load_messages=False skip logic is now duplicated across three agent implementations (BaseAgent, A2AAgent, _workflows/_agent.py). Consider moving this skip into BaseHistoryProvider.before_run itself (making it a no-op when load_messages=False) so agents don't need to know about this flag. This would be a follow-up cleanup, not a blocker for this fix.

---

Automated review by eavanvalkenburg's agents

[Copilot]

Pull request overview

Aligns A2AAgent context provider execution with other agent implementations by skipping inactive history providers and addressing static type-checker complaints in the run() streaming/non-streaming paths.

Changes:

• Refactors A2AAgent.run() to consistently invoke context providers (and adds _run_before_providers() helper).
• Skips BaseHistoryProvider instances when load_messages=False during before_run.
• Adds unit tests validating context provider invocation order and session handling.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
python/packages/a2a/agent_framework_a2a/_agent.py	Refactors run() to run provider hooks and skips inactive history providers; adds pyright suppression for private access and stream chaining.
python/packages/a2a/tests/test_a2a_agent.py	Adds coverage for context provider before_run/after_run invocation (streaming + non-streaming), ordering, and session propagation.

---

You can also share your feedback on Copilot code review. Take the survey.

[eavanvalkenburg]

Automated Code Review

Reviewers: 4 | Confidence: 85%

✓ Correctness

The fix correctly addresses an UnboundLocalError that occurs when self.context_providers is truthy at the guard check on line 314 but active_session and session_context were never assigned (or more accurately, when the variables would be referenced without being assigned in the if self.context_providers block). By initializing both variables to None before the conditional block and adding a session_context is not None guard before accessing it, the code now safely handles all paths. The streaming path already had equivalent guards (lines 328-329). The fix is minimal, correct, and consistent with the streaming path's defensive pattern.

✓ Security Reliability

The fix correctly addresses an UnboundLocalError that occurs when self.context_providers is truthy but _run_before_providers was never called (or failed before assignment). By initializing active_session and session_context to None and adding a session_context is not None guard, the code avoids referencing potentially unbound locals. The streaming path (lines 322-332) already had equivalent None-safety via holder dicts and an explicit if session_context is None: return guard, so this change brings the non-streaming path to parity. No security or reliability issues are introduced by this diff.

✗ Test Coverage

The fix initializes active_session and session_context to None and adds a defensive session_context is not None guard, aligning the non-streaming path with the streaming path's existing pattern (line 329). This is a sound type-safety/defensive fix. However, there is no new test covering the exact edge case this change guards against, and there is a pre-existing gap: no test exercises the non-streaming continuation_token path combined with context_providers, which is the code area being changed. Given the user noted failing tests, adding a targeted test for the non-streaming continuation+providers scenario would both validate the fix and prevent regressions.

✓ Design Approach

The diff correctly pre-initializes active_session and session_context to None before the conditional block to prevent potential UnboundLocalError (and satisfy pyright's 'possibly unbound' analysis), then adds a session_context is not None guard at the post-run call. The approach is sound: context_providers is a plain list attribute (not a property), so it cannot return different values between the two checks in the same invocation, meaning the is not None guard is technically redundant when self.context_providers is truthy—but it is correct and harmless. The underlying pattern (two separate if self.context_providers: guards that are coupled through local variables) is a bit fragile, but the fix is the minimal correct change. No design flaws are introduced. If tests are still failing on the PR, the root cause is likely in other files not shown in this diff (e.g., missing test fixtures, unimplemented streaming path parity, or other changes in the PR that are not reflected here), not in these two lines.

Flagged Issues

• No test covers the non-streaming continuation_token + context_providers combination. The existing test_resume_via_continuation_token uses the default a2a_agent fixture (no context providers), so the modified guard at line 314 is never exercised. A test like test_resume_via_continuation_token_with_context_providers is needed to validate the fix and prevent regressions.

Suggestions

• Add a test for the negative case: an agent with an empty context_providers list called with continuation_token, confirming session_context remains None and no crash occurs. This validates the new is not None guard specifically.
• Consider collapsing both if self.context_providers: blocks into a single try/finally or extracted helper to make the paired before/after contract structurally explicit and harder to break in future edits.

---

Automated review by eavanvalkenburg's agents

[markwallace-microsoft]

Python Test Coverage Report •

File	Stmts	Miss	Cover	Missing
packages/a2a/agent_framework_a2a
_agent.py	226	15	93%	330, 389, 478, 499, 520, 540, 554, 568, 580–581, 623–624, 653–655
TOTAL	24046	2640	89%

Python Unit Test Overview

Tests	Skipped	Failures	Errors	Time
5266	20 💤	0 ❌	0 🔥	1m 22s ⏱️

[eavanvalkenburg]

Automated Code Review

Reviewers: 4 | Confidence: 90%

✓ Correctness

The two new tests are well-structured and follow established patterns already used throughout the test file (e.g., test_resume_via_continuation_token on line 775 and test_run_invokes_context_providers on line 880). The mock setup, assertion patterns, and API usage are all correct. A2AContinuationToken is a TypedDict and the bracket-access pattern matches the production code. The _run_non_streaming path correctly gates context-provider calls behind if self.context_providers:, so the no-providers test should pass without error. If tests are currently failing on this PR, the root cause is likely in production code changes not shown in this diff rather than in these test definitions.

✓ Security Reliability

The diff adds two new test functions for the A2A agent's continuation_token resume flow: one verifying context providers are invoked during resume, and another verifying the agent works without context providers. Both tests are well-structured, use existing mock infrastructure (MockA2AClient.resubscribe_responses), and follow the same patterns as surrounding tests. No security or reliability issues are present — these are pure unit tests with no trust-boundary changes, no deserialization of untrusted data, no resource management concerns, and no secrets. The tests appear consistent with the existing implementation of A2AAgent.run() which calls context provider hooks regardless of whether a continuation_token is used.

✓ Test Coverage

The two new tests cover continuation-token resume paths with and without context providers. They are structurally sound and follow existing patterns in the file (TrackingContextProvider, MockA2AClient.resubscribe_responses). The assertions are meaningful, checking provider invocation flags and response content. However, the streaming resume path with context providers is not tested by these new tests (only the non-streaming path is), representing a gap. Also, the 'no providers' test does not verify that mock_a2a_client.call_count changed, which would confirm the resubscribe path was actually exercised rather than silently short-circuiting.

✓ Design Approach

The two new tests verify that context providers are correctly invoked (before_run / after_run) on the continuation-token / resubscribe code path, and that the no-providers path works without crashing. All 57 tests in the suite pass. The tests are well-targeted: they reuse the existing TrackingContextProvider and MockA2AClient infrastructure, and they test real observable behavior (context provider lifecycle on the resume path) rather than masking or papering over an underlying issue. No design concerns were found.

Suggestions

• Consider adding a test for the streaming resume path (stream=True) with context providers to match the non-streaming coverage added here.
• In test_resume_via_continuation_token_no_context_providers, assert mock_a2a_client.call_count == 1 to verify the resubscribe path was actually invoked, similar to how existing tests validate interaction with the mock client.

---

Automated review by eavanvalkenburg's agents