Skip to content

Commit 400930f

Browse files
Alan-TheGentlemanAlan-TheGentleman
authored
Merge pull request #408 from decode2/refactor/396-fail-closed-routing-apply
refactor(models): fail closed on invalid saved routing
2 parents eb8448e + cc46e6a commit 400930f

3 files changed

Lines changed: 211 additions & 4 deletions

File tree

‎extensions/gentle-ai.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1664,12 +1664,16 @@ export async function applyModelConfigAsync(
16641664

16651665
export async function applySavedModelConfig(
16661666
ctx: ExtensionContext,
1667+
applyConfig: typeof applyModelConfigAsync = applyModelConfigAsync,
16671668
): Promise<{ updated: number; skipped: number; invalidPath?: string }> {
1668-
const result = await readSavedModelConfigAsync(ctx.cwd);
1669+
const result = await readModelRoutingAuthorityAsync(
1670+
modelConfigPath(ctx.cwd),
1671+
legacyProjectModelConfigPath(ctx.cwd),
1672+
);
16691673
if (result.status === "invalid") {
16701674
return { updated: 0, skipped: 0, invalidPath: result.path };
16711675
}
1672-
return applyModelConfigAsync(
1676+
return applyConfig(
16731677
ctx.cwd,
16741678
result.status === "valid" ? result.config : {},
16751679
);

‎tests/gentle-ai.test.ts‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,72 @@ function writeMarkdown(path: string, content: string): void {
1919
writeFileSync(path, content);
2020
}
2121

22+
test("session startup reports invalid project routing without mutating the profile", async (t) => {
23+
const root = mkdtempSync(join(tmpdir(), "gentle-pi-model-routing-startup-"));
24+
const configHome = join(root, "global");
25+
const projectConfigDir = join(root, ".pi", "gentle-ai");
26+
const projectAgentsDir = join(root, ".pi", "agents");
27+
const projectProfileDir = join(root, ".pi");
28+
const rootAgentsDir = join(root, "agents");
29+
const agentHome = join(root, "agent-home");
30+
const agentHomeAgentsDir = join(agentHome, "agents");
31+
const agentHomeSubagentsDir = join(agentHome, "subagents");
32+
mkdirSync(configHome, { recursive: true });
33+
mkdirSync(projectConfigDir, { recursive: true });
34+
mkdirSync(projectAgentsDir, { recursive: true });
35+
mkdirSync(projectProfileDir, { recursive: true });
36+
mkdirSync(rootAgentsDir, { recursive: true });
37+
mkdirSync(agentHomeAgentsDir, { recursive: true });
38+
mkdirSync(agentHomeSubagentsDir, { recursive: true });
39+
t.after(() => rmSync(root, { recursive: true, force: true }));
40+
41+
const previousConfigHome = process.env.GENTLE_PI_CONFIG_HOME;
42+
const previousAgentHome = process.env.GENTLE_PI_AGENT_HOME;
43+
process.env.GENTLE_PI_CONFIG_HOME = configHome;
44+
process.env.GENTLE_PI_AGENT_HOME = agentHome;
45+
t.after(() => {
46+
if (previousConfigHome === undefined) delete process.env.GENTLE_PI_CONFIG_HOME;
47+
else process.env.GENTLE_PI_CONFIG_HOME = previousConfigHome;
48+
if (previousAgentHome === undefined) delete process.env.GENTLE_PI_AGENT_HOME;
49+
else process.env.GENTLE_PI_AGENT_HOME = previousAgentHome;
50+
});
51+
52+
writeFileSync(join(projectConfigDir, "models.json"), "[]");
53+
writeMarkdown(join(projectAgentsDir, "worker.md"), "---\nname: worker\ndescription: Worker\n---\nbody\n");
54+
const profilePath = join(projectProfileDir, "subagents.json");
55+
const profileBytes = `${JSON.stringify({ unrelated: { keep: true } }, null, 2)}\n`;
56+
writeFileSync(profilePath, profileBytes);
57+
const before = readFileSync(profilePath, "utf8");
58+
59+
const handlers = new Map<string, (event: unknown, ctx: ExtensionContext) => Promise<void>>();
60+
const pi = {
61+
on(name: string, handler: (event: unknown, ctx: ExtensionContext) => Promise<void>) {
62+
handlers.set(name, handler);
63+
},
64+
registerCommand() {},
65+
registerTool() {},
66+
} as unknown as ExtensionAPI;
67+
createGentleAiExtension({ nativeReviewCli: null })(pi);
68+
const sessionStart = handlers.get("session_start");
69+
assert.equal(typeof sessionStart, "function");
70+
const notifications: Array<{ message: string; severity: string }> = [];
71+
await sessionStart!({}, {
72+
cwd: root,
73+
hasUI: true,
74+
ui: {
75+
notify(message: string, severity: string) {
76+
notifications.push({ message, severity });
77+
},
78+
},
79+
} as unknown as ExtensionContext);
80+
81+
const warning = notifications.find((entry) => entry.message.includes(join(projectConfigDir, "models.json")));
82+
assert.ok(warning, JSON.stringify(notifications));
83+
assert.equal(warning!.severity, "warning");
84+
assert.match(warning!.message, /skipped model config/);
85+
assert.equal(readFileSync(profilePath, "utf8"), before);
86+
});
87+
2288
test("agent discovery skips skills directories", async (t) => {
2389
const root = mkdtempSync(join(tmpdir(), "gentle-pi-agents-"));
2490
t.after(() => rmSync(root, { recursive: true, force: true }));

‎tests/model-routing-authority.test.ts‎

Lines changed: 139 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import assert from "node:assert/strict";
2-
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
2+
import { mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from "node:fs";
33
import { tmpdir } from "node:os";
44
import { join } from "node:path";
55
import test from "node:test";
6-
import { readModelConfig, readModelConfigAsync } from "../extensions/gentle-ai.ts";
6+
import { applySavedModelConfig, readModelConfig, readModelConfigAsync } from "../extensions/gentle-ai.ts";
77

88
test("model routing authority normalizes and preserves sync/async source status", async (t) => {
99
const loaded = await import("../lib/model-routing-authority.ts").then(
@@ -118,3 +118,140 @@ test("model routing authority normalizes and preserves sync/async source status"
118118
assert.deepEqual(readModelConfig(projectDir), {});
119119
assert.deepEqual(await readModelConfigAsync(projectDir), {});
120120
});
121+
122+
test("saved-routing apply fails closed for invalid project and global sources", async (t) => {
123+
const root = mkdtempSync(join(tmpdir(), "gentle-pi-model-routing-apply-"));
124+
const configHome = join(root, "global");
125+
const projectConfigDir = join(root, ".pi", "gentle-ai");
126+
const projectAgentsDir = join(root, ".pi", "agents");
127+
const projectProfileDir = join(root, ".pi");
128+
const agentHome = join(root, "agent-home");
129+
const agentHomeAgentsDir = join(agentHome, "agents");
130+
const agentHomeSubagentsDir = join(agentHome, "subagents");
131+
mkdirSync(configHome, { recursive: true });
132+
mkdirSync(projectConfigDir, { recursive: true });
133+
mkdirSync(projectAgentsDir, { recursive: true });
134+
mkdirSync(projectProfileDir, { recursive: true });
135+
mkdirSync(agentHomeAgentsDir, { recursive: true });
136+
mkdirSync(agentHomeSubagentsDir, { recursive: true });
137+
t.after(() => rmSync(root, { recursive: true, force: true }));
138+
139+
const previousConfigHome = process.env.GENTLE_PI_CONFIG_HOME;
140+
const previousAgentHome = process.env.GENTLE_PI_AGENT_HOME;
141+
process.env.GENTLE_PI_CONFIG_HOME = configHome;
142+
process.env.GENTLE_PI_AGENT_HOME = agentHome;
143+
t.after(() => {
144+
if (previousConfigHome === undefined) delete process.env.GENTLE_PI_CONFIG_HOME;
145+
else process.env.GENTLE_PI_CONFIG_HOME = previousConfigHome;
146+
if (previousAgentHome === undefined) delete process.env.GENTLE_PI_AGENT_HOME;
147+
else process.env.GENTLE_PI_AGENT_HOME = previousAgentHome;
148+
});
149+
150+
const agentPath = join(projectAgentsDir, "worker.md");
151+
writeFileSync(agentPath, "---\nname: worker\ndescription: Worker\n---\nbody\n");
152+
const profilePath = join(projectProfileDir, "subagents.json");
153+
const profileBytes = JSON.stringify({
154+
unrelated: { keep: true },
155+
model_profiles: { worker: { model: "existing/model", effort: "low" } },
156+
}, null, 2) + "\n";
157+
writeFileSync(profilePath, profileBytes);
158+
const context = { cwd: root } as Parameters<typeof applySavedModelConfig>[0];
159+
const projectPath = join(projectConfigDir, "models.json");
160+
const globalPath = join(configHome, "models.json");
161+
let mutatorCalls = 0;
162+
const applyConfig = async () => {
163+
mutatorCalls += 1;
164+
return { updated: 0, skipped: 0 };
165+
};
166+
167+
for (const projectValue of ["{", "[]", "null"] as const) {
168+
writeFileSync(projectPath, projectValue);
169+
const before = statSync(profilePath);
170+
const result = await applySavedModelConfig(context, applyConfig);
171+
assert.deepEqual(result, { updated: 0, skipped: 0, invalidPath: projectPath });
172+
assert.equal(mutatorCalls, 0);
173+
assert.equal(readFileSync(profilePath, "utf8"), profileBytes);
174+
assert.equal(statSync(profilePath).mtimeMs, before.mtimeMs);
175+
}
176+
177+
writeFileSync(globalPath, "[]");
178+
writeFileSync(projectPath, JSON.stringify({ worker: "new/model" }));
179+
const before = statSync(profilePath);
180+
const result = await applySavedModelConfig(context, applyConfig);
181+
assert.deepEqual(result, { updated: 0, skipped: 0, invalidPath: globalPath });
182+
assert.equal(mutatorCalls, 0);
183+
assert.equal(readFileSync(profilePath, "utf8"), profileBytes);
184+
assert.equal(statSync(profilePath).mtimeMs, before.mtimeMs);
185+
});
186+
187+
test("saved-routing apply preserves missing, valid, null, inherit, and omission behavior", async (t) => {
188+
const root = mkdtempSync(join(tmpdir(), "gentle-pi-model-routing-apply-valid-"));
189+
const configHome = join(root, "global");
190+
const projectConfigDir = join(root, ".pi", "gentle-ai");
191+
const projectAgentsDir = join(root, ".pi", "agents");
192+
const projectProfileDir = join(root, ".pi");
193+
const agentHome = join(root, "agent-home");
194+
const agentHomeAgentsDir = join(agentHome, "agents");
195+
const agentHomeSubagentsDir = join(agentHome, "subagents");
196+
mkdirSync(configHome, { recursive: true });
197+
mkdirSync(projectConfigDir, { recursive: true });
198+
mkdirSync(projectAgentsDir, { recursive: true });
199+
mkdirSync(projectProfileDir, { recursive: true });
200+
mkdirSync(agentHomeAgentsDir, { recursive: true });
201+
mkdirSync(agentHomeSubagentsDir, { recursive: true });
202+
t.after(() => rmSync(root, { recursive: true, force: true }));
203+
204+
const previousConfigHome = process.env.GENTLE_PI_CONFIG_HOME;
205+
const previousAgentHome = process.env.GENTLE_PI_AGENT_HOME;
206+
process.env.GENTLE_PI_CONFIG_HOME = configHome;
207+
process.env.GENTLE_PI_AGENT_HOME = agentHome;
208+
t.after(() => {
209+
if (previousConfigHome === undefined) delete process.env.GENTLE_PI_CONFIG_HOME;
210+
else process.env.GENTLE_PI_CONFIG_HOME = previousConfigHome;
211+
if (previousAgentHome === undefined) delete process.env.GENTLE_PI_AGENT_HOME;
212+
else process.env.GENTLE_PI_AGENT_HOME = previousAgentHome;
213+
});
214+
215+
const agentPath = join(projectAgentsDir, "worker.md");
216+
writeFileSync(agentPath, "---\nname: worker\ndescription: Worker\n---\nbody\n");
217+
const profilePath = join(projectProfileDir, "subagents.json");
218+
const initialProfile = {
219+
unrelated: { keep: true },
220+
model_profiles: { worker: { model: "existing/model", effort: "low" } },
221+
};
222+
writeFileSync(profilePath, `${JSON.stringify(initialProfile, null, 2)}\n`);
223+
const context = { cwd: root } as Parameters<typeof applySavedModelConfig>[0];
224+
const projectPath = join(projectConfigDir, "models.json");
225+
226+
const missing = await applySavedModelConfig(context);
227+
assert.equal(missing.invalidPath, undefined);
228+
assert.deepEqual(JSON.parse(readFileSync(profilePath, "utf8")), initialProfile);
229+
230+
writeFileSync(projectPath, JSON.stringify({ worker: "inherit" }));
231+
const valid = await applySavedModelConfig(context);
232+
assert.equal(valid.invalidPath, undefined);
233+
const validProfile = JSON.parse(readFileSync(profilePath, "utf8")) as Record<string, any>;
234+
assert.deepEqual(validProfile.unrelated, initialProfile.unrelated);
235+
assert.deepEqual(validProfile.model_profiles.worker, { model: "inherit" });
236+
assert.match(readFileSync(agentPath, "utf8"), /model: inherit\n/);
237+
assert.doesNotMatch(readFileSync(agentPath, "utf8"), /thinking:/);
238+
239+
const afterValidBytes = readFileSync(profilePath, "utf8");
240+
const afterValid = statSync(profilePath);
241+
writeFileSync(projectPath, JSON.stringify({ worker: null }));
242+
const nullEntry = await applySavedModelConfig(context);
243+
assert.equal(nullEntry.invalidPath, undefined);
244+
assert.equal(readFileSync(profilePath, "utf8"), afterValidBytes);
245+
assert.equal(statSync(profilePath).mtimeMs, afterValid.mtimeMs);
246+
assert.doesNotMatch(readFileSync(agentPath, "utf8"), /model: null/);
247+
248+
let mutatorCalls = 0;
249+
const applyConfig = async () => {
250+
mutatorCalls += 1;
251+
return { updated: 0, skipped: 0 };
252+
};
253+
writeFileSync(projectPath, JSON.stringify({ worker: "new/model" }));
254+
const injected = await applySavedModelConfig(context, applyConfig);
255+
assert.equal(injected.invalidPath, undefined);
256+
assert.equal(mutatorCalls, 1);
257+
});

0 commit comments

Comments
 (0)