mirror of
https://github.com/tiennm99/ccs.git
synced 2026-07-17 06:17:09 +00:00
fix(thinking): handle clear no-op and tighten override coverage
This commit is contained in:
@@ -363,6 +363,7 @@ export async function execClaudeWithCLIProxy(
|
||||
thinkingParse.value,
|
||||
process.env.CCS_THINKING
|
||||
);
|
||||
const thinkingCfg = getThinkingConfig();
|
||||
|
||||
if (thinkingParse.duplicateDisplays.length > 0) {
|
||||
console.warn(
|
||||
@@ -813,7 +814,6 @@ export async function execClaudeWithCLIProxy(
|
||||
process.env.CCS_CODEX_REASONING_TRACE === '1' ||
|
||||
process.env.CCS_CODEX_REASONING_TRACE === 'true';
|
||||
const stripPathPrefix = useRemoteProxy ? '/api/provider/codex' : undefined;
|
||||
const thinkingCfg = getThinkingConfig();
|
||||
const codexThinkingOff = shouldDisableCodexReasoning(thinkingCfg, thinkingOverride);
|
||||
codexReasoningProxy = new CodexReasoningProxy({
|
||||
upstreamBaseUrl: postSanitizationBaseUrl,
|
||||
@@ -889,9 +889,8 @@ export async function execClaudeWithCLIProxy(
|
||||
|
||||
// 11b. Print thinking status feedback (TTY only, non-piped sessions)
|
||||
if (process.stderr.isTTY) {
|
||||
const thinkingCfgStatus = getThinkingConfig();
|
||||
const { thinkingLabel, sourceLabel } = buildThinkingStartupStatus(
|
||||
thinkingCfgStatus,
|
||||
thinkingCfg,
|
||||
thinkingOverride,
|
||||
thinkingSource,
|
||||
thinkingParse.sourceDisplay
|
||||
|
||||
@@ -18,7 +18,7 @@ export function parseEnvThinkingOverride(raw: string | undefined): string | numb
|
||||
const trimmed = raw.trim();
|
||||
if (!trimmed) return undefined;
|
||||
|
||||
if (/^-?\d+$/.test(trimmed)) {
|
||||
if (/^\d+$/.test(trimmed)) {
|
||||
const parsed = Number.parseInt(trimmed, 10);
|
||||
if (parsed < THINKING_BUDGET_MIN || parsed > THINKING_BUDGET_MAX) {
|
||||
return undefined;
|
||||
|
||||
@@ -13,7 +13,11 @@ import {
|
||||
} from '../config/unified-config-loader';
|
||||
import { DEFAULT_THINKING_TIER_DEFAULTS } from '../config/unified-config-types';
|
||||
import { VALID_THINKING_LEVELS } from '../cliproxy/thinking-validator';
|
||||
import { parseThinkingCommandArgs, parseThinkingOverrideInput } from './config-thinking-parser';
|
||||
import {
|
||||
clearProviderOverride,
|
||||
parseThinkingCommandArgs,
|
||||
parseThinkingOverrideInput,
|
||||
} from './config-thinking-parser';
|
||||
|
||||
const VALID_THINKING_MODES = ['auto', 'off', 'manual'] as const;
|
||||
|
||||
@@ -271,27 +275,21 @@ export async function handleConfigThinkingCommand(args: string[]): Promise<void>
|
||||
process.exitCode = 1;
|
||||
return;
|
||||
}
|
||||
const currentOverrides = thinkingConfig.provider_overrides ?? {};
|
||||
const nextOverrides = { ...currentOverrides };
|
||||
if (!nextOverrides[provider]) {
|
||||
// no-op, but still considered change request to keep command deterministic
|
||||
hasChanges = true;
|
||||
} else if (!tier) {
|
||||
delete nextOverrides[provider];
|
||||
const normalizedTier = tier as ThinkingTier | undefined;
|
||||
const clearResult = clearProviderOverride(
|
||||
thinkingConfig.provider_overrides,
|
||||
provider,
|
||||
normalizedTier
|
||||
);
|
||||
thinkingConfig.provider_overrides = clearResult.nextOverrides;
|
||||
if (clearResult.changed) {
|
||||
hasChanges = true;
|
||||
} else {
|
||||
const normalizedTier = tier as ThinkingTier;
|
||||
const providerEntry = { ...nextOverrides[provider] };
|
||||
delete providerEntry[normalizedTier];
|
||||
if (Object.keys(providerEntry).length === 0) {
|
||||
delete nextOverrides[provider];
|
||||
} else {
|
||||
nextOverrides[provider] = providerEntry;
|
||||
}
|
||||
hasChanges = true;
|
||||
console.log(
|
||||
info(`No provider override found for '${provider}'${tier ? ` tier '${tier}'` : ''}`)
|
||||
);
|
||||
console.log('');
|
||||
}
|
||||
thinkingConfig.provider_overrides =
|
||||
Object.keys(nextOverrides).length > 0 ? nextOverrides : undefined;
|
||||
}
|
||||
|
||||
if (hasChanges) {
|
||||
|
||||
@@ -15,6 +15,10 @@ interface ThinkingCommandOptions {
|
||||
help?: boolean;
|
||||
}
|
||||
|
||||
type ThinkingTier = 'opus' | 'sonnet' | 'haiku';
|
||||
export type ThinkingTierOverrideMap = Partial<Record<ThinkingTier, string>>;
|
||||
export type ThinkingProviderOverrides = Record<string, ThinkingTierOverrideMap>;
|
||||
|
||||
export interface ParseResult {
|
||||
options: ThinkingCommandOptions;
|
||||
error?: string;
|
||||
@@ -107,3 +111,48 @@ export function parseThinkingOverrideInput(rawOverride: string): {
|
||||
error: `Invalid override: ${rawOverride}`,
|
||||
};
|
||||
}
|
||||
|
||||
export function clearProviderOverride(
|
||||
currentOverrides: ThinkingProviderOverrides | undefined,
|
||||
provider: string,
|
||||
tier?: ThinkingTier
|
||||
): { nextOverrides: ThinkingProviderOverrides | undefined; changed: boolean } {
|
||||
const current = currentOverrides ?? {};
|
||||
const nextOverrides: ThinkingProviderOverrides = { ...current };
|
||||
|
||||
const providerEntry = nextOverrides[provider];
|
||||
if (!providerEntry) {
|
||||
return {
|
||||
nextOverrides: Object.keys(nextOverrides).length > 0 ? nextOverrides : undefined,
|
||||
changed: false,
|
||||
};
|
||||
}
|
||||
|
||||
if (!tier) {
|
||||
delete nextOverrides[provider];
|
||||
return {
|
||||
nextOverrides: Object.keys(nextOverrides).length > 0 ? nextOverrides : undefined,
|
||||
changed: true,
|
||||
};
|
||||
}
|
||||
|
||||
if (providerEntry[tier] === undefined) {
|
||||
return {
|
||||
nextOverrides: Object.keys(nextOverrides).length > 0 ? nextOverrides : undefined,
|
||||
changed: false,
|
||||
};
|
||||
}
|
||||
|
||||
const nextProviderEntry = { ...providerEntry };
|
||||
delete nextProviderEntry[tier];
|
||||
if (Object.keys(nextProviderEntry).length === 0) {
|
||||
delete nextOverrides[provider];
|
||||
} else {
|
||||
nextOverrides[provider] = nextProviderEntry;
|
||||
}
|
||||
|
||||
return {
|
||||
nextOverrides: Object.keys(nextOverrides).length > 0 ? nextOverrides : undefined,
|
||||
changed: true,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -26,6 +26,20 @@ import { validateFilePath } from './route-helpers';
|
||||
|
||||
const router = Router();
|
||||
|
||||
export function resolveThinkingProviderOverridesForSave(
|
||||
currentProviderOverrides: ThinkingConfig['provider_overrides'] | undefined,
|
||||
updatesProviderOverrides: Record<string, Partial<ThinkingConfig['tier_defaults']>> | undefined,
|
||||
shouldClearProviderOverrides: boolean
|
||||
): ThinkingConfig['provider_overrides'] | undefined {
|
||||
if (shouldClearProviderOverrides) {
|
||||
return undefined;
|
||||
}
|
||||
if (updatesProviderOverrides !== undefined) {
|
||||
return updatesProviderOverrides;
|
||||
}
|
||||
return currentProviderOverrides;
|
||||
}
|
||||
|
||||
// ==================== Generic File API ====================
|
||||
|
||||
/**
|
||||
@@ -437,11 +451,11 @@ router.put('/thinking', (req: Request, res: Response): void => {
|
||||
sonnet: updates.tier_defaults?.sonnet ?? config.thinking?.tier_defaults?.sonnet ?? 'medium',
|
||||
haiku: updates.tier_defaults?.haiku ?? config.thinking?.tier_defaults?.haiku ?? 'low',
|
||||
},
|
||||
provider_overrides: shouldClearProviderOverrides
|
||||
? undefined
|
||||
: updates.provider_overrides !== undefined
|
||||
? normalizedProviderOverrides
|
||||
: config.thinking?.provider_overrides,
|
||||
provider_overrides: resolveThinkingProviderOverridesForSave(
|
||||
config.thinking?.provider_overrides,
|
||||
updates.provider_overrides !== undefined ? normalizedProviderOverrides : undefined,
|
||||
shouldClearProviderOverrides
|
||||
),
|
||||
show_warnings: updates.show_warnings ?? config.thinking?.show_warnings ?? true,
|
||||
};
|
||||
|
||||
|
||||
@@ -5,6 +5,11 @@ import {
|
||||
CodexReasoningProxy,
|
||||
getEffortForModel,
|
||||
} from '../../../src/cliproxy/codex-reasoning-proxy';
|
||||
import {
|
||||
parseEnvThinkingOverride,
|
||||
resolveRuntimeThinkingOverride,
|
||||
shouldDisableCodexReasoning,
|
||||
} from '../../../src/cliproxy/executor/thinking-override-resolver';
|
||||
|
||||
type JsonRecord = Record<string, unknown>;
|
||||
|
||||
@@ -224,4 +229,122 @@ describe('CodexReasoningProxy extended-context compatibility', () => {
|
||||
expect(response.statusCode).toBe(200);
|
||||
expect(capturedBody?.model).toBe('enterprise-internal-high');
|
||||
});
|
||||
|
||||
it('keeps reasoning enabled when CCS_THINKING=high overrides config off', async () => {
|
||||
let capturedBody: JsonRecord | null = null;
|
||||
|
||||
expect(parseEnvThinkingOverride('high')).toBe('high');
|
||||
const { thinkingOverride } = resolveRuntimeThinkingOverride(undefined, 'high');
|
||||
const disableEffort = shouldDisableCodexReasoning(
|
||||
{
|
||||
mode: 'off',
|
||||
tier_defaults: {
|
||||
opus: 'high',
|
||||
sonnet: 'medium',
|
||||
haiku: 'low',
|
||||
},
|
||||
show_warnings: true,
|
||||
},
|
||||
thinkingOverride
|
||||
);
|
||||
|
||||
const upstream = http.createServer((req, res) => {
|
||||
let rawBody = '';
|
||||
req.setEncoding('utf8');
|
||||
req.on('data', (chunk) => {
|
||||
rawBody += chunk;
|
||||
});
|
||||
req.on('end', () => {
|
||||
capturedBody = rawBody ? (JSON.parse(rawBody) as JsonRecord) : {};
|
||||
res.writeHead(200, { 'Content-Type': 'application/json' });
|
||||
res.end(JSON.stringify({ ok: true }));
|
||||
});
|
||||
});
|
||||
cleanupServers.push(upstream);
|
||||
|
||||
const upstreamPort = await listenOnRandomPort(upstream);
|
||||
const proxy = new CodexReasoningProxy({
|
||||
upstreamBaseUrl: `http://127.0.0.1:${upstreamPort}`,
|
||||
disableEffort,
|
||||
defaultEffort: 'medium',
|
||||
modelMap: {
|
||||
defaultModel: 'gpt-5.3-codex',
|
||||
},
|
||||
});
|
||||
|
||||
const proxyPort = await proxy.start();
|
||||
const response = await postJson(
|
||||
`http://127.0.0.1:${proxyPort}/api/provider/codex/v1/messages`,
|
||||
{
|
||||
model: 'gpt-5.3-codex-high',
|
||||
messages: [],
|
||||
}
|
||||
);
|
||||
|
||||
proxy.stop();
|
||||
|
||||
expect(response.statusCode).toBe(200);
|
||||
expect(disableEffort).toBe(false);
|
||||
expect(capturedBody?.model).toBe('gpt-5.3-codex');
|
||||
expect((capturedBody?.reasoning as JsonRecord | undefined)?.effort).toBe('high');
|
||||
});
|
||||
|
||||
it('disables reasoning when CCS_THINKING=off is provided', async () => {
|
||||
let capturedBody: JsonRecord | null = null;
|
||||
|
||||
expect(parseEnvThinkingOverride('off')).toBe('off');
|
||||
const { thinkingOverride } = resolveRuntimeThinkingOverride(undefined, 'off');
|
||||
const disableEffort = shouldDisableCodexReasoning(
|
||||
{
|
||||
mode: 'auto',
|
||||
tier_defaults: {
|
||||
opus: 'high',
|
||||
sonnet: 'medium',
|
||||
haiku: 'low',
|
||||
},
|
||||
show_warnings: true,
|
||||
},
|
||||
thinkingOverride
|
||||
);
|
||||
|
||||
const upstream = http.createServer((req, res) => {
|
||||
let rawBody = '';
|
||||
req.setEncoding('utf8');
|
||||
req.on('data', (chunk) => {
|
||||
rawBody += chunk;
|
||||
});
|
||||
req.on('end', () => {
|
||||
capturedBody = rawBody ? (JSON.parse(rawBody) as JsonRecord) : {};
|
||||
res.writeHead(200, { 'Content-Type': 'application/json' });
|
||||
res.end(JSON.stringify({ ok: true }));
|
||||
});
|
||||
});
|
||||
cleanupServers.push(upstream);
|
||||
|
||||
const upstreamPort = await listenOnRandomPort(upstream);
|
||||
const proxy = new CodexReasoningProxy({
|
||||
upstreamBaseUrl: `http://127.0.0.1:${upstreamPort}`,
|
||||
disableEffort,
|
||||
defaultEffort: 'medium',
|
||||
modelMap: {
|
||||
defaultModel: 'gpt-5.3-codex',
|
||||
},
|
||||
});
|
||||
|
||||
const proxyPort = await proxy.start();
|
||||
const response = await postJson(
|
||||
`http://127.0.0.1:${proxyPort}/api/provider/codex/v1/messages`,
|
||||
{
|
||||
model: 'gpt-5.3-codex-high',
|
||||
messages: [],
|
||||
}
|
||||
);
|
||||
|
||||
proxy.stop();
|
||||
|
||||
expect(response.statusCode).toBe(200);
|
||||
expect(disableEffort).toBe(true);
|
||||
expect(capturedBody?.model).toBe('gpt-5.3-codex');
|
||||
expect((capturedBody?.reasoning as JsonRecord | undefined)?.effort).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3,6 +3,7 @@ import {
|
||||
parseThinkingCommandArgs,
|
||||
parseThinkingOverrideInput,
|
||||
} from '../../../src/commands/config-thinking-command';
|
||||
import { clearProviderOverride } from '../../../src/commands/config-thinking-parser';
|
||||
|
||||
describe('config thinking command parser', () => {
|
||||
it('rejects missing required option values', () => {
|
||||
@@ -41,3 +42,47 @@ describe('config thinking override normalization', () => {
|
||||
expect(parseThinkingOverrideInput('8192')).toEqual({ value: 8192 });
|
||||
});
|
||||
});
|
||||
|
||||
describe('config thinking provider override clearing', () => {
|
||||
it('is a no-op when provider override does not exist', () => {
|
||||
const result = clearProviderOverride(
|
||||
{
|
||||
codex: { opus: 'high' },
|
||||
},
|
||||
'gemini'
|
||||
);
|
||||
|
||||
expect(result.changed).toBe(false);
|
||||
expect(result.nextOverrides).toEqual({
|
||||
codex: { opus: 'high' },
|
||||
});
|
||||
});
|
||||
|
||||
it('is a no-op when provider exists but tier override does not', () => {
|
||||
const result = clearProviderOverride(
|
||||
{
|
||||
codex: { opus: 'high' },
|
||||
},
|
||||
'codex',
|
||||
'haiku'
|
||||
);
|
||||
|
||||
expect(result.changed).toBe(false);
|
||||
expect(result.nextOverrides).toEqual({
|
||||
codex: { opus: 'high' },
|
||||
});
|
||||
});
|
||||
|
||||
it('removes provider entry when last tier is cleared', () => {
|
||||
const result = clearProviderOverride(
|
||||
{
|
||||
codex: { opus: 'high' },
|
||||
},
|
||||
'codex',
|
||||
'opus'
|
||||
);
|
||||
|
||||
expect(result.changed).toBe(true);
|
||||
expect(result.nextOverrides).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,42 @@
|
||||
import { describe, expect, it } from 'bun:test';
|
||||
import { resolveThinkingProviderOverridesForSave } from '../../../src/web-server/routes/misc-routes';
|
||||
|
||||
describe('thinking routes logic', () => {
|
||||
it('clears provider overrides when clear flag is set', () => {
|
||||
const result = resolveThinkingProviderOverridesForSave(
|
||||
{
|
||||
codex: { opus: 'high' },
|
||||
},
|
||||
{
|
||||
gemini: { sonnet: 'medium' },
|
||||
},
|
||||
true
|
||||
);
|
||||
|
||||
expect(result).toBeUndefined();
|
||||
});
|
||||
|
||||
it('applies normalized updates when provided and clear flag is false', () => {
|
||||
const updates = {
|
||||
gemini: { sonnet: 'medium' },
|
||||
};
|
||||
const result = resolveThinkingProviderOverridesForSave(
|
||||
{
|
||||
codex: { opus: 'high' },
|
||||
},
|
||||
updates,
|
||||
false
|
||||
);
|
||||
|
||||
expect(result).toEqual(updates);
|
||||
});
|
||||
|
||||
it('preserves current overrides when no updates are provided', () => {
|
||||
const current = {
|
||||
codex: { opus: 'high' },
|
||||
};
|
||||
const result = resolveThinkingProviderOverridesForSave(current, undefined, false);
|
||||
|
||||
expect(result).toEqual(current);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user