Merge pull request #1384 from gsd-build/fix/codex-config-repair-trapped-keys-1379

fix: repair trapped non-boolean keys under [features] on Codex re-install

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',
     };
   }

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',