Fix: make AgentLoader process exit/signal handlers opt-in and removable

[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):
   No existing public issue; found during a review of AgentLoader lifecycle handling.

2. Or, if no issue exists, describe the change:
   Problem: The AgentLoader constructor unconditionally registered five process listeners (dev/src/utils/agent_loader.ts), so merely constructing a loader mutated global process state. Two defects followed.

3. The uncaughtException listener swallowed every crash in the process. Its handler ignored the error argument and called process.exit() with no code, i.e. exit status 0 and no stack trace. Once a single AgentLoader existed anywhere in the process, any unhandled exception — including one thrown by code unrelated to agent loading — became a silent, successful-looking exit. Registering the listener also suppressed Node's default crash reporting process-wide. In a Vitest worker this is actively dangerous: a later unhandled error tears the worker down instead of failing the test.

4. The listeners leaked. disposeAll() closed the file watcher and disposed the cached AgentFiles but left all five listeners attached, so every loader ever constructed added one listener per event for the process lifetime. AdkApiServer constructs a loader whenever the caller does not inject one, so the leak was reachable without ever naming AgentLoader. The existing loader test file already builds eight loaders in one worker, against Node's default defaultMaxListeners of 10.

Solution: Registration is now opt-in and removable.

• The constructor registers nothing. process.listenerCount(e) is unchanged for every e after new AgentLoader(...).
• A new installProcessHandlers() wires exit, SIGINT, SIGUSR1 and SIGUSR2 to the loader's cleanup. It is idempotent (a second call while installed is a no-op) and is only called from entrypoints that own the process: adk deploy cloud_run, adk deploy agent_engine, and AdkApiServer behind a new opt-in installProcessHandlers server option that adk web / adk api_server pass.
• No uncaughtException listener is installed at all (grep -rn "uncaughtException" dev/src now returns nothing). An uncaught exception keeps Node's default behaviour: stack trace printed, non-zero exit. Cleanup parity is retained because Node still emits exit on that path.
• disposeAll() removes every listener the loader installed, so install → dispose is listener-count neutral and a later install re-installs.

Why an explicit method rather than a constructor flag: the loader's teardown entry point is already disposeAll(), which every consumer calls, so the install/remove pair hangs off the existing lifecycle instead of adding a second one. Bookkeeping is a single private removeProcessHandlers?: () => void closure over the two listeners rather than a registry of {event, handler} records — the presence of that closure is the installed/not-installed state, which is what makes the guard and the teardown one line each, and there is no public uninstall method because disposeAll() is the single teardown door.

Behavioural change for embedders (intended, and the point of the fix): an embedder of AdkApiServer that relied on the server's internal loader installing signal handlers loses that side effect unless it passes installProcessHandlers: true. An embedded server must not call process.exit() on its host's behalf. adk CLI behaviour is unchanged — web, api_server, deploy cloud_run and deploy agent_engine all opt in, so their exit/SIGINT/SIGUSR1/SIGUSR2 behaviour is what it is today. The single deliberate difference is that an uncaught exception now crashes loudly with a non-zero exit code instead of exiting 0 silently.

