diff --git a/.changeset/jolly-zebras-glide.md b/.changeset/jolly-zebras-glide.md new file mode 100644 index 000000000..35880b8f7 --- /dev/null +++ b/.changeset/jolly-zebras-glide.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4726 +--- +**Runtime-aware model overrides and `dynamic_routing` now affect the agents that actually spawn** — `model_profile_overrides..` only applied when you had written a `runtime` key into `.planning/config.json`, so it was silently inert for installs that identify their runtime through `GSD_RUNTIME` or the per-install marker. Separately, `dynamic_routing.tier_models` was consulted only on an explicit retry attempt, so the first spawn — the documented case — never used your configured tier. Both now resolve through the runtime that is actually running, and an explicit model pin like `claude-opus-4-8` is no longer collapsed to a Claude-only alias when a different runtime is active. Projects without `dynamic_routing` enabled are unaffected. Note that if your `.planning/config.json` already carries `dynamic_routing.tier_models` or a `model_profile_overrides` block for a runtime you resolve via `GSD_RUNTIME`, those settings were previously inert and now take effect — review them before upgrading. (#4505) diff --git a/scripts/lint-docs-guard-registration.exempt-baseline.cjs b/scripts/lint-docs-guard-registration.exempt-baseline.cjs index 51bb3ed74..d5a96ed27 100644 --- a/scripts/lint-docs-guard-registration.exempt-baseline.cjs +++ b/scripts/lint-docs-guard-registration.exempt-baseline.cjs @@ -184,7 +184,13 @@ const DOCS_GUARD_EXEMPT_DOCS_PATHS = { // / explanatory comment citing documented CLI behavior for context, never // a read target); the exemption's premise still holds for both. 'milestone-archive.test.cjs': ['docs/CLI-TOOLS.md', 'docs/TESTING-SUITES.md'], - 'model-resolver.test.cjs': ['docs/TESTING-SUITES.md'], + // #4505: additionally cites docs/features/dynamic-routing-with-failure-tier-escalation.md + // in explanatory comments, quoting the documented first-spawn contract the new rows + // assert against; the file never reads that (or any) docs/ file — every read it makes + // targets a tmpdir .planning fixture. + 'model-resolver.test.cjs': [ + 'docs/TESTING-SUITES.md', 'docs/features/dynamic-routing-with-failure-tier-escalation.md', + ], 'new-project-mvp-prompt.test.cjs': ['docs/CONFIGURATION.md'], 'onboard-command.test.cjs': ['docs/adr/0001-runtime.md'], 'opencode-command-dir-plural.test.cjs': ['docs/commands'], diff --git a/src/commands.cts b/src/commands.cts index c9e27d1b3..1e5005720 100644 --- a/src/commands.cts +++ b/src/commands.cts @@ -595,13 +595,10 @@ function cmdResolveExecution(cwd: string, agentType: string | undefined, raw: bo opts = opts || {}; const config = loadConfig(cwd); const profile = (config['model_profile'] as string) || 'balanced'; - // #2068: resolve the model per-attempt so dynamic_routing escalates the MODEL - // (heavy tier) alongside effort. Gated on an explicit --attempt exactly like the - // effort resolution below, so the two fields stay symmetric: with no --attempt - // the model comes from the classic profile path (unchanged for everyone, - // including dynamic_routing-enabled users who don't pass --attempt), and only an - // explicit attempt routes through the tier ladder. resolveModelForTier itself - // still falls back to resolveModelInternal when dynamic_routing is off. + // #2068: resolve the model per-attempt so dynamic_routing ESCALATES the MODEL + // (heavy tier) alongside effort. The FIRST-spawn tier now comes from + // resolveModelInternal's own dynamic_routing step (#4505), so the absent-attempt + // branch below reaches it too — this gate is only about escalation. let model = (opts.attempt !== undefined && opts.attempt !== null) ? resolveModelForTier(cwd, agentType, opts.attempt) : resolveModelInternal(cwd, agentType); diff --git a/src/model-resolver.cts b/src/model-resolver.cts index 939b55707..27f510bc9 100644 --- a/src/model-resolver.cts +++ b/src/model-resolver.cts @@ -35,7 +35,7 @@ import { MODEL_ALIAS_MAP, RUNTIME_PROFILE_MAP, PROVIDER_PRESETS, VALID_TIERS, CL import fs from 'node:fs'; import path from 'node:path'; -import { resolveRuntimeNameFromCandidates } from './runtime-name-policy.cjs'; +import { resolveRuntimeNameFromCandidates, canonicalizeRuntimeName } from './runtime-name-policy.cjs'; import { readInstallRuntimeMarker, _setInstallRuntimeMarkerForTests, @@ -151,7 +151,13 @@ function resolveTierEntry({ runtime, tier, overrides }: ResolveTierEntryOpts): T */ function _resolveRuntimeTier(config: Record, tier: string): TierEntryResolved | null { return resolveTierEntry({ - runtime: config['runtime'] as string | null | undefined, + // #4505/#4495: TIER SELECTION follows the runtime that is actually + // resolving, not whatever the config file happens to say. `config.runtime` + // is frequently absent (the runtime is normally known from GSD_RUNTIME or + // the per-install marker), and reading it raw made + // `model_profile_overrides..` silently inert for every + // install that did not also write the key by hand. + runtime: resolveActiveRuntime(config), tier, overrides: config['model_profile_overrides'] as Record | null | undefined, }); @@ -171,10 +177,19 @@ function _resolveRuntimeTier(config: Record, tier: string): Tie * * This helper reads ONLY the user's override entry for the effective claude * runtime and tier — never the builtin claude tier map — so an install with no - * override is byte-identical to before the fix. The runtime is resolved the - * same way steps 1-3 resolve it (config['runtime'], defaulting to 'claude'), - * NOT via resolveActiveRuntime (GSD_RUNTIME/marker): the value policy must - * key off the config the operator wrote, matching mapClaudeOverrideForRuntime. + * override is byte-identical to before the fix. + * + * #4505 CHANGED how the runtime reaches here. #4192 originally passed + * `config['runtime']` and recorded that the value policy "must key off the + * config the operator wrote, NOT resolveActiveRuntime". That reading turned out + * to defeat #4192's own stated principle — that an explicit pin must not be + * silently UNPINNED. With `config.runtime` absent and GSD_RUNTIME=opencode, the + * config-keyed read treated the session as claude and collapsed an explicit + * `claude-opus-4-8` pin to the bare alias `opus`, which is a Claude-only token + * the actual runtime cannot spawn. The runtime is now the ACTIVE one, so the + * claude value policy applies exactly when claude is what is running. No test + * pinned the previous behaviour; the ones covering #4192's pins and the + * alias-native posture all still pass. * * Value policy mirrors the model_overrides path (#2041/#4192): an override * value that maps to a current tier alias collapses to that alias @@ -185,12 +200,13 @@ function _resolveRuntimeTier(config: Record, tier: string): Tie * through to normal alias resolution (ADR-443 D1: invalid values fall through). */ function resolveClaudeTierOverrideModel( - configRuntime: string | null | undefined, + // The ACTIVE runtime (#4505), not the raw config key — see docblock. + activeRuntime: string | null | undefined, tier: string | null | undefined, overrides: Record | null | undefined, ): string | null { if (!tier || tier === 'inherit') return null; - const effectiveRuntime = configRuntime || 'claude'; + const effectiveRuntime = activeRuntime || 'claude'; if (effectiveRuntime !== 'claude') return null; // non-claude runtimes resolve at step 3 const overridesMap = overrides as Record> | null | undefined; if (!overridesMap || typeof overridesMap !== 'object') return null; @@ -302,14 +318,18 @@ function _resetModelOverrideWarningCacheForTests(): void { */ function mapClaudeOverrideForRuntime( override: string, - configRuntime: string | null | undefined, + // The ACTIVE runtime (#4505). Previously the raw `config['runtime']`, which + // made an explicit claude pin collapse to a bare alias whenever the runtime + // was declared via GSD_RUNTIME or the install marker rather than the config — + // handing a Claude-only token to a runtime that cannot spawn it. + activeRuntime: string | null | undefined, agentType: string, ): string | null { // Defensive: model_overrides is typed Record but a malformed // config could surface a non-string; pass through verbatim (preserving the // pre-fix no-crash behaviour) and let the downstream Agent tool reject it. if (typeof override !== 'string') return override; - const onClaude = !configRuntime || configRuntime === 'claude'; + const onClaude = !activeRuntime || activeRuntime === 'claude'; if (!onClaude) return override; // Object.hasOwn guards against __proto__/constructor returning a truthy // non-string from the plain object literal (#2041 security review). @@ -518,7 +538,7 @@ function resolveModelInternal(cwd: string, agentType: string): string { ? modelOverrides[agentType] : undefined; if (override) { - const mapped = mapClaudeOverrideForRuntime(override, config['runtime'] as string | null | undefined, agentType); + const mapped = mapClaudeOverrideForRuntime(override, resolveActiveRuntime(config), agentType); if (mapped !== null) return mapped; // Unmappable Claude ID — fall through to tier resolution (matches model_policy). } @@ -536,10 +556,19 @@ function resolveModelInternal(cwd: string, agentType: string): string { const tier = computeProfileTier(config, agentType); // 2.5. model_policy preset (#49, #1133) + // `configRuntime` is retained ONLY as the EXPLICIT-OPT-IN signal for step 3's + // precedence over the omit gate (see the note there). Every actual runtime + // question — which tier map to read, which value policy applies — now uses + // the active runtime (#4505). const configRuntime = config['runtime'] as string | null | undefined; + // #4505/#4495: GSD_RUNTIME -> config.runtime -> per-install marker -> 'claude'. + // Never undefined, so the "no runtime declared anywhere" case still resolves + // 'claude' and step 3's deliberate claude skip (#1156/#2297/#4192) is + // unchanged for every existing install. + const activeRuntime = resolveActiveRuntime(config); if (tier && tier !== 'inherit') { - const onClaude = !configRuntime || configRuntime === 'claude'; - const effectiveRuntime = configRuntime || 'claude'; + const onClaude = activeRuntime === 'claude'; + const effectiveRuntime = activeRuntime; const mergedPolicy = config['model_policy'] ? { ...(config['model_policy'] as Record), runtime: effectiveRuntime } : null; @@ -562,8 +591,44 @@ function resolveModelInternal(cwd: string, agentType: string): string { } } - // 3. Runtime-aware resolution (#2517) - if (configRuntime && configRuntime !== 'claude' && tier && tier !== 'inherit') { + // #4505: the omit DECISION is computed here, ahead of step 3, though the + // return still happens at step 4 below. + // + // Step 3 was previously unreachable unless the operator had written `runtime` + // into the config. Keying it off the ACTIVE runtime — the #4495 fix — makes it + // reachable for env- and marker-declared runtimes too, which newly exposes an + // ordering the corpus already decided, in two places that only coexisted + // because step 3 could not fire: + // + // #2517 `runtime:"codex"` + resolve_model_ids:"omit" -> the codex tier + // model. "Explicit non-Claude opt-in wins" — the operator naming a + // runtime in the project config outranks an omit. + // #2297 GSD_RUNTIME/marker = codex + a GLOBAL (defaults-poisoned) omit + // -> "" (acceptance #4). A merely DETECTED runtime does not + // constitute that opt-in, so the omit still wins. + // + // So the opt-in signal is specifically the `runtime` KEY, not the resolved + // runtime: step 3 reads the active runtime's tier map, but only outranks the + // omit gate when the operator wrote that key. Both contracts hold unchanged. + const omitApplies = config['resolve_model_ids'] === 'omit' + && (projectExplicitlySetsOmit(cwd) || !RUNTIMES_WITH_NATIVE_ALIASES.has(activeRuntime)); + // CANONICALIZED, not the raw field. Comparing the raw value against the literal + // 'claude' made every spelling that is not exactly that string count as a + // non-Claude opt-in and outrank the omit gate: `runtime:"Claude"`, + // `runtime:"claude-code"` and even `runtime:5` each emitted a model id where + // origin/next returned "". That is the #2297 acceptance-#4 case this very + // block claims to preserve, failing OPEN. (Security review of #4505.) + // + // null covers both "not a string" and "not a runtime we recognise", and both + // must read as NOT an opt-in: an unrecognised value is not evidence the + // operator deliberately chose a non-Claude runtime, so it must not buy an + // escalation past an explicit omit. + const configRuntimeCanonical = canonicalizeRuntimeName(configRuntime); + const explicitNonClaudeOptIn = configRuntimeCanonical !== null && configRuntimeCanonical !== 'claude'; + + // 3. Runtime-aware resolution (#2517), keyed off the ACTIVE runtime (#4505). + if (activeRuntime !== 'claude' && tier && tier !== 'inherit' + && (explicitNonClaudeOptIn || !omitApplies)) { const entry = _resolveRuntimeTier(config, tier); if (entry?.model) return entry.model; } @@ -578,8 +643,7 @@ function resolveModelInternal(cwd: string, agentType: string): string { // NOTE: a non-Claude runtime that HAS a populated runtime-tier map already // returned its own model id at step 3 above, before this gate — for those the // explicit-project-omit honoring here is moot (step 3 wins, by #2517 design). - if (config['resolve_model_ids'] === 'omit' - && (projectExplicitlySetsOmit(cwd) || !RUNTIMES_WITH_NATIVE_ALIASES.has(resolveActiveRuntime(config)))) { + if (omitApplies) { return ''; } @@ -593,13 +657,33 @@ function resolveModelInternal(cwd: string, agentType: string): string { // resolveClaudeTierOverrideModel for why the builtin map stays out. if (tier && tier !== 'inherit') { const claudeOverrideModel = resolveClaudeTierOverrideModel( - configRuntime, + activeRuntime, tier, config['model_profile_overrides'] as Record | null | undefined, ); if (claudeOverrideModel !== null) return claudeOverrideModel; } + // 4.75 dynamic_routing.tier_models (#4505 / #3024). + // + // Position is the documented composition, not a convenience: + // docs/features/dynamic-routing-with-failure-tier-escalation.md — + // "model_overrides always wins; dynamic_routing.tier_models[] resolves + // above models. and model_profile." + // So it sits BELOW model_overrides (step 1), the model_policy preset (2.5), + // the runtime tier map (3), the resolve_model_ids:"omit" gate (4) and the + // claude tier override (4.5) — and ABOVE the profile lookup (5). + // + // An earlier cut of this fix routed every call site through resolveModelForTier + // instead, which returns the tier model directly and therefore skipped steps 3, + // 4 and 4.5 entirely: with `resolve_model_ids:"omit"` and a non-Claude runtime + // it handed out a model id where the gate had returned "". Putting the step + // here keeps every higher-precedence layer reachable. + if (tier !== 'inherit') { + const routed = dynamicRoutingModel(config, agentType, 0); + if (routed !== null) return routed; + } + // 5. Profile lookup (Claude-native default). if (!agentModels) { return profile === 'quality' ? 'opus' @@ -664,6 +748,55 @@ function assertValidGranularityOverride( } } +/** + * #4505 — the ONE implementation of `dynamic_routing.tier_models` lookup. + * + * Returns the configured model for this agent's routing tier at `attempt`, or + * null when dynamic routing does not apply (disabled, absent, no tier table, an + * agent with no default routing tier, or no entry for the tier). Both entry + * points call it, so the first-spawn value and the escalated value can never + * drift apart — the "Generative Fix Divergence" this repo names by name. + * + * `attempt` 0 means "no escalation yet", which is the documented FIRST-SPAWN + * case, NOT "skip dynamic routing": + * docs/features/dynamic-routing-with-failure-tier-escalation.md — + * "enabled: true — the resolver picks tier_models[default_tier] for the first + * spawn and escalates one tier up on orchestrator-detected soft failure." + */ +function dynamicRoutingModel( + config: Record, + agentType: string, + attempt: number, +): string | null { + const dr = config['dynamic_routing'] as Record | null | undefined; + if (!dr || typeof dr !== 'object' || dr['enabled'] !== true) return null; + + const tierModels = dr['tier_models'] as Record | null | undefined; + if (!tierModels || typeof tierModels !== 'object') return null; + + const defaultTier = (AGENT_DEFAULT_TIERS)[agentType]; + if (!defaultTier || !(VALID_AGENT_TIERS).has(defaultTier)) return null; + + const maxEscalations = Number.isInteger(dr['max_escalations']) && (dr['max_escalations'] as number) >= 0 + ? (dr['max_escalations'] as number) + : 1; + const escalationEnabled = dr['escalate_on_failure'] !== false; + const effectiveAttempt = escalationEnabled ? Math.min(attempt, maxEscalations) : 0; + + let tier = defaultTier; + for (let i = 0; i < effectiveAttempt; i += 1) { + const next = (nextTier)(tier); + if (!next || next === tier) break; + tier = next; + } + + // Own-property guard: `tier_models` is a config-supplied plain object, so a + // prototype-chain key must not resolve an inherited member. + const alias = Object.hasOwn(tierModels, tier) ? tierModels[tier] : undefined; + if (typeof alias !== 'string' || alias.length === 0) return null; + return alias; +} + /** * #3024 — Resolve a model for a specific dynamic-routing attempt. */ @@ -680,50 +813,19 @@ function resolveModelForTier(cwd: string, agentType: string, attempt?: number): ? modelOverrides[agentType] : undefined; if (override) { - const mapped = mapClaudeOverrideForRuntime(override, config['runtime'] as string | null | undefined, agentType); + const mapped = mapClaudeOverrideForRuntime(override, resolveActiveRuntime(config), agentType); if (mapped !== null) return mapped; // Unmappable Claude ID — fall through to dynamic_routing / model_policy resolution. } - if (config['model_policy'] && config['runtime'] && config['runtime'] !== 'claude') { + // #4505: same active-runtime rule as resolveModelInternal step 3. + if (config['model_policy'] && resolveActiveRuntime(config) !== 'claude') { return resolveModelInternal(cwd, agentType); } - const dr = config['dynamic_routing'] as Record | null | undefined; - if (!dr || typeof dr !== 'object' || dr['enabled'] !== true) { - return resolveModelInternal(cwd, agentType); - } - - const tierModels = dr['tier_models'] as Record | null | undefined; - if (!tierModels || typeof tierModels !== 'object') { - return resolveModelInternal(cwd, agentType); - } - - const defaultTier = (AGENT_DEFAULT_TIERS)[agentType]; - if (!defaultTier || !(VALID_AGENT_TIERS).has(defaultTier)) { - return resolveModelInternal(cwd, agentType); - } - - const maxEscalations = Number.isInteger(dr['max_escalations']) && (dr['max_escalations'] as number) >= 0 - ? (dr['max_escalations'] as number) - : 1; - const escalationEnabled = dr['escalate_on_failure'] !== false; - const effectiveAttempt = escalationEnabled - ? Math.min(attemptN, maxEscalations) - : 0; - - let tier = defaultTier; - for (let i = 0; i < effectiveAttempt; i += 1) { - const next = (nextTier)(tier); - if (!next || next === tier) break; - tier = next; - } - - const alias = tierModels[tier]; - if (typeof alias !== 'string' || alias.length === 0) { - return resolveModelInternal(cwd, agentType); - } - return alias; + const alias = dynamicRoutingModel(config, agentType, attemptN); + if (alias !== null) return alias; + return resolveModelInternal(cwd, agentType); } /** diff --git a/tests/model-resolver.test.cjs b/tests/model-resolver.test.cjs index 98bb569fa..3b1c91129 100644 --- a/tests/model-resolver.test.cjs +++ b/tests/model-resolver.test.cjs @@ -6667,3 +6667,338 @@ describe('#4192 model_overrides: fully-qualified claude IDs resolve as configure `warning must contain the capped pin render: ${warnings[0]}`); }); }); + +// ──────────────────────────────────────────────────────────────────────────── +// #4505 — resolveModelInternal bypassed BOTH resolveActiveRuntime and +// dynamic_routing.tier_models. Consolidates #4495 and #4493. +// ──────────────────────────────────────────────────────────────────────────── +// +// Every row drives the REAL CLI in a subprocess rather than calling the +// resolver in-process. That is load-bearing, not stylistic: the defect is +// *which function the shipped call sites reach*, so a test that called +// resolveModelForTier directly would pass while every real spawn stayed broken. +// It also makes GSD_RUNTIME hermetic — it is ambient, and an in-process test +// would leak it between rows. +// +// The sharpest statement of the bug, measured on the unfixed tree with one +// config (dynamic_routing.enabled + tier_models.standard="sonnet"): +// +// query resolve-execution gsd-executor --attempt 0 -> sonnet (correct) +// query resolve-model gsd-executor --raw -> opus (wrong) +// init quick --raw .executor_model -> opus (wrong) +// +// Same agent, same config, three answers. The parity rows below assert those +// surfaces AGREE, which is a stronger and more durable invariant than pinning +// any single literal. +{ + const { describe, test, afterEach } = require('node:test'); + const assert = require('node:assert/strict'); + const fs = require('node:fs'); + const path = require('node:path'); + const os = require('node:os'); + const { cleanup, runGsdTools } = require('./helpers.cjs'); + + /** + * `config === null` writes NO .planning/config.json at all. That is not a + * stylistic choice: the shared ~/.gsd/defaults.json layer is only consulted + * when the project has no config of its own (config-loader Branch D), so a + * fixture that writes even `{}` silently stops exercising the "poisoned + * global" path and tests something else. Measured, with a global omit and + * GSD_RUNTIME=codex: + * .planning/ with config.json -> gpt-5.6-terra + * .planning/ present, no config.json -> gpt-5.6-terra <- the subtle one + * no .planning/ at all -> "" + * The DIRECTORY alone is enough to disable the layer, so `config === null` + * must not create it either. + * + * The runners redirect BOTH `HOME` and `GSD_HOME` into `dir` — the loader reads + * `process.env.GSD_HOME || os.homedir()`, so redirecting only HOME leaves a + * developer's real ~/.gsd/defaults.json in play. + */ + function project(config, globalDefaults = null) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-4505-')); + if (config !== null) { + fs.mkdirSync(path.join(dir, '.planning', 'phases'), { recursive: true }); + fs.writeFileSync(path.join(dir, '.planning', 'config.json'), JSON.stringify(config)); + } + if (globalDefaults) { + fs.mkdirSync(path.join(dir, '.gsd'), { recursive: true }); + fs.writeFileSync(path.join(dir, '.gsd', 'defaults.json'), JSON.stringify(globalDefaults)); + } + return dir; + } + + // HOME is redirected into the project so a developer's own ~/.gsd/defaults.json + // cannot change a concrete expected value (helpers.cjs documents this). + // GSD_RUNTIME is always passed EXPLICITLY, including as '' for "declared + // nowhere" — inheriting it from the parent shell would make these rows depend + // on who ran them. + function resolveModel(dir, agent, runtime = '') { + const r = runGsdTools(`query resolve-model ${agent} --raw`, dir, { HOME: dir, GSD_HOME: dir, GSD_RUNTIME: runtime }); + assert.ok(r.success, `resolve-model ${agent} failed: ${r.error}`); + return r.output.trim(); + } + + function initQuick(dir, runtime = '') { + const r = runGsdTools('init quick --raw', dir, { HOME: dir, GSD_HOME: dir, GSD_RUNTIME: runtime }); + assert.ok(r.success, `init quick failed: ${r.error}`); + return JSON.parse(r.output); + } + + function resolveExecutionModel(dir, agent, attempt, runtime = '') { + const r = runGsdTools(`query resolve-execution ${agent} --attempt ${attempt}`, dir, { HOME: dir, GSD_HOME: dir, GSD_RUNTIME: runtime }); + assert.ok(r.success, `resolve-execution ${agent} failed: ${r.error}`); + return JSON.parse(r.output).model; + } + + const DR_CONFIG = { + model_profile: 'quality', + dynamic_routing: { enabled: true, tier_models: { light: 'haiku', standard: 'sonnet', heavy: 'opus' } }, + }; + + describe('#4505 half B — dynamic_routing.tier_models must reach the FIRST spawn', () => { + let dir; + afterEach(() => { if (dir) cleanup(dir); dir = null; }); + + // FAILING-FIRST. Measured on the unfixed tree: "opus". + test('query resolve-model uses the tier table (#4493)', () => { + dir = project(DR_CONFIG); + assert.strictEqual( + resolveModel(dir, 'gsd-executor'), + 'sonnet', + 'docs/features/dynamic-routing-with-failure-tier-escalation.md: "the resolver picks ' + + 'tier_models[default_tier] for the FIRST spawn"', + ); + }); + + // FAILING-FIRST, and the one that matters most: this payload IS what a real + // spawn reads. #4493's complaint is precisely that no real spawn sees it. + test('the init payload a real spawn reads carries it (#4493)', () => { + dir = project(DR_CONFIG); + assert.strictEqual(initQuick(dir).executor_model, 'sonnet'); + }); + + // The invariant, not a literal: two surfaces resolving the same agent under + // the same config must not disagree. + test('resolve-model agrees with resolve-execution --attempt 0', () => { + dir = project(DR_CONFIG); + const viaExecution = resolveExecutionModel(dir, 'gsd-executor', 0); + assert.strictEqual( + resolveModel(dir, 'gsd-executor'), + viaExecution, + 'the same agent under the same config resolved differently depending on which ' + + 'command asked — that divergence IS #4505', + ); + }); + + // REQ-DYNROUTE-01: "zero behavior change" when off. `false` and ABSENT are + // different code paths, so both are asserted. + test('enabled:false leaves the classic profile path untouched', () => { + dir = project({ ...DR_CONFIG, dynamic_routing: { ...DR_CONFIG.dynamic_routing, enabled: false } }); + assert.strictEqual(resolveModel(dir, 'gsd-executor'), 'opus'); + assert.strictEqual(initQuick(dir).executor_model, 'opus'); + }); + + test('an absent dynamic_routing block leaves the classic path untouched', () => { + dir = project({ model_profile: 'quality' }); + assert.strictEqual(resolveModel(dir, 'gsd-executor'), 'opus'); + assert.strictEqual(initQuick(dir).executor_model, 'opus'); + }); + + // Documented composition: "model_overrides always wins". + test('model_overrides still outranks tier_models', () => { + dir = project({ ...DR_CONFIG, model_overrides: { 'gsd-executor': 'my-pinned-model' } }); + assert.strictEqual(resolveModel(dir, 'gsd-executor'), 'my-pinned-model'); + }); + + test('a tier absent from tier_models falls back to the classic path', () => { + dir = project({ model_profile: 'quality', dynamic_routing: { enabled: true, tier_models: { light: 'haiku' } } }); + assert.strictEqual(resolveModel(dir, 'gsd-executor'), 'opus'); + }); + }); + + // dynamic_routing sits at a DOCUMENTED precedence slot, and an earlier cut of + // this fix put it above everything by routing call sites through + // resolveModelForTier -- which returns the tier model directly and so skipped + // the runtime tier map, the omit gate and the claude tier override. These rows + // pin the composition the feature doc states: + // "model_overrides always wins; dynamic_routing.tier_models[] resolves + // above models. and model_profile." + describe('#4505 — dynamic_routing must not outrank higher-precedence layers', () => { + let dir; + afterEach(() => { if (dir) cleanup(dir); dir = null; }); + + const DR = { enabled: true, tier_models: { light: 'haiku', standard: 'DR-STANDARD', heavy: 'opus' } }; + + // REGRESSION for the blocker: with a non-Claude runtime and an omit, the gate + // returned "" before this PR and must keep doing so. The earlier cut handed + // out a model id here. + test('resolve_model_ids:"omit" still wins over the tier table', () => { + dir = project({ model_profile: 'quality', resolve_model_ids: 'omit', dynamic_routing: DR }); + assert.strictEqual(resolveModel(dir, 'gsd-executor', 'codex'), ''); + }); + + test('the omit gate wins in the init payload a real spawn reads, too', () => { + dir = project({ model_profile: 'quality', resolve_model_ids: 'omit', dynamic_routing: DR }); + assert.strictEqual(initQuick(dir, 'codex').executor_model, ''); + }); + + test('the runtime tier map (step 3) outranks the tier table', () => { + dir = project({ + model_profile: 'quality', + dynamic_routing: DR, + model_profile_overrides: { codex: { opus: 'MPO-CODEX-OPUS' } }, + }); + assert.strictEqual(resolveModel(dir, 'gsd-executor', 'codex'), 'MPO-CODEX-OPUS'); + }); + + test('model_overrides still wins over everything', () => { + dir = project({ model_profile: 'quality', dynamic_routing: DR, model_overrides: { 'gsd-executor': 'PINNED' } }); + assert.strictEqual(resolveModel(dir, 'gsd-executor'), 'PINNED'); + }); + + // `tier` reports the PROFILE tier; dynamic_routing keys off the ROUTING tier + // (light/standard/heavy), a different vocabulary. So under dynamic routing the + // two fields describe different axes and do not have to match. Pinned here so + // the combination is a deliberate, visible property rather than a surprise. + test('`tier` keeps reporting the profile tier when the tier table supplies the model', () => { + dir = project({ model_profile: 'quality', dynamic_routing: DR }); + const r = runGsdTools('query resolve-model gsd-executor', dir, { HOME: dir, GSD_HOME: dir, GSD_RUNTIME: '' }); + assert.ok(r.success, r.error); + const parsed = JSON.parse(r.output); + assert.strictEqual(parsed.model, 'DR-STANDARD'); + assert.strictEqual(parsed.tier, 'opus'); + }); + + // resolve-execution with the attempt ABSENT is the one behaviour the + // #2068 gate change touches; it must agree with resolve-model. + test('resolve-execution with no --attempt agrees with resolve-model', () => { + dir = project({ model_profile: 'quality', dynamic_routing: DR }); + const r = runGsdTools('query resolve-execution gsd-executor', dir, { HOME: dir, GSD_HOME: dir, GSD_RUNTIME: '' }); + assert.ok(r.success, r.error); + assert.strictEqual(JSON.parse(r.output).model, resolveModel(dir, 'gsd-executor')); + }); + + // Boundary on the escalation cap: limit-1 / limit / limit+1. + for (const [attempt, expected] of [[0, 'DR-STANDARD'], [1, 'opus'], [2, 'opus']]) { + test(`max_escalations:1 — attempt ${attempt} resolves ${expected}`, () => { + dir = project({ model_profile: 'quality', dynamic_routing: { ...DR, max_escalations: 1 } }); + assert.strictEqual(resolveExecutionModel(dir, 'gsd-executor', attempt), expected); + }); + } + + test('max_escalations:0 pins attempt 1 to the default tier', () => { + dir = project({ model_profile: 'quality', dynamic_routing: { ...DR, max_escalations: 0 } }); + assert.strictEqual(resolveExecutionModel(dir, 'gsd-executor', 1), 'DR-STANDARD'); + }); + }); + + describe('#4505 half A — the runtime-aware tier map must follow the ACTIVE runtime', () => { + let dir; + afterEach(() => { if (dir) cleanup(dir); dir = null; }); + + const OVERRIDES = { model_profile_overrides: { opencode: { sonnet: 'TEST-OPENCODE' } } }; + + // FAILING-FIRST. Measured on the unfixed tree: "sonnet" — the override is + // silently ignored because config.runtime is absent. + test('GSD_RUNTIME selects the runtime tier map (#4495)', () => { + dir = project(OVERRIDES); + assert.strictEqual(resolveModel(dir, 'gsd-phase-researcher', 'opencode'), 'TEST-OPENCODE'); + }); + + // CONTROL — identical override, runtime declared in the config instead. + // Passes on the unfixed tree, so it proves the override machinery works and + // the fixture is live; without it the row above could be failing for any + // reason at all. + test('CONTROL: the same override already works when runtime is written into the config', () => { + dir = project({ runtime: 'opencode', ...OVERRIDES }); + assert.strictEqual(resolveModel(dir, 'gsd-phase-researcher'), 'TEST-OPENCODE'); + }); + + // NEGATIVE SPACE: with no runtime declared anywhere the active runtime is + // 'claude', so step 3's deliberate claude skip (#1156/#2297/#4192) must + // still apply and the opencode map must NOT leak in. + test('no runtime declared anywhere still resolves claude-native, not opencode', () => { + dir = project(OVERRIDES); + assert.strictEqual(resolveModel(dir, 'gsd-phase-researcher'), 'sonnet'); + }); + + test('claude stays alias-native via its own tier override path', () => { + dir = project({ model_profile_overrides: { claude: { sonnet: 'TEST-CLAUDE' } } }); + assert.strictEqual(resolveModel(dir, 'gsd-phase-researcher', 'claude'), 'TEST-CLAUDE'); + }); + + // The VALUE policy follows the active runtime too. #4192 originally keyed + // this off `config.runtime` and wrote that it must stay that way; applying + // #4505's criterion to it turned out to SERVE #4192's own principle ("an + // explicit pin must not be silently unpinned") rather than fight it, and no + // test pinned the old reading. Without the fix the first assertion returns + // 'opus' — a Claude-only alias handed to a runtime that cannot spawn it. + test('an explicit claude pin is not unpinned just because the runtime came from env', () => { + dir = project({ model_overrides: { 'gsd-phase-researcher': 'claude-opus-4-8' } }); + assert.strictEqual(resolveModel(dir, 'gsd-phase-researcher', 'opencode'), 'claude-opus-4-8'); + }); + + test('the same pin still collapses to its alias when claude IS the active runtime', () => { + dir = project({ model_overrides: { 'gsd-phase-researcher': 'claude-opus-4-8' } }); + assert.strictEqual(resolveModel(dir, 'gsd-phase-researcher', 'claude'), 'opus'); + }); + }); + + // Making step 3 reachable for env/marker-declared runtimes newly exposes an + // ordering that two shipped fixes had each decided in isolation, and which + // only coexisted because step 3 could not fire without `config.runtime`. + // Neither #4505 nor #4495/#4493 mentions it. These rows pin both halves so a + // future edit cannot quietly pick one and drop the other. + describe('#4505 — runtime tier map vs the resolve_model_ids:"omit" gate', () => { + let dir; + afterEach(() => { if (dir) cleanup(dir); dir = null; }); + + test('#2517: an explicit runtime opt-in in the config outranks an omit', () => { + dir = project({ runtime: 'codex', resolve_model_ids: 'omit' }); + // The concrete codex tier model, not merely "not empty" — `notStrictEqual('')` + // would also pass on `opus`, i.e. on the omit gate being skipped for the + // wrong reason. + assert.strictEqual(resolveModel(dir, 'gsd-executor'), 'gpt-5.6-terra'); + }); + + // The omit lives in the SHARED ~/.gsd/defaults.json, not the project config — + // that is the "poisoned global" #2297 acceptance #4 is actually about. An + // earlier cut of this row wrote it into the project config, which is the + // #2517 project-explicit path and exercises a different branch entirely. + test('#2297 acceptance #4: a merely DETECTED runtime still honors a GLOBAL omit', () => { + // No project config at all — see the `project` docblock: that is the only + // shape in which the shared defaults layer is consulted. + dir = project(null, { resolve_model_ids: 'omit' }); + assert.strictEqual(resolveModel(dir, 'gsd-executor', 'codex'), ''); + }); + + test('a project-explicit omit is honored for a detected runtime too', () => { + dir = project({ resolve_model_ids: 'omit' }); + assert.strictEqual(resolveModel(dir, 'gsd-executor', 'codex'), ''); + }); + + // The opt-in signal is CANONICALIZED. An earlier cut of this fix compared the + // raw config field against the literal 'claude', so every other spelling of + // Claude — and any non-string — counted as a deliberate non-Claude opt-in and + // bought an escalation past an explicit omit. Failing OPEN, in the exact case + // the guard exists to protect. Each row below returned a model id before the + // canonicalization and returns '' on origin/next. + for (const [label, runtimeValue] of [ + ['a case variant', 'Claude'], + ['a known alias', 'claude-code'], + ['a non-string', 5], + ['an unrecognised runtime', 'not-a-real-runtime'], + ]) { + test(`${label} in the runtime key is NOT a non-Claude opt-in`, () => { + dir = project({ runtime: runtimeValue, resolve_model_ids: 'omit' }); + assert.strictEqual( + resolveModel(dir, 'gsd-executor', 'codex'), + '', + 'only a value that canonicalizes to a recognised non-Claude runtime may ' + + 'outrank the omit gate — anything else must fail safe', + ); + }); + } + }); +}