Skip to content

Revert "Fix: Persist manual model selection on restart #19864 (#19891)" #19999

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

Open
Nixxx19 wants to merge 4 commits into
base: main
Choose a base branch
from
+10 −102
Open

Revert "Fix: Persist manual model selection on restart #19864 (#19891)" #19999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
faef784
Revert "Fix: Persist manual model selection on restart #19864 (#19891)"
Nixxx19 Feb 23, 2026
c13fd41
Merge branch 'main' into revert-19891-nityam/fix-19864-model-persistence
Nixxx19 Feb 23, 2026
6e9d68e
Merge branch 'main' into revert-19891-nityam/fix-19864-model-persistence
Nixxx19 Feb 23, 2026
c5e210b
Merge branch 'main' into revert-19891-nityam/fix-19864-model-persistence
Nixxx19 Feb 23, 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
100 changes: 5 additions & 95 deletions packages/core/src/config/config.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ import {
DEFAULT_OTLP_ENDPOINT,
uiTelemetryService,
} from '../telemetry/index.js';
import type {
ContentGeneratorConfig,
ContentGenerator,
} from '../core/contentGenerator.js';
import type { ContentGeneratorConfig } from '../core/contentGenerator.js';
import {
AuthType,
createContentGenerator,
Expand All @@ -47,11 +44,7 @@ import { ACTIVATE_SKILL_TOOL_NAME } from '../tools/tool-names.js';
import type { SkillDefinition } from '../skills/skillLoader.js';
import type { McpClientManager } from '../tools/mcp-client-manager.js';
import { DEFAULT_MODEL_CONFIGS } from './defaultModelConfigs.js';
import {
DEFAULT_GEMINI_MODEL,
PREVIEW_GEMINI_3_1_MODEL,
DEFAULT_GEMINI_MODEL_AUTO,
} from './models.js';
import { DEFAULT_GEMINI_MODEL } from './models.js';
import { Storage } from './storage.js';

vi.mock('fs', async (importOriginal) => {
Expand Down Expand Up @@ -2309,8 +2302,7 @@ describe('Config Quota & Preview Model Access', () => {
vi.mocked(getCodeAssistServer).mockReturnValue(undefined);
const result = await config.refreshUserQuota();
expect(result).toBeUndefined();
// Never set => stays null (unknown); getter returns true so UI shows preview
expect(config.getHasAccessToPreviewModel()).toBe(true);
expect(config.getHasAccessToPreviewModel()).toBe(false);
});

it('should return undefined if retrieveUserQuota fails', async () => {
Expand All @@ -2319,8 +2311,8 @@ describe('Config Quota & Preview Model Access', () => {
);
const result = await config.refreshUserQuota();
expect(result).toBeUndefined();
// Never set => stays null (unknown); getter returns true so UI shows preview
expect(config.getHasAccessToPreviewModel()).toBe(true);
// Should remain default (false)
expect(config.getHasAccessToPreviewModel()).toBe(false);
});
});

Expand Down Expand Up @@ -2807,85 +2799,3 @@ describe('syncPlanModeTools', () => {
expect(setToolsSpy).toHaveBeenCalled();
});
});

