Fix: clean up compiled-agent temp directories synchronously on process exit

[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: adk web, adk api_server and adk deploy ... leave a compiled-agent scratch directory behind in the OS temp directory on every run, and a new one accumulates each time.

AgentLoader's constructor installs its cleanup as an async 'exit' listener:

const exitHandler = async ({exit, cleanup}) => {
  if (cleanup) {
    await this.disposeAll();   // async
  }
  if (exit) {
    process.exit();
  }
};

process.on('exit', () => exitHandler({cleanup: true}));
process.on('SIGINT', () => exitHandler({exit: true}));
...

Node requires an 'exit' listener to be fully synchronous: the process terminates as soon as the last listener returns, and any pending promise or queued I/O is dropped. AgentLoader.disposeAll() awaits AgentFile.dispose(), which awaits fsPromises.unlink() and removeFolder(). None of those awaits ever resolve during exit, so the output directory created by AgentFile.load() (getTempDir('adk_agent_loader'), containing the compiled <agent>.cjs/.mjs and a node_modules symlink) survives the process.

Ctrl-C leaks for the same reason: SIGINT/SIGUSR1/SIGUSR2/uncaughtException all take the {exit: true} branch and call process.exit(), which synchronously emits 'exit' — whose listener is broken as above. process.exit() also cuts the current stack, so the try/finally blocks that call agentLoader.disposeAll() in cli_deploy_cloud_run.ts and cli_deploy_agent_engine.ts are never reached either.

Separately, AdkApiServer.stop() closed the HTTP server and nothing else. When no agentLoader is supplied through ServerOptions, the constructor builds one itself — that loader and its compiled directories outlive every start()/stop() cycle of an embedded server.

Solution: a synchronous best-effort cleanup path, which is the only construct that can do any work at all inside an 'exit' listener.

• AgentFile.disposeSync(): void removes the compiled output directory with fs.rmSync(dir, {recursive: true, force: true}). force: true makes a missing target a no-op; the catch covers EBUSY/EPERM, which is a real possibility on Windows, and reports through the module's existing AdkLogger rather than rethrowing — at exit there is no caller left to handle a failure, and one failed unlink must not stop the remaining agent files from being cleaned up or change the exit code.
• AgentLoader.disposeAllSync(): void closes the directory watcher and calls disposeSync() on every preloaded agent file, mirroring disposeAll().
• The 'exit' listener drops async/await and calls disposeAllSync(). The signal handlers are unchanged: they still call process.exit(), which synchronously emits 'exit', which now performs real cleanup. Ctrl-C stays instantaneous — no timer, no re-entrancy flag, no second-signal escape hatch, no new latency on the interactive path. The rejected alternative (awaiting disposeAll() in the signal handler with a bounded timeout plus a force-exit on a second signal) adds all of that and still leaves plain 'exit' broken.
• AdkApiServer records ownsAgentLoader and stop() disposes the loader it built itself in a finally, so the loader is released even when server.close() reports an error. A loader supplied through ServerOptions.agentLoader belongs to the caller and is never disposed.

Notes on the implementation:

• fs.rmSync does not follow symlinks, so it unlinks the node_modules symlink that linkProjectNodeModules() creates inside the output directory rather than deleting the project's real node_modules. This is load-bearing and is pinned by a test.
• No removeFolderSync was added to file_utils.ts: a single fs.rmSync call is not a helper cluster, the existing removeFolder has different (swallow-and-console.error) semantics, and file_utils.js is wholesale-mocked by agent_loader_test.ts — routing through it would make the new tests assert against a mock instead of the real filesystem.
• disposeSync() guards on cleanupDirPath, which load() always assigns in the same block as cleanupFilePath, so there is no unreachable ?? cleanupFilePath fallback to leave as a permanently-uncovered branch. Behaviour is identical in every reachable state: an agent file that was never compiled owns no artifact, stays usable, and getFilePath() keeps returning the original source path.
• The async dispose()/disposeAll() are untouched and keep their exact semantics. AgentLoader/AgentFile are not exported from dev/src/index.ts, so the two new methods are not new public API surface. stop() gains the async keyword; its declared return type is unchanged.
• Intentional, narrow behaviour change: AdkApiServer.stop() now disposes a self-constructed loader. Callers that inject their own loader see no change; every AdkApiServer built in dev/test/server/adk_api_server_test.ts today injects an agentLoader, so no existing test changes behaviour.

Collision check. All 551 open PRs on this fork were listed and every plausibly adjacent one was diffed before starting:

• #511 fix/agent-loader-opt-in-process-handlers — refactors the same constructor block into an opt-in installProcessHandlers(), but its listener is () => void this.disposeAll(), i.e. still the async-dropped-work bug. It does not fix this leak. The two changes touch adjacent lines and will be reconciled mechanically by whichever lands second; this diff is deliberately minimal to keep that easy, and does not introduce an opt-in flag.
• #452 fix/adk-api-server-stop-close-all-connections — also edits stop(), but only adds closeAllConnections() inside the close callback; different concern, no loader disposal.
• #604 fix/atomic-temp-dir-creation / #612 — make getTempDir atomic (mkdtemp); same files, unrelated concern.
• #633 feat/lazy-agent-discovery — splits discovery from loading; adds no sync disposal.

No open PR implements synchronous cleanup or loader ownership in stop(), so this is built on main rather than stacked.

Two related defects found while investigating are deliberately out of scope and are tracked separately: the await using misuse in adk_api_server.ts that disposes a cached AgentFile after every request, and the adk deploy staging folder (options.tempFolder) that leaks on Ctrl-C.

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
#  Test Files  2 passed (2)
#       Tests  94 passed (94)
npm run lint          # clean
npm run format:check  # clean

CI is green on ubuntu-latest, macos-latest and windows-latest. The windows job needed re-runs for two flakes that are unrelated to this diff and are documented on other branches: tests/integration/app_loader/app_loader_test.ts > should discover apps vs agents ... timing out at 40s (also failed on feat/lazy-agent-discovery, run 30911147137) and core/test/code_executors/unsafe_local_code_executor_test.ts > should execute shell code and return stdout timing out at 5s (also failed on fix/dev-workspace-undeclared-dependencies, run 30828447776 — and this PR changes no file in the core package).

12 new test cases, all additive — no existing test was edited, renamed, skipped or deleted.

dev/test/utils/agent_loader_test.ts

1. disposeSync() removes the compiled output directory, with no await between the call and the assertion — that ordering is what pins the bug — and getFilePath() then throws 'Agent is disposed and can not be used'.
2. disposeSync() twice is a no-op (the second call issues no further rmSync).
3. disposeSync() after dispose() touches the filesystem again — the mutual-idempotency invariant.
4. An uncompiled AgentFile stays usable after disposeSync(): getFilePath() still returns the original source path and the source file is untouched.
5. An fs.rmSync failure is swallowed: disposeSync() does not throw, the directory survives, and the file is still marked disposed.
6. disposeSync() does not follow the node_modules symlink — <agentsDir>/node_modules/@google/adk still resolves afterwards.
7. AgentLoader.disposeAllSync() removes every preloaded agent's directory, again with no await in between.
8. The 'exit' listener the constructor installs cleans up synchronously. The listener is located by diffing process.listeners('exit') and invoked directly rather than via process.emit('exit'), because every other loader built earlier in the file has its own listener registered; the listeners this test adds are removed in a finally.
9. The SIGINT handler calls process.exit() (which is what synchronously emits 'exit'), pinning the Ctrl-C chain.
10. disposeAllSync() closes the directory watcher, so the sync exit path leaves no watcher behind.

dev/test/server/adk_api_server_test.ts (new describe('AgentLoader ownership'))

11. stop() disposes the loader the server created itself. This drives a real esbuild compile of a dependency-free agent fixture through GET /list-apps (which is what triggers preloadAgents(); start() alone does not), asserts one directory appeared in a scratch root the test owns, then asserts it is empty after await stop().
12. stop() releases its own loader even when close() errors — a second stop() rejects with ERR_SERVER_NOT_RUNNING and disposeAll() still runs, which is the error path the finally exists for.
13. stop() does not dispose a caller-supplied loader (a real AgentLoader with a spied disposeAll).

Proof that each new test can fail. Every test was run against mutated source; each mutation was reverted before the next.

Mutation	Result
'exit' handler back to async + await this.disposeAll()	test 8 fails: AssertionError: expected true to be false (the directory survives)
disposeSync() body made asynchronous (void fsPromises.rm(...))	tests 1, 2, 6, 7, 8 fail: expected true to be false
fs.rmSync(target, {recursive: true, force: true}) → fs.rmSync(this.cleanupFilePath, {force: true})	tests 1, 2, 6, 7, 8 fail: the directory survives
try/catch removed from disposeSync()	test 5 fails: expected [Function] to not throw an error but 'Error: EBUSY: resource busy or locked' was thrown
disposed short-circuit dropped from disposeSync()	tests 2 and 3 fail: expected 3 to be 2 / expected 4 to be 3 (an extra rmSync)
uncompiled AgentFile marked disposed anyway	test 4 fails: Agent is disposed and can not be used
this.watcher?.close() removed from disposeAllSync()	test 10 fails: expected "close" to be called 1 times, but got 0 times
if (exit) process.exit() → this.disposeAllSync()	test 9 fails: expected [Function] to throw an error
ownsAgentLoader guard removed from stop()	test 13 fails: expected "disposeAll" to not be called at all, but actually been called 1 times
disposeAll() call removed from stop() entirely	test 11 fails: expected [ 'agent-0' ] to deeply equal []
finally in stop() replaced by sequential statements	test 12 fails: expected "disposeAll" to be called 2 times, but got 1 times

Coverage. 100% of the statements and branches added or changed by this PR are covered (measured with @vitest/coverage-v8 over the two suites): AgentFile.disposeSync, AgentLoader.disposeAllSync, the constructor's exit handler (including the if (exit) branch, which was uncovered before this PR), the ownsAgentLoader field and both sides of the guard in stop().

One deviation worth flagging: the plan called for vi.spyOn(nodeFs, 'rmSync') to exercise the failure branch. That is not possible — Vitest rejects it with "Cannot spy on export rmSync. Module namespace is not configurable in ESM". The test file therefore installs a pass-through vi.mock('node:fs') that delegates every export to the real implementation and only makes rmSync throw while a flag is set (the same mock records fs.watch instances, because FSWatcher is not an export of node:fs and capturing the instance is the only way to observe that the loader closed it). No any, no @ts-expect-error, no eslint-disable, and no type-checker or linter suppression of any kind is added by this PR.

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

1. ls <os.tmpdir()>/adk_agent_loader and note the current entries, if any.
2. From a sample agent project, run npx adk api_server ., then curl localhost:8000/list-apps so the agent is compiled. Confirm a new <uuid> directory appeared under <os.tmpdir()>/adk_agent_loader.
3. Press Ctrl-C. The shell returns immediately (no perceptible delay) and the directory is gone. Before this change it remained.
4. Repeat with npx adk web . and confirm the same.
5. Run npx adk deploy cloud_run . --project <project> --region <region> and interrupt it with Ctrl-C mid-deploy; the adk_agent_loader directory is gone. (The separate --temp_folder staging directory is a known, separately tracked leak and is still expected to remain.)

No new integration test is warranted: the existing suite already spawns real adk web/adk api_server processes and kills them with SIGINT, and this fix stops those runs littering the temp directory without changing any observable server behaviour.

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]

Superseded by #861, which folds this change onto #511's opt-in registration lifecycle. Every commit and every test from this PR is carried over there, except the duplicates named in the #861 body next to the surviving test. The branch stays, so this PR stays readable.