Fix: bill app_loader discovery fixture compilation to beforeAll instead of the first test

[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):
   Related: Fix: set project-wide integration hookTimeout/testTimeout in vitest.config.ts google/adk-js#548
   Related: Fix: give build_setup integration hooks an explicit timeout to stop CI flakiness google/adk-js#549
2. Or, if no issue exists, describe the change:

Problem: Two tests in tests/integration/app_loader/app_loader_test.ts intermittently fail on the macos-latest and windows-latest runners with Error: Test timed out in 40000ms. (ubuntu-latest green, and a re-run of the identical commit green on all three — a timing flake, not a correctness bug):

FAIL tests/integration/app_loader/app_loader_test.ts > AgentLoader discovery and loading integration >
  should discover apps vs agents across directories and standalone files
FAIL tests/integration/app_loader/app_loader_test.ts > AgentLoader discovery and loading integration >
  should load App from directory entrypoint and expose App and rootAgent

The cost is in the first test body, not in setup. AgentLoader loads lazily: the discovery beforeAll only ran npm install and new AgentLoader(projectPath), and that constructor (dev/src/utils/agent_loader.ts) just registers exit listeners. The first test's loader.listApps() then entered preloadAgents(), which for each of the four discovered entries in the discovery fixture (service_alpha/app.ts, service_beta/agent.ts, standalone_agent.ts, standalone_app.ts) runs a full esbuild.build with bundle: true, minify: true, packages: 'bundle' — bundling the whole @google/adk graph four times inside one 40s test body. The reported string was Test timed out, not Hook timed out, confirming the install hook completed and the body overran.

Why exactly two failures and not three. agentsAlreadyPreloaded is set only after the Promise.all in preloadAgents() resolves. When test 1 is aborted by the timeout the in-flight preload keeps running but the flag is still false, so test 2's getAppFile('service_alpha') re-enters preloadAgents() and starts a second full four-file bundle concurrently with the first, blowing the same budget. By the time test 3 runs one preload has settled, the flag is set, and it hits the AgentFile.load() cache and finishes in ~1ms. That cascade is why a plain timeout bump treats the symptom: the work would still be billed to a test body, and one slow bundle would still take a second test down with it.

Solution: three hunks, one test file, no production code.

1. Delete the per-file budget override. const TEST_EXECUTION_TIMEOUT = 40000 and all eight of its uses (three it()s, four beforeAll/afterAlls in the CLI suites, and the discovery hooks) are removed, so the file inherits testTimeout: 60000 / hookTimeout: 120000 from the integration project in vitest.config.ts. This is an alignment, not a bump: vitest.config.ts already documents those budgets and notes that per-file it()/hook timeouts still override them, and this file — added by Feat: Support apps  google/adk-js#489 — never picked up the defaults that Fix: set project-wide integration hookTimeout/testTimeout in vitest.config.ts google/adk-js#548 and Fix: give build_setup integration hooks an explicit timeout to stop CI flakiness google/adk-js#549 established. The override was also capping the four fixture npm install hooks at 40s, below the ~70s cold, network-bound install already measured and recorded in tests/integration/build_setup/build_setup_test.ts. Those install hooks are exactly what the project's 120s hookTimeout exists for.
2. Pay the compile cost in the hook. await loader.preloadAgents() is added to the discovery beforeAll, so the four esbuild passes are charged to the hook budget and the test bodies only assert on cached results. preloadAgents() is a public method already called directly by dev/test/utils/agent_loader_test.ts; no visibility was widened.
3. Make the loader non-nullable. let loader: AgentLoader assigned inside beforeAll meant a failed setup left afterAll dereferencing undefined and reporting a TypeError that hid the real error. The loader is now constructed at describe scope (const loader = new AgentLoader(projectPath)). That is safe because the constructor only registers process exit handlers and touches no filesystem (watchForChanges defaults to false), and disposeAll() is a no-op on a loader that never loaded anything (watcher?.close() with an undefined watcher, Promise.all over an empty map). afterAll keeps its original await loader.disposeAll() unchanged. No .catch(() => {}) was added — swallowing a real dispose error would hide temp-file leaks. An earlier revision guarded the teardown with loader?.disposeAll() instead; that was the wrong fix, because loader is typed AgentLoader and the optional chaining hedged against a state the type declares impossible. Removing the failure mode by construction needs no guard and keeps the type honest.

The diff is one file. No assertion is added, removed, weakened or reworded, and no describe/it name changes (CI history and flake tracking key off them). The apparent size of the diff is Prettier reflowing it(name, fn, TIMEOUT) into it(name, fn) once the trailing timeout argument is gone.

Measured before/after (--reporter=verbose, same machine, cold fixture in every run):

Phase	Before	After
discovery beforeAll	install only	install + 4 esbuild bundles
should discover apps vs agents across directories and standalone files	16710 ms	4 ms
should load App from directory entrypoint and expose App and rootAgent	1 ms	1 ms
should synthesize App when loadApp() is called on BaseAgent file	1 ms	1 ms

16710 ms is on an idle Linux workstation; the macOS/Windows runners execute this while npm run test:coverage drives four vitest projects with v8 coverage concurrently, which is what pushes the same work past 40s there.

Running the file unmodified on this machine also reproduced the latent install-hook half of the bug directly, which is the clearest evidence that the 40s cap — not the runner — was the problem: all four beforeAlls failed with Error: Hook timed out in 40000ms., and the discovery teardown then threw the masking TypeError: Cannot read properties of undefined (reading 'disposeAll') on top of it. After the change the same four hooks complete inside the 120s budget.

