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
This commit is contained in:
@@ -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))
|
||||
|
||||
|
||||
|
||||
@@ -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<string> {
|
||||
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<Api>[] {
|
||||
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 {
|
||||
|
||||
@@ -536,6 +536,7 @@ export async function runRootCommand(parsed: Args, rawArgs: string[]): Promise<v
|
||||
}
|
||||
|
||||
if (parsedArgs.listModels !== undefined) {
|
||||
await logger.timeAsync("settings:init:list-models", () => Settings.init({ cwd: getProjectDir() }));
|
||||
await modelRegistry.refresh("online");
|
||||
const searchPattern = typeof parsedArgs.listModels === "string" ? parsedArgs.listModels : undefined;
|
||||
await listModels(modelRegistry, searchPattern);
|
||||
|
||||
@@ -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 => {
|
||||
|
||||
Reference in New Issue
Block a user