Fix: import ADK symbols from source in core/test and pin the convention (281 -> 139 type errors)

[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: npm run ts:check (tsc --noEmit) is meant to be the repo-wide type-check gate, but it fails with 281 errors, so it is not wired into .github/workflows/validation.yaml. The test tree is therefore only ever run through Vitest, which transpiles with esbuild and does no type checking at all. Real bugs hide behind that gap.

About 124 of the 281 are not real type errors — they are an artefact of core/test mixing two import conventions. 125 statements imported ADK symbols from the package specifier '@google/adk', which TypeScript resolves through the node_modules/@google/adk workspace symlink to the built declarations in core/dist/types, while other statements imported the source at '../../src/…'. TypeScript then saw two structurally identical but nominally distinct copies of every class carrying a private member or a brand symbol, producing Types have separate declarations of a private property 'updateSessionState' and Property '[BASE_AGENT_SIGNATURE_SYMBOL]' is missing…. The repo already documented the confusion in a suppression comment at core/test/agents/functions_test.ts:712.

This duplication does not exist at runtime: vitest.config.ts aliases '@google/adk' to ./core/src for every project, so the suite has always loaded exactly one copy, from source. Only the type checker disagreed with the runtime.

Two clusters behind the remaining errors were outright bugs the transpile-only pipeline hid:

• core/test/runner/runner_test.ts wrote as unknown as Content['parts']![0] at 8 sites. ! is a value-position non-null assertion and is invalid in a type position (TS17019 ×8). esbuild strips annotations without parsing them, so this never surfaced.
• core/test/sessions/vertex_ai_session_service_test.ts imported 3 deep subpaths (@google/adk/sessions/session.js, …). core/package.json exports declares only ".", so these are unresolvable for a real consumer (TS2307 ×3); Vitest resolved them only via its directory alias.

Solution: Settle the convention: core/test imports ADK symbols from the source tree via relative paths, which is exactly what Vitest already loads, so the type checker and the runtime finally agree.

This is step 1 of a series. It deliberately does not try to reach zero — it clears the module-identity class plus the two hidden-bug clusters, and leaves the 136 genuine core/test type errors for follow-ups so each PR stays reviewable.

1. Import rewrite (124 files, 128 statements). from '@google/adk' → a depth-correct from '../../src/index.js'; the 3 deep subpaths → ../../src/<subpath>. The rewrite was anchored to specifiers that terminate an import/export … from statement, so the two prose occurrences of the text @google/adk (a test description at tools/example_tool_test.ts:152 and the suppression comment) were not corrupted. Non-ADK specifiers (@google/genai, @google-cloud/vertexai, vitest, zod) are untouched.
2. Format pass. prettier with prettier-plugin-organize-imports reorders each rewritten specifier into its new sort slot and merges duplicated import blocks. This is why most files show more than a one-line change.
3. 8 × TS17019 fixed by deleting the casts, not re-typing them — see Deviations below.
4. 2 stale @ts-expect-error directives removed (they became TS2578 once the above landed).
5. ESLint guard so the convention cannot silently drift back, plus a test that pins the guard.

Why relative imports rather than a tsconfig paths mapping: a root-level paths mapping reaches a similar residual, but it was measured to introduce 7 new TS2322 errors in dev/src/conformance/conformance_integrations.ts — real errors, invisible today only because --emitDeclarationOnly strips the types of private members and erases FunctionTool's contravariance in TParameters. Worse, paths is inherited by dev/tsconfig.json and integrations/tsconfig.json (both extends: "../tsconfig.json"), so it would change what those packages emit into their published .d.ts. The relative convention gets the identical benefit with none of that blast radius.

Collision check (required disclosure). I checked all 412 open PRs on the fork before writing any code (gh pr list --repo AmaadMartin/adk-js --state open --limit 1000) and read the diffs of every plausibly adjacent one. This area is crowded: at least 11 open PRs target npm run ts:check — #178, #204, #248, #293, #294, #326, #370, #408, #414, #421, #487.

The closest is #487 ("type-check core/test against core/src and fix the 154 errors it surfaces"). It reaches a similar end state by a different mechanism: a core/test/tsconfig.json whose paths mapping mirrors the Vitest alias, keeping the @google/adk specifier in test files. It also fixes the same 8 TS17019 sites and removes the same 2 stale directives, and goes further by fixing the genuine type errors this PR defers.

These two PRs are alternatives, not complements, and only one should land. They are mutually exclusive by construction: this PR's ESLint guard forbids exactly the specifier #487's paths mapping exists to bless. I am raising this explicitly so a maintainer can choose the mechanism rather than discover the conflict at merge time. The trade-off:

• This PR needs no build output and no extra tsconfig — core/test stops depending on core/dist/types entirely — and the convention is machine-enforced. Cost: a 124-file mechanical diff that will conflict textually with other in-flight core/test PRs.
• Fix: type-check core/test against core/src and fix the 154 errors it surfaces #487 has a far smaller diff and keeps the ergonomic @google/adk specifier. Cost: a second tsconfig plus a root exclude, the type-check path stays distinct from the ordinary tsc --noEmit, and nothing prevents a deep-subpath import from reappearing.

Related but not overlapping: #435/#325 already restrict deep @google/adk/* subpath imports repo-wide. This guard is core/test-scoped and also covers the package root, so it is a superset there; the two rules would coexist without conflict.

I did not stack on #487 because a stack would require rewriting the imports its paths mapping was added to support, i.e. reverting its central design decision, and because a stacked base gets no CI on this fork.

Deviations from the approved plan (disclosed, both make the change stricter — no scope was reduced):

1. The plan said to rewrite the 8 casts to as unknown as NonNullable<Content['parts']>[0]. I deleted them instead. Every one of the 8 annotated a {inlineData: {mimeType, data, displayName}} literal, and all three fields are declared on Blob in @google/genai (node_modules/@google/genai/dist/genai.d.ts:1135-1143), so each literal is already a valid Part. The enclosing const newMessage: Content contextually types the array. Keeping any as unknown as … form would have preserved an unchecked cast and left the literals unverified; deleting it makes them genuinely checked, so a typo in mimeType is now a compile error. This is a smaller diff and removes 8 casts rather than repairing them.
2. The plan asked for a manual spot-check of the ESLint guard; I added a permanent regression test instead (tests/integration/repo_config/eslint_core_test_import_convention_test.ts, 7 tests). A spot-check leaves nothing behind, and the guard is the only genuinely new behaviour here. It runs the real ESLint against the real eslint.config.js via ESLint#lintText, which resolves config for a virtual path and writes nothing to disk — no temp files, no cleanup path.
3. Guard pattern is ["@google/adk", "@google/adk/**"], not ["@google/adk", "@google/adk/*"]. Both were measured to behave identically against a deep subpath on ESLint 9.37; \*\*is used for explicitness. Both the baseno-restricted-importsand@typescript-eslint/no-restricted-importswere also measured and catch all four forms includingimport type; the base rule is used, as specified.

Review revision -- the one test whose subject was the import specifier. core/test/tools/example_tool_test.ts contained it('is importable from @google/adk (public export)'). Repointing its import at ../../src/index.js made that title false and silently demoted the guarantee from the published package surface to the source barrel, leaving the former covered by nothing. Fixed in two parts, without deleting anything:

• The existing test is retitled to 'is exported from the package barrel' -- what it now actually checks. Its assertion is unchanged.
• The lost guarantee is restored as a new test at tests/integration/tools/example_tool_test.ts, where the package specifier remains legal and the new guard deliberately does not reach. tsc resolves @google/adk there to core/dist/types, so a symbol dropped from the built package fails it even while the source barrel still exports it -- a check core/test can no longer perform by construction. It drives ExampleTool through real framework objects (a real LlmAgent, InvocationContext, Session and Context; no stubs, no casts) and asserts the few-shot block lands on the outgoing request, so it is not the tautology the old one had become.

core/test now contains zero occurrences of the string @google/adk, in code or prose.

Not shipped as a stack, despite the file count: 124 of the 126 files carry a uniform 1-2 line mechanical import rewrite, splitting it would leave intermediate commits in which the module-identity errors are only partly cleared (no coherent checkpoint), and a stacked base gets no CI on this fork. The three commits provide the review checkpoints instead.

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.

Baseline re-measured first. The plan pinned main at 281 errors; this branch's base is 44 commits later, so the precondition was re-checked before starting: npx tsc --noEmit → 281 errors (278 core/test, 3 tests/integration; TS2345 127, TS2322 53, TS2741 35, TS2339 25, TS17019 8, TS2739 6, TS2307 4) — an exact match, so the plan's numbers still applied.

Type-check progression (npx tsc --noEmit, after npm run build):

Stage	Errors
Baseline (fork/main, b390217e)	281
+ import rewrite & format	157
+ cast deletions & stale-directive removal	139

Final 139 = 136 core/test + 3 tests/integration, matching the planned postcondition. Verified at the pushed commit:

• TS17019: 0
• TS2578: 0
• TS2307 in core/test: 0
• module-identity errors (separate declarations of a private property, [*_SIGNATURE_SYMBOL]' is missing) in core/test: 0
• @google/adk import specifiers under core/test: 0

The 2 remaining separate declarations errors and the 1 remaining TS2307 are both in tests/integration and are pre-existing, out-of-scope items (a nested @google/genai copy bundled by @google-cloud/vertexai, and responseProcessor not being re-exported).

Residual core/test errors for the follow-ups: tools 37, models 37, utils 16, code_executors 10, auth 9, sessions 5, agents 5, integrations 4, events 4, context 3, apps 3, plugins 2, memory 1.

No runtime behaviour changed — measured, not assumed. npx vitest run --project unit:core:

• this branch: 168 files / 2351 tests passed
• fork/main baseline: 168 files / 2351 tests passed — identical.

Full CI suite (npm run test:coverage) run on both branches and the failure sets diffed. The only difference is tests/integration/app_loader/app_loader_test.ts, which is a pre-existing flake, not a regression: run 3× in isolation it gave fail/pass/fail on this branch and pass/fail/pass on fork/main. It fails with Error: Hook timed out in 40000ms in an install-heavy beforeAll, and this PR touches nothing it loads. Every other failing file is identical on both branches (tests/e2e/** needs live Gemini credentials; agent_dirname, skills/script_js, cli_create are install/sandbox-dependent).

Coverage thresholds could not be evaluated because those pre-existing failures abort the run identically on both branches. They are unaffected by construction: vitest.config.ts computes coverage over **/src/** only, and this PR modifies no src file.

Proof the new tests can fail. All 7 guard tests were run against three mutated configs:

Mutation	Result
Delete the guard block from eslint.config.js	4 rejection tests fail — AssertionError: expected [] to have a length of 1 but got +0
Widen files from core/test/**/*.ts to **/*.ts	scoping test fails — AssertionError: expected [ Array(1) ] to deeply equal []
Broaden group to ["**"]	both negative tests fail — AssertionError: expected [ …(2) ] to deeply equal []

The restored package-specifier test was mutation-checked the same way: dropping the ExampleTool re-export from core/src/common.ts fails it with TypeError: ExampleTool is not a constructor, and short-circuiting processLlmRequest fails the systemInstruction assertion. core/src was restored and re-verified clean after each.

Every assertion in the file is pinned by at least one mutation; the guard was restored and re-verified at 7/7 passing after each.

The guard was additionally verified to fire on the package root, a type-only import, a re-export and a deep subpath; to stay silent on @google/genai, zod and the relative path; and to remain scoped — the 8 dev/test files that still import @google/adk lint clean.

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

npm install
npm run build

# 1. Type check. Expected: exits non-zero with exactly 139 errors (down from 281).
#    This step is still red on purpose -- it is the documented postcondition of
#    step 1, and the follow-ups chain from this count.
npx tsc --noEmit --pretty false 2>&1 | grep -c 'error TS'   # -> 139

# 2. No ADK package specifier survives under core/test.
grep -rn "from '@google/adk" core/test --include='*.ts'      # -> no matches

# 3. The regression guard actually fires.
printf "import {App} from '@google/adk';\n" >> core/test/agents/functions_test.ts
npm run lint                                                 # -> fails with the guard message
git checkout core/test/agents/functions_test.ts

# 4. Runtime is unchanged.
npx vitest run --project unit:core                           # -> 168 files / 2351 tests pass
npx vitest run --project integration \
  tests/integration/repo_config/eslint_core_test_import_convention_test.ts \
  tests/integration/tools/example_tool_test.ts               # -> 8 pass

# 5. Repo gates.
npm run lint          # clean
npm run format:check  # clean
npx secretlint "**/*" # clean

All of the above were run locally on the pushed commit: npm run build exit 0, npm run lint exit 0, npm run format:check exit 0, npx secretlint "**/*" exit 0.

CI note (windows-latest). The three test jobs are green. windows-latest failed twice before passing on a third re-run of the same commit, always on the same unrelated case: core/test/code_executors/unsafe_local_code_executor_test.ts > 'should execute shell code and return stdout', timing out at ~5008ms. That test spawns a real Windows PowerShell and was measured at 3114ms on a windows-latest run that passed — vitest flags it slow — against vitest's 5000ms default testTimeout, so it is inherently marginal and tips over with runner variance. This PR's only change to that file is the import specifier; the runtime module graph is identical, since Vitest already aliased @google/adk to core/src. windows-latest is also already red on the base commit b390217e (on a different test, tests/integration/adk_web/webui_test.ts). I deliberately did not bundle a timeout fix for it into this PR — it is unrelated churn — and queued it as separate follow-up work instead.

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.