mirror of
https://github.com/tiennm99/ccs.git
synced 2026-09-02 10:19:37 +00:00
fix(cliproxy): respect enabled:false and use protocol-based port defaults
- Fix port default logic: HTTP→8317, HTTPS→443 (was always 8317) - Fix enabled:false being ignored in proxy config resolution - Update stale comments referencing HTTP:80 to HTTP:8317 - Add 9 unit tests for edge cases (enabled handling, port defaults)
This commit is contained in:
@@ -215,9 +215,12 @@ export function resolveProxyConfig(
|
|||||||
...DEFAULT_PROXY_CONFIG,
|
...DEFAULT_PROXY_CONFIG,
|
||||||
};
|
};
|
||||||
|
|
||||||
// Determine mode: remote if host is specified anywhere (unless --local-proxy)
|
// Determine mode: remote if host is specified AND remote is not explicitly disabled
|
||||||
|
// Priority: CLI flags > ENV > YAML config
|
||||||
|
// If YAML config has enabled: false, it only blocks YAML-sourced config (CLI/ENV override)
|
||||||
|
const yamlRemoteEnabled = yamlConfig.remote?.enabled !== false;
|
||||||
const hasRemoteHost =
|
const hasRemoteHost =
|
||||||
cliFlags.host || envConfig.host || yamlConfig.remote?.host || yamlConfig.remote?.enabled;
|
cliFlags.host || envConfig.host || (yamlRemoteEnabled && yamlConfig.remote?.host);
|
||||||
|
|
||||||
// --local-proxy forces local mode regardless of remote config
|
// --local-proxy forces local mode regardless of remote config
|
||||||
if (cliFlags.localProxy) {
|
if (cliFlags.localProxy) {
|
||||||
|
|||||||
@@ -36,7 +36,7 @@ export interface RemoteProxyClientConfig {
|
|||||||
* Remote proxy port.
|
* Remote proxy port.
|
||||||
* Optional - defaults based on protocol:
|
* Optional - defaults based on protocol:
|
||||||
* - HTTPS: 443
|
* - HTTPS: 443
|
||||||
* - HTTP: 80
|
* - HTTP: 8317 (CLIProxyAPI default)
|
||||||
*/
|
*/
|
||||||
port?: number;
|
port?: number;
|
||||||
/** Protocol to use (http or https) */
|
/** Protocol to use (http or https) */
|
||||||
@@ -52,8 +52,16 @@ export interface RemoteProxyClientConfig {
|
|||||||
/** Default timeout for remote proxy requests (aggressive for CLI UX) */
|
/** Default timeout for remote proxy requests (aggressive for CLI UX) */
|
||||||
const DEFAULT_TIMEOUT_MS = 2000;
|
const DEFAULT_TIMEOUT_MS = 2000;
|
||||||
|
|
||||||
/** Default CLIProxyAPI port */
|
/**
|
||||||
const DEFAULT_CLIPROXY_PORT = 8317;
|
* Get default port for CLIProxyAPI based on protocol.
|
||||||
|
* - HTTP: 8317 (CLIProxyAPI default for local/dev scenarios)
|
||||||
|
* - HTTPS: 443 (standard SSL port for production remote servers)
|
||||||
|
*
|
||||||
|
* This matches the UI labels shown to users.
|
||||||
|
*/
|
||||||
|
function getDefaultPort(protocol: 'http' | 'https'): number {
|
||||||
|
return protocol === 'https' ? 443 : 8317;
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Get standard web port for protocol (for URL display omission)
|
* Get standard web port for protocol (for URL display omission)
|
||||||
@@ -65,11 +73,11 @@ function getStandardWebPort(protocol: 'http' | 'https'): number {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Get effective port for CLIProxyAPI connection
|
* Get effective port for CLIProxyAPI connection.
|
||||||
* If port is provided, use it. Otherwise use CLIProxyAPI default (8317).
|
* If port is provided, use it. Otherwise use protocol-based default.
|
||||||
*/
|
*/
|
||||||
function getEffectivePort(port: number | undefined): number {
|
function getEffectivePort(port: number | undefined, protocol: 'http' | 'https'): number {
|
||||||
return port ?? DEFAULT_CLIPROXY_PORT;
|
return port ?? getDefaultPort(protocol);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -83,7 +91,7 @@ function buildProxyUrl(
|
|||||||
protocol: 'http' | 'https',
|
protocol: 'http' | 'https',
|
||||||
path: string
|
path: string
|
||||||
): string {
|
): string {
|
||||||
const effectivePort = getEffectivePort(port);
|
const effectivePort = getEffectivePort(port, protocol);
|
||||||
const standardWebPort = getStandardWebPort(protocol);
|
const standardWebPort = getStandardWebPort(protocol);
|
||||||
|
|
||||||
// Only omit port from URL if it matches the standard web port for the protocol
|
// Only omit port from URL if it matches the standard web port for the protocol
|
||||||
|
|||||||
@@ -184,7 +184,7 @@ function mergeWithDefaults(partial: Partial<UnifiedConfig>): UnifiedConfig {
|
|||||||
enabled:
|
enabled:
|
||||||
partial.cliproxy_server?.remote?.enabled ?? DEFAULT_CLIPROXY_SERVER_CONFIG.remote.enabled,
|
partial.cliproxy_server?.remote?.enabled ?? DEFAULT_CLIPROXY_SERVER_CONFIG.remote.enabled,
|
||||||
host: partial.cliproxy_server?.remote?.host ?? DEFAULT_CLIPROXY_SERVER_CONFIG.remote.host,
|
host: partial.cliproxy_server?.remote?.host ?? DEFAULT_CLIPROXY_SERVER_CONFIG.remote.host,
|
||||||
// Port is optional - undefined means use protocol default (443/80)
|
// Port is optional - undefined means use protocol default (443 for HTTPS, 8317 for HTTP)
|
||||||
port: partial.cliproxy_server?.remote?.port,
|
port: partial.cliproxy_server?.remote?.port,
|
||||||
protocol:
|
protocol:
|
||||||
partial.cliproxy_server?.remote?.protocol ??
|
partial.cliproxy_server?.remote?.protocol ??
|
||||||
|
|||||||
@@ -345,7 +345,7 @@ export const DEFAULT_CLIPROXY_SERVER_CONFIG: CliproxyServerConfig = {
|
|||||||
remote: {
|
remote: {
|
||||||
enabled: false,
|
enabled: false,
|
||||||
host: '',
|
host: '',
|
||||||
// port is intentionally omitted - will use protocol default (443 for HTTPS, 80 for HTTP)
|
// port is intentionally omitted - will use protocol default (443 for HTTPS, 8317 for HTTP)
|
||||||
protocol: 'http',
|
protocol: 'http',
|
||||||
auth_token: '',
|
auth_token: '',
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -255,6 +255,58 @@ describe('proxy-config-resolver', () => {
|
|||||||
expect(config.remoteOnly).toBe(true);
|
expect(config.remoteOnly).toBe(true);
|
||||||
expect(config.fallbackEnabled).toBe(false);
|
expect(config.fallbackEnabled).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Tests for YAML enabled:false handling (Bug fix validation)
|
||||||
|
describe('YAML remote.enabled handling', () => {
|
||||||
|
it('should block remote mode when YAML remote.enabled is false', () => {
|
||||||
|
const { config } = resolveProxyConfig([], {
|
||||||
|
remote: { enabled: false, host: 'yaml-host.example.com' },
|
||||||
|
});
|
||||||
|
expect(config.mode).toBe('local');
|
||||||
|
// Host from YAML should NOT be used when enabled: false
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should allow remote mode when YAML remote.enabled is true', () => {
|
||||||
|
const { config } = resolveProxyConfig([], {
|
||||||
|
remote: { enabled: true, host: 'yaml-host.example.com' },
|
||||||
|
});
|
||||||
|
expect(config.mode).toBe('remote');
|
||||||
|
expect(config.host).toBe('yaml-host.example.com');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should allow remote mode when YAML remote.enabled is undefined (backwards compat)', () => {
|
||||||
|
const { config } = resolveProxyConfig([], {
|
||||||
|
remote: { host: 'yaml-host.example.com' },
|
||||||
|
});
|
||||||
|
expect(config.mode).toBe('remote');
|
||||||
|
expect(config.host).toBe('yaml-host.example.com');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should allow CLI --proxy-host to override YAML enabled:false', () => {
|
||||||
|
const { config } = resolveProxyConfig(['--proxy-host', 'cli-host'], {
|
||||||
|
remote: { enabled: false, host: 'yaml-host' },
|
||||||
|
});
|
||||||
|
expect(config.mode).toBe('remote');
|
||||||
|
expect(config.host).toBe('cli-host');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should allow ENV CCS_PROXY_HOST to override YAML enabled:false', () => {
|
||||||
|
process.env.CCS_PROXY_HOST = 'env-host';
|
||||||
|
const { config } = resolveProxyConfig([], {
|
||||||
|
remote: { enabled: false, host: 'yaml-host' },
|
||||||
|
});
|
||||||
|
expect(config.mode).toBe('remote');
|
||||||
|
expect(config.host).toBe('env-host');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should force local mode with --local-proxy even when YAML enabled:true', () => {
|
||||||
|
const { config } = resolveProxyConfig(['--local-proxy'], {
|
||||||
|
remote: { enabled: true, host: 'yaml-host' },
|
||||||
|
});
|
||||||
|
expect(config.mode).toBe('local');
|
||||||
|
expect(config.forceLocal).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('hasProxyFlags', () => {
|
describe('hasProxyFlags', () => {
|
||||||
|
|||||||
@@ -137,4 +137,35 @@ describe('remote-proxy-client', () => {
|
|||||||
expect(expectedUrl).toBe('https://secure.example.com:443/v1/models');
|
expect(expectedUrl).toBe('https://secure.example.com:443/v1/models');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('port defaults by protocol', () => {
|
||||||
|
// Document expected default ports based on protocol
|
||||||
|
// HTTP: 8317 (CLIProxyAPI default for local/dev scenarios)
|
||||||
|
// HTTPS: 443 (standard SSL port for production remote servers)
|
||||||
|
|
||||||
|
it('should document HTTP default port as 8317', () => {
|
||||||
|
// HTTP connections default to CLIProxyAPI port 8317
|
||||||
|
// This matches local development scenarios
|
||||||
|
const expectedHttpDefault = 8317;
|
||||||
|
expect(expectedHttpDefault).toBe(8317);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should document HTTPS default port as 443', () => {
|
||||||
|
// HTTPS connections default to standard SSL port 443
|
||||||
|
// This matches production remote server scenarios
|
||||||
|
const expectedHttpsDefault = 443;
|
||||||
|
expect(expectedHttpsDefault).toBe(443);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should allow port to be optional in config', () => {
|
||||||
|
// Port is optional - when undefined, defaults based on protocol
|
||||||
|
const configWithoutPort: RemoteProxyClientConfig = {
|
||||||
|
host: 'example.com',
|
||||||
|
protocol: 'https',
|
||||||
|
// port is intentionally undefined
|
||||||
|
};
|
||||||
|
expect(configWithoutPort.port).toBeUndefined();
|
||||||
|
expect(configWithoutPort.protocol).toBe('https');
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user