Fix: resolve the exit_loop built-in tool in YAML agent configs

[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):

• N/A — no existing issue.

2. Or, if no issue exists, describe the change:

Problem:

ExitLoopTool and the EXIT_LOOP singleton already exist in core and are exported from @google/adk, but the conformance harness in the dev package still behaves as if they did not.

dev/src/integration/agent_registry.ts listed exit_loop in a BUILTIN_TOOLS array whose every entry was mapped to undefined and filtered out, so a YAML agent declaring:

tools:
  - name: exit_loop

was instantiated with zero tools. During a conformance replay the recorded exit_loop function call then fails with Function exit_loop is not found in the toolsDict., which is why workflow/loop_001 was permanently listed in SKIPPED_TESTS with the (now false) reason ExitLoopTool is not implemented yet.

Solution:

Two changes are required, and both are needed for the case to actually pass.

1. Registry resolution (dev/src/integration/agent_registry.ts). BUILTIN_TOOLS becomes a Record<string, BaseTool> of built-ins that resolve to a real tool object — currently just exit_loop -> EXIT_LOOP — mirroring adk-python's LlmAgent._resolve_tools, which resolves a bare, dot-less built-in name to the real tool object. The three purely server-side built-ins (google_search, url_context, google_maps_grounding) move to a separate SKIPPED_BUILTIN_TOOLS list and keep being dropped on purpose: they are executed by the Gemini backend and their processLlmRequest throws for a non-Gemini model name, and the replay harness injects DummyLlm (model name dummy-llm). The table is read through Object.hasOwn so that a YAML string naming an inherited member such as constructor falls through to findToolOrThrow instead of resolving to Object.prototype.constructor; Object.hasOwn is already the idiom used elsewhere in this package (dev/src/integration/test_runner.ts).

2. Replay side effects (dev/src/integration/replay_plugin.ts). This is the non-obvious half — reviewers will otherwise read it as unrelated. ReplayPlugin.beforeToolCallback returns the recorded tool response, and in core/src/agents/functions.ts a non-null plugin response short-circuits callToolAsync. The response recorded for a Python tool returning None is the dict {result: None} — a non-null object — so EXIT_LOOP.runAsync would never execute and neither escalate nor skipSummarization would be set. Without escalate the LoopAgent never stops; without skipSummarization the sub-agent asks the model for one more turn and the replay throws No LLM recording found .... Both flags are also deep-compared by validateSession. The plugin already replicated this class of side effect for transfer_to_agent; the if becomes a small switch with an exit_loop arm that sets both flags. (adk-python solves this differently — its replay plugin runs the real tool before returning the recording — but porting that would change execution for every replayed tool, so it is deliberately out of scope here.)

With both in place, workflow/loop_001 is removed from SKIPPED_TESTS; that list now holds exactly tool/example_tool_001, core/multi_005 and tool/long_running_tool_001.

No core/ change, no public API change, and no new dependency: AgentRegistry and ReplayPlugin are internal to the devtools conformance harness.

Testing Plan

Note on the conformance corpus: the replay corpus (spec.yaml / generated-recordings.yaml / generated-session.yaml triplets) is not vendored in this repo and is not exercised by .github/workflows/validation.yaml. The real adk integration conformance replay was NOT executed — no corpus was available in the environment. dev/test/integration/test_runner_test.ts was added as the in-repo, CI-runnable substitute: it drives the real TestRunner, AgentRegistry, ReplayPlugin, LoopAgent and LlmAgent with no mocks, over an in-memory equivalent of the loop_001 case, and asserts the produced session deep-equals the expected one — including escalate / skipSummarization on the final function-response event, which is only reachable when both halves of this fix are present.

Unit Tests:

• I have added or updated unit tests for my change.

• All unit tests pass locally.

