From f4d88581889d7f2a56dc83ff2667b499632e890c Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Wed, 25 Mar 2026 14:50:02 -0400 Subject: [PATCH] fix: repair trapped non-boolean keys under [features] on Codex re-install Pre-#1346 GSD installs prepended [features] before bare top-level keys in ~/.codex/config.toml, trapping keys like model="gpt-5.3-codex" under [features] where Codex expects only booleans. The #1346 fix prevented NEW corruption but did not repair EXISTING corrupted configs. Re-installing GSD left the trapped keys in place, causing "invalid type: string, expected a boolean" on every Codex launch. repairTrappedFeaturesKeys() now detects non-boolean key-value lines inside [features] and relocates them before the [features] header during ensureCodexHooksFeature(), so re-installs heal previously corrupted configs. Fixes #1379 Co-Authored-By: Claude Opus 4.6 (1M context) --- bin/install.js | 84 ++++++++++++++++++++++++++++++++++++- tests/codex-config.test.cjs | 43 +++++++++++++++++++ 2 files changed, 125 insertions(+), 2 deletions(-) diff --git a/bin/install.js b/bin/install.js index bc5e9be9f..c65b7ac52 100755 --- a/bin/install.js +++ b/bin/install.js @@ -2050,6 +2050,84 @@ function mergeCodexConfig(configPath, gsdBlock) { fs.writeFileSync(configPath, content); } +/** + * Repair config.toml files corrupted by pre-#1346 GSD installs. + * Non-boolean keys (e.g. model = "gpt-5.3-codex") that ended up under [features] + * are relocated before the [features] header so Codex can parse them correctly. + * Returns the content unchanged if no trapped keys are found. + */ +function repairTrappedFeaturesKeys(content) { + const eol = detectLineEnding(content); + const lineRecords = getTomlLineRecords(content); + const featuresSection = getTomlTableSections(content) + .find((section) => !section.array && section.path === 'features'); + + if (!featuresSection) { + return content; + } + + // Find non-boolean key-value lines inside [features] that don't belong there. + // Boolean keys (codex_hooks, multi_agent, etc.) are legitimate feature flags. + const trappedLines = lineRecords.filter((record) => { + if (record.tableHeader || record.startsInMultilineString) return false; + if (record.tablePath !== 'features') return false; + if (record.start < featuresSection.headerEnd) return false; + if (record.end + record.eol.length > featuresSection.end) return false; + if (!record.keySegments || record.keySegments.length === 0) return false; + + // Check if the value is a boolean — if so, it belongs under [features] + const equalsIndex = findTomlAssignmentEquals(record.text); + if (equalsIndex === -1) return false; + const commentStart = findTomlCommentStart(record.text); + const valueText = record.text + .slice(equalsIndex + 1, commentStart === -1 ? record.text.length : commentStart) + .trim(); + if (valueText === 'true' || valueText === 'false') return false; + + // Skip values that start a multiline string — they may legitimately live + // under [features] and spanning multiple lines makes relocation unsafe. + if (valueText.startsWith("'''") || valueText.startsWith('"""')) return false; + + // Non-boolean value — this key is trapped + return true; + }); + + if (trappedLines.length === 0) { + return content; + } + + // Build the relocated text block from trapped lines + const relocatedText = trappedLines.map((r) => r.text).join(eol) + eol; + + // Remove trapped lines from their current positions (with their EOLs) + const removalRanges = trappedLines.map((r) => ({ + start: r.start, + end: r.end + r.eol.length, + })); + let cleaned = removeContentRanges(content, removalRanges); + + // Collapse any runs of 3+ blank lines left behind + cleaned = collapseTomlBlankLines(cleaned); + + // Re-locate the [features] header in the cleaned content + const cleanedRecords = getTomlLineRecords(cleaned); + const cleanedFeaturesHeader = cleanedRecords.find( + (r) => r.tableHeader && r.tableHeader.path === 'features' && !r.tableHeader.array + ); + + if (!cleanedFeaturesHeader) { + return cleaned; + } + + // Insert relocated keys before [features] + const before = cleaned.slice(0, cleanedFeaturesHeader.start); + const after = cleaned.slice(cleanedFeaturesHeader.start); + const needsGap = before.length > 0 && !before.endsWith(eol + eol); + const trailingGap = after.length > 0 && !relocatedText.endsWith(eol + eol) ? eol : ''; + + return before + (needsGap ? eol : '') + relocatedText + trailingGap + after; +} + function ensureCodexHooksFeature(configContent) { const eol = detectLineEnding(configContent); const lineRecords = getTomlLineRecords(configContent); @@ -2071,8 +2149,9 @@ function ensureCodexHooksFeature(configContent) { ); if (sectionLines.length > 0) { + const rewritten = rewriteTomlKeyLines(configContent, sectionLines, 'codex_hooks'); return { - content: rewriteTomlKeyLines(configContent, sectionLines, 'codex_hooks'), + content: repairTrappedFeaturesKeys(rewritten), ownership: null, }; } @@ -2081,8 +2160,9 @@ function ensureCodexHooksFeature(configContent) { const needsSeparator = sectionBody.length > 0 && !sectionBody.endsWith('\n') && !sectionBody.endsWith('\r\n'); const insertPrefix = sectionBody.length === 0 && featuresSection.headerEnd === configContent.length ? eol : ''; const insertText = `${insertPrefix}${needsSeparator ? eol : ''}codex_hooks = true${eol}`; + const merged = configContent.slice(0, featuresSection.end) + insertText + configContent.slice(featuresSection.end); return { - content: configContent.slice(0, featuresSection.end) + insertText + configContent.slice(featuresSection.end), + content: repairTrappedFeaturesKeys(merged), ownership: 'section', }; } diff --git a/tests/codex-config.test.cjs b/tests/codex-config.test.cjs index ad5125913..bcd314ee6 100644 --- a/tests/codex-config.test.cjs +++ b/tests/codex-config.test.cjs @@ -760,6 +760,49 @@ describe('Codex install hook configuration (e2e)', () => { assert.ok(!content.includes('config_file = "agents/'), 'no relative config_file paths'); }); + test('re-install repairs non-boolean keys trapped under [features] by previous install (#1379)', () => { + // Bug: a pre-#1346 install prepended [features] before bare top-level keys, + // trapping model= under [features]. Re-installing with the fix must detect + // and relocate those keys back to the top level so Codex can parse them. + writeCodexConfig(codexHome, [ + '[features]', + 'codex_hooks = true', + '', + 'model = "gpt-5.3-codex"', + 'model_reasoning_effort = "high"', + '', + '[projects."/Users/oltmannk/myproject"]', + 'trust_level = "trusted"', + '', + ].join('\n')); + + runCodexInstall(codexHome); + + const content = readCodexConfig(codexHome); + + // model= and model_reasoning_effort= must NOT be under [features] + const featuresIndex = content.indexOf('[features]'); + const modelIndex = content.indexOf('model = "gpt-5.3-codex"'); + const reasoningIndex = content.indexOf('model_reasoning_effort = "high"'); + assert.ok(modelIndex !== -1, 'model key is present'); + assert.ok(reasoningIndex !== -1, 'model_reasoning_effort key is present'); + assert.ok(modelIndex < featuresIndex, 'model= relocated before [features]'); + assert.ok(reasoningIndex < featuresIndex, 'model_reasoning_effort= relocated before [features]'); + + // [features] should only contain boolean keys + const featuresMatch = content.match(/\[features\]\n([\s\S]*?)(?=\n\[|$)/); + assert.ok(featuresMatch, 'features section found'); + const featuresBody = featuresMatch[1]; + const nonBooleanKeys = featuresBody.split('\n') + .filter(line => line.match(/^\s*\w+\s*=/) && !line.match(/=\s*(true|false)\s*(#.*)?$/)); + assert.strictEqual(nonBooleanKeys.length, 0, 'no non-boolean keys under [features]'); + + // User content preserved + assert.ok(content.includes('[projects."/Users/oltmannk/myproject"]'), 'preserves project section'); + assert.ok(content.includes('trust_level = "trusted"'), 'preserves project trust level'); + assert.strictEqual(countMatches(content, /^codex_hooks = true$/gm), 1, 'one codex_hooks key'); + }); + test('existing LF config without [features] gets one features block and preserves user content', () => { writeCodexConfig(codexHome, [ '# user comment',