* fix(#4192): honor explicit model pins on the claude runtime Two documented model-configuration contracts did not hold on the claude runtime (confirmed-bug scope from the issue triage): Finding 1 — model_profile_overrides.claude.<tier> was inert. Step 3 of resolveModelInternal gated runtime-aware tier resolution on configRuntime !== 'claude', so the key's only reader was never consulted, while workflows/settings-advanced.md writes it for claude-runtime users. A new step 4.5 resolves ONLY the user's override entry (never the builtin claude tier map, so unpinned installs keep resolving aliases). An override value that maps to a current tier alias collapses to that alias (byte-equivalent, the #2041 protection); anything else — a pinned older generation, a bare alias repoint, a non-Anthropic id — resolves verbatim. It sits after the resolve_model_ids:'omit' gate so an explicit project omit still wins (#2297) and before the alias return so resolve_model_ids:true cannot re-materialize the pin to the latest id. Finding 2 — fully-qualified claude-* ids in model_overrides were warn-dropped to tier resolution (mapClaudeOverrideForRuntime unmappable branch, #2041), while the docs promise any fully-qualified model id is valid. The unmappable branch now passes the pin through verbatim with a warn-once breadcrumb (text describes the pass-through). Dropping it silently unpinned the operator's explicit choice — the exact 'profile can misrepresent what actually runs' defect of #4192. Mappable ids and non-claude values behave exactly as before; resolveModelForTier shares the mapping; the tier honesty signal is unchanged (raw ids still report 'unknown'); the model_policy path is untouched. Docs updated to the agreed contract (CONFIGURATION.md false 'Claude example' corrected; how-to + shipped reference document the pin semantics, the fable alias, and the tier-override composition). * test(#4192): pin explicit model pin resolution on the claude runtime 28 failing-first rows across the resolver seam and the resolve-model CLI: pinned-generation fidelity (tier override + per-agent verbatim pins, object form, explicit runtime), unpinned controls byte-stable (no override, other runtime/tier, inherit, project omit, precedence), adversarial rows (prototype-chain keys, malformed values, warn-once dedupe, 64-char stderr cap), and behavioral AC1/AC2 rows through runGsdTools. The stale #2041 fall-through assertions now pin the pass-through contract; mappable-id collapse assertions unchanged. * chore(#4192): add changeset fragment * chore(#4192): backfill PR number in changeset fragment --------- Co-authored-by: ZCode <zcode@localhost>
This commit is contained in:
@@ -157,6 +157,65 @@ function _resolveRuntimeTier(config: Record<string, unknown>, tier: string): Tie
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* #4192 — Resolve the claude-runtime TIER OVERRIDE model for (config, tier).
|
||||
*
|
||||
* Step 3's runtime-aware resolution deliberately skips the claude runtime to
|
||||
* preserve the alias-native posture (#1156/#2297): with no user override, the
|
||||
* resolver must keep returning bare tier aliases, and the builtin claude tier
|
||||
* map (`opus → claude-opus-4-8`, …) must never force full-ID emission on every
|
||||
* default install. But `model_profile_overrides.<runtime>.<tier>` is a
|
||||
* documented override point (docs/CONFIGURATION.md § Runtime-Aware Profiles)
|
||||
* that `workflows/settings-advanced.md` actively writes for claude-runtime
|
||||
* users — and #4192 Finding 1 measured the key inert on this runtime.
|
||||
*
|
||||
* 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.
|
||||
*
|
||||
* Value policy mirrors the model_overrides path (#2041/#4192): an override
|
||||
* value that maps to a current tier alias collapses to that alias
|
||||
* (byte-equivalent resolution, alias-form emission); anything else — a pinned
|
||||
* older generation (`claude-opus-4-7`), a bare alias/tier repoint (`sonnet`),
|
||||
* or a non-Claude vendor id (`openai/o3`) — is emitted verbatim as pinned.
|
||||
* Malformed entries (no usable `model` string) return null so the caller falls
|
||||
* through to normal alias resolution (ADR-443 D1: invalid values fall through).
|
||||
*/
|
||||
function resolveClaudeTierOverrideModel(
|
||||
configRuntime: string | null | undefined,
|
||||
tier: string | null | undefined,
|
||||
overrides: Record<string, unknown> | null | undefined,
|
||||
): string | null {
|
||||
if (!tier || tier === 'inherit') return null;
|
||||
const effectiveRuntime = configRuntime || 'claude';
|
||||
if (effectiveRuntime !== 'claude') return null; // non-claude runtimes resolve at step 3
|
||||
const overridesMap = overrides as Record<string, Record<string, unknown>> | null | undefined;
|
||||
if (!overridesMap || typeof overridesMap !== 'object') return null;
|
||||
// Own-property guards throughout: both levels are config-supplied plain
|
||||
// objects, so a prototype-chain key ("constructor", "toString") must not
|
||||
// resolve an inherited member instead of falling through (same hardening as
|
||||
// every other config-keyed lookup in this module).
|
||||
const runtimeEntry = Object.hasOwn(overridesMap, effectiveRuntime)
|
||||
? overridesMap[effectiveRuntime]
|
||||
: undefined;
|
||||
if (!runtimeEntry || typeof runtimeEntry !== 'object') return null;
|
||||
const userRaw = Object.hasOwn(runtimeEntry, tier) ? runtimeEntry[tier] : undefined;
|
||||
if (userRaw === undefined || userRaw === null) return null;
|
||||
const entry: Record<string, unknown> = typeof userRaw === 'string'
|
||||
? { model: userRaw }
|
||||
: (userRaw as Record<string, unknown>);
|
||||
if (!entry || typeof entry !== 'object') return null;
|
||||
const model = entry['model'];
|
||||
if (typeof model !== 'string' || model.length === 0) return null;
|
||||
if (Object.hasOwn(CLAUDE_POLICY_ID_TO_ALIAS, model)) {
|
||||
return CLAUDE_POLICY_ID_TO_ALIAS[model];
|
||||
}
|
||||
return model;
|
||||
}
|
||||
|
||||
// Reverse of the Claude tier-default IDs, plus the Fable alias which Claude
|
||||
// Code's Agent tool accepts but which is not a GSD model-profile tier (#1133).
|
||||
const CLAUDE_POLICY_ID_TO_ALIAS: Record<string, string> = {
|
||||
@@ -189,7 +248,10 @@ function _resetModelPolicyWarningCacheForTests(): void {
|
||||
_modelPolicyUnmappableWarned.clear();
|
||||
}
|
||||
|
||||
// Dedupe stderr warnings for unmappable model_overrides Claude IDs (#2041).
|
||||
// Dedupe stderr warnings for unmappable model_overrides Claude IDs (#2041 /
|
||||
// #4192). #2041 originally warned that such a value was being DROPPED to tier
|
||||
// resolution; #4192 keeps the warn-once breadcrumb but changes the behavior to
|
||||
// a verbatim pass-through, so the text now describes the pass-through.
|
||||
const _modelOverrideUnmappableWarned = new Set<string>();
|
||||
function warnModelOverrideUnmappable(agentType: string, overrideValue: string): void {
|
||||
const key = `${agentType}::${overrideValue}`;
|
||||
@@ -200,8 +262,9 @@ function warnModelOverrideUnmappable(agentType: string, overrideValue: string):
|
||||
// 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 "${safe}" for ${agentType} ` +
|
||||
`has no Claude agent alias; falling through to tier resolution.\n`,
|
||||
`gsd: warning — model_overrides value "${safe}" for ${agentType} is a fully-qualified ` +
|
||||
`Claude model ID with no tier alias; passing it through verbatim. Claude Code setups ` +
|
||||
`whose Agent tool accepts only tier aliases will not honor it. (#4192)\n`,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -213,12 +276,24 @@ function _resetModelOverrideWarningCacheForTests(): void {
|
||||
/**
|
||||
* #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.
|
||||
* Agent tool `model` parameter documents tier aliases (opus/sonnet/haiku/
|
||||
* fable) as the always-accepted form. Returns the value to emit verbatim, or
|
||||
* null to signal "fall through to normal tier/dynamic-routing resolution".
|
||||
* Non-Claude runtimes and non-Claude values always pass through verbatim.
|
||||
*
|
||||
* #4192 — an unmappable `claude-*` value (a pinned generation that is not the
|
||||
* current catalog default, e.g. `claude-opus-4-7`) is now PASSED THROUGH
|
||||
* VERBATIM with a warn-once stderr breadcrumb, instead of being dropped to
|
||||
* tier resolution. #2041's drop was correct when the value was plausibly a
|
||||
* mis-typed current default, but for an explicit pin it silently UNPINNED the
|
||||
* operator's choice — the resolver would report a tier the config never asked
|
||||
* for, the exact "profile can misrepresent what actually runs" defect #4192
|
||||
* files. The documented contract ("any fully-qualified model ID",
|
||||
* docs/CONFIGURATION.md § Per-Agent Overrides,
|
||||
* gsd-core/references/model-profiles.md § Per-Agent Overrides) is restored:
|
||||
* the pin is resolved as configured. Values that DO map to a current tier
|
||||
* alias still collapse to that alias — byte-equivalent resolution, the #2041
|
||||
* protection preserved — and a mappable pin never warns.
|
||||
*
|
||||
* 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
|
||||
@@ -243,8 +318,9 @@ function mapClaudeOverrideForRuntime(
|
||||
}
|
||||
if (CLAUDE_AGENT_ALIASES.has(override)) return override;
|
||||
if (override.startsWith('claude-')) {
|
||||
// #4192: explicit generation pin — resolve as configured (see docblock).
|
||||
warnModelOverrideUnmappable(agentType, override);
|
||||
return null;
|
||||
return override;
|
||||
}
|
||||
return override;
|
||||
}
|
||||
@@ -507,6 +583,23 @@ function resolveModelInternal(cwd: string, agentType: string): string {
|
||||
return '';
|
||||
}
|
||||
|
||||
// 4.5. Claude-runtime tier override (#4192 Finding 1). Sits AFTER the omit
|
||||
// gate so an explicit project `resolve_model_ids:"omit"` still wins (#2297:
|
||||
// explicit project omit is honored regardless of runtime), and BEFORE the
|
||||
// alias return so a pinned generation is not re-collapsed to a tier alias or
|
||||
// re-materialized to the LATEST catalog id by step 5's
|
||||
// `resolve_model_ids:true` path. Fires ONLY when the user wrote a
|
||||
// `model_profile_overrides.claude.<tier>` entry for this tier — see
|
||||
// resolveClaudeTierOverrideModel for why the builtin map stays out.
|
||||
if (tier && tier !== 'inherit') {
|
||||
const claudeOverrideModel = resolveClaudeTierOverrideModel(
|
||||
configRuntime,
|
||||
tier,
|
||||
config['model_profile_overrides'] as Record<string, unknown> | null | undefined,
|
||||
);
|
||||
if (claudeOverrideModel !== null) return claudeOverrideModel;
|
||||
}
|
||||
|
||||
// 5. Profile lookup (Claude-native default).
|
||||
if (!agentModels) {
|
||||
return profile === 'quality' ? 'opus'
|
||||
|
||||
Reference in New Issue
Block a user