Skip to content

Fix: Fix state mutation bad practice in content_processor_utils.ts#430

Open
AmaadMartin wants to merge 2 commits into
google:mainfrom
AmaadMartin:feat/fix-content-processor-state-mutation
Open

Fix: Fix state mutation bad practice in content_processor_utils.ts#430
AmaadMartin wants to merge 2 commits into
google:mainfrom
AmaadMartin:feat/fix-content-processor-state-mutation

Conversation

@AmaadMartin

@AmaadMartin AmaadMartin commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

  1. Link to an existing issue (if applicable):

Closes: N/A
Related: N/A
2. Or, if no issue exists, describe the change:

Problem: The rearrangeEventsForLatestFunctionResponse and mergeFunctionResponseEvents functions in content_processor_utils.ts had state mutation issues where the original events list and event content were mutated/modified directly, causing unexpected side effects.

Solution: Cloned the content of the first event during merge, and cloned media parts in convertForeignEvent fallback block to avoid state mutation by reference. Avoided using mutable state variables in rearrangeEventsForLatestFunctionResponse and cleaned up matching variables. Added comprehensive unit tests covering all edges and state mutation checks to guarantee correctness.

Testing Plan
Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.
    Please include a summary of passed npm test results.
  ✓ core/test/agents/processors/content_processor_utils_test.ts  (36 tests) 104ms

 Test Files  1 passed (1)
      Tests  36 passed (36)
   Start at  16:18:53
   Duration  29.34s

 % Statements  100%
 % Branch      100%
 % Functions   100%
 % Lines       100%

Manual End-to-End (E2E) Tests:

Please provide instructions on how to manually test your changes, including any necessary setup or configuration. Please provide logs or screenshots to help reviewers better understand the fix.

To manually verify:

  1. Run the vitest unit tests:
npx vitest run core/test/agents/processors/content_processor_utils_test.ts
  1. Run the targeted integration tests:
npx vitest run --project integration tests/integration/streaming_sse/sse_agent_test.ts
  1. The tests verify that no state mutation is performed on input event contents, that function response merging is correct, and that stream history assembly works end-to-end under StreamingMode.SSE.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.
    Additional context
    Add any other context or screenshots about the feature request here.

@AmaadMartin AmaadMartin force-pushed the feat/fix-content-processor-state-mutation branch from ffa41d6 to 897bbbd Compare June 10, 2026 21:44
@AmaadMartin AmaadMartin force-pushed the feat/fix-content-processor-state-mutation branch from 9e5758d to cc38a19 Compare June 11, 2026 00:05
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.

1 participant