Honest trade-off: a genuinely hung hook now takes up to 120s to surface instead of 40s, mirroring the note already recorded in tests/integration/build_setup/build_setup_test.ts. That is the cost of not reporting a slow-but-working cold install as a failure.

Collision check (required before implementing; gh pr list --state open --limit 1000): this area is crowded. Several open PRs overlap — most notably one that removes the same per-file override and adds the same preloadAgents() warm-up across the integration suites plus vitest.config.ts, and three that add the warm-up alone. This PR is the self-contained, single-file version scoped strictly to app_loader_test.ts, and it is the only one that also fixes the afterAll TypeError with optional chaining. If a broader budget-alignment PR lands first, close this one as a duplicate — the two are not meant to be stacked.

Out of scope (deliberately not done here): de-duplicating concurrent preloadAgents() callers in dev/src/utils/agent_loader.ts (a real improvement, but a production change), and the identical 40s install-hook override in tests/integration/agent_loader/agent_dirname_test.ts and tests/integration/tools/run_skill_script_tool_test.ts.

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.

Coverage: this change adds no production lines, so there is no new production code to cover — the coverage bar is satisfied vacuously and does not regress. No new test was added either: the task is to stop five existing tests from flaking while preserving every existing assertion byte-for-byte.

Unit Tests:
[x] I have added or updated unit tests for my change. — N/A, see above; the existing AgentLoader unit suite is untouched and confirmed green: npx vitest run --project unit:dev dev/test/utils/agent_loader_test.ts → 30 passed.
[x] All unit tests pass locally.

Targeted commands run (no full-repo suite):

npx vitest run --project integration tests/integration/app_loader/app_loader_test.ts --reporter=verbose
  -> Test Files 1 passed (1) | Tests 6 passed (6)
npx vitest run --project unit:dev dev/test/utils/agent_loader_test.ts
  -> Test Files 1 passed (1) | Tests 30 passed (30)
npm run lint          -> clean
npm run format:check  -> All matched files use Prettier code style!

Falsification proof 1 — the work really left the test bodies (hunks 1-2). With the fix in place, a deliberately tiny 2000 ms per-test budget was passed to the two previously-failing tests: both passed (4 ms and 1 ms). The same 2000 ms budget with only await loader.preloadAgents() removed — mutating the exact line the fix adds — failed, and reproduced the CI signature precisely (2 failed / 1 passed, third test green off the cache):

FAIL ... > should discover apps vs agents across directories and standalone files
Error: Test timed out in 2000ms.
FAIL ... > should load App from directory entrypoint and expose App and rootAgent
Error: Test timed out in 2000ms.
Tests  2 failed | 1 passed | 3 skipped (6)

A timeout bump alone cannot pass this check. Both temporary edits were reverted; grep -n "40000\|2000" tests/integration/app_loader/app_loader_test.ts returns nothing.

Falsification proof 2 — the non-nullable loader is load-bearing (hunk 3). With throw new Error('boom') injected as the first statement of the discovery beforeAll:

• with the fix (loader constructed at describe scope) — the run reports only the real error, Error: boom.
• with the fix reverted (let loader: AgentLoader assigned inside beforeAll), same injected throw — the misleading second error comes back on top of it: TypeError: Cannot read properties of undefined (reading 'disposeAll').

Both temporary edits were reverted. This failure mode was also observed unprompted: running the file on unmodified main on this machine produced exactly that TypeError after the hook blew its 40s budget.

Repeat-run stability. The file was run three times consecutively and was green each time — Tests 6 passed (6) on every run, with the previously-failing discovery test reporting 3 ms, 3 ms and 4 ms. This matters because each afterAll deletes its fixture's node_modules and package-lock.json, so residue from a previous run would surface on the second.

Manual End-to-End (E2E) Tests:

npm install && npm run build      # required once: the fixtures resolve
                                  # @google/adk and @google/adk-devtools
                                  # from local file: paths
npx vitest run --project integration tests/integration/app_loader/app_loader_test.ts --reporter=verbose

Confirm from the verbose output that the discovery beforeAll carries the multi-second setup and that all three discovery tests report in the low milliseconds. To approximate the loaded macOS/Windows runners, re-run under CPU contention (the unit:dev suite above was run concurrently with the stability runs); the file stayed green.

CI result. run-tests is green on all three matrix legs — ubuntu-latest (3m55s), macos-latest (4m22s) and windows-latest (8m57s) — i.e. on both runners where this flake was reported. In the Windows leg the file under change reported ✓ integration tests/integration/app_loader/app_loader_test.ts (6 tests) 76518ms.

Reported honestly: the first Windows attempt was red, but not on anything in this diff. Its single failure was unit:core core/test/code_executors/unsafe_local_code_executor_test.ts:161 > UnsafeLocalCodeExecutor > should execute shell code and return stdout — Error: Test timed out in 5000ms. (1 failed | 2679 passed), a real-subprocess unit test running under vitest's default 5s budget, in a file this PR does not touch. app_loader_test.ts passed on that attempt too. Re-running the job passed, confirming it as an unrelated Windows flake. It is deliberately not fixed here — that is a separate file with its own in-flight fixes, and folding it in would widen this diff beyond the one test file.

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]

Close as superseded by #506. Both warm the loader in beforeAll; #506 is the smaller diff and also drops the unused discovery fixture install. Reopen this if #506 does not land, because it keeps that install and is the more conservative form.