diff --git a/.changeset/gentle-jaguars-munch.md b/.changeset/gentle-jaguars-munch.md new file mode 100644 index 000000000..8419e74b2 --- /dev/null +++ b/.changeset/gentle-jaguars-munch.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 1661 +--- +**`gsd install`/upgrade now recovers a malformed `~/.gsd/defaults.json` instead of leaving it broken** — a `defaults.json` containing a valid-JSON-but-non-object value (`null`, `[]`, a number, or a string) bypassed the parse `catch` and flowed through unrecovered: `null` threw a TypeError (swallowed by the outer guard, logging a confusing "Could not write" warning and leaving the file as `null`), while `[]`/`42`/`"str"` silently kept their broken shape on every install. The non-Claude finishInstall step now resets any non-object parse result to a fresh `{}` before reading/writing it, so the file is repaired and `resolve_model_ids` defaults normally. diff --git a/bin/install.js b/bin/install.js index 37b893f8c..d9cbfeacf 100755 --- a/bin/install.js +++ b/bin/install.js @@ -11322,6 +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 */ } + // Recover a malformed (valid-JSON-but-non-object) defaults.json to a fresh object so + // the write below succeeds and the file is no longer broken. Without this, `null` / + // `[]` / a number / a string bypass the parse catch and either throw a TypeError on + // property access (swallowed by the outer try/catch, leaving the file broken) or get + // a property set that won't round-trip through JSON.stringify. (#1657) + if (defaults === null || typeof defaults !== 'object' || Array.isArray(defaults)) { + defaults = {}; + } // 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 diff --git a/tests/bug-410-install-defaults-test-mode-guard.test.cjs b/tests/bug-410-install-defaults-test-mode-guard.test.cjs index d0e73da92..0d5061ed7 100644 --- a/tests/bug-410-install-defaults-test-mode-guard.test.cjs +++ b/tests/bug-410-install-defaults-test-mode-guard.test.cjs @@ -254,3 +254,40 @@ describe('Bug #1569: non-Claude finishInstall preserves explicit resolve_model_i }); }); }); + +// Bug #1657 — finishInstall reads ~/.gsd/defaults.json with JSON.parse but did not +// validate the result is a plain object. A valid-JSON-but-non-object value (null, [], +// 42, "str") bypassed the catch and flowed through, leaving the malformed file on disk +// unrecovered (and, for null, throwing a TypeError swallowed by the outer try/catch). +// Folded into the owning install-defaults test (no new top-level bug-NNNN file). +describe('Bug #1657: finishInstall recovers a malformed (non-object) defaults.json', () => { + function seedDefaultsRaw(raw) { + fs.mkdirSync(GSD_DIR, { recursive: true }); + fs.writeFileSync(DEFAULTS_PATH, raw, 'utf8'); + } + function runAndRead(runtime) { + const saved = process.env.GSD_TEST_MODE; + delete process.env.GSD_TEST_MODE; + const log = console.log; console.log = () => {}; + let threw = null; + try { + installModule.finishInstall(SETTINGS_PATH, {}, null, false, runtime, true, null); + } catch (e) { threw = e.message; } finally { console.log = log; process.env.GSD_TEST_MODE = saved; } + let after = null; + try { after = JSON.parse(fs.readFileSync(DEFAULTS_PATH, 'utf8')); } catch (e) { after = 'UNPARSEABLE: ' + e.message; } + return { threw, after }; + } + + for (const [label, raw] of [['null', 'null'], ['array', '[]'], ['number', '42'], ['string', '"oops"']]) { + test(`seed ${label} (${raw}) recovers to a valid object with resolve_model_ids:omit`, () => { + seedDefaultsRaw(raw); + const { threw, after } = runAndRead('codex'); + assert.equal(threw, null, `must not throw for seed ${label} (got: ${threw})`); + assert.equal( + after !== null && typeof after === 'object' && !Array.isArray(after) && after.resolve_model_ids === 'omit', + true, + `seed ${label} must recover to { resolve_model_ids: 'omit' }, got: ${JSON.stringify(after)}`, + ); + }); + } +});