Fix: replace winston with a console logger in core to unbreak the web build

[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):
   Fixes part of: Web build leaks Node-only deps: the async_hooks shim is gated behind --bundle (never applied), and winston has no shim google/adk-js#611 (the winston half; the node:async_hooks half stays open — see below)
   Related: Tracking: @google/adk cannot be bundled for the browser (5 build-level defects) google/adk-js#607
2. Or, if no issue exists, describe the change:
   Problem: @google/adk advertises a browser build ("browser": "./dist/web/index_web.js"), but that artifact ships a literal import * as winston from 'winston'. core/build.js is a transpile-only esbuild pass (bundle: false, packages: 'external'), so module specifiers are copied through verbatim into core/dist/web/utils/logger.js, and winston transitively drags in os, fs, util, zlib and http. Any bundler targeting the browser fails to resolve them.

A build-layer alias is not available: core/build.js:53 gates the existing node:async_hooks alias behind platform === 'browser' && bundle, and that gate is load-bearing — esbuild rejects alias outright without bundle (✘ [ERROR] Cannot use "alias" without "bundle"). That approach was already attempted and corrected on the issue thread, so it is not re-attempted here.

Solution: Option A, as chosen by the repo owner on google#611 — drop the dependency at the source rather than patch the build for one platform. SimpleLogger only used winston for two things: a printf template, and a "level filter" that was already a pass-through (the real filter is the instance field private logLevel, and every method early-returned before touching winston). Rewriting it on console removes 49 lines and adds 29 in logger.ts, and lets winston leave core's dependencies entirely.

core/src/utils/logger.ts now imports nothing at all — no winston, no node:* builtin, no process reference. The four level methods collapse into a single log(), which builds the line inline and dispatches through a small CONSOLE_METHOD lookup. The console method is resolved at call time (never captured into a module-level constant), so vi.spyOn(console, ...) still intercepts it. That lookup replaced an exhaustive four-case switch and is strictly stricter than it: the switch had no default branch and so accepted a newly added LogLevel silently, whereas indexing the lookup with an uncovered member is a compile error — verified by temporarily adding a fifth member:

core/src/utils/logger.ts(60,13): error TS7053: Element implicitly has an 'any' type because expression of type 'LogLevel'
  can't be used to index type '{ readonly 0: "debug"; readonly 1: "info"; readonly 2: "warn"; readonly 3: "error"; }'.
  Property '[LogLevel.FATAL]' does not exist on type '{ readonly 0: "debug"; ... }'.

Nothing is added to common.ts or index.ts, so the public export surface is byte-for-byte unchanged.

Collision check. gh pr list --repo AmaadMartin/adk-js --state open --limit 1000 (555 open PRs) was scanned for adjacent work. Two open PRs touch core/src/utils/logger.ts: #432 ("deduplicate the winston logger into one level-gated implementation") and #431 ("add a getLogLevel() accessor"). Neither lands this change — #432 keeps winston in core and generalises it into a shared WinstonLogger, which is the opposite of the approved Option A, and #431 only adds an accessor. This PR is therefore branched from main rather than stacked: stacking on #432 would mean reverting most of it, and both are unmerged fork branches. If #432 lands first this file will need a manual conflict resolution, and Option A should win. No other open PR touches core/package.json's winston entry.

