mirror of
https://github.com/MoonshotAI/kimi-code.git
synced 2026-08-02 21:15:14 +00:00
fix(agent-core-v2): write refresh results in one atomic config transition (#2410)
Some checks are pending
CI / build (push) Waiting to run
CI / test (1) (push) Waiting to run
CI / test (2) (push) Waiting to run
CI / test (3) (push) Waiting to run
CI / test (4) (push) Waiting to run
CI / test (5) (push) Waiting to run
CI / test-pi-tui (push) Waiting to run
CI / test-windows (push) Waiting to run
CI / lint (push) Waiting to run
CI / typecheck (push) Waiting to run
Nix Build / Check flake.nix workspace sync (push) Waiting to run
Nix Build / nix build .#kimi-code (push) Blocked by required conditions
Release / Release (push) Waiting to run
Release / Deploy docs (push) Blocked by required conditions
Release / Native release artifact (push) Blocked by required conditions
Release / Publish native release assets (push) Blocked by required conditions
Some checks are pending
CI / build (push) Waiting to run
CI / test (1) (push) Waiting to run
CI / test (2) (push) Waiting to run
CI / test (3) (push) Waiting to run
CI / test (4) (push) Waiting to run
CI / test (5) (push) Waiting to run
CI / test-pi-tui (push) Waiting to run
CI / test-windows (push) Waiting to run
CI / lint (push) Waiting to run
CI / typecheck (push) Waiting to run
Nix Build / Check flake.nix workspace sync (push) Waiting to run
Nix Build / nix build .#kimi-code (push) Blocked by required conditions
Release / Release (push) Waiting to run
Release / Deploy docs (push) Blocked by required conditions
Release / Native release artifact (push) Blocked by required conditions
Release / Publish native release assets (push) Blocked by required conditions
* fix(agent-core-v2): write refresh results in one atomic config transition - add IConfigService.replaceSections: applies several domains in a single state transition — one disk write, one effective rebuild, change events fire only after all domains took effect - rework ProviderDiscoveryService to absorb the orchestrator's two-phase removeProvider/setConfig host contract into one replaceSections write, so the kosong registries never pass through a halfway-removed catalog - stop writing the env-synthesized __kimi_env__ slice to config; the bridge's event-driven sync carries it into the registries on its own - fixes sporadic "model is not configured" errors when starting kimi web, caused by the background refresh transiently clearing the model catalog while the first session was being created * fix(agent-core-v2): stage replaceSections writes before mutating raw config Validate and strip every domain into a staged copy of the raw/memory layer first, then swap it in only after the whole batch succeeds — previously a later domain failing validation left earlier domains already applied to this.raw/this.memory while the call reported failure, exposing a partially applied user layer to inspect() and future merges.
This commit is contained in:
parent
d8f455d694
commit
f1a3475ad5
7 changed files with 396 additions and 99 deletions
5
.changeset/fuzzy-pandas-refresh.md
Normal file
5
.changeset/fuzzy-pandas-refresh.md
Normal file
|
|
@ -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.
|
||||
|
|
@ -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[];
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ import type { ToolCall } from '#/kosong/contract/message';
|
|||
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'pathe';
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { IAgentProfileService, type ResolvedAgentProfile } from '#/agent/profile/profile';
|
||||
import { Error2, ErrorCodes, toErrorPayload } from '#/errors';
|
||||
|
|
@ -24,7 +24,7 @@ import { SyncDescriptor } from '#/_base/di/descriptors';
|
|||
import { DisposableStore } from '#/_base/di/lifecycle';
|
||||
import { TestInstantiationService } from '#/_base/di/test';
|
||||
import { IBootstrapService } from '#/app/bootstrap/bootstrap';
|
||||
import { IConfigRegistry, IConfigService } from '#/app/config/config';
|
||||
import { ConfigTarget, IConfigRegistry, IConfigService } from '#/app/config/config';
|
||||
import { ConfigRegistry, ConfigService } from '#/app/config/configService';
|
||||
import { SECONDARY_MODEL_FLAG_ID } from '#/session/subagent/flag';
|
||||
import '#/app/cron/configSection';
|
||||
|
|
@ -46,7 +46,9 @@ import {
|
|||
type LoopControl,
|
||||
} from '#/agent/loop/configSection';
|
||||
import {
|
||||
DEFAULT_MODEL_SECTION,
|
||||
MODELS_SECTION,
|
||||
PROVIDERS_SECTION,
|
||||
SECONDARY_MODEL_EFFORT_ENV,
|
||||
SECONDARY_MODEL_ENV,
|
||||
SECONDARY_MODEL_SECTION,
|
||||
|
|
@ -1889,3 +1891,153 @@ describe('ConfigService thinking effort max migration', () => {
|
|||
disposables.dispose();
|
||||
});
|
||||
});
|
||||
|
||||
describe('ConfigService replaceSections', () => {
|
||||
// Top-level keys must precede every [table] header in TOML.
|
||||
const SEED_TOML = [
|
||||
'default_model = "acme/m1"',
|
||||
'',
|
||||
'[providers.acme]',
|
||||
'type = "openai"',
|
||||
'api_key = "sk-acme"',
|
||||
'',
|
||||
'[models."acme/m1"]',
|
||||
'provider = "acme"',
|
||||
'model = "m1"',
|
||||
'max_context_size = 1000',
|
||||
'',
|
||||
'[thinking]',
|
||||
'enabled = true',
|
||||
'',
|
||||
].join('\n');
|
||||
|
||||
async function createSectionsConfig(toml = SEED_TOML) {
|
||||
const disposables = new DisposableStore();
|
||||
const ix = disposables.add(new TestInstantiationService());
|
||||
const storage = new InMemoryStorageService();
|
||||
await storage.write('', 'config.toml', new TextEncoder().encode(toml));
|
||||
ix.stub(ILogService, stubLog());
|
||||
ix.stub(IBootstrapService, stubBootstrap('/tmp/kimi-cfg-replace-sections'));
|
||||
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;
|
||||
const store = ix.get(IAtomicTomlDocumentStore);
|
||||
return { config, disposables, store, storage };
|
||||
}
|
||||
|
||||
it('applies every domain in one transition with a single disk write, clearing undefined domains', async () => {
|
||||
const { config, disposables, store } = await createSectionsConfig();
|
||||
const setSpy = vi.spyOn(store, 'set');
|
||||
|
||||
await config.replaceSections({
|
||||
[PROVIDERS_SECTION]: { acme: { type: 'openai', apiKey: 'sk-acme-2' } },
|
||||
[MODELS_SECTION]: { 'acme/m2': { provider: 'acme', model: 'm2', maxContextSize: 2000 } },
|
||||
[DEFAULT_MODEL_SECTION]: undefined,
|
||||
[THINKING_SECTION]: undefined,
|
||||
});
|
||||
|
||||
expect(setSpy).toHaveBeenCalledTimes(1);
|
||||
expect(config.get<Record<string, unknown>>(PROVIDERS_SECTION)).toEqual({
|
||||
acme: { type: 'openai', apiKey: 'sk-acme-2' },
|
||||
});
|
||||
expect(config.get<Record<string, unknown>>(MODELS_SECTION)).toEqual({
|
||||
'acme/m2': { provider: 'acme', model: 'm2', maxContextSize: 2000 },
|
||||
});
|
||||
expect(config.get(DEFAULT_MODEL_SECTION)).toBeUndefined();
|
||||
expect(config.get(THINKING_SECTION)).toEqual({});
|
||||
expect(config.inspect(DEFAULT_MODEL_SECTION).userValue).toBeUndefined();
|
||||
// `stripThinkingEnv` maps a clear to `{}` (`{...undefined}`), so the user
|
||||
// layer collapses to an empty object instead of disappearing — the
|
||||
// long-standing `replace(domain, undefined)` behavior, unchanged here.
|
||||
expect(config.inspect(THINKING_SECTION).userValue).toEqual({});
|
||||
|
||||
disposables.dispose();
|
||||
});
|
||||
|
||||
it('fires change events only after all domains have taken effect', async () => {
|
||||
const { config, disposables } = await createSectionsConfig();
|
||||
const domains: string[] = [];
|
||||
let snapshotDuringFirstEvent:
|
||||
| { providers: unknown; models: unknown; defaultModel: unknown; thinking: unknown }
|
||||
| undefined;
|
||||
config.onDidSectionChange((e) => {
|
||||
domains.push(e.domain);
|
||||
snapshotDuringFirstEvent ??= {
|
||||
providers: config.get(PROVIDERS_SECTION),
|
||||
models: config.get(MODELS_SECTION),
|
||||
defaultModel: config.get(DEFAULT_MODEL_SECTION),
|
||||
thinking: config.get(THINKING_SECTION),
|
||||
};
|
||||
});
|
||||
|
||||
await config.replaceSections({
|
||||
[PROVIDERS_SECTION]: { acme: { type: 'openai', apiKey: 'sk-acme-2' } },
|
||||
[MODELS_SECTION]: { 'acme/m2': { provider: 'acme', model: 'm2', maxContextSize: 2000 } },
|
||||
[DEFAULT_MODEL_SECTION]: undefined,
|
||||
[THINKING_SECTION]: undefined,
|
||||
});
|
||||
|
||||
// Every event — including the very first one — already observes the fully
|
||||
// applied state; no listener can catch the write half-applied. (The
|
||||
// cleared thinking section still resolves to its schema default `{}`.)
|
||||
expect(snapshotDuringFirstEvent).toEqual({
|
||||
providers: { acme: { type: 'openai', apiKey: 'sk-acme-2' } },
|
||||
models: { 'acme/m2': { provider: 'acme', model: 'm2', maxContextSize: 2000 } },
|
||||
defaultModel: undefined,
|
||||
thinking: {},
|
||||
});
|
||||
expect([...domains].sort()).toEqual(
|
||||
[PROVIDERS_SECTION, MODELS_SECTION, DEFAULT_MODEL_SECTION, THINKING_SECTION].sort(),
|
||||
);
|
||||
|
||||
disposables.dispose();
|
||||
});
|
||||
|
||||
it('supports the memory target without touching the persisted user layer', async () => {
|
||||
const { config, disposables, store } = await createSectionsConfig();
|
||||
const setSpy = vi.spyOn(store, 'set');
|
||||
|
||||
await config.replaceSections(
|
||||
{ [THINKING_SECTION]: { enabled: false, effort: 'low' } },
|
||||
ConfigTarget.Memory,
|
||||
);
|
||||
|
||||
expect(setSpy).not.toHaveBeenCalled();
|
||||
expect(config.get<ThinkingConfig>(THINKING_SECTION)).toEqual({
|
||||
enabled: false,
|
||||
effort: 'low',
|
||||
});
|
||||
expect(config.inspect<ThinkingConfig>(THINKING_SECTION).userValue).toEqual({ enabled: true });
|
||||
|
||||
disposables.dispose();
|
||||
});
|
||||
|
||||
it('leaves the user layer untouched when a later domain fails validation', async () => {
|
||||
const { config, disposables, store } = await createSectionsConfig();
|
||||
const setSpy = vi.spyOn(store, 'set');
|
||||
|
||||
// Providers is applied first in key order and validates fine; thinking
|
||||
// then fails schema validation (`enabled` must be a boolean). The batch
|
||||
// must reject with NO observable partial application.
|
||||
await expect(
|
||||
config.replaceSections({
|
||||
[PROVIDERS_SECTION]: { acme: { type: 'openai', apiKey: 'sk-acme-2' } },
|
||||
[THINKING_SECTION]: { enabled: 'yes' },
|
||||
}),
|
||||
).rejects.toThrow();
|
||||
|
||||
expect(setSpy).not.toHaveBeenCalled();
|
||||
expect(config.inspect<Record<string, unknown>>(PROVIDERS_SECTION).userValue).toEqual({
|
||||
acme: { type: 'openai', apiKey: 'sk-acme' },
|
||||
});
|
||||
expect(config.get<Record<string, unknown>>(PROVIDERS_SECTION)).toEqual({
|
||||
acme: { type: 'openai', apiKey: 'sk-acme' },
|
||||
});
|
||||
expect(config.inspect<ThinkingConfig>(THINKING_SECTION).userValue).toEqual({ enabled: true });
|
||||
|
||||
disposables.dispose();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -8,12 +8,16 @@
|
|||
* at them all survive;
|
||||
* - concurrent refreshes serialize (never overlap);
|
||||
* - custom-registry fetches carry the host `User-Agent`;
|
||||
* - provider/model patches land in kosong's in-memory registries via
|
||||
* `replaceAll` (the persistence bridge writes them back to config),
|
||||
* merging discovered aliases into user-owned provider records, while
|
||||
* `defaultModel` / `thinking` still go through `config.replace` directly —
|
||||
* restoring a surviving default selection and CLEARing a default (plus its
|
||||
* thinking) whose alias the upstream dropped, never a dangling `set`;
|
||||
* - provider/model patches land in config through ONE atomic
|
||||
* `replaceSections` transition (the persistence bridge then syncs them
|
||||
* into the registries), merging discovered aliases into user-owned
|
||||
* provider records, while `defaultModel` / `thinking` ride the same
|
||||
* transition — restoring a surviving default selection and CLEARing a
|
||||
* default (plus its thinking) whose alias the upstream dropped, never a
|
||||
* dangling `set`;
|
||||
* - the two-phase orchestrator host contract (removeProvider, then
|
||||
* setConfig) never exposes a halfway-removed catalog: the registries
|
||||
* stay untouched until the single atomic write;
|
||||
* - the `[modelCatalog]` config section self-registers and validates.
|
||||
*/
|
||||
|
||||
|
|
@ -22,6 +26,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest';
|
|||
|
||||
import { createScopedTestHost } from '#/_base/di/test';
|
||||
import { isError2 } from '#/_base/errors/errors';
|
||||
import { ILogService, type LogPayload } from '#/_base/log/log';
|
||||
import { IOAuthService } from '#/app/auth/auth';
|
||||
import { IConfigService } from '#/app/config/config';
|
||||
import { ConfigRegistry } from '#/app/config/configService';
|
||||
|
|
@ -29,18 +34,18 @@ import { IEventService } from '#/app/event/event';
|
|||
import { IProviderDiscoveryService } from '#/app/kosongConfig/discovery';
|
||||
import '#/app/kosongConfig/discoveryService';
|
||||
import { MODEL_CATALOG_SECTION } from '#/app/kosongConfig/configSection';
|
||||
import { IKosongConfigService } from '#/app/kosongConfig/kosongConfig';
|
||||
import '#/app/kosongConfig/kosongConfigService';
|
||||
import '#/kosong/model/errors';
|
||||
import { HostRequestHeaders, IHostRequestHeaders } from '#/kosong/model/hostRequestHeaders';
|
||||
import {
|
||||
IModelService,
|
||||
type ModelRecord,
|
||||
type ModelsSection,
|
||||
} from '#/kosong/model/model';
|
||||
import '#/kosong/model/modelService';
|
||||
import {
|
||||
IProviderService,
|
||||
type ProviderConfig,
|
||||
type ProvidersSection,
|
||||
} from '#/kosong/provider/provider';
|
||||
import '#/kosong/provider/providerService';
|
||||
import '#/kosong/provider/providers/kimi/kimi.contrib';
|
||||
|
|
@ -61,37 +66,50 @@ function stubEvents(): IEventService & { published: Array<{ type: string; payloa
|
|||
} as unknown as IEventService & { published: Array<{ type: string; payload: unknown }> };
|
||||
}
|
||||
|
||||
function createHost(
|
||||
function stubLogService(): ILogService {
|
||||
return {
|
||||
_serviceBrand: undefined,
|
||||
level: 'debug',
|
||||
setLevel: () => {},
|
||||
flush: async () => {},
|
||||
error: () => {},
|
||||
warn: () => {},
|
||||
info: () => {},
|
||||
debug: () => {},
|
||||
child: () => {
|
||||
throw new Error('child loggers are not used by KosongConfigService');
|
||||
},
|
||||
} satisfies ILogService;
|
||||
}
|
||||
|
||||
async function createHost(
|
||||
sections: Record<string, unknown> = {},
|
||||
oauth: IOAuthService = stubOAuthService(),
|
||||
): {
|
||||
): Promise<{
|
||||
host: ReturnType<typeof createScopedTestHost>;
|
||||
config: StubConfigService;
|
||||
events: ReturnType<typeof stubEvents>;
|
||||
discovery: IProviderDiscoveryService;
|
||||
providers: IProviderService;
|
||||
models: IModelService;
|
||||
} {
|
||||
}> {
|
||||
const config = new StubConfigService(sections);
|
||||
const events = stubEvents();
|
||||
const host = createScopedTestHost([
|
||||
[IConfigService, config],
|
||||
[IOAuthService, oauth],
|
||||
[IEventService, events],
|
||||
[ILogService, stubLogService()],
|
||||
[IHostRequestHeaders, new HostRequestHeaders({ 'User-Agent': 'kimi-test/1.0' })],
|
||||
]);
|
||||
// Seed the in-memory registries from the fixture sections (the persistence
|
||||
// bridge that normally does this is not instantiated here).
|
||||
const providers = host.app.accessor.get(IProviderService);
|
||||
providers.loadAll(
|
||||
(sections['providers'] ?? {}) as ProvidersSection,
|
||||
sections['defaultProvider'] as string | undefined,
|
||||
);
|
||||
const models = host.app.accessor.get(IModelService);
|
||||
models.loadAll(
|
||||
(sections['models'] ?? {}) as ModelsSection,
|
||||
sections['defaultModel'] as string | undefined,
|
||||
);
|
||||
// The real persistence bridge (DI-activated): refresh writes land in
|
||||
// config, and the bridge's config → kosong sync is what carries them into
|
||||
// the registries (exactly like production). Await its initialization so
|
||||
// the event subscriptions are in place before the test refreshes.
|
||||
const bridge = host.app.accessor.get(IKosongConfigService);
|
||||
await bridge.ready;
|
||||
return {
|
||||
host,
|
||||
config,
|
||||
|
|
@ -125,7 +143,7 @@ describe('refreshProviderModels modelSource short-circuit', () => {
|
|||
it('answers scoped refreshes of static providers with unchanged and no I/O', async () => {
|
||||
const fetchMock = vi.fn();
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
const { host, discovery } = createHost(staticSections);
|
||||
const { host, discovery } = await createHost(staticSections);
|
||||
try {
|
||||
const result = await discovery.refreshProviderModels({ providerId: 'static-p' });
|
||||
expect(result).toEqual({ changed: [], unchanged: ['static-p'], failed: [] });
|
||||
|
|
@ -136,7 +154,7 @@ describe('refreshProviderModels modelSource short-circuit', () => {
|
|||
});
|
||||
|
||||
it('returns an empty result when nothing is refreshable', async () => {
|
||||
const { host, discovery, events } = createHost(staticSections);
|
||||
const { host, discovery, events } = await createHost(staticSections);
|
||||
try {
|
||||
const result = await discovery.refreshProviderModels({ scope: 'all' });
|
||||
expect(result).toEqual({ changed: [], unchanged: [], failed: [] });
|
||||
|
|
@ -164,7 +182,7 @@ describe('refreshProviderModels modelSource short-circuit', () => {
|
|||
);
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
const { host, config, discovery, events, providers, models } = createHost({
|
||||
const { host, config, discovery, events, providers, models } = await createHost({
|
||||
providers: {
|
||||
...staticProviders,
|
||||
acme: {
|
||||
|
|
@ -207,7 +225,7 @@ describe('refreshProviderModels modelSource short-circuit', () => {
|
|||
});
|
||||
|
||||
it('throws provider.not_found for an unknown scoped provider', async () => {
|
||||
const { host, discovery } = createHost(staticSections);
|
||||
const { host, discovery } = await createHost(staticSections);
|
||||
try {
|
||||
await expect(discovery.refreshProviderModels({ providerId: 'missing' })).rejects.toSatisfy(
|
||||
(error) => isError2(error) && error.code === 'provider.not_found',
|
||||
|
|
@ -220,7 +238,7 @@ describe('refreshProviderModels modelSource short-circuit', () => {
|
|||
|
||||
describe('refreshProviderModels write behavior', () => {
|
||||
it('serializes concurrent runs so they never overlap', async () => {
|
||||
const { host, discovery } = createHost(
|
||||
const { host, discovery } = await createHost(
|
||||
{
|
||||
providers: {
|
||||
[KIMI_CODE_PROVIDER_NAME]: {
|
||||
|
|
@ -287,7 +305,7 @@ describe('refreshProviderModels write behavior', () => {
|
|||
);
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
const { host, discovery } = createHost({
|
||||
const { host, discovery } = await createHost({
|
||||
providers: {
|
||||
acme: {
|
||||
type: 'openai',
|
||||
|
|
@ -337,7 +355,7 @@ describe('refreshProviderModels write behavior', () => {
|
|||
);
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
const { host, config, discovery, events, providers, models } = createHost({
|
||||
const { host, config, discovery, events, providers, models } = await createHost({
|
||||
providers: {
|
||||
'my-kimi': { type: 'kimi', baseUrl, apiKey: 'sk-distributed-key' },
|
||||
},
|
||||
|
|
@ -397,7 +415,7 @@ describe('refreshProviderModels write behavior', () => {
|
|||
);
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
const { host, config, discovery, models } = createHost({
|
||||
const { host, config, discovery, models } = await createHost({
|
||||
providers: {
|
||||
'my-kimi': { type: 'kimi', baseUrl, apiKey: 'sk-distributed-key' },
|
||||
},
|
||||
|
|
@ -421,8 +439,8 @@ describe('refreshProviderModels write behavior', () => {
|
|||
]);
|
||||
// The dropped alias was the default: an explicit undefined in the patch
|
||||
// must clear the section instead of leaving the default dangling. It has
|
||||
// to go through `replace` — `set()`'s deepMerge would resolve undefined
|
||||
// back to the stale base value.
|
||||
// to go through a replacing write — `set()`'s deepMerge would resolve
|
||||
// undefined back to the stale base value.
|
||||
expect(config.get('defaultModel')).toBeUndefined();
|
||||
expect(config.get('thinking')).toBeUndefined();
|
||||
const modelRecords = models.list();
|
||||
|
|
@ -432,6 +450,73 @@ describe('refreshProviderModels write behavior', () => {
|
|||
host.dispose();
|
||||
}
|
||||
});
|
||||
|
||||
it('never exposes a halfway-removed catalog: the registries stay untouched until the single atomic write', async () => {
|
||||
const fetchMock = vi.fn(
|
||||
async () =>
|
||||
new Response(
|
||||
JSON.stringify({
|
||||
acme: {
|
||||
id: 'acme',
|
||||
name: 'Acme',
|
||||
api: 'https://acme.example.test/v1',
|
||||
type: 'openai',
|
||||
models: { m2: { id: 'm2', name: 'M2' } },
|
||||
},
|
||||
}),
|
||||
{ headers: { 'Content-Type': 'application/json' } },
|
||||
),
|
||||
);
|
||||
vi.stubGlobal('fetch', fetchMock);
|
||||
|
||||
const { host, config, discovery, providers, models } = await createHost({
|
||||
providers: {
|
||||
acme: {
|
||||
type: 'openai',
|
||||
apiKey: 'sk-acme',
|
||||
source: {
|
||||
kind: 'apiJson',
|
||||
url: 'https://registry.example.test/api.json',
|
||||
apiKey: 'sk-registry',
|
||||
},
|
||||
},
|
||||
},
|
||||
models: {
|
||||
'acme/m1': { provider: 'acme', model: 'm1', maxContextSize: 1000 },
|
||||
},
|
||||
defaultModel: 'acme/m1',
|
||||
});
|
||||
try {
|
||||
// The orchestrator's host contract is two-phase (removeProvider, then
|
||||
// setConfig). By the time the atomic write happens, the removal phase
|
||||
// must NOT have touched the runtime registries — a reader (e.g. a
|
||||
// profile bind racing the refresh) only ever sees the old catalog or
|
||||
// the new one, never a half-removed one.
|
||||
let seenDuringWrite: { providers: readonly string[]; models: readonly string[] } | undefined;
|
||||
const originalReplaceSections = config.replaceSections.bind(config);
|
||||
vi.spyOn(config, 'replaceSections').mockImplementation(async (sections) => {
|
||||
seenDuringWrite = {
|
||||
providers: Object.keys(providers.list()),
|
||||
models: Object.keys(models.list()),
|
||||
};
|
||||
await originalReplaceSections(sections);
|
||||
});
|
||||
|
||||
const result = await discovery.refreshProviderModels({ scope: 'all' });
|
||||
|
||||
expect(result.failed).toEqual([]);
|
||||
expect(seenDuringWrite).toEqual({ providers: ['acme'], models: ['acme/m1'] });
|
||||
expect(vi.mocked(config.replaceSections).mock.calls.length).toBe(1);
|
||||
// After the write, config and the registries converge on the new state:
|
||||
// the dropped alias (and its default selection) is gone everywhere.
|
||||
expect(providers.list()['acme']).toBeDefined();
|
||||
expect(models.list()['acme/m2']).toBeDefined();
|
||||
expect(models.list()['acme/m1']).toBeUndefined();
|
||||
expect(config.get('defaultModel')).toBeUndefined();
|
||||
} finally {
|
||||
host.dispose();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('modelCatalog config section', () => {
|
||||
|
|
|
|||
|
|
@ -69,6 +69,19 @@ export class StubConfigService implements IConfigService {
|
|||
return Promise.resolve();
|
||||
}
|
||||
|
||||
replaceSections(sections: Readonly<Record<string, unknown>>): Promise<void> {
|
||||
for (const [domain, value] of Object.entries(sections)) {
|
||||
const previousValue = this._values.get(domain);
|
||||
if (value === undefined) {
|
||||
this._values.delete(domain);
|
||||
} else {
|
||||
this._values.set(domain, value);
|
||||
}
|
||||
this._onDidChange.fire({ domain, source: 'set', value, previousValue });
|
||||
}
|
||||
return Promise.resolve();
|
||||
}
|
||||
|
||||
/**
|
||||
* Mutate a section WITHOUT firing the change event — simulates a config
|
||||
* write that bypasses the services' change events (the cache-invalidation
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue