eliott/get-shit-done
1
0
Fork 0
mirror of https://github.com/glittercowboy/get-shit-done synced 2026-04-25 17:25:23 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(#2645): emit [[agents]] array-of-tables in Codex config.toml (#2664)

Browse Source 
* fix(#2645): emit [[agents]] array-of-tables in Codex config.toml

Codex ≥0.116 rejects `[agents.<name>]` map tables with `invalid type:
map, expected a sequence`. Switch generateCodexConfigBlock to emit
`[[agents]]` array-of-tables with an explicit `name` field per entry.

Strip + merge paths now self-heal on reinstall — both the legacy
`[agents.gsd-*]` map shape (pre-#2645 configs) and the new
`[[agents]]` with `name = "gsd-*"` shape are recognized and replaced,
while user-authored `[[agents]]` entries are preserved.

Fixes #2645

* fix(#2645): use TOML-aware parser to strip managed [[agents]] sections

CodeRabbit flagged that the prior regex-based stripper for [[agents]]
array-of-tables only matched headers at column 0 and stopped at any line
beginning with `[`. An indented [[agents]] header would not terminate the
preceding match, so a managed `gsd-*` block could absorb a following
user-authored agent and silently delete it.

Replace the ad-hoc regex with the existing TOML-aware section parser
(getTomlTableSections + removeContentRanges) so section boundaries are
authoritative regardless of indentation. Same logic applies to legacy
[agents.gsd-*] map sections.

Add a comprehensive mixed-shape test covering multiple GSD entries (both
legacy map and new array-of-tables, double- and single-quoted names)
interleaved with multiple user-authored agents in both shapes — verifies
all GSD entries are stripped and every user entry is preserved.
This commit is contained in:
Tom Boucher
2026-04-24 18:09:01 -04:00
committed by GitHub
parent 0f8f7537da
commit 7ed05c8811
2 changed files with 150 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

62
bin/install.js
View File

@@ -2066,7 +2066,10 @@ function generateCodexConfigBlock(agents, targetDir) {
];
for (const { name, description } of agents) {
lines.push(`[agents.${name}]`);
// #2645 — Codex schema requires [[agents]] array-of-tables, not [agents.<name>] maps.
// Emitting [agents.<name>] produces `invalid type: map, expected a sequence` on load.
lines.push(`[[agents]]`);
lines.push(`name = ${JSON.stringify(name)}`);
lines.push(`description = ${JSON.stringify(description)}`);
lines.push(`config_file = "${agentsPrefix}/${name}.toml"`);
lines.push('');
@@ -2075,8 +2078,39 @@ function generateCodexConfigBlock(agents, targetDir) {
return lines.join('\n');
}
/**
* Strip any managed GSD agent sections from a TOML string.
*
* Handles BOTH shapes so reinstall self-heals broken legacy configs:
* - Legacy: `[agents.gsd-*]` single-keyed map tables (pre-#2645).
* - Current: `[[agents]]` array-of-tables whose `name = "gsd-*"`.
*
* A section runs from its header to the next `[` header or EOF.
*/
function stripCodexGsdAgentSections(content) {
return content.replace(/^\[agents\.gsd-[^\]]+\]\n(?:(?!\[)[^\n]*\n?)*/gm, '');
// Use the TOML-aware section parser so we never absorb adjacent user-authored
// tables — even if their headers are indented or otherwise oddly placed.
const sections = getTomlTableSections(content).filter((section) => {
// Legacy `[agents.gsd-<name>]` map tables (pre-#2645).
if (!section.array && /^agents\.gsd-/.test(section.path)) {
return true;
}
// Current `[[agents]]` array-of-tables — only strip blocks whose
// `name = "gsd-..."`, preserving user-authored [[agents]] entries.
if (section.array && section.path === 'agents') {
const body = content.slice(section.headerEnd, section.end);
const nameMatch = body.match(/^[ \t]*name[ \t]*=[ \t]*["']([^"']+)["']/m);
return Boolean(nameMatch && /^gsd-/.test(nameMatch[1]));
}
return false;
});
return removeContentRanges(
content,
sections.map(({ start, end }) => ({ start, end })),
);
}
/**
@@ -2774,13 +2808,27 @@ function isLegacyGsdAgentsSection(body) {
function stripLeakedGsdCodexSections(content) {
const leakedSections = getTomlTableSections(content)
.filter((section) =>
section.path.startsWith('agents.gsd-') ||
(
.filter((section) => {
// Legacy [agents.gsd-<name>] map tables (pre-#2645).
if (!section.array && section.path.startsWith('agents.gsd-')) return true;
// Legacy bare [agents] table with only the old max_threads/max_depth keys.
if (
!section.array &&
section.path === 'agents' &&
isLegacyGsdAgentsSection(content.slice(section.headerEnd, section.end))
)
);
) return true;
// Current [[agents]] array-of-tables whose name is gsd-*. Preserve
// user-authored [[agents]] entries (other names) untouched.
if (section.array && section.path === 'agents') {
const body = content.slice(section.headerEnd, section.end);
const nameMatch = body.match(/^[ \t]*name[ \t]*=[ \t]*["']([^"']+)["']/m);
if (nameMatch && /^gsd-/.test(nameMatch[1])) return true;
}
return false;
});
if (leakedSections.length === 0) {
return content;

115
tests/codex-config.test.cjs
View File

@@ -396,16 +396,38 @@ describe('generateCodexConfigBlock', () => {
assert.ok(!result.includes('[features]'), 'no features table');
assert.ok(!result.includes('multi_agent'), 'no multi_agent');
assert.ok(!result.includes('default_mode_request_user_input'), 'no request_user_input');
// Should not have bare [agents] table header (only [agents.gsd-*] sections)
// #2645 — must NOT use the legacy `[agents.<name>]` map shape (causes
// `invalid type: map, expected a sequence` when Codex loads config).
assert.ok(!result.match(/^\[agents\.gsd-/m), 'no legacy [agents.gsd-*] map sections');
// Should not have bare [agents] table header either.
assert.ok(!result.match(/^\[agents\]\s*$/m), 'no bare [agents] table');
assert.ok(!result.includes('max_threads'), 'no max_threads');
assert.ok(!result.includes('max_depth'), 'no max_depth');
});
test('#2645: emits [[agents]] array-of-tables with name field', () => {
const result = generateCodexConfigBlock(agents);
// One [[agents]] header per agent.
const headerCount = (result.match(/^\[\[agents\]\]\s*$/gm) || []).length;
assert.strictEqual(headerCount, 2, 'one [[agents]] header per agent');
// Each agent has a name field matching the input name.
assert.ok(result.includes('name = "gsd-executor"'), 'executor has name field');
assert.ok(result.includes('name = "gsd-planner"'), 'planner has name field');
});
test('#2645: block is a valid TOML array-of-tables shape (no stray map headers)', () => {
const result = generateCodexConfigBlock(agents);
// Strip comment/marker lines and count structural headers. There must be
// no `[agents.X]` or `[agents]` tables mixed in with `[[agents]]`, which
// would trigger the Codex parse error from #2645.
const stray = result.match(/^\[agents(\.[^\]]+)?\]\s*$/gm);
assert.strictEqual(stray, null, 'no map-shaped [agents] or [agents.X] headers present');
});
test('includes per-agent sections with relative paths (no targetDir)', () => {
const result = generateCodexConfigBlock(agents);
assert.ok(result.includes('[agents.gsd-executor]'), 'has executor section');
assert.ok(result.includes('[agents.gsd-planner]'), 'has planner section');
assert.ok(result.includes('name = "gsd-executor"'), 'has executor entry');
assert.ok(result.includes('name = "gsd-planner"'), 'has planner entry');
assert.ok(result.includes('config_file = "agents/gsd-executor.toml"'), 'relative config_file without targetDir');
assert.ok(result.includes('"Executes plans"'), 'has executor description');
});
@@ -462,12 +484,61 @@ describe('stripGsdFromCodexConfig', () => {
assert.ok(!result.includes(GSD_CODEX_MARKER), 'strips marker');
});
test('removes [agents.gsd-*] sections', () => {
test('removes legacy [agents.gsd-*] map sections (self-heal pre-#2645 configs)', () => {
const content = `[agents.gsd-executor]\ndescription = "test"\nconfig_file = "agents/gsd-executor.toml"\n\n[agents.custom-agent]\ndescription = "user agent"\n`;
const result = stripGsdFromCodexConfig(content);
assert.ok(!result.includes('[agents.gsd-executor]'), 'removes GSD agent section');
assert.ok(!result.includes('[agents.gsd-executor]'), 'removes legacy GSD agent map section');
assert.ok(result.includes('[agents.custom-agent]'), 'preserves user agent section');
});
test('#2645: removes [[agents]] array-of-tables entries whose name is gsd-*', () => {
const content = `[[agents]]\nname = "gsd-executor"\ndescription = "test"\nconfig_file = "agents/gsd-executor.toml"\n\n[[agents]]\nname = "custom-agent"\ndescription = "user agent"\n`;
const result = stripGsdFromCodexConfig(content);
assert.ok(!/name = "gsd-executor"/.test(result), 'removes managed GSD [[agents]] entry');
assert.ok(result.includes('name = "custom-agent"'), 'preserves user [[agents]] entry');
});
test('#2645: handles mixed legacy + new shapes and multiple user/gsd entries in one file', () => {
// Multiple GSD entries (both legacy map and new array-of-tables) interleaved
// with multiple user-authored agents in both shapes — none of the user
// entries may be removed and all GSD entries must be stripped.
const content = [
'[agents.gsd-executor]',
'description = "legacy gsd"',
'config_file = "agents/gsd-executor.toml"',
'',
'[agents.custom-legacy]',
'description = "user legacy"',
'',
'[[agents]]',
'name = "gsd-planner"',
'description = "new gsd"',
'',
'[[agents]]',
'name = "my-helper"',
'description = "user new"',
'',
'[[agents]]',
"name = 'gsd-debugger'",
'description = "single-quoted gsd"',
'',
'[[agents]]',
'name = "another-user"',
'description = "second user agent"',
'',
].join('\n');
const result = stripGsdFromCodexConfig(content);
// All GSD entries removed.
assert.ok(!result.includes('gsd-executor'), 'removes legacy gsd-executor');
assert.ok(!/name\s*=\s*"gsd-planner"/.test(result), 'removes new gsd-planner');
assert.ok(!/name\s*=\s*'gsd-debugger'/.test(result), 'removes single-quoted gsd-debugger');
// All user-authored entries preserved.
assert.ok(result.includes('[agents.custom-legacy]'), 'preserves user legacy [agents.custom-legacy]');
assert.ok(result.includes('user legacy'), 'preserves user legacy body');
assert.ok(result.includes('name = "my-helper"'), 'preserves user new [[agents]]');
assert.ok(result.includes('name = "another-user"'), 'preserves second user [[agents]]');
assert.ok(result.includes('second user agent'), 'preserves second user body');
});
});
// ─── mergeCodexConfig ───────────────────────────────────────────────────────────
@@ -494,7 +565,7 @@ describe('mergeCodexConfig', () => {
assert.ok(fs.existsSync(configPath), 'file created');
const content = fs.readFileSync(configPath, 'utf8');
assert.ok(content.includes(GSD_CODEX_MARKER), 'has marker');
assert.ok(content.includes('[agents.gsd-executor]'), 'has agent');
assert.ok(content.includes('name = "gsd-executor"'), 'has agent');
assert.ok(!content.includes('[features]'), 'no features section');
assert.ok(!content.includes('multi_agent'), 'no multi_agent');
});
@@ -514,7 +585,7 @@ describe('mergeCodexConfig', () => {
const content = fs.readFileSync(configPath, 'utf8');
assert.ok(content.includes('[model]'), 'preserves user content');
assert.ok(content.includes('Updated description'), 'has new description');
assert.ok(content.includes('[agents.gsd-planner]'), 'has new agent');
assert.ok(content.includes('name = "gsd-planner"'), 'has new agent');
// Verify no duplicate markers
const markerCount = (content.match(new RegExp(GSD_CODEX_MARKER.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'g')) || []).length;
assert.strictEqual(markerCount, 1, 'exactly one marker');
@@ -529,7 +600,7 @@ describe('mergeCodexConfig', () => {
const content = fs.readFileSync(configPath, 'utf8');
assert.ok(content.includes('[model]'), 'preserves user content');
assert.ok(content.includes(GSD_CODEX_MARKER), 'adds marker');
assert.ok(content.includes('[agents.gsd-executor]'), 'has agent');
assert.ok(content.includes('name = "gsd-executor"'), 'has agent');
});
test('case 3 with existing [features]: preserves user features, does not inject GSD keys', () => {
@@ -543,7 +614,7 @@ describe('mergeCodexConfig', () => {
assert.ok(!content.includes('multi_agent'), 'does not inject multi_agent');
assert.ok(!content.includes('default_mode_request_user_input'), 'does not inject request_user_input');
assert.ok(content.includes(GSD_CODEX_MARKER), 'adds marker for agents block');
assert.ok(content.includes('[agents.gsd-executor]'), 'has agent');
assert.ok(content.includes('name = "gsd-executor"'), 'has agent');
});
test('case 3 strips existing [agents.gsd-*] sections before appending fresh block', () => {
@@ -566,12 +637,14 @@ describe('mergeCodexConfig', () => {
mergeCodexConfig(configPath, sampleBlock);
const content = fs.readFileSync(configPath, 'utf8');
const gsdAgentCount = (content.match(/^\[agents\.gsd-executor\]\s*$/gm) || []).length;
const legacyGsdAgentCount = (content.match(/^\[agents\.gsd-executor\]\s*$/gm) || []).length;
const managedAgentCount = (content.match(/name = "gsd-executor"/g) || []).length;
const markerCount = (content.match(new RegExp(GSD_CODEX_MARKER.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'g')) || []).length;
assert.ok(content.includes('[model]'), 'preserves user content');
assert.ok(content.includes('[agents.custom-agent]'), 'preserves non-GSD agent section');
assert.strictEqual(gsdAgentCount, 1, 'keeps exactly one GSD agent section');
assert.strictEqual(legacyGsdAgentCount, 0, 'strips legacy map-shape GSD agent sections');
assert.strictEqual(managedAgentCount, 1, 'keeps exactly one managed GSD [[agents]] entry');
assert.strictEqual(markerCount, 1, 'adds exactly one marker block');
assert.ok(!/\n{3,}# GSD Agent Configuration/.test(content), 'does not leave extra blank lines before marker block');
});
@@ -598,7 +671,7 @@ describe('mergeCodexConfig', () => {
const featuresCount = (content.match(/^\[features\]\s*$/gm) || []).length;
assert.strictEqual(featuresCount, 1, 'exactly one [features] section');
assert.ok(content.includes('other_feature = true'), 'preserves user feature keys');
assert.ok(content.includes('[agents.gsd-executor]'), 'has agent');
assert.ok(content.includes('name = "gsd-executor"'), 'has agent');
// Verify no duplicate markers
const markerCount = (content.match(new RegExp(GSD_CODEX_MARKER.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), 'g')) || []).length;
assert.strictEqual(markerCount, 1, 'exactly one marker');
@@ -615,7 +688,7 @@ describe('mergeCodexConfig', () => {
assert.ok(!content.includes('multi_agent'), 'does not inject multi_agent');
assert.ok(!content.includes('default_mode_request_user_input'), 'does not inject request_user_input');
assert.ok(content.includes('other_feature = true'), 'preserves user feature');
assert.ok(content.includes('[agents.gsd-executor]'), 'has agent from fresh block');
assert.ok(content.includes('name = "gsd-executor"'), 'has agent from fresh block');
});
test('case 2 strips leaked [agents] and [agents.gsd-*] from before content', () => {
@@ -645,7 +718,7 @@ describe('mergeCodexConfig', () => {
const content = fs.readFileSync(configPath, 'utf8');
assert.ok(content.includes('child_agents_md = false'), 'preserves user feature keys');
assert.ok(content.includes('[agents.gsd-executor]'), 'has agent from fresh block');
assert.ok(content.includes('name = "gsd-executor"'), 'has agent from fresh block');
// Verify the leaked [agents] table header above marker was stripped
const markerIndex = content.indexOf(GSD_CODEX_MARKER);
const beforeMarker = content.substring(0, markerIndex);
@@ -685,7 +758,8 @@ describe('mergeCodexConfig', () => {
assert.ok(content.includes('child_agents_md = false'), 'preserves user feature keys');
assert.strictEqual(countMatches(beforeMarker, /^\[agents\]\s*$/gm), 0, 'removes leaked [agents] above marker');
assert.strictEqual(countMatches(beforeMarker, /^\[agents\.gsd-executor\]\s*$/gm), 0, 'removes leaked GSD agent section above marker');
assert.strictEqual(countMatches(content, /^\[agents\.gsd-executor\]\s*$/gm), 1, 'keeps one managed agent section');
assert.strictEqual(countMatches(content, /^\[agents\.gsd-executor\]\s*$/gm), 0, 'no legacy map-shape sections remain (all replaced by new [[agents]] block)');
assert.strictEqual(countMatches(content, /name = "gsd-executor"/g), 1, 'keeps one managed agent entry');
assertUsesOnlyEol(content, '\r\n');
});
@@ -720,7 +794,8 @@ describe('mergeCodexConfig', () => {
assert.ok(beforeMarker.includes('[agents]\r\ndefault = "custom-agent"\r\n'), 'preserves user-authored [agents] table');
assert.strictEqual(countMatches(beforeMarker, /^\[agents\.gsd-executor\]\s*$/gm), 0, 'removes leaked GSD agent section above marker');
assert.strictEqual(countMatches(content, /^\[agents\.gsd-executor\]\s*$/gm), 1, 'keeps one managed agent section in the GSD block');
assert.strictEqual(countMatches(content, /^\[agents\.gsd-executor\]\s*$/gm), 0, 'no legacy map-shape sections remain');
assert.strictEqual(countMatches(content, /name = "gsd-executor"/g), 1, 'keeps one managed agent entry in the GSD block');
assertUsesOnlyEol(content, '\r\n');
});
@@ -792,7 +867,7 @@ describe('installCodexConfig (integration)', () => {
assert.ok(fs.existsSync(configPath), 'config.toml exists');
const config = fs.readFileSync(configPath, 'utf8');
assert.ok(config.includes(GSD_CODEX_MARKER), 'has GSD marker');
assert.ok(config.includes('[agents.gsd-executor]'), 'has executor agent');
assert.ok(config.includes('name = "gsd-executor"'), 'has executor agent');
assert.ok(!config.includes('multi_agent'), 'no feature flags');
// Verify per-agent .toml files
@@ -1103,7 +1178,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 1, 'keeps one [features] section');
assert.strictEqual(countMatches(content, /^codex_hooks = true$/gm), 1, 'adds one codex_hooks key');
assert.ok(content.indexOf('codex_hooks = true') > content.indexOf('[features]'), 'adds codex_hooks after the existing EOF features header');
assert.ok(content.indexOf('codex_hooks = true') < content.indexOf('[agents.gsd-codebase-mapper]'), 'keeps codex_hooks before the next real table');
assert.ok(content.indexOf('codex_hooks = true') < content.indexOf('[[agents]]'), 'keeps codex_hooks before the first managed [[agents]] entry');
assertNoDraftRootKeys(content);
});
@@ -1233,7 +1308,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.strictEqual(countMatches(content, /^features\.codex_hooks = true$/gm), 0, 'does not append an invalid dotted codex_hooks key');
assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 0, 'does not prepend a features table');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 0, 'does not add the GSD hook block when codex_hooks cannot be enabled safely');
assert.ok(content.includes('[agents.gsd-executor]'), 'still installs the managed agent block');
assert.ok(content.includes('name = "gsd-executor"'), 'still installs the managed agent block');
assertNoDraftRootKeys(content);
});
@@ -1254,7 +1329,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.strictEqual(countMatches(content, /^features\.codex_hooks = true$/gm), 0, 'does not append an invalid dotted codex_hooks key');
assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 0, 'does not prepend a features table');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 0, 'does not add the GSD hook block when codex_hooks cannot be enabled safely');
assert.ok(content.includes('[agents.gsd-executor]'), 'still installs the managed agent block');
assert.ok(content.includes('name = "gsd-executor"'), 'still installs the managed agent block');
assertNoDraftRootKeys(content);
});