Skip to content

Commit c9d45e0

Browse files
committed
AI cleanup: strip the stranded plaintext key from settings.json and delete dead opt-out commands
Two follow-ups to the AI-search fixes, both around the pre-`ai.cloudProvider` schema refactor: **Plaintext key no longer lingers in `settings.json`.** The old schema left three flat keys behind — `ai.openaiApiKey` (a PLAINTEXT key), `ai.openaiBaseUrl`, `ai.openaiModel`. Nothing reads them (the key lives in the OS secret store, model/baseUrl in `ai.cloudProviderConfigs`), and the registry-driven save loop only manages registered ids, so they sat in `settings.json` forever — a plaintext key in Time Machine and cloud backups, exactly what the secret-store move was meant to prevent. `ai-config.ts::migrateLegacyOpenAiKeys` now lifts a stranded `ai.openaiApiKey` into the secret store if that's its only copy (so a tester who never re-entered it doesn't lose it AND gets working AI back), then deletes all three on startup. Split `migrateApiKeysFromSettings` into two independent steps so the flat-key cleanup still runs when `ai.cloudProviderConfigs` is missing/malformed (it used to early-return there). New raw-store helpers `getRawStoreValue` / `deleteRawStoreKeys` reach the non-registry keys. **Deleted dead opt-out machinery.** `opt_in_ai`, `is_ai_opted_out`, and the `AiState.opted_out` field had no readers after the onboarding revamp (`ai.provider` is the source of truth). Removed the commands, their IPC registration + specta collection, the TS wrappers, and the field. Kept `uninstall_ai` — it's still wired to the Uninstall button in `AiLocalSection.svelte`, so the old "orphan" doc note was wrong on that one. Fixed the stale `lib/ai/CLAUDE.md` claim that `ai.openai*` are registry settings; regenerated bindings. knip didn't catch the dead wrappers because `tauri-commands/**` is in its ignore list (IPC boundary).
1 parent c7b3039 commit c9d45e0

13 files changed

Lines changed: 133 additions & 65 deletions

File tree

apps/desktop/src-tauri/src/ai/CLAUDE.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ Three provider modes:
3232

3333
Core: `get_ai_status`, `get_ai_model_info`, `get_ai_runtime_status`, `configure_ai`, `start_ai_server`, `stop_ai_server`, `check_ai_connection`, `start_ai_download`, `cancel_ai_download`, `get_folder_suggestions`, `stream_folder_suggestions`, `cancel_folder_suggestions`. Note: `get_system_memory_info` moved to top-level `system_memory.rs`.
3434
API keys: `save_ai_api_key`, `get_ai_api_key`, `delete_ai_api_key`, `has_ai_api_key` (in `api_keys.rs`).
35-
Orphan (no frontend callers after the onboarding revamp; flagged for follow-up cleanup): `uninstall_ai`, `opt_in_ai`, `is_ai_opted_out`. The post-FDA AI offer toast is gone, so `dismiss_ai_offer` and `opt_out_ai` were deleted with it.
35+
Also: `uninstall_ai` (the Uninstall button in `AiLocalSection.svelte`). The dead opt-out machinery (`opt_in_ai`, `is_ai_opted_out`, `dismiss_ai_offer`, `opt_out_ai`, and the `AiState.opted_out` field) was removed with the onboarding revamp — `ai.provider` is the single source of truth for whether AI is on.
3636

3737
## Startup flow
3838

@@ -83,7 +83,6 @@ The frontend (`AiSection.svelte`) tracks `installStep` state and displays "Step
8383
- Binary re-extraction is possible if model exists but binary is missing.
8484
- Download guard: `download_in_progress` flag prevents concurrent downloads.
8585
- Server logs written to `llama-server.log` in the AI dir for debugging.
86-
- `opted_out` field in `AiState` is legacy. `ai.provider` in frontend settings store is the source of truth.
8786
- OpenAI config (api_key, base_url, model) stored in `ManagerState` so suggestions.rs can read without settings files. The api_key originates from the OS secret store (`api_keys.rs`), pushed in via `configure_ai`.
8887
- `configure_ai` is idempotent -- frontend calls it on startup and whenever any AI setting changes.
8988
- `ModelInfo` includes `kv_bytes_per_token` and `base_overhead_bytes` for frontend memory estimation.

