cody: add callback route and URI handler for vscode-insider#53313
Conversation
| // Example: 'vscode-insider://' for VS Code Insider | ||
| if (clientURLScheme && nextRequester.customURLScheme === 'vscode') { | ||
| const redirectURL = new URL(nextRequester.redirectURL) | ||
| redirectURL.protocol = clientURLScheme |
There was a problem hiding this comment.
This might be a security vulnerability vector, being a query param vs being hardcoded into the application. I'm just not sure if there's malicious URL schemes out there in the OS that people could take advantage of, but wouldn't be surprised.
We probably want to use specific hardcoded URL, like redirectURL, but for insiders. We could add a new CODY_INSIDERS entry, or condition it on the client param you added?
There was a problem hiding this comment.
@vdavid is looking into Jetbrains URL schemes now… there's 15 or so of them, and might need some conditional logic too. Perhaps there'll be some conditional logic there too, instead of a straight lookup table?
There was a problem hiding this comment.
This could be an issue because every time we add a new entry to the whitelist, the new entry will not work until the next release.
I guess my question for now is, do we only want to support vscode and vscode-insider only? (and ignore codespaces and remote hosts)
There was a problem hiding this comment.
Just added a new entry for vscode-insider.
If we want to support other clients we will need to open another PR for those
dominiccooney
left a comment
There was a problem hiding this comment.
Code looks good.
Let's write out the test plan in text and not rely on following a Loom alone. At the very least you want to see this working in vscode and vscode insiders.
Would be great to add an automated test so we can detect regressions automatically.
Feedback inline about naming.
Close https://github.com/sourcegraph/sourcegraph/issues/53312
This PR adds support to handle callback and redirections from VS Code Insider.
Ref: https://code.visualstudio.com/api/advanced-topics/remote-extensions#callbacks-and-uri-handlers
Test plan
See attached video as demo:
https://www.loom.com/share/cb1a6bc634ad4d44b5c1a719baaf2277