• dev/test/integration/agent_registry_test.ts — exit_loop now resolves to the shared EXIT_LOOP instance; the three server-side built-ins are still dropped; an inherited member name such as constructor is not resolved as a built-in.

  • Rewritten test, called out explicitly: the old should skip built-in tools case asserted expect(retrieved.tools.length).toBe(0) for exit_loop — it pinned exactly the behaviour this PR fixes, so it became should resolve exit_loop to the EXIT_LOOP built-in tool. Its original assertion is not lost: it survives verbatim in the new should skip server-side built-in tools case, which applies it to google_search / url_context / google_maps_grounding. No other existing test was changed or removed.

• dev/test/integration/replay_plugin_test.ts (new) — both switch arms, the no-side-effect arm, the not-found rejection, and single-consumption of a recording.

• dev/test/integration/test_runner_test.ts (new) — the end-to-end loop replay described above.

Commands run locally on the pushed commit:

npx vitest run --project unit:dev dev/test/integration   # 4 files, 24 tests passed
npm run build                                            # OK
npm run lint                                             # OK
npm run format:check                                     # OK
npm run docs:check                                       # 0 errors
npx secretlint "**/*"                                    # clean

Each half of the fix was verified to be load-bearing by reverting it in isolation and re-running dev/test/integration/test_runner_test.ts:

• without the agent_registry.ts resolution, the run fails with Tool exit_loop not found in registry;
• without the replay_plugin.ts exit_loop arm, the loop never terminates and the run fails with No LLM recording found for agent refiner_agent at turn 0.

Pre-existing failures unrelated to this change: the tests/integration/build_setup, tests/integration/app_loader, tests/integration/agent_loader and e2e suites fail in this sandbox because they shell out to real builds and to the live Gemini API. They fail on the base commit too, and repeated runs on an unchanged tree produce a different subset each time, so they are environmental rather than caused by this diff.

Manual End-to-End (E2E) Tests:

1. Write a YAML agent config for a refiner agent that exits a loop:

   agent_class: LlmAgent
   name: refiner_agent
   model: gemini-2.5-flash
   description: Refines a poem.
   instruction: Refine the poem, then call exit_loop.
   tools:
     - name: exit_loop

2. Load it via AgentRegistry.registerAgentConfig + getAgent and confirm the resulting LlmAgent.tools is [EXIT_LOOP] (before this change it was []).

3. If you have a conformance corpus available:

   npm run build
   npx adk integration conformance --agents_dir <corpus>/agents --tests_dir <corpus>/tests
   

   workflow/loop_001 should now run and pass instead of being reported as skipped.

Checklist

• I have read the CONTRIBUTING.md document.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have added tests that prove my fix is effective or that my feature works.
• New and existing unit tests pass locally with my changes.
• I have manually tested my changes end-to-end.
• Any dependent changes have been merged and published in downstream modules.

Additional context

The two unchecked boxes are accurate rather than oversights: the full end-to-end path is the adk integration conformance replay, whose corpus is not vendored in this repo and was not available in this environment, and this change has no dependent downstream modules.

[kalenkevich]

can it be a plain Record instead of the map? (Record<string, BaseTool>)

[AmaadMartin]

Done, switched to a plain Record in ca75004 (dev/src/integration/agent_registry.ts:34):

const BUILTIN_TOOLS: Record<string, BaseTool> = {exit_loop: EXIT_LOOP};

One deviation worth flagging so you can object: the lookup key comes straight from user YAML, so I read it through Object.hasOwn (agent_registry.ts:153) rather than a bare if (BUILTIN_TOOLS[toolConfig.name]). With a bare index, tools: [{name: constructor}] resolves to Object.prototype.constructor and gets handed to LlmAgent as a "tool" instead of falling through to findToolOrThrow. The Map did not have that hazard, so the guard is just paying for the shape change. Object.hasOwn is already the idiom used elsewhere in this package (dev/src/integration/test_runner.ts:170).

Added a regression test for it, should not resolve an inherited Object member as a built-in tool (dev/test/integration/agent_registry_test.ts:297), and confirmed it fails without the guard (the config instantiates with Object as a tool and no error is thrown).

Happy to drop the guard and index directly if you would rather keep the lookup bare.