fix: persist default kimi hub provider to BrowserOS prefs on first load

[Nikhil (shadowfax92)]

Summary

• When VITE_PUBLIC_KIMI_LAUNCH is enabled, loadProviders() in llm-hub/storage.ts returned the default Kimi provider in-memory but never persisted it to the BrowserOS pref
• The browser's C++ code reads the pref directly and found it empty, so Kimi didn't appear in the toolbar/hub until user manually edited and saved
• Now loadProviders() persists defaults and ensureKimiFirst() additions to the pref immediately

Fixes #428

Test plan

• Fresh install with VITE_PUBLIC_KIMI_LAUNCH=true → Kimi should appear in hub without manual edit/save
• Update from previous version → Kimi should be auto-added and persisted
• With VITE_PUBLIC_KIMI_LAUNCH=false → No change in behavior

🤖 Generated with Claude Code

[Nikhil (shadowfax92)]

Greptile (@greptileai) review

[greptile-apps]

Greptile Summary

This PR fixes a bug where the default Kimi hub provider was only returned in-memory but never persisted to the BrowserOS pref, causing the C++ browser layer to read an empty pref and not display Kimi in the toolbar until the user manually saved.

Key changes in apps/agent/lib/llm-hub/storage.ts:

• When providers.length === 0 and Kimi launch is enabled, loadProviders() now calls saveProviders() before returning the default [KIMI_PROVIDER] list.
• When ensureKimiFirst() adds Kimi to an existing non-empty providers list, loadProviders() now persists the augmented list immediately (guarded by a length-change check).

Observations:

• The save condition (normalized.length !== providers.length) works correctly for the current ensureKimiFirst implementation but is fragile — a reference inequality check (normalized !== providers) would be semantically cleaner and more resilient to future refactors of that function.
• The return value of saveProviders() is discarded in both new call-sites; if setPref fails the caller still receives an in-memory result that doesn't match what the C++ layer sees. Retries happen implicitly on the next loadProviders() call, but there is no failure visibility.
• No tests exist for loadProviders() in the llm-hub module; adding unit tests for the fresh-install and update paths would make regressions easier to catch.

Confidence Score: 4/5

• Safe to merge; the fix correctly addresses the pref-persistence bug with only minor robustness improvements suggested.
• The change is small and focused. The logic is correct for all current code paths, error handling follows the existing pattern in the file, and the catch block preserves the pre-existing fallback behaviour. Two minor issues were flagged (fragile length check and unchecked saveProviders return) but neither represents a regression or data-loss risk.
• No files require special attention.

Important Files Changed

Filename	Overview
apps/agent/lib/llm-hub/storage.ts	Adds pref persistence for default Kimi provider and for ensureKimiFirst additions. Logic is correct for the current implementation; minor fragility in the save condition (length vs. reference check) and a discarded saveProviders return value.

Sequence Diagram

sequenceDiagram
    participant UI as React UI (useLlmHubProviders)
    participant S as storage.ts (loadProviders)
    participant A as BrowserOSAdapter
    participant P as BrowserOS Prefs (C++)

    UI->>S: loadProviders()
    S->>A: getPref(THIRD_PARTY_LLM_PROVIDERS)
    A->>P: chrome.browserOS.getPref(...)
    P-->>A: PrefObject (value or empty)
    A-->>S: providers[]

    alt providers is empty AND Kimi launch enabled
        S->>A: saveProviders([KIMI_PROVIDER])
        A->>P: chrome.browserOS.setPref(..., [KIMI_PROVIDER])
        Note over P: Kimi now visible to C++ toolbar
        P-->>A: success
        A-->>S: boolean
        S-->>UI: [KIMI_PROVIDER]
    else providers is empty AND Kimi launch disabled
        S-->>UI: []
    else providers not empty
        S->>S: ensureKimiFirst(providers)
        alt Kimi was NOT in list (length changed)
            S->>A: saveProviders(normalized)
            A->>P: chrome.browserOS.setPref(..., normalized)
            Note over P: Kimi prepended & persisted
            P-->>A: success
            A-->>S: boolean
        end
        S-->>UI: normalized
    end

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: apps/agent/lib/llm-hub/storage.ts
Line: 42-44