Collision check (required before implementation): gh pr list --repo AmaadMartin/adk-js --state open --limit 1000 (408 open PRs) was filtered for agent-loader / process-handler / signal / listener keywords, and the diffs of the 14 plausibly adjacent PRs (#480, #457, #455, #375, #365, #328, #309, #285, #276, #275, #264, #260, #257, #247) were grepped for process.on|installProcessHandlers|uncaughtException|removeListener|SIGINT|SIGUSR|exitHandler. No open PR touches the process-handler block. Several touch dev/src/utils/agent_loader.ts in unrelated regions (esbuild options, logging, discovery performance); since none implements this change, this PR branches from main rather than stacking.

Scope note: the exit listener performs best-effort cleanup (void this.disposeAll()) exactly as before — Node cannot await inside an exit listener, so its async tail does not complete. That pre-existing limitation, making SIGINT await cleanup before exiting, and having AdkApiServer.stop() dispose the loader are deliberately out of scope here and left unchanged.

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.

npx vitest run --project unit:dev \
  dev/test/utils/agent_loader_test.ts \
  dev/test/server/adk_api_server_test.ts \
  dev/test/cli/cli_test.ts \
  dev/test/cli/cli_deploy_cloud_run_test.ts \
  dev/test/cli/cli_deploy_agent_engine_test.ts
#  Test Files  5 passed (5)     Tests  155 passed (155)

npm run build && npm run lint && npm run format:check && npm run docs:check   # all clean
npx tsc --noEmit   # no new errors (281 pre-existing errors on this tree both with and without this change; none in dev/)

New tests (all additions; no existing test was rewritten, weakened, skipped or deleted):

• dev/test/utils/agent_loader_test.ts — a describe('process handlers') block: constructor registers nothing (3 loaders, counts unchanged); install adds exactly one listener to each of the four events; never installs an uncaughtException listener; idempotent; disposeAll() removes them; reinstall after dispose works; disposeAll() on a never-installed loader is a no-op; the captured exit listener disposes cached agents; the captured SIGINT listener calls process.exit(). These tests never process.emit — that would fire Vitest's and Tinypool's own listeners and can take the worker down — they diff process.listeners(event) to capture this loader's listener and invoke it directly, and every test disposes in a finally so a failure cannot leak listeners into the rest of the worker.
• dev/test/server/adk_api_server_test.ts — the loader's installProcessHandlers is not called by default and is called exactly once when opted in; both also assert the real SIGINT listener count, so the flag is proven to reach process state rather than just the spy.
• dev/test/cli/cli_test.ts — web and api_server both pass installProcessHandlers: true.
• dev/test/cli/cli_deploy_{cloud_run,agent_engine}_test.ts — each deploy path installs handlers on its loader. The AgentLoader mock implementations in these two files gained an installProcessHandlers: vi.fn() member (4 sites); without it the deploy functions throw TypeError: agentLoader.installProcessHandlers is not a function. That is a mock-shape fix, not an assertion change — no existing assertion was touched.

Coverage: 100% of new lines and branches. Verified from the v8 JSON report over the five suites above: dev/src/utils/agent_loader.ts lines 369 and 386–406 (the idempotency guard, both listener bodies, the install and removal loops, and the closure clearing itself) and 489, dev/src/server/adk_api_server.ts 157–158 (both branches), dev/src/cli/cli.ts 242 and 289, and the deploy call sites are all absent from the uncovered sets. No coverage-tool suppression was added anywhere.

Proof each new test can fail. Every new test was run against mutated source and observed to fail:

Mutation	Test(s) killed	Failure message
Re-add the 5 process.on(...) calls to the constructor	does not register process listeners in the constructor, never installs an uncaughtException listener, disposeAll is a no-op…	expected [ 3, 3, 3, 3, 4 ] to deeply equal [ +0, +0, +0, +0, 1 ] / expected 6 to be 5
Add process.on('uncaughtException', …) inside installProcessHandlers()	installs exit and termination signal listeners on demand, never installs an uncaughtException listener, is idempotent, removes installed listeners on disposeAll, can reinstall after disposeAll	expected [ 1, 1, 1, 1, 2 ] to deeply equal [ 1, 1, 1, 1, 1 ]
Delete this.removeProcessHandlers?.(); from disposeAll()	removes installed listeners on disposeAll, can reinstall after disposeAll	expected [ 4, 4, 4, 4, 1 ] to deeply equal [ 3, 3, 3, 3, 1 ]
Delete the idempotency early-return	is idempotent	expected [ 2, 2, 2, 2, 1 ] to deeply equal [ 1, 1, 1, 1, 1 ]
Removal closure forgets the three signals (removes only exit)	removes installed listeners on disposeAll, can reinstall after disposeAll	expected [ +0, 4, 4, 4, 1 ] to deeply equal [ +0, 3, 3, 3, 1 ]
Removal closure does not clear itself, so reinstall is blocked	can reinstall after disposeAll	expected [ +0, +0, +0, +0, 1 ] to deeply equal [ 1, 1, 1, 1, 1 ]
onExit no longer disposes	the exit listener disposes cached agents	expected "disposeAll" to be called at least once
onSignal no longer exits	a termination signal listener exits the process	expected [Function] to throw an error
Drop installProcessHandlers: true from the web action (and separately from api_server)	the matching should opt the server into process handlers	expected undefined to be true
Server ignores the opt-in flag	installs agent loader process handlers when opted in	expected "installProcessHandlers" to be called once, but got 0 times
Server installs unconditionally	both server cases	expected "installProcessHandlers" to be called once, but got 0 times (and the default case)
Drop agentLoader.installProcessHandlers() from both deploy paths	both installs process handlers on the agent loader	expected "spy" to be called once, but got 0 times

The idempotency guard mutation is worth calling out: the combination test (is idempotent) is what pins it — line coverage of installProcessHandlers alone is satisfied without ever calling it twice.

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

Run npm run build -w dev first, then:

1. A crash surfaces again. repro1.mjs: import AgentLoader from dev/dist/esm/utils/agent_loader.js, new AgentLoader(process.cwd()), then setTimeout(() => { throw new Error('this should crash loudly'); }, 10).

   Error: this should crash loudly
       at Timeout._onTimeout (file:///tmp/adk_repro/repro1.mjs:4:9)
   exit code: 1
   

   On main this prints nothing and exits 0.
2. No leak. Construct 20 loaders and print the counts:

   exit listeners: 0
   SIGINT listeners: 0
   uncaughtException listeners: 0
   

   No MaxListenersExceededWarning. On main this is 20 per event plus the warning.
3. CLI behaviour unchanged. node dev/dist/esm/cli_entrypoint.js web dev/samples --port 0 → For local testing, access at http://localhost:42399.; SIGINT → web: terminated on SIGINT (exit 0). Same for api_server (http://localhost:40635, terminated on SIGINT (exit 0)).
4. Embedded server is clean. new AdkApiServer({agentsDir}) → embedded -> SIGINT:0 exit:0; new AdkApiServer({agentsDir, installProcessHandlers: true}) → opted in -> SIGINT:1 exit:1.

Regression gate — integration suite, unmodified: npx vitest run --project integration tests/integration/app_loader/app_loader_test.ts → Test Files 1 passed (1) Tests 6 passed (6). This suite is only green once its fixture node_modules exist; from cold it fails in beforeAll at execAsync('npm install') with Hook timed out in 40000ms (a cold install of the fixture measured 1m26s here) before any loader code runs. That timeout is pre-existing and environmental, not caused by this change.

Pre-existing failures on this tree, unrelated to this change (both reproduced on unmodified fork/main in a detached worktree): dev/test/cli/cli_create_test.ts > should handle Vertex AI selection with gcloud defaults (depends on the local gcloud default project) and the e2e project (requires live Gemini API credentials).

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.

No @ts-expect-error, @ts-ignore, eslint-disable, as any, as never, as unknown as or coverage-tool suppression was added by this branch (git diff main -U0 | grep -E '^\+.*(@ts-expect-error|…)' returns nothing). TERMINATION_SIGNALS is declared as const so its members satisfy the process.on/removeListener signal overloads without referencing the NodeJS global namespace, which the repo's no-undef lint rule rejects. The new CLI/deploy assertions read mock metadata through vi.mocked(...) rather than the as unknown as Mock cast used elsewhere in those files.