apps/desktop/src-tauri/src/ai/manager.rs

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -350,25 +350,6 @@ fn uninstall_ai_sync() {
350350
}
351351
}
352352

353-
/// Re-enables AI features after opting out.
354-
#[tauri::command]
355-
#[specta::specta]
356-
pub fn opt_in_ai() {
357-
let mut manager = MANAGER.lock_ignore_poison();
358-
if let Some(ref mut m) = *manager {
359-
m.state.opted_out = false;
360-
save_state(&m.ai_dir, &m.state);
361-
}
362-
}
363-
364-
/// Returns whether the user has opted out of AI features.
365-
#[tauri::command]
366-
#[specta::specta]
367-
pub fn is_ai_opted_out() -> bool {
368-
let manager = MANAGER.lock_ignore_poison();
369-
manager.as_ref().is_some_and(|m| m.state.opted_out)
370-
}
371-
372353
/// Model info returned to frontend.
373354
#[derive(Debug, Clone, serde::Serialize, specta::Type)]
374355
#[serde(rename_all = "camelCase")]
@@ -1195,7 +1176,6 @@ mod tests {
11951176
assert_eq!(state.pid, None);
11961177
assert_eq!(state.installed_model_id, "ministral-3b-instruct-q4km");
11971178
assert_eq!(state.dismissed_until, None);
1198-
assert!(!state.opted_out);
11991179
}
12001180

12011181
#[test]
@@ -1206,7 +1186,6 @@ mod tests {
12061186
pid: Some(12345),
12071187
installed_model_id: String::from("ministral-3b-instruct-q4km"),
12081188
dismissed_until: None,
1209-
opted_out: false,
12101189
model_download_complete: true,
12111190
partial_download_started: None,
12121191
};
@@ -1219,18 +1198,6 @@ mod tests {
12191198
assert!(parsed.model_download_complete);
12201199
}
12211200

