Skip to content

Commit b793a8f

Browse files
authored
[codex] hide Ollama until manually added (#170)
## Summary Fixes #159 by making Ollama opt-in in Settings. `Ollama (local)` no longer appears as a default provider row on fresh configs; users can add it explicitly from the Add provider menu, and the keyless provider is persisted without writing an empty secret. ## Type of change - [x] Bug fix - [ ] New feature - [ ] Refactor (no behavior change) - [ ] Documentation - [ ] Build / CI / tooling - [ ] Breaking change ## Linked issue Closes #159 ## Root cause `toProviderRows()` always injected keyless builtin providers into the rendered row set. Because Ollama is the only builtin with `requiresApiKey: false`, Settings showed it even when the user had never added or configured it. ## What changed - Removed automatic keyless builtin injection from provider row generation. - Added regression coverage for hidden-until-persisted and visible-after-persisted Ollama behavior. - Allowed the Settings add-provider IPC path to persist keyless Ollama without creating an empty secret. - Added an explicit Ollama item to the Add provider menu, disabled after it is already present. - Added English and Chinese labels for the new menu entry and toast. - Added a changeset for the user-visible Settings behavior change. ## Checklist - [x] I read [`CLAUDE.md`](../CLAUDE.md) before starting - [ ] I read [`docs/VISION.md`](../docs/VISION.md) and [`docs/PRINCIPLES.md`](../docs/PRINCIPLES.md) before starting (not present in this local checkout; `docs/` only contains `docs/research/`) - [x] Commits are signed with DCO (`git commit -s`) - [x] Added/updated tests for the change - [x] Added a changeset (`pnpm changeset`) because behavior changed - [x] Updated docs if behavior changed (not needed; behavior is covered by Settings UI copy) ## PRINCIPLES §5b checks - [x] Compatibility: Existing persisted provider/secret rows still render; keyless Ollama remains allowed after explicit add. - [x] Upgradeability: No schema changes; configs keep the same v3 shape. - [x] No bloat: No dependency additions; only targeted IPC/UI/tests/i18n changes. - [x] Elegance: The row list now reflects persisted state, while the add menu owns discovery. ## Validation - `corepack pnpm --filter @open-codesign/desktop exec vitest run src/main/provider-settings.test.ts src/main/onboarding-ipc.test.ts src/renderer/src/components/Settings.test.ts` — 57 passed - `corepack pnpm --filter @open-codesign/desktop typecheck` — passed - `corepack pnpm lint` — passed - `git diff --check` — passed I also attempted the full desktop test suite on Windows. It is currently blocked by environment/platform issues unrelated to this change: missing `better-sqlite3` native binding after dependency install, POSIX `/tmp` path expectations on Windows, and symlink permission failures. ## Screenshots / recordings (UI changes) Not attached; this is a small Settings menu behavior change. Signed-off-by: HUANG <15866338256@163.com>
1 parent 7e75226 commit b793a8f

8 files changed

Lines changed: 132 additions & 19 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@open-codesign/desktop": patch
3+
"@open-codesign/i18n": patch
4+
---
5+
6+
Fix Settings so Ollama is hidden until users add the local provider manually.

apps/desktop/src/main/onboarding-ipc.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,36 @@ describe('registerOnboardingIpc — channel versioning', () => {
165165
}
166166
});
167167

