-
Notifications
You must be signed in to change notification settings - Fork 36.2k
fetch: auto-approve mentioned urls #276987
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
|
|
||
| const invalid = [...Array.from(invalidUris), ...additionalInvalidUrls]; | ||
| const urlsNeedingConfirmation = [...webUris.values(), ...validFileUris]; | ||
| const urlsNeedingConfirmation = new ResourceSet([...webUris.values(), ...validFileUris]); |
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.
Should this pass some comparer to be case-insensitive?
70ce322 to
9c66c5c
Compare
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 implements auto-approval for URLs that users have mentioned in their chat messages, reducing unnecessary confirmation prompts when the fetch tool is invoked for URLs the user has already referenced.
Key changes:
- Changed
urlsNeedingConfirmationfrom an array to aResourceSetfor better URI handling - Added auto-approval logic that checks if URLs were mentioned in user messages
- Updated all array operations to work with the new
ResourceSetstructure usingIterableutilities
| const tool = new FetchWebPageTool( | ||
| new TestWebContentExtractorService(new ResourceMap<string>()), // Empty - all web requests fail | ||
| new ExtendedTestFileService(new ResourceMap<string | VSBuffer>()), // Empty - all file , | ||
| new MockTrustedDomainService([]), | ||
| new MockChatService(), |
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.
Lots of repetition, could TestInstantiationService help here?
Closes #276955