From 672051becda40f5dc5329075d66009101d15d974 Mon Sep 17 00:00:00 2001 From: Marzx13 <28298824+Marzx13@users.noreply.github.com> Date: Fri, 21 Aug 2026 23:11:50 -0400 Subject: [PATCH 1/2] fix(archive): preserve requirement order when renaming --- src/core/specs-apply.ts | 20 +++-- test/core/archive.test.ts | 124 ++++++++++++++++++++++++++ test/core/specs-apply.salvage.test.ts | 30 +++++++ 3 files changed, 167 insertions(+), 7 deletions(-) diff --git a/src/core/specs-apply.ts b/src/core/specs-apply.ts index f0a8ff3842..83100a4b07 100644 --- a/src/core/specs-apply.ts +++ b/src/core/specs-apply.ts @@ -371,11 +371,13 @@ export async function buildUpdatedSpec( for (const block of parts.bodyBlocks) { nameToBlock.set(normalizeRequirementName(block.name), block); } + // Keep source blocks immutable for loss attribution. This parallel key list + // carries only positional identity as renames change lookup keys. + const orderedKeys = parts.bodyBlocks.map((block) => normalizeRequirementName(block.name)); // Apply operations in order: RENAMED → REMOVED → MODIFIED → ADDED // RENAMED let renamedApplied = 0; - const renamedTargets = new Map(); for (const r of plan.renamed) { const from = normalizeRequirementName(r.from); const to = normalizeRequirementName(r.to); @@ -413,7 +415,12 @@ export async function buildUpdatedSpec( }; nameToBlock.delete(from); nameToBlock.set(to, renamedBlock); - renamedTargets.set(from, to); + // A Map delete+set moves the renamed block to insertion-order tail. Carry + // its new key in the source slot instead; chained renames update it again. + const orderIndex = orderedKeys.indexOf(from); + if (orderIndex >= 0) { + orderedKeys[orderIndex] = to; + } renamedApplied++; } @@ -499,8 +506,9 @@ export async function buildUpdatedSpec( // Recompose requirements section preserving original ordering where possible const keptOrder: RequirementBlock[] = []; const seen = new Set(); - for (const block of parts.bodyBlocks) { - const key = normalizeRequirementName(block.name); + for (let index = 0; index < parts.bodyBlocks.length; index++) { + const block = parts.bodyBlocks[index]; + const key = orderedKeys[index]; const replacement = nameToBlock.get(key); if (replacement) { keptOrder.push(replacement); @@ -512,9 +520,7 @@ export async function buildUpdatedSpec( // full absorbed suffix. RENAMED carries the original raw content under a // new map key, and MODIFIED may repeat the suffix deliberately; neither is // data loss. - const renamedTarget = renamedTargets.get(key); - const replacementFromOriginal = - replacement ?? (renamedTarget ? nameToBlock.get(renamedTarget) : undefined); + const replacementFromOriginal = replacement; if (replacementFromOriginal !== block) { const foreign = firstForeignTail(block.raw); const replacementRaw = replacementFromOriginal?.raw; diff --git a/test/core/archive.test.ts b/test/core/archive.test.ts index b120bb0faf..bbdfcf1bed 100644 --- a/test/core/archive.test.ts +++ b/test/core/archive.test.ts @@ -2780,6 +2780,130 @@ content D`; expect(updated).not.toContain('### Requirement: B'); }); + it('should preserve source order and lineage when renaming requirements', async () => { + const changeName = 'rename-order'; + const renamed = (pairs: Array<[string, string]>): string => + `## RENAMED Requirements\n\n${pairs + .map( + ([from, to]) => + `- FROM: \`### Requirement: ${from}\`\n- TO: \`### Requirement: ${to}\`` + ) + .join('\n\n')}`; + const cases = [ + { capability: 'first', names: ['A', 'B', 'C'], delta: renamed([['A', 'A2']]), expected: ['A2', 'B', 'C'] }, + { capability: 'middle', names: ['A', 'B', 'C'], delta: renamed([['B', 'B2']]), expected: ['A', 'B2', 'C'] }, + { capability: 'last', names: ['A', 'B', 'C'], delta: renamed([['C', 'C2']]), expected: ['A', 'B', 'C2'] }, + { + capability: 'multiple', + names: ['A', 'B', 'C'], + delta: renamed([['B', 'B2'], ['A', 'A2']]), + expected: ['A2', 'B2', 'C'], + }, + { + capability: 'chained', + names: ['X', 'A', 'Y'], + delta: renamed([['A', 'B'], ['B', 'C']]), + expected: ['X', 'C', 'Y'], + }, + { + capability: 'modified', + names: ['A', 'B', 'C'], + delta: `${renamed([['B', 'B2']])}\n\n## MODIFIED Requirements\n\n### Requirement: B2\nModified body.`, + expected: ['A', 'B2', 'C'], + expectedContent: '### Requirement: B2\nModified body.', + }, + { + capability: 'readded', + names: ['X', 'A', 'Y'], + delta: `${renamed([['A', 'B']])}\n\n## ADDED Requirements\n\n### Requirement: A\nNew body A.`, + expected: ['X', 'B', 'Y', 'A'], + }, + { + capability: 'foreign-tail', + names: ['A', 'B', 'C'], + delta: renamed([['B', 'B2']]), + expected: ['A', 'B2', 'C'], + foreignTail: '### Notes\nAuthored note travels with B.', + expectedContent: '### Requirement: B2\nBody B.\n\n### Notes\nAuthored note travels with B.', + }, + ]; + + for (const item of cases) { + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', item.capability); + const changeSpecDir = path.join( + tempDir, + 'openspec', + 'changes', + changeName, + 'specs', + item.capability + ); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.mkdir(changeSpecDir, { recursive: true }); + const blocks = item.names.map( + (name) => + `### Requirement: ${name}\nBody ${name}.` + + (name === 'B' && item.foreignTail ? `\n\n${item.foreignTail}` : '') + ); + await fs.writeFile( + path.join(mainSpecDir, 'spec.md'), + `# ${item.capability} Specification\n\n## Purpose\nOrdering fixture.\n\n## Requirements\n\n${blocks.join('\n\n')}\n` + ); + await fs.writeFile( + path.join(changeSpecDir, 'spec.md'), + `# ${item.capability} - Changes\n\n${item.delta}\n` + ); + } + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + for (const item of cases) { + const updated = await fs.readFile( + path.join(tempDir, 'openspec', 'specs', item.capability, 'spec.md'), + 'utf-8' + ); + const names = [...updated.matchAll(/^### Requirement:\s*(.+?)\s*$/gm)].map( + (match) => match[1] + ); + expect(names).toEqual(item.expected); + if (item.expectedContent) expect(updated).toContain(item.expectedContent); + } + const output = (console.log as unknown as { mock: { calls: unknown[][] } }).mock.calls + .flat() + .map(String) + .join('\n'); + expect(output).not.toContain('sits inside requirement "B"'); + }); + + it('should keep the target and change untouched when a later rename collides', async () => { + const changeName = 'late-rename-collision'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const changeSpecDir = path.join(changeDir, 'specs', 'demo'); + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'demo'); + const mainSpecPath = path.join(mainSpecDir, 'spec.md'); + const changeSpecPath = path.join(changeSpecDir, 'spec.md'); + const mainContent = `# demo Specification\n\n## Purpose\nTransaction fixture.\n\n## Requirements\n\n### Requirement: A\nBody A.\n\n### Requirement: B\nBody B.\n\n### Requirement: C\nBody C.\n`; + const changeContent = `# demo - Changes\n\n## RENAMED Requirements\n\n- FROM: \`### Requirement: A\`\n- TO: \`### Requirement: A2\`\n\n- FROM: \`### Requirement: A2\`\n- TO: \`### Requirement: C\`\n`; + await fs.mkdir(changeSpecDir, { recursive: true }); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.writeFile(mainSpecPath, mainContent); + await fs.writeFile(changeSpecPath, changeContent); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining( + 'RENAMED failed for header "### Requirement: C" - target already exists' + ) + ); + expect(process.exitCode).toBe(1); + await expect(fs.readFile(mainSpecPath, 'utf-8')).resolves.toBe(mainContent); + await expect(fs.readFile(changeSpecPath, 'utf-8')).resolves.toBe(changeContent); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + const archives = await fs.readdir(path.join(tempDir, 'openspec', 'changes', 'archive')); + expect(archives.some((entry) => entry.includes(changeName))).toBe(false); + }); + it('should abort with error when MODIFIED references non-existent requirements', async () => { const changeName = 'validate-missing'; const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); diff --git a/test/core/specs-apply.salvage.test.ts b/test/core/specs-apply.salvage.test.ts index af82c2be6e..e889195353 100644 --- a/test/core/specs-apply.salvage.test.ts +++ b/test/core/specs-apply.salvage.test.ts @@ -141,6 +141,36 @@ describe('buildUpdatedSpec (content absorbed into a requirement)', () => { expect(warnings.join('\n')).not.toContain('goes with it'); }); + it('warns against the source requirement when a rename-plus-modify drops its tail', async () => { + const tail = [' ### Notes', 'Kept by hand.']; + const renamedRequirement = [...REQUIREMENT]; + renamedRequirement[0] = '### Requirement: Renamed'; + const { rebuilt, counts, warnings } = await build(SPEC(tail), [ + '# demo - Changes', + '', + '## RENAMED Requirements', + '', + '- FROM: `### Requirement: Target`', + '- TO: `### Requirement: Renamed`', + '', + '## MODIFIED Requirements', + '', + ...renamedRequirement, + '', + ]); + + expect([...rebuilt.matchAll(/^### Requirement:\s*(.+?)\s*$/gm)].map((m) => m[1])).toEqual([ + 'Renamed', + 'Other', + ]); + expect(rebuilt).not.toContain(tail.join('\n')); + expect(counts).toMatchObject({ modified: 1, renamed: 1 }); + expect(warnings.join('\n')).toContain( + '"### Notes" sits inside requirement "Target" and goes with it' + ); + expect(warnings.join('\n')).not.toContain('requirement "Renamed"'); + }); + it('does not warn when MODIFIED carries the full absorbed tail forward', async () => { const tail = [' ### Notes', 'Kept by hand.']; const { rebuilt, counts, warnings } = await build(SPEC(tail), [ From 91cc70e4417912e91c5ad7eddf3f8d6aa85cce76 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Mon, 24 Aug 2026 11:30:12 -0500 Subject: [PATCH 2/2] chore(archive): add rename-order changeset --- .changeset/calm-otters-order.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/calm-otters-order.md diff --git a/.changeset/calm-otters-order.md b/.changeset/calm-otters-order.md new file mode 100644 index 0000000000..63495e4e56 --- /dev/null +++ b/.changeset/calm-otters-order.md @@ -0,0 +1,5 @@ +--- +'@fission-ai/openspec': patch +--- + +archive: preserve a requirement's original position when renaming it instead of moving the renamed block to the end of the spec.