fix(agent-core-v2): write refresh results in one atomic config transition

.changeset/fuzzy-pandas-refresh.md

@@ -0,0 +1,5 @@
+---
+"@moonshot-ai/kimi-code": patch
+---
+
+Fix sporadic "model is not configured" errors when starting kimi web, caused by the background provider-model refresh transiently clearing the model catalog while the first session was being created.

packages/agent-core-v2/src/app/config/config.ts

@@ -203,6 +203,19 @@ export interface IConfigService {
   getAll(): ResolvedConfig;
   set(domain: string, patch: unknown, target?: ConfigTarget): Promise<void>;
   replace(domain: string, value: unknown, target?: ConfigTarget): Promise<void>;
+  /**
+   * Replaces several sections in ONE state transition: every domain's raw
+   * value is updated first, the document is persisted with a single disk
+   * write, and the effective view is rebuilt once — change events fire only
+   * after all domains have taken effect, so a reader can never observe a
+   * half-applied multi-section write. A domain mapped to `undefined` is
+   * cleared (same as `replace(domain, undefined)`). Domain application order
+   * follows the `sections` key order.
+   */
+  replaceSections(
+    sections: Readonly<Record<string, unknown>>,
+    target?: ConfigTarget,
+  ): Promise<void>;
   reload(): Promise<void>;
   diagnostics(): readonly ConfigDiagnostic[];
 }

packages/agent-core-v2/src/app/config/configService.ts

@@ -354,6 +354,43 @@ export class ConfigService extends Disposable implements IConfigService {
     });
   }
 
+  async replaceSections(
+    sections: Readonly<Record<string, unknown>>,
+    target: ConfigTarget = ConfigTarget.User,
+  ): Promise<void> {
+    await this.ready;
+    const domains = Object.keys(sections);
+    if (domains.length === 0) return;
+    if (target === ConfigTarget.Memory) {
+      const staged: ResolvedConfig = { ...this.memory };
+      for (const domain of domains) {
+        const value = sections[domain];
+        if (value === undefined) {
+          delete staged[domain];
+        } else {
+          staged[domain] = this.registry.validate(domain, value);
+        }
+      }
+      this.memory = staged;
+      this.commit('set', domains);
+      return;
+    }
+    await this.enqueueStateTransition(async () => {
+      const staged: ResolvedConfig = { ...this.raw };
+      for (const domain of domains) {
+        const stripped = this.stripEnv(domain, sections[domain]);
+        if (stripped === undefined) {
+          delete staged[domain];
+        } else {
+          staged[domain] = this.registry.validate(domain, stripped);
+        }
+      }
+      this.raw = staged;
+      await this.persistDomains(domains);
+      this.rebuildEffective('set', domains);
+    });
+  }
+
   private stripEnv(domain: string, value: unknown): unknown {
     let result = value;
     const section = this.registry.getSection(domain);
@@ -565,7 +602,13 @@ export class ConfigService extends Disposable implements IConfigService {
   }
 
   private async persist(domain: string): Promise<void> {
-    applySectionToToml(this.rawSnake, domain, this.raw[domain], this.registry);
+    await this.persistDomains([domain]);
+  }
+
+  private async persistDomains(domains: readonly string[]): Promise<void> {
+    for (const domain of domains) {
+      applySectionToToml(this.rawSnake, domain, this.raw[domain], this.registry);
+    }
     await this.documentStore.set(CONFIG_SCOPE, this.configKey, this.rawSnake);
   }
 }

packages/agent-core-v2/src/app/kosongConfig/discoveryService.ts

