Skip to content

test(a2a): Prevent peer action allow-list regressions - #6534

Open
ch4n3-yoon wants to merge 1 commit into
google:mainfrom
ch4n3-yoon:test/a2a-peer-action-deny-coverage
Open

test(a2a): Prevent peer action allow-list regressions#6534
ch4n3-yoon wants to merge 1 commit into
google:mainfrom
ch4n3-yoon:test/a2a-peer-action-deny-coverage

Conversation

@ch4n3-yoon

Copy link
Copy Markdown

Link to Issue or Description of Change

2. Or, if no issue exists, describe the change:

Problem:

_extract_event_actions rebuilds only escalate and skip_summarization from a remote peer's
response metadata; the other twelve EventActions fields are dropped.
test_peer_supplied_actions_cannot_mutate_caller_session asserts five of those twelve
(state_delta, artifact_delta, transfer_to_agent, agent_state,
rewind_before_invocation_id), and no test checks the allow-list contract itself. Two regressions
therefore pass today:

  • adding one of the seven unasserted fields to _PEER_SETTABLE_ACTION_FIELDS — e.g. route;
  • dropping one of the accepted spellings. EventActions sets populate_by_name=True, so a peer can
    send either skip_summarization or skipSummarization and both have to stay listed.

Solution:

  • Assert all twelve dropped fields in the converter test, across all four converters.
  • Validate the raw metadata payload before the assertions and require every denied field to come back
    non-default. Without that step a field could assert clean because schema drift made its value
    invalid, rather than because the allow-list filtered it out.
  • Add test_peer_settable_action_fields_are_exactly_inert, pinning _PEER_SETTABLE_ACTION_FIELDS to
    exactly the two inert fields and their aliases.

Tests only; no behavior change.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.

pytest tests/unittests/a2a/407 passed, 19 skipped (406 passed before this change).
pyink --check and isort --check-only are clean on the changed file.

Both regressions above were mutation-checked. Each mutation was applied to
_PEER_SETTABLE_ACTION_FIELDS, then the peer tests were run against main and against this branch:

mutation on main with this change
add "route" to the allow-list passes test_peer_supplied_actions_cannot_mutate_caller_session and test_peer_settable_action_fields_are_exactly_inert fail
remove "skip_summarization", keeping only the camelCase alias passes test_peer_settable_action_fields_are_exactly_inert fails

Manual End-to-End (E2E) Tests:

Not applicable — tests only, no runtime behavior change.

_extract_event_actions rebuilds only escalate and skip_summarization
from a remote peer's response metadata; the other twelve EventActions
fields are dropped. The regression test asserts five of those twelve,
and no test checks the allow-list contract itself, so adding one of
the seven unasserted fields to it, or dropping one of the accepted
spellings, still passes.

Assert all twelve dropped fields in the converter test, and validate
the raw payload first so that a field cannot assert clean because
schema drift made its value invalid rather than because the allow-list
filtered it out.

Add a test pinning _PEER_SETTABLE_ACTION_FIELDS to exactly the inert
fields and their aliases, since populate_by_name leaves both spellings
reachable.
@adk-bot adk-bot added the core [Component] This issue is related to the core interface and implementation label Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core [Component] This issue is related to the core interface and implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants