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 <sim@local>
This commit is contained in:
Tom Boucher
5
.changeset/eager-tunas-parade.md
Normal file
5
.changeset/eager-tunas-parade.md
Normal file
@@ -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)
|
||||
@@ -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'],
|
||||
|
||||
@@ -45,8 +45,9 @@ import modelCatalog = require('./model-catalog.cjs');
|
||||
const { MODEL_PROFILES: GSD_MODEL_PROFILES } = modelCatalog as unknown as { MODEL_PROFILES: Record<string, Record<string, string>> };
|
||||
// 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<string, unknown>), runtime }
|
||||
: null;
|
||||
const policyModel = gsdResolveModelPolicy(policyForRuntime, tier);
|
||||
if (policyModel) return { model: policyModel };
|
||||
return gsdResolveTierEntry({
|
||||
runtime,
|
||||
tier,
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user