Fix: align integration install hooks on the project-wide hook timeout

[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: google#548 set a project-wide hookTimeout: 120000 on the integration vitest project. That number was chosen for a specific measured reason, recorded in tests/integration/build_setup/build_setup_test.ts: a cold, network-bound fixture npm install has been measured at ~70s, against ~16s warm.

Three sibling suites still passed an explicit hook-timeout argument, and Vitest resolves an explicit argument above the project setting. So those hooks shadowed the repo-wide floor downward, to below the very worst case the 120s figure exists to cover:

File	Hook	Work	Budget
agent_loader/agent_dirname_test.ts	beforeAll	npm install	40 000
agent_loader/agent_dirname_test.ts	afterAll	recursive node_modules teardown	40 000
app_loader/app_loader_test.ts	beforeAll ×2	npm install	40 000
app_loader/app_loader_test.ts	afterAll ×2	teardown (+ disposeAll)	40 000
skills/script_js/agent_test.ts	beforeAll	npm install	60 000

40 000 < ~70 000, so these are latent flakes today on exactly the failure mode #549 fixed, just with a longer fuse. This is not theoretical: see the measurement in the Testing Plan below, where these hooks actually blew the 40 000 ms ceiling on a loaded machine during verification.

Two secondary defects of the same root cause:

1. Budget conflation. All three files passed TEST_EXECUTION_TIMEOUT — the per-it() budget — as the hook budget. A test asserting on agent output and a cold registry install are unrelated quantities. build_setup_test.ts warned against exactly this in prose, but the warning was never applied to its siblings.
2. Redundant local constant. With hookTimeout: 120000 set project-wide by Chore: enforce the node: protocol for Node built-in imports in src (no new deps) #548, build_setup_test.ts's local HOOK_TIMEOUT = 120000 restated the project default. Two co-equal definitions of one policy is how the budget drifts apart again.

Solution: drop every explicit timeout argument from the install and teardown hooks under tests/integration/, so all of them inherit the single budget in vitest.config.ts. Because the integration project sets hookTimeout, omitting the argument is not "untimed" — it is "inherit 120s". Omission is therefore the correct expression of "use the repo-standard budget", and it makes the invariant structural rather than a comment somebody has to remember.

The rationale text and both measurements move from build_setup_test.ts into the doc comment on INTEGRATION_HOOK_TIMEOUT_MS, so no information is lost and the policy lives with the value.

A second commit narrows the neighbouring INTEGRATION_TEST_TIMEOUT_MS comment, which said "Per-file it()/hook timeouts still override both". Left as-is it would sit two lines below the new invariant and read as an endorsement of exactly the override that caused this bug. It now speaks only to the it() case, which does still override and is meant to.

A third commit removes an overclaim caught in review. The doc comment originally said "This is the only definition of that budget", which is false: adk_web/webui_test.ts:45 and the three a2a suites each pass their own beforeAll budget (20s / 60s), all below this floor. The comment now states only the rule this change actually enforces — that an install or teardown hook must not pass its own timeout argument — and both comments came out shorter than before.

Those remaining hooks were not swept, for three reasons. They start a server rather than install a fixture, so the ~70s cold-install measurement that justifies 120s does not apply to them. The a2a ones pass the same constant to both the hook budget and the server's own startFailureTimeout, so the number is deliberately coupled there rather than accidentally shadowing policy. And webui_test.ts is the subject of open PR #359, which is specifically about naming its hook timeouts — sweeping it here would collide with a live review.

One correction worth recording, since webui_test.ts:85 was also raised as a shadowing site: its trailing 20000 is not a hook budget. It is the third argument to describe(), which Vitest 3.2.6 types as TestOptions.timeout — a per-test timeout. Verified empirically rather than from the type docs: a probe suite shaped describe('...', () => { beforeAll(slow); it(...) }, 20000), run with the project hookTimeout temporarily set to 50, fails with Hook timed out in 50ms — not 20000ms. A describe-level number cannot shadow the hook floor, so there is nothing to fix at that line.

Why build_setup_test.ts is touched again, having only just been fixed by #549 — this is a consolidation, not a revert. #548 made its local HOOK_TIMEOUT constant redundant by setting the same value project-wide; this merges the two co-equal definitions into one so they cannot drift. That file's effective hook budget is unchanged at 120s. The behavioural fix is to the other three suites.

Effect: the hook budget rises 40 000 → 120 000 ms for six hooks and 60 000 → 120 000 ms for one; it is unchanged for the two build_setup hooks. No it() budget changes anywhere. There is no CI wall-clock cost on a healthy run — a timeout ceiling is only consumed on failure. The cost is strictly that a genuinely hung hook takes up to 120s to surface instead of 40s, which is the trade-off #548 already accepted repo-wide and this applies consistently.

Deliberately out of scope (called out so the omissions are not read as oversights):

• Hooks under tests/integration/ that start a server rather than install a fixture (a2a/*, adk_web/webui_test.ts) keep their explicit arguments. The a2a ones pair the hook budget with the server's own startFailureTimeout, so being explicit there is meaningful rather than accidental shadowing.
• The afterAll teardowns swallow errors via .catch(() => {}), so a cleanup failure cannot fail the suite. That is pre-existing; changing failure semantics under the banner of a timing fix would be the wrong scope.
• Per-it() budgets are untouched. One test body is still flaky under parallel load (see Testing Plan) — that is a separate, per-test concern.

Collision check (required before implementation). gh pr list --repo AmaadMartin/adk-js --state open --limit 300, then gh pr diff --name-only on every plausibly adjacent PR. Six open PRs touch these same hooks: #218, #235, #256, #257, #260, #276. None of them lands this change, and I did not stack on any of them, because:

• All six predate Chore: enforce the node: protocol for Node built-in imports in src (no new deps) #548 and are written against a main that had no project-wide hookTimeout — Fix: stabilise install-bound and server-spawning integration suites #218 and Fix: stop the agent_loader and build_setup integration suites from timing out on their fixture npm install #257 still show the pre-Feat: declare the Node.js floor on the workspace root and derive the checked manifest list (stacked on #544) #549 build_setup_test.ts as their base.
• Each raises the budget by introducing another per-file or per-module constant (INSTALL_TIMEOUT = 180_000, FIXTURE_HOOK_TIMEOUT_MS = 180_000, FIXTURE_SETUP_TIMEOUT = 180000). That keeps the shadowing and adds a second source of truth, which is the defect this PR removes; stacking on one would mean immediately deleting its constants.
• They conflict with each other, so there is no single branch to stack on.

This PR is the minimal consolidation on current main and supersedes the timeout portion of those PRs. #276 and #299 take a different and complementary direction (removing the fixture npm install altogether); this change does not conflict with that — a hook that no longer installs simply never approaches the budget.

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.

No unit tests are added: this change alters no product code, adds no executable product lines, and the deliverable is test-infrastructure hardening. Fabricating a unit test for a vitest config constant would assert nothing. Coverage is computed over core/src, dev/src, integrations/src only (vitest.config.ts), none of which is touched, so coverage does not move. The existing integration suites are the test, and they are verified below.

Inheritance proof (the mutation test). The mutation is on the config value the change makes authoritative: set INTEGRATION_HOOK_TIMEOUT_MS = 1 and check which hooks notice.

Against the unfixed files (git stash the four test files, probe value in place) — this is the run that proves the tests can fail and that the bug is real:

npx vitest run --project integration \
  tests/integration/agent_loader/agent_dirname_test.ts \
  tests/integration/app_loader/app_loader_test.ts \
  tests/integration/skills/script_js/agent_test.ts

Error: Hook timed out in 40000ms.   ❯ agent_dirname_test.ts:27  (beforeAll)
Error: Hook timed out in 40000ms.   ❯ app_loader_test.ts:30     (beforeAll)
Error: Hook timed out in 60000ms.   ❯ agent_test.ts:32          (beforeAll)
Error: Hook timed out in 1ms.       ❯ agent_test.ts:95          (afterAll — no explicit arg)

Two things this shows. First, the bug: with the project floor set to 1ms, the three hooks carrying an explicit argument ignored it entirely and reported 40 000 ms / 60 000 ms, while the one hook already written without an argument correctly inherited 1ms. Lowering the repo-wide floor has no effect on the opted-out files. Second, and unplanned: those hooks genuinely hit 40 000 ms and 60 000 ms in this run, with three suites installing in parallel. The 40s ceiling is not merely below the measured cold-install figure — it is reachable in practice.

Against the fixed files, same probe value — every install and teardown hook in all four suites now reports Hook timed out in 1ms, at agent_dirname_test.ts:27 and :49, app_loader_test.ts:30, :55 and :77, and agent_test.ts:32 and :95. One caveat reported honestly: the fourth app_loader hook (afterAll at :130) fails with TypeError: Cannot read properties of undefined (reading 'disposeAll') instead of a timeout, because its beforeAll was killed by the probe and never assigned loader, so the hook throws synchronously before any budget can elapse. That is an artifact of the probe cascade, not a missed argument — the hook carries no third argument, and its three siblings in the same file demonstrably inherit.

The probe value was then reverted; git diff main -- vitest.config.ts contains only the doc-comment change and no changed number.

Suite runs at the restored 120 000 budget. All four suites, run together:

npx vitest run --project integration \
  tests/integration/build_setup/build_setup_test.ts \
  tests/integration/agent_loader/agent_dirname_test.ts \
  tests/integration/app_loader/app_loader_test.ts \
  tests/integration/skills/script_js/agent_test.ts
# Test Files  1 failed | 3 passed (4)
# Tests  1 failed | 29 passed | 4 skipped (34)

The single failure is agent_dirname_test.ts:41, an assertion inside an it() body (expected '\n> dirname-test@1.0.0 start…' to contain "I'm stubby model response!") — the spawned agent had not answered within the per-test 40s budget while four suites competed for the machine. It is not a hook failure and this change does not touch it() budgets. Run on its own the suite is green:

npx vitest run --project integration tests/integration/agent_loader/agent_dirname_test.ts
# Test Files  1 passed (1)   Tests  3 passed (3)

That run is itself corroborating evidence for the fix: 228s wall clock of which only ~15s was test bodies (5.3s, 5.1s, 5.0s), leaving ~213s in six install/teardown hooks — ~35s per hook, against the 40 000 ms ceiling this PR removes.

Other gates on the pushed commit:

npm run build          # exit 0
npx eslint <5 changed files>                       # exit 0, no findings
npx prettier --check vitest.config.ts "tests/integration/**/*_test.ts"   # all match
npx tsc --noEmit       # 280 errors, all pre-existing

tsc --noEmit is not green on main (see #370, which exists to fix that). The count is 280 both with and without this change, verified by re-running against the stashed tree, and zero of them are in the five files touched here.

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

To reproduce the inheritance proof yourself, from a clean tree:

1. npm install && npm run build.
2. Temporarily set INTEGRATION_HOOK_TIMEOUT_MS = 1 in vitest.config.ts.
3. npx vitest run --project integration tests/integration/app_loader/app_loader_test.ts — every install/teardown hook must fail with Hook timed out in 1ms. On main (before this change) the same hooks instead report Hook timed out in 40000ms, proving the explicit argument shadows the project budget.
4. Restore INTEGRATION_HOOK_TIMEOUT_MS = 120000 and confirm the suite passes again. Do not ship the probe value — it breaks the integration matrix on all three operating systems.

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.

CI on this branch (commit bfd20d50, after the review revision) — real test jobs, all green:

Job	Result
run-tests (ubuntu-latest)	pass, 5m26s
run-tests (macos-latest)	pass, 5m51s
run-tests (windows-latest)	pass, 9m28s

The pre-revision commit 41ae0c12 was also green on all three (4m39s / 6m1s / 9m0s).