fix: prevent Codex config.toml corruption from non-boolean [features] keys (#1202)
When GSD installs codex_hooks = true under [features], any non-boolean keys already in that section (e.g. model = "gpt-5.4") cause Codex's TOML parser to fail with 'invalid type: string, expected a boolean'. Root cause: TOML sections extend until the next [section] header. If the user placed model/model_reasoning_effort under [features] (common since Codex's own config format encourages this), GSD's installer didn't detect or correct the structural issue. Fix: After injecting codex_hooks, scan the [features] section for non-boolean values and move them above [features] to the top level. This preserves the user's keys while keeping [features] clean for Codex's strict boolean parser. Includes 2 regression tests: - Detects non-boolean keys under [features] (model, model_reasoning_effort) - Confirms boolean keys (codex_hooks, multi_agent) are not flagged Closes #1202
This commit is contained in:
@@ -2992,11 +2992,35 @@ function install(isGlobal, runtime = 'claude') {
|
||||
|
||||
// Enable hooks feature flag if not present
|
||||
if (!configContent.includes('codex_hooks')) {
|
||||
const featuresSection = '[features]\ncodex_hooks = true\n';
|
||||
if (configContent.includes('[features]')) {
|
||||
configContent = configContent.replace(/\[features\]\n/, featuresSection);
|
||||
// Insert codex_hooks = true right after the [features] header.
|
||||
// Fixes #1202: previous approach could leave non-boolean keys (like
|
||||
// model = "gpt-5.4") under [features], causing Codex TOML parse errors.
|
||||
configContent = configContent.replace(/(\[features\]\n)/, '$1codex_hooks = true\n');
|
||||
} else {
|
||||
configContent = featuresSection + '\n' + configContent;
|
||||
configContent = '[features]\ncodex_hooks = true\n\n' + configContent;
|
||||
}
|
||||
}
|
||||
|
||||
// Safety check: detect non-boolean keys under [features] that would break Codex (#1202).
|
||||
// Extract the [features] section content (between [features] and next [section] or EOF).
|
||||
const featuresMatch = configContent.match(/\[features\]\n([\s\S]*?)(?=\n\[|$)/);
|
||||
if (featuresMatch) {
|
||||
const featuresBody = featuresMatch[1];
|
||||
const nonBooleanKeys = featuresBody.split('\n')
|
||||
.filter(line => line.match(/^\s*\w+\s*=/) && !line.match(/=\s*(true|false)\s*(#.*)?$/))
|
||||
.map(line => line.trim());
|
||||
if (nonBooleanKeys.length > 0) {
|
||||
// Move non-boolean keys above [features] to prevent TOML parse errors
|
||||
let cleanedFeatures = featuresBody.split('\n')
|
||||
.filter(line => !line.match(/^\s*\w+\s*=/) || line.match(/=\s*(true|false)\s*(#.*)?$/))
|
||||
.join('\n');
|
||||
const movedKeys = nonBooleanKeys.join('\n') + '\n';
|
||||
configContent = configContent.replace(
|
||||
/\[features\]\n[\s\S]*?(?=\n\[|$)/,
|
||||
movedKeys + '\n[features]\n' + cleanedFeatures.trim() + '\n'
|
||||
);
|
||||
console.log(` ${yellow}⚠${reset} Moved ${nonBooleanKeys.length} non-feature key(s) out of [features] section to prevent TOML errors`);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -492,3 +492,37 @@ describe('installCodexConfig (integration)', () => {
|
||||
assert.ok(checkerToml.includes('sandbox_mode = "read-only"'), 'plan-checker is read-only');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Codex config.toml [features] safety (#1202) ─────────────────────────────
|
||||
|
||||
describe('codex features section safety', () => {
|
||||
test('non-boolean keys under [features] are moved to top level', () => {
|
||||
// Simulate the bug from #1202: model = "gpt-5.4" under [features]
|
||||
// causes "invalid type: string, expected a boolean in features"
|
||||
const configContent = `[features]\ncodex_hooks = true\n\nmodel = "gpt-5.4"\nmodel_reasoning_effort = "medium"\n\n[agents.gsd-executor]\ndescription = "test"\n`;
|
||||
|
||||
const featuresMatch = configContent.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*(#.*)?$/))
|
||||
.map(line => line.trim());
|
||||
|
||||
assert.strictEqual(nonBooleanKeys.length, 2, 'should detect 2 non-boolean keys');
|
||||
assert.ok(nonBooleanKeys.includes('model = "gpt-5.4"'), 'detects model key');
|
||||
assert.ok(nonBooleanKeys.includes('model_reasoning_effort = "medium"'), 'detects model_reasoning_effort key');
|
||||
});
|
||||
|
||||
test('boolean keys under [features] are NOT flagged', () => {
|
||||
const configContent = `[features]\ncodex_hooks = true\nmulti_agent = false\n`;
|
||||
|
||||
const featuresMatch = configContent.match(/\[features\]\n([\s\S]*?)(?=\n\[|$)/);
|
||||
const featuresBody = featuresMatch[1];
|
||||
const nonBooleanKeys = featuresBody.split('\n')
|
||||
.filter(line => line.match(/^\s*\w+\s*=/) && !line.match(/=\s*(true|false)\s*(#.*)?$/))
|
||||
.map(line => line.trim());
|
||||
|
||||
assert.strictEqual(nonBooleanKeys.length, 0, 'no non-boolean keys in a clean config');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user