Fix: declare @types/express as a runtime dependency of @google/adk-devtools by AmaadMartin · Pull Request #562 · AmaadMartin/adk-js

AmaadMartin · 2026-08-03T05:16:08Z

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):
No existing issue; described below.
Or, if no issue exists, describe the change:

Problem: @google/adk-devtools ships declarations that name a module whose types it does not install for consumers.

AdkApiServer puts express on its public API surface, and the class is reachable from the package entry point:

dev/src/server/adk_api_server.ts:92 — readonly app: express.Application;
dev/src/index.ts:8 — export {AdkApiServer} from './server/adk_api_server.js';

so tsc --emitDeclarationOnly must name the namespace, and the shipped dev/dist/types/server/adk_api_server.d.ts:8 carries a top-level import express from 'express'; reachable from dist/types/index.d.ts (what typings / exports["."].types point at).

express@4 ships no bundled declarations, so those types can only come from @types/express — which dev/package.json declared under devDependencies. devDependencies are not installed for consumers of a published package. Two consumer-visible symptoms follow:

with skipLibCheck: false — a hard TS7016 raised from inside the shipped declaration;
with skipLibCheck: true (the common default) — no error, but AdkApiServer['app'] silently collapses to any, so every server.app.use(...) call in consumer code is unchecked.

Neither is visible from inside this repo: npm workspace hoisting puts a single @types/express at the monorepo root so resolution walks up and finds it, and tsconfig.json:9 sets skipLibCheck: true. Both maskings disappear in a consumer's tree.