describe('Model Persistence Bug Fix (#19864)', () => {
const baseParams: ConfigParameters = {
sessionId: 'test-session',
cwd: '/tmp',
targetDir: '/path/to/target',
debugMode: false,
model: PREVIEW_GEMINI_3_1_MODEL, // User saved preview model
};

it('should NOT reset preview model for CodeAssist auth when refreshUserQuota is not called (no projectId)', async () => {
const mockContentConfig = {
authType: AuthType.LOGIN_WITH_GOOGLE,
} as Partial<ContentGeneratorConfig> as ContentGeneratorConfig;

const mockContentGenerator = {
generateContent: vi.fn(),
} as Partial<ContentGenerator> as ContentGenerator;

vi.mocked(createContentGeneratorConfig).mockResolvedValue(
mockContentConfig,
);
vi.mocked(createContentGenerator).mockResolvedValue(mockContentGenerator);
// getCodeAssistServer returns undefined by default, so refreshUserQuota() isn't called;
// hasAccessToPreviewModel stays null; reset only when === false, so we don't reset.
const config = new Config(baseParams);

// Verify initial model is the preview model
expect(config.getModel()).toBe(PREVIEW_GEMINI_3_1_MODEL);

// Call refreshAuth to simulate restart (CodeAssist auth, no projectId)
await config.refreshAuth(AuthType.LOGIN_WITH_GOOGLE);

// Verify the model was NOT reset (bug fix)
expect(config.getModel()).toBe(PREVIEW_GEMINI_3_1_MODEL);
expect(config.getModel()).not.toBe(DEFAULT_GEMINI_MODEL_AUTO);
});

it('should NOT reset preview model for USE_GEMINI (hasAccessToPreviewModel is set to true)', async () => {
const mockContentConfig = {
authType: AuthType.USE_GEMINI,
} as Partial<ContentGeneratorConfig> as ContentGeneratorConfig;

const mockContentGenerator = {
generateContent: vi.fn(),
} as Partial<ContentGenerator> as ContentGenerator;

vi.mocked(createContentGeneratorConfig).mockResolvedValue(
mockContentConfig,
);
vi.mocked(createContentGenerator).mockResolvedValue(mockContentGenerator);

const config = new Config(baseParams);

// Verify initial model is the preview model
expect(config.getModel()).toBe(PREVIEW_GEMINI_3_1_MODEL);

// Call refreshAuth
await config.refreshAuth(AuthType.USE_GEMINI);

// For USE_GEMINI, hasAccessToPreviewModel should be set to true
// So the model should NOT be reset
expect(config.getModel()).toBe(PREVIEW_GEMINI_3_1_MODEL);
expect(config.getHasAccessToPreviewModel()).toBe(true);
});

it('should persist model when user selects it with persistMode=true', () => {
const onModelChange = vi.fn();
const config = new Config({
...baseParams,
model: DEFAULT_GEMINI_MODEL_AUTO, // Initial model
onModelChange,
});

// User selects preview model with persist mode enabled
config.setModel(PREVIEW_GEMINI_3_1_MODEL, false); // isTemporary = false

// Verify onModelChange was called to persist the model
expect(onModelChange).toHaveBeenCalledWith(PREVIEW_GEMINI_3_1_MODEL);
expect(config.getModel()).toBe(PREVIEW_GEMINI_3_1_MODEL);
});
});
12 changes: 5 additions & 7 deletions packages/core/src/config/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,8 +579,7 @@ export class Config {
private readonly bugCommand: BugCommandSettings | undefined;
private model: string;
private readonly disableLoopDetection: boolean;
// null = unknown (quota not fetched); true = has access; false = definitively no access
private hasAccessToPreviewModel: boolean | null = null;
private hasAccessToPreviewModel: boolean = false;
private readonly noBrowser: boolean;
private readonly folderTrust: boolean;
private ideMode: boolean;
Expand Down Expand Up @@ -1117,9 +1116,8 @@ export class Config {
this.setHasAccessToPreviewModel(true);
}

// Only reset when we have explicit "no access" (hasAccessToPreviewModel === false).
// When null (quota not fetched) or true, we preserve the saved model.
if (isPreviewModel(this.model) && this.hasAccessToPreviewModel === false) {
// Update model if user no longer has access to the preview model
if (!this.hasAccessToPreviewModel && isPreviewModel(this.model)) {
this.setModel(DEFAULT_GEMINI_MODEL_AUTO);
}

Expand Down Expand Up @@ -1456,10 +1454,10 @@ export class Config {
}

getHasAccessToPreviewModel(): boolean {
return this.hasAccessToPreviewModel !== false;
return this.hasAccessToPreviewModel;
}

setHasAccessToPreviewModel(hasAccess: boolean | null): void {
setHasAccessToPreviewModel(hasAccess: boolean): void {
this.hasAccessToPreviewModel = hasAccess;
}

Expand Down