Fix: wire the code-execution response processor into the LlmAgent defaults

[AmaadMartin]

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: #issue_number
   Related: #issue_number
2. Or, if no issue exists, describe the change:

Problem: LlmAgent builds its request-side and response-side processor lists asymmetrically. The default requestProcessors list includes CODE_EXECUTION_REQUEST_PROCESSOR, but responseProcessors defaulted to a bare empty array (config.responseProcessors ?? []). The matching CodeExecutionResponseProcessor was fully implemented but nothing ever added it to an agent.

The result was a half-wired feature. An agent configured with only codeExecutor:

const agent = new LlmAgent({
  model,
  name: 'coderAgent',
  codeExecutor: new UnsafeLocalCodeExecutor(),
});

...would run the request-side processor, get a fenced code block back from the model, then iterate an empty responseProcessors list in postprocess. The code was never extracted, executeCode was never called, no execution-result event was emitted, and the raw markdown code block was surfaced to the user as the final response. Getting code execution to work required the caller to hand-assemble responseProcessors, which is undiscoverable and is not required by the Python SDK.

The only existing end-to-end test of code execution (tests/integration/agents/agent_with_sandbox_executor_test.ts) did not catch this, because it was also the only place that supplied the missing wiring by hand.

Solution: Default responseProcessors to [CODE_EXECUTION_RESPONSE_PROCESSOR].

1. The default response processor list now contains the code-execution response processor, matching adk-python. SingleFlow._create_response_processors() in flows/llm_flows/single_flow.py returns [_nl_planning.response_processor, _code_execution.response_processor], and SingleFlow.__init__ adds it to every flow (AutoFlow inherits it). So in Python the code-execution processor is unconditionally part of the default response pipeline for every LlmAgent, exactly as _code_execution.request_processor is unconditionally part of the default request pipeline. Python drives both sides from one place; adk-js drove only one. This aligns the two. (Only the code-execution half is ported here — adk-js has no NL-planning response processor to add.)

2. Why the wiring is unconditional rather than gated on config.codeExecutor. Gating it would mirror the conditional wiring used a few lines above for the context compactor, but the compactor is conditional for a reason that does not apply here: it is a stateful per-agent instance that must be constructed from the caller's list (new ContextCompactorRequestProcessor(config.contextCompactors)), so there is genuinely nothing to add unless the config supplies one. The code-execution response processor is a stateless module singleton that self-guards — runPostProcessor returns early for a non-LlmAgent, for no codeExecutor, for a non-BaseCodeExecutor, for an absent llmResponse.content, and for BuiltInCodeExecutor; runAsync additionally returns early for partial streaming responses. That is exactly the shape of CODE_EXECUTION_REQUEST_PROCESSOR, which already sits in the unconditional default list one field above. Making one side conditional and the other unconditional would reintroduce the same asymmetry in a new form, add a branch with no observable effect, and diverge from Python. Unconditional is both the parity answer and the smaller diff. Test 2 below asserts the processor is present even with no executor, so a future "optimization" to a conditional has to justify itself against a failing test.

3. Rename: responseProcessor -> CODE_EXECUTION_RESPONSE_PROCESSOR. The singleton was exported under an extremely generic name. It is now referenced from llm_agent.ts directly next to CODE_EXECUTION_REQUEST_PROCESSOR, and every other processor singleton in the repo uses a specific SCREAMING_SNAKE_CASE name (BASIC_LLM_REQUEST_PROCESSOR, CONTENT_REQUEST_PROCESSOR, INTERACTIONS_REQUEST_PROCESSOR, AGENT_TRANSFER_LLM_REQUEST_PROCESSOR, TOOL_FILTER_REQUEST_PROCESSOR). This symbol is not part of the published @google/adk export surface — core/package.json declares only the "." export and the symbol is not re-exported from core/src/index.ts or core/src/common.ts — so this is not a breaking change for npm consumers. It is breaking for anyone deep-importing @google/adk/agents/processors/code_execution_request_processor.js; the only such consumer in the repo is the integration test, updated here. No deprecated alias is kept. core/src/index.ts and core/src/common.ts are deliberately untouched: promoting an internal symbol to public API is a separate decision and does not belong in this fix.

4. Behavioural change worth calling out. RunSkillScriptTool and RunSkillInlineScriptTool fall back to agent.codeExecutor when the toolset has none. An agent that sets codeExecutor only to power those skill tools will now also auto-execute fenced code blocks the model writes in free text. This is precisely how adk-python already behaves for the same configuration, so it is a parity alignment rather than a regression — but it is a real change in observable behaviour for that configuration. Callers who want the old behaviour can pass responseProcessors: [] explicitly; ?? (never ||) is used so an explicit empty array is respected.