Solution: move @types/express from devDependencies to dependencies in dev/package.json, keeping the range at ^4.17.21 so it stays on the same semver major as express: "^4.21.2" (shipping @types/express@5 types for express@4 would be a worse bug than the one being fixed). This is the standard remedy for a package whose published declarations reference a @types/* package.

Scope is deliberately three files: dev/package.json, the regenerated root package-lock.json, and one new regression test.

Collision check against open PRs on this fork

Ran before writing any code: gh pr list --repo AmaadMartin/adk-js --state open --limit 1000 (461 open PRs), filtered for express/types/dependency/manifest/devtools/lockfile keywords, then gh pr diff --name-only and the dev/package.json hunks of every adjacent candidate.

Fix: declare @types/express in dev, and gate the published type closure in CI #382 (fix/publish-public-type-deps) contains a byte-identical dev/package.json hunk plus a new scripts/check_published_types.mjs CI gate and a .github/workflows/validation.yaml change. It is stacked on Fix: declare @types/express as a runtime dependency of @google/adk #292 (the separate core @types/express promotion), so it is not mergeable against main on its own. This PR is the minimal, main-based version of the same manifest fix. If Fix: declare @types/express as a runtime dependency of @google/adk #292 and Fix: declare @types/express in dev, and gate the published type closure in CI #382 land first, this PR becomes a no-op and should be closed — it is not intended to compete with the CI gate, only to make the one-line manifest fix landable independently.
Overlapping but distinct, all confirmed not to touch this line: Fix: declare dev's undeclared runtime dependencies and prefix node:path #485 / Fix: declare dev workspace's undeclared runtime dependencies #249 / Fix: declare @opentelemetry/api as a direct dependency of @google/adk-devtools (stacked on #244) #484 / Fix: declare @google/genai as a dependency of @google/adk-devtools #244 (dev phantom runtime deps — @google/genai, @opentelemetry/*, lodash-es, @google-cloud/vertexai, @types/lodash-es), Fix: declare @types/express as a runtime dependency of @google/adk #292 (the core workspace @types/express promotion), Fix: declare openapi-types as a runtime dependency of @google/adk #273 (core openapi-types), Fix: declare hoisted runtime imports in dev/core and enforce with import/no-extraneous-dependencies #541 (lint-enforced declaration sweep).

dev/test/package_manifest_test.ts is also used by #485/#484/#244 for their own assertions; those are additive it() cases in the same file and will merge as an ordinary textual conflict, not a semantic one.

Why the lockfile diff is 13 changed lines

Regenerated with npm install --registry=https://registry.npmjs.org/ (pinned so a mirror cannot rewrite resolved URLs) — never hand-edited. The complete delta is 2 lines for the manifest move inside packages["dev"], plus the removal of "dev": true from the 11 entries that enter the production closure:

node_modules/@types/body-parser                            1.19.6
node_modules/@types/connect                                3.4.38
node_modules/@types/express                                4.17.25
node_modules/@types/express-serve-static-core              4.19.8
node_modules/@types/http-errors                            2.0.5
node_modules/@types/mime                                   1.3.5
node_modules/@types/qs                                     6.15.1
node_modules/@types/range-parser                           1.2.7
node_modules/@types/send                                   1.2.1
node_modules/@types/serve-static                           1.15.10
node_modules/@types/serve-static/node_modules/@types/send  0.17.6

Verified there is no other churn — every changed +/- line is one of those 13:

$ git diff -U0 -- package-lock.json | grep -E '^[-+]' | grep -v '^[-+][-+]' \
    | grep -vE '^-\s+"dev": true,$' | grep -vE '@types/express'
(no output)
$ ... | grep -c '"resolved"'   -> 0
$ ... | grep -c '"version"'    -> 0
$ ... | grep -c '"integrity"'  -> 0

No version bumps, no resolved/integrity rewrites. npm audit fix, npm update and npm dedupe were not run.

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 file dev/test/package_manifest_test.ts (picked up by the unit:dev vitest project). It reads dev/package.json relative to the test file and pins two facts: @types/express is declared in dependencies and not in devDependencies; and express and @types/express share a semver major (derived from each range, not hardcoded to 4, so a coordinated bump to major 5 keeps passing while a one-sided bump fails).

$ npx vitest run --project unit:dev dev/test/package_manifest_test.ts
 ✓ |unit:dev| dev/test/package_manifest_test.ts (2 tests) 6ms
 Test Files  1 passed (1)
      Tests  2 passed (2)

Mutation proof — each assertion was shown to fail. Three independent mutations, each reverted afterwards:

Mutation 1 — revert the dev/package.json hunk (put @types/express back under devDependencies). Both cases fail:

 × dev/package.json > declares @types/express as a runtime dependency
   → expected undefined to be defined
     ❯ dev/test/package_manifest_test.ts:35:55
        35|     expect(manifest.dependencies?.['@types/express']).toBeDefined();
 × dev/package.json > keeps express and @types/express on the same semver major
   → dev/package.json must declare both express and @types/express as dependencies
 Tests  2 failed (2)

Mutation 2 — keep the fix but bump @types/express to ^5.0.0 (major skew). Only the major-parity case fails, proving it is not vacuous:

 ✓ declares @types/express as a runtime dependency
 × keeps express and @types/express on the same semver major
   → expected '5' to be '4' // Object.is equality
 Tests  1 failed | 1 passed (2)

Mutation 3 — declare @types/express in both blocks. Only the classification case fails, proving the devDependencies half of it carries weight:

 × declares @types/express as a runtime dependency
   → expected '^4.17.21' to be undefined
 ✓ keeps express and @types/express on the same semver major
 Tests  1 failed | 1 passed (2)

Coverage. This change adds zero lines under any src/ tree, and vitest.config.ts restricts coverage to */src/**/*.ts, so the new-code coverage bar is vacuously met and the thresholds cannot move. The meaningful evidence is the mutation proof above and the clean-room proof below.

No automated integration test was added, deliberately. An automated version of the clean-room repro needs npm pack plus a full out-of-tree npm install (the tarball drags in @google/adk, five MikroORM drivers and esbuild) plus a tsc run, on all three OSes in the CI matrix, for a change that adds no code paths. The existing tests/integration/build_setup fixtures cannot host it either: they live inside the repo and install with file:, and they set skipLibCheck: true — i.e. they reproduce exactly the two maskings this bug hides behind, so the test would pass before and after. Flipping those fixtures to skipLibCheck: false would not detect this bug (hoisting still masks it) and would surface unrelated pre-existing declaration errors across the suite.

Other CI gates, run locally on the pushed commit:

npm run build          -> ok
npm run lint           -> ok (eslint "**/*.ts", 0 problems)
npm run format:check   -> ok (All matched files use Prettier code style!)
npm run docs:check     -> ok (typedoc --emit none --treatWarningsAsErrors)
npx secretlint "**/*"  -> ok

