Don't try watching readonly file systems

[mjbvz]

For #313281

[Copilot]

Pull request overview

Updates the Copilot extension’s external-ingest workspace indexer to avoid registering file watchers on read-only file systems (relevant for extension-contributed/virtual file systems per #313281).

Changes:

• Skip watcher registration for workspace folders whose filesystem scheme is reported as non-writable.
• Wrap watcher creation in a try/catch and log a warning instead of failing initialization.

Show a summary per file

File	Description
extensions/copilot/src/platform/workspaceChunkSearch/node/codeSearch/externalIngestIndex.ts	Avoid creating watchers for non-writable schemes and handle watcher creation failures gracefully.

Copilot's findings

Comments suppressed due to low confidence (2)

extensions/copilot/src/platform/workspaceChunkSearch/node/codeSearch/externalIngestIndex.ts:974

• DisposableStore is declared inside the try block, so if createFileSystemWatcher(...) (or any later disposables.add(...)) throws, there’s no way to dispose anything that may already have been added to the store. Consider creating the DisposableStore before the try, and in the catch ensure it’s disposed before returning Disposable.None to avoid leaks / partially-initialized watchers lingering.

			try {
				const disposables = new DisposableStore();

				const watcher = disposables.add(this._fileSystemService.createFileSystemWatcher(new RelativePattern(folder, '**/*')));
				disposables.add(watcher.onDidCreate(uri => this.onFileAdded(uri)));
				disposables.add(watcher.onDidChange(uri => this.onFileChanged(uri)));
				disposables.add(watcher.onDidDelete(uri => this.onFileDeleted(uri)));

				return disposables;
			} catch (e) {
				this._logService.warn(`ExternalIngestIndex::registerWatcher() Failed to create watcher for ${folder.toString()}`, e);
				return Disposable.None;
			}

extensions/copilot/src/platform/workspaceChunkSearch/node/codeSearch/externalIngestIndex.ts:960

• This calls this._fileSystemService.isWritableFileSystem(...) before the try/catch. In the existing unit tests, IFileSystemService is provided by a lightweight mock that does not implement isWritableFileSystem at runtime, so registerWatcher() will throw a TypeError during initialize()/doIngest(). Either update the test double to implement isWritableFileSystem, or defensively handle a missing function here (e.g. check typeof ... === 'function') so tests and other custom implementations don’t crash.

			if (this._fileSystemService.isWritableFileSystem(folder.scheme) === false) {
				return Disposable.None;
			}

• Files reviewed: 1/1 changed files
• Comments generated: 1

[Copilot]

New control flow is introduced here (skip watcher registration for non-writable schemes and swallow watcher-creation errors). There are already dedicated ExternalIngestIndex tests, but none currently exercise this branch (e.g. verifying no watcher is created/log spam avoided for read-only schemes, and that a thrown createFileSystemWatcher is handled). Please add/update unit tests to cover these cases so the behavior stays stable across refactors.

This issue also appears in the following locations of the same file:

• line 958
• line 962