* fix(#1569): preserve explicit resolve_model_ids in non-Claude installs The non-Claude finishInstall step keyed its resolve_model_ids:"omit" write on !== "omit", so an explicit true opt-in (resolveModelInternal returns full model IDs) was silently clobbered on every install/upgrade across all 14 non-Claude runtimes, making generated agent manifests inherit the active chat model instead of pinning the resolved model. Now only absent/falsy is defaulted to "omit"; an explicit true (and an existing "omit") is preserved. Regression test parameterizes across codex/opencode/gemini and covers the absent/false/idempotent/ claude/malformed boundaries. * chore(#1569): backfill changeset pr ref to 1653 * fix(#1569): default non-canonical resolve_model_ids values to omit (codex review) Adversarial review (codex, gpt-5.5/high) flagged that the original allowlist-by- enumeration condition (undefined/null/false -> omit) preserved malformed values (0, "", "yes", {}) instead of defaulting them to omit, letting them leak Claude aliases a non-Claude runtime cannot resolve. Switch to an allowlist condition (existing !== true && existing !== 'omit') so only an explicit canonical true opt-in and an existing omit are preserved; everything else defaults to the safe non-Claude omit. Adds a parameterized test over [0, "", "yes", {}].
This commit is contained in:
5
.changeset/sharp-eagles-wake.md
Normal file
5
.changeset/sharp-eagles-wake.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 1653
|
||||
---
|
||||
**Non-Claude installs no longer rewrite an explicit `resolve_model_ids: true` to "omit"** — Codex, OpenCode, Gemini, and the other non-Claude runtimes were silently clobbering the deliberate opt-in to full materialized model IDs on every install/upgrade, so generated agent manifests inherited the active chat model instead of pinning the resolved model. The finish step now only defaults `resolve_model_ids` to "omit" when it is absent or falsy; an explicit `true` is preserved. (#1569)
|
||||
@@ -11307,10 +11307,14 @@ function finishInstall(settingsPath, settings, statuslineCommand, shouldInstallS
|
||||
configureKiloPermissions(isGlobal, configDir);
|
||||
}
|
||||
|
||||
// For non-Claude runtimes, set resolve_model_ids: "omit" in ~/.gsd/defaults.json
|
||||
// so resolveModelInternal() returns '' instead of Claude aliases (opus/sonnet/haiku)
|
||||
// that the runtime can't resolve. Users can still use model_overrides for explicit IDs.
|
||||
// See #1156. Guard matches the #130-class pattern on configureOpencodePermissions above.
|
||||
// For non-Claude runtimes, DEFAULT resolve_model_ids to "omit" in ~/.gsd/defaults.json
|
||||
// when it is absent or falsy, so resolveModelInternal() returns '' instead of Claude
|
||||
// aliases (opus/sonnet/haiku) the runtime can't resolve. An explicit `true` opt-in
|
||||
// (resolveModelInternal returns full materialized model IDs) MUST be preserved —
|
||||
// rewriting it to "omit" would make generated agent manifests inherit the active
|
||||
// chat model instead of pinning the resolved model. See #1156 (default-to-omit
|
||||
// intent) and #1569 (preserve explicit true). Guard matches the #130-class pattern
|
||||
// on configureOpencodePermissions above.
|
||||
if (runtime !== 'claude' && !process.env.GSD_TEST_MODE) {
|
||||
const gsdDir = path.join(os.homedir(), '.gsd');
|
||||
const defaultsPath = path.join(gsdDir, 'defaults.json');
|
||||
@@ -11318,7 +11322,14 @@ function finishInstall(settingsPath, settings, statuslineCommand, shouldInstallS
|
||||
fs.mkdirSync(gsdDir, { recursive: true });
|
||||
let defaults = {};
|
||||
try { defaults = JSON.parse(fs.readFileSync(defaultsPath, 'utf8')); } catch { /* new file */ }
|
||||
if (defaults.resolve_model_ids !== 'omit') {
|
||||
// Three-valued domain: false/absent → aliases; true → full IDs; "omit" → ''.
|
||||
// Honor ONLY an explicit canonical `true` opt-in (full model IDs) and an existing
|
||||
// "omit"; default everything else — absent, falsy, OR any non-canonical value — to
|
||||
// "omit", the safe non-Claude default. Allowlist-based so malformed values
|
||||
// (0, "", "yes", {}, …) don't leak Claude aliases the runtime can't resolve (#1569).
|
||||
const existing = defaults.resolve_model_ids;
|
||||
const shouldDefaultToOmit = existing !== true && existing !== 'omit';
|
||||
if (shouldDefaultToOmit) {
|
||||
defaults.resolve_model_ids = 'omit';
|
||||
fs.writeFileSync(defaultsPath, JSON.stringify(defaults, null, 2) + '\n');
|
||||
console.log(` ${green}✓${reset} Set resolve_model_ids: "omit" in ~/.gsd/defaults.json`);
|
||||
|
||||
@@ -115,3 +115,142 @@ describe('Bug #410: finishInstall non-Claude runtime + GSD_TEST_MODE side-effect
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// Bug #1569 folded here (sibling on the SAME finishInstall resolve_model_ids block):
|
||||
// the #1156 default-to-"omit" step keyed its write on `!== "omit"`, so an explicit
|
||||
// `resolve_model_ids: true` opt-in (resolveModelInternal returns full materialized
|
||||
// model IDs) was silently clobbered across all 14 non-Claude runtimes. The fix
|
||||
// preserves `true` and only defaults absent/falsy → "omit". Reuses the #410 harness.
|
||||
|
||||
describe('Bug #1569: non-Claude finishInstall preserves explicit resolve_model_ids:true', () => {
|
||||
function seedDefaults(obj) {
|
||||
fs.mkdirSync(GSD_DIR, { recursive: true });
|
||||
fs.writeFileSync(DEFAULTS_PATH, JSON.stringify(obj, null, 2) + '\n', 'utf8');
|
||||
}
|
||||
|
||||
function withUserPath(fn) {
|
||||
const saved = process.env.GSD_TEST_MODE;
|
||||
delete process.env.GSD_TEST_MODE;
|
||||
try {
|
||||
return fn();
|
||||
} finally {
|
||||
process.env.GSD_TEST_MODE = saved;
|
||||
}
|
||||
}
|
||||
|
||||
test('explicit resolve_model_ids:true survives a codex global install (the reported case)', () => {
|
||||
withUserPath(() => {
|
||||
seedDefaults({ runtime: 'codex', model_profile: 'balanced', resolve_model_ids: true });
|
||||
callFinishInstallForRuntime('codex');
|
||||
const after = JSON.parse(fs.readFileSync(DEFAULTS_PATH, 'utf8'));
|
||||
assert.equal(
|
||||
after.resolve_model_ids,
|
||||
true,
|
||||
'explicit resolve_model_ids:true must be preserved across a codex install, not clobbered to "omit"',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// The clobber guard is runtime-agnostic (`runtime !== 'claude'`); parameterize
|
||||
// across a representative slice of non-Claude runtimes.
|
||||
for (const runtime of ['codex', 'opencode', 'gemini']) {
|
||||
test(`explicit resolve_model_ids:true survives a ${runtime} global install`, () => {
|
||||
withUserPath(() => {
|
||||
seedDefaults({ runtime, resolve_model_ids: true });
|
||||
callFinishInstallForRuntime(runtime);
|
||||
const after = JSON.parse(fs.readFileSync(DEFAULTS_PATH, 'utf8'));
|
||||
assert.equal(
|
||||
after.resolve_model_ids,
|
||||
true,
|
||||
`explicit resolve_model_ids:true must be preserved for ${runtime}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
test('absent resolve_model_ids still defaults to "omit" (preserves #1156 intent)', () => {
|
||||
withUserPath(() => {
|
||||
seedDefaults({ runtime: 'codex' });
|
||||
callFinishInstallForRuntime('codex');
|
||||
const after = JSON.parse(fs.readFileSync(DEFAULTS_PATH, 'utf8'));
|
||||
assert.equal(
|
||||
after.resolve_model_ids,
|
||||
'omit',
|
||||
'absent resolve_model_ids must still default to "omit" for non-Claude runtimes',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
test('explicit resolve_model_ids:false still defaults to "omit"', () => {
|
||||
withUserPath(() => {
|
||||
seedDefaults({ runtime: 'codex', resolve_model_ids: false });
|
||||
callFinishInstallForRuntime('codex');
|
||||
const after = JSON.parse(fs.readFileSync(DEFAULTS_PATH, 'utf8'));
|
||||
assert.equal(after.resolve_model_ids, 'omit', 'false must still be normalized to "omit"');
|
||||
});
|
||||
});
|
||||
|
||||
test('non-canonical resolve_model_ids values (0, "", "yes", {}) default to "omit" — no Claude alias leak (#1569 codex review)', () => {
|
||||
// The domain is true/false/"omit"/absent. Any OTHER value is malformed; the safe
|
||||
// non-Claude default is "omit" (don't leak Claude aliases the runtime can't resolve).
|
||||
withUserPath(() => {
|
||||
for (const bad of [0, '', 'yes', {}]) {
|
||||
seedDefaults({ runtime: 'codex', resolve_model_ids: bad });
|
||||
callFinishInstallForRuntime('codex');
|
||||
const after = JSON.parse(fs.readFileSync(DEFAULTS_PATH, 'utf8'));
|
||||
assert.equal(
|
||||
after.resolve_model_ids,
|
||||
'omit',
|
||||
`non-canonical resolve_model_ids:${JSON.stringify(bad)} must default to "omit", not pass through`,
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
test('already-"omit" is left unchanged (idempotent, no rewrite churn)', () => {
|
||||
withUserPath(() => {
|
||||
seedDefaults({ runtime: 'codex', resolve_model_ids: 'omit' });
|
||||
const beforeMtime = fs.statSync(DEFAULTS_PATH).mtimeMs;
|
||||
// fs mtime resolution can be coarse; wait briefly so an accidental rewrite is detectable.
|
||||
const start = Date.now();
|
||||
while (Date.now() - start < 20) { /* spin briefly */ }
|
||||
callFinishInstallForRuntime('codex');
|
||||
const after = JSON.parse(fs.readFileSync(DEFAULTS_PATH, 'utf8'));
|
||||
const afterMtime = fs.statSync(DEFAULTS_PATH).mtimeMs;
|
||||
assert.equal(after.resolve_model_ids, 'omit');
|
||||
assert.equal(
|
||||
afterMtime,
|
||||
beforeMtime,
|
||||
'defaults.json must not be rewritten when resolve_model_ids is already "omit" (idempotent)',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
test('claude runtime never touches resolve_model_ids (cross-runtime parity)', () => {
|
||||
withUserPath(() => {
|
||||
seedDefaults({ runtime: 'claude', resolve_model_ids: true });
|
||||
callFinishInstallForRuntime('claude');
|
||||
const after = JSON.parse(fs.readFileSync(DEFAULTS_PATH, 'utf8'));
|
||||
assert.equal(
|
||||
after.resolve_model_ids,
|
||||
true,
|
||||
'claude install must never rewrite resolve_model_ids',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
test('malformed defaults.json does not crash — still defaults to "omit"', () => {
|
||||
withUserPath(() => {
|
||||
fs.mkdirSync(GSD_DIR, { recursive: true });
|
||||
fs.writeFileSync(DEFAULTS_PATH, '{ not valid json }', 'utf8');
|
||||
// Must not throw.
|
||||
callFinishInstallForRuntime('codex');
|
||||
const after = JSON.parse(fs.readFileSync(DEFAULTS_PATH, 'utf8'));
|
||||
assert.equal(
|
||||
after.resolve_model_ids,
|
||||
'omit',
|
||||
'malformed defaults.json must be recovered to a valid state with resolve_model_ids:omit',
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user