CI on this PR is green on all three matrix OSes (run-tests ubuntu / macOS / windows). Two intermittent failures were hit on the first attempt and cleared on re-run; both are pre-existing harness flakes with zero individual test failures, unaffected by a manifest-only change:

macOS: tests/integration/app_loader/app_loader_test.ts > should discover apps vs agents across directories and standalone files — Test timed out in 40000ms (2679 passed / 1 failed).
windows: tests/integration/a2a/stream/stream_test.ts > A2A: RemoteAgent Streaming — Error: CLI exited prematurely with code 1 at tests/integration/test_case_utils.ts:341 (2680 passed, 0 failed; the failure is at suite level, in the spawned test server). The same harness error fails the windows job on this repo's main branch itself, in tests/integration/adk_web/webui_test.ts.

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

The defect is only observable outside the monorepo, so the proof is a packed tarball installed into a scratch project outside the repo tree, type-checked with skipLibCheck: false:

npm install --registry=https://registry.npmjs.org/
npm run build

SCRATCH="$(mktemp -d)"                       # outside the repo tree
npm pack --ignore-scripts --workspace dev --pack-destination "$SCRATCH"

mkdir -p "$SCRATCH/consumer" && cd "$SCRATCH/consumer"
npm init -y >/dev/null && npm pkg set type=module
npm i --save-dev typescript@5.9.2
npm i "$SCRATCH"/google-adk-devtools-*.tgz   # NOT `npm i @types/express`

cat > tsconfig.json <<'JSON'
{"compilerOptions":{"target":"es2022","module":"nodenext","moduleResolution":"nodenext",
"strict":true,"skipLibCheck":false,"noEmit":true},"include":["index.ts"]}
JSON
cat > index.ts <<'TS'
import {AdkApiServer} from '@google/adk-devtools';
export type ServerApp = AdkApiServer['app'];
TS
npx tsc

Before (tarball packed from the base commit) — node_modules/@types/express is absent, and the shipped declaration raises the error itself:

node_modules/@a2a-js/sdk/dist/server/express/index.d.ts(1,71): error TS7016: Could not find a declaration file for module 'express'. '.../consumer_before/node_modules/express/index.js' implicitly has an 'any' type.
node_modules/@google/adk-devtools/dist/types/server/adk_api_server.d.ts(8,21): error TS7016: Could not find a declaration file for module 'express'. '.../consumer_before/node_modules/express/index.js' implicitly has an 'any' type.
node_modules/@google/adk/dist/types/a2a/agent_to_a2a.d.ts(8,21): error TS7016: Could not find a declaration file for module 'express'. '.../consumer_before/node_modules/express/index.js' implicitly has an 'any' type.
node_modules/@google/adk/dist/types/auth/auth_schemes.d.ts(6,27): error TS2307: Cannot find module 'openapi-types' or its corresponding type declarations.
node_modules/@google/adk/dist/types/tools/openapi_tool/openapi_spec_parser/openapi_spec_parser.d.ts(6,27): error TS2307: Cannot find module 'openapi-types' ...
node_modules/@google/adk/dist/types/tools/openapi_tool/openapi_spec_parser/operation_parser.d.ts(6,27): error TS2307: Cannot find module 'openapi-types' ...
node_modules/@google/adk/dist/types/tools/openapi_tool/openapi_spec_parser/tool_auth_handler.d.ts(6,27): error TS2307: Cannot find module 'openapi-types' ...
node_modules/@google/adk/dist/types/tools/openapi_tool/openapi_toolset.d.ts(6,27): error TS2307: Cannot find module 'openapi-types' ...
node_modules/@google/adk/dist/types/tools/openapi_tool/rest_api_tool.d.ts(7,27): error TS2307: Cannot find module 'openapi-types' ...

diagnostics originating under node_modules/@google/adk-devtools/ : 1

After (tarball packed from this branch) — node_modules/@types/express is present purely as a transitive install, and every TS7016 for express is gone:

