fix(archive): preserve requirement order when renaming (#1712)

* fix(archive): preserve requirement order when renaming

* chore(archive): add rename-order changeset

---------

Co-authored-by: Clay Good <hi@claygood.com>

Author: Marzx13

.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.

src/core/specs-apply.ts

@@ -375,11 +375,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<string, string>();
   for (const r of plan.renamed) {
     const from = normalizeRequirementName(r.from);
     const to = normalizeRequirementName(r.to);
@@ -417,7 +419,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++;
   }
 
@@ -503,8 +510,9 @@ export async function buildUpdatedSpec(
   // Recompose requirements section preserving original ordering where possible
   const keptOrder: RequirementBlock[] = [];
   const seen = new Set<string>();
-  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);
@@ -516,9 +524,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;

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);

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), [