Skip to content

Conversation

catrielmuller
Copy link
Contributor

@catrielmuller catrielmuller commented Oct 14, 2025

Context

Resolve: #2971

Implementation

  • Proxy Support

Screenshots

image

How to Test

HTTP_PROXY=http://localhost:8080 HTTPS_PROXY=http://localhost:8080 NODE_TLS_REJECT_UNAUTHORIZED=0 pnpm run start "what time is now?"

Copy link

changeset-bot bot commented Oct 14, 2025

⚠️ No Changeset found

Latest commit: 48fbbe3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements CLI proxy support and fixes duplicated request handling between the CLI and extension. The changes include conditional model fetching to prevent unnecessary requests to unconfigured providers, proper message routing to avoid duplicates, and comprehensive proxy configuration supporting HTTP/HTTPS proxies with authentication, NO_PROXY patterns, and self-signed certificates.

Key Changes:

  • Proxy Infrastructure: Added comprehensive proxy support with environment variable configuration, NO_PROXY pattern matching, and HTTP client integration
  • Conditional Model Fetching: Modified webview message handler to only fetch models from providers with valid API keys or active configurations
  • Message Handler Cleanup: Removed duplicate TUI message handlers and streamlined routing to prevent request duplication

Reviewed Changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/core/webview/webviewMessageHandler.ts Refactored model fetching to be conditional based on provider configuration
src/core/webview/__tests__/webviewMessageHandler.spec.ts Updated tests to reflect conditional model fetching behavior
cli/src/utils/proxy-matcher.ts New utility for NO_PROXY pattern matching with wildcard and CIDR support
cli/src/utils/proxy-config.ts New proxy configuration module supporting axios, fetch, and undici
cli/src/ui/UI.tsx Fixed dependency array to prevent unnecessary re-renders
cli/src/services/extension.ts Removed duplicate TUI request handler to fix message duplication
cli/src/index.ts Added proxy configuration initialization at startup
cli/src/host/ExtensionHost.ts Enhanced proxy setup and streamlined webview message handling
cli/package.json Added proxy agent dependencies
cli/package.dist.json Added proxy agent dependencies for distribution
cli/esbuild.config.mjs Added proxy agents to external dependencies
cli/README.md Added comprehensive proxy configuration documentation
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

Copy link
Contributor

@RSO RSO left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unclear to me why we'd want to implement proxy support in the CLI at this point, or why you decided to combine the duplicated request fix in the same PR.

Combined with the changes outside of the CLI folder, I'm inclined to say we don't want this and should instead look for other solutions. So can you maybe answer a couple of questions?

  • Why did you combine the two fixes in one PR?
  • Why do you think we need HTTP proxy support in the CLI?
  • Why are there so many changes in the ClineProvider test file?

@catrielmuller
Copy link
Contributor Author

catrielmuller commented Oct 15, 2025

It's unclear to me why we'd want to implement proxy support in the CLI at this point, or why you decided to combine the duplicated request fix in the same PR.

Combined with the changes outside of the CLI folder, I'm inclined to say we don't want this and should instead look for other solutions. So can you maybe answer a couple of questions?

  • Why did you combine the two fixes in one PR?
  • Why do you think we need HTTP proxy support in the CLI?

I Combined the duplicated calls with the proxy support because the proxy was essential to resolve the duplicated call issues and debug this type of request issues.
In terms of feature, support HTTP(S)_PROXY env vars is essential to run the CLI in restricted corporate environments.

  • Why are there so many changes in the ClineProvider test file?

I reverted the changes of the Cline Provider, for fetch only the models for the providers that are configured. The changes of this PR only affect the CLI folder.

@RSO
Copy link
Contributor

RSO commented Oct 15, 2025

Does the extension have proxy support?

@RSO
Copy link
Contributor

RSO commented Oct 15, 2025

In terms of feature, support HTTP(S)_PROXY env vars is essential to run the CLI in restricted corporate environments.

How does this work? Wouldn't the machine be set up to talk through a proxy?

@catrielmuller
Copy link
Contributor Author

s the extension have proxy support?

Yes, vscode wrap the all network request

image

@catrielmuller
Copy link
Contributor Author

In terms of feature, support HTTP(S)_PROXY env vars is essential to run the CLI in restricted corporate environments.

How does this work? Wouldn't the machine be set up to talk through a proxy?

The proxies works on the application layer in general. For example. there are some companies/corporation that don't have access to internet on their offices, however some users have access to use a proxy for this, with a high regulated traffic log.

@catrielmuller catrielmuller changed the title CLI Proxy Support & Fix duplicated request CLI Proxy Support Oct 15, 2025
@chrarnoldus
Copy link
Collaborator

Does the extension have proxy support?

based on GitHub sentiment it is spotty at best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Selfsigned/Rewritten Certificates not supported

3 participants