-
Notifications
You must be signed in to change notification settings - Fork 37k
check the end part of legacyToolReferenceFullNames when evaluating EligibleForAutoApproval
#283887
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR enhances the EligibleForAutoApproval evaluation logic to support backward compatibility for namespaced legacy tool names. When a tool was previously namespaced (e.g., gitTools/gitCommit) and is now renamed or moved, users can configure eligibility using either the full legacy name or just the last segment (e.g., gitCommit).
Key Changes
- Added logic to extract and check the last segment of namespaced legacy tool names when evaluating eligibility for auto-approval
- Replaced hardcoded string literals with the
ChatConfiguration.EligibleForAutoApprovalconstant in tests - Added comprehensive test coverage for namespaced legacy tool name scenarios (both eligible and ineligible cases)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/vs/workbench/contrib/chat/browser/languageModelToolsService.ts |
Enhanced isToolEligibleForAutoApproval to check both full legacy names and their trimmed segments (after the last /) when evaluating eligibility configuration |
src/vs/workbench/contrib/chat/test/browser/languageModelToolsService.test.ts |
Replaced hardcoded configuration strings with constant; added four new tests covering namespaced legacy tool name scenarios for both eligible and ineligible cases |
src/vs/workbench/contrib/chat/test/browser/languageModelToolsService.test.ts
Show resolved
Hide resolved
src/vs/workbench/contrib/chat/test/browser/languageModelToolsService.test.ts
Show resolved
Hide resolved
…`EligibleForAutoApproval` (#283887) * check the end part of legacyToolReferenceFullNames when evaluating EligibleForAutoApproval * improve * tests
ref https://github.com/microsoft/vscode-internalbacklog/issues/6465