From e40e9670f81dcde178c3bed8b21e076549d25ca4 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 25 Aug 2026 12:11:15 -0400 Subject: [PATCH] fix(#3705): consult model_policy in the install-time bake so agent frontmatter matches dispatch (#3863) * test(#3705): failing-first coverage for model_policy in the install-time bake * fix(#3705): consult model_policy in the install-time bake so frontmatter matches dispatch * fix(#3705): inject the effective runtime into the policy so runtime_tiers is reached * test(#3705): use assert.doesNotMatch, the assertion that exists * chore(#3705): backfill changeset PR number --------- Co-authored-by: sim --- .changeset/eager-tunas-parade.md | 5 + ...ocs-guard-registration.exempt-baseline.cjs | 2 +- src/install-model-override-resolver.cts | 43 +++- tests/install-runtime-artifacts.test.cjs | 229 +++++++++++++++++- 4 files changed, 276 insertions(+), 3 deletions(-) create mode 100644 .changeset/eager-tunas-parade.md diff --git a/.changeset/eager-tunas-parade.md b/.changeset/eager-tunas-parade.md new file mode 100644 index 000000000..9bd6a5ea9 --- /dev/null +++ b/.changeset/eager-tunas-parade.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3863 +--- +**Agent frontmatter no longer reverts to catalog Anthropic models when `model_policy` is configured** — the install-time bake for the static-frontmatter runtimes (OpenCode, Kilo) read `model_profile` and `model_profile_overrides` but never `model_policy`, so every update rewrote agent `model:` fields to `anthropic/claude-*` IDs that a custom provider does not serve, while dispatch-time resolution honored the policy correctly. The bake now consults the same policy resolver dispatch uses, at the same precedence: an explicit per-agent `model_overrides` entry still wins, then `model_policy`, then the tier table. (#3705) diff --git a/scripts/lint-docs-guard-registration.exempt-baseline.cjs b/scripts/lint-docs-guard-registration.exempt-baseline.cjs index a9486526c..a646626ed 100644 --- a/scripts/lint-docs-guard-registration.exempt-baseline.cjs +++ b/scripts/lint-docs-guard-registration.exempt-baseline.cjs @@ -139,7 +139,7 @@ const DOCS_GUARD_EXEMPT_DOCS_PATHS = { 'gsd-agent-isolation-guard.test.cjs': ['docs/adr/1239-...md', 'docs/adr/1239-gsd-embeddable-orchestration-engine.md'], 'hermes-dispatch-upgrade.test.cjs': ['docs/guides/delegation-patterns.md'], 'install-minimal-hooks.test.cjs': ['docs/en/hooks', 'docs/en/users/features/hooks'], - 'install-runtime-artifacts.test.cjs': ['docs/adr/58-...md', 'docs/adr/58-runtime-install-policy-module.md', 'docs/cli/slash-commands'], + 'install-runtime-artifacts.test.cjs': ['docs/CONFIGURATION.md', 'docs/adr/58-...md', 'docs/adr/58-runtime-install-policy-module.md', 'docs/cli/slash-commands'], 'installer-migration-config-root-marker.test.cjs': ['docs/installer-migrations.md'], 'installer-migration-pi-extension-ext.test.cjs': ['docs/installer-migrations.md'], 'installer-migrations.test.cjs': ['docs/installer-migrations.md'], diff --git a/src/install-model-override-resolver.cts b/src/install-model-override-resolver.cts index 23fa14086..1ec43ed02 100644 --- a/src/install-model-override-resolver.cts +++ b/src/install-model-override-resolver.cts @@ -45,8 +45,9 @@ import modelCatalog = require('./model-catalog.cjs'); const { MODEL_PROFILES: GSD_MODEL_PROFILES } = modelCatalog as unknown as { MODEL_PROFILES: Record> }; // eslint-disable-next-line @typescript-eslint/no-require-imports -- model-resolver.cjs is an export= CommonJS module import modelResolverModule = require('./model-resolver.cjs'); -const { resolveTierEntry: gsdResolveTierEntry } = modelResolverModule as { +const { resolveTierEntry: gsdResolveTierEntry, resolveModelPolicy: gsdResolveModelPolicy } = modelResolverModule as { resolveTierEntry: (opts: { runtime: string; tier: string; overrides: unknown }) => { model?: string } | null; + resolveModelPolicy: (policy: unknown, tier: string | null | undefined) => string | null; }; interface ReadOptions { @@ -129,6 +130,14 @@ interface RuntimeProfileMergedConfig { runtime: string | null; model_profile: string; model_profile_overrides: unknown; + /** + * #3705: the provider-neutral policy (#49). Install-time never read it, so a + * project configured for a non-Anthropic provider had its agent frontmatter + * rebaked to catalog `anthropic/claude-*` IDs on every update while + * dispatch-time resolution honoured the policy correctly — two answers from + * one config, and only the frontmatter is what a spawn actually uses. + */ + model_policy: unknown; } /** @@ -169,6 +178,11 @@ function readGsdRuntimeProfileResolver(targetDir: string | null = null): Runtime (projectConfig && projectConfig.model_profile_overrides) || (homeDefaults && homeDefaults.model_profile_overrides) || null, + // #3705: same project-wins-over-home precedence as every other key here. + model_policy: + (projectConfig && projectConfig.model_policy) || + (homeDefaults && homeDefaults.model_policy) || + null, }; if (!merged.runtime) return null; @@ -188,6 +202,33 @@ function readGsdRuntimeProfileResolver(targetDir: string | null = null): Runtime if (!agentModels) return null; const tier = agentModels[profile] || agentModels.balanced; if (!tier) return null; + // #3705: policy sits between the per-agent override and the tier table — + // the position dispatch uses. Measured against `query resolve-model` on the + // same config: policy-only -> the policy model; tier-overrides-only -> the + // override; BOTH -> the policy model. So policy outranks + // `model_profile_overrides`, and an explicit `model_overrides[agent]` + // (applied by resolveAgentModelOverride below) outranks policy. + // + // Calls the exported owner (#49) rather than re-deriving tier -> model here: + // a second implementation of that mapping is the very divergence this issue + // is, one layer down. A null from it (unknown provider, missing tier key, a + // `runtime_tiers` miss) falls through to the tier table — never to null, + // which would omit a frontmatter key that used to be written. + // The policy object must carry the effective runtime before it is resolved. + // `resolveModelPolicy`'s `runtime_tiers` branch reads `policy['runtime']`, but + // the documented config shape (docs/CONFIGURATION.md) puts `runtime` at the + // TOP level and keeps only `provider`/`runtime_tiers` inside `model_policy`. + // Dispatch injects it — `{ ...config.model_policy, runtime: effectiveRuntime }` + // (model-resolver.cts) — and passing the policy unmodified here silently + // skipped `runtime_tiers` entirely, falling through to the flat hi/med/lo keys + // or the catalog tier. That is the same "catalog Anthropic ID baked over a + // configured provider" defect this fix exists to close, so the injection is + // mirrored rather than assumed. + const policyForRuntime = merged.model_policy + ? { ...(merged.model_policy as Record), runtime } + : null; + const policyModel = gsdResolveModelPolicy(policyForRuntime, tier); + if (policyModel) return { model: policyModel }; return gsdResolveTierEntry({ runtime, tier, diff --git a/tests/install-runtime-artifacts.test.cjs b/tests/install-runtime-artifacts.test.cjs index 9f67468d3..dd8fd6ff2 100644 --- a/tests/install-runtime-artifacts.test.cjs +++ b/tests/install-runtime-artifacts.test.cjs @@ -2704,7 +2704,7 @@ const path = require('node:path'); const { install, } = require('../bin/install.js'); -const { readGsdRuntimeProfileResolver } = require('../gsd-core/bin/lib/install-model-override-resolver.cjs'); +const { readGsdRuntimeProfileResolver, resolveAgentModelOverride } = require('../gsd-core/bin/lib/install-model-override-resolver.cjs'); const { createTempDir, cleanup } = require('./helpers.cjs'); const makeTmp = (prefix) => createTempDir(`gsd-2794-${prefix}-`); @@ -2781,6 +2781,233 @@ describe('bug-2794: readGsdRuntimeProfileResolver resolves opencode tier overrid }); }); +// ──────────────────────────────────────────────────────────────────────── +// #3705 — install-time bake honours model_policy, matching dispatch +// ──────────────────────────────────────────────────────────────────────── +// +// The install-time chain read `runtime` / `model_profile` / +// `model_profile_overrides` and nothing else — `model_policy` appeared ZERO +// times in bin/install.js and in install-model-override-resolver.cts, against 16 +// in the dispatch resolver. So a project configured for a non-Anthropic provider +// had every agent's frontmatter rebaked to catalog `anthropic/claude-*` IDs on +// each update, while `query resolve-model` returned the policy model. Only the +// frontmatter is what a spawn uses, so the disagreement was silent — OpenCode +// then falls back to the un-typed `general` subagent on the session model. +// +// Precedence here was MEASURED against dispatch on the same config, not assumed: +// policy-only -> policy model; tier-overrides-only -> the override; BOTH -> +// policy model. So `model_overrides` > `model_policy` > tier table > catalog. +// +// Home is sandboxed via HOME *and* USERPROFILE (the #2794 block's convention) — +// `os.homedir()` reads both, and setting only HOME passes vacuously on Windows. +describe('#3705: install-time bake honours model_policy', () => { + let projectDir; + let homeDir; + let origHome; + let origUP; + + const POLICY = { + provider: 'generic', + high: 'synthetic/hf:moonshotai/Kimi-K3', + medium: 'synthetic/hf:zai-org/GLM-5.2', + low: 'synthetic/hf:zai-org/GLM-5.2', + }; + const TIER_OVERRIDES = { opencode: { opus: 'TIEROVERRIDE-opus', sonnet: 'TIEROVERRIDE-sonnet' } }; + + beforeEach(() => { + projectDir = makeTmp('proj-3705'); + homeDir = makeTmp('home-3705'); + origHome = process.env.HOME; + origUP = process.env.USERPROFILE; + process.env.HOME = homeDir; + process.env.USERPROFILE = homeDir; + }); + + afterEach(() => { + if (origHome === undefined) delete process.env.HOME; else process.env.HOME = origHome; + if (origUP === undefined) delete process.env.USERPROFILE; else process.env.USERPROFILE = origUP; + cleanup(projectDir); + cleanup(homeDir); + }); + + const projectConfig = (cfg) => writeJson(path.join(projectDir, '.planning', 'config.json'), cfg); + const homeDefaults = (cfg) => writeJson(path.join(homeDir, '.gsd', 'defaults.json'), cfg); + const bake = (agent, overrides = null) => + resolveAgentModelOverride(agent, overrides, readGsdRuntimeProfileResolver(projectDir)); + + // ── the defect ──────────────────────────────────────────────────────── + + test('a generic policy is baked instead of the catalog anthropic tier', () => { + projectConfig({ runtime: 'opencode', model_policy: POLICY }); + + // gsd-executor's balanced tier is sonnet -> the policy's `medium`. + assert.strictEqual(bake('gsd-executor'), POLICY.medium); + assert.doesNotMatch(String(bake('gsd-executor')), /^anthropic\/claude-/, + 'the whole defect is the catalog Anthropic ID being baked over a configured provider'); + }); + + test('tiers map to the policy the same way across agents', () => { + projectConfig({ runtime: 'opencode', model_policy: POLICY }); + + // gsd-planner is an opus-tier agent, gsd-code-reviewer a sonnet-tier one. + assert.strictEqual(bake('gsd-planner'), POLICY.high); + assert.strictEqual(bake('gsd-code-reviewer'), POLICY.medium); + }); + + test('policy outranks model_profile_overrides, matching dispatch', () => { + projectConfig({ + runtime: 'opencode', + model_policy: POLICY, + model_profile_overrides: TIER_OVERRIDES, + }); + + assert.strictEqual(bake('gsd-executor'), POLICY.medium, + 'measured dispatch order: with both configured, policy wins'); + }); + + test('an explicit per-agent model_overrides entry still outranks policy (#2256)', () => { + projectConfig({ + runtime: 'opencode', + model_policy: POLICY, + model_profile_overrides: TIER_OVERRIDES, + }); + + assert.strictEqual(bake('gsd-executor', { 'gsd-executor': 'EXPLICIT' }), 'EXPLICIT'); + }); + + test('kilo is fixed by the same shared seam, not an opencode special case (#2794 J8)', () => { + // The docstring on this seam says kilo and opencode "can never diverge" + // because they route through one function. That claim is only worth + // anything if it is exercised. + projectConfig({ runtime: 'kilo', model_policy: POLICY }); + + assert.strictEqual(bake('gsd-executor'), POLICY.medium); + }); + + // ── negative space: no policy must bake EXACTLY as before ───────────── + + test('without a policy, tier overrides bake exactly as they did', () => { + projectConfig({ runtime: 'opencode', model_profile_overrides: TIER_OVERRIDES }); + + assert.strictEqual(bake('gsd-executor'), 'TIEROVERRIDE-sonnet'); + }); + + test('without a policy or overrides, the catalog tier bakes exactly as it did', () => { + // The load-bearing control: this change adds a step to a chain that runs on + // EVERY install for both static-frontmatter runtimes. If a project with no + // policy bakes anything different, the fix has silently re-baked every + // existing user. + projectConfig({ runtime: 'opencode' }); + + assert.match(String(bake('gsd-executor')), /^anthropic\/claude-/); + }); + + test('model_profile: inherit still omits the key, policy notwithstanding', () => { + // Policy must not resurrect a bake the user disabled (#3543's intent). + projectConfig({ runtime: 'opencode', model_profile: 'inherit', model_policy: POLICY }); + + assert.strictEqual(bake('gsd-executor'), null); + }); + + test('a policy that resolves to nothing falls through to the tier table, never to null', () => { + // Returning null here would OMIT a frontmatter key that used to be written — + // a different regression than the one being fixed. + projectConfig({ runtime: 'opencode', model_policy: { provider: 'not-a-real-provider' } }); + assert.match(String(bake('gsd-executor')), /^anthropic\/claude-/, 'unknown provider'); + + projectConfig({ runtime: 'opencode', model_policy: { provider: 'generic', medium: 'M', low: 'L' } }); + assert.match(String(bake('gsd-planner')), /^anthropic\/claude-/, + 'policy declares no `high`, so the opus-tier agent falls through'); + assert.strictEqual(bake('gsd-executor'), 'M', 'while the tiers it DOES declare still resolve'); + }); + + test('an agent absent from MODEL_PROFILES yields null, not a throw', () => { + projectConfig({ runtime: 'opencode', model_policy: POLICY }); + + assert.strictEqual(bake('gsd-nonexistent-agent'), null); + }); + + // ── config precedence ──────────────────────────────────────────────── + + test('a policy in ~/.gsd/defaults.json is honoured', () => { + homeDefaults({ runtime: 'opencode', model_profile: 'balanced', model_policy: POLICY }); + projectConfig({ runtime: 'opencode' }); + + assert.strictEqual(bake('gsd-executor'), POLICY.medium); + }); + + test('a project policy wins over a home-defaults policy', () => { + homeDefaults({ runtime: 'opencode', model_profile: 'balanced', model_policy: POLICY }); + projectConfig({ + runtime: 'opencode', + model_policy: { provider: 'generic', high: 'P-high', medium: 'P-medium', low: 'P-low' }, + }); + + assert.strictEqual(bake('gsd-executor'), 'P-medium'); + }); + + // ── the rest of resolveModelPolicy's contract, reached through the bake ── + + test('a named provider preset resolves through the same owner', () => { + projectConfig({ runtime: 'opencode', model_policy: { provider: 'openai' } }); + + const baked = bake('gsd-executor'); + assert.ok(baked, 'a known preset must resolve'); + assert.doesNotMatch(String(baked), /^anthropic\/claude-/, 'the preset, not the catalog default'); + }); + + test('the runtime_tiers escape hatch is honoured for the DOCUMENTED config shape', () => { + // Review finding (blocker). The first version of this test put a `runtime` + // key INSIDE `model_policy`, duplicating the top-level one. No real config + // does that — docs/CONFIGURATION.md puts `runtime` at the top level and keeps + // only `provider`/`runtime_tiers` inside the policy — and that one fabricated + // key was the only reason the test passed. `resolveModelPolicy`'s + // runtime_tiers branch reads `policy['runtime']`, which dispatch injects and + // the bake did not, so runtime_tiers was silently skipped for every real + // config. The fixture below is the documented shape verbatim. + projectConfig({ + runtime: 'opencode', + model_policy: { + provider: 'generic', + runtime_tiers: { opencode: { sonnet: 'runtime-tiers-model' } }, + medium: 'GENERIC-medium', + }, + }); + + assert.strictEqual(bake('gsd-executor'), 'runtime-tiers-model', + 'runtime_tiers must outrank the flat generic keys — and must be reached at all'); + }); + + test('runtime_tiers accepts the object entry form the docs show', () => { + // docs/CONFIGURATION.md's own example uses `{ model, reasoning_effort }` + // rather than a bare string. + projectConfig({ + runtime: 'codex', + model_policy: { + provider: 'openai', + runtime_tiers: { codex: { sonnet: { model: 'gpt-5.6-terra', reasoning_effort: 'medium' } } }, + }, + }); + + assert.strictEqual(bake('gsd-executor'), 'gpt-5.6-terra'); + }); + + test('a runtime_tiers block for a DIFFERENT runtime does not leak', () => { + // The injected runtime is what selects the block; a mismatch must fall + // through rather than borrow another runtime's pins. + projectConfig({ + runtime: 'opencode', + model_policy: { + provider: 'generic', + runtime_tiers: { codex: { sonnet: 'CODEX-only' } }, + medium: 'GENERIC-medium', + }, + }); + + assert.strictEqual(bake('gsd-executor'), 'GENERIC-medium'); + }); +}); + describe('bug-2794: OpenCode agent install embeds model_profile_overrides model', () => { let projectDir; let homeDir;