Agent: support unstable-codegen access via SOCKS proxy #836

cbart · 2023-08-28T09:33:57Z

Add cody.proxy that makes agent connect with a SOCKS proxy for unstable-codegen.

This change paired with sourcegraph/sourcegraph#56264 routes autocomplete LLM traffic through a SOCKS proxy by extending ~/.sourcegraph-jetbrains.properties :

cody.proxy: socks5://127.0.0.1:9999

For VSCode the setting is not shown in the UI, not suggested as an autocomplete option in settings.json, and when typed manually, the deprecation message is shown (#836 (comment)).

Test plan

Manually tested with IntelliJ + agent
Manually tested with VS Code (using http.proxy as described by the config description)

Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>

cbart · 2023-08-28T11:14:30Z

I think I might be a little confused with dependency management here. Do I need to (and if so how do I) add transitive (?) dependencies of newly added socks-proxy?

➜  cody git:(cb/s/56254) pnpm -C vscode run build

> cody-ai@0.8.0 build /Users/cbart/cody/vscode
> tsc --build && pnpm run -s _build:esbuild:desktop && pnpm run -s _build:esbuild:web && pnpm run -s _build:webviews --mode production


  dist/extension.node.js      6.1mb ⚠️
  dist/extension.node.js.map  9.8mb

⚡ Done in 160ms
✘ [ERROR] Could not resolve "dns"

    ../node_modules/.pnpm/socks-proxy-agent@8.0.1/node_modules/socks-proxy-agent/dist/index.js:33:33:
      33 │ const dns = __importStar(require("dns"));
         ╵                                  ~~~~~

  The package "dns" wasn't found on the file system but is built into node. Are you trying to bundle
  for node? You can use "--platform=node" to do that, which will remove this error.

✘ [ERROR] Could not resolve "net"

    ../node_modules/.pnpm/socks-proxy-agent@8.0.1/node_modules/socks-proxy-agent/dist/index.js:34:33:
      34 │ const net = __importStar(require("net"));
         ╵                                  ~~~~~

  The package "net" wasn't found on the file system but is built into node. Are you trying to bundle
  for node? You can use "--platform=node" to do that, which will remove this error.

✘ [ERROR] Could not resolve "tls"

    ../node_modules/.pnpm/socks-proxy-agent@8.0.1/node_modules/socks-proxy-agent/dist/index.js:35:33:
      35 │ const tls = __importStar(require("tls"));
         ╵                                  ~~~~~

  The package "tls" wasn't found on the file system but is built into node. Are you trying to bundle
  for node? You can use "--platform=node" to do that, which will remove this error.

✘ [ERROR] Could not resolve "http"

    ../node_modules/.pnpm/agent-base@7.1.0/node_modules/agent-base/dist/index.js:30:34:
      30 │ const http = __importStar(require("http"));
         ╵                                   ~~~~~~

  The package "http" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "--platform=node" to do that, which will remove this error.

✘ [ERROR] Could not resolve "http"

    ../node_modules/.pnpm/agent-base@7.1.0/node_modules/agent-base/dist/helpers.js:27:34:
      27 │ const http = __importStar(require("http"));
         ╵                                   ~~~~~~

  The package "http" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "--platform=node" to do that, which will remove this error.

✘ [ERROR] Could not resolve "https"

    ../node_modules/.pnpm/agent-base@7.1.0/node_modules/agent-base/dist/helpers.js:28:35:
      28 │ const https = __importStar(require("https"));
         ╵                                    ~~~~~~~

  The package "https" wasn't found on the file system but is built into node. Are you trying to
  bundle for node? You can use "--platform=node" to do that, which will remove this error.

6 of 12 errors shown (disable the message limit with --log-limit=0)
 ELIFECYCLE  Command failed with exit code 1.

philipp-spiess · 2023-08-28T11:21:55Z

Nice! This is the same "solution" we came up with for the VS Code extension 🙃 https://github.com/sourcegraph/sourcegraph/pull/42802/files

I wonder if we should make this config extension wide (e.g. cody.proxy or so)? We just need to make sure we pass it to all fetch() calls but we don't have to do that immediatly

philipp-spiess · 2023-08-28T11:24:46Z

DMed you this as well but posting it here for completeness too.

The CI issues come beacuse socks is a node only library but VS Code extensions are generic to the underlying platform so they need to work on both node (desktop) and web (browser).

To fix this, we'll need to make sure to only include this library in the desktop build. We currently split based on these two entry files:

So you need to make sure that the code for this is injected at these endpoints and stubbed out for web.

philipp-spiess · 2023-08-31T10:08:35Z

@cbart I've done the branching now with a mutable ref object pattern to enable keeping TCP connections alive. I think if you rebase your change on top of this PR, it should be relatively easy for you to fix the CI issue you are seeing: #868

This reverts commit 22b98f5.

chwarwick · 2023-09-06T18:43:30Z

agent/src/protocol.ts

@@ -154,6 +154,7 @@ export interface ExtensionConfiguration {
    customHeaders: Record<string, string>
    autocompleteAdvancedProvider?: string
    autocompleteAdvancedServerEndpoint?: string | null
+    autocompleteAdvancedServerSocksProxy?: string | null


Would this probably be better as a Proxy or AgentProxy setting in the current use we need it only for a socks proxy and only for autocomplete, but if a customer were to need it, I would think it's likely to need it for all network requests.

Makes sense. In this case we use autocompleteAdvancedServerSocksProxy for autocompleteAdvancedServerEndpoint. serverEndpoint might have its own proxy. What do you think about having proxies as Record<string, string where we can map endpoints to proxies, e.g.

proxies: { "https://server-endpoint.com": "https://server-endpoint-proxy.com", "https://autocomplete-endpoint.com": "socks://your-name%40gmail.com:abcdef12345124@br41.nordvpn.com" }

Then in fetch.node.ts we can check whether there is a proxy for the requested URL and return the corresponding proxy agent. For example, if the proxy endpoint protocol is socks, we return the socks proxy agent instance.
cc: @chwarwick @philipp-spiess

@taras-yemets I don't think that's something that would be common. VS Code's proxy setting also only allows one proxy server. If they really need to have a complicated setup like this, I imagine that they would configure this on the proxy level?

Renamed to "proxy" 01f9c04

Corresponding change in the paired PR: sourcegraph/sourcegraph@22f0dde

chwarwick · 2023-09-06T18:46:26Z

vscode/package.json

@@ -869,6 +869,10 @@
          "type": "string",
          "markdownDescription": "The server endpoint used for code autocomplete. This is only supported with a provider other than `anthropic`."
        },
+        "cody.autocomplete.advanced.serverSocksProxy": {
+          "type": "string",
+          "markdownDeprecationMessage": "The SOCKS proxy endpoint to access server endpoint. This is only supported with `unstable-codegen` provider and only for use with the Cody Agent (for instance with JetBrains plugin). For VS Code please use http.proxy instead."


It feels wrong to add a new setting to the settings UI that is marked as Deprecated but in reality it isn't. Is it possible to not display this setting in the settings UI? I feel like there used to settings that were could only be set via the json and this I think would be a better user experience.

As we provide markdownDeprecationMessage this setting is not visible in the settings UI (docs). This setting is:

not shown in settings UI

not suggested in autocomplete options in settings.json

can be typed manually with the deprecation message shown

Updated the description.

Nice the picture in the description was what got me.

slimsag · 2023-09-07T22:53:18Z

Nice work on bringing this to the finish line @taras-yemets :D

vscode/src/fetch.node.ts

slimsag and others added 9 commits August 24, 2023 16:19

socks5 proxy

74d8eb6

Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>

Merge branch 'main' into cb/s/56254

535734e

SocksProxy created dynamically based on codegen setting

bf0f86c

TODO

714dce0

Merge branch 'main' into cb/s/56254

59ff479

Tidy

ff5c8cd

Tidy

f51807f

Use markdownDeprecationMessage

df22ce3

Remove printfs

68c160b

cbart mentioned this pull request Aug 28, 2023

JetBrains: forward SOCKS proxy setting to agent sourcegraph/sourcegraph#56264

Merged

1 task

Fix configuration.test.ts

448b0e9

cbart requested review from a team August 28, 2023 11:00

philipp-spiess mentioned this pull request Aug 31, 2023

Autocomplete: Enable keepalive agent for Node #868

Merged

taras-yemets added 9 commits September 5, 2023 12:25

include socks proxy only in desktop build

22b98f5

Revert "include socks proxy only in desktop build"

821365e

This reverts commit 22b98f5.

Merge remote-tracking branch 'origin/main' into cb/s/56254

d030d08

Merge branch 'main' into cb/s/56254

cbc6319

add socks proxy

d30136a

init socks agent on extension startup

0a6b175

reinit socks agent on configuration change

91cef11

simplify socks proxy agent creation

3888346

Merge remote-tracking branch 'origin/main' into cb/s/56254

b6ffcdb

olafurpg changed the title ~~Agent supports unstable-codegen access via SOCKS proxy~~ Agent: support unstable-codegen access via SOCKS proxy Sep 6, 2023

taras-yemets added 2 commits September 6, 2023 14:27

fix TS error

347a276

fix formatting

710bd87

taras-yemets marked this pull request as ready for review September 6, 2023 12:00

taras-yemets requested a review from a team September 6, 2023 12:16

chwarwick reviewed Sep 6, 2023

View reviewed changes

taras-yemets added 2 commits September 7, 2023 12:18

Merge branch 'main' into cb/s/56254

f6917cc

rename config param

01f9c04

taras-yemets requested review from chwarwick and philipp-spiess September 11, 2023 09:27

philipp-spiess approved these changes Sep 11, 2023

View reviewed changes

vscode/src/fetch.node.ts Outdated Show resolved Hide resolved

simplify condition

dc38d86

taras-yemets merged commit 4fad583 into main Sep 11, 2023
10 checks passed

taras-yemets deleted the cb/s/56254 branch September 11, 2023 11:39

taras-yemets mentioned this pull request Sep 11, 2023

S: (low priority, post-close) Allow enabling LLM proxy through IntelliJ sourcegraph/sourcegraph#56254

Closed

arafatkatze mentioned this pull request May 8, 2024

Agent - adding support for proxies(Repeat PR) #4073

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Agent: support unstable-codegen access via SOCKS proxy #836

Agent: support unstable-codegen access via SOCKS proxy #836

cbart commented Aug 28, 2023 •

edited by taras-yemets

Loading

cbart commented Aug 28, 2023

philipp-spiess commented Aug 28, 2023

philipp-spiess commented Aug 28, 2023

philipp-spiess commented Aug 31, 2023

chwarwick Sep 6, 2023

taras-yemets Sep 7, 2023 •

edited

Loading

philipp-spiess Sep 7, 2023

taras-yemets Sep 7, 2023

taras-yemets Sep 7, 2023

chwarwick Sep 6, 2023

taras-yemets Sep 7, 2023 •

edited

Loading

chwarwick Sep 7, 2023

slimsag commented Sep 7, 2023

Agent: support unstable-codegen access via SOCKS proxy #836

Agent: support unstable-codegen access via SOCKS proxy #836

Conversation

cbart commented Aug 28, 2023 • edited by taras-yemets Loading

Test plan

cbart commented Aug 28, 2023

philipp-spiess commented Aug 28, 2023

philipp-spiess commented Aug 28, 2023

philipp-spiess commented Aug 31, 2023

chwarwick Sep 6, 2023

Choose a reason for hiding this comment

taras-yemets Sep 7, 2023 • edited Loading

Choose a reason for hiding this comment

philipp-spiess Sep 7, 2023

Choose a reason for hiding this comment

taras-yemets Sep 7, 2023

Choose a reason for hiding this comment

taras-yemets Sep 7, 2023

Choose a reason for hiding this comment

chwarwick Sep 6, 2023

Choose a reason for hiding this comment

taras-yemets Sep 7, 2023 • edited Loading

Choose a reason for hiding this comment

chwarwick Sep 7, 2023

Choose a reason for hiding this comment

slimsag commented Sep 7, 2023

cbart commented Aug 28, 2023 •

edited by taras-yemets

Loading

taras-yemets Sep 7, 2023 •

edited

Loading

taras-yemets Sep 7, 2023 •

edited

Loading