Folder added to workspace displays '!' warning in Explorer if folder is hosted by a FileSystemProvider implemented in an extension that is not yet activated

[gjsjohnmurray]

Issue Type: Bug

1. Install the MemFS extension from Marketplace, authored by @jrieken
2. Create an empty .code-workspace file with this content:

{
	"folders": [],
	"settings": {}
}

3. Open it as a workspace.
4. Edit the workspace file to add one folder that uses the memfs scheme with the MemFS extension implements.

{
	"folders": [
		{
			"name": "MemFS - Manually added",
			"uri": "memfs:/"
		}
	],
	"settings": {}
}

5. After saving the updated workspace file, observe that Explorer shows the added folder but decorates it with a warning exclamation point. Hover over the folder label to see the message \ ● Unable to resolve workspace folder

I think the problem occurs because even though the MemFS extension specifies "onFileSystem:memfs" as one of its activation events, when VS Code adds the new folder to the currently-open (though empty) workspace it apparently doesn't activate the extension in time before going looking for a .vscode folder within the new memfs root.

By running Developer: Show Running Extensions before doing this we can see that MemFS does get launched. I guess this just doesn't happen soon enough.

By contrast, if the memfs root is defined in the workspace at the point where the .code-workspace file is loaded the extension activates early enough and no warning indicator appears. This can be observed by running the Developer: Reload Window command. The memfs:/ root now displays normally.

VS Code version: Code - Insiders 1.56.0-insider (0310f02, 2021-04-23T05:13:46.805Z)
OS version: Windows_NT x64 10.0.19042

[isidorn]

@gjsjohnmurray I see you are already deep down investigating this. If you have a good proposal for a PR how to fix this I would be open to review.
Thanks a lot for letting us know about this.

[gjsjohnmurray]

@isidorn is it true that the extension host won't be running at the time that this gets called?

vscode/src/vs/workbench/services/configuration/browser/configurationService.ts

Lines 382 to 403 in 66f0c29

	/**
	* At present, all workspaces (empty, single-folder, multi-root) in local and remote
	* can be initialized without requiring extension host except following case:
	*
	* A multi root workspace with .code-workspace file that has to be resolved by an extension.
	* Because of readonly `rootPath` property in extension API we have to resolve multi root workspace
	* before extension host starts so that `rootPath` can be set to first folder.
	*
	* This restriction is lifted partially for web in `MainThreadWorkspace`.
	* In web, we start extension host with empty `rootPath` in this case.
	*
	* Related root path issue discussion is being tracked here - https://github.com/microsoft/vscode/issues/69335
	*/
	async initialize(arg: IWorkspaceInitializationPayload): Promise<void> {
	mark('code/willInitWorkspaceService');
	const workspace = await this.createWorkspace(arg);
	await this.updateWorkspaceAndInitializeConfiguration(workspace);
	this.checkAndMarkWorkspaceComplete(false);
	mark('code/didInitWorkspaceService');
	}

The comments suggest that's true, in which case the line 400 call to checkAndMarkWorkspaceComplete goes via toValidWorkspaceFolders into FileService. Then resolve -> doResolveFile -> withProvider -> activateProvider which I think would normally be where the onFileSystem extension activation event would get the FSP extension activated. But if there's no EH then I assume this fails, so withProvider throws.

[isidorn]

@sandy081 might be better to answer this.

[gjsjohnmurray]

@sandy081 I began trying to enhance this in WorkspaceService:

vscode/src/vs/workbench/services/configuration/browser/configurationService.ts

Lines 823 to 839 in 8fa0540

	// Filter out workspace folders which are files (not directories)
	// Workspace folders those cannot be resolved are not filtered because they are handled by the Explorer.
	private async toValidWorkspaceFolders(workspaceFolders: WorkspaceFolder[]): Promise<WorkspaceFolder[]> {
	const validWorkspaceFolders: WorkspaceFolder[] = [];
	for (const workspaceFolder of workspaceFolders) {
	try {
	const result = await this.fileService.resolve(workspaceFolder.uri);
	if (!result.isDirectory) {
	continue;
	}
	} catch (e) {
	this.logService.warn(`Ignoring the error while validating workspace folder ${workspaceFolder.uri.toString()} - ${toErrorMessage(e)}`);
	}
	validWorkspaceFolders.push(workspaceFolder);
	}
	return validWorkspaceFolders;
	}

My idea was to gather the failures during the first (currently, the only) pass, then add a second pass which begins by awaiting whenInstalledExtensionsRegistered in the extension service. But the workspace service doesn't currently have access to the extension service, right?

[sandy081]

@gjsjohnmurray I do not think above validation is causing the warning in the explorer. Above validation is only to check if the entry is a directory or not.

The warning in explorer is shown when it cannot resolve the workspace folder. I think following events are happening in this case

• A new folder is added to workspace file as first folder
• Workspace service triggers change event about this and there are following listeners which are prominent in this scenario
  • Extension host is restarted as first folder is changed
  • Explorer shows the folder

I think explorer is not recovering from extension host restart and hence warning is shown even after ext host is restarted. @isidorn Please check and confirm.

[gjsjohnmurray]

@sandy081 it's not the !result.isDirectory which is triggering. Rather, the resolve on line 829 is throwing, and the catch merely writes it to the console log and continues.

[sandy081]

Rather, the resolve on line 829 is throwing, and the catch merely writes it to the console log and continues.

Yeah that is intended and I think this is nothing to do with the warning in the explorer.

[isidorn]

@sandy081 ok, so it seems like the explorer needs a fresh start on et host restart... Thanks for investigating

[isidorn]

This should behave a bit better now with our aggressive explorer refresh when window gains focus again or explorer gets visible.
Closing as no other action planned for now