Comment:
**Fragile save condition: length check misses reorders**

The condition `normalized.length !== providers.length` works correctly for `ensureKimiFirst`'s current implementation, which only ever prepends Kimi. However, `ensureKimiFirst` already returns the **same object reference** when unchanged (`return hasKimi ? providers : [KIMI_PROVIDER, ...providers]`), so a reference inequality check is both more semantically precise and more future-proof — it would also catch any future change to `ensureKimiFirst` that reorders items (e.g. moving Kimi from the middle to the front) without changing the length.

```suggestion
    const normalized = ensureKimiFirst(providers)
    if (normalized !== providers) {
      await saveProviders(normalized)
    }
    return normalized
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: apps/agent/lib/llm-hub/storage.ts
Line: 32-36

Comment:
**`saveProviders` return value not checked**

The result of `await saveProviders(defaults)` is silently discarded. `saveProviders` swallows errors and returns `false` when `setPref` fails, so if the pref write doesn't succeed the function still returns `defaults` to the caller — meaning the C++ side won't see Kimi in the pref even though the in-memory state looks correct. The fix retries on the *next* `loadProviders()` call (since prefs will still be empty), but there's no visibility into the failure. Consider at least logging when the save returns `false`, or propagating the result to aid debugging.

```suggestion
      if (isKimiLaunchEnabled()) {
        const defaults = [KIMI_PROVIDER]
        const saved = await saveProviders(defaults)
        if (!saved) {
          // setPref failed; C++ will retry on next loadProviders call
        }
        return defaults
      }
```

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: 31b1b11}

[greptile-apps]

Fragile save condition: length check misses reorders

The condition normalized.length !== providers.length works correctly for ensureKimiFirst's current implementation, which only ever prepends Kimi. However, ensureKimiFirst already returns the same object reference when unchanged (return hasKimi ? providers : [KIMI_PROVIDER, ...providers]), so a reference inequality check is both more semantically precise and more future-proof — it would also catch any future change to ensureKimiFirst that reorders items (e.g. moving Kimi from the middle to the front) without changing the length.

Suggested change

	if (normalized.length !== providers.length) {
	await saveProviders(normalized)
	}
	const normalized = ensureKimiFirst(providers)
	if (normalized !== providers) {
	await saveProviders(normalized)
	}
	return normalized

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/agent/lib/llm-hub/storage.ts
Line: 42-44

Comment:
**Fragile save condition: length check misses reorders**

The condition `normalized.length !== providers.length` works correctly for `ensureKimiFirst`'s current implementation, which only ever prepends Kimi. However, `ensureKimiFirst` already returns the **same object reference** when unchanged (`return hasKimi ? providers : [KIMI_PROVIDER, ...providers]`), so a reference inequality check is both more semantically precise and more future-proof — it would also catch any future change to `ensureKimiFirst` that reorders items (e.g. moving Kimi from the middle to the front) without changing the length.

```suggestion
    const normalized = ensureKimiFirst(providers)
    if (normalized !== providers) {
      await saveProviders(normalized)
    }
    return normalized
```

How can I resolve this? If you propose a fix, please make it concise.

[Nikhil (shadowfax92)]

Good catch — accepted. Reference equality is cleaner and more future-proof here since ensureKimiFirst returns the same array reference when unchanged. Updated to normalized !== providers.

[Nikhil (shadowfax92)]

Re: saveProviders return value not checked — Respectfully declining this one. The suggestion is an empty if (!saved) {} block with a comment, which is dead code. saveProviders already handles errors internally (catches, returns false). If the save fails, the pref stays empty and the next loadProviders() call will naturally retry (it'll see the empty pref again and re-attempt the save). The self-healing behavior is inherent to the design — no logging or action needed at this call site.