smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(coding-agent): supported models yaml config

Browse Source 
Loaded default custom model configuration from models.yaml when models.yml is absent while preserving yml precedence over yaml and legacy json migration.

Fixes #5145
This commit is contained in:
roboomp
2026-07-11 03:52:45 +00:00
parent 7aa1d581c6
commit 2d4d445f95
6 changed files with 246 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/models.md
+8 -7
View File
@@ -1,4 +1,4 @@
# Model and Provider Configuration (`models.yml`)
# Model and Provider Configuration (`models.yml` / `models.yaml`)
This document describes how the coding-agent currently loads models, applies overrides, resolves credentials, and chooses models at runtime.
@@ -14,16 +14,17 @@ Primary implementation files:
## Config file location and legacy behavior
Default config path:
Default config paths, in precedence order:
- `~/.omp/agent/models.yml`
- `~/.omp/agent/models.yaml`
Legacy behavior still present:
- If `models.yml` is missing and `models.json` exists at the same location, it is migrated to `models.yml`.
- If both YAML files are missing and `models.json` exists at the same location, it is migrated to `models.yml`.
- Explicit `.json` / `.jsonc` config paths are still supported when passed programmatically to `ModelRegistry`.
## `models.yml` shape
## `models.yml` / `models.yaml` shape
```yaml
providers:
@@ -158,7 +159,7 @@ Successful command outputs are cached for the process lifetime so the command is
ModelRegistry pipeline (on refresh):
1. Load built-in providers/models from `@oh-my-pi/pi-catalog` (`getBundledProviders` / `getBundledModels`).
2. Load `models.yml` custom config.
2. Load `models.yml` / `models.yaml` custom config.
3. Apply provider overrides (`baseUrl`, `headers`, `disableStrictTools`) to built-in models.
4. Apply `modelOverrides` (per provider + model id).
5. Merge custom `models`:
@@ -735,11 +736,11 @@ providers:
## Legacy consumer caveat
Most model configuration now flows through `models.yml` via `ModelRegistry`. Explicit `.json` / `.jsonc` paths remain supported only when passed programmatically to `ModelRegistry`; the default user config is `~/.omp/agent/models.yml`.
Most model configuration now flows through `models.yml` / `models.yaml` via `ModelRegistry`. Explicit `.json` / `.jsonc` paths remain supported only when passed programmatically to `ModelRegistry`; the default user config prefers `~/.omp/agent/models.yml`, then falls back to `~/.omp/agent/models.yaml`.
## Failure mode
If `models.yml` fails schema or validation checks:
If `models.yml` / `models.yaml` fails schema or validation checks:
- registry keeps operating with built-in models
- error is exposed via `ModelRegistry.getError()` and surfaced in UI/notifications
packages/coding-agent/CHANGELOG.md
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Fixed custom model/provider config discovery so `~/.omp/agent/models.yaml` loads when `models.yml` is absent, while preserving `.yml` precedence and only migrating legacy `models.json` when neither YAML file exists. ([#5145](https://github.com/can1357/oh-my-pi/issues/5145))
## [16.4.2] - 2026-07-10
### Fixed
packages/coding-agent/src/config/config-file.ts
+30 -12
View File
@@ -11,9 +11,9 @@ interface ConfigSchemaError {
}
/**
* Module-private cache of (jsonPath, ymlPath) pairs we already migrated this
* process. Prevents `ConfigFile.relocate()` / repeated `tryLoad()` calls from
* re-running the migration over and over on the boot path.
* Module-private cache of JSON → YAML migrations this process already ran.
* Prevents `ConfigFile.relocate()` / repeated `tryLoad()` calls from re-running
* the migration over and over on the boot path.
*/
const migratedPaths = new Set<string>();
@@ -123,6 +123,7 @@ export type LoadResult<T> =
export class ConfigFile<T> implements IConfigFile<T> {
readonly #basePath: string;
readonly #yamlFallbackPath: string | null;
readonly #jsonMigrationPath: string | null;
#cache?: LoadResult<T>;
#auxValidate?: (value: T) => void;
@@ -134,13 +135,17 @@ export class ConfigFile<T> implements IConfigFile<T> {
) {
this.#basePath = configPath;
if (configPath.endsWith(".yml")) {
this.#yamlFallbackPath = `${configPath.slice(0, -4)}.yaml`;
this.#jsonMigrationPath = `${configPath.slice(0, -4)}.json`;
} else if (configPath.endsWith(".yaml")) {
this.#yamlFallbackPath = null;
this.#jsonMigrationPath = `${configPath.slice(0, -5)}.json`;
} else if (configPath.endsWith(".json") || configPath.endsWith(".jsonc")) {
this.#yamlFallbackPath = null;
// JSON configs are still supported without migration.
this.#jsonMigrationPath = null;
} else {
this.#yamlFallbackPath = null;
throw new Error(`Invalid config file path: ${configPath}`);
}
}
@@ -150,9 +155,11 @@ export class ConfigFile<T> implements IConfigFile<T> {
* Sync callers (tests, settings init) hit this implicitly via {@link tryLoad}.
*/
#ensureMigrated(): void {
if (this.#jsonMigrationPath) {
migrateJsonToYml(this.#jsonMigrationPath, this.#basePath);
if (!this.#jsonMigrationPath) return;
if (this.#yamlFallbackPath && !fs.existsSync(this.#basePath) && fs.existsSync(this.#yamlFallbackPath)) {
return;
}
migrateJsonToYml(this.#jsonMigrationPath, this.#basePath);
}
relocate(configPath?: string): ConfigFile<T> {
@@ -163,9 +170,19 @@ export class ConfigFile<T> implements IConfigFile<T> {
return result;
}
#resolveReadPath(): string {
if (fs.existsSync(this.#basePath)) {
return this.#basePath;
}
if (this.#yamlFallbackPath && fs.existsSync(this.#yamlFallbackPath)) {
return this.#yamlFallbackPath;
}
return this.#basePath;
}
getMtimeMs(): number | null {
try {
return fs.statSync(this.path()).mtimeMs;
return fs.statSync(this.#resolveReadPath()).mtimeMs;
} catch (err) {
if (isEnoent(err)) return null;
throw err;
@@ -211,12 +228,13 @@ export class ConfigFile<T> implements IConfigFile<T> {
#parseContent(content: string): LoadResult<T> {
try {
let parsed: unknown;
if (this.#basePath.endsWith(".json") || this.#basePath.endsWith(".jsonc")) {
const readPath = this.#resolveReadPath();
if (readPath.endsWith(".json") || readPath.endsWith(".jsonc")) {
parsed = JSONC.parse(content);
} else if (this.#basePath.endsWith(".yml") || this.#basePath.endsWith(".yaml")) {
} else if (readPath.endsWith(".yml") || readPath.endsWith(".yaml")) {
parsed = YAML.parse(content);
} else {
throw new Error(`Invalid config file path: ${this.#basePath}`);
throw new Error(`Invalid config file path: ${readPath}`);
}
const checked = this.schema(parsed);
@@ -255,7 +273,7 @@ export class ConfigFile<T> implements IConfigFile<T> {
let content: string;
try {
content = fs.readFileSync(this.path(), "utf-8").trim();
content = fs.readFileSync(this.#resolveReadPath(), "utf-8").trim();
} catch (error) {
if (isEnoent(error)) {
return this.#storeCache({ status: "not-found" });
@@ -275,7 +293,7 @@ export class ConfigFile<T> implements IConfigFile<T> {
let content: string;
try {
content = (await Bun.file(this.path()).text()).trim();
content = (await Bun.file(this.#resolveReadPath()).text()).trim();
} catch (error) {
if (isEnoent(error)) {
return this.#storeCache({ status: "not-found" });
@@ -306,7 +324,7 @@ export class ConfigFile<T> implements IConfigFile<T> {
}
path(): string {
return this.#basePath;
return this.#resolveReadPath();
}
invalidate() {
packages/coding-agent/src/config/model-registry.ts
+7 -7
View File
@@ -258,7 +258,7 @@ export interface ProviderDiscoveryState {
error?: string;
}
/** Result of loading custom models from models.json */
/** Result of loading custom models config. */
interface CustomModelsResult {
models?: CustomModelOverlay[];
overrides?: Map<string, ProviderOverride>;
@@ -309,7 +309,7 @@ interface CommandApiKeyResolution {
value?: string;
}
/**
* Resolve a models.yml secret/config value to an actual value.
* Resolve a models.yml/models.yaml secret/config value to an actual value.
* `!cmd` runs a shell command and returns trimmed stdout, otherwise env vars are
* checked first and the input falls back to a literal value.
*/
@@ -822,7 +822,7 @@ export class ModelRegistry {
}
/**
* Reload models from disk (built-in + custom from models.json).
* Reload models from disk (built-in + custom config).
*/
async refresh(strategy: ModelRefreshStrategy = "online-if-uncached"): Promise<void> {
this.#reloadStaticModels();
@@ -938,7 +938,7 @@ export class ModelRegistry {
#reloadStaticModels(): void {
const currentMtime = this.#modelsConfigFile.getMtimeMs();
if (currentMtime !== null && currentMtime === this.#lastStaticLoadMtime) {
// models.json unchanged since last load; reloading would be redundant.
// Models config unchanged since last load; reloading would be redundant.
return;
}
this.#modelsConfigFile.invalidate();
@@ -962,14 +962,14 @@ export class ModelRegistry {
}
/**
* Get any error from loading models.json (undefined if no error).
* Get any error from loading custom models config (undefined if no error).
*/
getError(): ConfigError | undefined {
return this.#configError;
}
#loadModels() {
// Load custom models from models.json first (to know which providers to override)
// Load custom config first (to know which providers to override).
const {
models: customModels = [],
overrides = new Map(),
@@ -1909,7 +1909,7 @@ export class ModelRegistry {
/**
* Get all models (built-in + custom).
* If models.json had errors, returns only built-in models.
* If custom config had errors, returns only built-in models.
*/
getAll(): Model<Api>[] {
return this.#models;
packages/coding-agent/src/config/models-config.ts
+1 -1
View File
@@ -1,5 +1,5 @@
/**
* models.json config file handle and provider configuration validation.
* Custom model/provider config file handle and validation.
*/
import type { Api, ModelSpec } from "@oh-my-pi/pi-ai/types";
packages/coding-agent/test/model-registry-default-config.test.ts
+196
View File
@@ -0,0 +1,196 @@
import { afterEach, beforeEach, describe, expect, test } from "bun:test";
import * as fs from "node:fs";
import * as path from "node:path";
import { TempDir } from "@oh-my-pi/pi-utils";
const packageRoot = path.resolve(import.meta.dir, "..");
let tempDir: TempDir;
describe("ModelRegistry default custom models config", () => {
beforeEach(() => {
tempDir = TempDir.createSync("@model-registry-default-config-");
});
afterEach(async () => {
await tempDir.remove().catch(() => {});
});
test("loads custom provider models from default models.yaml when models.yml is absent", () => {
writeModelsYaml("models.yaml", {
provider: "yaml-default-only",
modelId: "yaml-model",
modelName: "YAML default model",
baseUrl: "https://yaml-default.example.com/v1",
});
const model = loadDefaultRegistryModel({
provider: "yaml-default-only",
modelId: "yaml-model",
});
expect(model?.name).toBe("YAML default model");
expect(model?.baseUrl).toBe("https://yaml-default.example.com/v1");
});
test("prefers default models.yml over models.yaml when both exist", () => {
writeModelsYaml("models.yml", {
provider: "yaml-precedence",
modelId: "from-yml",
modelName: "YML winner",
baseUrl: "https://yml-winner.example.com/v1",
});
writeModelsYaml("models.yaml", {
provider: "yaml-precedence",
modelId: "from-yaml",
modelName: "YAML loser",
baseUrl: "https://yaml-loser.example.com/v1",
});
const ymlModel = loadDefaultRegistryModel({
provider: "yaml-precedence",
modelId: "from-yml",
});
const yamlModel = loadDefaultRegistryModel({
provider: "yaml-precedence",
modelId: "from-yaml",
});
expect(ymlModel?.baseUrl).toBe("https://yml-winner.example.com/v1");
expect(yamlModel).toBeUndefined();
});
test("prefers default models.yaml over legacy models.json when models.yml is absent", () => {
writeModelsYaml("models.yaml", {
provider: "yaml-json-precedence",
modelId: "from-yaml",
modelName: "YAML winner over JSON",
baseUrl: "https://yaml-over-json.example.com/v1",
});
writeModelsJson({
provider: "yaml-json-precedence",
modelId: "from-json",
modelName: "JSON loser",
baseUrl: "https://json-loser.example.com/v1",
});
const yamlModel = loadDefaultRegistryModel({
provider: "yaml-json-precedence",
modelId: "from-yaml",
});
const jsonModel = loadDefaultRegistryModel({
provider: "yaml-json-precedence",
modelId: "from-json",
});
expect(yamlModel?.baseUrl).toBe("https://yaml-over-json.example.com/v1");
expect(jsonModel).toBeUndefined();
});
});
interface ProviderFixture {
provider: string;
modelId: string;
modelName: string;
baseUrl: string;
}
interface ModelLookup {
provider: string;
modelId: string;
}
interface ModelSnapshot {
provider: string;
id: string;
name: string;
baseUrl: string | undefined;
}
function writeModelsYaml(file: "models.yml" | "models.yaml", fixture: ProviderFixture): void {
fs.writeFileSync(
path.join(tempDir.path(), file),
[
"providers:",
` ${fixture.provider}:`,
` baseUrl: ${fixture.baseUrl}`,
" apiKey: TEST_KEY",
" api: anthropic-messages",
" models:",
` - id: ${fixture.modelId}`,
` name: ${fixture.modelName}`,
" reasoning: false",
" input: [text]",
" cost:",
" input: 0",
" output: 0",
" cacheRead: 0",
" cacheWrite: 0",
" contextWindow: 100000",
" maxTokens: 8000",
"",
].join("\n"),
);
}
function writeModelsJson(fixture: ProviderFixture): void {
fs.writeFileSync(
path.join(tempDir.path(), "models.json"),
JSON.stringify({
providers: {
[fixture.provider]: {
baseUrl: fixture.baseUrl,
apiKey: "TEST_KEY",
api: "anthropic-messages",
models: [
{
id: fixture.modelId,
name: fixture.modelName,
reasoning: false,
input: ["text"],
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
contextWindow: 100000,
maxTokens: 8000,
},
],
},
},
}),
);
}
function loadDefaultRegistryModel(lookup: ModelLookup): ModelSnapshot | undefined {
const script = `
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
const authStorage = await AuthStorage.create(":memory:");
try {
const registry = new ModelRegistry(authStorage);
const model = registry.find(${JSON.stringify(lookup.provider)}, ${JSON.stringify(lookup.modelId)});
process.stdout.write(JSON.stringify(model ? {
provider: model.provider,
id: model.id,
name: model.name,
baseUrl: model.baseUrl,
} : null));
} finally {
authStorage.close();
}
`;
const result = Bun.spawnSync([process.execPath, "-e", script], {
cwd: packageRoot,
env: {
...process.env,
PI_CODING_AGENT_DIR: tempDir.path(),
},
stdout: "pipe",
stderr: "pipe",
});
const stdout = new TextDecoder().decode(result.stdout).trim();
const stderr = new TextDecoder().decode(result.stderr).trim();
if (result.exitCode !== 0) {
throw new Error(`default ModelRegistry lookup failed: ${stderr || stdout || `exit ${result.exitCode}`}`);
}
return JSON.parse(stdout) ?? undefined;
}