fix(specs): warn before archiving deletes a note next to a requirement

.changeset/indented-atx-headings.md

@@ -0,0 +1,5 @@
+---
+"@fission-ai/openspec": patch
+---
+
+Say before confirmation when archiving a change will delete a note written next to a requirement. A requirement absorbs anything below it that OpenSpec doesn't recognize as a new heading — a note indented by the one to three spaces Markdown allows, for example — so removing or modifying that requirement took the note with it, silently. `openspec archive` now names content the rebuilt spec would actually drop and where to move it to keep it. The merge itself is unchanged: nothing is relocated, because a `#` line inside a scenario looks identical to a note and moving one of those would rewrite the spec wrongly.

src/core/archive.ts

@@ -558,6 +558,32 @@ export class ArchiveCommand {
           }
         }
 
+        // Build the proposed updates before asking permission to apply them.
+        // buildUpdatedSpec also reports content that the merge would drop, so
+        // the confirmation must come after this preview.
+        const prepared: Array<{
+          update: SpecUpdate;
+          rebuilt: string;
+          counts: { added: number; modified: number; removed: number; renamed: number };
+        }> = [];
+        let prepareError: unknown;
+        try {
+          for (const update of specUpdates) {
+            const built = await buildUpdatedSpec(update, changeName!, { silent: true });
+            prepared.push({ update, rebuilt: built.rebuilt, counts: built.counts });
+            specWarnings.push(...built.warnings);
+          }
+        } catch (err: unknown) {
+          // A user may still decline spec updates and archive the change, as
+          // before this preview existed. Defer the error until they accept.
+          prepareError = err;
+        }
+        if (prepareError === undefined && !json) {
+          for (const warning of specWarnings) {
+            console.log(chalk.yellow(`⚠️  Warning: ${warning}`));
+          }
+        }
+
         let shouldUpdateSpecs = true;
         if (!options.yes) {
           if (json) {
@@ -585,32 +611,24 @@ export class ArchiveCommand {
         }
 
         if (shouldUpdateSpecs) {
-          // Prepare all updates first (validation pass, no writes)
-          const prepared: Array<{ update: SpecUpdate; rebuilt: string; counts: { added: number; modified: number; removed: number; renamed: number } }> = [];
-          try {
-            for (const update of specUpdates) {
-              const built = await buildUpdatedSpec(update, changeName!, { silent: json });
-              prepared.push({ update, rebuilt: built.rebuilt, counts: built.counts });
-              // Carried into the result so JSON mode (where nothing was
-              // printed) still surfaces them; human mode discards the result.
-              specWarnings.push(...built.warnings);
-            }
-          } catch (err: any) {
+          if (prepareError !== undefined) {
+            const message =
+              prepareError instanceof Error ? prepareError.message : String(prepareError);
             if (json) {
               throw new ArchiveBlockedError(
                 'archive_spec_update_failed',
-                String(err.message || err),
+                message,
                 'Fix the change delta specs and rerun. No files were changed.'
               );
             }
-            console.log(String(err.message || err));
+            console.log(message);
             console.log('Aborted. No files were changed.');
             process.exitCode = 1;
             return null;
           }
 
-          // Validate every rebuilt spec before writing any of them, so a
-          // late validation failure really does leave all targets unchanged.
+          // Validate every rebuilt spec before writing any of them, so a late
+          // validation failure really does leave all targets unchanged.
           if (!skipValidation) {
             for (const p of prepared) {
               const specName = p.update.id;

src/core/specs-apply.ts

@@ -282,6 +282,7 @@ export async function buildUpdatedSpec(
   // 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);
@@ -319,6 +320,7 @@ export async function buildUpdatedSpec(
     };
     nameToBlock.delete(from);
     nameToBlock.set(to, renamedBlock);
+    renamedTargets.set(from, to);
     renamedApplied++;
   }
 
@@ -411,6 +413,31 @@ export async function buildUpdatedSpec(
       keptOrder.push(replacement);
       seen.add(key);
     }
+    // A block's raw runs to the next header the parser RECOGNISES, so a note
+    // under an unrecognized heading can be absorbed into the requirement.
+    // Warn only when the replacement from this same original block drops the
+    // 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);
+    if (replacementFromOriginal !== block) {
+      const foreign = firstForeignTail(block.raw);
+      const replacementRaw = replacementFromOriginal?.raw;
+      const normalizedForeign = foreign ? normalizeBlockRaw(foreign.raw) : '';
+      const keepsForeignTail =
+        foreign !== undefined &&
+        replacementRaw !== undefined &&
+        countOccurrences(normalizeBlockRaw(replacementRaw), normalizedForeign) >=
+          countOccurrences(normalizeBlockRaw(block.raw), normalizedForeign);
+      if (foreign && !keepsForeignTail) {
+        warn(
+          `${specName} - "${foreign.heading}" sits inside requirement "${block.name}" and goes with it. ` +
+            'Move it under its own requirement, or above `## Requirements`, to keep it.'
+        );
+      }
+    }
   }
   // Append any newly added that were not in original order
   for (const [key, block] of nameToBlock.entries()) {
@@ -442,10 +469,50 @@ export async function buildUpdatedSpec(
   };
 }
 
+/**
+ * The suffix of a requirement block that begins with content the requirement
+ * parser did not recognize as a boundary: a `#`, `##`, or `###` heading after
+ * the block's own header.
+ *
+ * `####` is excluded: a requirement's `#### Scenario:` headings are its own.
+ * Fenced lines are skipped, so a heading inside an example does not count.
+ *
+ * Approximate on purpose, and only ever used to WARN. A `#` line inside a
+ * scenario looks the same as a note written below the requirement, and no
+ * line-based rule separates them; a wrong warning costs a line of output, while
+ * acting on a wrong answer would rewrite the spec.
+ */
+function firstForeignTail(raw: string): { heading: string; raw: string } | undefined {
+  const lines = raw.replace(/\r\n?/g, '\n').split('\n');
+  const fenceMask = buildCodeFenceMask(lines);
+  for (let index = 1; index < lines.length; index++) {
+    if (fenceMask[index]) continue;
+    if (/^ {0,3}#{1,3}(?:[ \t]|$)/.test(lines[index])) {
+      return {
+        heading: lines[index].trim(),
+        raw: lines.slice(index).join('\n').trimEnd(),
+      };
+    }
+  }
+  return undefined;
+}
+
 function normalizeBlockRaw(raw: string): string {
   return raw.replace(/\r\n?/g, '\n').trim();
 }
 
+/** Count non-overlapping copies so one retained duplicate cannot mask another copy's loss. */
+function countOccurrences(haystack: string, needle: string): number {
+  if (!needle) return 0;
+  let count = 0;
+  let start = 0;
+  while ((start = haystack.indexOf(needle, start)) !== -1) {
+    count++;
+    start += needle.length;
+  }
+  return count;
+}
+
 /**
  * Write an updated spec to disk.
  */
@@ -567,4 +634,3 @@ export function buildSpecSkeleton(specFolderName: string, changeName: string, pu
     purpose?.trim() || `TBD - created by archiving change ${changeName}. Update Purpose after archive.`;
   return `# ${titleBase} Specification\n\n## Purpose\n${purposeBody}\n\n## Requirements\n`;
 }
-

test/core/archive.test.ts

@@ -1842,6 +1842,132 @@ Then expected result happens`;
       expect(archives[0]).toMatch(new RegExp(`\\d{4}-\\d{2}-\\d{2}-${changeName}`));
     });
 
+    it('warns about absorbed content before asking to apply the destructive spec update', async () => {
+      const { confirm } = await import('@inquirer/prompts');
+      const mockConfirm = confirm as unknown as ReturnType<typeof vi.fn>;
+      const changeName = 'warn-before-spec-update';
+      const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
+      const changeSpecDir = path.join(changeDir, 'specs', 'demo');
+      const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'demo');
+      await fs.mkdir(changeSpecDir, { recursive: true });
+      await fs.mkdir(mainSpecDir, { recursive: true });
+
+      const mainSpec = `# demo Specification
+
+## Purpose
+This capability exists to exercise archive warning behavior.
+
+## Requirements
+
+### Requirement: Target
+The system SHALL target.
+
+#### Scenario: Target works
+- **WHEN** it runs
+- **THEN** it works
+
+   ### Notes
+Keep this note.
+
+### Requirement: Survivor
+The system SHALL survive.
+
+#### Scenario: Survivor works
+- **WHEN** it runs
+- **THEN** it survives
+`;
+      await fs.writeFile(path.join(mainSpecDir, 'spec.md'), mainSpec);
+      await fs.writeFile(
+        path.join(changeSpecDir, 'spec.md'),
+        `# demo - Changes
+
+## REMOVED Requirements
+
+### Requirement: Target
+**Reason**: It is obsolete.
+`
+      );
+
+      mockConfirm.mockReset();
+      mockConfirm.mockImplementationOnce(async () => {
+        expect(console.log).toHaveBeenCalledWith(
+          expect.stringContaining('"### Notes" sits inside requirement "Target"')
+        );
+        return false;
+      });
+
+      await archiveCommand.execute(changeName);
+
+      expect(mockConfirm).toHaveBeenCalledWith({
+        message: 'Proceed with spec updates?',
+        default: true,
+      });
+      await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.toBe(mainSpec);
+      await expect(fs.access(changeDir)).rejects.toThrow();
+    });
+
+    it('prints the loss warning before --yes writes the spec', async () => {
+      const changeName = 'warn-before-yes-write';
+      const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);
+      const changeSpecDir = path.join(changeDir, 'specs', 'demo');
+      const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'demo');
+      await fs.mkdir(changeSpecDir, { recursive: true });
+      await fs.mkdir(mainSpecDir, { recursive: true });
+      await fs.writeFile(
+        path.join(mainSpecDir, 'spec.md'),
+        `# demo Specification
+
+## Purpose
+This capability exists to exercise archive warning behavior.
+
+## Requirements
+
+### Requirement: Target
+The system SHALL target.
+
+#### Scenario: Target works
+- **WHEN** it runs
+- **THEN** it works
+
+   ### Notes
+Keep this note.
+
+### Requirement: Survivor
+The system SHALL survive.
+
+#### Scenario: Survivor works
+- **WHEN** it runs
+- **THEN** it survives
+`
+      );
+      await fs.writeFile(
+        path.join(changeSpecDir, 'spec.md'),
+        `# demo - Changes
+
+## REMOVED Requirements
+
+### Requirement: Target
+**Reason**: It is obsolete.
+`
+      );
+
+      await archiveCommand.execute(changeName, { yes: true });
+
+      const output = (
+        console.log as unknown as { mock: { calls: unknown[][] } }
+      ).mock.calls.flat().map(String);
+      const warningIndex = output.findIndex((line) =>
+        line.includes('"### Notes" sits inside requirement "Target"')
+      );
+      const successIndex = output.indexOf('Specs updated successfully.');
+      expect(warningIndex).toBeGreaterThanOrEqual(0);
+      expect(successIndex).toBeGreaterThan(warningIndex);
+      await expect(fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8')).resolves.not.toContain(
+        'Keep this note.'
+      );
+      await expect(fs.access(changeDir)).rejects.toThrow();
+    });
+
     it('should support header trim-only normalization for matching', async () => {
       const changeName = 'normalize-headers';
       const changeDir = path.join(tempDir, 'openspec', 'changes', changeName);