From c463de8da2eb8a3ce0350afb4e7c03ef97dfd2ed Mon Sep 17 00:00:00 2001 From: User Date: Tue, 14 Jul 2026 18:33:15 +0200 Subject: [PATCH] fix(backend): scope Pi model listing to enabled models --- backend/src/pi/list-models.ts | 31 +++++-- backend/test/list-models.test.ts | 148 ++++++++++++++++++------------- 2 files changed, 108 insertions(+), 71 deletions(-) diff --git a/backend/src/pi/list-models.ts b/backend/src/pi/list-models.ts index 5e30e348..7d68d16b 100644 --- a/backend/src/pi/list-models.ts +++ b/backend/src/pi/list-models.ts @@ -2,7 +2,10 @@ import { spawn as nodeSpawn, type ChildProcessWithoutNullStreams } from "node:ch import type { AppConfig } from "../config.js"; import { RpcClient } from "../rpc/rpc-client.js"; import { buildPiChildEnv } from "./provider-credentials.js"; -import { secretValue } from "../config/secret-bundle.js"; +import { + loadPiEnabledModels, + type PiEnabledModelsResult, +} from "./enabled-models.js"; export interface PiModel { provider: string; @@ -19,6 +22,8 @@ interface Opts { ) => ChildProcessWithoutNullStreams; ttlMs?: number; nowMs?: () => number; + loadEnabledModels?: () => PiEnabledModelsResult; + warn?: (message: string) => void; } /** @@ -36,11 +41,15 @@ export function createPiModelLister(cfg: AppConfig, opts: Opts = {}): () => Prom return async function listModels(): Promise { if (cache && now() - cache.at < ttlMs) return cache.models; - const env = buildPiChildEnv({ - provider: cfg.defaults.provider, - credentialValue: secretValue(cfg, "THT_MODEL_API_KEY"), - credentialFile: cfg.modelApiKeyFile, - }); + const enabled = (opts.loadEnabledModels + ?? (() => loadPiEnabledModels({ harnessDir: cfg.harnessDir })))(); + for (const warning of enabled.warnings) opts.warn?.(warning); + if (enabled.ids.length === 0) { + cache = { at: now(), models: [] }; + return []; + } + + const env = buildPiChildEnv({}); delete env.THT_DATA_ROOT; if (cfg.dataRoot !== undefined) env.THT_DATA_ROOT = cfg.dataRoot; const child = spawnFn(cfg.piBin, ["--mode", "rpc"], { cwd: cfg.harnessDir, env }); @@ -52,12 +61,20 @@ export function createPiModelLister(cfg: AppConfig, opts: Opts = {}): () => Prom new Promise((_, rej) => setTimeout(() => rej(new Error("pi model list timeout")), 8000)), ]); const raw = (resp?.data?.models ?? []) as Array>; - const models: PiModel[] = raw.map((m) => ({ + const available: PiModel[] = raw.map((m) => ({ provider: String(m.provider ?? ""), id: String(m.id ?? ""), name: String(m.name ?? m.id ?? ""), reasoning: Boolean(m.reasoning), })); + const byCompositeId = new Map( + available.map((model) => [`${model.provider}/${model.id}`, model]), + ); + const models = enabled.ids.flatMap((id) => { + const model = byCompositeId.get(id); + return model ? [model] : []; + }); + if (models.length === 0) opts.warn?.("No Pi-enabled models are currently available"); cache = { at: now(), models }; return models; } finally { diff --git a/backend/test/list-models.test.ts b/backend/test/list-models.test.ts index f43532e4..84699025 100644 --- a/backend/test/list-models.test.ts +++ b/backend/test/list-models.test.ts @@ -1,6 +1,6 @@ import { test, expect } from "vitest"; import { spawn } from "node:child_process"; -import { chmodSync, mkdtempSync, writeFileSync, rmSync } from "node:fs"; +import { mkdtempSync, writeFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import path from "node:path"; @@ -16,6 +16,10 @@ function scriptWith(models: unknown[]): string { return file; } +function enabled(...ids: string[]) { + return () => ({ ids, warnings: [], source: "/test/settings.json" }); +} + test("createPiModelLister returns mapped PiModel[] from get_available_models", async () => { const script = scriptWith([ { provider: "zai", id: "glm-5.2", name: "GLM 5.2", reasoning: true, extra: "ignored" }, @@ -23,6 +27,7 @@ test("createPiModelLister returns mapped PiModel[] from get_available_models", a ]); try { const lister = createPiModelLister(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + loadEnabledModels: enabled("zai/glm-5.2", "anthropic/claude-opus-4-8"), spawnFn: () => spawn("node", [FAKE, script]) as any, }); const models = await lister(); @@ -40,6 +45,7 @@ test("createPiModelLister caches within ttl (spawns once for two calls)", async try { let spawns = 0; const lister = createPiModelLister(loadConfig({ THT_HARNESS_DIR: "../harness" }), { + loadEnabledModels: enabled("zai/glm-5.2"), spawnFn: () => { spawns++; return spawn("node", [FAKE, script]) as any; }, ttlMs: 10_000, nowMs: () => 1000, @@ -63,6 +69,7 @@ test("production model-list spawn preserves PATH and passes the portable data ro PI_BIN: "/usr/local/bin/pi", THT_DATA_ROOT: "/data", }), { + loadEnabledModels: enabled("test/unavailable"), spawnFn: (...args: any[]) => { calls.push(args); return spawn("node", [FAKE, script]) as any; @@ -94,6 +101,7 @@ test("model-list spawn scrubs ambient provider credentials and generic secret me process.env.CLOUDFLARE_ACCOUNT_ID = "must-not-leak"; try { const lister = createPiModelLister(loadConfig({ PI_BIN: "/usr/local/bin/pi" }), { + loadEnabledModels: enabled("test/unavailable"), spawnFn: (...args: any[]) => { calls.push(args); return spawn("node", [FAKE, script]) as any; @@ -117,71 +125,83 @@ test("model-list spawn scrubs ambient provider credentials and generic secret me } }); -test("model-list spawn loads only the selected canonical provider credential", async () => { - const script = scriptWith([]); - const secret = join(path.dirname(script), "model-key"); - writeFileSync(secret, "selected-secret", { mode: 0o600 }); - chmodSync(secret, 0o600); +test("model listing does not require PI_PROVIDER or read the generic credential", async () => { + const script = scriptWith([ + { provider: "zai", id: "glm-5.2", name: "GLM-5.2", reasoning: true }, + ]); const calls: any[][] = []; - const lister = createPiModelLister(loadConfig({ - PI_PROVIDER: "Gemini", - THT_MODEL_API_KEY_FILE: secret, - }), { - spawnFn: (...args: any[]) => { - calls.push(args); - return spawn("node", [FAKE, script]) as any; - }, - }); try { - await lister(); - expect(calls[0][2].env.GEMINI_API_KEY).toBe("selected-secret"); - expect(calls[0][2].env).not.toHaveProperty("THT_MODEL_API_KEY_FILE"); - expect(JSON.stringify(calls[0].slice(0, 2))).not.toContain("selected-secret"); - } finally { - rmSync(path.dirname(script), { recursive: true, force: true }); - } -}); - -test("model-list spawn uses the same single secret bundle as sessions", async () => { - const script = scriptWith([]); - const bundle = join(path.dirname(script), "bundle"); - writeFileSync(bundle, "THT_MODEL_API_KEY=selected-bundle-secret\n", { mode: 0o600 }); - const calls: any[][] = []; - const lister = createPiModelLister(loadConfig({ - PI_PROVIDER: "openai", THT_SECRETS_FILE: bundle, - }), { - spawnFn: (...args: any[]) => { - calls.push(args); - return spawn("node", [FAKE, script]) as any; - }, - }); - try { - await lister(); - expect(calls[0][2].env.OPENAI_API_KEY).toBe("selected-bundle-secret"); - expect(calls[0][2].env).not.toHaveProperty("THT_SECRETS_FILE"); - } finally { - rmSync(path.dirname(script), { recursive: true, force: true }); - } -}); - -test.each(["amazon-bedrock", "azure-openai-responses", "cloudflare-workers-ai", "cloudflare-ai-gateway"])( - "model listing rejects compound provider %s before spawning Pi", async (provider) => { - const script = scriptWith([]); - const secret = join(path.dirname(script), "model-key"); - writeFileSync(secret, "selected-secret", { mode: 0o600 }); - let spawns = 0; const lister = createPiModelLister(loadConfig({ - PI_PROVIDER: provider, THT_MODEL_API_KEY_FILE: secret, + PI_BIN: "/usr/local/bin/pi", + THT_MODEL_API_KEY_FILE: "/missing-and-must-not-be-read", }), { - spawnFn: () => { spawns += 1; throw new Error("must not spawn"); }, + loadEnabledModels: enabled("zai/glm-5.2"), + spawnFn: (...args: any[]) => { + calls.push(args); + return spawn("node", [FAKE, script]) as any; + }, }); - try { - await expect(lister()).rejects.toThrow( - "compound credential bundles are unsupported by THT_MODEL_API_KEY_FILE; dedicated provider configuration is required", - ); - expect(spawns).toBe(0); - } finally { - rmSync(path.dirname(script), { recursive: true, force: true }); - } - }, -); + await expect(lister()).resolves.toHaveLength(1); + expect(calls[0][2].env).not.toHaveProperty("ZAI_API_KEY"); + expect(calls[0][2].env).not.toHaveProperty("THT_MODEL_API_KEY_FILE"); + } finally { + rmSync(path.dirname(script), { recursive: true, force: true }); + } +}); + +test("returns only enabled available models in enabledModels order", async () => { + const script = scriptWith([ + { provider: "zai", id: "glm-5v-turbo", name: "GLM-5V-Turbo", reasoning: true }, + { provider: "local-qwen", id: "qwen3.6-35b-a3b", name: "Qwen3.6 Local", reasoning: false }, + { provider: "zai", id: "glm-5.2", name: "GLM-5.2", reasoning: true }, + { provider: "deepseek", id: "deepseek-v4-flash", name: "DeepSeek V4 Flash", reasoning: true }, + ]); + try { + const lister = createPiModelLister(loadConfig({}), { + loadEnabledModels: enabled( + "zai/glm-5.2", + "deepseek/deepseek-v4-flash", + "local-qwen/qwen3.6-35b-a3b", + ), + spawnFn: () => spawn("node", [FAKE, script]) as any, + }); + expect((await lister()).map((m) => `${m.provider}/${m.id}`)).toEqual([ + "zai/glm-5.2", + "deepseek/deepseek-v4-flash", + "local-qwen/qwen3.6-35b-a3b", + ]); + } finally { + rmSync(path.dirname(script), { recursive: true, force: true }); + } +}); + +test("empty enabled model scope fails closed without spawning Pi", async () => { + let spawns = 0; + const warnings: string[] = []; + const lister = createPiModelLister(loadConfig({}), { + loadEnabledModels: () => ({ ids: [], warnings: ["scope invalid"] }), + warn: (message) => warnings.push(message), + spawnFn: () => { spawns += 1; throw new Error("must not spawn"); }, + }); + await expect(lister()).resolves.toEqual([]); + expect(spawns).toBe(0); + expect(warnings).toEqual(["scope invalid"]); +}); + +test("warns and returns empty when enabled identifiers are unavailable", async () => { + const script = scriptWith([ + { provider: "zai", id: "glm-5v-turbo", name: "GLM-5V-Turbo", reasoning: true }, + ]); + const warnings: string[] = []; + try { + const lister = createPiModelLister(loadConfig({}), { + loadEnabledModels: enabled("zai/glm-5.2"), + warn: (message) => warnings.push(message), + spawnFn: () => spawn("node", [FAKE, script]) as any, + }); + await expect(lister()).resolves.toEqual([]); + expect(warnings).toEqual(["No Pi-enabled models are currently available"]); + } finally { + rmSync(path.dirname(script), { recursive: true, force: true }); + } +});