Fix: make the app_loader integration suite independent of the previous run

[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: tests/integration/app_loader/app_loader_test.ts fails on every
other consecutive run on an unmodified checkout, and flakes intermittently on
macos-latest. Because the validation OS matrix uses the default fail-fast,
that flake cancels the windows-latest job, so Windows coverage is regularly
lost.

Reproduced locally on main before touching anything — three consecutive runs,
nothing else changed between them:

run	result	duration
1	FAIL	168.43s
2	pass (6/6)	50.91s
3	FAIL	168.46s

Runs 1 and 3 both reported:

FAIL  tests/integration/app_loader/app_loader_test.ts > AgentLoader discovery and loading integration
Error: Hook timed out in 40000ms.
 ❯ tests/integration/app_loader/app_loader_test.ts:77:3

FAIL  tests/integration/app_loader/app_loader_test.ts > AgentLoader discovery and loading integration
TypeError: Cannot read properties of undefined (reading 'disposeAll')
 ❯ tests/integration/app_loader/app_loader_test.ts:131:18

and left partial fixture trees behind:

tests/integration/app_loader/app_default/node_modules
tests/integration/app_loader/app_js/node_modules
tests/integration/app_loader/app_js/package-lock.json
tests/integration/app_loader/app_ts/node_modules
tests/integration/app_loader/app_ts/package-lock.json

Two coupled defects produce that alternation:

Defect 1 — fixture state leaks between runs. Both beforeAll hooks run
npm install in-tree; the matching afterAll hooks delete node_modules and
package-lock.json and swallow every error with .catch(() => {}). A run
killed mid-install leaves a partial node_modules behind, which changes the
next run's install cost — that is the mechanism behind the alternation (run 2
above was fast because run 1 had leaked a warm partial tree; run 3 started
cold again and blew the budget). In the discovery block, afterAll calls
loader.disposeAll() before the filesystem cleanup, so when that throws the
cleanup below never runs — the leak is guaranteed on exactly the runs that
already went wrong. .catch(() => {}) also makes a genuine EACCES/EPERM/
EBUSY indistinguishable from the benign "already gone" case.

Defect 2 — one 40s constant doing two jobs, whose failure mode masks the real
error. TEST_EXECUTION_TIMEOUT = 40000 was passed as both the per-test budget
and the hook budget. 40s is too tight for a cold npm install, and a per-file
beforeAll(fn, 40000) / it(name, fn, 40000) argument overrides the
project-level budgets vitest.config.ts already defines for exactly this reason
(INTEGRATION_HOOK_TIMEOUT_MS = 120000, INTEGRATION_TEST_TIMEOUT_MS = 60000),
so the file opted itself back down to 40s. When the hook budget fired, loader
— declared let loader: AgentLoader; and assigned after the install — was
still undefined, so afterAll added a TypeError on top of the real failure.

Solution: test-infrastructure only; no production source file is touched and
no assertion is weakened, skipped, retried, or removed. Two files:
app_loader_test.ts and the shared tests/integration/test_case_utils.ts the
helpers live in.

1. Pre-clean before installing. Both beforeAll hooks now call
   removeInstallArtifacts(projectPath) before npm install, so every run
   starts from an identical fixture state regardless of how the previous run
   ended. Determinism is now owned by setup, not by teardown having succeeded.
2. Two cleanup helpers with deliberately different error policies, in
   tests/integration/test_case_utils.ts (the shared module this suite already
   imports for sendInput). They are fixture-install helpers, not app_loader
   helpers — build_setup_test.ts open-codes the same node_modules +
   package-lock.json teardown today — so they are shared rather than
   co-located. removeInstallArtifacts is strict: force: true makes a missing
   path a no-op, so anything that still rejects is a real failure and
   propagates. cleanUpFixture is the teardown variant: it catches,
   console.warns with the fixture path, and does not rethrow. This asymmetry
   is intentional: a pre-clean that cannot remove the tree is exactly the
   non-deterministic state this change exists to eliminate and must fail loudly,
   whereas failing an otherwise-green suite on a teardown EBUSY would trade
   one flake for another. .catch(() => {}) no longer appears anywhere in the
   test file. fs.unlink is replaced by fs.rm(..., {force: true}), which is
   why the blanket catch was needed in the first place (unlink throws
   ENOENT).
3. Both budgets are now inherited from the integration project. The
   per-file TEST_EXECUTION_TIMEOUT = 40000 is deleted and no replacement
   constant is introduced: vitest.config.ts already applies
   hookTimeout: INTEGRATION_HOOK_TIMEOUT_MS = 120000 and
   testTimeout: INTEGRATION_TEST_TIMEOUT_MS = 60000 to
   tests/integration/**/*_test.ts, which matches this path. Declaring a
   per-file HOOK_TIMEOUT = 120000 would have been a third copy of the same
   number and would re-create the shadowing structure this change exists to
   remove. Both budgets are verified empirically below rather than assumed.
