From 496e2754835497308da517533414421a0454ab21 Mon Sep 17 00:00:00 2001 From: wangyuxun6699 Date: Tue, 7 Jul 2026 16:23:27 +0800 Subject: [PATCH] fix(config): validate auto-orchestrate router model refs --- ui/server/services/pilotdeckConfig.js | 28 +++-- ui/server/services/pilotdeckConfig.test.js | 85 ++++++++++++- .../view/tabs/PilotDeckConfigTab.cron.test.ts | 23 +++- .../settings/view/tabs/PilotDeckConfigTab.tsx | 83 +------------ .../settings/view/tabs/pilotDeckConfigForm.ts | 114 ++++++++++++++++++ 5 files changed, 244 insertions(+), 89 deletions(-) diff --git a/ui/server/services/pilotdeckConfig.js b/ui/server/services/pilotdeckConfig.js index 0933b4c43..82bbda5c9 100644 --- a/ui/server/services/pilotdeckConfig.js +++ b/ui/server/services/pilotdeckConfig.js @@ -212,16 +212,22 @@ function validateRouterModelRefs(config, errors) { } const tokenSaver = router.tokenSaver; - if (!isRecord(tokenSaver)) return; + if (isRecord(tokenSaver)) { + validateModelRef(config, tokenSaver.judge, 'router.tokenSaver.judge', errors); - validateModelRef(config, tokenSaver.judge, 'router.tokenSaver.judge', errors); - - if (isRecord(tokenSaver.tiers)) { - for (const [key, tier] of Object.entries(tokenSaver.tiers)) { - if (!isRecord(tier)) continue; - validateModelRef(config, tier.model, `router.tokenSaver.tiers.${key}.model`, errors); + if (isRecord(tokenSaver.tiers)) { + for (const [key, tier] of Object.entries(tokenSaver.tiers)) { + if (!isRecord(tier)) continue; + validateModelRef(config, tier.model, `router.tokenSaver.tiers.${key}.model`, errors); + } } } + + const autoOrchestrate = router.autoOrchestrate; + if (isRecord(autoOrchestrate)) { + validateModelRef(config, autoOrchestrate.mainAgentModel, 'router.autoOrchestrate.mainAgentModel', errors); + validateModelRef(config, autoOrchestrate.subagentModel, 'router.autoOrchestrate.subagentModel', errors); + } } function validateGatewayConfig(config, errors, warnings) { @@ -580,6 +586,14 @@ function purgeBootstrapPlaceholder(config) { } } } + if (isRecord(router.autoOrchestrate)) { + if (isOrphanRef(router.autoOrchestrate.mainAgentModel)) { + router.autoOrchestrate.mainAgentModel = agentRef || router.autoOrchestrate.mainAgentModel; + } + if (isOrphanRef(router.autoOrchestrate.subagentModel)) { + router.autoOrchestrate.subagentModel = agentRef || router.autoOrchestrate.subagentModel; + } + } return config; } diff --git a/ui/server/services/pilotdeckConfig.test.js b/ui/server/services/pilotdeckConfig.test.js index 207933b55..ddb330d4c 100644 --- a/ui/server/services/pilotdeckConfig.test.js +++ b/ui/server/services/pilotdeckConfig.test.js @@ -1,5 +1,25 @@ import { describe, expect, it } from 'vitest'; -import { validatePilotDeckConfig } from './pilotdeckConfig.js'; +import fs from 'fs/promises'; +import os from 'os'; +import path from 'path'; +import { validatePilotDeckConfig, writePilotDeckConfig } from './pilotdeckConfig.js'; + +function validConfig(overrides = {}) { + return { + agent: { model: 'new/m' }, + model: { + providers: { + new: { + protocol: 'openai', + url: 'https://example.test/v1', + apiKey: 'key', + models: { m: {} }, + }, + }, + }, + ...overrides, + }; +} describe('validatePilotDeckConfig gateway validation', () => { it('rejects non-object gateway config', () => { @@ -44,3 +64,66 @@ describe('validatePilotDeckConfig gateway validation', () => { expect(validation.errors).toEqual([]); }); }); + +describe('validatePilotDeckConfig router auto-orchestrate refs', () => { + it('rejects unknown mainAgentModel provider refs', () => { + const validation = validatePilotDeckConfig(validConfig({ + router: { + enabled: true, + autoOrchestrate: { + mainAgentModel: 'old/m', + }, + }, + })); + + expect(validation.valid).toBe(false); + expect(validation.errors).toContain( + 'router.autoOrchestrate.mainAgentModel="old/m" doesn\'t resolve to a configured provider/model', + ); + }); + + it('rejects unknown subagentModel provider refs', () => { + const validation = validatePilotDeckConfig(validConfig({ + router: { + enabled: true, + autoOrchestrate: { + subagentModel: 'old/m', + }, + }, + })); + + expect(validation.valid).toBe(false); + expect(validation.errors).toContain( + 'router.autoOrchestrate.subagentModel="old/m" doesn\'t resolve to a configured provider/model', + ); + }); + + it('repairs auto-orchestrate refs orphaned by provider deletion before saving', async () => { + const previousConfigPath = process.env.PILOTDECK_CONFIG_PATH; + const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'pilotdeck-config-')); + process.env.PILOTDECK_CONFIG_PATH = path.join(tempDir, 'pilotdeck.yaml'); + + try { + const result = await writePilotDeckConfig(validConfig({ + router: { + enabled: true, + autoOrchestrate: { + mainAgentModel: 'old/main', + subagentModel: 'old/sub', + }, + }, + })); + + expect(result.validation.valid).toBe(true); + expect(result.config.router.autoOrchestrate.mainAgentModel).toBe('new/m'); + expect(result.config.router.autoOrchestrate.subagentModel).toBe('new/m'); + } finally { + if (previousConfigPath === undefined) { + delete process.env.PILOTDECK_CONFIG_PATH; + } else { + process.env.PILOTDECK_CONFIG_PATH = previousConfigPath; + } + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); +}); diff --git a/ui/src/components/settings/view/tabs/PilotDeckConfigTab.cron.test.ts b/ui/src/components/settings/view/tabs/PilotDeckConfigTab.cron.test.ts index 4fb5c40a6..f78e7c1d5 100644 --- a/ui/src/components/settings/view/tabs/PilotDeckConfigTab.cron.test.ts +++ b/ui/src/components/settings/view/tabs/PilotDeckConfigTab.cron.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { isCronConfigEnabled, patch } from './pilotDeckConfigForm'; +import { isCronConfigEnabled, patch, rewriteProviderRefs } from './pilotDeckConfigForm'; describe('PilotDeckConfigTab Cron settings', () => { it.each([ @@ -27,3 +27,24 @@ describe('PilotDeckConfigTab Cron settings', () => { expect(config).not.toHaveProperty('cron'); }); }); + +describe('PilotDeckConfigTab router refs', () => { + it('rewrites auto-orchestrate model refs when renaming a provider', () => { + const config = { + agent: { model: 'old/main' }, + router: { + autoOrchestrate: { + mainAgentModel: 'old/main', + subagentModel: 'old/sub', + }, + }, + }; + + const updated = rewriteProviderRefs(config, 'old', 'new'); + + expect(updated.router?.autoOrchestrate?.mainAgentModel).toBe('new/main'); + expect(updated.router?.autoOrchestrate?.subagentModel).toBe('new/sub'); + expect(config.router.autoOrchestrate.mainAgentModel).toBe('old/main'); + expect(config.router.autoOrchestrate.subagentModel).toBe('old/sub'); + }); +}); diff --git a/ui/src/components/settings/view/tabs/PilotDeckConfigTab.tsx b/ui/src/components/settings/view/tabs/PilotDeckConfigTab.tsx index 224c9c1d5..5a1dcb8f6 100644 --- a/ui/src/components/settings/view/tabs/PilotDeckConfigTab.tsx +++ b/ui/src/components/settings/view/tabs/PilotDeckConfigTab.tsx @@ -59,7 +59,7 @@ import { } from '../../../../shared/catalogProviders'; import { fetchProviderModels, type ApiModelListItem } from '../../../../shared/modelListApi'; import type { SettingsProject } from '../../types/types'; -import { isCronConfigEnabled, patch } from './pilotDeckConfigForm'; +import { isCronConfigEnabled, patch, rewriteProviderRefs } from './pilotDeckConfigForm'; // ── V2 schema types ──────────────────────────────────────────────────── // Schema mirrors ~/.pilotdeck/pilotdeck.yaml exactly. No more @@ -183,6 +183,8 @@ type PilotDeckConfig = { }; autoOrchestrate?: { enabled?: boolean; + mainAgentModel?: string; + subagentModel?: string; triggerTiers?: string[]; slimSystemPrompt?: boolean; }; @@ -375,85 +377,6 @@ function configToYamlString(config: PilotDeckConfig): string { return stringifyYaml(config, { indent: 2, lineWidth: 0 }); } -function rewriteProviderRef(value: unknown, oldProviderId: string, newProviderId: string): unknown { - const oldPrefix = `${oldProviderId}/`; - if (typeof value !== 'string' || !value.startsWith(oldPrefix)) return value; - return `${newProviderId}/${value.slice(oldPrefix.length)}`; -} - -function rewriteProviderRefs(config: PilotDeckConfig, oldProviderId: string, newProviderId: string): PilotDeckConfig { - let next = config; - - const agentModel = rewriteProviderRef(next.agent?.model, oldProviderId, newProviderId); - if (agentModel !== next.agent?.model) { - next = patch(next, ['agent', 'model'], agentModel); - } - - const subagentDefault = rewriteProviderRef(next.agent?.subagents?.default, oldProviderId, newProviderId); - if (subagentDefault !== next.agent?.subagents?.default) { - next = patch(next, ['agent', 'subagents', 'default'], subagentDefault); - } - - const memoryModel = rewriteProviderRef(next.memory?.model, oldProviderId, newProviderId); - if (memoryModel !== next.memory?.model) { - next = patch(next, ['memory', 'model'], memoryModel); - } - - const memoryLlm = (next.memory as Record | undefined)?.llm; - if (memoryLlm && typeof memoryLlm === 'object' && !Array.isArray(memoryLlm)) { - const llm = memoryLlm as Record; - if (llm.provider === oldProviderId) { - next = patch(next, ['memory', 'llm', 'provider'], newProviderId); - } - } - - const scenarios = next.router?.scenarios; - if (scenarios) { - const rewritten = Object.fromEntries( - Object.entries(scenarios).map(([key, ref]) => [key, rewriteProviderRef(ref, oldProviderId, newProviderId) as string]), - ); - if (Object.entries(scenarios).some(([key, ref]) => rewritten[key] !== ref)) { - next = patch(next, ['router', 'scenarios'], rewritten); - } - } - - const fallback = next.router?.fallback; - if (fallback) { - const rewritten = Object.fromEntries( - Object.entries(fallback).map(([key, refs]) => [ - key, - refs.map((ref) => rewriteProviderRef(ref, oldProviderId, newProviderId) as string), - ]), - ); - if (Object.entries(fallback).some(([key, refs]) => rewritten[key].some((ref, index) => ref !== refs[index]))) { - next = patch(next, ['router', 'fallback'], rewritten); - } - } - - const judge = rewriteProviderRef(next.router?.tokenSaver?.judge, oldProviderId, newProviderId); - if (judge !== next.router?.tokenSaver?.judge) { - next = patch(next, ['router', 'tokenSaver', 'judge'], judge); - } - - const tiers = next.router?.tokenSaver?.tiers; - if (tiers) { - const rewritten = Object.fromEntries( - Object.entries(tiers).map(([key, tier]) => [ - key, - { - ...tier, - model: rewriteProviderRef(tier.model, oldProviderId, newProviderId) as string | undefined, - }, - ]), - ); - if (Object.entries(tiers).some(([key, tier]) => rewritten[key].model !== tier.model)) { - next = patch(next, ['router', 'tokenSaver', 'tiers'], rewritten); - } - } - - return next; -} - function replaceFallbackModelRef(config: PilotDeckConfig, oldRef: string, newRef: string): PilotDeckConfig { const fallback = config.router?.fallback; if (!fallback || !oldRef || oldRef === newRef) return config; diff --git a/ui/src/components/settings/view/tabs/pilotDeckConfigForm.ts b/ui/src/components/settings/view/tabs/pilotDeckConfigForm.ts index 5f70ec48b..9539afa5c 100644 --- a/ui/src/components/settings/view/tabs/pilotDeckConfigForm.ts +++ b/ui/src/components/settings/view/tabs/pilotDeckConfigForm.ts @@ -6,6 +6,31 @@ type CronConfigShape = { }; }; +type ProviderRefConfig = { + agent?: { + model?: string; + subagents?: { + default?: string; + }; + }; + memory?: { + model?: string; + llm?: unknown; + }; + router?: { + scenarios?: Record; + fallback?: Record; + tokenSaver?: { + judge?: string; + tiers?: Record; + }; + autoOrchestrate?: { + mainAgentModel?: string; + subagentModel?: string; + }; + }; +}; + export function patch(config: T, path: Path, value: unknown): T { // Immutable deep set. Each key cloned along the way so React picks up the // change. Numeric segments materialise arrays; everything else materialises @@ -28,3 +53,92 @@ export function patch(config: T, path: Path, value: unknown): T { export function isCronConfigEnabled(config: CronConfigShape): boolean { return config.cron !== undefined && config.cron.enabled !== false; } + +function rewriteProviderRef(value: unknown, oldProviderId: string, newProviderId: string): unknown { + const oldPrefix = `${oldProviderId}/`; + if (typeof value !== 'string' || !value.startsWith(oldPrefix)) return value; + return `${newProviderId}/${value.slice(oldPrefix.length)}`; +} + +export function rewriteProviderRefs(config: T, oldProviderId: string, newProviderId: string): T { + let next = config; + + const agentModel = rewriteProviderRef(next.agent?.model, oldProviderId, newProviderId); + if (agentModel !== next.agent?.model) { + next = patch(next, ['agent', 'model'], agentModel); + } + + const subagentDefault = rewriteProviderRef(next.agent?.subagents?.default, oldProviderId, newProviderId); + if (subagentDefault !== next.agent?.subagents?.default) { + next = patch(next, ['agent', 'subagents', 'default'], subagentDefault); + } + + const memoryModel = rewriteProviderRef(next.memory?.model, oldProviderId, newProviderId); + if (memoryModel !== next.memory?.model) { + next = patch(next, ['memory', 'model'], memoryModel); + } + + const memoryLlm = next.memory?.llm; + if (memoryLlm && typeof memoryLlm === 'object' && !Array.isArray(memoryLlm)) { + const llm = memoryLlm as Record; + if (llm.provider === oldProviderId) { + next = patch(next, ['memory', 'llm', 'provider'], newProviderId); + } + } + + const scenarios = next.router?.scenarios; + if (scenarios) { + const rewritten = Object.fromEntries( + Object.entries(scenarios).map(([key, ref]) => [key, rewriteProviderRef(ref, oldProviderId, newProviderId) as string]), + ); + if (Object.entries(scenarios).some(([key, ref]) => rewritten[key] !== ref)) { + next = patch(next, ['router', 'scenarios'], rewritten); + } + } + + const fallback = next.router?.fallback; + if (fallback) { + const rewritten = Object.fromEntries( + Object.entries(fallback).map(([key, refs]) => [ + key, + refs.map((ref) => rewriteProviderRef(ref, oldProviderId, newProviderId) as string), + ]), + ); + if (Object.entries(fallback).some(([key, refs]) => rewritten[key].some((ref, index) => ref !== refs[index]))) { + next = patch(next, ['router', 'fallback'], rewritten); + } + } + + const judge = rewriteProviderRef(next.router?.tokenSaver?.judge, oldProviderId, newProviderId); + if (judge !== next.router?.tokenSaver?.judge) { + next = patch(next, ['router', 'tokenSaver', 'judge'], judge); + } + + const tiers = next.router?.tokenSaver?.tiers; + if (tiers) { + const rewritten = Object.fromEntries( + Object.entries(tiers).map(([key, tier]) => [ + key, + { + ...tier, + model: rewriteProviderRef(tier.model, oldProviderId, newProviderId) as string | undefined, + }, + ]), + ); + if (Object.entries(tiers).some(([key, tier]) => rewritten[key].model !== tier.model)) { + next = patch(next, ['router', 'tokenSaver', 'tiers'], rewritten); + } + } + + const mainAgentModel = rewriteProviderRef(next.router?.autoOrchestrate?.mainAgentModel, oldProviderId, newProviderId); + if (mainAgentModel !== next.router?.autoOrchestrate?.mainAgentModel) { + next = patch(next, ['router', 'autoOrchestrate', 'mainAgentModel'], mainAgentModel); + } + + const subagentModel = rewriteProviderRef(next.router?.autoOrchestrate?.subagentModel, oldProviderId, newProviderId); + if (subagentModel !== next.router?.autoOrchestrate?.subagentModel) { + next = patch(next, ['router', 'autoOrchestrate', 'subagentModel'], subagentModel); + } + + return next; +}