Skip to content

Fix: drop the unresolvable @google/adk subpath imports from the Vertex AI session service test (stacked on #460) - #512

Open
AmaadMartin wants to merge 1 commit into
fix/export-code-execution-response-processorfrom
fix/code-execution-response-processor-export
Open

Fix: drop the unresolvable @google/adk subpath imports from the Vertex AI session service test (stacked on #460)#512
AmaadMartin wants to merge 1 commit into
fix/export-code-execution-response-processorfrom
fix/code-execution-response-processor-export

Conversation

@AmaadMartin

Copy link
Copy Markdown
Owner

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):
    N/A — no public issue tracks this.
  2. Or, if no issue exists, describe the change:

Stacked on #460 (fix/export-code-execution-response-processor). This PR targets that branch, not main. Review #460 first; the diff below is only this PR's 3-line delta on top of it.

Problem: core/package.json declares exactly one subpath in its exports map ("."). Under moduleResolution: nodenext an exports map is exhaustive, so any specifier other than the bare package name is unresolvable — for TypeScript and for Node alike. Four files imported deep @google/adk/<subpath> specifiers anyway:

File Specifier Symbol
tests/integration/agents/agent_with_sandbox_executor_test.ts:9 @google/adk/agents/processors/code_execution_request_processor.js responseProcessor
core/test/sessions/vertex_ai_session_service_test.ts:9 @google/adk/sessions/session.js Session
core/test/sessions/vertex_ai_session_service_test.ts:27 @google/adk/sessions/vertex_ai_session_service.js isVertexAiConnectionString, quoteFilterLiteral
core/test/sessions/vertex_ai_session_service_test.ts:28 @google/adk/utils/logger.js logger

All four produce error TS2307 under npm run ts:check. They appear to work only because vitest.config.ts aliases the @google/adk prefix onto core/src, and Vite alias matching is a prefix replacement — so under vitest the deep specifier silently degrades to a path into the source tree, and nowhere else. That alias is what hid the defect.

#460 fixes the first row by publishing the symbol as CODE_EXECUTION_RESPONSE_PROCESSOR on the public barrel. This PR fixes the remaining three, which #460 does not touch.

Solution: retarget each of the three imports at a specifier that is actually valid. The right target differs per symbol, because only one of the three is public:

  • Session is already publiccore/src/common.ts:230 exports it (export type {CompositeSessionKey, Session}). It joins the existing '@google/adk' import on the line above. A plain (non-type) named import of a type-only export is the established style here; core/test/a2a/executor_context_test.ts:8 does exactly import {Session} from '@google/adk';.
  • isVertexAiConnectionString and quoteFilterLiteral are internal — exported from neither common.ts, index.ts, nor index_web.ts. This is a core/test/** unit test of internal helpers, so it uses the relative path into src that the other 155 core/test imports already use.
  • logger is internal too. common.ts:283-284 exports LogLevel, getLogger, setLogLevel, setLogger and the Logger type, but not the logger const. Matches core/test/utils/logger_test.ts:9, core/test/a2a/agent_to_a2a_test.ts:24 and three others.

Deliberately not done, to keep the change minimal and behaviour-neutral:

  • core/package.json is untouched — no subpath entries, no "./*" wildcard. Widening the exports map would make the broken imports resolve by publishing new API surface by accident. Verified below that the deep specifier is still rejected.
  • No symbol was promoted to public just to make a test import shorter. Two of the three are internal and stay internal.
  • vitest.config.ts is untouched. Anchoring the alias so deep specifiers fail at test time is the natural follow-up, but it is already in flight as Fix: anchor the vitest workspace aliases so deep @google/adk/* specifiers fail at test time #380 and duplicating it here would guarantee a conflict.
  • tsconfig.json is untouched, and has no exclude array to remove.

Collision check (run before writing any code, per the fork's 412 open PRs):

gh pr list --repo AmaadMartin/adk-js --state open --limit 1000 --json number,title,headRefName
gh pr diff <n> --repo AmaadMartin/adk-js --name-only   # for each plausibly adjacent PR

Three adjacent PRs found:

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.

This PR's own delta rewrites import specifiers only; it adds no executable source line, so there is nothing new to cover. The guarantee is a resolution gate, not line coverage, and it is enforced by npm run ts:check:

$ npm run ts:check 2>&1 | grep TS2307 | grep '@google/adk'
(no output)

Targeted runs, all on the pushed commit:

$ npx vitest run --project unit:core \
    core/test/sessions/vertex_ai_session_service_test.ts \
    core/test/agents/processors/code_execution_request_processor_test.ts
  Test Files  2 passed (2)
       Tests  66 passed (66)

$ npx vitest run --project integration tests/integration/agents/agent_with_sandbox_executor_test.ts
  Test Files  1 passed (1)
       Tests  1 passed (1)

$ npx vitest run --project integration tests/integration/build_setup/build_setup_test.ts -t ts_esm
  Test Files  1 passed (1)
       Tests  6 passed | 18 skipped (24)

Proof the checks can fail. Each guard was run against the unfixed code and confirmed to fail.

Mutation A — revert this PR's file to the stack base (git checkout fork/main -- core/test/sessions/vertex_ai_session_service_test.ts), then npm run ts:check:

core/test/sessions/vertex_ai_session_service_test.ts:9:23  - error TS2307: Cannot find module '@google/adk/sessions/session.js' or its corresponding type declarations.
core/test/sessions/vertex_ai_session_service_test.ts:27:8  - error TS2307: Cannot find module '@google/adk/sessions/vertex_ai_session_service.js' or its corresponding type declarations.
core/test/sessions/vertex_ai_session_service_test.ts:28:22 - error TS2307: Cannot find module '@google/adk/utils/logger.js' or its corresponding type declarations.

Restored, the same command prints nothing. Total tsc --noEmit error count across the whole stack: 281 before, 277 after — exactly the four TS2307 the bug report names, and no new diagnostics. (The remaining 277 are pre-existing and unrelated; the root tsconfig.json declares no include, so tsc sweeps the entire repo including the consumer fixtures. Making that clean is not this task.)

Mutation B — delete the common.ts re-export block, then the barrel guard inherited from #460:

× CODE_EXECUTION_RESPONSE_PROCESSOR > is exported from the package root as a usable response processor
  → expected undefined to be an instance of CodeExecutionResponseProcessor

Mutation C — same deletion, rebuilt, then the real-consumer fixture, which resolves through the genuine exports map ("@google/adk": "file:../../../../core", noEmitOnError: true):

agent.ts:9:3 - error TS2305: Module '"@google/adk"' has no exported member 'CODE_EXECUTION_RESPONSE_PROCESSOR'.
 FAIL  |integration| tests/integration/build_setup/build_setup_test.ts > Build setup > ts_esm

C is the one with real teeth: it is the only check in the tree that would still fail if the vitest alias were reintroduced, because it does not go through vitest resolution at all.

Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any necessary setup or configuration.

Proves the specifier resolves for a genuinely out-of-tree consumer — the property that was actually broken and that no in-repo check covers:

npm run build
cd "$(mktemp -d)" && npm init -y >/dev/null
npm install "file:/path/to/adk-js/core"
cat > probe.mjs <<'EOF'
import {CODE_EXECUTION_RESPONSE_PROCESSOR} from '@google/adk';
console.log('runAsync typeof:', typeof CODE_EXECUTION_RESPONSE_PROCESSOR.runAsync);
EOF
node probe.mjs
# -> runAsync typeof: function

And the negative — confirming the exports map was not widened to make the old specifiers resolve:

node -e "import('@google/adk/agents/processors/code_execution_request_processor.js').then(
  () => {console.error('FAIL: subpath resolved; exports map was widened'); process.exit(1)},
  (e) => console.log('OK: subpath still not exported (' + e.code + ')'))"
# -> OK: subpath still not exported (ERR_PACKAGE_PATH_NOT_EXPORTED)

CI status: absent, validated locally instead. .github/workflows/validation.yaml triggers on pull_request: branches: [main], so a PR based on fix/export-code-execution-response-processor never triggers it. Everything CI would run was run locally on the pushed commit:

Command Result
npm run build pass
npm run lint pass (exit 0, no findings)
npm run format:check All matched files use Prettier code style!
npm run docs:check pass (typedoc --treatWarningsAsErrors, exit 0)
npm run ts:check | grep TS2307 | grep @google/adk no output
targeted vitest runs 73 passed, above

npm run test:coverage (whole-suite) was not run, per the instruction to run only targeted tests.

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.

…x AI session service test

The core package's exports map declares only the "." subpath, so
'@google/adk/sessions/session.js', '@google/adk/sessions/vertex_ai_session_service.js'
and '@google/adk/utils/logger.js' resolve for nobody: not for tsc under
moduleResolution nodenext, and not for Node. They appeared to work only because
the vitest config aliases the '@google/adk' prefix onto core/src, which turns a
deep specifier into a plain path into the source tree.

Point each import at a specifier that is actually valid. Session is public
(core/src/common.ts exports it), so it joins the existing '@google/adk' import.
isVertexAiConnectionString, quoteFilterLiteral and logger are internal, so they
use the relative path into src that the other 155 core/test imports already use.
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