node_modules/@google/adk/dist/types/auth/auth_schemes.d.ts(6,27): error TS2307: Cannot find module 'openapi-types' or its corresponding type declarations.
node_modules/@google/adk/dist/types/tools/openapi_tool/openapi_spec_parser/openapi_spec_parser.d.ts(6,27): error TS2307: Cannot find module 'openapi-types' ...
node_modules/@google/adk/dist/types/tools/openapi_tool/openapi_spec_parser/operation_parser.d.ts(6,27): error TS2307: Cannot find module 'openapi-types' ...
node_modules/@google/adk/dist/types/tools/openapi_tool/openapi_spec_parser/tool_auth_handler.d.ts(6,27): error TS2307: Cannot find module 'openapi-types' ...
node_modules/@google/adk/dist/types/tools/openapi_tool/openapi_toolset.d.ts(6,27): error TS2307: Cannot find module 'openapi-types' ...
node_modules/@google/adk/dist/types/tools/openapi_tool/rest_api_tool.d.ts(7,27): error TS2307: Cannot find module 'openapi-types' ...

diagnostics originating under node_modules/@google/adk-devtools/ : 0

The acceptance criterion is scoped by file path: zero diagnostics under node_modules/@google/adk-devtools/. The residual six TS2307: Cannot find module 'openapi-types' come from inside node_modules/@google/adk (installed from the registry) and are the subject of a separate change to core/package.json; they are present identically before and after and were deliberately not touched here. Adding npm i --save-dev openapi-types to the scratch consumer makes the run fully green. The @google/adk and @a2a-js/sdk express diagnostics also clear in the "after" run, because npm hoists the now-transitive @types/express to the consumer's node_modules root where it is globally visible — incidental to this fix, not a substitute for the separate core/package.json change.

Second symptom (skipLibCheck: true) — the silent any. Re-checking the same two consumers with skipLibCheck: true and export const app: AdkApiServer['app'] = 42;:

### before : exit 0        <- 42 assignable, so `app` is `any`
### after  : probe.ts(2,14): error TS2322: Type 'number' is not assignable to type 'Application'.   exit 2

That error in the "after" run is the desired outcome: AdkApiServer['app'] is genuinely express.Application for consumers instead of collapsing to any.

No runtime or declaration output changed. dev/dist/types is byte-identical across the two tarballs (sha256 over the sorted file digests: d4df2eb5cc78059f77cc3f5118c954a4abae9dad86a9b469392652f902b91cce in both), and the two tarballs' dependencies blocks differ by exactly the one added line. Only whether a consumer can resolve the express import changes.

Not a breaking change, with one behaviour worth naming: a consumer type-checking with skipLibCheck: false who had been tolerating or suppressing the old TS7016 will now find AdkApiServer['app'] genuinely typed, which can surface pre-existing type errors in their own code. That is the fix working. Consumers who already installed @types/express themselves are unaffected — npm dedupes to one copy when the ranges overlap.

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.

AdkApiServer exposes `readonly app: express.Application` and is exported from dev/src/index.ts, so `tsc --emitDeclarationOnly` writes a top-level `import express from 'express'` into dev/dist/types/server/adk_api_server.d.ts, which is reachable from the package entry point. express@4 ships no bundled declarations, so those types can only come from @types/express -- and a devDependency is never installed for a consumer of the published package. Consumers therefore saw either a TS7016 raised from inside the shipped declaration (skipLibCheck: false) or a silent collapse of AdkApiServer['app'] to `any` (skipLibCheck: true). In-repo builds were masked by npm workspace hoisting plus skipLibCheck. Move @types/express from devDependencies to dependencies, keeping the range at ^4.17.21 so it stays on the same semver major as express ^4.21.2. The emitted declarations are byte-identical; only whether a consumer can resolve the express import changes.

Guards the fix against a future manifest edit silently demoting @types/express back to a devDependency, and pins express and @types/express to a shared semver major so a one-sided bump fails while a coordinated one keeps passing.

Amaad Martin added 2 commits August 2, 2026 22:10

This was referenced Aug 3, 2026

Fix: declare @types/js-yaml in dev/package.json #567

Open

Fix: align dev's express and @types/express ranges with core #637

Open

Navigation Menu

Fix: declare @types/express as a runtime dependency of @google/adk-devtools - #562

Fix: declare @types/express as a runtime dependency of @google/adk-devtools#562
AmaadMartin wants to merge 2 commits into
mainfrom
fix/dev-types-express-runtime-dep

AmaadMartin commented Aug 3, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

AmaadMartin commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Link to Issue or Description of Change

Testing Plan

Checklist

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

AmaadMartin commented Aug 3, 2026 •

edited

Loading