fix(agent-core-v2): guard config persistence against lossy writes

.changeset/config-persist-guard.md

@@ -0,0 +1,5 @@
+---
+"@moonshot-ai/kimi-code": patch
+---
+
+Fix config.toml entries being lost when the file had a syntax error or was edited outside the app.

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

@@ -3,7 +3,7 @@ import { Disposable } from '#/_base/di/lifecycle';
 import { LifecycleScope } from '#/app/scopes';
 import { ScopeActivation, registerScopedService } from '#/_base/di/scope';
 import { Emitter, type Event } from '#/_base/event';
-import { BugIndicatingError, onUnexpectedError } from '#/errors';
+import { BugIndicatingError, Error2, ErrorCodes, onUnexpectedError } from '#/errors';
 import { IBootstrapService } from '#/app/bootstrap/bootstrap';
 import { ILogService } from '#/_base/log/log';
 import {
@@ -309,6 +309,7 @@ export class ConfigService extends Disposable implements IConfigService {
   private readonly diagnosticsList: ConfigDiagnostic[] = [];
   private lastDiagnosticsSnapshot = '[]';
   private readonly configKey: string;
+  private tainted = false;
 
   constructor(
     @IConfigRegistry private readonly registry: IConfigRegistry,
@@ -402,17 +403,19 @@ export class ConfigService extends Disposable implements IConfigService {
       return;
     }
     await this.enqueueStateTransition(async () => {
-      const base = this.raw[domain];
-      const next = this.registry.merge(domain, base, patch);
-      const validated = this.registry.validate(domain, next);
-      const stripped = this.stripEnv(domain, validated);
-      if (stripped === undefined) {
-        delete this.raw[domain];
-      } else {
-        this.registry.validate(domain, stripped);
-        this.raw[domain] = stripped;
-      }
-      await this.persist(domain);
+      this.assertPersistable();
+      await this.persist(domain, (freshRaw) => {
+        this.raw = freshRaw;
+        const next = this.registry.merge(domain, freshRaw[domain], patch);
+        const validated = this.registry.validate(domain, next);
+        const stripped = this.stripEnv(domain, validated);
+        if (stripped === undefined) {
+          delete this.raw[domain];
+        } else {
+          this.registry.validate(domain, stripped);
+          this.raw[domain] = stripped;
+        }
+      });
       this.rebuildEffective('set', [domain]);
     });
   }
@@ -434,6 +437,7 @@ export class ConfigService extends Disposable implements IConfigService {
       return;
     }
     await this.enqueueStateTransition(async () => {
+      this.assertPersistable();
       const stripped = this.stripEnv(domain, effectiveValue);
       if (stripped === undefined) {
         delete this.raw[domain];
@@ -467,6 +471,7 @@ export class ConfigService extends Disposable implements IConfigService {
       return;
     }
     await this.enqueueStateTransition(async () => {
+      this.assertPersistable();
       const staged: ResolvedConfig = { ...this.raw };
       for (const domain of domains) {
         const value = sections[domain] === null ? undefined : sections[domain];
@@ -526,7 +531,11 @@ export class ConfigService extends Disposable implements IConfigService {
           : describeUnknownError(error);
       this.pushDiagnostic({ severity: 'error', message });
       this.log.warn('config load failed', { error: describeUnknownError(error) });
+      this.tainted = true;
+      this.emitDiagnosticsIfChanged();
+      return;
     }
+    this.tainted = false;
     const nextRawSnake = cloneRecord(fileData);
     for (const diagnostic of collectKeyDeprecations(nextRawSnake, this.registry.listSections())) {
       this.pushDiagnostic(diagnostic);
@@ -732,15 +741,58 @@ export class ConfigService extends Disposable implements IConfigService {
     this.commit('reload', [domain]);
   }
 
-  private async persist(domain: string): Promise<void> {
-    await this.persistDomains([domain]);
+  private assertPersistable(): void {
+    if (!this.tainted) return;
+    throw new Error2(
+      ErrorCodes.CONFIG_PERSIST_BLOCKED,
+      `Refusing to persist config: ${this.bootstrap.configPath} could not be read; fix the file and reload before writing.`,
+    );
   }
 
-  private async persistDomains(domains: readonly string[]): Promise<void> {
+  private async persist(domain: string, rebase?: (onDiskRaw: ResolvedConfig) => void): Promise<void> {
+    await this.persistDomains([domain], rebase);
+  }
+
+  private async persistDomains(
+    domains: readonly string[],
+    rebase?: (onDiskRaw: ResolvedConfig) => void,
+  ): Promise<void> {
+    this.assertPersistable();
+    let onDisk: ResolvedConfig = {};
+    try {
+      const data = await this.documentStore.get<ResolvedConfig>(CONFIG_SCOPE, this.configKey);
+      onDisk = data !== undefined && isPlainObject(data) ? data : {};
+    } catch (error) {
+      const message =
+        error instanceof TomlError
+          ? `Failed to parse ${this.bootstrap.configPath}: ${describeTomlSyntaxError(error)}`
+          : describeUnknownError(error);
+      this.pushDiagnostic({ severity: 'error', message });
+      this.emitDiagnosticsIfChanged();
+      this.log.warn('config persist aborted: re-read failed', {
+        error: describeUnknownError(error),
+      });
+      this.tainted = true;
+      throw new Error2(
+        ErrorCodes.CONFIG_PERSIST_BLOCKED,
+        `Refusing to persist config: ${this.bootstrap.configPath} could not be read; fix the file and reload before writing.`,
+        { cause: error },
+      );
+    }
+    if (rebase !== undefined) {
+      rebase(transformTomlData(onDisk, this.registry));
+    }
+    const absorbedExternal = JSON.stringify(onDisk) !== JSON.stringify(this.rawSnake);
+    const nextRawSnake = cloneRecord(onDisk);
     for (const domain of domains) {
-      applySectionToToml(this.rawSnake, domain, this.raw[domain], this.registry);
+      applySectionToToml(nextRawSnake, domain, this.raw[domain], this.registry);
     }
-    await this.documentStore.set(CONFIG_SCOPE, this.configKey, this.rawSnake);
+    await this.documentStore.set(CONFIG_SCOPE, this.configKey, nextRawSnake);
+    if (absorbedExternal) {
+      await this.load('reload');
+      return;
+    }
+    this.rawSnake = nextRawSnake;
   }
 }
 

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

@@ -4,6 +4,7 @@ import { CONFIG_INVALID_ERROR_CODE } from '#/kosong/contract/errors';
 export const ConfigErrors = {
   codes: {
     CONFIG_INVALID: CONFIG_INVALID_ERROR_CODE,
+    CONFIG_PERSIST_BLOCKED: 'config.persist_blocked',
   },
 } as const satisfies ErrorDomain;
 

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

@@ -24,6 +24,7 @@ import { createDecorator, type ProvideHandle } from '#/_base/di/instantiation';
 import { DisposableStore } from '#/_base/di/lifecycle';
 import { Service } from '#/_base/di/service';
 import { TestInstantiationService } from '#/_base/di/test';
+import { Event } from '#/_base/event';
 import { IBootstrapService } from '#/app/bootstrap/bootstrap';
 import {
   type ConfigSchema,
@@ -2596,3 +2597,160 @@ describe('ConfigService replaceSections', () => {
     disposables.dispose();
   });
 });
+
+describe('ConfigService persistence guards', () => {
+  class SilentStorage extends InMemoryStorageService {
+    override watch(): Event<void> {
+      return Event.None as Event<void>;
+    }
+  }
+
+  async function createGuardedConfig(toml: string) {
+    const disposables = new DisposableStore();
+    const ix = disposables.add(new TestInstantiationService());
+    const storage = new SilentStorage();
+    await storage.write('', 'config.toml', new TextEncoder().encode(toml));
+    ix.stub(ILogService, stubLog());
+    ix.stub(IBootstrapService, stubBootstrap('/tmp/kimi-cfg-guards'));
+    ix.stub(IFileSystemStorageService, storage);
+    ix.set(IAtomicTomlDocumentStore, new SyncDescriptor(TomlAtomicDocumentStore));
+    ix.set(IConfigRegistry, new SyncDescriptor(ConfigRegistry));
+    ix.set(IConfigService, new SyncDescriptor(ConfigService));
+    const config = ix.get(IConfigService);
+    await config.ready;
+    return { config, disposables, storage };
+  }
+
+  async function overwrite(storage: InMemoryStorageService, toml: string): Promise<void> {
+    await storage.write('', 'config.toml', new TextEncoder().encode(toml));
+  }
+
+  async function stored(storage: InMemoryStorageService): Promise<string> {
+    const bytes = await storage.read('', 'config.toml');
+    return new TextDecoder().decode(bytes);
+  }
+
+  async function expectPersistBlocked(promise: Promise<unknown>): Promise<void> {
+    const error = await promise.then(
+      () => undefined,
+      (e: unknown) => e,
+    );
+    expect(isError2(error)).toBe(true);
+    expect((error as Error2).code).toBe(ErrorCodes.CONFIG_PERSIST_BLOCKED);
+  }
+
+  it('refuses to persist when the initial load fails and keeps the file untouched', async () => {
+    const broken = '[providers\nbroken';
+    const { config, disposables, storage } = await createGuardedConfig(broken);
+
+    expect(config.diagnostics().some((d) => d.severity === 'error')).toBe(true);
+
+    await expectPersistBlocked(config.set(THINKING_SECTION, { enabled: true }));
+    await expectPersistBlocked(config.replace(THINKING_SECTION, { enabled: true }));
+    await expectPersistBlocked(config.replaceSections({ [THINKING_SECTION]: { enabled: true } }));
+
+    expect(await stored(storage)).toBe(broken);
+
+    await config.set(THINKING_SECTION, { enabled: true }, ConfigTarget.Memory);
+    expect(config.get<ThinkingConfig>(THINKING_SECTION)).toEqual({ enabled: true });
+
+    disposables.dispose();
+  });
+
+  it('keeps last-known-good values when a reload hits a broken file, and recovers after the file is fixed', async () => {
+    const { config, disposables, storage } = await createGuardedConfig(
+      '[providers.acme]\ntype = "openai"\napi_key = "sk-acme"\n',
+    );
+    expect(config.get<Record<string, unknown>>(PROVIDERS_SECTION)).toEqual({
+      acme: { type: 'openai', apiKey: 'sk-acme' },
+    });
+
+    await overwrite(storage, '= broken =');
+    await config.reload();
+
+    expect(config.get<Record<string, unknown>>(PROVIDERS_SECTION)).toEqual({
+      acme: { type: 'openai', apiKey: 'sk-acme' },
+    });
+    await expectPersistBlocked(config.set(THINKING_SECTION, { enabled: true }));
+    expect(await stored(storage)).toBe('= broken =');
+
+    await overwrite(storage, '[providers.beta]\ntype = "openai"\napi_key = "sk-beta"\n');
+    await config.reload();
+
+    expect(config.get<Record<string, unknown>>(PROVIDERS_SECTION)).toEqual({
+      beta: { type: 'openai', apiKey: 'sk-beta' },
+    });
+    await config.set(THINKING_SECTION, { enabled: true });
+    expect(config.get<ThinkingConfig>(THINKING_SECTION)).toEqual({ enabled: true });
+
+    disposables.dispose();
+  });
+
+  it('merges external edits observed at persist time instead of clobbering them', async () => {
+    const { config, disposables, storage } = await createGuardedConfig(
+      'default_model = "acme/m1"\n\n[providers.acme]\ntype = "openai"\napi_key = "sk-acme"\n',
+    );
+
+    await overwrite(
+      storage,
+      'default_model = "acme/m1"\n\n[providers.acme]\ntype = "openai"\napi_key = "sk-acme-2"\n\n[providers.beta]\ntype = "openai"\napi_key = "sk-beta"\n',
+    );
+
+    const changed: string[] = [];
+    config.onDidSectionChange((e) => changed.push(e.domain));
+    await config.set(THINKING_SECTION, { enabled: true });
+
+    const doc = await stored(storage);
+    expect(doc).toContain('sk-acme-2');
+    expect(doc).toContain('[providers.beta]');
+    expect(doc).toContain('[thinking]');
+    expect(config.get<Record<string, unknown>>(PROVIDERS_SECTION)).toEqual({
+      acme: { type: 'openai', apiKey: 'sk-acme-2' },
+      beta: { type: 'openai', apiKey: 'sk-beta' },
+    });
+    expect(config.get<ThinkingConfig>(THINKING_SECTION)).toEqual({ enabled: true });
+    expect(changed).toContain(PROVIDERS_SECTION);
+    expect(changed).toContain(THINKING_SECTION);
+
+    disposables.dispose();
+  });
+
+  it('honors an external delete instead of resurrecting the in-memory copy', async () => {
+    const { config, disposables, storage } = await createGuardedConfig(
+      '[providers.acme]\ntype = "openai"\napi_key = "sk-acme"\n',
+    );
+
+    await storage.delete('', 'config.toml');
+    await config.set(THINKING_SECTION, { enabled: true });
+
+    const doc = await stored(storage);
+    expect(doc).toContain('[thinking]');
+    expect(doc).not.toContain('[providers.acme]');
+    expect(config.inspect(PROVIDERS_SECTION).userValue).toBeUndefined();
+
+    disposables.dispose();
+  });
+
+  it('rebases a set() merge onto external edits of the same section', async () => {
+    const { config, disposables, storage } = await createGuardedConfig(
+      '[providers.acme]\ntype = "openai"\napi_key = "sk-acme"\n',
+    );
+
+    await overwrite(
+      storage,
+      '[providers.acme]\ntype = "openai"\napi_key = "sk-acme"\n\n[providers.beta]\ntype = "openai"\napi_key = "sk-beta"\n',
+    );
+    await config.set(PROVIDERS_SECTION, { gamma: { type: 'openai', apiKey: 'sk-gamma' } });
+
+    const doc = await stored(storage);
+    expect(doc).toContain('[providers.beta]');
+    expect(doc).toContain('[providers.gamma]');
+    expect(config.get<Record<string, unknown>>(PROVIDERS_SECTION)).toEqual({
+      acme: { type: 'openai', apiKey: 'sk-acme' },
+      beta: { type: 'openai', apiKey: 'sk-beta' },
+      gamma: { type: 'openai', apiKey: 'sk-gamma' },
+    });
+
+    disposables.dispose();
+  });
+});

packages/protocol/src/events.ts

@@ -229,6 +229,7 @@ export interface GoalChange {
 
 export type KimiErrorCode =
   | 'config.invalid'
+  | 'config.persist_blocked'
   | 'session.not_found'
   | 'session.already_exists'
   | 'session.id_invalid'
@@ -1259,6 +1260,7 @@ export const goalChangeSchema = z.object({
 
 export const kimiErrorCodeSchema = z.enum([
   'config.invalid',
+  'config.persist_blocked',
   'session.not_found',
   'session.already_exists',
   'session.id_invalid',