4. loader is a const initialised at declaration. The AgentLoader
   constructor only records the directory and registers exit handlers
   (dev/src/utils/agent_loader.ts) — it performs no filesystem access, so it
   is safe to construct before the install. disposeAll() on a loader that
   never loaded anything is Promise.all([]). afterAll therefore always has a
   callable loader, and disposeAll() runs inside a try/finally so its
   failure can no longer skip the fixture cleanup.

Collision check (required before implementing; recorded here per process).
Scanned all 419 open PRs on the fork:

gh pr list --repo AmaadMartin/adk-js --state open --limit 1000 --json number,title,headRefName
gh pr diff <n> --repo AmaadMartin/adk-js --name-only

Eight open PRs also edit this file or its budgets — #506, #499, #478, #407,
#405, #260, #256, #235 (plus #218, which extracts the fixture install/teardown
into a shared tests/integration/fixture_project.ts). All of them overlap
only on the timeout dimension (item 3 above), and they conflict with each
other: #405/#478 drop the per-file argument entirely, #260 raises it to 180s,
#235 to 120s/180s, #256 to 60s, #506 splits it per-suite. None of them
addresses the state leak: no open PR pre-cleans before installing, #218 still
swallows every teardown error with .catch(() => {}), and no PR makes loader
safe to dereference in afterAll. Items 1, 2 and 4 above are unique to this PR.

This PR branches from main rather than stacking, because there is no single
"the" overlapping branch to stack on — the eight candidates are mutually
exclusive, so picking one would make this change un-mergeable if a different one
lands. If any of them merges first, this PR should be rebased onto it and item 3
dropped; items 1, 2 and 4 apply unchanged on top of every one of them.

Deliberately out of scope (the same defect exists there, tracked separately;
this change cannot conflict with either):
tests/integration/agent_loader/agent_dirname_test.ts and
tests/integration/skills/script_js/agent_test.ts carry the same hook-budget
and best-effort-cleanup defect, and adding fail-fast: false to the
validation.yaml OS matrix.

Known remaining duplication, called out rather than left silent.
build_setup_test.ts still open-codes the same node_modules +
package-lock.json teardown with its own .catch(() => {}), and still declares
its own HOOK_TIMEOUT = 120000 duplicating INTEGRATION_HOOK_TIMEOUT_MS. Both
are now one-line changes on top of this PR (the helpers it needs are exported
from test_case_utils.ts), but retiring them touches a second flaky-teardown
suite, so they are left to the separately-queued task for that file rather than
widened into this diff.

