Fix: remove the no-op @ts-ignore on the esbuild-shim-plugin import

[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):
   N/A — no existing issue.

2. Or, if no issue exists, describe the change:

Problem: dev/src/utils/agent_loader.ts carried an unreasoned checker suppression on the esbuild-shim-plugin import:

import esbuild from 'esbuild';
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
import {shimPlugin} from 'esbuild-shim-plugin';

This @ts-ignore suppresses nothing. The natural assumption is that esbuild-shim-plugin ships no type declarations — that assumption is wrong, and it was checked rather than guessed:

• esbuild-shim-plugin@1.0.3 (the version pinned by package-lock.json) declares "types": "dist/index.d.ts" and ships it:

  $ cat node_modules/esbuild-shim-plugin/dist/index.d.ts
  import { Plugin } from 'esbuild';
  
  declare const shimPlugin: () => Plugin;
  
  export { shimPlugin as default, shimPlugin };
  

• TypeScript really resolves it under this repo's moduleResolution: nodenext. The package's exports map has no "types" condition, but it does not need one — TS resolves "import": "./dist/index.js" and then applies its .js → .d.ts substitution rule. From npx tsc --noEmit --traceResolution -p tsconfig.json:

  File name '<repo>/node_modules/esbuild-shim-plugin/dist/index.js' has a '.js' extension - stripping it.
  File '<repo>/node_modules/esbuild-shim-plugin/dist/index.d.ts' exists - use it as a name resolution result.
  ======== Module name 'esbuild-shim-plugin' was successfully resolved to
           '<repo>/node_modules/esbuild-shim-plugin/dist/index.d.ts'
           with Package ID 'esbuild-shim-plugin/dist/index.d.ts@1.0.3'. ========
  

• Direct proof the directive is dead. Swapping @ts-ignore for @ts-expect-error (which errors when it has nothing to suppress) and compiling gives:

  dev/src/utils/agent_loader.ts(10,1): error TS2578: Unused '@ts-expect-error' directive.
  

  TS2578 is conclusive: there is no error on that import line.

• Corroborating evidence already in the repo: dev/build.js:15 imports the very same symbol from the very same package with no suppression at all. If the import needed one, that line would be a problem too.

Left in place, the directive is worse than noise — it blinds the compiler to any future breakage of this import. If esbuild-shim-plugin ever changes its export shape, @ts-ignore would silently swallow the resulting error.

Solution: delete both lines. They are a co-dependent pair: ban-ts-comment fires on @ts-ignore, so the eslint-disable is load-bearing only because of the @ts-ignore beneath it. Removing the @ts-ignore alone would leave a stale directive behind.

 import esbuild from 'esbuild';
-// eslint-disable-next-line @typescript-eslint/ban-ts-comment
-// @ts-ignore
 import {shimPlugin} from 'esbuild-shim-plugin';

1 file changed, 2 deletions(-). Net suppression count falls by exactly 2; git grep "@ts-ignore" -- "*.ts" now returns zero hits repo-wide. No new suppression of any kind was introduced.

Why there is no new .d.ts. Hand-writing declare module 'esbuild-shim-plugin' would not just be redundant, it would be unsound. Because the module already resolves, TypeScript treats such a block as a module augmentation that merges into the package's real declarations — so a hand-written signature that drifts from upstream (e.g. one accepting options the real zero-argument function ignores) would compile cleanly with no error to warn anyone. That would reintroduce exactly the class of silent unsoundness this change removes.

Runtime impact: none. This is a comment-only deletion. shimPlugin() is still wired into the esbuild plugins array at agent_loader.ts:189, unchanged.

