From d3cc47cdb78444b76150acce36665467608f25bc Mon Sep 17 00:00:00 2001 From: NicolasIppoliti Date: Thu, 10 Sep 2026 12:24:31 -0300 Subject: [PATCH] refactor(models): migrate saved routing consumers to shared authority --- extensions/gentle-ai.ts | 20 +++- tests/gentle-ai.test.ts | 159 +++++++++++++++++++++++++- tests/model-routing-authority.test.ts | 12 ++ 3 files changed, 185 insertions(+), 6 deletions(-) diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index 47b665bb6..0813b1296 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -1785,7 +1785,10 @@ function parseModelExport(value: unknown): AgentModelConfig | undefined { } async function exportSavedModelConfig(ctx: ExtensionContext): Promise { - const saved = await readSavedModelConfigAsync(ctx.cwd); + const saved = await readModelRoutingAuthorityAsync( + modelConfigPath(ctx.cwd), + legacyProjectModelConfigPath(ctx.cwd), + ); if (saved.status === "invalid") throw new Error(`Invalid model config: ${saved.path}`); const agents = saved.status === "valid" ? saved.config : {}; const path = modelExportPath(ctx.cwd); @@ -2863,7 +2866,10 @@ async function showSddModelPanel( async function handleModelsCommand(ctx: ExtensionContext): Promise { migrateLegacyProjectModelOverrides(ctx.cwd); - const savedConfig = await readSavedModelConfigAsync(ctx.cwd); + const savedConfig = await readModelRoutingAuthorityAsync( + modelConfigPath(ctx.cwd), + legacyProjectModelConfigPath(ctx.cwd), + ); if (savedConfig.status === "invalid") { ctx.ui.notify( `el Gentleman cannot open model config because ${savedConfig.path} is invalid JSON or not an object. Fix or remove the file, then run /gentle:models again.`, @@ -7350,7 +7356,10 @@ function createGentleAiExtensionForTesting( const openspecConfigured = existsSync( join(ctx.cwd, "openspec", "config.yaml"), ); - const modelConfig = await readModelConfigAsync(ctx.cwd); + const savedConfig = await readModelRoutingAuthorityAsync( + modelConfigPath(ctx.cwd), + legacyProjectModelConfigPath(ctx.cwd), + ); const devBinary = await describeDevBinaryOverride(); ctx.ui.notify( [ @@ -7360,9 +7369,10 @@ function createGentleAiExtensionForTesting( ...assetLines, `OpenSpec config: ${openspecConfigured ? "present" : "missing"}`, `Global model config: ${existsSync(modelConfigPath(ctx.cwd)) ? "present" : "missing"}`, - ...describeModelConfig(ctx.cwd, modelConfig), + `Saved model routing: ${savedConfig.status}${savedConfig.status === "invalid" ? ` (${savedConfig.path})` : ""}`, + ...(savedConfig.status === "invalid" ? [] : describeModelConfig(ctx.cwd, savedConfig.status === "valid" ? savedConfig.config : {})), ].join("\n"), - assetLines.some((line) => line.startsWith("warn:")) || devBinary.state !== "inactive" ? "warning" : "info", + savedConfig.status === "invalid" || assetLines.some((line) => line.startsWith("warn:")) || devBinary.state !== "inactive" ? "warning" : "info", ); }, }); diff --git a/tests/gentle-ai.test.ts b/tests/gentle-ai.test.ts index 75591ade9..274503868 100644 --- a/tests/gentle-ai.test.ts +++ b/tests/gentle-ai.test.ts @@ -1,6 +1,6 @@ import assert from "node:assert/strict"; import { createHash } from "node:crypto"; -import { mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import test from "node:test"; @@ -204,6 +204,148 @@ test("registered Gentle Review tools preserve result envelopes and redact collap } }); +function routingConsumerFixture(t: test.TestContext) { + const root = mkdtempSync(join(tmpdir(), "gentle-pi-routing-consumers-")); + const configHome = join(root, "global"); + const agentHome = join(root, "agent-home"); + const projectPath = join(root, ".pi", "gentle-ai", "models.json"); + const globalPath = join(configHome, "models.json"); + const exportPath = join(configHome, "models.export.json"); + for (const dir of [dirname(projectPath), join(root, "agents"), join(agentHome, "agents"), join(agentHome, "subagents")]) { + mkdirSync(dir, { recursive: true }); + } + writeMarkdown(join(root, ".pi", "agents", "worker.md"), "---\nname: worker\ndescription: Worker\n---\nbody\n"); + const previousConfigHome = process.env.GENTLE_PI_CONFIG_HOME; + const previousAgentHome = process.env.GENTLE_PI_AGENT_HOME; + process.env.GENTLE_PI_CONFIG_HOME = configHome; + process.env.GENTLE_PI_AGENT_HOME = agentHome; + t.after(() => { + if (previousConfigHome === undefined) delete process.env.GENTLE_PI_CONFIG_HOME; + else process.env.GENTLE_PI_CONFIG_HOME = previousConfigHome; + if (previousAgentHome === undefined) delete process.env.GENTLE_PI_AGENT_HOME; + else process.env.GENTLE_PI_AGENT_HOME = previousAgentHome; + rmSync(root, { recursive: true, force: true }); + }); + const commands = new Map[1]>(); + createGentleAiExtension({ nativeReviewCli: null })({ + on() {}, + registerTool() {}, + registerCommand(name, command) { commands.set(name, command); }, + } as ExtensionAPI); + const notifications: Array<{ message: string; severity: string }> = []; + let panelVisits = 0; + const panels: string[] = []; + let onPanel = () => ({ type: "cancel", config: {} }); + const ctx = { + cwd: root, + hasUI: true, + modelRegistry: { getAvailable: async () => [] }, + ui: { + notify(message: string, severity: string) { notifications.push({ message, severity }); }, + custom: async (factory: (tui: unknown, theme: Theme, keybindings: unknown, done: () => void) => { render(width: number): string[] }) => { + panels.push(stripAnsi(renderComponent(factory(undefined, { fg: (_color: string, text: string) => text } as unknown as Theme, undefined, () => {})))); + panelVisits += 1; + return onPanel(); + }, + }, + } as unknown as Parameters[1]["handler"]>[1]; + return { + configHome, projectPath, globalPath, exportPath, notifications, panels, + panelVisits: () => panelVisits, + onPanel(action: typeof onPanel) { onPanel = action; }, + run: (name: string) => commands.get(name)!.handler("", ctx), + }; +} + +test("models rejects invalid project routing with its selected source path", async (t) => { + const fixture = routingConsumerFixture(t); + writeFileSync(fixture.projectPath, "[]"); + await fixture.run("gentle:models"); + assert.equal(fixture.notifications[0]?.severity, "warning"); + assert.ok(fixture.notifications[0]?.message.includes(fixture.projectPath)); + assert.equal(fixture.panelVisits(), 0); +}); + +test("export re-reads saved routing and rejects invalid project before creating its destination parent", async (t) => { + const fixture = routingConsumerFixture(t); + writeFileSync(fixture.projectPath, '{"worker":"openai/gpt-5"}'); + fixture.onPanel(() => { + if (fixture.panelVisits() > 1) return { type: "cancel", config: {} }; + writeFileSync(fixture.projectPath, "[]"); + return { type: "export", config: {} }; + }); + await fixture.run("gentle:models"); + assert.equal(fixture.notifications[0]?.severity, "warning"); + assert.ok(fixture.notifications[0]?.message.includes(`Invalid model config: ${fixture.projectPath}`)); + assert.equal(existsSync(fixture.exportPath), false); + assert.equal(existsSync(fixture.configHome), false); +}); + +test("status reports invalid saved routing path instead of default agent routing", async (t) => { + const fixture = routingConsumerFixture(t); + writeFileSync(fixture.projectPath, "[]"); + await fixture.run("gentle:status"); + const report = fixture.notifications.at(-1)!; + assert.match(report.message, /Saved model routing: invalid/); + assert.ok(report.message.includes(fixture.projectPath)); + assert.equal(report.severity, "warning"); + assert.doesNotMatch(report.message, /worker: model=/); +}); + +test("models exports missing, normalized project, and global-precedence saved routing", async (t) => { + for (const source of ["missing", "project", "global"] as const) { + await t.test(source, async (t) => { + const fixture = routingConsumerFixture(t); + if (source !== "missing") writeFileSync(fixture.projectPath, '{"worker":" openai/gpt-5 ","ignored":null}'); + if (source === "global") writeMarkdown(fixture.globalPath, '{"worker":{"model":" anthropic/opus ","thinking":"high"}}'); + fixture.onPanel(() => ({ type: fixture.panelVisits() === 1 ? "export" : "cancel", config: {} })); + await fixture.run("gentle:models"); + const agents = source === "missing" ? {} : source === "project" + ? { worker: { model: "openai/gpt-5" } } + : { worker: { model: "anthropic/opus", thinking: "high" } }; + assert.deepEqual(JSON.parse(readFileSync(fixture.exportPath, "utf8")).agents, agents); + assert.equal(fixture.notifications[0]?.severity, "info"); + assert.match(fixture.notifications[0]!.message, /exported/); + assert.equal(fixture.panelVisits(), 2); + await fixture.run("gentle:status"); + const report = fixture.notifications.at(-1)!.message; + assert.ok(report.includes(`Saved model routing: ${source === "missing" ? "missing" : "valid"}`)); + assert.ok(report.includes(`Global model config: ${source === "global" ? "present" : "missing"}`)); + const expectedRouting = source === "missing" ? "inherit, effort=inherit" + : source === "project" ? "openai/gpt-5, effort=inherit" : "anthropic/opus, effort=high"; + assert.ok(report.includes(`worker: model=${expectedRouting}`), report); + assert.ok(fixture.panels[0].includes(`model=${expectedRouting}`), fixture.panels[0]); + }); + } +}); + +test("invalid global routing overrides valid project in models, status, and export re-read", async (t) => { + for (const atExport of [false, true]) { + await t.test(atExport ? "invalidated during panel" : "invalid before panel", async (t) => { + const fixture = routingConsumerFixture(t); + writeFileSync(fixture.projectPath, '{"worker":"openai/gpt-5"}'); + if (!atExport) writeMarkdown(fixture.globalPath, "[]"); + fixture.onPanel(() => { + if (fixture.panelVisits() > 1) return { type: "cancel", config: {} }; + writeMarkdown(fixture.globalPath, "[]"); + return { type: "export", config: {} }; + }); + await fixture.run("gentle:models"); + assert.equal(fixture.notifications[0]?.severity, "warning"); + assert.ok(fixture.notifications[0]?.message.includes(fixture.globalPath)); + assert.match(fixture.notifications[0]!.message, atExport ? /export failed/ : /cannot open model config/); + assert.equal(fixture.panelVisits(), atExport ? 2 : 0); + assert.equal(existsSync(fixture.exportPath), false); + await fixture.run("gentle:status"); + const report = fixture.notifications.at(-1)!; + assert.match(report.message, /Global model config: present\nSaved model routing: invalid/); + assert.ok(report.message.includes(fixture.globalPath)); + assert.doesNotMatch(report.message, /worker: model=/); + assert.equal(report.severity, "warning"); + }); + } +}); + test("session startup reports invalid project routing without mutating the profile", async (t) => { const root = mkdtempSync(join(tmpdir(), "gentle-pi-model-routing-startup-")); const configHome = join(root, "global"); @@ -268,6 +410,21 @@ test("session startup reports invalid project routing without mutating the profi assert.equal(warning!.severity, "warning"); assert.match(warning!.message, /skipped model config/); assert.equal(readFileSync(profilePath, "utf8"), before); + + writeFileSync(join(projectConfigDir, "models.json"), '{"worker":"openai/gpt-5"}'); + const globalPath = join(configHome, "models.json"); + writeFileSync(globalPath, "[]"); + notifications.length = 0; + await sessionStart!({}, { + cwd: root, + hasUI: true, + ui: { notify(message: string, severity: string) { notifications.push({ message, severity }); } }, + } as unknown as ExtensionContext); + const globalWarning = notifications.find((entry) => entry.message.includes(globalPath)); + assert.ok(globalWarning, JSON.stringify(notifications)); + assert.equal(globalWarning.severity, "warning"); + assert.match(globalWarning.message, /skipped model config/); + assert.equal(readFileSync(profilePath, "utf8"), before); }); test("agent discovery skips skills directories", async (t) => { diff --git a/tests/model-routing-authority.test.ts b/tests/model-routing-authority.test.ts index c29fbef81..b402c7c2c 100644 --- a/tests/model-routing-authority.test.ts +++ b/tests/model-routing-authority.test.ts @@ -94,6 +94,18 @@ test("model routing authority normalizes and preserves sync/async source status" path: invalidGlobalPath, }); + const sourceCases = [ + [missingPath, missingPath, { status: "missing" }], + [missingPath, projectPath, { status: "valid", config: { project: { model: "google/gemini" } } }], + [validGlobalPath, projectPath, validSync], + [missingPath, invalidGlobalPath, { status: "invalid", path: invalidGlobalPath }], + [invalidGlobalPath, projectPath, { status: "invalid", path: invalidGlobalPath }], + ] as const; + for (const [globalSource, projectSource, expected] of sourceCases) { + assert.deepEqual(authority.readSavedModelConfig(globalSource, projectSource), expected); + assert.deepEqual(await authority.readSavedModelConfigAsync(globalSource, projectSource), expected); + } + const previousConfigHome = process.env.GENTLE_PI_CONFIG_HOME; process.env.GENTLE_PI_CONFIG_HOME = globalDir; t.after(() => {