No unit tests are added, and that is deliberate. The entire diff lives in a
test file. The coverage report's include globs are core/src/**, dev/src/**
and integrations/src/**, so nothing here is measured, and the thresholds in
vitest.config.ts (statements: 86, branches: 87, functions: 88,
lines: 86) are unaffected. A unit test for a test-file helper would be
inverted; both helpers are exercised on every run of this suite —
removeInstallArtifacts twice per fixture (four beforeAll and four afterAll
paths), and cleanUpFixture's catch branch by the fault injection below.

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. — N/A, see above: the
change is confined to a test file that is itself the integration test. No
production code is added, so there is no new line or branch to cover.
[x] All unit tests pass locally.

1. Three consecutive runs, started from the leaked fixture state that baseline
run 3 left behind:

for i in 1 2 3; do
  npx vitest run --project integration tests/integration/app_loader/app_loader_test.ts
done

run	result	duration
1	pass (6/6)	348.60s
2	pass (6/6)	331.90s
3	pass (6/6)	337.80s

Re-run after the review revision (helpers relocated, budget constants deleted):
pass (6/6) 338.57s, pass (6/6) 336.76s.

Afterwards, no fixture artifacts remain and git status --porcelain reports
only the edited files. Note the flat durations: previously the runtime swung
50s ↔ 168s because each run inherited the previous run's tree. (The absolute
number is this machine's cold, proxied npm install — ~84s per fixture,
comfortably inside the 120s hook budget. Steady-state cost is unchanged versus
main: the old afterAll already removed node_modules after every green run,
so the pre-clean is a no-op except on runs that previously leaked.)

2. Mutation test for Defect 1 — proving the pre-clean is load-bearing.

First, the corruption suggested during design (node_modules/@google/adk/ package.json = {}) turned out not to be a valid mutation: npm install
repairs it, and the suite passed with and without the pre-clean. A corruption
that npm genuinely does not repair was needed, so I measured what survives:

mkdir -p discovery/node_modules/.aborted-reify-staging && npm install
# => DEBRIS SURVIVES npm install

npm install does not sweep leftover content under node_modules. The faithful
version of that — a half-extracted package, exactly what an aborted reify leaves
— is deterministic:

cd tests/integration/app_loader/discovery
rm -rf node_modules package-lock.json && npm install   # clean tree
rm -rf node_modules/@google/adk/dist                   # simulate aborted reify
npm install                                            # => STILL GUTTED

Run against that identical corrupted fixture, with only the pre-clean line
mutated away:

Mutation — the single await removeInstallArtifacts(projectPath); line deleted
from the discovery beforeAll: FAILS

× AgentLoader discovery and loading integration > should discover apps vs agents across directories and standalone files 57ms
  → Build failed with 1 error:
tests/integration/app_loader/discovery/service_alpha/app.ts:6:30: ERROR: Could not resolve "@google/adk"
 Test Files  1 failed (1)

Line restored, same corruption re-applied: PASSES (1 passed | 5 skipped).

Both directions were re-run after the review revision moved the helper into
test_case_utils.ts, and both still hold.

3. Mutation test for Defect 2 — proving the loader guard removes the
masking, and simultaneously proving the inherited hook budget. The discovery
install was forced to fail (npm install --registry http://127.0.0.1:1/):

With const loader = new AgentLoader(projectPath) (this PR) — exactly one
error, the real one; grep -c TypeError returns 0:

⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯
 FAIL  tests/integration/app_loader/app_loader_test.ts > AgentLoader discovery and loading integration
Error: Hook timed out in 120000ms.
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯

Reverted to let loader: AgentLoader; assigned inside the hook (main's
shape), same forced failure — the masking reappears:

⎯⎯⎯⎯⎯⎯ Failed Suites 1 ⎯⎯⎯⎯⎯⎯⎯
Error: Hook timed out in 120000ms.
TypeError: Cannot read properties of undefined (reading 'disposeAll')

Both experiments were reverted; the committed file is byte-identical to the one
that produced the passing runs in step 1.

Note the reported budget: Hook timed out in 120000ms with no per-hook
timeout argument anywhere in the file. That is the proof that the hooks
inherit INTEGRATION_HOOK_TIMEOUT_MS from the integration project rather than
falling back to vitest's 10s default — which is what makes the deleted
HOOK_TIMEOUT constant redundant rather than load-bearing. This probe was
re-run against the final revision specifically to establish it.

4. Confirming the inherited per-test budget. After removing the per-test
third arguments, a temporary
await new Promise((r) => setTimeout(r, 45000)); at the top of the discovery
test made the runner name the inherited budget explicitly:

→ Test timed out in 60000ms.

60000ms is INTEGRATION_TEST_TIMEOUT_MS — not vitest's 5s default and not the
old 40s. The probe was removed.

5. Neighbouring integration suites.
tests/integration/build_setup/build_setup_test.ts — passes
(20 passed | 4 skipped (24), 529.62s), re-run after the helpers moved into
the shared module it also imports.| 4 skipped (24), 532.91s). tests/integration/agent_loader/agent_dirname_test.ts— fails on this machine withError: Hook timed out in 40000ms`, its own pre-existing budget. That
file is not in this diff and is the out-of-scope sibling noted above; the
failure is identical with and without this change.

6. Repository gates, on the exact commit pushed:

npm run lint          # clean
npm run format:check  # "All matched files use Prettier code style!"
npm run build         # clean
npm run ts:check      # 41 pre-existing failing files on main, neither of them
                      # ours (`grep -cE 'app_loader_test|test_case_utils'` => 0).
                      # ts:check is not a CI step in
                      # .github/workflows/validation.yaml, which runs
                      # install → secretlint → build → test:coverage → lint →
                      # format:check → docs:check.

No suppressions were added: git diff fork/main -U0 | grep -E '@ts-expect-error|@ts-ignore|eslint-disable|as any|as never|: any' returns
nothing. The one console.warn is the mechanism that makes a swallowed teardown
failure visible, matching existing practice in this directory
(build_setup_test.ts uses console.error in the same position); there is no
console.log in the diff and no no-console rule in eslint.config.js.
git diff fork/main -w -- tests/integration/app_loader/app_loader_test.ts | grep 'expect(' returns nothing — no assertion line was added or removed, only
re-indented by Prettier when the third it() argument went away.

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

npm install && npm run build   # fixtures depend on file:../../../../core and .../dev

# 1. Baseline the flake on an unmodified checkout: expect fail / pass / fail.
for i in 1 2 3; do
  npx vitest run --project integration tests/integration/app_loader/app_loader_test.ts
done

# 2. Reproduce the user-visible symptom: kill a run mid-install (Ctrl-C while a
#    fixture `npm install` is running), confirm a partial node_modules is left
#    behind, then run the suite again. On main the next run is affected by it;
#    with this change it is not.
ls tests/integration/app_loader/discovery/node_modules

# 3. With this change, three consecutive runs pass and leave nothing behind.
for i in 1 2 3; do
  npx vitest run --project integration tests/integration/app_loader/app_loader_test.ts || echo "RUN $i FAILED"
done
git status --porcelain                       # only the edited test file
ls tests/integration/app_loader/*/node_modules 2>/dev/null   # nothing

Windows is the leg that exercises the maxRetries/retryDelay path in
removeInstallArtifacts; it is covered by the validation matrix rather than
locally.

CI on this branch — all three legs reached a terminal state and passed:

job	result	duration
run-tests (ubuntu-latest)	pass	5m49s
run-tests (macos-latest)	pass	6m55s
run-tests (windows-latest)	pass	8m46s
run-tests	pass	1m54s

macos-latest is the leg the flake was reported on, and windows-latest — the
leg that fail-fast kept cancelling, and the only one that exercises the
maxRetries/retryDelay retry path — ran to completion and passed.

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.