Behaviour differences (all four intentional, per the plan's breaking-change analysis):

1. winston is no longer a dependency of @google/adk. This is the point of the change. A consumer that relied on winston being installed transitively via @google/adk must now declare it. @google/adk-devtools declares it itself, so it is unaffected.
2. ANSI colour codes are gone. winston wrapped the level token in \x1b[3Xm...\x1b[39m. The replacement emits plain text. ANSI escapes render as literal garbage in a browser devtools console — the environment this change exists to support — and the dev CLI keeps its own colourised winston logger for terminal output. The line's text layout is unchanged: LEVEL: [ADK] <ISO-8601> <message>, with the same new Date().toISOString() timestamp winston's default format.timestamp() produced.
3. warn and error now go to stderr. winston's Console transport put all four levels on stdout (its stderrLevels set is empty for these custom levels). console.warn/console.error write to stderr in Node and light up the corresponding devtools level in a browser.
4. logger.log(level, ...) no longer throws. It previously passed the numeric level ('0'..'3') as a winston level name, which is not registered; winston logged [winston] Unknown logger level: 1 and logform's colorizer then threw TypeError: colors[Colorizer.allColors[lookup]] is not a function. No existing test covered that path. Collapsing the four methods into one log() fixes it incidentally, and there is now a regression test for it.

Not changed: message text, level-filter semantics (this.logLevel > level), the LogLevel.INFO default, the SimpleLogger/NoOpLogger class names (existing tests assert constructor.name), the public export surface, and all 41 core/src call sites — no call-site edits.

Scope kept tight, deliberately. No env-var/config knob for the log level, no typeof console !== 'undefined' guard, no ANSI support, no change to core/build.js, index_web.ts, common.ts, index.ts, or anything under dev/. No suppressions of any kind were added — no any, @ts-expect-error, eslint-disable or coverage pragma appears in this diff (eslint.config.js has no no-console rule, so console.* inside the logger lints clean without one).

node:async_hooks — reported, not fixed. Removing winston does not remove the second leak, and this PR deliberately does not attempt it. Verified on the built artifact:

$ grep -rn "winston" core/dist/web            # BEFORE: 9 hits
core/dist/web/utils/logger.js:8:import * as winston from "winston";
core/dist/web/utils/logger.js:19:    this.logger = winston.createLogger({
... (7 more)

$ grep -rn "winston" core/dist/web            # AFTER: no output
$ grep -rn "node:async_hooks" core/dist/web   # AFTER: still exactly 1 hit
core/dist/web/utils/client_labels.js:8:import { AsyncLocalStorage } from "node:async_hooks";

It is pulled in by core/src/utils/client_labels.ts:7, which is reachable from the web entry point via index_web.ts → export * from './common.js' → core/src/common.ts:282 (export {getClientLabels, runWithClientLabel} from './utils/client_labels.js'). core/src/utils/async_hooks_shim.ts exists and is correct but is only wired through buildOptions.alias, which esbuild refuses without bundle, so it is dead in every published artifact. That is a separate task; google#611 should stay open for it.

Lockfile. The only package-lock.json change is the single "winston": "^3.19.0", line inside the "core" workspace's dependencies block (1 file changed, 0 insertions(+), 1 deletion(-)). The "dev" workspace block and the node_modules/winston entry are untouched, because dev still depends on winston. npm ci was used (never npm install) and passes, which is itself the proof the hand-edit is correct and complete; grep -c us-npm.pkg.dev package-lock.json → 0, and prettier still resolves to 3.8.4.

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.

Seven tests were added as a new top-level describe('SimpleLogger', ...) block in the existing core/test/utils/logger_test.ts. Nothing in the existing describe('setLogger', ...) block was edited, weakened, skipped or deleted — it passes unchanged.

$ npx vitest run --project unit:core core/test/utils/logger_test.ts
 Test Files  1 passed (1)
      Tests  15 passed (15)          # 8 pre-existing + 7 new

Coverage of core/src/utils/logger.ts: 100% branch, 89.87% statements. Every uncovered statement (lines 129, 130, 132, 133, 138, 139, 141, 142) is inside the pre-existing deprecated logger const wrapper at the bottom of the file, which this change does not touch. All new code — every branch of SimpleLogger — is 100% line- and branch-covered. The single-file run also reports a function-threshold miss (78.26% vs 88%), which is the same untouched wrapper's six methods measured against a global threshold applied to a one-file selection; it is not a real gate and does not appear in the repo-wide run.

Proving each test can fail. Two techniques were needed, because the winston baseline never touches console at all and would pass negative assertions vacuously.

Against the unfixed source (git checkout main -- core/src/utils/logger.ts, re-run, restore) — all 7 new tests fail, the 8 pre-existing ones still pass:

 × SimpleLogger > emits a message at the configured level
   → expected "info" to be called 1 times, but got 0 times
 × SimpleLogger > suppresses a message below the configured level
   → expected "warn" to be called 1 times, but got 0 times
 × SimpleLogger > defaults to INFO
   → expected "info" to be called 1 times, but got 0 times
 × SimpleLogger > routes each level to its matching console method
   → expected "debug" to be called 1 times, but got 0 times
 × SimpleLogger > joins arguments with a single space
   → expected "info" to be called with arguments: [ StringMatching{…} ]
 × SimpleLogger > log() emits without throwing
   → expected [Function] to not throw an error but 'TypeError: colors[Colorizer.allColors…' was thrown
     TypeError: colors[Colorizer.allColors[lookup]] is not a function
 × SimpleLogger > formats the full line for a warning
   → expected "warn" to be called with arguments: [ StringMatching{…} ]
 Tests  7 failed | 8 passed (15)

Note that test 6 reproduces the real crash described above, and that test 2 fails on its positive half — a "not called" assertion alone would have passed vacuously here, which is why both halves are asserted in one test.

By mutating the new source (each mutation reverted afterwards):

Mutation	Result
Delete if (this.logLevel > level) { return; } from SimpleLogger.log	× suppresses a message below the configured level → expected "debug" to not be called at all, but actually been called 1 times; × defaults to INFO → expected "debug" to not be called at all, but actually been called 1 times — 2 failed | 13 passed
private logLevel: LogLevel = LogLevel.DEBUG	× defaults to INFO → expected "debug" to not be called at all, but actually been called 1 times — 1 failed | 14 passed
messages.join(',') instead of join(' ')	× joins arguments with a single space → expected "info" to be called with arguments: [ StringMatching{…} ] — 1 failed | 14 passed
Point [LogLevel.WARN] at 'info' in the CONSOLE_METHOD lookup	× suppresses a message below the configured level → expected "warn" to be called 1 times, but got 0 times; × routes each level to its matching console method → expected "info" to be called 1 times, but got 2 times; × formats the full line for a warning → expected "warn" to be called with arguments: [ StringMatching{…} ] — 3 failed | 12 passed
Drop the [ADK] label from the line template	× emits a message at the configured level; × routes each level to its matching console method → expected "debug" to be called with arguments: [ StringContaining "DEBUG: [ADK] " ]; × joins arguments with a single space; × formats the full line for a warning — 4 failed | 11 passed

Collateral check. The default level is INFO, so core's logger output now flows through console.info/warn/error in-process. The four dev test files that spy on those methods were run to confirm the extra calls break nothing:

$ npx vitest run --project unit:dev dev/test/cli/cli_deploy_cloud_run_test.ts \
    dev/test/cli/cli_deploy_agent_engine_test.ts dev/test/utils/agent_loader_test.ts \
    dev/test/server/adk_api_server_test.ts
 Test Files  4 passed (4)
      Tests  116 passed (116)

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

The build artifact is the end-to-end test for this fix — no mocks, the real published-shape output:

npm ci
cd core && npm run build     # note: no --bundle; this is exactly what ships
grep -rn "winston" dist/web            # BEFORE: 9 hits   AFTER: no output
grep -rn "node:async_hooks" dist/web   # BEFORE: 1 hit    AFTER: 1 hit (unchanged, see above)

The issue's bundler repro was also run against the built web entry point. Locally winston itself still resolves (the dev workspace keeps it in node_modules, so esbuild walks into it rather than reporting Could not resolve "winston" the way a published-package consumer would); the measurable effect is that winston's transitive Node-builtin imports disappear from the unresolved set — 345 → 325 unresolved import sites:

npx esbuild core/dist/web/index_web.js --bundle --platform=browser \
  --outfile=/dev/null --log-limit=0 2>&1 | grep "Could not resolve"

module	before	after
util	20	12
os	14	8
fs	25	23
https	14	13
path	13	12
http	7	6
zlib	3	2

No module disappears from the list entirely, because every one of those builtins is also reached through other dependencies and through the node:async_hooks chain that this PR reports rather than fixes.

Other gates, run locally on the exact commit pushed: npm run build (exit 0), npx eslint core/src/utils/logger.ts core/test/utils/logger_test.ts (clean), npm run format:check (clean, repo-wide), npm run docs:check (exit 0), scripts/check_license.sh (clean), npx secretlint (clean). npm run ts:check is not run by CI and is dirty on main with 281 pre-existing dist/types-vs-src identity errors in unrelated test files; that count is unchanged by this PR and zero of those errors are in either file touched here.

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.

[AmaadMartin]

Closing: Option A was rejected upstream.

This was ported to google#617, which the maintainer (kalenkevich) closed 13 minutes later with:

No, we need to keep winston! Please close this pr

So dropping the winston dependency is off the table. Issue google#611 will instead be addressed via Option B (platform-split the logger: winston stays in a Node-only module, the web build gets a console implementation wired through the existing setLogger seam), which keeps winston on Node as requested.

Branch left intact — the console-logger implementation written here is directly reusable as the web-side implementation under Option B.

Do not re-port this PR.