test(a2a): Prevent peer action allow-list regressions - #6534
Open
ch4n3-yoon wants to merge 1 commit into
Open
Conversation
_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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Link to Issue or Description of Change
2. Or, if no issue exists, describe the change:
Problem:
_extract_event_actionsrebuilds onlyescalateandskip_summarizationfrom a remote peer'sresponse metadata; the other twelve
EventActionsfields are dropped.test_peer_supplied_actions_cannot_mutate_caller_sessionasserts 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 regressionstherefore pass today:
_PEER_SETTABLE_ACTION_FIELDS— e.g.route;EventActionssetspopulate_by_name=True, so a peer cansend either
skip_summarizationorskipSummarizationand both have to stay listed.Solution:
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.
test_peer_settable_action_fields_are_exactly_inert, pinning_PEER_SETTABLE_ACTION_FIELDStoexactly the two inert fields and their aliases.
Tests only; no behavior change.
Testing Plan
Unit Tests:
pytest tests/unittests/a2a/→ 407 passed, 19 skipped (406 passed before this change).pyink --checkandisort --check-onlyare 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 againstmainand against this branch:main"route"to the allow-listtest_peer_supplied_actions_cannot_mutate_caller_sessionandtest_peer_settable_action_fields_are_exactly_inertfail"skip_summarization", keeping only the camelCase aliastest_peer_settable_action_fields_are_exactly_inertfailsManual End-to-End (E2E) Tests:
Not applicable — tests only, no runtime behavior change.