microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
162.37k stars 28.61k forks source link

Configuration defaults passed via workbench options don't work #224236

Open connor4312 opened 1 month ago

connor4312 commented 1 month ago
  1. Register some configuration defaults like { hello: "world" } using the web API (e.g. here for the code-server.sh script)
  2. 🐛 notice they are still undefined when read from the local extension host (at least before a remote is resolved)

It seems like it should work because the overrides are registered here

https://github.com/microsoft/vscode/blob/25dd3cf86fb66e1e3dab023458b3cf2d0d55c69c/src/vs/workbench/services/configuration/browser/configuration.ts#L48-L50

before the MainThreadConfiguration is even instantiated, so I would expect they get passed during initial hydration here

https://github.com/microsoft/vscode/blob/25dd3cf86fb66e1e3dab023458b3cf2d0d55c69c/src/vs/workbench/api/browser/mainThreadConfiguration.ts#L29

...but they do not. I did some debugging myself but got a bit lost in config land.

Having these options is needed for a couple partner team feature asks.

sandy081 commented 3 weeks ago

Assigning to August for investigation.

sandy081 commented 2 weeks ago

I changed the default of workbench.colorTheme setting and it works (github.dev also uses the same). You should have the setting registered for the default to be applied. I suspect the setting is not registered in your scenario?

connor4312 commented 1 week ago

The setting is registered. Repro is a little annoying since it requires an extension doing the resolution of the remote.

  1. Run vscode.dev locally so that you can debug things (npm install && npm run build && node out/server/main.js)
  2. Clone https://github.com/microsoft/vscode-remote-tunnels. git checkout connor4312/preference-auth-provider && npm install && npm run watch in one terminal, and cd extension && npx serve -p 3001 --cors in another
  3. With devtools open, go to http://localhost:3000/+aHR0cDovL2xvY2FsaG9zdDozMDAx/some-tunnel-here (first segment is base64-encoded localhost:3001; the tunnel does not actually need to be hosted.) You can add ?vscode-quality=dev to that URL to use your local vscode build found in a sibling folder to vscode-dev.
  4. Notice that even though we set a default value for remote.tunnels.useAuthProvider, it's undefined when you hit the debugger statement.
sandy081 commented 1 week ago

Thanks for the steps. I can confirm that the setting is not yet registered at the time when you are accessing this setting in your extension. Your extension (remote resolver) is accessing this setting while resolving the authority and I suspect, contribution points of this extension are not read/handled yet.

@alexdima Can you please confirm if this is true.

alexdima commented 6 days ago

Yes, this is an old issue since we first did remotes https://github.com/microsoft/vscode/issues/88044 . Extension points are not handled until after the resolver resolves. In code:

There are some historical reasons behind it, I think back then we did not support disabling extensions without a window reload, so the extension points themselves were not ready for an extension point removal. It also did not seem to be so bad of an issue because the only affected extensions would be our resolver extensions and they mostly suffered when reading their own default configs, which they should normally know since they contribute their extensions. But it looks like you have a new pattern where an embedder is driving the resolver's default config value, which makes the initial issue more severe.

I don't know if a workaround can be made in the configuration service to be able to give values for settings that don't have their schema registered @sandy081 or if we now must handle at least the resolver's extension points before resolving. If it's the latter, then @connor4312 maybe you can look into a PR. I think something must be added before invoking the resolver here which would potentially call _doHandleExtensionPoints. But be aware that it would not be good to handle the extension points of all locally available extensions since many of them will be available remotely and that would just cause a lot of churn. Now that I write this, I remember the second reason why I didn't tackle this. Some extension point handlers use the fact that they are being called as a way to think of themselves as being "ready"/"finished", which in this situation is clearly not the case. I think the debugger was one of them, but maybe others, I don't remember.

sandy081 commented 6 days ago

if a workaround can be made in the configuration service to be able to give values for settings that don't have their schema registered

I feel this allows misusing of the API for any random property.

How about handling only configuration contribution of a resolver extension? Or as a generic approach, let the contribution point define if it is ready for handling a contribution point during resolving phase?