fix(client): keep tool metadata intact when a listTools() refresh fails (v1.x) - #2618
Open
harshasiddartha wants to merge 1 commit into
Open
Conversation
cacheToolMetadata() cleared the output-validator and task-support caches before compiling the replacement catalog. If a later output schema failed to compile, listTools() rejected but the client was left with empty or partially replaced metadata, so subsequent callTool() invocations skipped output validation and lost task-support information. Build the replacement Map/Set locally and publish them only once every tool has been processed, so a failed refresh leaves the metadata from the last successful listTools() in effect.
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2614.
What's broken
cacheToolMetadata()clears_cachedToolOutputValidators,_cachedKnownTaskToolsand_cachedRequiredTaskToolsbefore it starts compiling the replacement catalog. If a later tool'soutputSchemafails to compile,listTools()rejects as expected — but the caches have already been wiped or partially overwritten.The client is then left in a worse state than before the refresh:
callTool()finds no validator for tools that previously had one and silently skips output validation, and tools markedtaskSupport: "required"are no longer recognised as such.Change
Build the replacement
Map/Setlocally and assign them only after every tool has been processed. A failed refresh now leaves the metadata from the last successfullistTools()in effect.Test
Added a regression test to
test/client/index.test.tsunderoutputSchema validation: it lists a catalog containing a tool with an output schema and ataskSupport: "required"tool, then serves a second catalog whose output schema fails to compile, and asserts both pieces of metadata are still enforced afterwards.Without the source change it fails with:
npm test(1640 tests),npm run typecheck,npm run lintandnpm run buildall pass.Scope
This is deliberately limited to the failure-atomicity of the cache write. The separate validator-lookup ordering issue in #2612 (
callTool()resolving the validator after awaiting the response) is untouched — the two changes are in different methods and don't conflict.Targeting
v1.xsince the reported behaviour is in the v1 client;mainno longer has this code path.