1222-
#[test]
1223-
fn test_state_with_opted_out() {
1224-
let state = AiState {
1225-
opted_out: true,
1226-
..Default::default()
1227-
};
1228-
1229-
let json = serde_json::to_string(&state).unwrap();
1230-
let parsed: AiState = serde_json::from_str(&json).unwrap();
1231-
assert!(parsed.opted_out);
1232-
}
1233-
12341201
#[test]
12351202
fn test_get_ai_status_no_manager() {
12361203
// When manager is not initialized, status is Unavailable

apps/desktop/src-tauri/src/ai/mod.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,6 @@ pub struct AiState {
172172
/// Unix timestamp (seconds).
173173
#[serde(default)]
174174
pub dismissed_until: Option<u64>,
175-
#[serde(default)]
176-
pub opted_out: bool,
177175
/// Verified by file size.
178176
#[serde(default)]
179177
pub model_download_complete: bool,
@@ -194,7 +192,6 @@ impl Default for AiState {
194192
pid: None,
195193
installed_model_id: default_model_id(),
196194
dismissed_until: None,
197-
opted_out: false,
198195
model_download_complete: false,
199196
partial_download_started: None,
200197
}

apps/desktop/src-tauri/src/ipc.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -516,8 +516,6 @@ pub fn builder() -> Builder<tauri::Wry> {
516516
crate::ai::manager::start_ai_download,
517517
crate::ai::manager::cancel_ai_download,
518518
crate::ai::manager::uninstall_ai,
519-
crate::ai::manager::opt_in_ai,
520-
crate::ai::manager::is_ai_opted_out,
521519
crate::ai::api_keys::save_ai_api_key,
522520
crate::ai::api_keys::get_ai_api_key,
523521
crate::ai::api_keys::delete_ai_api_key,

apps/desktop/src-tauri/src/ipc_collectors.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,6 @@ pub(crate) fn collect_cross_platform_types(types: &mut Types) -> Vec<Function> {
155155
crate::system_strings::get_localized_system_strings,
156156
crate::ai::manager::cancel_ai_download,
157157
crate::ai::manager::uninstall_ai,
158-
crate::ai::manager::opt_in_ai,
159-
crate::ai::manager::is_ai_opted_out,
160158
crate::ai::api_keys::save_ai_api_key,
161159
crate::ai::api_keys::get_ai_api_key,
162160
crate::ai::api_keys::delete_ai_api_key,

apps/desktop/src/lib/ai/CLAUDE.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,11 @@ Intel Macs with an explanatory tooltip (controlled by `AiRuntimeStatus.localAiSu
3333

3434
### AI settings in registry
3535

36-
Settings `ai.provider`, `ai.openaiApiKey`, `ai.openaiBaseUrl`, `ai.openaiModel`, `ai.localContextSize` are defined in
37-
`settings-registry.ts`. The main layout calls `configureAi(...)` after `initSettingsApplier()` to push config to
38-
backend.
36+
Settings `ai.provider`, `ai.cloudProvider`, `ai.cloudProviderConfigs`, `ai.localContextSize` are defined in
37+
`settings-registry.ts`. The main layout calls `configureAi(...)` after `initSettingsApplier()` to push config to backend
38+
(the API key is fetched separately from the OS secret store). The pre-refactor flat keys (`ai.openaiApiKey`,
39+
`ai.openaiBaseUrl`, `ai.openaiModel`) are gone from the registry; `ai-config.ts::migrateLegacyOpenAiKeys` lifts any
40+
stranded plaintext `ai.openaiApiKey` into the secret store and deletes all three on startup.
3941

4042
### The wizard owns AI consent
4143

apps/desktop/src/lib/ipc/bindings.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,10 +1165,6 @@ export const commands = {
11651165
* Async because file deletion may block briefly.
11661166
*/
11671167
uninstallAi: () => __TAURI_INVOKE<void>('uninstall_ai'),
1168-
// Re-enables AI features after opting out.
1169-
optInAi: () => __TAURI_INVOKE<void>('opt_in_ai'),
1170-
// Returns whether the user has opted out of AI features.
1171-
isAiOptedOut: () => __TAURI_INVOKE<boolean>('is_ai_opted_out'),
11721168
saveAiApiKey: (providerId: string, apiKey: string) =>
11731169
typedError<null, AiApiKeyError>(__TAURI_INVOKE('save_ai_api_key', { providerId, apiKey })),
11741170
/**

apps/desktop/src/lib/settings/ai-config.test.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,18 +15,23 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'
1515

1616
const saveAiApiKey = vi.fn<(id: string, key: string) => Promise<null>>(() => Promise.resolve(null))
1717
const getAiApiKey = vi.fn<(id: string) => Promise<string>>(() => Promise.resolve(''))
18+
const hasAiApiKey = vi.fn<(id: string) => Promise<boolean>>(() => Promise.resolve(false))
1819
const configureAi = vi.fn<
1920
(provider: string, contextSize: number, apiKey: string, baseUrl: string, model: string) => Promise<null>
2021
>(() => Promise.resolve(null))
2122

2223
vi.mock('$lib/tauri-commands', () => ({
2324
saveAiApiKey: (id: string, key: string) => saveAiApiKey(id, key),
2425
getAiApiKey: (id: string) => getAiApiKey(id),
26+
hasAiApiKey: (id: string) => hasAiApiKey(id),
2527
configureAi: (provider: string, contextSize: number, apiKey: string, baseUrl: string, model: string) =>
2628
configureAi(provider, contextSize, apiKey, baseUrl, model),
2729
}))
2830

2931
const settingsMap: Record<string, string> = {}
32+
// Backs the raw-store helpers (`getRawStoreValue`/`deleteRawStoreKeys`) the legacy-key migration
33+
// uses for non-registry keys; in a real app these hit the Tauri store plugin.
34+
const rawStoreMap: Record<string, string> = {}
3035
vi.mock('$lib/settings', async (importOriginal) => {
3136
const actual = await importOriginal<Record<string, unknown>>()
3237
return {
@@ -35,6 +40,14 @@ vi.mock('$lib/settings', async (importOriginal) => {
3540
setSetting: (id: string, value: string) => {
3641
settingsMap[id] = value
3742
},
43+
getRawStoreValue: (key: string) => Promise.resolve(rawStoreMap[key]),
44+
deleteRawStoreKeys: (keys: readonly string[]) => {
45+
for (const k of keys) {
46+
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete -- test fixture
47+
delete rawStoreMap[k]
48+
}
49+
return Promise.resolve()
50+
},
3851
}
3952
})
4053

@@ -71,10 +84,16 @@ function resetState(): void {
7184
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete -- test fixture reset
7285
delete settingsMap[k]
7386
}
87+
for (const k of Object.keys(rawStoreMap)) {
88+
// eslint-disable-next-line @typescript-eslint/no-dynamic-delete -- test fixture reset
89+
delete rawStoreMap[k]
90+
}
7491
saveAiApiKey.mockReset()
7592
saveAiApiKey.mockResolvedValue(null)
7693
getAiApiKey.mockReset()
7794
getAiApiKey.mockResolvedValue('')
95+
hasAiApiKey.mockReset()
96+
hasAiApiKey.mockResolvedValue(false)
7897
configureAi.mockReset()
7998
configureAi.mockResolvedValue(null)
8099
addToast.mockReset()
@@ -180,6 +199,40 @@ describe('migrateApiKeysFromSettings', () => {
180199
await migrateApiKeysFromSettings()
181200
expect(saveAiApiKey).not.toHaveBeenCalled()
182201
})
202+
203+
it('lifts a stranded legacy ai.openaiApiKey into the secret store, then drops the flat keys', async () => {
204+
rawStoreMap['ai.openaiApiKey'] = 'sk-legacy-123'
205+
rawStoreMap['ai.openaiBaseUrl'] = 'https://api.openai.com/v1'
206+
rawStoreMap['ai.openaiModel'] = 'gpt-4o-mini'
207+
hasAiApiKey.mockResolvedValue(false) // not yet in the secret store
208+
209+
await migrateApiKeysFromSettings()
210+
211+
expect(saveAiApiKey).toHaveBeenCalledWith('openai', 'sk-legacy-123')
212+
expect(rawStoreMap['ai.openaiApiKey']).toBeUndefined()
213+
expect(rawStoreMap['ai.openaiBaseUrl']).toBeUndefined()
214+
expect(rawStoreMap['ai.openaiModel']).toBeUndefined()
215+
})
216+
217+
it('drops the flat keys without re-saving when the secret store already has the key', async () => {
218+
rawStoreMap['ai.openaiApiKey'] = 'sk-legacy-123'
219+
hasAiApiKey.mockResolvedValue(true) // already migrated
220+
221+
await migrateApiKeysFromSettings()
222+
223+
expect(saveAiApiKey).not.toHaveBeenCalled()
224+
expect(rawStoreMap['ai.openaiApiKey']).toBeUndefined()
225+
})
226+
227+
it('keeps the legacy key if the secret-store save fails (never loses the only copy)', async () => {
228+
rawStoreMap['ai.openaiApiKey'] = 'sk-legacy-123'
229+
hasAiApiKey.mockResolvedValue(false)
230+
saveAiApiKey.mockRejectedValueOnce(new Error('keychain locked'))
231+
232+
await migrateApiKeysFromSettings()
233+
234+
expect(rawStoreMap['ai.openaiApiKey']).toBe('sk-legacy-123')
235+
})
183236
})
184237

185238
describe('pushConfigToBackend', () => {

apps/desktop/src/lib/settings/ai-config.ts

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@
2323
* settings UI all reach for. `sections/` is reserved for UI subcomponents.
2424
*/
2525

26-
import { getSetting, setSetting, resolveCloudConfig } from '$lib/settings'
27-
import { configureAi, getAiApiKey, saveAiApiKey } from '$lib/tauri-commands'
26+
import { getSetting, setSetting, resolveCloudConfig, getRawStoreValue, deleteRawStoreKeys } from '$lib/settings'
27+
import { configureAi, getAiApiKey, saveAiApiKey, hasAiApiKey } from '$lib/tauri-commands'
2828
import { getAppLogger } from '$lib/logging/logger'
2929
import { addToast } from '$lib/ui/toast'
3030
import { describeSecretError } from './sections/ai-secret-error'
@@ -36,8 +36,9 @@ const secretErrorToastId = 'ai-secret-store-error'
3636

3737
/**
3838
* One-time migration: move `apiKey` fields out of `ai.cloudProviderConfigs` (settings.json) into
39-
* the OS secret store. Runs idempotently at startup; once the JSON blob no longer contains any
40-
* `apiKey` field, this is a near-zero-cost no-op.
39+
* the OS secret store, then (via `migrateLegacyOpenAiKeys`) lift + drop the even older flat
40+
* `ai.openaiApiKey` / `ai.openaiBaseUrl` / `ai.openaiModel` keys. Runs idempotently at startup;
41+
* once the JSON blob and the flat keys are gone, this is a near-zero-cost no-op.
4142
*
4243
* Per-provider semantics: if saving to the secret store fails for one provider, that provider's
4344
* `apiKey` stays in settings.json so the user can retry later. Other providers still migrate.
@@ -47,6 +48,14 @@ const secretErrorToastId = 'ai-secret-store-error'
4748
* step temporarily handles plaintext keys).
4849
*/
4950
export async function migrateApiKeysFromSettings(): Promise<void> {
51+
// Two independent steps. The flat-key cleanup runs even when the cloud-config blob is
52+
// missing or malformed (a returning user might have ONLY the pre-refactor flat keys).
53+
await migrateCloudProviderConfigKeys()
54+
await migrateLegacyOpenAiKeys()
55+
}
56+
57+
/** Lifts `apiKey` fields out of the `ai.cloudProviderConfigs` JSON blob into the secret store. */
58+
async function migrateCloudProviderConfigKeys(): Promise<void> {
5059
const raw = getSetting('ai.cloudProviderConfigs')
5160
let parsed: Record<string, Record<string, unknown> | undefined>
5261
try {
@@ -85,6 +94,37 @@ export async function migrateApiKeysFromSettings(): Promise<void> {
8594
}
8695
}
8796

97+
/**
98+
* The pre-`ai.cloudProvider` schema stored OpenAI config as three flat keys:
99+
* `ai.openaiApiKey` (a PLAINTEXT key), `ai.openaiBaseUrl`, `ai.openaiModel`. The current code
100+
* reads none of them (the key lives in the OS secret store; model/baseUrl in
101+
* `ai.cloudProviderConfigs`), and the registry-driven save loop only manages registered ids, so
102+
* they'd otherwise linger in `settings.json` forever — a plaintext key sitting in Time Machine
103+
* and cloud backups, exactly what the secret-store move was meant to prevent.
104+
*
105+
* This lifts the flat key into the secret store under `openai` IF that's its only copy (so a
106+
* tester who set it pre-refactor and never re-entered it doesn't lose it AND gets working AI
107+
* back), then drops all three dead keys. Idempotent: once the keys are gone it's a no-op.
108+
*/
109+
const LEGACY_OPENAI_KEYS = ['ai.openaiApiKey', 'ai.openaiBaseUrl', 'ai.openaiModel'] as const
110+
111+
async function migrateLegacyOpenAiKeys(): Promise<void> {
112+
const legacyKey = (await getRawStoreValue<string>('ai.openaiApiKey')) ?? ''
113+
if (legacyKey.length > 0 && !(await hasAiApiKey('openai'))) {
114+
try {
115+
await saveAiApiKey('openai', legacyKey)
116+
logger.info('Lifted a legacy flat OpenAI API key into the secret store')
117+
} catch (e) {
118+
// Don't delete the only copy if we couldn't preserve it; retry next launch.
119+
logger.warn("Couldn't move the legacy OpenAI key to the secret store; leaving settings.json untouched: {error}", {
120+
error: e,
121+
})
122+
return
123+
}
124+
}
125+
await deleteRawStoreKeys(LEGACY_OPENAI_KEYS)
126+
}
127+
88128
/**
89129
* Push current AI config (provider, context size, cloud credentials) to the Rust backend. The API
90130
* key is fetched from the OS secret store; the rest comes from `settings.json`. Surfaces secret

apps/desktop/src/lib/settings/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@ export type { SettingsSection } from './settings-registry'
4848

4949
// Store
5050
export {
51+
deleteRawStoreKeys,
5152
forceSave,
53+
getRawStoreValue,
5254
getSetting,
5355
initializeSettings,
5456
isModified,

0 commit comments

Comments
 (0)