Skip to content

feat(dashscope): support DASHSCOPE_PROXY_BASE_URL for prompt cache via API gateway #3991

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.

Sign up for GitHub

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

Merged
wenshao merged 12 commits into from
May 12, 2026
Merged

feat(dashscope): support DASHSCOPE_PROXY_BASE_URL for prompt cache via API gateway #3991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1724132
support DASHSCOPE_PROXY_BASE_URL for prompt cache via API gateway
HeZiGang May 9, 2026
7aa10d4
fix: normalize proxy URL trailing slash and prevent test env leak
zackkkkkkk May 11, 2026
68e011e
Apply suggestions from code review
HeZiGang May 11, 2026
79cb463
chore: standardize variable usage to avoid confusion。
zackkkkkkk May 11, 2026
8144535
Remove the function and comments; keep only the const that productio…
zackkkkkkk May 12, 2026
7d29bf5
Case-sensitivity inconsistency between origin check and proxy check
zackkkkkkk May 12, 2026
669cd4a
Test mock diverges from production and is fragile
zackkkkkkk May 12, 2026
68ef1f8
Add diagnostic logging when proxy URL doesn't match
zackkkkkkk May 12, 2026
6524671
remove the duplicate vi.mock calls for the same '../constants.js' mod…
zackkkkkkk May 12, 2026
4696d34
Update packages/core/src/core/openaiContentGenerator/provider/dashsco…
HeZiGang May 12, 2026
420694b
- Renamed variables to adhere to project naming conventions.
zackkkkkkk May 12, 2026
e5bd8aa
1. Move it to module level for consistency and to avoid unnecessary a…
zackkkkkkk May 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/core/src/core/openaiContentGenerator/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ export const DEFAULT_DASHSCOPE_BASE_URL =
'https://dashscope.aliyuncs.com/compatible-mode/v1';
export const DEFAULT_DEEPSEEK_BASE_URL = 'https://api.deepseek.com/v1';
export const DEFAULT_OPEN_ROUTER_BASE_URL = 'https://openrouter.ai/api/v1';
export const DASHSCOPE_PROXY_BASE_URL = process.env['DASHSCOPE_PROXY_BASE_URL'];
Comment thread
HeZiGang marked this conversation as resolved.
95 changes: 95 additions & 0 deletions packages/core/src/core/openaiContentGenerator/provider/dashscope.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ import { DEFAULT_TIMEOUT, DEFAULT_MAX_RETRIES } from '../constants.js';
import { buildRuntimeFetchOptions } from '../../../utils/runtimeFetchOptions.js';
import type { OpenAIRuntimeFetchOptions } from '../../../utils/runtimeFetchOptions.js';

const mockDebugLogger = vi.hoisted(() => ({
debug: vi.fn(),
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
}));
vi.mock('../../../utils/debugLogger.js', () => ({
createDebugLogger: vi.fn(() => mockDebugLogger),
}));