168+
it('lets settings add the keyless Ollama builtin without storing an empty secret', async () => {
169+
const { readConfig, writeConfig } = await import('./config');
170+
vi.mocked(readConfig).mockResolvedValueOnce(null);
171+
vi.mocked(writeConfig).mockClear();
172+
const { loadConfigOnBoot, registerOnboardingIpc } = await import('./onboarding-ipc');
173+
await loadConfigOnBoot();
174+
registerOnboardingIpc();
175+
176+
const handler = handlers.get('settings:v1:add-provider');
177+
expect(handler).toBeDefined();
178+
179+
const rows = (await handler?.(
180+
{},
181+
{
182+
provider: 'ollama',
183+
apiKey: '',
184+
modelPrimary: 'llama3.2',
185+
},
186+
)) as Array<{ provider: string }>;
187+
188+
const written = vi.mocked(writeConfig).mock.calls.at(-1)?.[0];
189+
expect(written?.providers['ollama']).toMatchObject({
190+
id: 'ollama',
191+
name: 'Ollama (local)',
192+
requiresApiKey: false,
193+
});
194+
expect(written?.secrets['ollama']).toBeUndefined();
195+
expect(rows.some((row) => row.provider === 'ollama')).toBe(true);
196+
});
197+
168198
it('registers the canonical config:v1:set-provider-and-models handler', async () => {
169199
const { registerOnboardingIpc } = await import('./onboarding-ipc');
170200
registerOnboardingIpc();

apps/desktop/src/main/onboarding-ipc.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -247,13 +247,17 @@ function parseSaveKey(raw: unknown): SaveKeyInput {
247247
ERROR_CODES.IPC_BAD_INPUT,
248248
);
249249
}
250-
if (typeof apiKey !== 'string' || apiKey.trim().length === 0) {
250+
const providerId = provider.trim();
251+
const isKeylessBuiltin =
252+
isSupportedOnboardingProvider(providerId) &&
253+
BUILTIN_PROVIDERS[providerId].requiresApiKey === false;
254+
if (typeof apiKey !== 'string' || (apiKey.trim().length === 0 && !isKeylessBuiltin)) {
251255
throw new CodesignError('apiKey must be a non-empty string', ERROR_CODES.IPC_BAD_INPUT);
252256
}
253257
if (typeof modelPrimary !== 'string' || modelPrimary.trim().length === 0) {
254258
throw new CodesignError('modelPrimary must be a non-empty string', ERROR_CODES.IPC_BAD_INPUT);
255259
}
256-
const out: SaveKeyInput = { provider, apiKey, modelPrimary };
260+
const out: SaveKeyInput = { provider: providerId, apiKey: apiKey.trim(), modelPrimary };
257261
if (typeof baseUrl === 'string' && baseUrl.trim().length > 0) {
258262
try {
259263
new URL(baseUrl);
@@ -336,7 +340,6 @@ function parseSetProviderAndModels(raw: unknown): SetProviderAndModelsInput {
336340
* after Settings mutations.
337341
*/
338342
async function runSetProviderAndModels(input: SetProviderAndModelsInput): Promise<OnboardingState> {
339-
const secretRef = buildSecretRef(input.apiKey);
340343
const nextProviders: Record<string, ProviderEntry> = { ...(cachedConfig?.providers ?? {}) };
341344
const existing = nextProviders[input.provider];
342345
const builtin = BUILTIN_PROVIDERS[input.provider as SupportedOnboardingProvider];
@@ -354,10 +357,12 @@ async function runSetProviderAndModels(input: SetProviderAndModelsInput): Promis
354357
baseUrl: input.baseUrl ?? seed.baseUrl,
355358
defaultModel: input.modelPrimary || seed.defaultModel,
356359
};
357-
const nextSecrets = {
358-
...(cachedConfig?.secrets ?? {}),
359-
[input.provider]: secretRef,
360-
};
360+
const nextSecrets = { ...(cachedConfig?.secrets ?? {}) };
361+
if (input.apiKey.length > 0) {
362+
nextSecrets[input.provider] = buildSecretRef(input.apiKey);
363+
} else {
364+
delete nextSecrets[input.provider];
365+
}
361366
const activate = input.setAsActive || cachedConfig === null;
362367
const nextActiveProvider = activate
363368
? input.provider

apps/desktop/src/main/provider-settings.test.ts

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
import { CodesignError, type Config, hydrateConfig } from '@open-codesign/shared';
1+
import {
2+
BUILTIN_PROVIDERS,
3+
CodesignError,
4+
type Config,
5+
hydrateConfig,
6+
} from '@open-codesign/shared';
27
import { describe, expect, it } from 'vitest';
38
import {
49
assertProviderHasStoredSecret,
@@ -117,6 +122,39 @@ describe('toProviderRows', () => {
117122
expect(anthropicRow?.hasKey).toBe(false);
118123
expect(anthropicRow?.maskedKey).toBe('');
119124
});
125+
126+
it('does not surface Ollama until the user has persisted it', () => {
127+
const cfg = makeCfg({
128+
provider: 'openai',
129+
modelPrimary: 'gpt-4o',
130+
secrets: { openai: { ciphertext: 'enc' } },
131+
});
132+
133+
const rows = toProviderRows(cfg, () => 'sk-test-token-1234567890');
134+
135+
expect(rows.some((row) => row.provider === 'ollama')).toBe(false);
136+
});
137+
138+
it('surfaces Ollama after it has been persisted', () => {
139+
const cfg = makeCfg({
140+
provider: 'openai',
141+
modelPrimary: 'gpt-4o',
142+
secrets: { openai: { ciphertext: 'enc' } },
143+
providers: {
144+
ollama: { ...BUILTIN_PROVIDERS.ollama },
145+
},
146+
});
147+
148+
const rows = toProviderRows(cfg, () => 'sk-test-token-1234567890');
149+
const ollamaRow = rows.find((row) => row.provider === 'ollama');
150+
151+
expect(ollamaRow).toMatchObject({
152+
provider: 'ollama',
153+
label: 'Ollama (local)',
154+
hasKey: true,
155+
maskedKey: '',
156+
});
157+
});
120158
});
121159

122160
describe('assertProviderHasStoredSecret', () => {

apps/desktop/src/main/provider-settings.ts

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import {
88
PROVIDER_SHORTLIST,
99
type ProviderEntry,
1010
type ReasoningLevel,
11-
SUPPORTED_ONBOARDING_PROVIDERS,
1211
type WireApi,
1312
isSupportedOnboardingProvider,
1413
} from '@open-codesign/shared';
@@ -101,16 +100,6 @@ export function toProviderRows(
101100
...Object.keys(cfg.providers ?? {}),
102101
...Object.keys(cfg.secrets ?? {}),
103102
]);
104-
// Keyless builtins (e.g. Ollama) always surface as rows so users can
105-
// discover + enable them without going through onboarding first. Without
106-
// this, a fresh v3 install would hide Ollama entirely — the providers
107-
// map gets populated lazily during onboarding, but Ollama has no
108-
// onboarding step to run.
109-
for (const builtinId of SUPPORTED_ONBOARDING_PROVIDERS) {
110-
if (BUILTIN_PROVIDERS[builtinId].requiresApiKey === false) {
111-
allIds.add(builtinId);
112-
}
113-
}
114103
for (const provider of allIds) {
115104
const ref = cfg.secrets?.[provider];
116105
const entry = resolveEntryFor(cfg, provider);

apps/desktop/src/renderer/src/components/Settings.tsx

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,6 +1015,27 @@ function ModelsTab() {
10151015
}
10161016
}
10171017

1018+
async function handleAddOllama() {
1019+
if (!window.codesign) return;
1020+
try {
1021+
await window.codesign.settings.addProvider({
1022+
provider: 'ollama',
1023+
apiKey: '',
1024+
modelPrimary: SHORTLIST.ollama.defaultPrimary,
1025+
});
1026+
await reloadRows();
1027+
pushToast({ variant: 'success', title: t('settings.providers.import.ollamaDone') });
1028+
} catch (err) {
1029+
reportableErrorToast({
1030+
code: 'OLLAMA_ADD_FAILED',
1031+
scope: 'settings',
1032+
title: t('settings.providers.toast.saveFailed'),
1033+
description: err instanceof Error ? err.message : t('settings.common.unknownError'),
1034+
...(err instanceof Error && err.stack !== undefined ? { stack: err.stack } : {}),
1035+
});
1036+
}
1037+
}
1038+
10181039
async function handleImportGemini() {
10191040
if (!window.codesign) return;
10201041
// Capture pre-import warnings (e.g. "AIzaSy pattern mismatch") so a
@@ -1512,6 +1533,7 @@ function ModelsTab() {
15121533
open={showAddMenu}
15131534
setOpen={setShowAddMenu}
15141535
hasClaudeCodeImported={rows.some((r) => r.provider === 'claude-code-imported')}
1536+
hasOllamaImported={rows.some((r) => r.provider === 'ollama')}
15151537
onImportCodex={() => {
15161538
setShowAddMenu(false);
15171539
void handleImportCodex();
@@ -1520,6 +1542,10 @@ function ModelsTab() {
15201542
setShowAddMenu(false);
15211543
void handleImportClaudeCode();
15221544
}}
1545+
onAddOllama={() => {
1546+
setShowAddMenu(false);
1547+
void handleAddOllama();
1548+
}}
15231549
onAddCustom={() => {
15241550
setShowAddMenu(false);
15251551
setShowAddCustom(true);
@@ -2137,8 +2163,10 @@ interface AddProviderMenuProps {
21372163
open: boolean;
21382164
setOpen: (v: boolean) => void;
21392165
hasClaudeCodeImported: boolean;
2166+
hasOllamaImported: boolean;
21402167
onImportCodex: () => void;
21412168
onImportClaudeCode: () => void;
2169+
onAddOllama: () => void;
21422170
onAddCustom: () => void;
21432171
onAddCliProxyApi: () => void;
21442172
}
@@ -2147,8 +2175,10 @@ function AddProviderMenu({
21472175
open,
21482176
setOpen,
21492177
hasClaudeCodeImported,
2178+
hasOllamaImported,
21502179
onImportCodex,
21512180
onImportClaudeCode,
2181+
onAddOllama,
21522182
onAddCustom,
21532183
onAddCliProxyApi,
21542184
}: AddProviderMenuProps) {
@@ -2198,6 +2228,13 @@ function AddProviderMenu({
21982228
disabled: hasClaudeCodeImported,
21992229
onClick: onImportClaudeCode,
22002230
},
2231+
{
2232+
key: 'ollama',
2233+
label: t('settings.providers.import.ollamaMenu'),
2234+
desc: t('settings.providers.import.ollamaMenuDesc'),
2235+
disabled: hasOllamaImported,
2236+
onClick: onAddOllama,
2237+
},
22012238
{
22022239
key: 'custom',
22032240
label: t('settings.providers.import.customMenu', { defaultValue: '自定义服务' }),

packages/i18n/src/locales/en.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,9 @@
264264
"codexMenuDesc": "Read ~/.codex/config.toml",
265265
"claudeCodeMenu": "Import from Claude Code",
266266
"claudeCodeMenuDesc": "Read the signed-in Claude Code session",
267+
"ollamaMenu": "Ollama (local)",
268+
"ollamaMenuDesc": "Add the local localhost:11434 provider",
269+
"ollamaDone": "Ollama provider added",
267270
"customMenu": "Custom provider",
268271
"customMenuDesc": "Enter API key and URL manually",
269272
"alreadyImported": "Already imported"
@@ -293,6 +296,7 @@
293296
"switchedTo": "Switched to {{label}}",
294297
"switchFailed": "Switch failed",
295298
"saved": "Provider saved",
299+
"saveFailed": "Failed to save provider",
296300
"modelSaveFailed": "Failed to save model selection",
297301
"reasoningSaved": "Reasoning depth saved",
298302
"reasoningSaveFailed": "Failed to save reasoning depth",

packages/i18n/src/locales/zh-CN.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,9 @@
264264
"codexMenuDesc": "读取 ~/.codex/config.toml",
265265
"claudeCodeMenu": "从 Claude Code 导入",
266266
"claudeCodeMenuDesc": "读取已登录的 Claude Code 会话",
267+
"ollamaMenu": "Ollama(本地)",
268+
"ollamaMenuDesc": "添加 localhost:11434 本地模型服务",
269+
"ollamaDone": "已添加 Ollama 服务",
267270
"customMenu": "自定义服务",
268271
"customMenuDesc": "手动填写 API key 和 URL",
269272
"alreadyImported": "已导入"
@@ -293,6 +296,7 @@
293296
"switchedTo": "已切换到 {{label}}",
294297
"switchFailed": "切换失败",
295298
"saved": "服务已保存",
299+
"saveFailed": "保存服务失败",
296300
"modelSaveFailed": "保存模型选择失败",
297301
"reasoningSaved": "已保存推理深度",
298302
"reasoningSaveFailed": "保存推理深度失败",

0 commit comments

Comments
 (0)