Not affected: agents using BuiltInCodeExecutor (server-side execution), including the CFC path where Runner swaps one in — the processor returns early for those. Not affected: partial streaming responses. Every other agent pays one extra guarded generator call per non-partial response; the guard is four cheap checks.

Collision check. Per contribution hygiene I checked the 386 open PRs on this fork before writing anything. Two are adjacent and are disclosed here rather than silently duplicated:

• Feat: Wire code executor into LlmAgent by default #42 "Feat: Wire code executor into LlmAgent by default" proposes the same one-line default. This PR differs in that it also does the singleton rename, uses createSession() and a real ScopedArtifactService/InMemoryArtifactService in the unit test instead of as unknown as Session cast literals and a hand-rolled BaseArtifactService stub, and adds the default-wiring case to the existing sandbox integration test instead of a new file. If Feat: Wire code executor into LlmAgent by default #42 lands first, this PR reduces to the rename plus the tests.
• Fix: export CODE_EXECUTION_RESPONSE_PROCESSOR from the @google/adk public API #460 "Fix: export CODE_EXECUTION_RESPONSE_PROCESSOR from the @google/adk public API" performs the same rename and additionally adds the symbol to core/src/common.ts. Whichever of the two lands second will conflict on the singleton declaration and on the integration-test import; the conflict is mechanical. Note that this PR intentionally does not add the public export.

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:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.

New describe('LlmAgent Default Response Processors') in core/test/agents/llm_agent_test.ts, placed immediately after its structural sibling describe('LlmAgent Default Request Processors'):