@@ -3,10 +3,11 @@
  *
  * Owns the all-provider model refresh: delegates to the shared
  * `@moonshot-ai/kimi-code-oauth` orchestrator (managed OAuth + open
- * platforms + custom registries), applies the discovered providers/models
- * to kosong's in-memory registries (the persistence bridge writes them back
- * to config), and publishes `event.model_catalog.changed` on change. Bound
- * at App scope.
+ * platforms + custom registries), writes the discovered providers/models
+ * into config through ONE atomic `replaceSections` transition (the
+ * persistence bridge then syncs them into kosong's in-memory registries),
+ * and publishes `event.model_catalog.changed` on change. Bound at App
+ * scope.
  *
  * `modelSource: 'static'` short-circuits refresh: a provider whose effective
  * model source is `static` (config-declared, or declared by its vendor
@@ -18,14 +19,18 @@
  * refresh them nor drop them (or a default model pointing at them).
  *
  * Two write-path details preserve the legacy semantics exactly:
- *  - Registry replaces preserve the entries the orchestrator could not see:
- *    the static exclusion AND the config-file-external entries (the
- *    env-synthesized `__kimi_env__` slice), which the orchestrator's
- *    user-value view does not contain.
- *  - `defaultModel` / `thinking` stay direct `config.replace` writes (like
- *    the OAuth flows): the env overlay may pin the runtime default to the
- *    env-synthesized model, and only the config effective view knows that —
- *    the bridge then syncs the effective pointer into the registry.
+ *  - The orchestrator's two-phase host contract (removeProvider, then
+ *    setConfig) is absorbed into a single atomic write: the removal is
+ *    computed in memory only (`shapeWithoutProvider`), because the patch's
+ *    full providers/models records already express it. The runtime
+ *    registries therefore never pass through a halfway-removed state — that
+ *    intermediate state was the source of the "provider/model not
+ *    configured" startup race against profile binding.
+ *  - The env-synthesized `__kimi_env__` slice is never written to config:
+ *    it lives in the effective overlay, and the bridge's event-driven sync
+ *    carries it into the registries on its own. `defaultModel` / `thinking`
+ *    also go through config (like the OAuth flows), since the env overlay
+ *    may pin the runtime default and only the config effective view knows.
  *
  * Credential detection goes through the provider-definition registry
  * (`resolveProviderEndpoint` against the provider's config env bag), not a
@@ -47,7 +52,7 @@ import { IConfigService } from '#/app/config/config';
 import { IEventService } from '#/app/event/event';
 import { ModelCatalogErrors } from '#/kosong/model/errors';
 import { IHostRequestHeaders } from '#/kosong/model/hostRequestHeaders';
-import { IModelService, type ModelRecord } from '#/kosong/model/model';
+import { type ModelRecord } from '#/kosong/model/model';
 import {
   IProviderService,
   type ModelSource,
@@ -88,7 +93,6 @@ export class ProviderDiscoveryService implements IProviderDiscoveryService {
   private refreshChain: Promise<unknown> = Promise.resolve();
 
   constructor(
-    @IModelService private readonly modelService: IModelService,
     @IProviderService private readonly providerService: IProviderService,
     @IConfigService private readonly config: IConfigService,
     @IOAuthService private readonly oauth: IOAuthService,
@@ -188,7 +192,7 @@ export class ProviderDiscoveryService implements IProviderDiscoveryService {
   private buildRefreshHost(exclusion: StaticExclusion): RefreshProviderHost {
     return {
       getConfig: async () => this.readUserConfigShape(exclusion),
-      removeProvider: (providerId) => this.removeProviderForRefresh(providerId),
+      removeProvider: (providerId) => this.shapeWithoutProvider(providerId),
       setConfig: (patch) => this.applyRefreshPatch(patch, exclusion),
       resolveOAuthToken: (providerName, oauthRef) => this.resolveOAuthToken(providerName, oauthRef),
       userAgent: this.hostRequestHeaders.headers['User-Agent'],
@@ -212,23 +216,16 @@ export class ProviderDiscoveryService implements IProviderDiscoveryService {
   }
 
   /**
-   * The registry entries the orchestrator's user-value view cannot see (the
-   * env-synthesized slice): preserved verbatim across every registry
-   * replace, or a refresh would drop the runtime env model/provider.
+   * The orchestrator's host contract is two-phase (removeProvider, then
+   * setConfig) because its original merge-semantics host could not delete
+   * keys through a patch. This host writes with replace semantics and the
+   * patch always carries the FULL providers/models records, so the removal
+   * is already expressed by the patch itself — computing it here in memory
+   * keeps the runtime registries untouched until the single atomic write in
+   * {@link applyRefreshPatch} (a halfway-removed catalog is what used to
+   * produce the "provider/model not configured" startup race).
    */
-  private syntheticProviders(
-    userProviders: Readonly<Record<string, unknown>>,
-  ): Record<string, ProviderConfig> {
-    return withoutKeys(this.providerService.list(), userProviders);
-  }
-
-  private syntheticModels(
-    userModels: Readonly<Record<string, unknown>>,
-  ): Record<string, ModelRecord> {
-    return withoutKeys(this.modelService.list(), userModels);
-  }
-
-  private async removeProviderForRefresh(providerId: string): Promise<ManagedKimiConfigShape> {
+  private shapeWithoutProvider(providerId: string): Promise<ManagedKimiConfigShape> {
     const current = this.readUserConfigShape();
     const providers = current.providers as Record<string, ProviderConfig>;
     const restProviders = Object.fromEntries(
@@ -238,16 +235,11 @@ export class ProviderDiscoveryService implements IProviderDiscoveryService {
     const restModels = Object.fromEntries(
       Object.entries(models).filter(([, record]) => record.provider !== providerId),
     );
-    await this.providerService.replaceAll({
-      ...this.syntheticProviders(providers),
-      ...restProviders,
-    });
-    await this.modelService.replaceAll({ ...this.syntheticModels(models), ...restModels });
-    return {
+    return Promise.resolve({
       ...current,
       providers: restProviders,
       models: restModels,
-    } as ManagedKimiConfigShape;
+    } as ManagedKimiConfigShape);
   }
 
   private async applyRefreshPatch(
@@ -258,56 +250,50 @@ export class ProviderDiscoveryService implements IProviderDiscoveryService {
       this.config.inspect<Record<string, ProviderConfig>>(PROVIDERS_SECTION).userValue ?? {};
     const userModels =
       this.config.inspect<Record<string, ModelRecord>>(MODELS_SECTION).userValue ?? {};
+    // All four sections land in ONE config transition: a single disk write,
+    // then one effective rebuild whose change events synchronously push the
+    // new records into the kosong registries — no reader can observe a
+    // half-applied refresh. The env-synthesized slice (`__kimi_env__` etc.)
+    // is NOT written here: it lives in the effective overlay, and the
+    // bridge's event-driven sync carries it into the registries on its own.
+    const sections: Record<string, unknown> = {};
     if (patch.providers !== undefined) {
-      await this.providerService.replaceAll({
-        ...this.syntheticProviders(userProviders),
+      sections[PROVIDERS_SECTION] = {
         ...exclusion.providers,
         ...patch.providers,
-      });
+      };
     }
     if (patch.models !== undefined) {
-      await this.modelService.replaceAll({
-        ...this.syntheticModels(userModels),
+      sections[MODELS_SECTION] = {
         ...exclusion.models,
         // The orchestrator's alias shape is a structural superset of
         // ModelRecord at runtime (its protocol union additionally allows
         // vendor spellings the records never actually carry); the legacy
         // config.write path took `unknown`, so cast here.
         ...(patch.models as Record<string, ModelRecord>),
-      });
+      };
     }
     // The refresh orchestrator always sends all four keys, so key presence is
     // the write intent and an explicit `undefined` means CLEAR, not "leave
-    // alone". `set()` cannot express that — its deepMerge resolves an
-    // undefined patch back to the base value — so these go through `replace`,
-    // which deletes the section on undefined. Otherwise a default model (and
-    // its thinking setting) whose alias the upstream dropped would dangle in
-    // the user config forever.
+    // alone" — `replaceSections` deletes the section on undefined. Otherwise
+    // a default model (and its thinking setting) whose alias the upstream
+    // dropped would dangle in the user config forever.
     //
     // Exception: when the user's default points at a statically-sourced model
     // the orchestrator could not see, its clamp/restore logic would silently
     // clear or re-point the selection (and its thinking) — restore both.
-    //
-    // `defaultModel` / `thinking` go through config directly (not the
-    // registry): the env overlay may pin the runtime default, and only the
-    // config effective view knows — the bridge syncs the effective pointer
-    // into the registry afterwards.
     const restoreDefault = exclusion.defaultModel !== undefined;
     if ('defaultModel' in patch) {
-      await this.config.replace(
-        DEFAULT_MODEL_SECTION,
-        restoreDefault ? exclusion.defaultModel : patch.defaultModel,
-      );
+      sections[DEFAULT_MODEL_SECTION] = restoreDefault
+        ? exclusion.defaultModel
+        : patch.defaultModel;
     }
     if ('thinking' in patch) {
-      await this.config.replace(
-        THINKING_SECTION,
-        restoreDefault ? exclusion.thinking : patch.thinking,
-      );
+      sections[THINKING_SECTION] = restoreDefault ? exclusion.thinking : patch.thinking;
     }
-    // The writes above landed in the registries / config; compute the
-    // post-patch shape in memory (re-reading config would race the bridge's
-    // asynchronous persist of the registry changes).
+    await this.config.replaceSections(sections);
+    // The write above landed in config (and, through the bridge's synchronous
+    // event sync, the registries); compute the post-patch shape in memory.
     return {
       providers:
         patch.providers !== undefined