// Mock OpenAI
vi.mock('openai', () => ({
default: vi.fn().mockImplementation((config) => ({
Expand All @@ -38,13 +48,28 @@ vi.mock('../../../utils/runtimeFetchOptions.js', () => ({
buildRuntimeFetchOptions: vi.fn(),
}));

@wenshao wenshao May 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] Test mock diverges from production and is fragile

The vi.mock('../constants.js') uses a getter for DASHSCOPE_PROXY_BASE_URL (dynamic read from process.env), but production uses export const (frozen at import time). Additionally, the mock duplicates all 6 other constant exports verbatim — any future constant added to constants.ts would be silently undefined in these tests. Consider using importOriginal to preserve real exports:

Suggested change
}));
vi.mock('../constants.js', async (importOriginal) => {
const actual = await importOriginal();
return {
...actual,
get DASHSCOPE_PROXY_BASE_URL() {
return process.env['DASHSCOPE_PROXY_BASE_URL'];
},
};
});

— glm-5.1 via Qwen Code /review

@HeZiGang HeZiGang May 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This logic is already implemented at lines 60–62. Why is this recommendation showing up?


// Mock DASHSCOPE_PROXY_BASE_URL so tests can control its value
vi.mock('../constants.js', () => ({
DEFAULT_TIMEOUT: 120000,
DEFAULT_MAX_RETRIES: 3,
DEFAULT_OPENAI_BASE_URL: 'https://api.openai.com/v1',
DEFAULT_DASHSCOPE_BASE_URL:
'https://dashscope.aliyuncs.com/compatible-mode/v1',
DEFAULT_DEEPSEEK_BASE_URL: 'https://api.deepseek.com/v1',
DEFAULT_OPEN_ROUTER_BASE_URL: 'https://openrouter.ai/api/v1',
get DASHSCOPE_PROXY_BASE_URL() {
return process.env['DASHSCOPE_PROXY_BASE_URL'];
},
}));

describe('DashScopeOpenAICompatibleProvider', () => {
let provider: DashScopeOpenAICompatibleProvider;
let mockContentGeneratorConfig: ContentGeneratorConfig;
let mockCliConfig: Config;

beforeEach(() => {
vi.clearAllMocks();
vi.unstubAllEnvs();
const mockedBuildRuntimeFetchOptions =
buildRuntimeFetchOptions as unknown as MockedFunction<
(sdkType: 'openai', proxyUrl?: string) => OpenAIRuntimeFetchOptions
Expand Down Expand Up @@ -162,6 +187,76 @@ describe('DashScopeOpenAICompatibleProvider', () => {
expect(result).toBe(false);
});
});

it('should return true when baseUrl matches DASHSCOPE_PROXY_BASE_URL', () => {
vi.stubEnv(
'DASHSCOPE_PROXY_BASE_URL',
'https://your-proxy.com/dashscope',
);

const config = {
authType: AuthType.USE_OPENAI,
baseUrl: 'https://your-proxy.com/dashscope',
} as ContentGeneratorConfig;

const result =
DashScopeOpenAICompatibleProvider.isDashScopeProvider(config);
expect(result).toBe(true);
});

it('should return false when baseUrl does not match DASHSCOPE_PROXY_BASE_URL', () => {
vi.stubEnv(
'DASHSCOPE_PROXY_BASE_URL',
'https://your-proxy.com/dashscope',
);

const config = {
authType: AuthType.USE_OPENAI,
baseUrl: 'https://other-proxy.com/dashscope',
} as ContentGeneratorConfig;

const result =
DashScopeOpenAICompatibleProvider.isDashScopeProvider(config);
expect(result).toBe(false);
});

it('should debug log when baseUrl does not match DASHSCOPE_PROXY_BASE_URL', () => {
vi.stubEnv(
'DASHSCOPE_PROXY_BASE_URL',
'https://your-proxy.com/dashscope',
);

const config = {
authType: AuthType.USE_OPENAI,
baseUrl: 'https://other-proxy.com/dashscope',
} as ContentGeneratorConfig;

const result =
DashScopeOpenAICompatibleProvider.isDashScopeProvider(config);

expect(result).toBe(false);
expect(mockDebugLogger.debug).toHaveBeenCalledWith(
expect.stringContaining(
'DASHSCOPE_PROXY_BASE_URL is configured but the request baseUrl does not match',
),

@wenshao wenshao May 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Critical] Test assertion does not match production code — the test will fail.

The test expects the debug log to contain "DASHSCOPE_PROXY_BASE_URL is configured as 'https://your-proxy.com/dashscope'", but the actual code in dashscope.ts outputs "DASHSCOPE_PROXY_BASE_URL is configured but the request baseUrl does not match. DashScope headers/cache control will be skipped.".

The expect.stringContaining check will fail because the actual message uses "configured but" instead of "configured as", and does not include the URL value.

Suggested change
),
"DASHSCOPE_PROXY_BASE_URL is configured but the request baseUrl does not match. DashScope headers/cache control will be skipped.",

— DeepSeek/deepseek-v4-pro via Qwen Code /review

);
});

it('should return true when baseUrl matches DASHSCOPE_PROXY_BASE_URL with trailing slash', () => {
vi.stubEnv(
'DASHSCOPE_PROXY_BASE_URL',
'https://your-proxy.com/dashscope',
);

const config = {
authType: AuthType.USE_OPENAI,
baseUrl: 'https://your-proxy.com/dashscope/',
} as ContentGeneratorConfig;

const result =
DashScopeOpenAICompatibleProvider.isDashScopeProvider(config);
expect(result).toBe(true);
});
});

