From 9ea5519bc05733640680d889ddb14d7b6418a03d Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 6 Jul 2026 19:33:43 -0400 Subject: [PATCH 1/5] test(#2041): add regression test for model_overrides claude alias mapping Mirrors the #1133 model_policy alias-mapping tests for the model_overrides path. Covers AC1-AC6: mappable Claude full IDs (claude-sonnet-5/opus-4-8/ haiku-4-5/fable-5) resolve to aliases on runtime:claude; bare aliases pass through; non-claude runtimes keep full IDs verbatim; unmappable Claude IDs warn-once + fall through; resolveModelForTier escalation path also maps; non-Claude custom/vendor values pass through verbatim (regression guards). Expected RED against unfixed model-resolver.cts (override short-circuit at lines 162-167 / 288-290 returns override verbatim with no alias mapping). --- tests/model-resolver.test.cjs | 158 ++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) diff --git a/tests/model-resolver.test.cjs b/tests/model-resolver.test.cjs index 9fc7fa30e..95c5addc0 100644 --- a/tests/model-resolver.test.cjs +++ b/tests/model-resolver.test.cjs @@ -3770,6 +3770,164 @@ describe('#49 resolveModelPolicy: prototype-pollution guards', () => { }); }); +// ─── #2041: model_overrides Claude full ID → Agent-tool alias on claude runtime ─ +// +// Mirrors the #1133 model_policy alias-mapping tests (above) for the +// model_overrides path. Bug: a full Claude model ID in model_overrides +// (e.g. "claude-sonnet-5") was returned VERBATIM on the claude runtime and +// handed to the Claude Agent tool, whose typed `model` parameter documents only +// tier aliases (opus/sonnet/haiku/fable). The model_policy path already maps +// full IDs → aliases via CLAUDE_POLICY_ID_TO_ALIAS (#1144); model_overrides +// skipped that mapping entirely. The fix mirrors #1144 on the override path. +// Non-Claude runtimes and non-Claude values pass through verbatim (parity). + +describe('#2041 model_overrides: Claude full ID → alias on claude runtime', () => { + let tmpDir; + beforeEach(() => { + tmpDir = makeTmp('2041'); + resetRuntimeWarningCaches(); + }); + afterEach(() => { + rmr(tmpDir); + resetRuntimeWarningCaches(); + }); + + // AC1 + AC2: mappable Claude full IDs resolve to their aliases on claude runtime + test('model_overrides claude-sonnet-5 → "sonnet" on runtime:claude (resolveModelInternal)', () => { + writeConfig(tmpDir, { + runtime: 'claude', + model_overrides: { 'gsd-executor': 'claude-sonnet-5' }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-executor'), 'sonnet'); + }); + + test('model_overrides claude-opus-4-8 → "opus" on runtime:claude', () => { + writeConfig(tmpDir, { + runtime: 'claude', + model_overrides: { 'gsd-planner': 'claude-opus-4-8' }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'opus'); + }); + + test('model_overrides claude-haiku-4-5 → "haiku" on runtime:claude', () => { + writeConfig(tmpDir, { + runtime: 'claude', + model_overrides: { 'gsd-codebase-mapper': 'claude-haiku-4-5' }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-codebase-mapper'), 'haiku'); + }); + + test('model_overrides claude-fable-5 → "fable" on runtime:claude', () => { + writeConfig(tmpDir, { + runtime: 'claude', + model_overrides: { 'gsd-planner': 'claude-fable-5' }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'fable'); + }); + + // AC3: bare aliases pass through verbatim + test('model_overrides bare "sonnet" alias passes through verbatim on runtime:claude', () => { + writeConfig(tmpDir, { + runtime: 'claude', + model_overrides: { 'gsd-executor': 'sonnet' }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-executor'), 'sonnet'); + }); + + test('model_overrides bare "fable" alias passes through verbatim on runtime:claude', () => { + writeConfig(tmpDir, { + runtime: 'claude', + model_overrides: { 'gsd-planner': 'fable' }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'fable'); + }); + + // AC1 (implicit claude): mapping fires when runtime key is absent (defaults to claude) + test('model_overrides claude-sonnet-5 → "sonnet" with implicit claude runtime (no runtime key)', () => { + writeConfig(tmpDir, { + model_overrides: { 'gsd-executor': 'claude-sonnet-5' }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-executor'), 'sonnet'); + }); + + // AC4: non-claude runtimes keep full IDs verbatim (parity with model_policy path) + test('model_overrides claude-sonnet-5 → verbatim ID on non-claude runtime (opencode)', () => { + writeConfig(tmpDir, { + runtime: 'opencode', + model_overrides: { 'gsd-executor': 'claude-sonnet-5' }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-executor'), 'claude-sonnet-5'); + }); + + // AC5: unmappable Claude full ID warns once + falls through to tier alias + test('model_overrides unmappable claude ID (claude-opus-4-5) falls through to tier alias on claude', () => { + resetRuntimeWarningCaches(); + writeConfig(tmpDir, { + runtime: 'claude', + model_profile: 'balanced', + model_overrides: { 'gsd-planner': 'claude-opus-4-5' }, + }); + // gsd-planner balanced → opus tier; claude-opus-4-5 has no alias → warn + fall through → 'opus' + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'opus'); + }); + + test('model_overrides unmappable claude ID emits a stderr warning exactly once (dedupe)', () => { + resetRuntimeWarningCaches(); + writeConfig(tmpDir, { + runtime: 'claude', + model_profile: 'balanced', + model_overrides: { 'gsd-planner': 'claude-opus-4-5' }, + }); + const writes = []; + const original = process.stderr.write.bind(process.stderr); + process.stderr.write = (chunk) => { writes.push(String(chunk)); return true; }; + try { + resolveModelInternal(tmpDir, 'gsd-planner'); + resolveModelInternal(tmpDir, 'gsd-planner'); // second call — dedupe must suppress + } finally { + process.stderr.write = original; + } + const warnings = writes.filter((w) => w.includes('model_overrides') && w.includes('claude-opus-4-5')); + assert.strictEqual(warnings.length, 1, + `expected exactly one override warning, got ${warnings.length}: ${JSON.stringify(writes)}`); + }); + + // AC6: resolveModelForTier (escalation / --attempt path) maps the same way + test('resolveModelForTier maps claude-sonnet-5 → "sonnet" on runtime:claude', () => { + writeConfig(tmpDir, { + runtime: 'claude', + model_overrides: { 'gsd-executor': 'claude-sonnet-5' }, + }); + assert.strictEqual(resolveModelForTier(tmpDir, 'gsd-executor', 0), 'sonnet'); + }); + + test('resolveModelForTier keeps full ID verbatim on non-claude runtime', () => { + writeConfig(tmpDir, { + runtime: 'opencode', + model_overrides: { 'gsd-executor': 'claude-sonnet-5' }, + }); + assert.strictEqual(resolveModelForTier(tmpDir, 'gsd-executor', 0), 'claude-sonnet-5'); + }); + + // Regression guard: non-Claude custom / vendor values still pass through verbatim + // on the claude runtime (the fix must NOT touch values that aren't Claude IDs). + test('model_overrides non-Claude custom model passes through verbatim on runtime:claude', () => { + writeConfig(tmpDir, { + runtime: 'claude', + model_overrides: { 'gsd-planner': 'my-custom-model' }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-planner'), 'my-custom-model'); + }); + + test('model_overrides non-Claude vendor ID (openai/gpt-5) passes through verbatim on runtime:claude', () => { + writeConfig(tmpDir, { + runtime: 'claude', + model_overrides: { 'gsd-executor': 'openai/gpt-5' }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-executor'), 'openai/gpt-5'); + }); +}); + // ─── resolveModelForTier: model_policy beats dynamic_routing ───────────────── describe('#49 resolveModelForTier: model_policy beats dynamic_routing', () => { From f214f1320d61296f9bbb8530112646ee3687a1de Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 6 Jul 2026 20:06:30 -0400 Subject: [PATCH 2/5] fix(#2041): map model_overrides full claude IDs to agent-tool aliases model_overrides values that are full Claude model IDs (claude-sonnet-5, claude-opus-4-8, claude-haiku-4-5, claude-fable-5) were returned verbatim on the claude runtime and handed to the Claude Agent tool, whose typed model parameter documents only tier aliases (opus/sonnet/haiku/fable). The model_policy path already mapped full IDs -> aliases via CLAUDE_POLICY_ID_TO_ALIAS (#1144); model_overrides skipped that mapping, so the two resolver paths produced different shapes for the same underlying Claude model. The fix mirrors #1144 on the override path via a shared mapClaudeOverrideForRuntime helper used by both resolveModelInternal and resolveModelForTier. Bare aliases pass through verbatim; non-Claude runtimes and non-Claude custom/vendor values keep full IDs verbatim (parity). An unmappable Claude ID (e.g. claude-opus-4-5) warns once to stderr and falls through to tier resolution, exactly as the model_policy path already does. Alias mapping is also the documented best practice (prevents staleness when new model versions ship). --- src/model-resolver.cts | 61 +++++++++++++++++++++++++++++++++++++++--- tests/helpers.cjs | 1 + 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/src/model-resolver.cts b/src/model-resolver.cts index 2fea5d5e1..2820b3510 100644 --- a/src/model-resolver.cts +++ b/src/model-resolver.cts @@ -105,6 +105,51 @@ function _resetModelPolicyWarningCacheForTests(): void { _modelPolicyUnmappableWarned.clear(); } +// Dedupe stderr warnings for unmappable model_overrides Claude IDs (#2041). +const _modelOverrideUnmappableWarned = new Set(); +function warnModelOverrideUnmappable(agentType: string, overrideValue: string): void { + const key = `${agentType}::${overrideValue}`; + if (_modelOverrideUnmappableWarned.has(key)) return; + _modelOverrideUnmappableWarned.add(key); + // MUST go to stderr — resolve-model's JSON result is parsed from stdout. + process.stderr.write( + `gsd: warning — model_overrides value "${overrideValue}" for ${agentType} ` + + `has no Claude agent alias; falling through to tier resolution.\n`, + ); +} + +// Test-only: reset the model_overrides warn-dedupe cache between cases (#2041). +function _resetModelOverrideWarningCacheForTests(): void { + _modelOverrideUnmappableWarned.clear(); +} + +/** + * #2041 — Map a `model_overrides` value to its Claude Agent-tool alias on the + * claude runtime, mirroring the `model_policy` path (#1144). Claude Code's + * Agent tool `model` parameter documents only tier aliases (opus/sonnet/haiku/ + * fable); a full Claude model ID returned verbatim is silently dropped by the + * spawner. Returns the value to return verbatim, or null to signal "fall + * through to normal tier/dynamic-routing resolution" (used when a Claude full + * ID has no alias — matches model_policy's warn-and-fall-through). Non-Claude + * runtimes and non-Claude values always pass through verbatim. + */ +function mapClaudeOverrideForRuntime( + override: string, + configRuntime: string | null | undefined, + agentType: string, +): string | null { + const onClaude = !configRuntime || configRuntime === 'claude'; + if (!onClaude) return override; + const alias = CLAUDE_POLICY_ID_TO_ALIAS[override]; + if (alias) return alias; + if (CLAUDE_AGENT_ALIASES.has(override)) return override; + if (override.startsWith('claude-')) { + warnModelOverrideUnmappable(agentType, override); + return null; + } + return override; +} + /** * #49 — Provider-neutral model policy preset resolution. */ @@ -159,11 +204,15 @@ function resolveModelPolicy(policy: Record | null | undefined, function resolveModelInternal(cwd: string, agentType: string): string { const config = loadConfig(cwd); - // 1. Per-agent override + // 1. Per-agent override (#2041: map Claude full IDs → Agent-tool aliases on + // the claude runtime, mirroring the model_policy path #1144; non-Claude + // runtimes and non-Claude values pass through verbatim). const modelOverrides = config['model_overrides'] as Record | null | undefined; const override = modelOverrides?.[agentType]; if (override) { - return override; + const mapped = mapClaudeOverrideForRuntime(override, config['runtime'] as string | null | undefined, agentType); + if (mapped !== null) return mapped; + // Unmappable Claude ID — fall through to tier resolution (matches model_policy). } // 2. Compute the tier @@ -287,7 +336,11 @@ function resolveModelForTier(cwd: string, agentType: string, attempt?: number): const modelOverrides = config['model_overrides'] as Record | null | undefined; const override = modelOverrides?.[agentType]; - if (override) return override; + if (override) { + const mapped = mapClaudeOverrideForRuntime(override, config['runtime'] as string | null | undefined, 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') { return resolveModelInternal(cwd, agentType); @@ -508,6 +561,8 @@ export = { resolveModelPolicy, resolveModelInternal, _resetModelPolicyWarningCacheForTests, + _resetModelOverrideWarningCacheForTests, + mapClaudeOverrideForRuntime, VALID_GRANULARITIES, resolveGranularityInternal, assertValidGranularityOverride, diff --git a/tests/helpers.cjs b/tests/helpers.cjs index 3538a26a5..234bc8f99 100644 --- a/tests/helpers.cjs +++ b/tests/helpers.cjs @@ -423,6 +423,7 @@ function resetRuntimeWarningCaches() { const modelResolver = require('../gsd-core/bin/lib/model-resolver.cjs'); configLoader._resetRuntimeWarningCacheForTests(); modelResolver._resetModelPolicyWarningCacheForTests(); + modelResolver._resetModelOverrideWarningCacheForTests(); } module.exports = { runGsdTools, createTempDir, createTempProject, createTempGitProject, cleanup, parseFrontmatter, isUsageOutput, captureConsole, toPosixPath, runNpm, isolatedNpmEnv, withIsolatedProcessState, delay, waitFor, resetRuntimeWarningCaches, TOOLS_PATH }; From 13e40fcbf748205b51dafd508b3834dc97d66138 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 6 Jul 2026 20:07:54 -0400 Subject: [PATCH 3/5] docs(#2041): add changeset fragment for model_overrides alias fix --- .changeset/gentle-badgers-roar.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/gentle-badgers-roar.md diff --git a/.changeset/gentle-badgers-roar.md b/.changeset/gentle-badgers-roar.md new file mode 100644 index 000000000..f92ea1e0a --- /dev/null +++ b/.changeset/gentle-badgers-roar.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 0 +--- +**`model_overrides` Claude model IDs now resolve to Agent-tool aliases on the claude runtime** — a full Claude model ID (e.g. `claude-sonnet-5`) in `model_overrides` was returned verbatim and silently dropped by the Claude Agent tool (whose `model` parameter documents only tier aliases), causing the spawned subagent to inherit the parent session model instead of the configured one. It now maps to the tier alias (`sonnet`/`opus`/`haiku`/`fable`), consistent with the `model_policy` path (#1144). Bare aliases, non-Claude values, and non-Claude runtimes are unchanged; a Claude ID with no alias warns once and falls through to tier resolution. (#2041) From e95af39a8c5552b2bd87550c9d752d4cb40e8a5b Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 6 Jul 2026 20:16:30 -0400 Subject: [PATCH 4/5] fix(#2041): address code+security review findings - add typeof guard so a non-string override passes through verbatim instead of crashing on .startsWith (preserves pre-fix no-crash behaviour) [LOW-1] - use Object.hasOwn() for the alias lookup so __proto__/constructor cannot return a truthy non-string from the plain object literal [LOW-D3] - cap the unmappable-override stderr warning at 64 chars so an oversized or secret-shaped value cannot leak in full to stderr/logs [LOW-D4] - remove the unused mapClaudeOverrideForRuntime export (helpers are covered behaviourally via resolveModelInternal/resolveModelForTier) [NIT] - add resolveModelForTier unmappable-override fall-through test (closes the mutation-score gap) [MEDIUM-1] - add case-sensitivity contract test (Claude-Sonnet-5 passes through verbatim) [LOW-2] Both orthogonal reviews returned APPROVE with no Critical/High findings. --- src/model-resolver.cts | 24 +++++++++++++++++++----- tests/model-resolver.test.cjs | 26 ++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/model-resolver.cts b/src/model-resolver.cts index 2820b3510..dc1bc31f1 100644 --- a/src/model-resolver.cts +++ b/src/model-resolver.cts @@ -111,9 +111,12 @@ function warnModelOverrideUnmappable(agentType: string, overrideValue: string): const key = `${agentType}::${overrideValue}`; if (_modelOverrideUnmappableWarned.has(key)) return; _modelOverrideUnmappableWarned.add(key); - // MUST go to stderr — resolve-model's JSON result is parsed from stdout. + // Cap emission length so an oversized or secret-shaped value cannot leak in + // full to stderr/logs (#2041 security review). MUST go to stderr — resolve- + // model's JSON result is parsed from stdout. + const safe = overrideValue.length > 64 ? overrideValue.slice(0, 64) + '…' : overrideValue; process.stderr.write( - `gsd: warning — model_overrides value "${overrideValue}" for ${agentType} ` + + `gsd: warning — model_overrides value "${safe}" for ${agentType} ` + `has no Claude agent alias; falling through to tier resolution.\n`, ); } @@ -132,16 +135,28 @@ function _resetModelOverrideWarningCacheForTests(): void { * through to normal tier/dynamic-routing resolution" (used when a Claude full * ID has no alias — matches model_policy's warn-and-fall-through). Non-Claude * runtimes and non-Claude values always pass through verbatim. + * + * Hardening (code+security review): a `typeof` guard preserves the pre-fix + * no-crash behavior if a malformed config surfaces a non-string value, and an + * `Object.hasOwn` lookup defeats `__proto__`/`constructor` lookups on the plain + * object literal so those reserved keys cannot return a truthy non-string. */ function mapClaudeOverrideForRuntime( override: string, configRuntime: 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'; if (!onClaude) return override; - const alias = CLAUDE_POLICY_ID_TO_ALIAS[override]; - if (alias) return alias; + // Object.hasOwn guards against __proto__/constructor returning a truthy + // non-string from the plain object literal (#2041 security review). + if (Object.hasOwn(CLAUDE_POLICY_ID_TO_ALIAS, override)) { + return CLAUDE_POLICY_ID_TO_ALIAS[override]; + } if (CLAUDE_AGENT_ALIASES.has(override)) return override; if (override.startsWith('claude-')) { warnModelOverrideUnmappable(agentType, override); @@ -562,7 +577,6 @@ export = { resolveModelInternal, _resetModelPolicyWarningCacheForTests, _resetModelOverrideWarningCacheForTests, - mapClaudeOverrideForRuntime, VALID_GRANULARITIES, resolveGranularityInternal, assertValidGranularityOverride, diff --git a/tests/model-resolver.test.cjs b/tests/model-resolver.test.cjs index 95c5addc0..3e2d06ddf 100644 --- a/tests/model-resolver.test.cjs +++ b/tests/model-resolver.test.cjs @@ -3909,6 +3909,32 @@ describe('#2041 model_overrides: Claude full ID → alias on claude runtime', () assert.strictEqual(resolveModelForTier(tmpDir, 'gsd-executor', 0), 'claude-sonnet-5'); }); + // MEDIUM-1 (review): exercise the unmappable-override fall-through branch in + // resolveModelForTier (closes the mutation-score gap — a future refactor that + // accidentally returned the verbatim override instead of falling through + // would otherwise survive the suite). + test('resolveModelForTier unmappable claude ID falls through to tier alias on claude', () => { + resetRuntimeWarningCaches(); + writeConfig(tmpDir, { + runtime: 'claude', + model_profile: 'balanced', + model_overrides: { 'gsd-planner': 'claude-opus-4-5' }, + }); + // unmappable override → fall through → no dynamic_routing → resolveModelInternal → 'opus' + assert.strictEqual(resolveModelForTier(tmpDir, 'gsd-planner', 0), 'opus'); + }); + + // LOW-2 (review): pin the case-sensitive contract — a case-variant like + // "Claude-Sonnet-5" is NOT mapped (alias keys are case-sensitive, matching + // the model_policy path and the Claude API). + test('model_overrides case-variant "Claude-Sonnet-5" passes through verbatim (case-sensitive contract)', () => { + writeConfig(tmpDir, { + runtime: 'claude', + model_overrides: { 'gsd-executor': 'Claude-Sonnet-5' }, + }); + assert.strictEqual(resolveModelInternal(tmpDir, 'gsd-executor'), 'Claude-Sonnet-5'); + }); + // Regression guard: non-Claude custom / vendor values still pass through verbatim // on the claude runtime (the fix must NOT touch values that aren't Claude IDs). test('model_overrides non-Claude custom model passes through verbatim on runtime:claude', () => { From 6136aa20de2233916916b0765d09403bde9e4d65 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 6 Jul 2026 20:59:38 -0400 Subject: [PATCH 5/5] docs(#2041): backfill changeset pr number to 2048 --- .changeset/gentle-badgers-roar.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/gentle-badgers-roar.md b/.changeset/gentle-badgers-roar.md index f92ea1e0a..a20eb5765 100644 --- a/.changeset/gentle-badgers-roar.md +++ b/.changeset/gentle-badgers-roar.md @@ -1,5 +1,5 @@ --- type: Fixed -pr: 0 +pr: 2048 --- **`model_overrides` Claude model IDs now resolve to Agent-tool aliases on the claude runtime** — a full Claude model ID (e.g. `claude-sonnet-5`) in `model_overrides` was returned verbatim and silently dropped by the Claude Agent tool (whose `model` parameter documents only tier aliases), causing the spawned subagent to inherit the parent session model instead of the configured one. It now maps to the tier alias (`sonnet`/`opus`/`haiku`/`fable`), consistent with the `model_policy` path (#1144). Bare aliases, non-Claude values, and non-Claude runtimes are unchanged; a Claude ID with no alias warns once and falls through to tier resolution. (#2041)