Fix: export CODE_EXECUTION_RESPONSE_PROCESSOR from the @google/adk public API - #460
Open
AmaadMartin wants to merge 2 commits into
Open
Fix: export CODE_EXECUTION_RESPONSE_PROCESSOR from the @google/adk public API#460AmaadMartin wants to merge 2 commits into
AmaadMartin wants to merge 2 commits into
Conversation
added 2 commits
August 1, 2026 08:25
LlmAgent advertises a public responseProcessors option, but the only processor that implements code-execution result handling was unreachable from the published package: it was absent from common.ts / index.ts, and core/package.json declares only the "." subpath, so a deep import is rejected too. The advertised knob could not be used by a consumer of npm install @google/adk. Rename the singleton from the bare, ambiguous responseProcessor to CODE_EXECUTION_RESPONSE_PROCESSOR, matching the repo's convention for processor singletons (CONTENT_REQUEST_PROCESSOR, INTERACTIONS_REQUEST_PROCESSOR), and re-export it with its class from common.ts. index.ts already does 'export * from ./common.js', so both symbols reach the package root and the TypeDoc entry point.
…umer The sandbox-executor integration test deep-imported the processor, which only resolved because vitest aliases @google/adk to core/src -- a false-green that proved nothing about the shipped package. Retarget it at the package root. Add the symbol to the ts_esm build_setup fixture, the only place in the repo that consumes @google/adk as an installed package (file:../core, tsc --noEmitOnError through the exports map), so a regression in core/package.json's exports map is caught too. The addition is inert: the fixture agent has no codeExecutor, so runPostProcessor returns early. Add one unit test asserting the package-root singleton is an instance of the module's class.
This was referenced Aug 2, 2026
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.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
Link to an existing issue (if applicable):
N/A
Or, if no issue exists, describe the change:
Problem:
LlmAgentadvertises a publicresponseProcessors?: BaseLlmResponseProcessor[]option (core/src/agents/llm_agent.ts:304), stores it (line 372), defaults it to[](line 434), and iterates it duringpostprocess(line 910). UnlikerequestProcessors, nothing is auto-installed into it, so an empty array means code-execution results are never turned into events.The one implementation a user would put there —
export const responseProcessor = new CodeExecutionResponseProcessor()incore/src/agents/processors/code_execution_request_processor.ts— is unreachable from the published package:core/src/common.tsre-exports the sibling processors (CONTENT_REQUEST_PROCESSOR,INTERACTIONS_REQUEST_PROCESSOR) but had no entry forcode_execution_request_processor.js, andcore/src/index.tsonly doesexport * from './common.js'.core/package.json'sexportsmap declares exactly one subpath,".". There is no wildcard, so a consumer cannot deep-import the module either.Net effect: a published consumer could not enable code-execution result handling at all. Both spellings fail:
The in-repo integration test hid the defect rather than exposing it.
tests/integration/agents/agent_with_sandbox_executor_test.ts:9used the deep specifier, which resolved only becausevitest.config.tsaliases@google/adktocore/src. Under the alias it degrades to a plain relative path into the source tree, so the test passed while the shipped package was broken — a false-green.Solution: Four edits, no behavioural change.
core/src/agents/processors/code_execution_request_processor.ts— rename the singleton from the bare, ambiguousresponseProcessortoCODE_EXECUTION_RESPONSE_PROCESSOR, mirroringCODE_EXECUTION_REQUEST_PROCESSORdeclared in the same file and the repo'sSCREAMING_SNAKE_CASEconvention for processor singletons. ExportingresponseProcessorverbatim would put an ambiguous bare noun into the top-level@google/adknamespace. The TSDoc block is kept and expanded, since the symbol is becoming public.core/src/common.ts— re-exportCODE_EXECUTION_RESPONSE_PROCESSORandCodeExecutionResponseProcessorin one statement, singleton first, matching the two sibling processor blocks exactly. Placed between thebase_llm_processor.jsandcontent_request_processor.jsblocks soprettier-plugin-organize-importsordering holds.core/src/index.ts— no change. Itsexport * from './common.js'already propagates both symbols to the package root and to the TypeDoc entry point (typedoc.jsonuses./core/src/index.tsas its sole entry point). A duplicate entry would create an export conflict and a TypeDoc warning.Why rename rather than export the legacy name — this is technically a breaking change to a symbol nobody can consume, therefore effectively non-breaking:
responseProcessorhas never been reachable from@google/adk, neither via the root barrel nor via a subpath, so no published version ever exposed it under any spelling and no consumer's build can break. Renaming now, at the moment of first export, avoids being stuck with the awkward name. No deprecation alias is added, since that would ship two names for one object on day one.Scope discipline —
CODE_EXECUTION_REQUEST_PROCESSORis deliberately not exported:LlmAgentinstalls it itself, so a consumer never needs to name it.core/package.jsonis not modified (adding a wildcard subpath would publish the entire internal module tree as public API).vitest.config.tsis not modified. No new dependency, no new file, no change torunPostProcessoror any code-execution logic.Cross-language note —
adk-pythonis not a reference here. There the equivalent class is private (_CodeExecutionResponseProcessor) and unexported, because Python'sLlmAgentwires code execution through its flow rather than through a publicresponseProcessorsoption. adk-js intentionally diverges by exposingresponseProcessorspublicly; that divergence is exactly what creates the obligation to export the processor. Local convention wins over parity here because the divergence is in the observable public API by design.Collision check —
gh pr list --repo AmaadMartin/adk-js --state open --limit 300reviewed; no open PR lands this change. Three PRs touch adjacent ground and are disclosed here:fix/ban-deep-package-imports) and Fix: anchor the vitest workspace aliases so deep @google/adk/* specifiers fail at test time #380 (fix/vitest-alias-exact-match) each rewrite the same single line —agent_with_sandbox_executor_test.ts:9— replacing the deep@google/adk/...specifier with a relative path intocore/src. That is a workaround for the symptom; this PR removes the deep import by exporting the symbol properly, which makes their hunk in that file unnecessary. Whichever lands second needs a one-line conflict resolution. Not stacked on either, because Fix: ban deep subpath imports of ADK workspace packages (ESLint guard) #325 and Fix: anchor the vitest workspace aliases so deep @google/adk/* specifiers fail at test time #380 already collide with each other on that exact line and neither is a prerequisite for this change.fix/remove-deep-package-subpath-imports) adds an ESLint guard against deep subpath imports; this PR is complementary — it removes the last such import fromtests/integration.Neither the naming nor the export was proposed by any of them.
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.
One
describeblock appended to the existingcore/test/agents/processors/code_execution_request_processor_test.ts(the five existingCodeExecutionResponseProcessor.runAsynccases are untouched and still pass). It importsCODE_EXECUTION_RESPONSE_PROCESSORfrom'@google/adk'while keepingCodeExecutionResponseProcessoron the existing relative import, so the assertion compares the package-root symbol against the module symbol and fails ifcommon.tsever stops re-exporting it.toBeInstanceOfhere is an identity assertion in a test, not runtime type detection insrc/.No new executable lines or branches are introduced (a rename plus a re-export), so new-line coverage is satisfied vacuously.
Integration tests:
CODE_EXECUTION_RESPONSE_PROCESSORwas also added to thets_esmbuild_setup fixture. That is the only place in the repo that consumes@google/adkas a real installed package — itspackage.jsondeclares"@google/adk": "file:../../../../core"and its build runstscwithnoEmitOnError,strictandpreserveSymlinks, so type resolution goes throughcore/package.json'sexportsmap intocore/dist/types/index.d.ts, with no vitest alias in sight. Nothing else in the repo can catch an exports-map regression. Onlyts_esmwas modified; the other five fixtures are untouched (one fixture proves the path, each extra one costs anothernpm installin CI). The addition is behaviourally inert: the fixture agent has nocodeExecutor, sorunPostProcessorreturns at its early-exit guard and the fixture's existingtest-llm-model-responseassertion is unaffected.Mutation proof (each new/changed test proven able to fail). Mutation applied: revert only the
core/src/common.tshunk, keeping the rename.AssertionError: expected undefined to be an instance of CodeExecutionResponseProcessor(Tests 1 failed | 8 passed (9)).AssertionError: expected 1 to be greater than or equal to 3(Tests 1 failed (1)); with the processor gone the code block is never executed, so the execution-result event is never emitted.ts_esmconsumer fixture, afternpm run build --workspace=core:The hunk was restored and all three re-verified green afterwards.
Independent confirmation of the defect and the fix.
npm run ts:checkis red onmainin this checkout (281 errors, pre-existing; see the in-flight work in #370/#408/#414/#421). Diffing the error list before and after this change: no new error is introduced, the 12 pre-existing errors in the unit test file merely shift by one line, and exactly one error disappears —That is the original defect, reported by
tscitself, and it is gone. Count: 281 -> 280.Manual End-to-End (E2E) Tests:
The published-package surface, exercised outside the repo with no vitest alias and no test framework:
Prints
ok. Both built entry points were checked directly as well —core/dist/esm/index.jsandcore/dist/cjs/index.jseach resolve the named export and the singleton is an instance of the exported class. Before the fix the same command fails withSyntaxError: The requested module '@google/adk' does not provide an export named 'CODE_EXECUTION_RESPONSE_PROCESSOR'.Full gate, all run on the pushed commit:
npm run buildnpx vitest run --project unit:core core/test/agents/processors/code_execution_request_processor_test.tsnpx vitest run --project integration tests/integration/agents/agent_with_sandbox_executor_test.tsnpx vitest run --project integration tests/integration/build_setup/build_setup_test.tsnpm run ts:checkmain); 280 with this change, no new errorsnpm run lintnpm run format:checkAll matched files use Prettier code style!npm run docs:check--treatWarningsAsErrorsbuild_setup_test.tsperforms a freshnpm installper fixture and fails in this sandbox withnpm ERR! code E403 ... Forbidden - GET .../@a2a-js%2fsdkfrom the restricted registry proxy. This was verified to fail identically on a clean, unmodified tree, so it is an environment limitation, not a regression. To cover the gap thets_esmfixture was type-checked directly against the built package instead — its twofile:dependencies linked intonode_modules/@google/, thentsc -p .with the fixture's owntsconfig.json(module: nodenext,noEmitOnError,strict,preserveSymlinks) — which resolves throughcore/package.json'sexportsmap exactly as the CI fixture build does. It passes with the fix and fails withTS2305without it, as quoted above. The scratchnode_modules/distwere removed and are not in the diff.npm run docs:checkwas the one most likely to surprise, since two new symbols enter the documented surface. Both already carry TSDoc, and every type in their public signatures (BaseLlmResponseProcessor,InvocationContext,LlmResponse,Event) was verified to be exported fromcommon.tsalready, so no export was added for them.Postconditions verified by grep:
grep -rn "responseProcessor" --include="*.ts" . | grep -v node_modules— zero remaining imports of the old name; the only surviving hits arellm_agent.ts's unrelatedresponseProcessorsoption (plural) and the unit test's own localconst responseProcessor.grep -rn "from '@google/adk/" --include="*.ts" . | grep -v node_modules—tests/integrationis now free of deep imports. Three hits remain incore/test/sessions/vertex_ai_session_service_test.ts; those are explicitly out of scope for this task, are already owned by Fix: anchor the vitest workspace aliases so deep @google/adk/* specifiers fail at test time #380, and are unrelated to code execution.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
The first
validationrun went green onubuntu-latestbut red onmacos-latest, withwindows-latestcancelled by the matrix'sfail-fast(all of its own steps had already succeeded — 2681 tests passed, plus lint, format and docs). The single macOS failure wastests/integration/app_loader/app_loader_test.ts > AgentLoader discovery and loading integration > should discover apps vs agents across directories and standalone files,Error: Test timed out in 40000mson a runner that spent 418s total with 380s of that in collection alone.That test is unrelated to this change: it only scans
tests/integration/app_loader/discovery, a different tree from thetests/integration/build_setup/ts_esmfixture touched here, and nothing in this diff affects agent discovery. Re-running the failed jobs with no code change turned all three matrix legs green, confirming a runner-slowness timeout flake rather than a regression.