From ed0a895d75a5697c72812db3aecdaf9c3d4cea1b Mon Sep 17 00:00:00 2001 From: Haileyesus Date: Tue, 7 Apr 2026 11:56:28 +0300 Subject: [PATCH] refactor: remove session model and thinking mode management from providers and related services --- server/src/modules/llm/llm.routes.ts | 50 --------- .../llm/providers/abstract.provider.ts | 103 ------------------ .../llm/providers/base-cli.provider.ts | 14 +-- .../llm/providers/base-sdk.provider.ts | 14 +-- .../modules/llm/providers/claude.provider.ts | 4 - .../llm/providers/provider.interface.ts | 4 - .../src/modules/llm/services/llm.service.ts | 14 --- .../llm/tests/llm-unifier.providers.test.ts | 44 +++----- 8 files changed, 21 insertions(+), 226 deletions(-) diff --git a/server/src/modules/llm/llm.routes.ts b/server/src/modules/llm/llm.routes.ts index 22ad47a3..27c6fa2c 100644 --- a/server/src/modules/llm/llm.routes.ts +++ b/server/src/modules/llm/llm.routes.ts @@ -305,56 +305,6 @@ router.post( }), ); -router.patch( - '/providers/:provider/sessions/:sessionId/model', - asyncHandler(async (req: Request, res: Response) => { - const provider = parseProvider(req.params.provider); - const sessionId = readPathParam(req.params.sessionId, 'sessionId'); - const model = typeof req.body?.model === 'string' ? req.body.model.trim() : ''; - if (!model) { - throw new AppError('model is required.', { - code: 'MODEL_REQUIRED', - statusCode: 400, - }); - } - - await llmService.setSessionModel(provider, sessionId, model); - res.json( - createApiSuccessResponse({ - provider, - sessionId, - model, - }), - ); - }), -); - -router.patch( - '/providers/:provider/sessions/:sessionId/thinking', - asyncHandler(async (req: Request, res: Response) => { - const provider = parseProvider(req.params.provider); - const sessionId = readPathParam(req.params.sessionId, 'sessionId'); - const thinkingMode = - typeof req.body?.thinkingMode === 'string' ? req.body.thinkingMode.trim() : ''; - - if (!thinkingMode) { - throw new AppError('thinkingMode is required.', { - code: 'THINKING_MODE_REQUIRED', - statusCode: 400, - }); - } - - await llmService.setSessionThinkingMode(provider, sessionId, thinkingMode); - res.json( - createApiSuccessResponse({ - provider, - sessionId, - thinkingMode, - }), - ); - }), -); - /** * Uploads one or more images into `.cloudcli/assets` so providers can reuse file paths. */ diff --git a/server/src/modules/llm/providers/abstract.provider.ts b/server/src/modules/llm/providers/abstract.provider.ts index 4344b237..b8023cc5 100644 --- a/server/src/modules/llm/providers/abstract.provider.ts +++ b/server/src/modules/llm/providers/abstract.provider.ts @@ -1,4 +1,3 @@ -import { AppError } from '@/shared/utils/app-error.js'; import type { IProvider, MutableProviderSession, @@ -11,11 +10,6 @@ import type { } from '@/modules/llm/providers/provider.interface.js'; import type { LLMProvider } from '@/shared/types/app.js'; -type SessionPreference = { - model?: string; - thinkingMode?: string; -}; - const MAX_EVENT_BUFFER_SIZE = 2_000; /** @@ -27,7 +21,6 @@ export abstract class AbstractProvider implements IProvider { readonly capabilities: ProviderCapabilities; protected readonly sessions = new Map(); - protected readonly sessionPreferences = new Map(); protected constructor( id: LLMProvider, @@ -90,98 +83,6 @@ export abstract class AbstractProvider implements IProvider { return stopped; } - /** - * Validates/supports model switching and updates both live and persisted state. - */ - async setSessionModel(sessionId: string, model: string): Promise { - if (!this.capabilities.supportsModelSwitching) { - throw new AppError(`Provider "${this.id}" does not support model switching.`, { - code: 'MODEL_SWITCH_NOT_SUPPORTED', - statusCode: 400, - }); - } - - const trimmedModel = model.trim(); - if (!trimmedModel) { - throw new AppError('Model cannot be empty.', { - code: 'INVALID_MODEL', - statusCode: 400, - }); - } - - const session = this.sessions.get(sessionId); - if (session?.setModel) { - await session.setModel(trimmedModel); - } - - const currentPreference = this.sessionPreferences.get(sessionId) ?? {}; - this.sessionPreferences.set(sessionId, { ...currentPreference, model: trimmedModel }); - - if (session) { - session.model = trimmedModel; - this.appendEvent(session, { - timestamp: new Date().toISOString(), - channel: 'system', - message: `Model updated to "${trimmedModel}".`, - }); - } - } - - /** - * Validates/supports thinking mode updates and applies them to live/persisted state. - */ - async setSessionThinkingMode(sessionId: string, thinkingMode: string): Promise { - if (!this.capabilities.supportsThinkingModeControl) { - throw new AppError(`Provider "${this.id}" does not support thinking mode control.`, { - code: 'THINKING_MODE_NOT_SUPPORTED', - statusCode: 400, - }); - } - - const trimmedMode = thinkingMode.trim(); - if (!trimmedMode) { - throw new AppError('Thinking mode cannot be empty.', { - code: 'INVALID_THINKING_MODE', - statusCode: 400, - }); - } - - const session = this.sessions.get(sessionId); - if (session?.setThinkingMode) { - await session.setThinkingMode(trimmedMode); - } - - const currentPreference = this.sessionPreferences.get(sessionId) ?? {}; - this.sessionPreferences.set(sessionId, { ...currentPreference, thinkingMode: trimmedMode }); - - if (session) { - session.thinkingMode = trimmedMode; - this.appendEvent(session, { - timestamp: new Date().toISOString(), - channel: 'system', - message: `Thinking mode updated to "${trimmedMode}".`, - }); - } - } - - /** - * Reads saved preferences for resumed sessions. - */ - protected getSessionPreference(sessionId: string): SessionPreference { - return this.sessionPreferences.get(sessionId) ?? {}; - } - - /** - * Stores session preferences for subsequent resume/start operations. - */ - protected rememberSessionPreference(sessionId: string, preference: SessionPreference): void { - const currentPreference = this.sessionPreferences.get(sessionId) ?? {}; - this.sessionPreferences.set(sessionId, { - ...currentPreference, - ...preference, - }); - } - /** * Creates mutable internal session state and registers it in memory. */ @@ -206,10 +107,6 @@ export abstract class AbstractProvider implements IProvider { }; this.sessions.set(sessionId, session); - this.rememberSessionPreference(sessionId, { - model: input.model, - thinkingMode: input.thinkingMode, - }); this.appendEvent(session, { timestamp: session.startedAt, diff --git a/server/src/modules/llm/providers/base-cli.provider.ts b/server/src/modules/llm/providers/base-cli.provider.ts index db31ce8a..207cdde5 100644 --- a/server/src/modules/llm/providers/base-cli.provider.ts +++ b/server/src/modules/llm/providers/base-cli.provider.ts @@ -133,20 +133,12 @@ export abstract class BaseCliProvider extends AbstractProvider { * Boots one CLI child process and wires stream handlers to the session buffer. */ private async startSessionInternal(input: CreateCliInvocationInput): Promise { - const preferred = this.getSessionPreference(input.sessionId); - const effectiveModel = input.model ?? preferred.model; - const effectiveThinking = input.thinkingMode ?? preferred.thinkingMode; - const session = this.createSessionRecord(input.sessionId, { - model: effectiveModel, - thinkingMode: effectiveThinking, + model: input.model, + thinkingMode: input.thinkingMode, }); - const invocation = this.createCliInvocation({ - ...input, - model: effectiveModel, - thinkingMode: effectiveThinking, - }); + const invocation = this.createCliInvocation(input); const child = spawn(invocation.command, invocation.args, { cwd: invocation.cwd ?? input.workspacePath ?? process.cwd(), diff --git a/server/src/modules/llm/providers/base-sdk.provider.ts b/server/src/modules/llm/providers/base-sdk.provider.ts index 72948fa3..790a245c 100644 --- a/server/src/modules/llm/providers/base-sdk.provider.ts +++ b/server/src/modules/llm/providers/base-sdk.provider.ts @@ -19,8 +19,6 @@ type CreateSdkExecutionInput = StartSessionInput & { type SdkExecution = { stream: AsyncIterable; stop: () => Promise; - setModel?: (model: string) => Promise; - setThinkingMode?: (thinkingMode: string) => Promise; }; /** @@ -72,21 +70,15 @@ export abstract class BaseSdkProvider extends AbstractProvider { * Initializes one SDK execution and wires it to the internal session record. */ private async startSessionInternal(input: CreateSdkExecutionInput): Promise { - const preferred = this.getSessionPreference(input.sessionId); - const effectiveModel = input.model ?? preferred.model; - const effectiveThinking = input.thinkingMode ?? preferred.thinkingMode; - const session = this.createSessionRecord(input.sessionId, { - model: effectiveModel, - thinkingMode: effectiveThinking, + model: input.model, + thinkingMode: input.thinkingMode, }); let execution: SdkExecution; try { execution = await this.createSdkExecution({ ...input, - model: effectiveModel, - thinkingMode: effectiveThinking, emitEvent: (event) => { this.appendEvent(session, event); }, @@ -103,8 +95,6 @@ export abstract class BaseSdkProvider extends AbstractProvider { } session.stop = execution.stop; - session.setModel = execution.setModel; - session.setThinkingMode = execution.setThinkingMode; session.completion = this.consumeStream(session, execution.stream); return this.toSnapshot(session); diff --git a/server/src/modules/llm/providers/claude.provider.ts b/server/src/modules/llm/providers/claude.provider.ts index 82fc5f13..5aac5816 100644 --- a/server/src/modules/llm/providers/claude.provider.ts +++ b/server/src/modules/llm/providers/claude.provider.ts @@ -104,7 +104,6 @@ export class ClaudeProvider extends BaseSdkProvider { protected async createSdkExecution(input: ClaudeExecutionInput): Promise<{ stream: AsyncIterable; stop: () => Promise; - setModel: (model: string) => Promise; }> { const options: Options = { cwd: input.workspacePath, @@ -131,9 +130,6 @@ export class ClaudeProvider extends BaseSdkProvider { await queryInstance.interrupt(); return true; }, - setModel: async (model: string) => { - await queryInstance.setModel(model); - }, }; } diff --git a/server/src/modules/llm/providers/provider.interface.ts b/server/src/modules/llm/providers/provider.interface.ts index 3db3e3be..5c7e39dd 100644 --- a/server/src/modules/llm/providers/provider.interface.ts +++ b/server/src/modules/llm/providers/provider.interface.ts @@ -84,8 +84,6 @@ export interface IProvider { resumeSession(input: StartSessionInput & { sessionId: string }): Promise; stopSession(sessionId: string): Promise; - setSessionModel(sessionId: string, model: string): Promise; - setSessionThinkingMode(sessionId: string, thinkingMode: string): Promise; getSession(sessionId: string): ProviderSessionSnapshot | null; listSessions(): ProviderSessionSnapshot[]; @@ -98,6 +96,4 @@ export type MutableProviderSession = Omit & { events: ProviderSessionEvent[]; completion: Promise; stop: () => Promise; - setModel?: (model: string) => Promise; - setThinkingMode?: (thinkingMode: string) => Promise; }; diff --git a/server/src/modules/llm/services/llm.service.ts b/server/src/modules/llm/services/llm.service.ts index 3274c3cd..02470e0b 100644 --- a/server/src/modules/llm/services/llm.service.ts +++ b/server/src/modules/llm/services/llm.service.ts @@ -127,20 +127,6 @@ export const llmService = { const provider = llmProviderRegistry.resolveProvider(providerName); return provider.stopSession(sessionId); }, - - async setSessionModel(providerName: string, sessionId: string, model: string): Promise { - const provider = llmProviderRegistry.resolveProvider(providerName); - await provider.setSessionModel(sessionId, model); - }, - - async setSessionThinkingMode( - providerName: string, - sessionId: string, - thinkingMode: string, - ): Promise { - const provider = llmProviderRegistry.resolveProvider(providerName); - await provider.setSessionThinkingMode(sessionId, thinkingMode); - }, }; /** diff --git a/server/src/modules/llm/tests/llm-unifier.providers.test.ts b/server/src/modules/llm/tests/llm-unifier.providers.test.ts index 9e7c16a0..459c0c1d 100644 --- a/server/src/modules/llm/tests/llm-unifier.providers.test.ts +++ b/server/src/modules/llm/tests/llm-unifier.providers.test.ts @@ -220,15 +220,15 @@ test('codex provider reads models_cache.json and maps model metadata', async () } }); -// This test covers persisted session-level model/thinking preferences flowing into Codex thread options. -test('codex provider applies saved model/thinking preferences on subsequent launch', async () => { +// This test covers explicit start/resume payload control for model/thinking without implicit persistence. +test('codex provider does not persist model/thinking between launches', async () => { const provider = new CodexProvider() as any; - let threadOptions: Record | null = null; + const threadOptionsHistory: Record[] = []; provider.loadCodexSdkModule = async () => ({ Codex: class { startThread(options?: Record) { - threadOptions = options ?? null; + threadOptionsHistory.push(options ?? {}); return { async runStreamed() { return { events: asyncEvents([]) }; @@ -246,17 +246,23 @@ test('codex provider applies saved model/thinking preferences on subsequent laun }, }); - await provider.setSessionModel('codex-pref-1', 'gpt-5.4'); - await provider.setSessionThinkingMode('codex-pref-1', 'xhigh'); + await provider.launchSession({ + prompt: 'explicit launch options', + sessionId: 'codex-pref-1', + model: 'gpt-5.4', + thinkingMode: 'xhigh', + }); await provider.launchSession({ - prompt: 'use stored preferences', + prompt: 'follow-up launch without options', sessionId: 'codex-pref-1', }); - assert.ok(threadOptions); - assert.equal((threadOptions as { model?: string }).model, 'gpt-5.4'); - assert.equal((threadOptions as { modelReasoningEffort?: string }).modelReasoningEffort, 'xhigh'); + assert.equal(threadOptionsHistory.length, 2); + assert.equal((threadOptionsHistory[0] as { model?: string }).model, 'gpt-5.4'); + assert.equal((threadOptionsHistory[0] as { modelReasoningEffort?: string }).modelReasoningEffort, 'xhigh'); + assert.equal((threadOptionsHistory[1] as { model?: string }).model, undefined); + assert.equal((threadOptionsHistory[1] as { modelReasoningEffort?: string }).modelReasoningEffort, undefined); }); // This test covers Claude thinking-level mapping, runtime permission handlers, and model/event normalization. @@ -318,21 +324,3 @@ test('llmService rejects unsupported runtime permission and thinking mode combin error.statusCode === 400, ); }); - -// This test covers model/thinking capability gates on providers before any external process/SDK usage. -test('providers enforce capability gates for model/thinking updates', async () => { - const claudeProvider = new ClaudeProvider(); - const cursorProvider = new CursorProvider(); - - await assert.rejects( - cursorProvider.setSessionThinkingMode('cursor-session', 'high'), - (error: unknown) => - error instanceof AppError && - error.code === 'THINKING_MODE_NOT_SUPPORTED' && - error.statusCode === 400, - ); - - await claudeProvider.setSessionModel('claude-session', 'sonnet'); - const preference = (claudeProvider as any).getSessionPreference('claude-session'); - assert.equal(preference.model, 'sonnet'); -});