1. Wiring, executor set — new LlmAgent({name, model, codeExecutor}) contains CODE_EXECUTION_RESPONSE_PROCESSOR.
2. Wiring, no executor — new LlmAgent({name}) still contains it, documenting the unconditional-by-design choice.
3. Explicit override wins — new LlmAgent({name, responseProcessors: []}) has length 0, pinning the ??-not-|| behaviour.
4. Behavioural, end to end through agent.runAsync — an LlmAgent with only codeExecutor set, driven by a queue-backed SequencedMockLlm (the file's existing MockLlm returns the same response on every call, which would loop forever here: after an execution the response content is cleared and the result event is not a final response, so the agent calls the model again). Asserts executeCode was called exactly once with code === 'print("hello")', that exactly one emitted part carries a codeExecutionResult with outcome === Outcome.OUTCOME_OK and text containing hello, and that the final event text is 'Execution finished.'. It does not assert on event count alone, since a count does not prove execution happened.

Failure paths and negative cases covered: the no-executor path (test 2 constructs an agent the processor must no-op for), the explicit-override path (test 3), and the pre-existing early-return guards, which remain exercised by the 8 untouched tests in core/test/agents/processors/code_execution_request_processor_test.ts (partial response, non-LlmAgent, no executor, BuiltInCodeExecutor).

Proof the tests can fail (mutation). Reverted only the llm_agent.ts hunk — this.responseProcessors = config.responseProcessors ?? [CODE_EXECUTION_RESPONSE_PROCESSOR]; back to ?? []; — leaving the rename in place, and re-ran. 3 of the 4 new unit tests failed:

× includes CODE_EXECUTION_RESPONSE_PROCESSOR when a codeExecutor is set
  → AssertionError: expected [] to include CodeExecutionResponseProcessor{}
× includes CODE_EXECUTION_RESPONSE_PROCESSOR when no codeExecutor is set
  → AssertionError: expected [] to include CodeExecutionResponseProcessor{}
✓ uses caller-supplied responseProcessors verbatim, including an empty list
× executes a model-emitted code block with only codeExecutor configured
  → AssertionError: expected [] to have a length of 1 but got +0
Tests  3 failed | 1 passed | 27 skipped (31)

The third failure is expect(executor.calls).toHaveLength(1) — i.e. executeCode was never invoked, which is the reported bug exactly. Test 3 passes on the reverted source, as expected: it pins the override path, which the fix does not change. The new integration case also fails on the reverted source: AssertionError: expected "spy" to be called 1 times, but got 0 times. The source was restored and all tests re-run green afterwards.

Coverage of new source lines: 100%. Both branches of the new ?? are exercised — the assignment statement is hit 31 times and the default array literal 29 times across the file's tests, so 2 constructions took the caller-supplied branch. (Whole-file percentages for llm_agent.ts are lower, but that reflects pre-existing untested code in a 1,288-line file when only targeted test files are run, not uncovered new lines.)

Manual End-to-End (E2E) Tests:

tests/integration/agents/agent_with_sandbox_executor_test.ts now has two cases. The existing case is kept unchanged apart from the renamed symbol — it remains the regression test for the explicit-override path. The added case is identical except that it omits responseProcessors entirely, and asserts the sandbox executor's executeCodeInternal was called exactly once and that an execution-result part with Outcome.OUTCOME_OK containing hello was emitted. That is the end-to-end proof through InMemoryRunner (which supplies the InMemoryArtifactService that postProcessCodeExecutionResult requires) that the default wiring alone is sufficient. The ~60-line mock-client / mock-response fixture is lifted into file-level helpers rather than copy-pasted.

To reproduce locally:

npx vitest run --project unit:core core/test/agents/llm_agent_test.ts                                  # 31 passed
npx vitest run --project unit:core core/test/agents/processors/code_execution_request_processor_test.ts # 8 passed
npx vitest run --project integration tests/integration/agents/agent_with_sandbox_executor_test.ts       # 2 passed
npm run build && npm run lint && npm run format:check                                                   # all clean

For a live smoke check, an LlmAgent with codeExecutor: new UnsafeLocalCodeExecutor() and no responseProcessors, prompted to compute 2+2 in Python, now emits an execution-result event containing 4.

Two disclosures on checks:

• npm run ts:check reports 281 errors on this branch. It reports the identical 281 errors on unmodified main (verified by diffing the two error lists — they match exactly). They come from core/dist/types and core/src resolving to distinct type identities once npm run build has run, and are unrelated to this change, which adds zero new type errors. ts:check is not part of the CI workflow (validation.yaml runs build, test:coverage, lint, format:check, docs:check).
• The pre-PR suppression grep flags one line in the integration test: client: mockClient as unknown as Client. That cast is pre-existing (verbatim at line 85 of the file on main); it moved into the extracted helper. No suppression is added by this PR — the change in fact removes one, replacing an any[] plus its eslint-disable @typescript-eslint/no-explicit-any with a real Event[].

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.

CI note

All three run-tests legs (ubuntu-latest, windows-latest, macos-latest) pass on this branch.

Getting there took two reruns of an unrelated flake: tests/integration/app_loader/app_loader_test.ts > AgentLoader discovery and loading integration > should discover apps vs agents across directories and standalone files timed out at the suite's 40000ms budget, first on macOS and then on Windows (at 42480ms — just over budget), while 2684 of 2685 other tests passed on each run. Both legs went green on rerun with no code change.

The cause is in that suite, not in this diff: its beforeAll hooks shell out to a real npm install inside a fixture project, so the whole 40s budget is spent on network-bound dependency installation before any assertion runs. It is reproducible locally on a slow network for the same reason. This change cannot affect app loading — it does not even alter the module import graph, since llm_agent.ts already imported code_execution_request_processor.js for CODE_EXECUTION_REQUEST_PROCESSOR. Deliberately not fixed here, to keep this diff to the change under review; filed as separate follow-up work.

The revision commit hit a second, different Windows-only flake: tests/e2e/tools/mcp/load_mcp_resource_e2e_test.ts and core/test/code_executors/unsafe_local_code_executor_test.ts > should execute shell code and return stdout, both Test timed out in 5000ms, with 2683 of 2685 tests passing. Both spawn a child process (a real MCP server over stdio, and a Windows shell running echo) against a 5s budget on a slow runner. Green on rerun. The executor one is code-executor-adjacent by name but cannot be reached by this change: it calls executor.executeCode(params) directly and never runs the agent, so the default responseProcessors list is never iterated. It is also independently known — PR #254 on this fork exists specifically to harden these Windows shell cases. Both pass locally (18/18).

Complexity review response

• Shrank the integration-test edit to the defensible minimum. The existing 'executes code generated by the agent' case had been restructured more than the new case required. Its body is now byte-identical to its previous form apart from four forced substitutions: the responseProcessor -> CODE_EXECUTION_RESPONSE_PROCESSOR rename, the MOCK_RESPONSES and createMockClient() hoists (the only two fixtures large enough that sharing beats duplicating), and any[] -> Event[]. Verified mechanically by re-applying exactly those four substitutions to the original file and diffing against the new one — they match.
• Removed createSandboxExecutor() and collectEvents(). Both wrapped two call sites without removing duplication worth the indirection, and createSandboxExecutor() only existed to force a ReturnType<typeof createMockClient> annotation. Both are inlined; the suppression removal that collectEvents() was carrying is preserved by declaring const events: Event[] = [] inline. Integration test file: 174 lines, down from 177, with 19 fewer changed lines.
• Did not apply the Pick<Client, 'agentEnginesInternal'> suggestion for client: mockClient as unknown as Client (raised as non-blocking). I probed it, and it makes things worse rather than removing the cast — it produces two new type errors: the mock is missing four members of Sandboxes (apiClient, deleteInternal, listInternal, getSandboxOperationInternal), so it does not satisfy the Pick; and Pick<Client, 'agentEnginesInternal'> is still not assignable to Client (missing apiClient, _agentEnginesInternal), so the call site would need the cast anyway. The cast is carried over verbatim from the pre-existing test and is left as-is. Inlining the executor construction does mean it now appears in both cases rather than one; making it appear once would require reintroducing the wrapper that was just removed.