From 9f4734e84d07fc6229a8e6dbf31c597efebc2918 Mon Sep 17 00:00:00 2001 From: jinye Date: Sat, 18 Apr 2026 10:31:50 +0800 Subject: [PATCH] fix(tool-registry): add lazy factory registration with inflight concurrency dedup (#3297) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #3221. Introduces a lazy factory API on ToolRegistry (registerFactory, ensureTool, warmAll, getAllToolNames) as infrastructure for future esbuild code-splitting (#3226). With the current single-bundle build, the lazy API does not change startup time on its own — the primary immediate value is fixing three pre-existing bugs uncovered while designing it. Bug fixes: - Concurrent instantiation (P0): the original ensureTool had no concurrency protection around `await factory()` — two concurrent calls for the same tool both passed the cache check and each ran the factory, producing two instances. AgentTool and SkillTool register SubagentManager listeners in their constructors, so the extra instance leaked listeners. Fix: a per-name `inflight: Map>` so concurrent ensureTool() calls share a single promise. On factory rejection the inflight entry is cleared so a subsequent call can retry. - stop() resource leak: stop() only disposed tools already in `this.tools`; tools still loading in `inflight` when stop() ran finished afterward and were never disposed. Fix: await Promise.allSettled(inflight.values()) before the dispose loop. - Cache hit left stale factory: ensureTool's cache-hit branch did not delete the factory entry, so warmAll() would re-invoke the factory for an already-loaded tool. Fix: delete the factory on cache hit. Additional hardening in response to review feedback: - warmAll({ strict?: boolean }): strict mode re-throws the first factory failure rather than swallowing it. Config.initialize() uses strict: true so a broken built-in tool fails startup fast instead of silently leaving a partially initialized registry; runtime-path callers (GeminiChat, agent runtime, etc.) continue to use the non-strict default and log failures via debugLogger. - getAllTools() and getFunctionDeclarationsFiltered() emit a debug warning when called while unloaded factories remain, nudging callers toward warmAll() without hard-breaking existing code paths. - copyDiscoveredToolsFrom() now iterates source.tools.values() directly instead of source.getAllTools() — the copy path deals only with already-discovered MCP/command tools and should not trigger the unloaded-factory warning. - MemoryTool and SkillTool config parsing was extracted into memory-config.ts and skill-utils.ts so a factory can resolve tool metadata without importing the tool module. Tests: - tool-registry.test.ts adds 128 lines covering: concurrent ensureTool runs the factory exactly once, warmAll and ensureTool overlap, retries succeed after a prior factory failure, stop() disposes tools that finish loading after stop was called, and warmAll strict vs default behavior. - 33 existing call sites across cli, core, agents, and subagents were updated to await warmAll() before bulk tool access. --- package-lock.json | 61 ++++-- .../src/acp-integration/session/Session.ts | 11 +- .../session/SubAgentTracker.test.ts | 8 +- .../session/emitters/ToolCallEmitter.test.ts | 20 +- .../session/emitters/ToolCallEmitter.ts | 10 +- packages/cli/src/config/config.test.ts | 74 ++++---- packages/cli/src/config/config.ts | 12 +- packages/cli/src/ui/commands/clearCommand.ts | 5 +- .../cli/src/ui/commands/contextCommand.ts | 7 +- .../cli/src/ui/hooks/useToolScheduler.test.ts | 3 + packages/cli/src/ui/utils/export/normalize.ts | 4 +- .../core/src/agents/runtime/agent-core.ts | 4 +- .../src/agents/runtime/agent-headless.test.ts | 7 +- .../core/src/agents/runtime/agent-headless.ts | 2 +- .../src/agents/runtime/agent-interactive.ts | 2 +- packages/core/src/config/config.test.ts | 161 ++++++++-------- packages/core/src/config/config.ts | 178 +++++++++++------- packages/core/src/core/client.test.ts | 2 + packages/core/src/core/client.ts | 11 +- .../core/src/core/coreToolScheduler.test.ts | 35 +++- packages/core/src/core/coreToolScheduler.ts | 18 +- packages/core/src/core/geminiChat.ts | 4 +- .../core/nonInteractiveToolExecutor.test.ts | 1 + packages/core/src/followup/speculation.ts | 2 +- packages/core/src/index.ts | 64 +++++-- .../src/subagents/subagent-manager.test.ts | 23 ++- .../core/src/subagents/subagent-manager.ts | 13 +- packages/core/src/telemetry/loggers.test.ts | 2 +- packages/core/src/tools/mcp-tool.test.ts | 12 +- packages/core/src/tools/mcp-tool.ts | 2 +- packages/core/src/tools/memory-config.ts | 48 +++++ packages/core/src/tools/skill-utils.ts | 14 ++ packages/core/src/tools/skill.ts | 11 +- packages/core/src/tools/tool-registry.test.ts | 128 +++++++++++++ packages/core/src/tools/tool-registry.ts | 110 ++++++++++- 35 files changed, 739 insertions(+), 330 deletions(-) create mode 100644 packages/core/src/tools/memory-config.ts create mode 100644 packages/core/src/tools/skill-utils.ts diff --git a/package-lock.json b/package-lock.json index bd2d4520c..9c18517c6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -236,6 +236,7 @@ "integrity": "sha512-H3mcG6ZDLTlYfaSNi0iOKkigqMFvkTKlGUYlD8GW7nNOYRrevuA46iTypPyv+06V3fEmvvazfntkBU34L0azAw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@babel/code-frame": "^7.28.6", "@babel/generator": "^7.28.6", @@ -711,6 +712,7 @@ } ], "license": "MIT", + "peer": true, "engines": { "node": ">=18" }, @@ -734,6 +736,7 @@ } ], "license": "MIT", + "peer": true, "engines": { "node": ">=18" } @@ -2175,6 +2178,7 @@ "resolved": "https://registry.npmjs.org/@opentelemetry/api/-/api-1.9.0.tgz", "integrity": "sha512-3giAOQvZiH5F9bMlMiv8+GSPMeqg0dbaeo58/0SlA9sxSqZhnUtxzX9/2FzyhS9sWQf5S0GJE0AKBrFqjpeYcg==", "license": "Apache-2.0", + "peer": true, "engines": { "node": ">=8.0.0" } @@ -3597,6 +3601,7 @@ "resolved": "https://registry.npmjs.org/@testing-library/dom/-/dom-10.4.1.tgz", "integrity": "sha512-o4PXJQidqJl82ckFaXUeoAW+XysPLauYI43Abki5hABd853iMhitooc6znOnczgbTYmEP6U6/y1ZyKAIsvMKGg==", "license": "MIT", + "peer": true, "dependencies": { "@babel/code-frame": "^7.10.4", "@babel/runtime": "^7.12.5", @@ -4068,6 +4073,7 @@ "integrity": "sha512-WPigyYuGhgZ/cTPRXB2EwUw+XvsRA3GqHlsP4qteqrnnjDrApbS7MxcGr/hke5iUoeB7E/gQtrs9I37zAJ0Vjw==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "csstype": "^3.2.2" } @@ -4078,6 +4084,7 @@ "integrity": "sha512-jp2L/eY6fn+KgVVQAOqYItbF0VY/YApe5Mz2F0aykSO8gx31bYCZyvSeYxCHKvzHG5eZjc+zyaS5BrBWya2+kQ==", "dev": true, "license": "MIT", + "peer": true, "peerDependencies": { "@types/react": "^19.2.0" } @@ -4283,6 +4290,7 @@ "integrity": "sha512-6sMvZePQrnZH2/cJkwRpkT7DxoAWh+g6+GFRK6bV3YQo7ogi3SX5rgF6099r5Q53Ma5qeT7LGmOmuIutF4t3lA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.35.0", "@typescript-eslint/types": "8.35.0", @@ -4528,6 +4536,7 @@ "integrity": "sha512-tJxiPrWmzH8a+w9nLKlQMzAKX/7VjFs50MWgcAj7p9XQ7AQ9/35fByFYptgPELyLw+0aixTnC4pUWV+APcZ/kw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@testing-library/dom": "^10.4.0", "@testing-library/user-event": "^14.6.1", @@ -4678,6 +4687,7 @@ "integrity": "sha512-oukfKT9Mk41LreEW09vt45f8wx7DordoWUZMYdY/cyAk7w5TWkTRCNZYF7sX7n2wB7jyGAl74OxgwhPgKaqDMQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vitest/utils": "3.2.4", "pathe": "^2.0.3", @@ -4851,6 +4861,7 @@ "resolved": "https://registry.npmjs.org/acorn/-/acorn-8.15.0.tgz", "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -5265,8 +5276,7 @@ "version": "1.1.1", "resolved": "https://registry.npmjs.org/array-flatten/-/array-flatten-1.1.1.tgz", "integrity": "sha512-PCVAQswWemu6UdxsDFFX/+gVeYqKAod3D3UVm91jHwynguOwAvYPhx8nNlM++NqRcK6CxxpUafjmhIdKiHibqg==", - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/array-includes": { "version": "3.1.9", @@ -5814,6 +5824,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "baseline-browser-mapping": "^2.8.25", "caniuse-lite": "^1.0.30001754", @@ -6474,7 +6485,6 @@ "resolved": "https://registry.npmjs.org/content-disposition/-/content-disposition-0.5.4.tgz", "integrity": "sha512-FveZTNuGw04cxlAiWbzi6zTAL/lhehaWbTtgluJh4/E95DqMwTmha3KZN1aAWA8cFIhHzMZUvLevkw5Rqk+tSQ==", "license": "MIT", - "peer": true, "dependencies": { "safe-buffer": "5.2.1" }, @@ -7552,6 +7562,7 @@ "integrity": "sha512-GsGizj2Y1rCWDu6XoEekL3RLilp0voSePurjZIkxL3wlm5o5EC9VpgaP7lrCvjnkuLvzFBQWB3vWB3K5KQTveQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.2.0", "@eslint-community/regexpp": "^4.12.1", @@ -8256,7 +8267,6 @@ "resolved": "https://registry.npmjs.org/express/-/express-4.21.2.tgz", "integrity": "sha512-28HqgMZAmih1Czt9ny7qr6ek2qddF4FclbMzwhCREB6OFfH+rXAnuNCwo1/wFvrtbgsQDb4kSbX9de9lFbrXnA==", "license": "MIT", - "peer": true, "dependencies": { "accepts": "~1.3.8", "array-flatten": "1.1.1", @@ -8318,7 +8328,6 @@ "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.7.1.tgz", "integrity": "sha512-6DnInpx7SJ2AK3+CTUE/ZM0vWTUboZCegxhC2xiIydHR9jNuTAASBrfEpHhiGOZw/nX51bHt6YQl8jsGo4y/0w==", "license": "MIT", - "peer": true, "engines": { "node": ">= 0.6" } @@ -8328,7 +8337,6 @@ "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", "license": "MIT", - "peer": true, "dependencies": { "ms": "2.0.0" } @@ -8338,7 +8346,6 @@ "resolved": "https://registry.npmjs.org/statuses/-/statuses-2.0.1.tgz", "integrity": "sha512-RwNA9Z/7PrK06rYLIzFMlaF+l73iwpzsqRIFgbMLbTcLD6cOao82TaWefPXQvB2fOC4AjuYSEndS7N/mTCbkdQ==", "license": "MIT", - "peer": true, "engines": { "node": ">= 0.8" } @@ -8546,7 +8553,6 @@ "resolved": "https://registry.npmjs.org/finalhandler/-/finalhandler-1.3.1.tgz", "integrity": "sha512-6BN9trH7bp3qvnrRyzsBz+g3lZxTNZTbVO2EV1CS0WIcDbawYVdYvGflME/9QP0h0pYlCDBCTjYa9nZzMDpyxQ==", "license": "MIT", - "peer": true, "dependencies": { "debug": "2.6.9", "encodeurl": "~2.0.0", @@ -8565,7 +8571,6 @@ "resolved": "https://registry.npmjs.org/debug/-/debug-2.6.9.tgz", "integrity": "sha512-bC7ElrdJaJnPbAP+1EotYvqZsb3ecl5wi6Bfi6BJTUcNowp6cvspg0jXznRTKDjm/E7AdgFBVeAPVMNcKGsHMA==", "license": "MIT", - "peer": true, "dependencies": { "ms": "2.0.0" } @@ -8574,15 +8579,13 @@ "version": "2.0.0", "resolved": "https://registry.npmjs.org/ms/-/ms-2.0.0.tgz", "integrity": "sha512-Tpp60P6IUJDTuOq/5Z8cdskzJujfwqfOTkrwIwj7IRISpnkJnT6SyJ4PCPnGMoFjC9ddhal5KVIYtAt97ix05A==", - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/finalhandler/node_modules/statuses": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/statuses/-/statuses-2.0.1.tgz", "integrity": "sha512-RwNA9Z/7PrK06rYLIzFMlaF+l73iwpzsqRIFgbMLbTcLD6cOao82TaWefPXQvB2fOC4AjuYSEndS7N/mTCbkdQ==", "license": "MIT", - "peer": true, "engines": { "node": ">= 0.8" } @@ -9639,6 +9642,7 @@ "resolved": "https://registry.npmjs.org/ink/-/ink-6.2.3.tgz", "integrity": "sha512-fQkfEJjKbLXIcVWEE3MvpYSnwtbbmRsmeNDNz1pIuOFlwE+UF2gsy228J36OXKZGWJWZJKUigphBSqCNMcARtg==", "license": "MIT", + "peer": true, "dependencies": { "@alcalzone/ansi-tokenize": "^0.2.0", "ansi-escapes": "^7.0.0", @@ -10616,6 +10620,7 @@ "integrity": "sha512-/imKNG4EbWNrVjoNC/1H5/9GFy+tqjGBHCaSsN+P2RnPqjsLmv6UD3Ej+Kj8nBWaRAwyk7kK5ZUc+OEatnTR3A==", "dev": true, "license": "MIT", + "peer": true, "bin": { "jiti": "bin/jiti.js" } @@ -11495,7 +11500,6 @@ "resolved": "https://registry.npmjs.org/methods/-/methods-1.1.2.tgz", "integrity": "sha512-iclAHeNqNm68zFtnZ0e+1L2yUIdvzNoauKU4WBA3VvH/vPFieF7qfRlwUZU+DA9P9bPXIS90ulxoUoCH23sV2w==", "license": "MIT", - "peer": true, "engines": { "node": ">= 0.6" } @@ -12678,8 +12682,7 @@ "version": "0.1.12", "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-0.1.12.tgz", "integrity": "sha512-RA1GjUVMnvYFxuqovrEqZoxxW5NUZqbwKtYz/Tt7nXerk0LbLblQmrsgdeOxV5SFHf0UDggjS/bSeOZwt1pmEQ==", - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/path-type": { "version": "3.0.0", @@ -12842,7 +12845,6 @@ "os": [ "darwin" ], - "peer": true, "engines": { "node": "^8.16.0 || ^10.6.0 || >=11.0.0" } @@ -12877,6 +12879,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "nanoid": "^3.3.11", "picocolors": "^1.1.1", @@ -13036,6 +13039,7 @@ "integrity": "sha512-5xGWRa90Sp2+x1dQtNpIpeOQpTDBs9cZDmA/qs2vDNN2i18PdapqY7CmBeyLlMuGqXJRIOPaCaVZTLNQRWUH/A==", "dev": true, "license": "MIT", + "peer": true, "bin": { "prettier": "bin/prettier.cjs" }, @@ -13351,6 +13355,7 @@ "resolved": "https://registry.npmjs.org/react/-/react-19.2.4.tgz", "integrity": "sha512-9nfp2hYpCwOjAN+8TZFGhtWEwgvWHXqESH8qT89AT/lWklpLON22Lc8pEtnpsZz7VmawabSU0gCjnj8aC0euHQ==", "license": "MIT", + "peer": true, "engines": { "node": ">=0.10.0" } @@ -13361,6 +13366,7 @@ "integrity": "sha512-cq/o30z9W2Wb4rzBefjv5fBalHU0rJGZCHAkf/RHSBWSSYwh8PlQTqqOJmgIIbBtpj27T6FIPXeomIjZtCNVqA==", "devOptional": true, "license": "MIT", + "peer": true, "dependencies": { "shell-quote": "^1.6.1", "ws": "^7" @@ -13438,6 +13444,7 @@ "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-19.2.4.tgz", "integrity": "sha512-AXJdLo8kgMbimY95O2aKQqsz2iWi9jMgKJhRBAxECE4IFxfcazB2LmzloIoibJI3C12IlY20+KFaLv+71bUJeQ==", "license": "MIT", + "peer": true, "dependencies": { "scheduler": "^0.27.0" }, @@ -14621,6 +14628,7 @@ "integrity": "sha512-fIQnFtpksRRgHR1CO1onGX3djaog4qsW/c5U8arqYTkUEr2TaWpn05mIJDOBoPJFlOdqFrB4Ttv0PZJxV7avhw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@storybook/global": "^5.0.0", "@storybook/icons": "^2.0.1", @@ -15309,6 +15317,7 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -15508,7 +15517,8 @@ "version": "2.8.1", "resolved": "https://registry.npmjs.org/tslib/-/tslib-2.8.1.tgz", "integrity": "sha512-oJFu94HQb+KVduSUQL7wnpmqnfmLsOA/nAh6b6EH0wCEoK0/mPeXU6c3wKDV83MkOuHPRHtSXKKU99IBazS/2w==", - "license": "0BSD" + "license": "0BSD", + "peer": true }, "node_modules/tsx": { "version": "4.20.3", @@ -15516,6 +15526,7 @@ "integrity": "sha512-qjbnuR9Tr+FJOMBqJCW5ehvIo/buZq7vH7qD7JziU98h6l3qGy0a/yPFjwO+y0/T7GFpNgNAvEcPPVfyT8rrPQ==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "~0.25.0", "get-tsconfig": "^4.7.5" @@ -15674,6 +15685,7 @@ "integrity": "sha512-p1diW6TqL9L07nNxvRMM7hMMw4c5XOo/1ibL4aAIGmSAt9slTE1Xgw5KWuof2uTOvCg9BY7ZRi+GaF+7sfgPeQ==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -15997,7 +16009,6 @@ "resolved": "https://registry.npmjs.org/utils-merge/-/utils-merge-1.0.1.tgz", "integrity": "sha512-pMZTvIkT1d+TFGvDOqodOclx0QWkkgi6Tdoa8gC8ffGAAqz9pzPTZWAybbsHHoED/ztMtkv/VoYTYyShUn81hA==", "license": "MIT", - "peer": true, "engines": { "node": ">= 0.4.0" } @@ -16040,6 +16051,7 @@ "integrity": "sha512-ixXJB1YRgDIw2OszKQS9WxGHKwLdCsbQNkpJN171udl6szi/rIySHL6/Os3s2+oE4P/FLD4dxg4mD7Wust+u5g==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.25.0", "fdir": "^6.4.6", @@ -16153,6 +16165,7 @@ "integrity": "sha512-M7BAV6Rlcy5u+m6oPhAPFgJTzAioX/6B0DxyvDlo9l8+T3nLKbrczg2WLUyzd45L8RqfUMyGPzekbMvX2Ldkwg==", "dev": true, "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -16166,6 +16179,7 @@ "integrity": "sha512-LUCP5ev3GURDysTWiP47wRRUpLKMOfPh+yKTx3kVIEiu5KOMeqzpnYNsKyOoVrULivR8tLcks4+lga33Whn90A==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@types/chai": "^5.2.2", "@vitest/expect": "3.2.4", @@ -16684,6 +16698,7 @@ "integrity": "sha512-lcYcMxX2PO9XMGvAJkJ3OsNMw+/7FKes7/hgerGUYWIoWu5j/+YQqcZr5JnPZWzOsEBgMbSbiSTn/dv/69Mkpw==", "dev": true, "license": "ISC", + "peer": true, "bin": { "yaml": "bin.mjs" }, @@ -16854,6 +16869,7 @@ "resolved": "https://registry.npmjs.org/zod/-/zod-3.25.76.tgz", "integrity": "sha512-gzUt/qt81nXsFGKIFcC3YnfEAx5NkunCfnDlvuBSSFS02bcXu4Lmea0AFIUwbLWxWPx3d9p8S5QoaujKcNQxcQ==", "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } @@ -17024,6 +17040,7 @@ "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.25.1.tgz", "integrity": "sha512-yO28oVFFC7EBoiKdAn+VqRm+plcfv4v0xp6osG/VsCB0NlPZWi87ajbCZZ8f/RvOFLEu7//rSRmuZZ7lMoe3gQ==", "license": "MIT", + "peer": true, "dependencies": { "@hono/node-server": "^1.19.7", "ajv": "^8.17.1", @@ -17682,6 +17699,7 @@ "resolved": "https://registry.npmjs.org/@modelcontextprotocol/sdk/-/sdk-1.25.1.tgz", "integrity": "sha512-yO28oVFFC7EBoiKdAn+VqRm+plcfv4v0xp6osG/VsCB0NlPZWi87ajbCZZ8f/RvOFLEu7//rSRmuZZ7lMoe3gQ==", "license": "MIT", + "peer": true, "dependencies": { "@hono/node-server": "^1.19.7", "ajv": "^8.17.1", @@ -18076,6 +18094,7 @@ "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -18838,6 +18857,7 @@ "integrity": "sha512-4Z+L8I2OqhZV8qA132M4wNL30ypZGYOQVBfMgxDH/K5UX0PNqTu1c6za9ST5r9+tavvHiTWmBnKzpCJ/GlVFtg==", "dev": true, "license": "BSD-2-Clause", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "7.18.0", "@typescript-eslint/types": "7.18.0", @@ -19318,6 +19338,7 @@ "deprecated": "This version is no longer supported. Please see https://eslint.org/version-support for other options.", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.2.0", "@eslint-community/regexpp": "^4.6.1", @@ -20444,6 +20465,7 @@ "integrity": "sha512-Ljb1cnSJSivGN0LqXd/zmDbWEM0RNNg2t1QW/XUhYl/qPqyu7CsqeWtqQXHVaJsecLPuDoak2oJcZN2QoRIOag==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@vitest/expect": "1.6.1", "@vitest/runner": "1.6.1", @@ -21680,6 +21702,7 @@ "integrity": "sha512-z9VXpC7MWrhfWipitjNdgCauoMLRdIILQsAEV+ZesIzBq/oUlxk0m3ApZuMFCXdnS4U7KrI+l3WRUEGQ8K1QKw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@types/prop-types": "*", "csstype": "^3.2.2" @@ -22653,6 +22676,7 @@ "integrity": "sha512-aJn6wq13/afZp/jT9QZmwEjDqqvSGp1VT5GVg+f/t6/oVyrgXM6BY1h9BRh/O5p3PlUPAe+WuiEZOmb/49RqoQ==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -22667,6 +22691,7 @@ "integrity": "sha512-o5a9xKjbtuhY6Bi5S3+HvbRERmouabWbyUcpXXUA1u+GNUKoROi9byOJ8M0nHbHYHkYICiMlqxkg1KkYmm25Sw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "esbuild": "^0.21.3", "postcss": "^8.4.43", diff --git a/packages/cli/src/acp-integration/session/Session.ts b/packages/cli/src/acp-integration/session/Session.ts index 1f25aff9e..c56fcc9d0 100644 --- a/packages/cli/src/acp-integration/session/Session.ts +++ b/packages/cli/src/acp-integration/session/Session.ts @@ -33,10 +33,7 @@ import { logToolCall, logUserPrompt, getErrorStatus, - AgentTool, UserPromptEvent, - TodoWriteTool, - ExitPlanModeTool, readManyFiles, Storage, ToolNames, @@ -1128,7 +1125,7 @@ export class Session implements SessionContext { error: Error, toolName = fc.name ?? 'unknown_tool', ) => { - if (toolName !== TodoWriteTool.Name) { + if (toolName !== ToolNames.TODO_WRITE) { await this.toolCallEmitter.emitError(callId, toolName, error); } @@ -1168,9 +1165,9 @@ export class Session implements SessionContext { } // Detect TodoWriteTool early - route to plan updates instead of tool_call events - const isTodoWriteTool = tool.name === TodoWriteTool.Name; - const isAgentTool = tool.name === AgentTool.Name; - const isExitPlanModeTool = tool.name === ExitPlanModeTool.Name; + const isTodoWriteTool = tool.name === ToolNames.TODO_WRITE; + const isAgentTool = tool.name === ToolNames.AGENT; + const isExitPlanModeTool = tool.name === ToolNames.EXIT_PLAN_MODE; // Track cleanup functions for sub-agent event listeners let subAgentCleanupFunctions: Array<() => void> = []; diff --git a/packages/cli/src/acp-integration/session/SubAgentTracker.test.ts b/packages/cli/src/acp-integration/session/SubAgentTracker.test.ts index 573a9afee..fb52eeea4 100644 --- a/packages/cli/src/acp-integration/session/SubAgentTracker.test.ts +++ b/packages/cli/src/acp-integration/session/SubAgentTracker.test.ts @@ -21,7 +21,7 @@ import type { import { AgentEventType, ToolConfirmationOutcome, - TodoWriteTool, + ToolNames, } from '@qwen-code/qwen-code-core'; import type { AgentSideConnection } from '@agentclientprotocol/sdk'; import { EventEmitter } from 'node:events'; @@ -253,7 +253,7 @@ describe('SubAgentTracker', () => { tracker.setup(eventEmitter, abortController.signal); const event = createToolCallEvent({ - name: TodoWriteTool.Name, + name: ToolNames.TODO_WRITE, callId: 'call-todo', args: { todos: [] }, }); @@ -358,7 +358,7 @@ describe('SubAgentTracker', () => { eventEmitter.emit( AgentEventType.TOOL_CALL, createToolCallEvent({ - name: TodoWriteTool.Name, + name: ToolNames.TODO_WRITE, callId: 'call-todo', args: { todos: [{ id: '1', content: 'Task 1', status: 'pending' }], @@ -368,7 +368,7 @@ describe('SubAgentTracker', () => { // Emit result with todo_list display const resultEvent = createToolResultEvent({ - name: TodoWriteTool.Name, + name: ToolNames.TODO_WRITE, callId: 'call-todo', success: true, resultDisplay: JSON.stringify({ diff --git a/packages/cli/src/acp-integration/session/emitters/ToolCallEmitter.test.ts b/packages/cli/src/acp-integration/session/emitters/ToolCallEmitter.test.ts index 9bfeb4fcb..6b0e6449e 100644 --- a/packages/cli/src/acp-integration/session/emitters/ToolCallEmitter.test.ts +++ b/packages/cli/src/acp-integration/session/emitters/ToolCallEmitter.test.ts @@ -13,7 +13,7 @@ import type { AnyDeclarativeTool, AnyToolInvocation, } from '@qwen-code/qwen-code-core'; -import { Kind, TodoWriteTool } from '@qwen-code/qwen-code-core'; +import { Kind, ToolNames } from '@qwen-code/qwen-code-core'; import type { Part } from '@google/genai'; // Helper to create mock message parts for tests @@ -107,7 +107,7 @@ describe('ToolCallEmitter', () => { it('should skip emit for TodoWriteTool and return false', async () => { const result = await emitter.emitStart({ - toolName: TodoWriteTool.Name, + toolName: ToolNames.TODO_WRITE, callId: 'call-todo', args: { todos: [] }, }); @@ -279,7 +279,7 @@ describe('ToolCallEmitter', () => { describe('TodoWriteTool handling', () => { it('should emit plan update instead of tool_call_update for TodoWriteTool', async () => { await emitter.emitResult({ - toolName: TodoWriteTool.Name, + toolName: ToolNames.TODO_WRITE, callId: 'call-todo', success: true, message: [], @@ -304,7 +304,7 @@ describe('ToolCallEmitter', () => { it('should use args as fallback for TodoWriteTool todos', async () => { await emitter.emitResult({ - toolName: TodoWriteTool.Name, + toolName: ToolNames.TODO_WRITE, callId: 'call-todo', success: true, message: [], @@ -324,7 +324,7 @@ describe('ToolCallEmitter', () => { it('should not emit anything for TodoWriteTool with empty todos', async () => { await emitter.emitResult({ - toolName: TodoWriteTool.Name, + toolName: ToolNames.TODO_WRITE, callId: 'call-todo', success: true, message: [], @@ -336,7 +336,7 @@ describe('ToolCallEmitter', () => { it('should not emit anything for TodoWriteTool with no extractable todos', async () => { await emitter.emitResult({ - toolName: TodoWriteTool.Name, + toolName: ToolNames.TODO_WRITE, callId: 'call-todo', success: true, message: [], @@ -370,8 +370,8 @@ describe('ToolCallEmitter', () => { }); describe('isTodoWriteTool', () => { - it('should return true for TodoWriteTool.Name', () => { - expect(emitter.isTodoWriteTool(TodoWriteTool.Name)).toBe(true); + it('should return true for ToolNames.TODO_WRITE', () => { + expect(emitter.isTodoWriteTool(ToolNames.TODO_WRITE)).toBe(true); }); it('should return false for other tool names', () => { @@ -578,7 +578,7 @@ describe('ToolCallEmitter', () => { describe('Fix 6: Empty plan emission when args has todos', () => { it('should emit empty plan when args had todos but result has none', async () => { await emitter.emitResult({ - toolName: TodoWriteTool.Name, + toolName: ToolNames.TODO_WRITE, callId: 'call-todo-empty', success: true, message: [], @@ -596,7 +596,7 @@ describe('ToolCallEmitter', () => { it('should emit empty plan when result todos is empty but args had todos', async () => { await emitter.emitResult({ - toolName: TodoWriteTool.Name, + toolName: ToolNames.TODO_WRITE, callId: 'call-todo-cleared', success: true, message: [], diff --git a/packages/cli/src/acp-integration/session/emitters/ToolCallEmitter.ts b/packages/cli/src/acp-integration/session/emitters/ToolCallEmitter.ts index 24dbe8662..a84e4f4e7 100644 --- a/packages/cli/src/acp-integration/session/emitters/ToolCallEmitter.ts +++ b/packages/cli/src/acp-integration/session/emitters/ToolCallEmitter.ts @@ -19,11 +19,7 @@ import type { ToolKind, } from '@agentclientprotocol/sdk'; import type { Part } from '@google/genai'; -import { - TodoWriteTool, - Kind, - ExitPlanModeTool, -} from '@qwen-code/qwen-code-core'; +import { ToolNames, Kind } from '@qwen-code/qwen-code-core'; /** * Unified tool call event emitter. @@ -185,14 +181,14 @@ export class ToolCallEmitter extends BaseEmitter { * Exposed for external use in components that need to check this. */ isTodoWriteTool(toolName: string): boolean { - return toolName === TodoWriteTool.Name; + return toolName === ToolNames.TODO_WRITE; } /** * Checks if a tool name is the ExitPlanModeTool. */ isExitPlanModeTool(toolName: string): boolean { - return toolName === ExitPlanModeTool.Name; + return toolName === ToolNames.EXIT_PLAN_MODE; } /** diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index 7f3b754c1..3033e4124 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -8,9 +8,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import * as os from 'node:os'; import * as path from 'node:path'; import { - ShellTool, - EditTool, - WriteFileTool, + ToolNames, DEFAULT_QWEN_MODEL, OutputFormat, NativeLspService, @@ -1020,7 +1018,11 @@ describe('loadCliConfig telemetry', () => { }); describe('mergeExcludeTools', () => { - const defaultExcludes = [ShellTool.Name, EditTool.Name, WriteFileTool.Name]; + const defaultExcludes = [ + ToolNames.SHELL, + ToolNames.EDIT, + ToolNames.WRITE_FILE, + ]; const originalIsTTY = process.stdin.isTTY; beforeEach(() => { @@ -1083,9 +1085,9 @@ describe('Approval mode tool exclusion logic', () => { const config = await loadCliConfig(settings, argv, undefined, []); const excludedTools = config.getPermissionsDeny(); - expect(excludedTools).toContain(ShellTool.Name); - expect(excludedTools).toContain(EditTool.Name); - expect(excludedTools).toContain(WriteFileTool.Name); + expect(excludedTools).toContain(ToolNames.SHELL); + expect(excludedTools).toContain(ToolNames.EDIT); + expect(excludedTools).toContain(ToolNames.WRITE_FILE); }); it('should exclude all interactive tools in non-interactive mode with plan approval mode', async () => { @@ -1102,9 +1104,9 @@ describe('Approval mode tool exclusion logic', () => { const config = await loadCliConfig(settings, argv, undefined, []); const excludedTools = config.getPermissionsDeny(); - expect(excludedTools).toContain(ShellTool.Name); - expect(excludedTools).toContain(EditTool.Name); - expect(excludedTools).toContain(WriteFileTool.Name); + expect(excludedTools).toContain(ToolNames.SHELL); + expect(excludedTools).toContain(ToolNames.EDIT); + expect(excludedTools).toContain(ToolNames.WRITE_FILE); }); it('should exclude all interactive tools in non-interactive mode with explicit default approval mode', async () => { @@ -1122,9 +1124,9 @@ describe('Approval mode tool exclusion logic', () => { const config = await loadCliConfig(settings, argv, undefined, []); const excludedTools = config.getPermissionsDeny(); - expect(excludedTools).toContain(ShellTool.Name); - expect(excludedTools).toContain(EditTool.Name); - expect(excludedTools).toContain(WriteFileTool.Name); + expect(excludedTools).toContain(ToolNames.SHELL); + expect(excludedTools).toContain(ToolNames.EDIT); + expect(excludedTools).toContain(ToolNames.WRITE_FILE); }); it('should not exclude a tool explicitly allowed in tools.allowed', async () => { @@ -1132,16 +1134,16 @@ describe('Approval mode tool exclusion logic', () => { const argv = await parseArguments(); const settings: Settings = { tools: { - allowed: [ShellTool.Name], + allowed: [ToolNames.SHELL], }, }; const config = await loadCliConfig(settings, argv, undefined, []); const excludedTools = config.getPermissionsDeny(); - expect(excludedTools).not.toContain(ShellTool.Name); - expect(excludedTools).toContain(EditTool.Name); - expect(excludedTools).toContain(WriteFileTool.Name); + expect(excludedTools).not.toContain(ToolNames.SHELL); + expect(excludedTools).toContain(ToolNames.EDIT); + expect(excludedTools).toContain(ToolNames.WRITE_FILE); }); it('should not exclude a tool explicitly allowed in tools.core', async () => { @@ -1149,16 +1151,16 @@ describe('Approval mode tool exclusion logic', () => { const argv = await parseArguments(); const settings: Settings = { tools: { - core: [ShellTool.Name], + core: [ToolNames.SHELL], }, }; const config = await loadCliConfig(settings, argv, undefined, []); const excludedTools = config.getPermissionsDeny(); - expect(excludedTools).not.toContain(ShellTool.Name); - expect(excludedTools).toContain(EditTool.Name); - expect(excludedTools).toContain(WriteFileTool.Name); + expect(excludedTools).not.toContain(ToolNames.SHELL); + expect(excludedTools).toContain(ToolNames.EDIT); + expect(excludedTools).toContain(ToolNames.WRITE_FILE); }); it('should exclude only shell tools in non-interactive mode with auto-edit approval mode', async () => { @@ -1176,9 +1178,9 @@ describe('Approval mode tool exclusion logic', () => { const config = await loadCliConfig(settings, argv, undefined, []); const excludedTools = config.getPermissionsDeny(); - expect(excludedTools).toContain(ShellTool.Name); - expect(excludedTools).not.toContain(EditTool.Name); - expect(excludedTools).not.toContain(WriteFileTool.Name); + expect(excludedTools).toContain(ToolNames.SHELL); + expect(excludedTools).not.toContain(ToolNames.EDIT); + expect(excludedTools).not.toContain(ToolNames.WRITE_FILE); }); it('should exclude no interactive tools in non-interactive mode with yolo approval mode', async () => { @@ -1196,9 +1198,9 @@ describe('Approval mode tool exclusion logic', () => { const config = await loadCliConfig(settings, argv, undefined, []); const excludedTools = config.getPermissionsDeny(); - expect(excludedTools).not.toContain(ShellTool.Name); - expect(excludedTools).not.toContain(EditTool.Name); - expect(excludedTools).not.toContain(WriteFileTool.Name); + expect(excludedTools).not.toContain(ToolNames.SHELL); + expect(excludedTools).not.toContain(ToolNames.EDIT); + expect(excludedTools).not.toContain(ToolNames.WRITE_FILE); }); it('should exclude no interactive tools in non-interactive mode with legacy yolo flag', async () => { @@ -1209,9 +1211,9 @@ describe('Approval mode tool exclusion logic', () => { const config = await loadCliConfig(settings, argv, undefined, []); const excludedTools = config.getPermissionsDeny(); - expect(excludedTools).not.toContain(ShellTool.Name); - expect(excludedTools).not.toContain(EditTool.Name); - expect(excludedTools).not.toContain(WriteFileTool.Name); + expect(excludedTools).not.toContain(ToolNames.SHELL); + expect(excludedTools).not.toContain(ToolNames.EDIT); + expect(excludedTools).not.toContain(ToolNames.WRITE_FILE); }); it('should not exclude interactive tools in interactive mode regardless of approval mode', async () => { @@ -1234,9 +1236,9 @@ describe('Approval mode tool exclusion logic', () => { const config = await loadCliConfig(settings, argv, undefined, []); const excludedTools = config.getPermissionsDeny(); - expect(excludedTools).not.toContain(ShellTool.Name); - expect(excludedTools).not.toContain(EditTool.Name); - expect(excludedTools).not.toContain(WriteFileTool.Name); + expect(excludedTools).not.toContain(ToolNames.SHELL); + expect(excludedTools).not.toContain(ToolNames.EDIT); + expect(excludedTools).not.toContain(ToolNames.WRITE_FILE); } }); @@ -1255,9 +1257,9 @@ describe('Approval mode tool exclusion logic', () => { const excludedTools = config.getPermissionsDeny(); expect(excludedTools).toContain('custom_tool'); // From settings - expect(excludedTools).toContain(ShellTool.Name); // From approval mode - expect(excludedTools).not.toContain(EditTool.Name); // Should be allowed in auto-edit - expect(excludedTools).not.toContain(WriteFileTool.Name); // Should be allowed in auto-edit + expect(excludedTools).toContain(ToolNames.SHELL); // From approval mode + expect(excludedTools).not.toContain(ToolNames.EDIT); // Should be allowed in auto-edit + expect(excludedTools).not.toContain(ToolNames.WRITE_FILE); // Should be allowed in auto-edit }); it('should throw an error for invalid approval mode values in loadCliConfig', async () => { diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index 567f7436a..c8400fb82 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -24,9 +24,7 @@ import { type ResumedSessionData, type LspClient, type ToolName, - EditTool, - ShellTool, - WriteFileTool, + ToolNames, NativeLspClient, createDebugLogger, NativeLspService, @@ -955,13 +953,13 @@ export async function loadCliConfig( case ApprovalMode.PLAN: case ApprovalMode.DEFAULT: // Deny all write/execute tools unless explicitly allowed. - denyUnlessAllowed(ShellTool.Name as ToolName); - denyUnlessAllowed(EditTool.Name as ToolName); - denyUnlessAllowed(WriteFileTool.Name as ToolName); + denyUnlessAllowed(ToolNames.SHELL as ToolName); + denyUnlessAllowed(ToolNames.EDIT as ToolName); + denyUnlessAllowed(ToolNames.WRITE_FILE as ToolName); break; case ApprovalMode.AUTO_EDIT: // Only shell requires a prompt in auto-edit mode. - denyUnlessAllowed(ShellTool.Name as ToolName); + denyUnlessAllowed(ToolNames.SHELL as ToolName); break; case ApprovalMode.YOLO: // No extra denials for YOLO mode. diff --git a/packages/cli/src/ui/commands/clearCommand.ts b/packages/cli/src/ui/commands/clearCommand.ts index 571ee5c6c..c0c2bc5ea 100644 --- a/packages/cli/src/ui/commands/clearCommand.ts +++ b/packages/cli/src/ui/commands/clearCommand.ts @@ -12,7 +12,6 @@ import { SessionEndReason, SessionStartSource, ToolNames, - SkillTool, type PermissionMode, } from '@qwen-code/qwen-code-core'; @@ -45,8 +44,8 @@ export const clearCommand: SlashCommand = { .getToolRegistry() ?.getAllTools() .find((tool) => tool.name === ToolNames.SKILL); - if (skillTool instanceof SkillTool) { - skillTool.clearLoadedSkills(); + if (skillTool && 'clearLoadedSkills' in skillTool) { + (skillTool as { clearLoadedSkills(): void }).clearLoadedSkills(); } if (newSessionId && context.session.startNewSession) { diff --git a/packages/cli/src/ui/commands/contextCommand.ts b/packages/cli/src/ui/commands/contextCommand.ts index 0d3275ccf..499670995 100644 --- a/packages/cli/src/ui/commands/contextCommand.ts +++ b/packages/cli/src/ui/commands/contextCommand.ts @@ -23,7 +23,6 @@ import { getCoreSystemPrompt, DEFAULT_TOKEN_LIMIT, ToolNames, - SkillTool, buildSkillLlmContent, } from '@qwen-code/qwen-code-core'; import { t } from '../../i18n/index.js'; @@ -138,8 +137,10 @@ export async function collectContextData( : 0; const loadedSkillNames: ReadonlySet = - skillTool instanceof SkillTool - ? skillTool.getLoadedSkillNames() + skillTool && 'getLoadedSkillNames' in skillTool + ? ( + skillTool as { getLoadedSkillNames(): ReadonlySet } + ).getLoadedSkillNames() : new Set(); const skillManager = config.getSkillManager(); diff --git a/packages/cli/src/ui/hooks/useToolScheduler.test.ts b/packages/cli/src/ui/hooks/useToolScheduler.test.ts index db8c878bc..48bf9196b 100644 --- a/packages/cli/src/ui/hooks/useToolScheduler.test.ts +++ b/packages/cli/src/ui/hooks/useToolScheduler.test.ts @@ -45,6 +45,7 @@ vi.mock('@qwen-code/qwen-code-core', async () => { const mockToolRegistry = { getTool: vi.fn(), + ensureTool: vi.fn(async (name: string) => mockToolRegistry.getTool(name)), getAllToolNames: vi.fn(() => ['mockTool', 'anotherTool']), }; @@ -101,6 +102,7 @@ describe('useReactToolScheduler in YOLO Mode', () => { onComplete = vi.fn(); setPendingHistoryItem = vi.fn(); mockToolRegistry.getTool.mockClear(); + mockToolRegistry.ensureTool.mockClear(); (mockToolRequiresConfirmation.execute as Mock).mockClear(); (mockToolRequiresConfirmation.getConfirmationDetails as Mock).mockClear(); @@ -207,6 +209,7 @@ describe('useReactToolScheduler', () => { setPendingHistoryItem = vi.fn(); mockToolRegistry.getTool.mockClear(); + mockToolRegistry.ensureTool.mockClear(); (mockTool.execute as Mock).mockClear(); (mockToolRequiresConfirmation.execute as Mock).mockClear(); (mockToolRequiresConfirmation.getConfirmationDetails as Mock).mockClear(); diff --git a/packages/cli/src/ui/utils/export/normalize.ts b/packages/cli/src/ui/utils/export/normalize.ts index cf9f80cdc..99ab62c32 100644 --- a/packages/cli/src/ui/utils/export/normalize.ts +++ b/packages/cli/src/ui/utils/export/normalize.ts @@ -5,7 +5,7 @@ */ import type { Part } from '@google/genai'; -import { ExitPlanModeTool, ToolNames } from '@qwen-code/qwen-code-core'; +import { ToolNames } from '@qwen-code/qwen-code-core'; import type { ChatRecord, Config, Kind } from '@qwen-code/qwen-code-core'; import type { ExportMessage, ExportSessionData } from './types.js'; @@ -244,7 +244,7 @@ function resolveToolMetadata( * Maps tool kind to allowed export kinds. */ function mapToolKind(kind: Kind | undefined, toolName?: string): string { - if (toolName && toolName === ExitPlanModeTool.Name) { + if (toolName && toolName === ToolNames.EXIT_PLAN_MODE) { return 'switch_mode'; } diff --git a/packages/core/src/agents/runtime/agent-core.ts b/packages/core/src/agents/runtime/agent-core.ts index 97627b7e6..7effc025f 100644 --- a/packages/core/src/agents/runtime/agent-core.ts +++ b/packages/core/src/agents/runtime/agent-core.ts @@ -300,8 +300,9 @@ export class AgentCore { * If no explicit toolConfig or it contains "*" or is empty, * inherits all tools (excluding AgentTool to prevent recursion). */ - prepareTools(): FunctionDeclaration[] { + async prepareTools(): Promise { const toolRegistry = this.runtimeContext.getToolRegistry(); + await toolRegistry.warmAll(); const toolsList: FunctionDeclaration[] = []; const excludedFromSubagents = EXCLUDED_TOOLS_FOR_SUBAGENTS; @@ -941,6 +942,7 @@ export class AgentCore { /** * Safely retrieves the description of a tool by attempting to build it. * Returns an empty string if any error occurs during the process. + * Note: Assumes tools are warmed via warmAll() before the reasoning loop. */ getToolDescription(toolName: string, args: Record): string { try { diff --git a/packages/core/src/agents/runtime/agent-headless.test.ts b/packages/core/src/agents/runtime/agent-headless.test.ts index 89760ee0b..f377a12a3 100644 --- a/packages/core/src/agents/runtime/agent-headless.test.ts +++ b/packages/core/src/agents/runtime/agent-headless.test.ts @@ -133,11 +133,16 @@ async function createMockConfig( await config.refreshAuth(AuthType.USE_GEMINI); // Mock ToolRegistry - const mockToolRegistry = { + const mockToolRegistryBase = { + warmAll: vi.fn().mockResolvedValue(undefined), getTool: vi.fn(), getFunctionDeclarations: vi.fn().mockReturnValue([]), getFunctionDeclarationsFiltered: vi.fn().mockReturnValue([]), getAllToolNames: vi.fn().mockReturnValue([]), + }; + const mockToolRegistry = { + ...mockToolRegistryBase, + ensureTool: vi.fn(async (name: string) => mockToolRegistry.getTool(name)), ...toolRegistryMocks, } as unknown as ToolRegistry; diff --git a/packages/core/src/agents/runtime/agent-headless.ts b/packages/core/src/agents/runtime/agent-headless.ts index ac02f80df..bba61038c 100644 --- a/packages/core/src/agents/runtime/agent-headless.ts +++ b/packages/core/src/agents/runtime/agent-headless.ts @@ -212,7 +212,7 @@ export class AgentHeadless { abortController.abort(); } - const toolsList = this.core.prepareTools(); + const toolsList = await this.core.prepareTools(); const initialTaskText = String( (context.get('task_prompt') as string) ?? 'Get Started!', diff --git a/packages/core/src/agents/runtime/agent-interactive.ts b/packages/core/src/agents/runtime/agent-interactive.ts index 42e9dedce..0575eee18 100644 --- a/packages/core/src/agents/runtime/agent-interactive.ts +++ b/packages/core/src/agents/runtime/agent-interactive.ts @@ -110,7 +110,7 @@ export class AgentInteractive { return; } - this.toolsList = this.core.prepareTools(); + this.toolsList = await this.core.prepareTools(); this.core.stats.start(Date.now()); if (this.config.chatHistory?.length) { diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index a7bf42d7a..fba2afec9 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -30,13 +30,11 @@ import { import { GeminiClient } from '../core/client.js'; import { GitService } from '../services/gitService.js'; import { ShellTool } from '../tools/shell.js'; -import { ReadFileTool } from '../tools/read-file.js'; -import { GrepTool } from '../tools/grep.js'; import { canUseRipgrep } from '../utils/ripgrepUtils.js'; -import { RipGrepTool } from '../tools/ripGrep.js'; import { logRipgrepFallback } from '../telemetry/loggers.js'; import { RipgrepFallbackEvent } from '../telemetry/types.js'; import { ToolRegistry } from '../tools/tool-registry.js'; +import { ToolNames } from '../tools/tool-names.js'; import { fireNotificationHook } from '../core/toolHookTriggers.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { loadServerHierarchicalMemory } from '../utils/memoryDiscovery.js'; @@ -74,6 +72,9 @@ vi.mock('node:fs', async (importOriginal) => { vi.mock('../tools/tool-registry', () => { const ToolRegistryMock = vi.fn(); ToolRegistryMock.prototype.registerTool = vi.fn(); + ToolRegistryMock.prototype.registerFactory = vi.fn(); + ToolRegistryMock.prototype.ensureTool = vi.fn(); + ToolRegistryMock.prototype.warmAll = vi.fn(); ToolRegistryMock.prototype.discoverAllTools = vi.fn(); ToolRegistryMock.prototype.getAllTools = vi.fn(() => []); // Mock methods if needed ToolRegistryMock.prototype.getAllToolNames = vi.fn(() => []); @@ -137,6 +138,15 @@ vi.mock('../memory/const.js', () => ({ DEFAULT_CONTEXT_FILENAME: 'QWEN.md', QWEN_CONFIG_DIR: '.qwen', })); +vi.mock('../tools/memory-config', () => ({ + setGeminiMdFilename: vi.fn(), + getCurrentGeminiMdFilename: vi.fn(() => 'QWEN.md'), + getAllGeminiMdFilenames: vi.fn(() => ['QWEN.md', 'AGENTS.md']), + DEFAULT_CONTEXT_FILENAME: 'QWEN.md', + AGENT_CONTEXT_FILENAME: 'AGENTS.md', + QWEN_CONFIG_DIR: '.qwen', + MEMORY_SECTION_HEADER: '## Qwen Added Memories', +})); vi.mock('../core/contentGenerator.js'); @@ -962,20 +972,20 @@ describe('Server Config (config.ts)', () => { // The ToolRegistry class is mocked, so we can inspect its prototype's methods. const registerToolMock = ( (await vi.importMock('../tools/tool-registry')) as { - ToolRegistry: { prototype: { registerTool: Mock } }; + ToolRegistry: { prototype: { registerFactory: Mock } }; } - ).ToolRegistry.prototype.registerTool; + ).ToolRegistry.prototype.registerFactory; // Check that registerTool was called for ShellTool const wasShellToolRegistered = (registerToolMock as Mock).mock.calls.some( - (call) => call[0] instanceof vi.mocked(ShellTool), + (call) => call[0] === ToolNames.SHELL, ); expect(wasShellToolRegistered).toBe(true); // Check that registerTool was NOT called for ReadFileTool const wasReadFileToolRegistered = ( registerToolMock as Mock - ).mock.calls.some((call) => call[0] instanceof vi.mocked(ReadFileTool)); + ).mock.calls.some((call) => call[0] === ToolNames.READ_FILE); expect(wasReadFileToolRegistered).toBe(false); }); @@ -989,12 +999,12 @@ describe('Server Config (config.ts)', () => { const registerToolMock = ( (await vi.importMock('../tools/tool-registry')) as { - ToolRegistry: { prototype: { registerTool: Mock } }; + ToolRegistry: { prototype: { registerFactory: Mock } }; } - ).ToolRegistry.prototype.registerTool; + ).ToolRegistry.prototype.registerFactory; const wasShellToolRegistered = (registerToolMock as Mock).mock.calls.some( - (call) => call[0] instanceof vi.mocked(ShellTool), + (call) => call[0] === ToolNames.SHELL, ); expect(wasShellToolRegistered).toBe(true); }); @@ -1009,12 +1019,12 @@ describe('Server Config (config.ts)', () => { const registerToolMock = ( (await vi.importMock('../tools/tool-registry')) as { - ToolRegistry: { prototype: { registerTool: Mock } }; + ToolRegistry: { prototype: { registerFactory: Mock } }; } - ).ToolRegistry.prototype.registerTool; + ).ToolRegistry.prototype.registerFactory; const wasShellToolRegistered = (registerToolMock as Mock).mock.calls.some( - (call) => call[0] instanceof vi.mocked(ShellTool), + (call) => call[0] === ToolNames.SHELL, ); expect(wasShellToolRegistered).toBe(true); }); @@ -1030,12 +1040,12 @@ describe('Server Config (config.ts)', () => { const registerToolMock = ( (await vi.importMock('../tools/tool-registry')) as { - ToolRegistry: { prototype: { registerTool: Mock } }; + ToolRegistry: { prototype: { registerFactory: Mock } }; } - ).ToolRegistry.prototype.registerTool; + ).ToolRegistry.prototype.registerFactory; const wasGrepToolRegistered = (registerToolMock as Mock).mock.calls.some( - (call) => call[0] instanceof vi.mocked(GrepTool), + (call) => call[0] === ToolNames.GREP, ); expect(wasGrepToolRegistered).toBe(true); }); @@ -1052,12 +1062,12 @@ describe('Server Config (config.ts)', () => { const registerToolMock = ( (await vi.importMock('../tools/tool-registry')) as { - ToolRegistry: { prototype: { registerTool: Mock } }; + ToolRegistry: { prototype: { registerFactory: Mock } }; } - ).ToolRegistry.prototype.registerTool; + ).ToolRegistry.prototype.registerFactory; const wasGrepToolRegistered = (registerToolMock as Mock).mock.calls.some( - (call) => call[0] instanceof vi.mocked(GrepTool), + (call) => call[0] === ToolNames.GREP, ); expect(wasGrepToolRegistered).toBe(false); }); @@ -1094,13 +1104,13 @@ describe('Server Config (config.ts)', () => { const registerToolMock = ( (await vi.importMock('../tools/tool-registry')) as { - ToolRegistry: { prototype: { registerTool: Mock } }; + ToolRegistry: { prototype: { registerFactory: Mock } }; } - ).ToolRegistry.prototype.registerTool; + ).ToolRegistry.prototype.registerFactory; const wasShellToolRegistered = ( registerToolMock as Mock - ).mock.calls.some((call) => call[0] instanceof vi.mocked(ShellTool)); + ).mock.calls.some((call) => call[0] === ToolNames.SHELL); expect(wasShellToolRegistered).toBe(true); }); @@ -1114,13 +1124,13 @@ describe('Server Config (config.ts)', () => { const registerToolMock = ( (await vi.importMock('../tools/tool-registry')) as { - ToolRegistry: { prototype: { registerTool: Mock } }; + ToolRegistry: { prototype: { registerFactory: Mock } }; } - ).ToolRegistry.prototype.registerTool; + ).ToolRegistry.prototype.registerFactory; const wasShellToolRegistered = ( registerToolMock as Mock - ).mock.calls.some((call) => call[0] instanceof vi.mocked(ShellTool)); + ).mock.calls.some((call) => call[0] === ToolNames.SHELL); expect(wasShellToolRegistered).toBe(true); }); @@ -1135,13 +1145,13 @@ describe('Server Config (config.ts)', () => { const registerToolMock = ( (await vi.importMock('../tools/tool-registry')) as { - ToolRegistry: { prototype: { registerTool: Mock } }; + ToolRegistry: { prototype: { registerFactory: Mock } }; } - ).ToolRegistry.prototype.registerTool; + ).ToolRegistry.prototype.registerFactory; const wasShellToolRegistered = ( registerToolMock as Mock - ).mock.calls.some((call) => call[0] instanceof vi.mocked(ShellTool)); + ).mock.calls.some((call) => call[0] === ToolNames.SHELL); expect(wasShellToolRegistered).toBe(false); }); @@ -1156,13 +1166,13 @@ describe('Server Config (config.ts)', () => { const registerToolMock = ( (await vi.importMock('../tools/tool-registry')) as { - ToolRegistry: { prototype: { registerTool: Mock } }; + ToolRegistry: { prototype: { registerFactory: Mock } }; } - ).ToolRegistry.prototype.registerTool; + ).ToolRegistry.prototype.registerFactory; const wasShellToolRegistered = ( registerToolMock as Mock - ).mock.calls.some((call) => call[0] instanceof vi.mocked(ShellTool)); + ).mock.calls.some((call) => call[0] === ToolNames.SHELL); expect(wasShellToolRegistered).toBe(false); }); @@ -1176,13 +1186,13 @@ describe('Server Config (config.ts)', () => { const registerToolMock = ( (await vi.importMock('../tools/tool-registry')) as { - ToolRegistry: { prototype: { registerTool: Mock } }; + ToolRegistry: { prototype: { registerFactory: Mock } }; } - ).ToolRegistry.prototype.registerTool; + ).ToolRegistry.prototype.registerFactory; const wasShellToolRegistered = ( registerToolMock as Mock - ).mock.calls.some((call) => call[0] instanceof vi.mocked(ShellTool)); + ).mock.calls.some((call) => call[0] === ToolNames.SHELL); expect(wasShellToolRegistered).toBe(true); }); @@ -1196,13 +1206,13 @@ describe('Server Config (config.ts)', () => { const registerToolMock = ( (await vi.importMock('../tools/tool-registry')) as { - ToolRegistry: { prototype: { registerTool: Mock } }; + ToolRegistry: { prototype: { registerFactory: Mock } }; } - ).ToolRegistry.prototype.registerTool; + ).ToolRegistry.prototype.registerFactory; const wasShellToolRegistered = ( registerToolMock as Mock - ).mock.calls.some((call) => call[0] instanceof vi.mocked(ShellTool)); + ).mock.calls.some((call) => call[0] === ToolNames.SHELL); expect(wasShellToolRegistered).toBe(true); }); }); @@ -1397,25 +1407,22 @@ describe('setApprovalMode with folder trust', () => { vi.clearAllMocks(); }); - it('should register RipGrepTool when useRipgrep is true and it is available', async () => { + it('should register grep tool when useRipgrep is true and it is available', async () => { (canUseRipgrep as Mock).mockResolvedValue(true); const config = new Config({ ...baseParams, useRipgrep: true }); await config.initialize(); - const calls = (ToolRegistry.prototype.registerTool as Mock).mock.calls; - const wasRipGrepRegistered = calls.some( - (call) => call[0] instanceof vi.mocked(RipGrepTool), - ); - const wasGrepRegistered = calls.some( - (call) => call[0] instanceof vi.mocked(GrepTool), + const calls = (ToolRegistry.prototype.registerFactory as Mock).mock.calls; + const grepRegistrations = calls.filter( + (call) => call[0] === ToolNames.GREP, ); - expect(wasRipGrepRegistered).toBe(true); - expect(wasGrepRegistered).toBe(false); + // Exactly one grep tool should be registered + expect(grepRegistrations.length).toBe(1); expect(canUseRipgrep).toHaveBeenCalledWith(true); }); - it('should register RipGrepTool with system ripgrep when useBuiltinRipgrep is false', async () => { + it('should register grep tool with system ripgrep when useBuiltinRipgrep is false', async () => { (canUseRipgrep as Mock).mockResolvedValue(true); const config = new Config({ ...baseParams, @@ -1424,16 +1431,12 @@ describe('setApprovalMode with folder trust', () => { }); await config.initialize(); - const calls = (ToolRegistry.prototype.registerTool as Mock).mock.calls; - const wasRipGrepRegistered = calls.some( - (call) => call[0] instanceof vi.mocked(RipGrepTool), - ); - const wasGrepRegistered = calls.some( - (call) => call[0] instanceof vi.mocked(GrepTool), + const calls = (ToolRegistry.prototype.registerFactory as Mock).mock.calls; + const grepRegistrations = calls.filter( + (call) => call[0] === ToolNames.GREP, ); - expect(wasRipGrepRegistered).toBe(true); - expect(wasGrepRegistered).toBe(false); + expect(grepRegistrations.length).toBe(1); expect(canUseRipgrep).toHaveBeenCalledWith(false); }); @@ -1446,16 +1449,12 @@ describe('setApprovalMode with folder trust', () => { }); await config.initialize(); - const calls = (ToolRegistry.prototype.registerTool as Mock).mock.calls; - const wasRipGrepRegistered = calls.some( - (call) => call[0] instanceof vi.mocked(RipGrepTool), - ); - const wasGrepRegistered = calls.some( - (call) => call[0] instanceof vi.mocked(GrepTool), + const calls = (ToolRegistry.prototype.registerFactory as Mock).mock.calls; + const grepRegistrations = calls.filter( + (call) => call[0] === ToolNames.GREP, ); - expect(wasRipGrepRegistered).toBe(false); - expect(wasGrepRegistered).toBe(true); + expect(grepRegistrations.length).toBe(1); expect(canUseRipgrep).toHaveBeenCalledWith(false); expect(logRipgrepFallback).toHaveBeenCalledWith( config, @@ -1470,16 +1469,12 @@ describe('setApprovalMode with folder trust', () => { const config = new Config({ ...baseParams, useRipgrep: true }); await config.initialize(); - const calls = (ToolRegistry.prototype.registerTool as Mock).mock.calls; - const wasRipGrepRegistered = calls.some( - (call) => call[0] instanceof vi.mocked(RipGrepTool), - ); - const wasGrepRegistered = calls.some( - (call) => call[0] instanceof vi.mocked(GrepTool), + const calls = (ToolRegistry.prototype.registerFactory as Mock).mock.calls; + const grepRegistrations = calls.filter( + (call) => call[0] === ToolNames.GREP, ); - expect(wasRipGrepRegistered).toBe(false); - expect(wasGrepRegistered).toBe(true); + expect(grepRegistrations.length).toBe(1); expect(canUseRipgrep).toHaveBeenCalledWith(true); expect(logRipgrepFallback).toHaveBeenCalledWith( config, @@ -1495,16 +1490,12 @@ describe('setApprovalMode with folder trust', () => { const config = new Config({ ...baseParams, useRipgrep: true }); await config.initialize(); - const calls = (ToolRegistry.prototype.registerTool as Mock).mock.calls; - const wasRipGrepRegistered = calls.some( - (call) => call[0] instanceof vi.mocked(RipGrepTool), - ); - const wasGrepRegistered = calls.some( - (call) => call[0] instanceof vi.mocked(GrepTool), + const calls = (ToolRegistry.prototype.registerFactory as Mock).mock.calls; + const grepRegistrations = calls.filter( + (call) => call[0] === ToolNames.GREP, ); - expect(wasRipGrepRegistered).toBe(false); - expect(wasGrepRegistered).toBe(true); + expect(grepRegistrations.length).toBe(1); expect(logRipgrepFallback).toHaveBeenCalledWith( config, expect.any(RipgrepFallbackEvent), @@ -1517,16 +1508,12 @@ describe('setApprovalMode with folder trust', () => { const config = new Config({ ...baseParams, useRipgrep: false }); await config.initialize(); - const calls = (ToolRegistry.prototype.registerTool as Mock).mock.calls; - const wasRipGrepRegistered = calls.some( - (call) => call[0] instanceof vi.mocked(RipGrepTool), - ); - const wasGrepRegistered = calls.some( - (call) => call[0] instanceof vi.mocked(GrepTool), + const calls = (ToolRegistry.prototype.registerFactory as Mock).mock.calls; + const grepRegistrations = calls.filter( + (call) => call[0] === ToolNames.GREP, ); - expect(wasRipGrepRegistered).toBe(false); - expect(wasGrepRegistered).toBe(true); + expect(grepRegistrations.length).toBe(1); expect(canUseRipgrep).not.toHaveBeenCalled(); }); }); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 175646119..9b4850bf8 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -44,30 +44,12 @@ import { import { GitService } from '../services/gitService.js'; import { CronScheduler } from '../services/cronScheduler.js'; -// Tools -import { AskUserQuestionTool } from '../tools/askUserQuestion.js'; -import { EditTool } from '../tools/edit.js'; -import { ExitPlanModeTool } from '../tools/exitPlanMode.js'; -import { GlobTool } from '../tools/glob.js'; -import { GrepTool } from '../tools/grep.js'; -import { LSTool } from '../tools/ls.js'; +// Tools — only lightweight imports; tool classes are lazy-loaded via dynamic import import type { SendSdkMcpMessage } from '../tools/mcp-client.js'; import { setGeminiMdFilename } from '../memory/const.js'; -import { ReadFileTool } from '../tools/read-file.js'; import { canUseRipgrep } from '../utils/ripgrepUtils.js'; -import { RipGrepTool } from '../tools/ripGrep.js'; -import { ShellTool } from '../tools/shell.js'; -import { SkillTool } from '../tools/skill.js'; -import { AgentTool } from '../tools/agent/agent.js'; -import { TodoWriteTool } from '../tools/todoWrite.js'; -import { ToolRegistry } from '../tools/tool-registry.js'; -import { WebFetchTool } from '../tools/web-fetch.js'; -import { WebSearchTool } from '../tools/web-search/index.js'; -import { WriteFileTool } from '../tools/write-file.js'; -import { LspTool } from '../tools/lsp.js'; -import { CronCreateTool } from '../tools/cron-create.js'; -import { CronListTool } from '../tools/cron-list.js'; -import { CronDeleteTool } from '../tools/cron-delete.js'; +import { ToolRegistry, type ToolFactory } from '../tools/tool-registry.js'; +import { ToolNames } from '../tools/tool-names.js'; import type { LspClient } from '../lsp/types.js'; // Other modules @@ -1103,6 +1085,10 @@ export class Config { // Detect and capture runtime model snapshot (from CLI/ENV/credentials) this.modelsConfig.detectAndCaptureRuntimeModel(); + // Warm all lazy tool factories so telemetry can access tool metadata synchronously. + // Use strict mode so a broken built-in tool surfaces immediately at startup. + await this.toolRegistry.warmAll({ strict: true }); + logStartSession(this, new StartSessionEvent(this)); this.debugLogger.info('Config initialization completed'); } @@ -2446,45 +2432,51 @@ export class Config { sendSdkMcpMessage, ); - // Helper to create & register core tools that are enabled - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const registerCoreTool = async (ToolClass: any, ...args: unknown[]) => { - const toolName = ToolClass?.Name as ToolName | undefined; - const className = ToolClass?.name ?? 'UnknownTool'; - - if (!toolName) { - // Log warning and skip this tool instead of crashing + // Helper: check permission then register a lazy factory (no module import + // happens here — the dynamic import() only runs when the tool is first used). + const registerLazy = async ( + toolName: ToolName, + factory: ToolFactory, + ): Promise => { + // PermissionManager handles both the coreTools allowlist (registry-level) + // and deny rules (runtime-level) in a single check. + let pmEnabled = true; + try { + pmEnabled = this.permissionManager + ? await this.permissionManager.isToolEnabled(toolName) + : true; // Should never reach here after initialize(), but safe default. + } catch (error) { this.debugLogger.warn( - `Skipping tool registration: ${className} is missing static Name property. ` + - `Tools must define a static Name property to be registered.`, + `Failed to check permissions for tool "${toolName}", skipping registration:`, + error, ); return; } - // PermissionManager handles both the coreTools allowlist (registry-level) - // and deny rules (runtime-level) in a single check. - const pmEnabled = this.permissionManager - ? await this.permissionManager.isToolEnabled(toolName) - : true; // Should never reach here after initialize(), but safe default. - if (pmEnabled) { - try { - registry.registerTool(new ToolClass(...args)); - } catch (error) { - this.debugLogger.error( - `Failed to register tool ${className} (${toolName}):`, - error, - ); - throw error; // Re-throw after logging context - } + registry.registerFactory(toolName, factory); } }; - await registerCoreTool(AgentTool, this); - await registerCoreTool(SkillTool, this); - await registerCoreTool(LSTool, this); - await registerCoreTool(ReadFileTool, this); + // --- Core tools (always registered) --- + await registerLazy(ToolNames.AGENT, async () => { + const { AgentTool } = await import('../tools/agent/agent.js'); + return new AgentTool(this); + }); + await registerLazy(ToolNames.SKILL, async () => { + const { SkillTool } = await import('../tools/skill.js'); + return new SkillTool(this); + }); + await registerLazy(ToolNames.LS, async () => { + const { LSTool } = await import('../tools/ls.js'); + return new LSTool(this); + }); + await registerLazy(ToolNames.READ_FILE, async () => { + const { ReadFileTool } = await import('../tools/read-file.js'); + return new ReadFileTool(this); + }); + // --- Grep / RipGrep (conditional) --- if (this.getUseRipgrep()) { let useRipgrep = false; let errorString: undefined | string = undefined; @@ -2494,9 +2486,11 @@ export class Config { errorString = getErrorMessage(error); } if (useRipgrep) { - await registerCoreTool(RipGrepTool, this); + await registerLazy(ToolNames.GREP, async () => { + const { RipGrepTool } = await import('../tools/ripGrep.js'); + return new RipGrepTool(this); + }); } else { - // Log for telemetry logRipgrepFallback( this, new RipgrepFallbackEvent( @@ -2505,34 +2499,82 @@ export class Config { errorString || 'ripgrep is not available', ), ); - await registerCoreTool(GrepTool, this); + await registerLazy(ToolNames.GREP, async () => { + const { GrepTool } = await import('../tools/grep.js'); + return new GrepTool(this); + }); } } else { - await registerCoreTool(GrepTool, this); + await registerLazy(ToolNames.GREP, async () => { + const { GrepTool } = await import('../tools/grep.js'); + return new GrepTool(this); + }); } - await registerCoreTool(GlobTool, this); - await registerCoreTool(EditTool, this); - await registerCoreTool(WriteFileTool, this); - await registerCoreTool(ShellTool, this); - await registerCoreTool(TodoWriteTool, this); - await registerCoreTool(AskUserQuestionTool, this); - !this.sdkMode && (await registerCoreTool(ExitPlanModeTool, this)); - await registerCoreTool(WebFetchTool, this); + await registerLazy(ToolNames.GLOB, async () => { + const { GlobTool } = await import('../tools/glob.js'); + return new GlobTool(this); + }); + await registerLazy(ToolNames.EDIT, async () => { + const { EditTool } = await import('../tools/edit.js'); + return new EditTool(this); + }); + await registerLazy(ToolNames.WRITE_FILE, async () => { + const { WriteFileTool } = await import('../tools/write-file.js'); + return new WriteFileTool(this); + }); + await registerLazy(ToolNames.SHELL, async () => { + const { ShellTool } = await import('../tools/shell.js'); + return new ShellTool(this); + }); + await registerLazy(ToolNames.TODO_WRITE, async () => { + const { TodoWriteTool } = await import('../tools/todoWrite.js'); + return new TodoWriteTool(this); + }); + await registerLazy(ToolNames.ASK_USER_QUESTION, async () => { + const { AskUserQuestionTool } = await import( + '../tools/askUserQuestion.js' + ); + return new AskUserQuestionTool(this); + }); + if (!this.sdkMode) { + await registerLazy(ToolNames.EXIT_PLAN_MODE, async () => { + const { ExitPlanModeTool } = await import('../tools/exitPlanMode.js'); + return new ExitPlanModeTool(this); + }); + } + await registerLazy(ToolNames.WEB_FETCH, async () => { + const { WebFetchTool } = await import('../tools/web-fetch.js'); + return new WebFetchTool(this); + }); // Conditionally register web search tool if web search provider is configured if (this.getWebSearchConfig()) { - await registerCoreTool(WebSearchTool, this); + await registerLazy(ToolNames.WEB_SEARCH, async () => { + const { WebSearchTool } = await import('../tools/web-search/index.js'); + return new WebSearchTool(this); + }); } if (this.isLspEnabled() && this.getLspClient()) { - // Register the unified LSP tool - await registerCoreTool(LspTool, this); + await registerLazy(ToolNames.LSP, async () => { + const { LspTool } = await import('../tools/lsp.js'); + return new LspTool(this); + }); } // Register cron tools unless disabled if (this.isCronEnabled()) { - await registerCoreTool(CronCreateTool, this); - await registerCoreTool(CronListTool, this); - await registerCoreTool(CronDeleteTool, this); + await registerLazy(ToolNames.CRON_CREATE, async () => { + const { CronCreateTool } = await import('../tools/cron-create.js'); + return new CronCreateTool(this); + }); + await registerLazy(ToolNames.CRON_LIST, async () => { + const { CronListTool } = await import('../tools/cron-list.js'); + return new CronListTool(this); + }); + await registerLazy(ToolNames.CRON_DELETE, async () => { + const { CronDeleteTool } = await import('../tools/cron-delete.js'); + return new CronDeleteTool(this); + }); } if (!options?.skipDiscovery) { diff --git a/packages/core/src/core/client.test.ts b/packages/core/src/core/client.test.ts index 2beb6d217..6ca290c77 100644 --- a/packages/core/src/core/client.test.ts +++ b/packages/core/src/core/client.test.ts @@ -311,6 +311,8 @@ describe('Gemini Client (client.ts)', () => { // that depends on a fully-formed Config object, we need to mock the // entire implementation of Config for these tests. const mockToolRegistry = { + warmAll: vi.fn().mockResolvedValue(undefined), + ensureTool: vi.fn().mockResolvedValue(null), getFunctionDeclarations: vi.fn().mockReturnValue([]), getTool: vi.fn().mockReturnValue(null), }; diff --git a/packages/core/src/core/client.ts b/packages/core/src/core/client.ts index 493c21869..e3a93e784 100644 --- a/packages/core/src/core/client.ts +++ b/packages/core/src/core/client.ts @@ -47,8 +47,8 @@ import { import { LoopDetectionService } from '../services/loopDetectionService.js'; // Tools -import { AgentTool } from '../tools/agent/agent.js'; import type { RelevantAutoMemoryPromptResult } from '../memory/manager.js'; +import { ToolNames } from '../tools/tool-names.js'; // Telemetry import { @@ -228,12 +228,13 @@ export class GeminiClient { this.forceFullIdeContext = true; } - setTools(): void { + async setTools(): Promise { if (!this.isInitialized()) { return; } const toolRegistry = this.config.getToolRegistry(); + await toolRegistry.warmAll(); const toolDeclarations = toolRegistry.getFunctionDeclarations(); const tools: Tool[] = [{ functionDeclarations: toolDeclarations }]; this.getChat().setTools(tools); @@ -303,7 +304,7 @@ export class GeminiClient { uiTelemetryService, ); - this.setTools(); + await this.setTools(); return this.chat; } catch (error) { @@ -845,9 +846,9 @@ export class GeminiClient { } // add subagent system reminder if there are subagents - const hasAgentTool = this.config + const hasAgentTool = await this.config .getToolRegistry() - .getTool(AgentTool.Name); + .ensureTool(ToolNames.AGENT); const subagents = (await this.config.getSubagentManager().listSubagents()) .filter((subagent) => subagent.level !== 'builtin') .map((subagent) => subagent.name); diff --git a/packages/core/src/core/coreToolScheduler.test.ts b/packages/core/src/core/coreToolScheduler.test.ts index 6efa17c32..8c0b0edbe 100644 --- a/packages/core/src/core/coreToolScheduler.test.ts +++ b/packages/core/src/core/coreToolScheduler.test.ts @@ -24,8 +24,8 @@ import { ToolConfirmationOutcome, DEFAULT_TRUNCATE_TOOL_OUTPUT_LINES, DEFAULT_TRUNCATE_TOOL_OUTPUT_THRESHOLD, - SkillTool, } from '../index.js'; +import { SkillTool } from '../tools/skill.js'; import type { ToolCall, WaitingToolCall } from './coreToolScheduler.js'; import { CoreToolScheduler, @@ -239,6 +239,7 @@ describe('CoreToolScheduler', () => { const declarativeTool = mockTool; const mockToolRegistry = { getTool: () => declarativeTool, + ensureTool: async () => declarativeTool, getFunctionDeclarations: () => [], tools: new Map(), discovery: {}, @@ -318,6 +319,7 @@ describe('CoreToolScheduler', () => { const mockToolRegistry = { getTool: () => declarativeTool, + ensureTool: async () => declarativeTool, getFunctionDeclarations: () => [], tools: new Map(), discovery: {}, @@ -395,6 +397,7 @@ describe('CoreToolScheduler', () => { const mockToolRegistry = { getAllToolNames: () => ['list_files', 'read_file', 'write_file'], getTool: () => undefined, // No SkillTool in this test + ensureTool: async () => undefined, } as unknown as ToolRegistry; const mockConfig = { getToolRegistry: () => mockToolRegistry, @@ -436,6 +439,7 @@ describe('CoreToolScheduler', () => { const mockToolRegistry = { getAllToolNames: () => ['list_files', 'read_file'], getTool: () => undefined, // No SkillTool in this test + ensureTool: async () => undefined, } as unknown as ToolRegistry; // Create mocked config with excluded tools @@ -468,6 +472,7 @@ describe('CoreToolScheduler', () => { const mockToolRegistry = { getAllToolNames: () => ['list_files', 'read_file'], getTool: () => undefined, // No SkillTool in this test + ensureTool: async () => undefined, } as unknown as ToolRegistry; // Create mocked config with excluded tools @@ -497,7 +502,7 @@ describe('CoreToolScheduler', () => { ); }); - it('should suggest using Skill tool when unknown tool name matches a skill name', () => { + it('should suggest using Skill tool when unknown tool name matches a skill name', async () => { // Create a mock that passes instanceof SkillTool check const mockSkillTool = Object.create(SkillTool.prototype); mockSkillTool.getAvailableSkillNames = () => [ @@ -511,6 +516,8 @@ describe('CoreToolScheduler', () => { getAllToolNames: () => ['skill', 'list_files', 'read_file'], getTool: (name: string) => name === 'skill' ? mockSkillTool : undefined, + ensureTool: async (name: string) => + name === 'skill' ? mockSkillTool : undefined, } as unknown as ToolRegistry; // Create mocked config @@ -533,7 +540,7 @@ describe('CoreToolScheduler', () => { // Test that when unknown tool name matches a skill name, we get skill-specific message // @ts-expect-error accessing private method - const skillMessage = scheduler.getToolNotFoundMessage('pdf'); + const skillMessage = await scheduler.getToolNotFoundMessage('pdf'); expect(skillMessage).toContain('is a skill name, not a tool name'); expect(skillMessage).toContain('skill'); expect(skillMessage).toContain('skill: "pdf"'); @@ -542,13 +549,14 @@ describe('CoreToolScheduler', () => { // Test another skill name // @ts-expect-error accessing private method - const xlsxMessage = scheduler.getToolNotFoundMessage('xlsx'); + const xlsxMessage = await scheduler.getToolNotFoundMessage('xlsx'); expect(xlsxMessage).toContain('is a skill name, not a tool name'); expect(xlsxMessage).toContain('skill: "xlsx"'); // Test that non-skill names still use standard message with Levenshtein suggestions - // @ts-expect-error accessing private method - const nonSkillMessage = scheduler.getToolNotFoundMessage('list_fils'); + const nonSkillMessage = + // eslint-disable-next-line @typescript-eslint/no-explicit-any + await (scheduler as any).getToolNotFoundMessage('list_fils'); expect(nonSkillMessage).toContain('not found in registry'); expect(nonSkillMessage).toContain('Did you mean'); expect(nonSkillMessage).not.toContain('is a skill name'); @@ -562,6 +570,7 @@ describe('CoreToolScheduler', () => { const mockToolRegistry = { getTool: () => undefined, // Tool not in registry + ensureTool: async () => undefined, getAllToolNames: () => ['list_files', 'read_file'], getFunctionDeclarations: () => [], tools: new Map(), @@ -650,6 +659,7 @@ describe('CoreToolScheduler', () => { const mockToolRegistry = { getTool: () => undefined, // Tool not in registry + ensureTool: async () => undefined, getAllToolNames: () => ['list_files', 'read_file'], getFunctionDeclarations: () => [], tools: new Map(), @@ -740,6 +750,7 @@ describe('CoreToolScheduler with payload', () => { const declarativeTool = mockTool; const mockToolRegistry = { getTool: () => declarativeTool, + ensureTool: async () => declarativeTool, getFunctionDeclarations: () => [], tools: new Map(), discovery: {}, @@ -1085,6 +1096,7 @@ describe('CoreToolScheduler edit cancellation', () => { const mockEditTool = new MockEditTool(); const mockToolRegistry = { getTool: () => mockEditTool, + ensureTool: async () => mockEditTool, getFunctionDeclarations: () => [], tools: new Map(), discovery: {}, @@ -1192,6 +1204,7 @@ describe('CoreToolScheduler YOLO mode', () => { const mockToolRegistry = { getTool: () => declarativeTool, + ensureTool: async () => declarativeTool, getToolByName: () => declarativeTool, // Other properties are not needed for this test but are included for type consistency. getFunctionDeclarations: () => [], @@ -1346,6 +1359,7 @@ describe('CoreToolScheduler cancellation during executing with live output', () const tool = new StreamingTool(); const mockToolRegistry = { getTool: () => tool, + ensureTool: async () => tool, getFunctionDeclarations: () => [], tools: new Map(), discovery: {}, @@ -1439,6 +1453,7 @@ describe('CoreToolScheduler request queueing', () => { const mockToolRegistry = { getTool: () => declarativeTool, + ensureTool: async () => declarativeTool, getToolByName: () => declarativeTool, getFunctionDeclarations: () => [], tools: new Map(), @@ -1564,6 +1579,7 @@ describe('CoreToolScheduler request queueing', () => { const declarativeTool = mockTool; const mockToolRegistry = { getTool: () => declarativeTool, + ensureTool: async () => declarativeTool, getToolByName: () => declarativeTool, getFunctionDeclarations: () => [], tools: new Map(), @@ -1686,6 +1702,7 @@ describe('CoreToolScheduler request queueing', () => { const testTool = new TestApprovalTool(mockConfig); const toolRegistry = { getTool: () => testTool, + ensureTool: async () => testTool, getFunctionDeclarations: () => [], getFunctionDeclarationsFiltered: () => [], registerTool: () => {}, @@ -1813,6 +1830,7 @@ describe('CoreToolScheduler truncated output protection', () => { const mockToolRegistry = { getTool: () => tool, + ensureTool: async () => tool, getAllToolNames: () => toolNames, getFunctionDeclarations: () => [], tools: new Map(), @@ -2004,6 +2022,7 @@ describe('CoreToolScheduler Sequential Execution', () => { const mockToolRegistry = { getTool: () => declarativeTool, + ensureTool: async () => declarativeTool, getToolByName: () => declarativeTool, getFunctionDeclarations: () => [], tools: new Map(), @@ -2126,6 +2145,7 @@ describe('CoreToolScheduler Sequential Execution', () => { const mockToolRegistry = { getTool: () => declarativeTool, + ensureTool: async () => declarativeTool, getToolByName: () => declarativeTool, getFunctionDeclarations: () => [], tools: new Map(), @@ -2302,6 +2322,7 @@ describe('CoreToolScheduler plan mode with ask_user_question', () => { ) { const mockToolRegistry = { getTool: () => tool, + ensureTool: async () => tool, getToolByName: () => tool, getFunctionDeclarations: () => [], tools: new Map(), @@ -3025,6 +3046,7 @@ describe('Fire hook functions integration', () => { ) { const mockToolRegistry = { getTool: (name: string) => tools.get(name), + ensureTool: async (name: string) => tools.get(name), getFunctionDeclarations: () => [], tools, discovery: {}, @@ -3467,6 +3489,7 @@ describe('CoreToolScheduler IDE interaction', () => { const mockToolRegistry = { getTool: () => mockModifiableTool, + ensureTool: async () => mockModifiableTool, getFunctionDeclarations: () => [], tools: new Map(), discovery: {}, diff --git a/packages/core/src/core/coreToolScheduler.ts b/packages/core/src/core/coreToolScheduler.ts index 8269770de..99944fd5d 100644 --- a/packages/core/src/core/coreToolScheduler.ts +++ b/packages/core/src/core/coreToolScheduler.ts @@ -40,7 +40,6 @@ import { ToolCallEvent, InputFormat, Kind, - SkillTool, } from '../index.js'; import type { FunctionResponse, @@ -682,13 +681,18 @@ export class CoreToolScheduler { * Generates error message for unknown tool. Returns early with skill-specific * message if the name matches a skill, otherwise uses Levenshtein suggestions. */ - private getToolNotFoundMessage(unknownToolName: string, topN = 3): string { + private async getToolNotFoundMessage( + unknownToolName: string, + topN = 3, + ): Promise { // Check if the unknown tool name matches an available skill name. // This handles the case where the model tries to invoke a skill as a tool // (e.g., Tool: "pdf" instead of Tool: "Skill" with skill: "pdf") - const skillTool = this.toolRegistry.getTool(ToolNames.SKILL); - if (skillTool instanceof SkillTool) { - const availableSkillNames = skillTool.getAvailableSkillNames(); + const skillTool = await this.toolRegistry.ensureTool(ToolNames.SKILL); + if (skillTool && 'getAvailableSkillNames' in skillTool) { + const availableSkillNames = ( + skillTool as { getAvailableSkillNames(): string[] } + ).getAvailableSkillNames(); if (availableSkillNames.includes(unknownToolName)) { return `"${unknownToolName}" is a skill name, not a tool name. To use this skill, invoke the "${ToolNames.SKILL}" tool with parameter: skill: "${unknownToolName}"`; } @@ -859,10 +863,10 @@ export class CoreToolScheduler { } } - const toolInstance = this.toolRegistry.getTool(reqInfo.name); + const toolInstance = await this.toolRegistry.ensureTool(reqInfo.name); if (!toolInstance) { // Tool is not in registry and not excluded - likely hallucinated or typo - const errorMessage = this.getToolNotFoundMessage(reqInfo.name); + const errorMessage = await this.getToolNotFoundMessage(reqInfo.name); newToolCalls.push({ status: 'error', request: reqInfo, diff --git a/packages/core/src/core/geminiChat.ts b/packages/core/src/core/geminiChat.ts index 51785f198..070acf9f2 100644 --- a/packages/core/src/core/geminiChat.ts +++ b/packages/core/src/core/geminiChat.ts @@ -796,7 +796,9 @@ export class GeminiChat { isSchemaDepthError(error.message) || isInvalidArgumentError(error.message) ) { - const tools = this.config.getToolRegistry().getAllTools(); + const toolRegistry = this.config.getToolRegistry(); + await toolRegistry.warmAll(); + const tools = toolRegistry.getAllTools(); const cyclicSchemaTools: string[] = []; for (const tool of tools) { if ( diff --git a/packages/core/src/core/nonInteractiveToolExecutor.test.ts b/packages/core/src/core/nonInteractiveToolExecutor.test.ts index 9a755ec13..413300fe2 100644 --- a/packages/core/src/core/nonInteractiveToolExecutor.test.ts +++ b/packages/core/src/core/nonInteractiveToolExecutor.test.ts @@ -35,6 +35,7 @@ describe('executeToolCall', () => { mockToolRegistry = { getTool: vi.fn(), + ensureTool: vi.fn(async (name: string) => mockToolRegistry.getTool(name)), getAllToolNames: vi.fn(), } as unknown as ToolRegistry; diff --git a/packages/core/src/followup/speculation.ts b/packages/core/src/followup/speculation.ts index d8450f1b4..f26d7c9cb 100644 --- a/packages/core/src/followup/speculation.ts +++ b/packages/core/src/followup/speculation.ts @@ -297,7 +297,7 @@ async function runSpeculativeLoop( // SECURITY: Only reaches here for read-only tools or writes gated by approvalMode try { const toolRegistry = config.getToolRegistry(); - const tool = toolRegistry.getTool(name); + const tool = await toolRegistry.ensureTool(name); if (!tool) { functionResponses.push({ functionResponse: { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 2ce016b98..b949b2ab3 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -74,32 +74,56 @@ export * from './tools/tool-error.js'; export * from './tools/tool-registry.js'; export * from './tools/tools.js'; -// Individual tools -export * from './tools/edit.js'; -export * from './tools/exitPlanMode.js'; -export * from './tools/glob.js'; -export * from './tools/grep.js'; -export * from './tools/ls.js'; -export * from './tools/lsp.js'; +// Individual tools — MCP/SDK infrastructure only (tool classes are lazy-loaded) export * from './tools/mcp-client.js'; export * from './tools/mcp-client-manager.js'; export * from './tools/mcp-tool.js'; -export * from './memory/const.js'; export * from './tools/read-file.js'; export * from './tools/ripGrep.js'; export * from './tools/sdk-control-client-transport.js'; -export * from './tools/shell.js'; -export * from './tools/skill.js'; -export * from './tools/agent/agent.js'; -export * from './tools/todoWrite.js'; -export * from './tools/tool-error.js'; -export * from './tools/tool-registry.js'; -export * from './tools/web-fetch.js'; -export * from './tools/web-search/index.js'; -export * from './tools/write-file.js'; -export * from './tools/cron-create.js'; -export * from './tools/cron-list.js'; -export * from './tools/cron-delete.js'; +export * from './tools/modifiable-tool.js'; + +// Selective re-exports of types/utilities from tool files (avoids loading full tool modules) +export type { WebSearchProviderConfig } from './tools/web-search/types.js'; +export { buildSkillLlmContent } from './tools/skill-utils.js'; + +// Backward-compatible type re-exports for tool classes removed from eager loading. +// These preserve TypeScript type compatibility for downstream consumers. +// Note: runtime value imports (e.g. `new EditTool(...)`) must use the direct +// module path (e.g. `@qwen-code/qwen-code-core/dist/tools/edit.js`) as these +// classes are now lazy-loaded and are not exported as values from the package root. +export type { EditTool, EditToolParams } from './tools/edit.js'; +export type { + ExitPlanModeTool, + ExitPlanModeParams, +} from './tools/exitPlanMode.js'; +export type { GlobTool, GlobToolParams, GlobPath } from './tools/glob.js'; +export type { GrepTool, GrepToolParams } from './tools/grep.js'; +export type { LSTool, LSToolParams, FileEntry } from './tools/ls.js'; +export type { LspTool, LspToolParams, LspOperation } from './tools/lsp.js'; +export type { + ShellTool, + ShellToolParams, + ShellToolInvocation, +} from './tools/shell.js'; +export type { SkillTool, SkillParams } from './tools/skill.js'; +export type { AgentTool, AgentParams } from './tools/agent/agent.js'; +export type { + TodoWriteTool, + TodoItem, + TodoWriteParams, +} from './tools/todoWrite.js'; +export type { WebFetchTool, WebFetchToolParams } from './tools/web-fetch.js'; +export type { + WebSearchTool, + WebSearchToolParams, + WebSearchToolResult, + WebSearchConfig, +} from './tools/web-search/index.js'; +export type { WriteFileTool, WriteFileToolParams } from './tools/write-file.js'; +export type { CronCreateTool, CronCreateParams } from './tools/cron-create.js'; +export type { CronListTool, CronListParams } from './tools/cron-list.js'; +export type { CronDeleteTool, CronDeleteParams } from './tools/cron-delete.js'; // ============================================================================ // Services diff --git a/packages/core/src/subagents/subagent-manager.test.ts b/packages/core/src/subagents/subagent-manager.test.ts index 82e3c620e..649d50f56 100644 --- a/packages/core/src/subagents/subagent-manager.test.ts +++ b/packages/core/src/subagents/subagent-manager.test.ts @@ -66,6 +66,7 @@ describe('SubagentManager', () => { beforeEach(() => { mockToolRegistry = { + warmAll: vi.fn().mockResolvedValue(undefined), getAllTools: vi.fn().mockReturnValue([ { name: 'read_file', displayName: 'Read File' }, { name: 'write_file', displayName: 'Write File' }, @@ -1296,8 +1297,8 @@ System prompt 3`); describe('Runtime Configuration Methods', () => { describe('convertToRuntimeConfig', () => { - it('should convert basic configuration', () => { - const runtimeConfig = manager.convertToRuntimeConfig(validConfig); + it('should convert basic configuration', async () => { + const runtimeConfig = await manager.convertToRuntimeConfig(validConfig); expect(runtimeConfig.promptConfig.systemPrompt).toBe( validConfig.systemPrompt, @@ -1307,13 +1308,14 @@ System prompt 3`); expect(runtimeConfig.toolConfig).toBeUndefined(); }); - it('should include tool configuration when tools are specified', () => { + it('should include tool configuration when tools are specified', async () => { const configWithTools: SubagentConfig = { ...validConfig, tools: ['read_file', 'write_file'], }; - const runtimeConfig = manager.convertToRuntimeConfig(configWithTools); + const runtimeConfig = + await manager.convertToRuntimeConfig(configWithTools); expect(runtimeConfig.toolConfig).toBeDefined(); expect(runtimeConfig.toolConfig!.tools).toEqual([ @@ -1322,13 +1324,13 @@ System prompt 3`); ]); }); - it('should transform display names to tool names in tool configuration', () => { + it('should transform display names to tool names in tool configuration', async () => { const configWithDisplayNames: SubagentConfig = { ...validConfig, tools: ['Read File', 'write_file', 'Search Files', 'unknown_tool'], }; - const runtimeConfig = manager.convertToRuntimeConfig( + const runtimeConfig = await manager.convertToRuntimeConfig( configWithDisplayNames, ); @@ -1341,26 +1343,27 @@ System prompt 3`); ]); }); - it('should set modelConfig.model from model selector and merge run configurations', () => { + it('should set modelConfig.model from model selector and merge run configurations', async () => { const configWithCustom: SubagentConfig = { ...validConfig, model: 'custom-model', runConfig: { max_time_minutes: 5 }, }; - const runtimeConfig = manager.convertToRuntimeConfig(configWithCustom); + const runtimeConfig = + await manager.convertToRuntimeConfig(configWithCustom); expect(runtimeConfig.modelConfig.model).toBe('custom-model'); expect(runtimeConfig.runConfig.max_time_minutes).toBe(5); }); - it('should accept cross-provider model selectors', () => { + it('should accept cross-provider model selectors', async () => { const configWithCrossProvider: SubagentConfig = { ...validConfig, model: 'openai:gpt-4', }; - const runtimeConfig = manager.convertToRuntimeConfig( + const runtimeConfig = await manager.convertToRuntimeConfig( configWithCrossProvider, ); expect(runtimeConfig.modelConfig.model).toBe('gpt-4'); diff --git a/packages/core/src/subagents/subagent-manager.ts b/packages/core/src/subagents/subagent-manager.ts index dcac1a880..5869b3974 100644 --- a/packages/core/src/subagents/subagent-manager.ts +++ b/packages/core/src/subagents/subagent-manager.ts @@ -636,7 +636,7 @@ export class SubagentManager { }, ): Promise { try { - const runtimeConfig = this.convertToRuntimeConfig(config); + const runtimeConfig = await this.convertToRuntimeConfig(config); // When the model selector specifies a different provider, build a // per-agent Config with a dedicated ContentGenerator so the subagent @@ -723,7 +723,9 @@ export class SubagentManager { * @param config - File-based subagent configuration * @returns Runtime configuration for AgentHeadless */ - convertToRuntimeConfig(config: SubagentConfig): SubagentRuntimeConfig { + async convertToRuntimeConfig( + config: SubagentConfig, + ): Promise { const promptConfig: PromptConfig = { systemPrompt: config.systemPrompt, }; @@ -743,13 +745,13 @@ export class SubagentManager { (config.disallowedTools && config.disallowedTools.length > 0) ) { const toolNames = config.tools - ? this.transformToToolNames(config.tools) + ? await this.transformToToolNames(config.tools) : ['*']; toolConfig = { tools: toolNames, ...(config.disallowedTools && config.disallowedTools.length > 0 ? { - disallowedTools: this.transformToToolNames( + disallowedTools: await this.transformToToolNames( config.disallowedTools, ), } @@ -773,12 +775,13 @@ export class SubagentManager { * @returns Array of tool names * @private */ - private transformToToolNames(tools: string[]): string[] { + private async transformToToolNames(tools: string[]): Promise { const toolRegistry = this.config.getToolRegistry(); if (!toolRegistry) { return tools; } + await toolRegistry.warmAll(); const allTools = toolRegistry.getAllTools(); const result: string[] = []; diff --git a/packages/core/src/telemetry/loggers.test.ts b/packages/core/src/telemetry/loggers.test.ts index 19a3997dc..506c68739 100644 --- a/packages/core/src/telemetry/loggers.test.ts +++ b/packages/core/src/telemetry/loggers.test.ts @@ -16,12 +16,12 @@ import type { } from '../index.js'; import { AuthType, - EditTool, GeminiClient, ToolConfirmationOutcome, ToolErrorType, ToolRegistry, } from '../index.js'; +import { EditTool } from '../tools/edit.js'; import { OutputFormat } from '../output/types.js'; import { EVENT_API_REQUEST, diff --git a/packages/core/src/tools/mcp-tool.test.ts b/packages/core/src/tools/mcp-tool.test.ts index b8b5702e5..0873dfbd3 100644 --- a/packages/core/src/tools/mcp-tool.test.ts +++ b/packages/core/src/tools/mcp-tool.test.ts @@ -1262,12 +1262,12 @@ describe('DiscoveredMCPTool', () => { ); const discoverToolsForServer = vi.fn().mockResolvedValue(undefined); - const getTool = vi.fn().mockReturnValue(newTool); + const ensureTool = vi.fn().mockResolvedValue(newTool); const mockConfig = { isTrustedFolder: () => true, getToolRegistry: () => ({ discoverToolsForServer, - getTool, + ensureTool, }), getTruncateToolOutputThreshold: () => 0, getTruncateToolOutputLines: () => 0, @@ -1309,7 +1309,7 @@ describe('DiscoveredMCPTool', () => { isTrustedFolder: () => true, getToolRegistry: () => ({ discoverToolsForServer, - getTool: vi.fn().mockReturnValue(null), + ensureTool: vi.fn().mockResolvedValue(null), }), }; @@ -1365,7 +1365,7 @@ describe('DiscoveredMCPTool', () => { isTrustedFolder: () => true, getToolRegistry: () => ({ discoverToolsForServer, - getTool: vi.fn().mockReturnValue(secondTool), + ensureTool: vi.fn().mockResolvedValue(secondTool), }), }; @@ -1436,7 +1436,7 @@ describe('DiscoveredMCPTool', () => { isTrustedFolder: () => true, getToolRegistry: () => ({ discoverToolsForServer, - getTool: vi.fn().mockReturnValue(newTool), + ensureTool: vi.fn().mockResolvedValue(newTool), }), getTruncateToolOutputThreshold: () => 0, getTruncateToolOutputLines: () => 0, @@ -1494,7 +1494,7 @@ describe('DiscoveredMCPTool', () => { isTrustedFolder: () => true, getToolRegistry: () => ({ discoverToolsForServer, - getTool: vi.fn().mockReturnValue(newTool), + ensureTool: vi.fn().mockResolvedValue(newTool), }), getTruncateToolOutputThreshold: () => 0, getTruncateToolOutputLines: () => 0, diff --git a/packages/core/src/tools/mcp-tool.ts b/packages/core/src/tools/mcp-tool.ts index d3d18ff66..cb6ba2877 100644 --- a/packages/core/src/tools/mcp-tool.ts +++ b/packages/core/src/tools/mcp-tool.ts @@ -208,7 +208,7 @@ class DiscoveredMCPToolInvocation extends BaseToolInvocation< const toolRegistry = this.cliConfig.getToolRegistry(); await toolRegistry.discoverToolsForServer(this.serverName); - const newTool = toolRegistry.getTool( + const newTool = await toolRegistry.ensureTool( `mcp__${this.serverName}__${this.serverToolName}`, ); if (newTool instanceof DiscoveredMCPTool) { diff --git a/packages/core/src/tools/memory-config.ts b/packages/core/src/tools/memory-config.ts new file mode 100644 index 000000000..5c158df95 --- /dev/null +++ b/packages/core/src/tools/memory-config.ts @@ -0,0 +1,48 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Lightweight configuration for memory/context file naming. + * Extracted from memoryTool.ts to avoid loading the full tool module + * when only the filename configuration is needed. + */ + +export const QWEN_CONFIG_DIR = '.qwen'; +export const DEFAULT_CONTEXT_FILENAME = 'QWEN.md'; +export const AGENT_CONTEXT_FILENAME = 'AGENTS.md'; +export const MEMORY_SECTION_HEADER = '## Qwen Added Memories'; + +// This variable will hold the currently configured filename for context files. +// It defaults to include both QWEN.md and AGENTS.md but can be overridden by setGeminiMdFilename. +// QWEN.md is first to maintain backward compatibility (used by /init command and save_memory tool). +let currentGeminiMdFilename: string | string[] = [ + DEFAULT_CONTEXT_FILENAME, + AGENT_CONTEXT_FILENAME, +]; + +export function setGeminiMdFilename(newFilename: string | string[]): void { + if (Array.isArray(newFilename)) { + if (newFilename.length > 0) { + currentGeminiMdFilename = newFilename.map((name) => name.trim()); + } + } else if (newFilename && newFilename.trim() !== '') { + currentGeminiMdFilename = newFilename.trim(); + } +} + +export function getCurrentGeminiMdFilename(): string { + if (Array.isArray(currentGeminiMdFilename)) { + return currentGeminiMdFilename[0]; + } + return currentGeminiMdFilename; +} + +export function getAllGeminiMdFilenames(): string[] { + if (Array.isArray(currentGeminiMdFilename)) { + return currentGeminiMdFilename; + } + return [currentGeminiMdFilename]; +} diff --git a/packages/core/src/tools/skill-utils.ts b/packages/core/src/tools/skill-utils.ts new file mode 100644 index 000000000..fe94d0fc2 --- /dev/null +++ b/packages/core/src/tools/skill-utils.ts @@ -0,0 +1,14 @@ +/** + * @license + * Copyright 2025 Qwen + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * Builds the LLM-facing content string when a skill body is injected. + * Shared between SkillToolInvocation (runtime) and /context (estimation) + * so that token estimates stay in sync with actual usage. + */ +export function buildSkillLlmContent(baseDir: string, body: string): string { + return `Base directory for this skill: ${baseDir}\nImportant: ALWAYS resolve absolute paths from this base directory when working with skills.\n\n${body}\n`; +} diff --git a/packages/core/src/tools/skill.ts b/packages/core/src/tools/skill.ts index ab47beabe..c4e90ea76 100644 --- a/packages/core/src/tools/skill.ts +++ b/packages/core/src/tools/skill.ts @@ -21,14 +21,9 @@ export interface SkillParams { skill: string; } -/** - * Builds the LLM-facing content string when a skill body is injected. - * Shared between SkillToolInvocation (runtime) and /context (estimation) - * so that token estimates stay in sync with actual usage. - */ -export function buildSkillLlmContent(baseDir: string, body: string): string { - return `Base directory for this skill: ${baseDir}\nImportant: ALWAYS resolve absolute paths from this base directory when working with skills.\n\n${body}\n`; -} +// Re-export for backward compatibility +export { buildSkillLlmContent } from './skill-utils.js'; +import { buildSkillLlmContent } from './skill-utils.js'; /** * Skill tool that enables the model to access skill definitions. diff --git a/packages/core/src/tools/tool-registry.test.ts b/packages/core/src/tools/tool-registry.test.ts index 818a73393..02f412f46 100644 --- a/packages/core/src/tools/tool-registry.test.ts +++ b/packages/core/src/tools/tool-registry.test.ts @@ -184,6 +184,18 @@ describe('ToolRegistry', () => { // Assert that the returned array contains all tool names expect(toolNames).toEqual(['c-tool', 'a-tool', 'b-tool']); }); + + it('should include factory-registered tools that have not yet been loaded', () => { + toolRegistry.registerTool(new MockTool({ name: 'loaded-tool' })); + toolRegistry.registerFactory('lazy-tool', async () => { + throw new Error('should not be called'); + }); + + const names = toolRegistry.getAllToolNames(); + + expect(names).toContain('loaded-tool'); + expect(names).toContain('lazy-tool'); + }); }); describe('getToolsByServer', () => { @@ -429,4 +441,120 @@ describe('ToolRegistry', () => { expect(description).toBe(JSON.stringify(params)); }); }); + + describe('ensureTool concurrency', () => { + it('runs the factory only once when two calls are made concurrently', async () => { + let callCount = 0; + const tool = new MockTool({ name: 'concurrent-tool' }); + toolRegistry.registerFactory('concurrent-tool', async () => { + callCount++; + return tool; + }); + + const [result1, result2] = await Promise.all([ + toolRegistry.ensureTool('concurrent-tool'), + toolRegistry.ensureTool('concurrent-tool'), + ]); + + expect(callCount).toBe(1); + expect(result1).toBe(tool); + expect(result2).toBe(tool); + }); + + it('runs the factory only once when warmAll() and ensureTool() overlap', async () => { + let callCount = 0; + const tool = new MockTool({ name: 'overlap-tool' }); + toolRegistry.registerFactory('overlap-tool', async () => { + callCount++; + return tool; + }); + + const warmPromise = toolRegistry.warmAll(); + const ensurePromise = toolRegistry.ensureTool('overlap-tool'); + await Promise.all([warmPromise, ensurePromise]); + + expect(callCount).toBe(1); + }); + + it('clears the inflight entry on failure so subsequent calls can retry', async () => { + let callCount = 0; + const tool = new MockTool({ name: 'retry-tool' }); + toolRegistry.registerFactory('retry-tool', async () => { + callCount++; + if (callCount === 1) throw new Error('transient failure'); + return tool; + }); + + await expect(toolRegistry.ensureTool('retry-tool')).rejects.toThrow( + 'transient failure', + ); + + // Factory remains in the registry after a failure — the second call retries it. + const result = await toolRegistry.ensureTool('retry-tool'); + expect(result).toBe(tool); + expect(callCount).toBe(2); + }); + }); + + describe('warmAll strict mode', () => { + it('throws when a factory fails and strict is true', async () => { + toolRegistry.registerFactory('bad-tool', async () => { + throw new Error('factory error'); + }); + + await expect(toolRegistry.warmAll({ strict: true })).rejects.toThrow( + 'factory error', + ); + }); + + it('does not throw when a factory fails and strict is false (default)', async () => { + toolRegistry.registerFactory('bad-tool', async () => { + throw new Error('factory error'); + }); + + await expect(toolRegistry.warmAll()).resolves.toBeUndefined(); + }); + + it('still loads successful tools before throwing in strict mode', async () => { + const goodTool = new MockTool({ name: 'good-tool' }); + toolRegistry.registerFactory('good-tool', async () => goodTool); + toolRegistry.registerFactory('bad-tool', async () => { + throw new Error('factory error'); + }); + + await expect(toolRegistry.warmAll({ strict: true })).rejects.toThrow( + 'factory error', + ); + + // The good tool should still have been loaded despite the failure. + expect(await toolRegistry.ensureTool('good-tool')).toBe(goodTool); + }); + }); + + describe('stop', () => { + it('disposes tools that were still inflight when stop() was called', async () => { + let resolveFactory!: (tool: MockTool) => void; + const factoryPromise = new Promise((resolve) => { + resolveFactory = resolve; + }); + + const disposeSpy = vi.fn(); + const tool = new MockTool({ name: 'inflight-tool' }); + (tool as unknown as { dispose: () => void }).dispose = disposeSpy; + + toolRegistry.registerFactory('inflight-tool', () => factoryPromise); + + // Start loading the tool but don't await — it's inflight when stop() is called. + const ensurePromise = toolRegistry.ensureTool('inflight-tool'); + + // Resolve the factory after stop() has started but before it returns. + const stopPromise = toolRegistry.stop(); + resolveFactory(tool); + + await stopPromise; + await ensurePromise; + + expect(disposeSpy).toHaveBeenCalledOnce(); + }); + }); }); diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts index e2110810b..d7e419ef2 100644 --- a/packages/core/src/tools/tool-registry.ts +++ b/packages/core/src/tools/tool-registry.ts @@ -27,6 +27,9 @@ import type { ReadResourceResult } from '@modelcontextprotocol/sdk/types.js'; type ToolParams = Record; +/** Factory function for lazy tool instantiation via dynamic import. */ +export type ToolFactory = () => Promise; + const debugLogger = createDebugLogger('TOOL_REGISTRY'); class DiscoveredToolInvocation extends BaseToolInvocation< @@ -174,6 +177,11 @@ Signal: Signal number or \`(none)\` if no signal was received. export class ToolRegistry { // The tools keyed by tool name as seen by the LLM. private tools: Map = new Map(); + // Lazy tool factories keyed by tool name — resolved on first use. + private factories: Map = new Map(); + // In-flight factory promises — ensures concurrent ensureTool() calls for the + // same name share one promise instead of running the factory multiple times. + private inflight: Map> = new Map(); private config: Config; private mcpClientManager: McpClientManager; @@ -209,13 +217,82 @@ export class ToolRegistry { this.tools.set(tool.name, tool); } + /** + * Registers a lazy tool factory. The tool module is not imported and the tool + * is not instantiated until {@link ensureTool} or {@link warmAll} is called. + */ + registerFactory(name: string, factory: ToolFactory): void { + this.factories.set(name, factory); + } + + /** + * Ensures a specific tool is loaded. Returns the cached instance if already + * loaded, otherwise invokes the factory, caches the result, and returns it. + * Concurrent calls for the same name share a single in-flight promise so the + * factory is never executed more than once. + */ + async ensureTool(name: string): Promise { + const cached = this.tools.get(name); + if (cached) { + // Clean up any stale factory for this name so warmAll() and bulk + // accessors don't treat it as still pending. + this.factories.delete(name); + return cached; + } + + const existing = this.inflight.get(name); + if (existing) return existing; + + const factory = this.factories.get(name); + if (!factory) return undefined; + + const load = factory() + .then((tool) => { + this.tools.set(name, tool); + this.factories.delete(name); + this.inflight.delete(name); + return tool; + }) + .catch((err: unknown) => { + this.inflight.delete(name); + throw err; + }); + + this.inflight.set(name, load); + return load; + } + + /** + * Loads all pending tool factories in parallel. Safe to call multiple times + * (no-op when all factories have been resolved). Call this before any bulk + * access such as {@link getAllTools} or {@link getFunctionDeclarations}. + * + * @param options.strict - When `true`, re-throws the first factory failure + * instead of swallowing it. Use this during startup (e.g. in + * `Config.initialize`) so a broken built-in tool surfaces immediately + * rather than leaving the session partially initialised. + */ + async warmAll(options?: { strict?: boolean }): Promise { + const pending = Array.from(this.factories.keys()); + if (pending.length === 0) return; + const results = await Promise.allSettled( + pending.map((name) => this.ensureTool(name)), + ); + for (const result of results) { + if (result.status === 'rejected') { + if (options?.strict) throw result.reason as Error; + debugLogger.warn('Failed to warm tool factory:', result.reason); + } + } + } + /** * Copies discovered (non-core) tools from another registry into this one. * Used to share MCP/command-discovered tools with per-agent registries * that were built with skipDiscovery. */ copyDiscoveredToolsFrom(source: ToolRegistry): void { - for (const tool of source.getAllTools()) { + for (const tool of source.tools.values()) { if ( (tool instanceof DiscoveredTool || tool instanceof DiscoveredMCPTool) && !this.tools.has(tool.name) @@ -482,8 +559,17 @@ export class ToolRegistry { * Retrieves a filtered list of tool schemas based on a list of tool names. * @param toolNames - An array of tool names to include. * @returns An array of FunctionDeclarations for the specified tools. + * @remarks Requires all tool factories to be resolved first. Call + * {@link warmAll} before invoking this method, otherwise factory-registered + * tools that have not yet been loaded will be silently omitted. */ getFunctionDeclarationsFiltered(toolNames: string[]): FunctionDeclaration[] { + if (this.factories.size > 0) { + debugLogger.warn( + `getFunctionDeclarationsFiltered() called with ${this.factories.size} unloaded ` + + `tool factories. Call warmAll() first to avoid incomplete results.`, + ); + } const declarations: FunctionDeclaration[] = []; for (const name of toolNames) { const tool = this.tools.get(name); @@ -495,16 +581,27 @@ export class ToolRegistry { } /** - * Returns an array of all registered and discovered tool names. + * Returns an array of all registered and discovered tool names, + * including tools that are registered via factory but not yet loaded. */ getAllToolNames(): string[] { - return Array.from(this.tools.keys()); + const names = new Set([...this.tools.keys(), ...this.factories.keys()]); + return Array.from(names); } /** * Returns an array of all registered and discovered tool instances. + * @remarks Requires all tool factories to be resolved first. Call + * {@link warmAll} before invoking this method, otherwise factory-registered + * tools that have not yet been loaded will be absent from the result. */ getAllTools(): AnyDeclarativeTool[] { + if (this.factories.size > 0) { + debugLogger.warn( + `getAllTools() called with ${this.factories.size} unloaded tool factories. ` + + `Call warmAll() first to avoid incomplete results.`, + ); + } return Array.from(this.tools.values()).sort((a, b) => a.displayName.localeCompare(b.displayName), ); @@ -547,6 +644,13 @@ export class ToolRegistry { * This method is idempotent and safe to call multiple times. */ async stop(): Promise { + // Wait for any in-flight factory promises to settle before disposing, so + // that tools which finish loading after stop() is called are still cleaned + // up rather than leaking their listeners and resources. + if (this.inflight.size > 0) { + await Promise.allSettled(this.inflight.values()); + } + for (const tool of this.tools.values()) { if ('dispose' in tool && typeof tool.dispose === 'function') { try {