Fix fetch tool to fallback to file service for non-http schemes (microsoft#252683)

* Initial plan

* Fix fetch tool to only handle HTTP/HTTPS schemes

This prevents MCP resource URIs (like test://static/resource/50) from being
incorrectly processed by the fetch tool. MCP resources should be handled
by VS Code's filesystem providers instead.

- Modified FetchWebPageTool._parseUris() to filter out non-HTTP(S) schemes
- Added comprehensive tests for the new scheme filtering behavior
- Non-HTTP(S) URIs will now be treated as invalid and skipped

Co-authored-by: TylerLeonhardt <[email protected]>

* Refactor tests to use TestWebContentExtractorService instead of testing private methods

Co-authored-by: TylerLeonhardt <[email protected]>

* Enhance fetch tool to serve file content via IFileService for non-HTTP schemes

Co-authored-by: connor4312 <[email protected]>

* Revert tool description to original text as requested

Co-authored-by: TylerLeonhardt <[email protected]>

* Implement FileService-based WebContentExtractorService for web platform

Co-authored-by: TylerLeonhardt <[email protected]>

* Handle binary data

* style

* don't adopt this yet

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: TylerLeonhardt <[email protected]>
Co-authored-by: connor4312 <[email protected]>
Co-authored-by: Tyler Leonhardt <[email protected]>

Author: 3 people

src/vs/workbench/contrib/chat/electron-browser/tools/fetchPageTool.ts

@@ -8,9 +8,11 @@ import { MarkdownString } from '../../../../../base/common/htmlContent.js';
 import { ResourceSet } from '../../../../../base/common/map.js';
 import { URI } from '../../../../../base/common/uri.js';
 import { localize } from '../../../../../nls.js';
+import { IFileService } from '../../../../../platform/files/common/files.js';
 import { IWebContentExtractorService } from '../../../../../platform/webContentExtractor/common/webContentExtractor.js';
 import { CountTokensCallback, IPreparedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolResult, IToolResultTextPart, ToolDataSource, ToolProgress } from '../../common/languageModelToolsService.js';
 import { InternalFetchWebPageToolId } from '../../common/tools/tools.js';
+import { detectEncodingFromBuffer } from '../../../../services/textfile/common/encoding.js';
 
 export const FetchWebPageToolData: IToolData = {
 	id: InternalFetchWebPageToolId,
@@ -38,78 +40,125 @@ export class FetchWebPageTool implements IToolImpl {
 
 	constructor(
 		@IWebContentExtractorService private readonly _readerModeService: IWebContentExtractorService,
+		@IFileService private readonly _fileService: IFileService,
 	) { }
 
-	async invoke(invocation: IToolInvocation, _countTokens: CountTokensCallback, _progress: ToolProgress, _token: CancellationToken): Promise<IToolResult> {
-		const parsedUriResults = this._parseUris((invocation.parameters as { urls?: string[] }).urls);
-		const validUris = Array.from(parsedUriResults.values()).filter((uri): uri is URI => !!uri);
-		if (!validUris.length) {
+	async invoke(invocation: IToolInvocation, _countTokens: CountTokensCallback, _progress: ToolProgress, token: CancellationToken): Promise<IToolResult> {
+		const urls = (invocation.parameters as { urls?: string[] }).urls || [];
+		const { webUris, fileUris, invalidUris } = this._parseUris(urls);
+		const allValidUris = [...webUris.values(), ...fileUris.values()];
+
+		if (!allValidUris.length && invalidUris.size === 0) {
 			return {
 				content: [{ kind: 'text', value: localize('fetchWebPage.noValidUrls', 'No valid URLs provided.') }]
 			};
 		}
 
 		// We approved these via confirmation, so mark them as "approved" in this session
 		// if they are not approved via the trusted domain service.
-		for (const uri of validUris) {
+		for (const uri of webUris.values()) {
 			this._alreadyApprovedDomains.add(uri);
 		}
 
-		const contents = await this._readerModeService.extract(validUris);
-		// Make an array that contains either the content or undefined for invalid URLs
-		const contentsWithUndefined: (string | undefined)[] = [];
-		let indexInContents = 0;
-		parsedUriResults.forEach((uri) => {
-			if (uri) {
-				contentsWithUndefined.push(contents[indexInContents]);
-				indexInContents++;
+		// Get contents from web URIs
+		const webContents = webUris.size > 0 ? await this._readerModeService.extract([...webUris.values()]) : [];
+
+		// Get contents from file URIs
+		const fileContents: (string | undefined)[] = [];
+		const successfulFileUris: URI[] = [];
+		for (const uri of fileUris.values()) {
+			try {
+				const fileContent = await this._fileService.readFile(uri, undefined, token);
+
+				// Check if the content is binary
+				const detected = detectEncodingFromBuffer({ buffer: fileContent.value, bytesRead: fileContent.value.byteLength });
+
+				if (detected.seemsBinary) {
+					// For binary files, return a message indicating they're not supported
+					// We do this for now until the tools that leverage this internal tool can support binary content
+					fileContents.push(localize('fetchWebPage.binaryNotSupported', 'Binary files are not supported at the moment.'));
+				} else {
+					// For text files, convert to string
+					fileContents.push(fileContent.value.toString());
+				}
+
+				successfulFileUris.push(uri);
+			} catch (error) {
+				// If file service can't read it, treat as invalid
+				fileContents.push(undefined);
+			}
+		}
+
+		// Build results array in original order
+		const results: (string | undefined)[] = [];
+		let webIndex = 0;
+		let fileIndex = 0;
+		for (const url of urls) {
+			if (invalidUris.has(url)) {
+				results.push(undefined);
+			} else if (webUris.has(url)) {
+				results.push(webContents[webIndex]);
+				webIndex++;
+			} else if (fileUris.has(url)) {
+				results.push(fileContents[fileIndex]);
+				fileIndex++;
 			} else {
-				contentsWithUndefined.push(undefined);
+				results.push(undefined);
 			}
-		});
+		}
+
+		// Only include URIs that actually had content successfully fetched
+		const actuallyValidUris = [...webUris.values(), ...successfulFileUris];
 
 		return {
-			content: this._getPromptPartsForResults(contentsWithUndefined),
+			content: this._getPromptPartsForResults(results),
 			// Have multiple results show in the dropdown
-			toolResultDetails: validUris.length > 1 ? validUris : undefined
+			toolResultDetails: actuallyValidUris.length > 1 ? actuallyValidUris : undefined
 		};
 	}
 
 	async prepareToolInvocation(context: IToolInvocationPreparationContext, token: CancellationToken): Promise<IPreparedToolInvocation | undefined> {
-		const map = this._parseUris(context.parameters.urls);
-		const invalid = new Array<string>();
-		const valid = new Array<URI>();
-		map.forEach((uri, url) => {
-			if (!uri) {
-				invalid.push(url);
-			} else {
-				valid.push(uri);
+		const { webUris, fileUris, invalidUris } = this._parseUris(context.parameters.urls);
+
+		// Check which file URIs can actually be read
+		const validFileUris: URI[] = [];
+		const additionalInvalidUrls: string[] = [];
+		for (const [originalUrl, uri] of fileUris.entries()) {
+			try {
+				await this._fileService.stat(uri);
+				validFileUris.push(uri);
+			} catch (error) {
+				// If file service can't stat it, treat as invalid
+				additionalInvalidUrls.push(originalUrl);
 			}
-		});
-		const urlsNeedingConfirmation = valid.filter(url => !this._alreadyApprovedDomains.has(url));
+		}
+
+		const invalid = [...Array.from(invalidUris), ...additionalInvalidUrls];
+		const valid = [...webUris.values(), ...validFileUris];
+		const urlsNeedingConfirmation = webUris.size > 0 ? [...webUris.values()].filter(url => !this._alreadyApprovedDomains.has(url)) : [];
 
 		const pastTenseMessage = invalid.length
 			? invalid.length > 1
 				// If there are multiple invalid URLs, show them all
 				? new MarkdownString(
 					localize(
 						'fetchWebPage.pastTenseMessage.plural',
-						'Fetched {0} web pages, but the following were invalid URLs:\n\n{1}\n\n', valid.length, invalid.map(url => `- ${url}`).join('\n')
+						'Fetched {0} resources, but the following were invalid URLs:\n\n{1}\n\n', valid.length, invalid.map(url => `- ${url}`).join('\n')
 					))
 				// If there is only one invalid URL, show it
 				: new MarkdownString(
 					localize(
 						'fetchWebPage.pastTenseMessage.singular',
-						'Fetched web page, but the following was an invalid URL:\n\n{0}\n\n', invalid[0]
+						'Fetched resource, but the following was an invalid URL:\n\n{0}\n\n', invalid[0]
 					))
 			// No invalid URLs
 			: new MarkdownString();
 
 		const invocationMessage = new MarkdownString();
 		if (valid.length > 1) {
-			pastTenseMessage.appendMarkdown(localize('fetchWebPage.pastTenseMessageResult.plural', 'Fetched {0} web pages', valid.length));
-			invocationMessage.appendMarkdown(localize('fetchWebPage.invocationMessage.plural', 'Fetching {0} web pages', valid.length));
-		} else {
+			pastTenseMessage.appendMarkdown(localize('fetchWebPage.pastTenseMessageResult.plural', 'Fetched {0} resources', valid.length));
+			invocationMessage.appendMarkdown(localize('fetchWebPage.invocationMessage.plural', 'Fetching {0} resources', valid.length));
+		} else if (valid.length === 1) {
 			const url = valid[0].toString();
 			// If the URL is too long, show it as a link... otherwise, show it as plain text
 			if (url.length > 400) {
@@ -119,14 +168,14 @@ export class FetchWebPageTool implements IToolImpl {
 						// Make sure the link syntax is correct
 						'{Locked="]({0})"}',
 					]
-				}, 'Fetched [web page]({0})', url));
+				}, 'Fetched [resource]({0})', url));
 				invocationMessage.appendMarkdown(localize({
 					key: 'fetchWebPage.invocationMessage.singularAsLink',
 					comment: [
 						// Make sure the link syntax is correct
 						'{Locked="]({0})"}',
 					]
-				}, 'Fetching [web page]({0})', url));
+				}, 'Fetching [resource]({0})', url));
 			} else {
 				pastTenseMessage.appendMarkdown(localize('fetchWebPage.pastTenseMessageResult.singular', 'Fetched {0}', url));
 				invocationMessage.appendMarkdown(localize('fetchWebPage.invocationMessage.singular', 'Fetching {0}', url));
@@ -157,17 +206,26 @@ export class FetchWebPageTool implements IToolImpl {
 		return result;
 	}
 
-	private _parseUris(urls?: string[]): Map<string, URI | undefined> {
-		const results = new Map<string, URI | undefined>();
-		urls?.forEach(uri => {
+	private _parseUris(urls?: string[]): { webUris: Map<string, URI>; fileUris: Map<string, URI>; invalidUris: Set<string> } {
+		const webUris = new Map<string, URI>();
+		const fileUris = new Map<string, URI>();
+		const invalidUris = new Set<string>();
+
+		urls?.forEach(url => {
 			try {
-				const uriObj = URI.parse(uri);
-				results.set(uri, uriObj);
+				const uriObj = URI.parse(url);
+				if (uriObj.scheme === 'http' || uriObj.scheme === 'https') {
+					webUris.set(url, uriObj);
+				} else {
+					// Try to handle other schemes via file service
+					fileUris.set(url, uriObj);
+				}
 			} catch (e) {
-				results.set(uri, undefined);
+				invalidUris.add(url);
 			}
 		});
-		return results;
+
+		return { webUris, fileUris, invalidUris };
 	}
 
 	private _getPromptPartsForResults(results: (string | undefined)[]): IToolResultTextPart[] {