describe('buildHeaders', () => {
Expand Down
29 changes: 28 additions & 1 deletion packages/core/src/core/openaiContentGenerator/provider/dashscope.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
DEFAULT_TIMEOUT,
DEFAULT_MAX_RETRIES,
DEFAULT_DASHSCOPE_BASE_URL,
DASHSCOPE_PROXY_BASE_URL,
} from '../constants.js';
import type {
DashScopeRequestMetadata,
Expand All @@ -15,8 +16,11 @@
ChatCompletionToolWithCache,
} from './types.js';
import { buildRuntimeFetchOptions } from '../../../utils/runtimeFetchOptions.js';
import { createDebugLogger } from '../../../utils/debugLogger.js';
import { DefaultOpenAICompatibleProvider } from './default.js';

const debugLogger = createDebugLogger('DashScopeOpenAICompatibleProvider');

export class DashScopeOpenAICompatibleProvider extends DefaultOpenAICompatibleProvider {
constructor(
contentGeneratorConfig: ContentGeneratorConfig,
Expand All @@ -33,8 +37,31 @@
if (authType === AuthType.QWEN_OAUTH) return true;
if (!baseUrl) return true;

const normalizedBaseUrl = baseUrl.endsWith('/')
? baseUrl.slice(0, -1)
: baseUrl;

// Matches: dashscope.aliyuncs.com, *.dashscope.aliyuncs.com, or *.dashscope-intl.aliyuncs.com
return /([\w-]+\.)?dashscope(-intl)?\.aliyuncs\.com/i.test(baseUrl);
const isDashscopeOrigin =
/([\w-]+\.)?dashscope(-intl)?\.aliyuncs\.com/i.test(normalizedBaseUrl);

// Check if proxy is configured and matches
const normalizedProxyUrl = DASHSCOPE_PROXY_BASE_URL?.endsWith('/')
? DASHSCOPE_PROXY_BASE_URL.slice(0, -1)
: DASHSCOPE_PROXY_BASE_URL;

@wenshao wenshao May 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] Case-sensitivity inconsistency between origin check and proxy check

The origin-domain regex on line 43 uses the /i (case-insensitive) flag, but the proxy URL comparison here uses strict ===. If DASHSCOPE_PROXY_BASE_URL and baseUrl differ only in casing (common in CI/CD environments where env vars and config files come from different sources), the proxy match silently fails and DashScope-specific headers, cache control, and session tracking are all skipped.

Suggested change
const isProxyConfigured = Boolean(
normalizedProxyUrl &&
normalizedBaseUrl.toLowerCase() === normalizedProxyUrl.toLowerCase(),
);

— glm-5.1 via Qwen Code /review

const isProxyMatch = Boolean(

@wenshao wenshao May 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] Proxy URL comparison uses exact equality — may miss path prefix mismatches

The proxy match uses normalizedBaseUrl.toLowerCase() === normalizedProxyUrl.toLowerCase() (strict full-URL equality). If a user sets DASHSCOPE_PROXY_BASE_URL=https://proxy.com/dashscope but their baseUrl includes a versioned path like https://proxy.com/dashscope/v1, the match fails silently. DashScope-specific behavior (cache headers, metadata) would be dropped with no error — only a debug-level log that most users won't see.

The origin-domain check on line 46 uses a lenient regex substring match, so it's immune to this. The asymmetry is intentional but the strict proxy comparison could cause real misconfiguration issues.

Suggested change
const isProxyMatch = Boolean(
const isProxyMatch = Boolean(normalizedProxyUrl && (() => {
try {
const base = new URL(normalizedBaseUrl);
const proxy = new URL(normalizedProxyUrl);
return base.origin.toLowerCase() === proxy.origin.toLowerCase() &&
(base.pathname === proxy.pathname ||
base.pathname.startsWith(
proxy.pathname.endsWith('/') ? proxy.pathname : proxy.pathname + '/',
));
} catch {
return false;
}
})());

— glm-5.1 via Qwen Code /review

normalizedProxyUrl &&
normalizedBaseUrl.toLowerCase() === normalizedProxyUrl.toLowerCase(),
);

if (normalizedProxyUrl && !isDashscopeOrigin && !isProxyMatch) {

@wenshao wenshao May 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] createDebugLogger called inside static method body — inconsistent with codebase convention.

The codebase has 100+ usages of createDebugLogger at module level (e.g., converter.ts, errorHandler.ts, openaiContentGenerator.ts). This is the only place it's called inside a method body, creating a new logger object on every invocation.

Move it to module level for consistency and to avoid unnecessary allocations.

Suggested change
if (normalizedProxyUrl && !isDashscopeOrigin && !isProxyMatch) {
// At module level (outside the class), after imports:
const debugLogger = createDebugLogger('DashScopeOpenAICompatibleProvider');

— DeepSeek/deepseek-v4-pro via Qwen Code /review

debugLogger.debug(
`DASHSCOPE_PROXY_BASE_URL is configured but the request baseUrl does not match. DashScope headers/cache control will be skipped.`,
);

@wenshao wenshao May 12, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] Debug log on proxy mismatch does not include the actual URL values being compared.

When DASHSCOPE_PROXY_BASE_URL is configured but doesn't match, the current log message does not show either the proxy URL or the request baseUrl. Users cannot diagnose why the match failed without adding their own logging.

Suggested change
);
debugLogger.debug(
`DASHSCOPE_PROXY_BASE_URL is configured as '${normalizedProxyUrl}' ` +
`but the request baseUrl ('${normalizedBaseUrl}') does not match. ` +
`DashScope headers/cache control will be skipped.`,
);

— DeepSeek/deepseek-v4-pro via Qwen Code /review

@HeZiGang HeZiGang May 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

remove the url in logs to prevent URL exposure.

}

return isDashscopeOrigin || isProxyMatch;
}

override buildHeaders(): Record<string, string | undefined> {
Expand Down
Loading