From 6645a7a0598a567445ebf25970c46c4e4e83270e Mon Sep 17 00:00:00 2001 From: can1357 Date: Wed, 1 Apr 2026 04:49:16 +0200 Subject: [PATCH] fix(model-registry): filtered disabled providers from model listing - Applied `disabledProviders` setting to `getAvailable()` and `getDiscoverableProviders()` so `--list-models` and `/model` hide models from disabled providers. - Initialized settings before model refresh in the `--list-models` code path. - Added tests verifying disabled providers are excluded from both methods. Fixes #588 --- packages/coding-agent/CHANGELOG.md | 1 + .../coding-agent/src/config/model-registry.ts | 22 ++++++++++-- packages/coding-agent/src/main.ts | 1 + .../coding-agent/test/model-registry.test.ts | 35 +++++++++++++++++++ 4 files changed, 56 insertions(+), 3 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 1e25a1519..43a82da80 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -7,6 +7,7 @@ ### Fixed +- Fixed `--list-models` and `/model` provider filtering to hide models from disabled providers ([#588](https://github.com/can1357/oh-my-pi/issues/588)) - Fixed edit tool diffstats to use diff-specific add/remove theme colors instead of success/error status colors ([#589](https://github.com/can1357/oh-my-pi/issues/589)) diff --git a/packages/coding-agent/src/config/model-registry.ts b/packages/coding-agent/src/config/model-registry.ts index ff85de248..d53ed3244 100644 --- a/packages/coding-agent/src/config/model-registry.ts +++ b/packages/coding-agent/src/config/model-registry.ts @@ -31,7 +31,7 @@ import { type ConfigError, ConfigFile } from "../config"; import { parseModelString } from "../config/model-resolver"; import { isValidThemeColor, type ThemeColor } from "../modes/theme/theme"; import type { AuthStorage, OAuthCredential } from "../session/auth-storage"; -import type { Settings } from "./settings"; +import { settings, type Settings } from "./settings"; export const kNoAuth = "N/A"; @@ -730,6 +730,14 @@ function normalizeSuppressedSelector(selector: string): string { return `${parsed.provider}/${parsed.id}`; } +function getDisabledProviderIdsFromSettings(): Set { + try { + return new Set(settings.get("disabledProviders")); + } catch { + return new Set(); + } +} + /** * Model registry - loads and manages models, resolves API keys via AuthStorage. */ @@ -1670,11 +1678,19 @@ export class ModelRegistry { * This is a fast check that doesn't refresh OAuth tokens. */ getAvailable(): Model[] { - return this.#models.filter(m => this.#keylessProviders.has(m.provider) || this.authStorage.hasAuth(m.provider)); + const disabledProviders = getDisabledProviderIdsFromSettings(); + return this.#models.filter( + m => + !disabledProviders.has(m.provider) && + (this.#keylessProviders.has(m.provider) || this.authStorage.hasAuth(m.provider)), + ); } getDiscoverableProviders(): string[] { - return this.#discoverableProviders.map(provider => provider.provider); + const disabledProviders = getDisabledProviderIdsFromSettings(); + return this.#discoverableProviders + .filter(provider => !disabledProviders.has(provider.provider)) + .map(provider => provider.provider); } getProviderDiscoveryState(provider: string): ProviderDiscoveryState | undefined { diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index c5ec44e18..f6ad36c5c 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -536,6 +536,7 @@ export async function runRootCommand(parsed: Args, rawArgs: string[]): Promise Settings.init({ cwd: getProjectDir() })); await modelRegistry.refresh("online"); const searchPattern = typeof parsedArgs.listModels === "string" ? parsedArgs.listModels : undefined; await listModels(modelRegistry, searchPattern); diff --git a/packages/coding-agent/test/model-registry.test.ts b/packages/coding-agent/test/model-registry.test.ts index 2a2c1e3ec..f18bb52fb 100644 --- a/packages/coding-agent/test/model-registry.test.ts +++ b/packages/coding-agent/test/model-registry.test.ts @@ -4,6 +4,7 @@ import * as os from "node:os"; import * as path from "node:path"; import { Effort, type OpenAICompat, type ThinkingConfig } from "@oh-my-pi/pi-ai"; import { kNoAuth, MODEL_ROLES, ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; +import { _resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; import { hookFetch, Snowflake } from "@oh-my-pi/pi-utils"; @@ -18,6 +19,7 @@ describe("ModelRegistry", () => { }); beforeEach(async () => { + _resetSettingsForTest(); tempDir = path.join(os.tmpdir(), `pi-test-model-registry-${Snowflake.next()}`); fs.mkdirSync(tempDir, { recursive: true }); modelsJsonPath = path.join(tempDir, "models.json"); @@ -25,6 +27,7 @@ describe("ModelRegistry", () => { }); afterEach(() => { + _resetSettingsForTest(); authStorage.close(); if (tempDir && fs.existsSync(tempDir)) { fs.rmSync(tempDir, { recursive: true }); @@ -1034,6 +1037,38 @@ describe("ModelRegistry", () => { expect(model.baseUrl).toBe(initialBaseUrl); }); }); + + describe("disabled provider filtering", () => { + test("getAvailable and getDiscoverableProviders exclude disabled providers from settings", async () => { + writeRawModelsJson({ + ollama: { + baseUrl: "http://127.0.0.1:11434/v1", + api: "openai-completions", + auth: "none", + discovery: { type: "ollama" }, + }, + }); + await authStorage.set("github-copilot", [ + { + type: "oauth", + access: "tid=1;proxy-ep=proxy.individual.githubcopilot.com;exp=9999999999", + refresh: "refresh-individual", + expires: Date.now() + 60_000, + }, + ]); + await Settings.init({ + inMemory: true, + overrides: { + disabledProviders: ["github-copilot", "ollama"], + }, + }); + + const registry = new ModelRegistry(authStorage, modelsJsonPath); + + expect(registry.getAvailable().some(model => model.provider === "github-copilot")).toBe(false); + expect(registry.getDiscoverableProviders()).not.toContain("ollama"); + }); + }); describe("runtime discovery", () => { test("auto-discovers ollama models without provider config", async () => { using _hook = hookFetch(input => {