-
Notifications
You must be signed in to change notification settings - Fork 297
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
added(webviews): Adds protocol based webview loading (CODY-3536) #5354
Conversation
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.
First up, I don't know what the motivation for this change is. I could guess but I won't. If I understood that better I could probably help more.
With that said, on the face of it, let's not do this.
What's the benefit? Agent/client already need to know where Agent's index.js is on disk... so what if the webview resources come from the filesystem or not?
We don't want resource URIs like webview:// because (I assume) CSP won't understand they are secure.
It is another divergence from VSCode.
It has the potential to make JetBrains CODY_DIR development a different now, because in one case the resources are on disk and in this other case, they're...?
It could be integrated at the webview layer with less effect on the extension code. The native webview provider can give a prefix string when it sets capabilities, and the extension can use the VSCode API's URL rewriter to rewrite to it, without the branching added here.
Adds support for linux for Cody Eclipse. Replacement for #5354. There is a bug in the logic for processing the webview index.html file that was allowing the eclipse plugin to work accidentally, because the MacOS and Windows integrated browsers were effectively ignoring the content security policy. This patches that logic and allows a setting for injecting scripts and removing the CSP (required for inline-scripts) and allows skipping converting all embedded links into file:// paths (the eclipse browser cannot handle custom protocols). Also fixes two bugs around Java code generation. ## Test plan Unit tests added.
Adds protocol based webview loading by calling into the extension client via readUriUTF8. This way the extension can dynamically produce the webview assets without them needing to exist in a shared directory on disk.
However this currently only works for the index.html file. The other assets are relativized here and I can't think of a good way to support relativizing those paths to support them same protocol.
My first attempt looked like adding:
but then the extension client would need to implement some behavior to capture those requests triggered by something like
which at least in Eclipse wasn't trivial.
Currently I just leave all relative links in place if the
webviewasset
loader is specified like:and rely on the extension client to be running some sort of asset server that will capture those requests but that's a sneaky bit of undocumented dependency.
Test plan
Ran with local Eclipse build of this branch after deleting all on-disk webviews and confirmed it was served.
Changelog
Allows dynamic serving of webview assets from non-VSCode clients