diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 452bdbb9..ba483cb9 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -72,6 +72,7 @@ import { } from '../schemas.js'; import { readCustomModelHosts, resolveCustomModelContextLength } from '../../custom-model-hosts.js'; import { applyCustomModelInjection, removeConfigDir } from '../../custom-model-injection-apply.js'; +import { buildCustomModelInjection } from '../../custom-model-injection.js'; import { getLlamaSwapStatus, triggerLlamaSwapLoad, @@ -1615,7 +1616,24 @@ export function registerSessionRoutes( // selected id too). The argv engine DROPS a token that // fails its pattern rather than quoting it, which would silently launch the CLI on its // own default provider again, so refuse an id the pattern cannot carry up front. + // The check runs on the PURE build result, before applyCustomModelInjection below: + // the forced launch id only comes out of that build, and writing first would mean a + // refused id already replaced the config dir a live codex session uses as CODEX_HOME. const modelSpec = entry.launch.params.model; + const injection = buildCustomModelInjection(entry, endpoint, body.modelId, contextLength); + if (injection.kind === 'unsupported') { + return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `${session.mode} has no known custom-model mechanism`); + } + if ( + injection.launchModel !== undefined && + modelSpec?.type === 'token' && + !matchesPattern(modelSpec.pattern, injection.launchModel) + ) { + return createErrorResponse( + ApiErrorCode.INVALID_INPUT, + `Model id ${JSON.stringify(body.modelId)} cannot be passed to ${session.mode} on its command line` + ); + } const applied = applyCustomModelInjection( entry, endpoint, @@ -1627,17 +1645,6 @@ export function registerSessionRoutes( if (!applied) { return createErrorResponse(ApiErrorCode.OPERATION_FAILED, `${session.mode} has no known custom-model mechanism`); } - if ( - applied.launchModel !== undefined && - modelSpec?.type === 'token' && - !matchesPattern(modelSpec.pattern, applied.launchModel) - ) { - removeConfigDir(applied.configDir); - return createErrorResponse( - ApiErrorCode.INVALID_INPUT, - `Model id ${JSON.stringify(body.modelId)} cannot be passed to ${session.mode} on its command line` - ); - } const { previousConfigDir } = session.setCustomModel( { diff --git a/test/routes/session-custom-model.test.ts b/test/routes/session-custom-model.test.ts index 3a15dfc7..71c6c4a8 100644 --- a/test/routes/session-custom-model.test.ts +++ b/test/routes/session-custom-model.test.ts @@ -9,7 +9,7 @@ import { createRouteTestHarness } from './_route-test-utils.js'; import { createMockSession } from '../mocks/index.js'; import { getDataDir } from '../../src/config/instance.js'; import { writeCustomModelHosts, type CustomModelHost } from '../../src/custom-model-hosts.js'; -import { existsSync, readFileSync, statSync } from 'node:fs'; +import { existsSync, readFileSync, rmSync, statSync } from 'node:fs'; import { join } from 'node:path'; import { webviewFetch } from '../../src/web/webview-egress.js'; @@ -203,6 +203,10 @@ describe('POST /api/sessions/:id/custom-model', () => { const { app, ctx } = await setup(); const session = ctx.sessions.get('test-session-1')!; session.mode = 'pi'; + // Start from no dir at all (a sibling test's successful pi apply may have left one), + // so the existsSync below says something about THIS request. + const dir = join(getDataDir(), 'custom-model-configs', 'test-session-1'); + rmSync(dir, { recursive: true, force: true }); const res = await app.inject({ method: 'POST', @@ -214,8 +218,45 @@ describe('POST /api/sessions/:id/custom-model', () => { expect(res.json().errorCode).toBe('INVALID_INPUT'); expect(session.setCustomModel).not.toHaveBeenCalled(); expect(session.restartCli).not.toHaveBeenCalled(); - // The config dir written before the check is cleaned up again. - expect(existsSync(join(getDataDir(), 'custom-model-configs', 'test-session-1'))).toBe(false); + // The pattern check runs on the pure build BEFORE any write, so a refused id leaves + // no directory behind at all, nothing to clean up. + expect(existsSync(dir)).toBe(false); + }); + + it('codex: a refused forced launch id never touches the live config dir the session runs on as CODEX_HOME', async () => { + const { app, ctx } = await setup(); + const session = ctx.sessions.get('test-session-1')!; + session.mode = 'codex'; + + // First apply succeeds: the session now runs codex on qwen3:32b, whose config.toml + // lives in the per-session dir the running CLI itself uses as CODEX_HOME. + const ok = await app.inject({ + method: 'POST', + url: '/api/sessions/test-session-1/custom-model', + payload: { endpointId: 'ep1', modelId: 'qwen3:32b' }, + }); + expect(ok.statusCode).toBe(200); + const dir = join(getDataDir(), 'custom-model-configs', 'test-session-1'); + const written = join(dir, 'config.toml'); + expect(existsSync(written)).toBe(true); + + // `@` is outside codex's `model-plain-colon` pattern. The pattern check now runs on + // the pure build BEFORE any write, so the refusal must leave the dir — and the file + // the live CLI reads — exactly as they were, rather than replacing and deleting the + // CODEX_HOME of a session that stays on qwen3:32b. + const before = readFileSync(written, 'utf8'); + const res = await app.inject({ + method: 'POST', + url: '/api/sessions/test-session-1/custom-model', + payload: { endpointId: 'ep1', modelId: 'org/model@q4_k_m' }, + }); + + expect(res.json().success).toBe(false); + expect(res.json().errorCode).toBe('INVALID_INPUT'); + expect(session.setCustomModel).toHaveBeenCalledTimes(1); + expect(session.restartCli).toHaveBeenCalledTimes(1); + expect(existsSync(dir)).toBe(true); + expect(readFileSync(written, 'utf8')).toBe(before); }); it('clear removes the previous config dir the session reports', async () => {