Collision check. Before starting I checked the 285 open PRs on this fork for a competing or overlapping change (gh pr list ... --json number,title,headRefName, then gh pr diff --name-only / full-diff greps on every plausibly adjacent PR). No open PR touches @ts-ignore, ban-ts-comment, or esbuild-shim. Six open PRs do touch dev/src/utils/agent_loader.ts (#365, #328, #309, #285, #275, #264), but the earliest hunk in any of them starts at line 49 — none touches the import block at lines 6–16, so there is no line-level overlap and this branch is based on main rather than stacked.

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. — No; deliberately none, see below.
[x] All unit tests pass locally.

Why no unit test accompanies this change. The change adds zero new lines of executable code — it deletes two comment lines, which are erased at compile time and counted by no coverage tool. There is no behavior delta to assert: the emitted JavaScript is byte-identical (demonstrated below). Any test written for this change would pass identically against the fixed and unfixed code, which is a green light with no signal. Fabricating one to hit a coverage number would be worse than writing none.

The real regression guard here is static and machine-checkable, and both gates were run:

$ git grep -c "@ts-ignore" -- "*.ts"      # zero hits (git grep exits 1 and prints nothing)
$ npx eslint "**/*.ts" --max-warnings 0   # exit 0 — proves no stale eslint-disable was left behind

The strict --max-warnings 0 form matters: ESLint reports unused directives only as warnings, so plain npm run lint would not catch a half-done job.

Full acceptance run, all on commit bbda488:

Command	Result
npm run build	exit 0
npm run lint	exit 0
npx eslint "**/*.ts" --max-warnings 0	exit 0
npm run format:check	exit 0 — "All matched files use Prettier code style!"
npm run docs:check	exit 0
npx vitest run --project unit:dev dev/test/utils/agent_loader_test.ts	30/30 passed
npx vitest run --project unit:dev	222 passed, 1 pre-existing failure (below)

ts:check error set is unchanged. Captured npx tsc --noEmit --pretty false on a clean tree and again with the change applied: 280 errors before, 280 after, and diff of the two reports the outputs are byte-identical. These 280 are pre-existing and out of scope (the root tsconfig.json declares no include, so tsc --noEmit sweeps test files that rely on Vitest path aliases); none of them is in agent_loader.ts and none mentions the esbuild-shim-plugin module specifier. Note --pretty false is required — ANSI colour codes sit between error and TS in the default output, so grep -c "error TS" silently matches nothing.

Pre-existing failures, verified unrelated (both reproduced on pristine main at fcfd043 with zero diff applied):

• dev/test/cli/cli_create_test.ts > createAgent > Interactive Mode > should handle Vertex AI selection with gcloud defaults — asserts a mocked project id of gcloud-project but picks up the host machine's real configured gcloud default project. Fails identically on main.
• tests/integration/agent_loader/agent_dirname_test.ts — its beforeAll runs npm install inside the fixture with a 40 s budget and times out in this network-restricted environment, so the assertions never execute. Fails on main too.

Manual End-to-End (E2E) Tests

1. The emitted output is byte-identical — the strongest possible demonstration that this is behavior-preserving:

git stash          # pristine HEAD
npm run build -w dev
cp dev/dist/esm/utils/agent_loader.js /tmp/agent_loader.before.js
git stash pop      # reapply the 2-line deletion
npm run build -w dev
diff /tmp/agent_loader.before.js dev/dist/esm/utils/agent_loader.js

diff reports no differences (exit 0; 15024 bytes both times).

2. Real no-mock load through AgentFile.load(). The existing agent_loader_test.ts mocks esbuild, so it does not exercise shimPlugin for real. To prove the import still works at runtime I drove the built dev/dist/esm/utils/agent_loader.js against a scratch .ts agent that uses all three constructs shimPlugin injects — __dirname (reading a sidecar JSON beside it), __filename, and import.meta.url:

RUNTIME E2E OK: read-via-__dirname|agent.ts|meta-ok

And that harness was proved able to fail. Mutating the source to drop shimPlugin() from the esbuild plugins array and rebuilding makes the same run die with:

Error: Dynamic require of "util" is not supported
    at file:///tmp/adk_agent_loader/.../agent.mjs:1:387

Restoring shimPlugin() returns it to RUNTIME E2E OK. So the check has real signal, and it confirms the plugin is still correctly wired after the suppression was removed. (The harness was scratch scaffolding and is intentionally not part of this diff — it asserts pre-existing behavior that this comment-only change cannot alter.)

To reproduce manually instead: point the ADK CLI at tests/integration/agent_loader/__dirname/agent.ts and confirm it still compiles, bundles, and loads.

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. — N/A; this change only removes comments.
[x] I have added tests that prove my fix is effective or that my feature works. — See "Why no unit test accompanies this change" above: no executable lines are added and no behavior changes, so the guard is the static gate plus the byte-identical-output check.
[x] New and existing unit tests pass locally with my changes.