diff --git a/bin/install.js b/bin/install.js index aa62fd27c..962ea2eb3 100755 --- a/bin/install.js +++ b/bin/install.js @@ -638,9 +638,31 @@ function mergeCodexConfig(configPath, gsdBlock) { // Case 2: Has GSD marker — truncate and re-append if (markerIndex !== -1) { - const before = existing.substring(0, markerIndex).trimEnd(); - const newContent = before ? before + '\n\n' + gsdBlock + '\n' : gsdBlock + '\n'; - fs.writeFileSync(configPath, newContent); + let before = existing.substring(0, markerIndex).trimEnd(); + if (before) { + // Strip any GSD-managed sections that leaked above the marker from previous installs + before = before.replace(/^\[agents\.gsd-[^\]]+\]\n(?:(?!\[)[^\n]*\n?)*/gm, ''); + before = before.replace(/^\[agents\]\n(?:(?!\[)[^\n]*\n?)*/m, ''); + before = before.replace(/\n{3,}/g, '\n\n').trimEnd(); + + // Re-inject feature keys if user has [features] above the marker + const hasFeatures = /^\[features\]\s*$/m.test(before); + if (hasFeatures) { + if (!before.includes('multi_agent')) { + before = before.replace(/^\[features\]\s*$/m, '[features]\nmulti_agent = true'); + } + if (!before.includes('default_mode_request_user_input')) { + before = before.replace(/^\[features\].*$/m, '$&\ndefault_mode_request_user_input = true'); + } + } + // Skip [features] from gsdBlock if user already has it + const block = hasFeatures + ? GSD_CODEX_MARKER + '\n' + gsdBlock.substring(gsdBlock.indexOf('[agents]')) + : gsdBlock; + fs.writeFileSync(configPath, before + '\n\n' + block + '\n'); + } else { + fs.writeFileSync(configPath, gsdBlock + '\n'); + } return; } diff --git a/get-shit-done/bin/lib/milestone.cjs b/get-shit-done/bin/lib/milestone.cjs index 9021d2bea..71dc918de 100644 --- a/get-shit-done/bin/lib/milestone.cjs +++ b/get-shit-done/bin/lib/milestone.cjs @@ -4,7 +4,7 @@ const fs = require('fs'); const path = require('path'); -const { escapeRegex, output, error } = require('./core.cjs'); +const { escapeRegex, getMilestonePhaseFilter, output, error } = require('./core.cjs'); const { extractFrontmatter } = require('./frontmatter.cjs'); const { writeStateMd } = require('./state.cjs'); @@ -93,42 +93,10 @@ function cmdMilestoneComplete(cwd, version, options, raw) { // Ensure archive directory exists fs.mkdirSync(archiveDir, { recursive: true }); - // Extract milestone phase numbers from ROADMAP.md to scope stats. - // Only phases listed in the current ROADMAP are counted — phases from - // prior milestones that remain on disk are excluded. - // - // Related upstream PRs (getMilestoneInfo, not milestone complete): - // #756 — fix(core): detect current milestone correctly in getMilestoneInfo - // #783 — fix: getMilestoneInfo() returns wrong version after completion - // Those PRs fix *which* milestone is detected; this fix scopes *stats* - // and *accomplishments* to only the phases belonging to that milestone. - const milestonePhaseNums = new Set(); - if (fs.existsSync(roadmapPath)) { - try { - const roadmapContent = fs.readFileSync(roadmapPath, 'utf-8'); - const phasePattern = /#{2,4}\s*Phase\s+(\d+[A-Z]?(?:\.\d+)*)\s*:/gi; - let phaseMatch; - while ((phaseMatch = phasePattern.exec(roadmapContent)) !== null) { - milestonePhaseNums.add(phaseMatch[1]); - } - } catch {} - } - - // Pre-normalize phase numbers for O(1) lookup — strip leading zeros - // and lowercase for case-insensitive matching of letter suffixes (e.g. 3A/3a). - const normalizedPhaseNums = new Set( - [...milestonePhaseNums].map(num => (num.replace(/^0+/, '') || '0').toLowerCase()) - ); - - // Match a phase directory name to the milestone's phase set. - // Handles: "01-foo" → "1", "3A-bar" → "3a", "3.1-baz" → "3.1" - // Returns false for non-phase directories (no leading digit). - function isDirInMilestone(dirName) { - if (normalizedPhaseNums.size === 0) return true; // no scoping - const m = dirName.match(/^0*(\d+[A-Za-z]?(?:\.\d+)*)/); - if (!m) return false; // not a phase directory - return normalizedPhaseNums.has(m[1].toLowerCase()); - } + // Scope stats and accomplishments to only the phases belonging to the + // current milestone's ROADMAP. Uses the shared filter from core.cjs + // (same logic used by cmdPhasesList and other callers). + const isDirInMilestone = getMilestonePhaseFilter(cwd); // Gather stats from phases (scoped to current milestone only) let phaseCount = 0; @@ -190,15 +158,20 @@ function cmdMilestoneComplete(cwd, version, options, raw) { if (fs.existsSync(milestonesPath)) { const existing = fs.readFileSync(milestonesPath, 'utf-8'); - // Insert after the header line(s) for reverse chronological order (newest first) - const headerMatch = existing.match(/^(#{1,3}\s+[^\n]*\n\n?)/); - if (headerMatch) { - const header = headerMatch[1]; - const rest = existing.slice(header.length); - fs.writeFileSync(milestonesPath, header + milestoneEntry + rest, 'utf-8'); + if (!existing.trim()) { + // Empty file — treat like new + fs.writeFileSync(milestonesPath, `# Milestones\n\n${milestoneEntry}`, 'utf-8'); } else { - // No recognizable header — prepend the entry - fs.writeFileSync(milestonesPath, milestoneEntry + existing, 'utf-8'); + // Insert after the header line(s) for reverse chronological order (newest first) + const headerMatch = existing.match(/^(#{1,3}\s+[^\n]*\n\n?)/); + if (headerMatch) { + const header = headerMatch[1]; + const rest = existing.slice(header.length); + fs.writeFileSync(milestonesPath, header + milestoneEntry + rest, 'utf-8'); + } else { + // No recognizable header — prepend the entry + fs.writeFileSync(milestonesPath, milestoneEntry + existing, 'utf-8'); + } } } else { fs.writeFileSync(milestonesPath, `# Milestones\n\n${milestoneEntry}`, 'utf-8'); diff --git a/tests/codex-config.test.cjs b/tests/codex-config.test.cjs index e9cf8ad32..758eedc84 100644 --- a/tests/codex-config.test.cjs +++ b/tests/codex-config.test.cjs @@ -365,6 +365,84 @@ describe('mergeCodexConfig', () => { assert.strictEqual(first, second, 'idempotent merge'); }); + + test('case 2 after case 3 with existing [features]: no duplicate sections', () => { + const configPath = path.join(tmpDir, 'config.toml'); + fs.writeFileSync(configPath, '[features]\nother_feature = true\n\n[model]\nname = "o3"\n'); + mergeCodexConfig(configPath, sampleBlock); + + mergeCodexConfig(configPath, sampleBlock); + + const content = fs.readFileSync(configPath, 'utf8'); + const featuresCount = (content.match(/^\[features\]\s*$/gm) || []).length; + const agentsCount = (content.match(/^\[agents\]\s*$/gm) || []).length; + assert.strictEqual(featuresCount, 1, 'exactly one [features] section'); + assert.strictEqual(agentsCount, 1, 'exactly one [agents] section'); + assert.ok(content.includes('other_feature = true'), 'preserves user feature keys'); + assert.ok(content.includes('multi_agent = true'), 'has GSD feature key'); + assert.ok(content.includes('[agents.gsd-executor]'), 'has agent'); + }); + + test('case 2 re-injects missing feature keys', () => { + const configPath = path.join(tmpDir, 'config.toml'); + const manualContent = '[features]\nother_feature = true\n\n' + GSD_CODEX_MARKER + '\n[agents]\nmax_threads = 4\n'; + fs.writeFileSync(configPath, manualContent); + + mergeCodexConfig(configPath, sampleBlock); + + const content = fs.readFileSync(configPath, 'utf8'); + assert.ok(content.includes('multi_agent = true'), 're-injects multi_agent'); + assert.ok(content.includes('default_mode_request_user_input = true'), 're-injects request_user_input'); + assert.ok(content.includes('other_feature = true'), 'preserves user feature'); + }); + + test('case 2 strips leaked [agents] from before content', () => { + const configPath = path.join(tmpDir, 'config.toml'); + const brokenContent = [ + '[features]', + 'default_mode_request_user_input = true', + 'multi_agent = true', + 'child_agents_md = false', + '', + '[agents]', + 'max_threads = 4', + 'max_depth = 2', + '', + '[agents.gsd-executor]', + 'description = "old"', + 'config_file = "agents/gsd-executor.toml"', + '', + GSD_CODEX_MARKER, + '[agents]', + 'max_threads = 4', + '', + ].join('\n'); + fs.writeFileSync(configPath, brokenContent); + + mergeCodexConfig(configPath, sampleBlock); + + const content = fs.readFileSync(configPath, 'utf8'); + const agentsCount = (content.match(/^\[agents\]\s*$/gm) || []).length; + assert.strictEqual(agentsCount, 1, 'exactly one [agents] section'); + assert.ok(content.includes('child_agents_md = false'), 'preserves user feature keys'); + assert.ok(content.includes('[agents.gsd-executor]'), 'has agent from fresh block'); + }); + + test('case 2 idempotent after case 3 with existing [features]', () => { + const configPath = path.join(tmpDir, 'config.toml'); + fs.writeFileSync(configPath, '[features]\nother_feature = true\n'); + mergeCodexConfig(configPath, sampleBlock); + const first = fs.readFileSync(configPath, 'utf8'); + + mergeCodexConfig(configPath, sampleBlock); + const second = fs.readFileSync(configPath, 'utf8'); + + mergeCodexConfig(configPath, sampleBlock); + const third = fs.readFileSync(configPath, 'utf8'); + + assert.strictEqual(first, second, 'idempotent after 2nd merge'); + assert.strictEqual(second, third, 'idempotent after 3rd merge'); + }); }); // ─── Integration: installCodexConfig ────────────────────────────────────────────