Skip to content

fix(vscode-ide-companion): pass proxy configuration to CLI#2501

Merged
tanzhenxin merged 2 commits intoQwenLM:mainfrom
qqqys:fix/vscode_proxy
Mar 20, 2026
Merged

fix(vscode-ide-companion): pass proxy configuration to CLI#2501
tanzhenxin merged 2 commits intoQwenLM:mainfrom
qqqys:fix/vscode_proxy

Conversation

@qqqys
Copy link
Copy Markdown
Collaborator

@qqqys qqqys commented Mar 19, 2026

TLDR

Fix proxy support in VS Code IDE companion by reading VS Code proxy settings and passing them to the CLI via --proxy argument. This resolves connection issues when users are behind a corporate proxy.

Dive Deeper

Users behind corporate proxies were unable to use the VS Code extension because the proxy settings from VS Code were not being passed to the Qwen CLI process. This fix:

  1. Reads the http.proxy setting from VS Code configuration
  2. Falls back to https.proxy if http.proxy is not set
  3. Passes the proxy URL to the CLI via --proxy argument when a proxy is configured
  4. Includes comprehensive unit tests covering:
    • Proxy passed when http.proxy is set
    • Proxy passed when only https.proxy is set (fallback)
    • Preference for http.proxy over https.proxy
    • No proxy argument when no proxy is configured
    • No proxy argument when proxy is empty string

Reviewer Test Plan

  1. Pull this branch and build the VS Code extension
  2. Configure a proxy in VS Code settings (http.proxy)
  3. Start the extension and verify the CLI receives the proxy argument (check console logs)
  4. Run tests: npm run test in packages/vscode-ide-companion

Testing Matrix

🍏 🪟 🐧
npm run
npx
Docker
Podman - -
Seatbelt - -

Linked issues / bugs

Resolves #2492

@github-actions
Copy link
Copy Markdown
Contributor

📋 Review Summary

This PR addresses a critical connectivity issue for users behind corporate proxies by reading VS Code's proxy configuration and passing it to the CLI via the --proxy argument. The implementation is clean, well-tested, and follows existing code patterns. The test coverage is comprehensive and covers all edge cases.

🔍 General Feedback

  • Positive aspects:

    • Clear problem statement and solution in the PR description
    • Comprehensive test coverage with 5 test cases covering all scenarios
    • Minimal code change that follows the existing architecture
    • Good use of logging for debugging proxy configuration
    • Proper fallback from http.proxy to https.proxy
  • Code quality:

    • Follows existing TypeScript patterns in the codebase
    • Maintains consistency with the existing code style
    • Test file follows the established vitest mocking patterns

🎯 Specific Feedback

🔵 Low

  • File: qwenConnectionHandler.ts:78-85 - Consider extracting the proxy configuration logic into a separate private method for better testability and separation of concerns. This would make the connect method more focused and allow easier mocking in tests if needed.

    private getProxyConfiguration(): string | undefined {
      const httpConfig = vscode.workspace.getConfiguration('http');
      const proxyUrl =
        httpConfig.get<string>('proxy') || httpConfig.get<string>('https.proxy');
      return proxyUrl || undefined;
    }
  • File: qwenConnectionHandler.ts:82 - The condition if (proxyUrl) will pass through even if proxyUrl is an empty string (falsy check). While the test covers this case, consider being more explicit:

    if (proxyUrl && proxyUrl.trim() !== '')

    This ensures whitespace-only strings are also treated as no proxy configured.

  • File: qwenConnectionHandler.test.ts:44-56 - Consider adding a test case for when proxy URL is whitespace-only string to verify the behavior matches expectations.

  • File: qwenConnectionHandler.ts:81 - The typing httpConfig.get<string>('proxy') could potentially return undefined which is handled by the || operator, but TypeScript might infer this better with explicit typing:

    const proxyUrl =
      httpConfig.get<string>('proxy') ?? httpConfig.get<string>('https.proxy');

    Using nullish coalescing (??) instead of logical OR (||) is more explicit about handling only null/undefined vs other falsy values like empty strings.

✅ Highlights

  • Excellent test coverage that validates:

    • Primary http.proxy configuration
    • Fallback to https.proxy
    • Preference for http.proxy when both are set
    • No proxy argument when configuration is missing
    • No proxy argument when configuration is empty string
  • The fix directly addresses a real user pain point (corporate proxy connectivity)

  • Logging added for debugging: console.log('[QwenAgentManager] Using proxy from VSCode settings:', proxyUrl) will help troubleshoot proxy-related issues

  • Minimal invasive change that doesn't affect other parts of the connection handling logic

Copy link
Copy Markdown
Collaborator

@yiliang114 yiliang114 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Collaborator

@tanzhenxin tanzhenxin left a comment

Choose a reason for hiding this comment

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

LGTM!

@tanzhenxin tanzhenxin merged commit f9b5bc5 into QwenLM:main Mar 20, 2026
15 checks passed
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.

vscode扩展似乎没有正确使用proxy代理

3 participants