Skip to content

Commit 68159b7

Browse files
committed
fix(agent-core-v2): keep running sessions' models alive across concurrent provider refreshes
1 parent ff7ed1c commit 68159b7

8 files changed

Lines changed: 201 additions & 114 deletions

File tree

.changeset/olive-camels-refresh.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@moonshot-ai/kimi-code": patch
3+
---
4+
5+
Fix models in use by a running session failing with "not configured in config.toml" when another Kimi Code process refreshes the provider model list at the same time.

apps/kimi-code/src/tui/controllers/auth-flow.ts

Lines changed: 53 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -217,10 +217,11 @@ export class AuthFlowController {
217217
* `replaceSections`), the orchestrator's two-phase contract (removeProvider
218218
* then setConfig) is absorbed the same way the v2 engine's own refresh path
219219
* does it: the removal is staged in memory only, and the following
220-
* setConfig persists the complete records in a single write — so a process
221-
* exit mid-refresh can never leave config.toml in a "provider removed, not
222-
* yet restored" state. The v1 harness keeps the legacy host (two
223-
* whole-document writes, each atomic on its own).
220+
* setConfig merges the sparse patch onto a fresh read and persists
221+
* everything in a single write — so a process exit mid-refresh can never
222+
* leave config.toml in a "provider removed, not yet restored" state, and
223+
* concurrent writes from other processes survive. The v1 harness keeps the
224+
* legacy host (two whole-document writes, each atomic on its own).
224225
*/
225226
private buildRefreshHost(): RefreshProviderHost {
226227
const { host } = this;
@@ -239,6 +240,7 @@ export class AuthFlowController {
239240
};
240241
}
241242
let staged: KimiConfig | undefined;
243+
const pendingRemovals = new Set<string>();
242244
const requireStaged = (): KimiConfig => {
243245
if (staged === undefined) {
244246
throw new Error('refresh host: getConfig must be called before writes');
@@ -251,17 +253,60 @@ export class AuthFlowController {
251253
return staged;
252254
},
253255
removeProvider: (id) => {
256+
pendingRemovals.add(id);
254257
staged = removeProviderFromConfig(requireStaged(), id);
255258
return Promise.resolve(staged);
256259
},
257260
setConfig: async (patch) => {
258-
// The orchestrator always passes complete records (built from a full
259-
// clone), so the Partial-shaped patch is a full KimiConfig overlay.
260-
staged = { ...requireStaged(), ...patch } as KimiConfig;
261+
// The patch is sparse — only the refreshed providers' entries and
262+
// their owned aliases — so merge it onto a fresh read for unrelated
263+
// keys to survive, and fold the staged removals into the same write.
264+
const fresh = await host.harness.getConfig({ reload: true });
265+
const sections: Record<string, unknown> = {};
266+
if (pendingRemovals.size > 0 || patch.providers !== undefined) {
267+
const providers: Record<string, unknown> = { ...fresh.providers };
268+
for (const id of pendingRemovals) delete providers[id];
269+
if (patch.providers !== undefined) Object.assign(providers, patch.providers);
270+
sections['providers'] = providers;
271+
}
272+
if (pendingRemovals.size > 0 || patch.models !== undefined) {
273+
const models: Record<string, unknown> = { ...fresh.models };
274+
for (const [key, record] of Object.entries(models)) {
275+
const owner =
276+
typeof record === 'object' && record !== null
277+
? (record as { provider?: string }).provider
278+
: undefined;
279+
if (owner !== undefined && pendingRemovals.has(owner)) delete models[key];
280+
}
281+
if (patch.models !== undefined) Object.assign(models, patch.models);
282+
sections['models'] = models;
283+
}
261284
// Object.entries keeps keys whose value is `undefined`, so a cleared
262285
// section (e.g. a dangling defaultModel) is expressed as a removal in
263286
// the atomic write; sections absent from the patch stay untouched.
264-
await host.harness.replaceConfigSections(Object.fromEntries(Object.entries(patch)));
287+
for (const [key, value] of Object.entries(patch)) {
288+
if (key === 'providers' || key === 'models') continue;
289+
sections[key] = value;
290+
}
291+
if (pendingRemovals.size > 0 && !('defaultModel' in patch)) {
292+
const defaultModel = fresh.defaultModel;
293+
const owner =
294+
defaultModel === undefined
295+
? undefined
296+
: (fresh.models?.[defaultModel] as { provider?: string } | undefined)?.provider;
297+
if (owner !== undefined && pendingRemovals.has(owner)) {
298+
sections['defaultModel'] = undefined;
299+
}
300+
}
301+
if (pendingRemovals.size > 0 && !('defaultProvider' in patch)) {
302+
const defaultProvider = fresh.defaultProvider;
303+
if (defaultProvider !== undefined && pendingRemovals.has(defaultProvider)) {
304+
sections['defaultProvider'] = undefined;
305+
}
306+
}
307+
pendingRemovals.clear();
308+
await host.harness.replaceConfigSections(sections);
309+
staged = { ...fresh, ...sections } as KimiConfig;
265310
return staged;
266311
},
267312
resolveOAuthToken,

apps/kimi-code/test/tui/utils/refresh-providers.test.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,18 @@ function makeRefreshHost(initial: KimiConfig): {
4040
return structuredClone(persisted);
4141
});
4242
const setConfig = vi.fn(async (patch: Partial<KimiConfig>) => {
43-
persisted = { ...persisted, ...patch };
43+
const next = { ...persisted };
44+
if (patch.providers !== undefined) {
45+
next.providers = { ...persisted.providers, ...patch.providers };
46+
}
47+
if (patch.models !== undefined) {
48+
next.models = { ...persisted.models, ...patch.models };
49+
}
50+
for (const [key, value] of Object.entries(patch)) {
51+
if (key === 'providers' || key === 'models') continue;
52+
(next as Record<string, unknown>)[key] = value;
53+
}
54+
persisted = next;
4455
return structuredClone(persisted);
4556
});
4657
return {

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

Lines changed: 46 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import { getProviderDefinition } from '#/kosong/provider/providerDefinition';
2525

2626
import {
2727
DEFAULT_MODEL_SECTION,
28+
DEFAULT_PROVIDER_SECTION,
2829
MODELS_SECTION,
2930
PROVIDERS_SECTION,
3031
THINKING_SECTION,
@@ -142,10 +143,11 @@ export class ProviderDiscoveryService implements IProviderDiscoveryService {
142143
}
143144

144145
private buildRefreshHost(exclusion: StaticExclusion, userAgent: string): RefreshProviderHost {
146+
const pendingRemovals = new Set<string>();
145147
return {
146148
getConfig: async () => this.readUserConfigShape(exclusion),
147-
removeProvider: (providerId) => this.shapeWithoutProvider(providerId),
148-
setConfig: (patch) => this.applyRefreshPatch(patch, exclusion),
149+
removeProvider: (providerId) => this.queueProviderRemoval(pendingRemovals, providerId),
150+
setConfig: (patch) => this.applyRefreshPatch(patch, pendingRemovals),
149151
resolveOAuthToken: (providerName, oauthRef) => this.resolveOAuthToken(providerName, oauthRef),
150152
userAgent,
151153
};
@@ -173,7 +175,11 @@ export class ProviderDiscoveryService implements IProviderDiscoveryService {
173175
};
174176
}
175177

176-
private shapeWithoutProvider(providerId: string): Promise<ManagedKimiConfigShape> {
178+
private queueProviderRemoval(
179+
pendingRemovals: Set<string>,
180+
providerId: string,
181+
): Promise<ManagedKimiConfigShape> {
182+
pendingRemovals.add(providerId);
177183
const current = this.readUserConfigShape();
178184
const providers = current.providers as Record<string, ProviderConfig>;
179185
const restProviders = Object.fromEntries(
@@ -192,57 +198,54 @@ export class ProviderDiscoveryService implements IProviderDiscoveryService {
192198

193199
private async applyRefreshPatch(
194200
patch: ManagedKimiConfigShape,
195-
exclusion: StaticExclusion,
201+
pendingRemovals: Set<string>,
196202
): Promise<ManagedKimiConfigShape> {
197-
const userProviders =
203+
await this.config.reload();
204+
const removals = [...pendingRemovals];
205+
pendingRemovals.clear();
206+
const removed = new Set(removals);
207+
const providers =
198208
this.config.inspect<Record<string, ProviderConfig>>(PROVIDERS_SECTION).userValue ?? {};
199-
const userModels =
209+
const models =
200210
this.config.inspect<Record<string, ModelRecord>>(MODELS_SECTION).userValue ?? {};
201211
const sections: Record<string, unknown> = {};
202-
if (patch.providers !== undefined) {
203-
sections[PROVIDERS_SECTION] = {
204-
...exclusion.providers,
205-
...patch.providers,
206-
};
212+
if (removals.length > 0 || patch.providers !== undefined) {
213+
const nextProviders: Record<string, unknown> = Object.fromEntries(
214+
Object.entries(providers).filter(([id]) => !removed.has(id)),
215+
);
216+
if (patch.providers !== undefined) Object.assign(nextProviders, patch.providers);
217+
sections[PROVIDERS_SECTION] = nextProviders;
207218
}
208-
if (patch.models !== undefined) {
209-
sections[MODELS_SECTION] = {
210-
...exclusion.models,
211-
...(patch.models as Record<string, ModelRecord>),
212-
};
219+
if (removals.length > 0 || patch.models !== undefined) {
220+
const nextModels: Record<string, unknown> = Object.fromEntries(
221+
Object.entries(models).filter(
222+
([, record]) => record.provider === undefined || !removed.has(record.provider),
223+
),
224+
);
225+
if (patch.models !== undefined) Object.assign(nextModels, patch.models);
226+
sections[MODELS_SECTION] = nextModels;
213227
}
214-
const restoreDefault = exclusion.defaultModel !== undefined;
215228
if ('defaultModel' in patch) {
216-
sections[DEFAULT_MODEL_SECTION] = restoreDefault
217-
? exclusion.defaultModel
218-
: patch.defaultModel;
229+
sections[DEFAULT_MODEL_SECTION] = patch.defaultModel;
230+
} else if (removals.length > 0) {
231+
const defaultModel = this.config.inspect<string>(DEFAULT_MODEL_SECTION).userValue;
232+
if (defaultModel !== undefined && removed.has(models[defaultModel]?.provider ?? '')) {
233+
sections[DEFAULT_MODEL_SECTION] = undefined;
234+
}
219235
}
220236
if ('thinking' in patch) {
221-
sections[THINKING_SECTION] = restoreDefault ? exclusion.thinking : patch.thinking;
237+
sections[THINKING_SECTION] = patch.thinking;
238+
}
239+
if ('defaultProvider' in patch) {
240+
sections[DEFAULT_PROVIDER_SECTION] = patch['defaultProvider'];
241+
} else if (removals.length > 0) {
242+
const defaultProvider = this.config.inspect<string>(DEFAULT_PROVIDER_SECTION).userValue;
243+
if (defaultProvider !== undefined && removed.has(defaultProvider)) {
244+
sections[DEFAULT_PROVIDER_SECTION] = undefined;
245+
}
222246
}
223247
await this.config.replaceSections(sections);
224-
return {
225-
providers:
226-
patch.providers !== undefined
227-
? ({ ...exclusion.providers, ...patch.providers } as ManagedKimiConfigShape['providers'])
228-
: (userProviders as ManagedKimiConfigShape['providers']),
229-
models:
230-
patch.models !== undefined
231-
? ({ ...exclusion.models, ...patch.models } as ManagedKimiConfigShape['models'])
232-
: (userModels as ManagedKimiConfigShape['models']),
233-
defaultModel:
234-
'defaultModel' in patch
235-
? restoreDefault
236-
? exclusion.defaultModel
237-
: patch.defaultModel
238-
: this.config.inspect<string>(DEFAULT_MODEL_SECTION).userValue,
239-
thinking:
240-
'thinking' in patch
241-
? restoreDefault
242-
? exclusion.thinking
243-
: patch.thinking
244-
: this.config.inspect<ManagedKimiConfigShape['thinking']>(THINKING_SECTION).userValue,
245-
};
248+
return this.readUserConfigShape();
246249
}
247250

248251
private async resolveOAuthToken(

packages/agent-core-v2/src/kosong/model/catalogService.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,9 @@ export class ModelCatalog extends Disposable implements IModelCatalog {
9696
}
9797

9898
notifyConfigChanged(): void {
99-
this.cache.clear();
99+
for (const id of this.cache.keys()) {
100+
if (this.models.get(id) !== undefined) this.cache.delete(id);
101+
}
100102
}
101103

102104
get(id: string): Model {

packages/agent-core-v2/test/app/kosongConfig/discovery.test.ts

Lines changed: 35 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -639,69 +639,53 @@ describe('refreshProviderModels defaultModel self-heal', () => {
639639
}
640640
});
641641

642-
it('keeps a default model the user selected while the catalog fetch was in flight', async () => {
643-
const twoModels = {
644-
'kimi-code/kimi-k2': {
645-
provider: KIMI_CODE_PROVIDER_NAME,
646-
model: 'kimi-k2',
647-
maxContextSize: 131072,
648-
capabilities: ['thinking', 'tool_use'],
649-
displayName: 'Kimi K2',
650-
},
651-
'kimi-code/kimi-k3': {
652-
provider: KIMI_CODE_PROVIDER_NAME,
653-
model: 'kimi-k3',
654-
maxContextSize: 131072,
655-
capabilities: ['thinking', 'tool_use'],
656-
displayName: 'Kimi K3',
657-
},
658-
};
659-
const { host, config, discovery, events } = await createHost(
642+
it('keeps user writes that landed while the catalog fetch was in flight', async () => {
643+
const { host, config, discovery, models } = await createHost(
660644
{
661645
providers: managedProviders,
662-
models: twoModels,
646+
models: managedModels,
663647
},
664648
stubOAuthService(stubTokenProvider(['access-token'])),
665649
);
666650
try {
667651
vi.stubGlobal(
668652
'fetch',
669-
vi.fn(
670-
async () => {
671-
await config.set('defaultModel', 'kimi-code/kimi-k3');
672-
return new Response(
673-
JSON.stringify({
674-
data: [
675-
{
676-
id: 'kimi-k2',
677-
context_length: 131072,
678-
supports_reasoning: true,
679-
display_name: 'Kimi K2',
680-
},
681-
{
682-
id: 'kimi-k3',
683-
context_length: 131072,
684-
supports_reasoning: true,
685-
display_name: 'Kimi K3',
686-
},
687-
],
688-
}),
689-
{ status: 200, headers: { 'Content-Type': 'application/json' } },
690-
);
691-
},
692-
),
653+
vi.fn(async () => {
654+
await config.set('defaultModel', 'kimi-code/kimi-k3');
655+
await config.set('models', {
656+
'other/keep': { provider: 'other', model: 'keep', maxContextSize: 1000 },
657+
});
658+
return new Response(
659+
JSON.stringify({
660+
data: [
661+
{
662+
id: 'kimi-k2',
663+
context_length: 131072,
664+
supports_reasoning: true,
665+
display_name: 'Kimi K2',
666+
},
667+
{
668+
id: 'kimi-k3',
669+
context_length: 131072,
670+
supports_reasoning: true,
671+
display_name: 'Kimi K3',
672+
},
673+
],
674+
}),
675+
{ status: 200, headers: { 'Content-Type': 'application/json' } },
676+
);
677+
}),
693678
);
694-
const replaceSections = vi.spyOn(config, 'replaceSections');
695679
const result = await discovery.refreshProviderModels({ scope: 'all' });
696680

697-
expect(result).toEqual({
698-
changed: [],
699-
unchanged: [KIMI_CODE_PROVIDER_NAME],
700-
failed: [],
701-
});
702-
expect(replaceSections).not.toHaveBeenCalled();
703-
expect(events.published).toEqual([]);
681+
expect(result.failed).toEqual([]);
682+
expect(result.changed).toEqual([
683+
{ provider_id: KIMI_CODE_PROVIDER_NAME, provider_name: 'Kimi Code', added: 1, removed: 0 },
684+
]);
704685
expect(config.get<string>('defaultModel')).toBe('kimi-code/kimi-k3');
686+
const modelRecords = models.list();
687+
expect(modelRecords['kimi-code/kimi-k3']).toBeDefined();
688+
expect(modelRecords['other/keep']).toBeDefined();
705689
} finally {
706690
host.dispose();
707691
}

packages/agent-core-v2/test/kosong/model/catalog.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,7 @@ describe('ModelCatalog caching and config-event invalidation', () => {
517517
}
518518
});
519519

520-
it('drops the cache when a watched config section changes', async () => {
520+
it('drops the cache on edits but keeps entries whose model was removed', async () => {
521521
const { host, catalog, models, providers } = createHost(kimiSections);
522522
try {
523523
const before = catalog.get('k1');
@@ -528,6 +528,10 @@ describe('ModelCatalog caching and config-event invalidation', () => {
528528

529529
await providers.set('kimi', { type: 'kimi', apiKey: 'sk-2', baseUrl: 'https://other.example.test/v1' });
530530
expect(catalog.get('k1').baseUrl).toBe('https://other.example.test/v1');
531+
532+
const updated = catalog.get('k1');
533+
await models.delete('k1');
534+
expect(catalog.get('k1')).toBe(updated);
531535
} finally {
532536
host.dispose();
533537
}

0 commit comments

Comments
 (0)