feat(validate): report the deltas archive would refuse

docs-lab/reference/cli.md

@@ -657,6 +657,14 @@ One line per item. Bulk runs end with totals:
 Totals: 2 passed, 0 failed (2 items)
 ```
 
+When a change's deltas are validated against the main specs, validate also runs the merge archive would run and reports anything that merge refuses — a `MODIFIED` naming a requirement the main spec does not have, a `RENAMED` whose source is gone, an `ADDED` whose name already exists. Without this the delta only fails at `openspec archive`, often weeks after the implementing PR shipped.
+
+These are reported as `INFO` and never change the exit code, including under `--strict`. The same shape has two causes: a mistyped header, and a change modifying a requirement a sibling change introduced but has not archived yet. The second becomes applicable the moment that sibling lands, so the report tells you the collision exists and leaves the verdict to you.
+
+```text
+ℹ [INFO] api/spec.md: Archive would refuse this delta: api MODIFIED failed for header "### Requirement: Rate limiting" - not found
+```
+
 A failing item lists each issue and the fix:
 
 ```

src/core/validation/validator.ts

@@ -5,6 +5,7 @@ import { SpecSchema, ChangeSchema, Spec, Change } from '../schemas/index.js';
 import { MarkdownParser } from '../parsers/markdown-parser.js';
 import { ChangeParser } from '../parsers/change-parser.js';
 import { ValidationReport, ValidationIssue, ValidationLevel } from './types.js';
+import { findSpecUpdates, buildUpdatedSpec } from '../specs-apply.js';
 import {
   MIN_PURPOSE_LENGTH,
   MAX_REQUIREMENT_TEXT_LENGTH,
@@ -394,6 +395,27 @@ export class Validator {
           }
         }
       }
+
+      // Everything above checks the delta against itself. Archive additionally
+      // refuses a delta the main spec cannot supply a target for - a MODIFIED
+      // naming a requirement that is not there, a RENAMED whose source is
+      // gone, an ADDED whose name already exists - and validate has never
+      // looked at any of those, so they surface at archive time, typically
+      // weeks after the implementing PR shipped (#1112).
+      if (options.mainSpecsDir) {
+        issues.push(
+          ...(await this.findArchiveBlockers(changeDir, options.mainSpecsDir, [
+            ...issues.filter((issue) => issue.level === 'ERROR').map((issue) => issue.path),
+            // Collected in the loop above but not turned into issues until
+            // after this try block, so they are invisible to the filter. A
+            // delta with no parsed sections has nothing for the merge to
+            // apply, which it reports as a failure of its own - on top of the
+            // error that actually names the mistake.
+            ...missingHeaderSpecs,
+            ...emptySectionSpecs.map((spec) => spec.path),
+          ]))
+        );
+      }
     } catch (error) {
       // A missing specs dir (or a stray `specs` file) means no deltas;
       // anything else (EACCES, EIO) must stay loud — discoverSpecFiles
@@ -765,6 +787,74 @@ export class Validator {
     return dotIndex > 0 ? fileName.slice(0, dotIndex) : fileName;
   }
 
+  /**
+   * What archive would refuse when it merges this change's deltas into the
+   * main specs, found by running that merge in memory and throwing the result
+   * away.
+   *
+   * The preconditions are not restated here on purpose. `buildUpdatedSpec` is
+   * the code that does the writing, and several of its rules deliberately read
+   * a missing target as already-synced rather than as a failure (a RENAMED
+   * whose source is gone but whose target is present, for one). A second model
+   * of those rules would be free to disagree with the one that decides, and a
+   * preflight that reports a change archive accepts is worse than no
+   * preflight. So this calls it: same function, same inputs archive gives it,
+   * `rebuilt` discarded. It writes nothing.
+   *
+   * Reported as INFO, so it never changes a verdict in any mode, because the
+   * same shape has two causes. A MODIFIED whose target is missing is a real
+   * blocker when the header is a typo, but it is also what a change looks like
+   * when it modifies a requirement a sibling change introduced and has not
+   * archived yet - and that one becomes applicable the moment the sibling
+   * lands. `validate` deliberately stays valid for that case today, and
+   * deciding which of the two a delta is needs the opt-in marker #1112 asks
+   * for, not a guess made here. What is missing until then is not a verdict,
+   * it is the information: the author cannot currently see the collision at
+   * all until archive refuses it weeks later. This says it, and leaves the
+   * verdict alone.
+   */
+  private async findArchiveBlockers(
+    changeDir: string,
+    mainSpecsDir: string,
+    alreadyReportedPaths: string[]
+  ): Promise<ValidationIssue[]> {
+    const alreadyReported = new Set(alreadyReportedPaths);
+    // Only ever reaches a generated skeleton's placeholder Purpose, which this
+    // dry run discards.
+    const changeName = path.basename(changeDir);
+    const issues: ValidationIssue[] = [];
+
+    for (const update of await findSpecUpdates(changeDir, mainSpecsDir)) {
+      // discoverSpecFiles builds both this id and the entryPath the checks
+      // above report under, from the same walk.
+      const entryPath = `${update.id}/spec.md`;
+      // A delta those checks already rejected would be reported twice, the
+      // second time in archive's wording rather than the wording that names
+      // the actual mistake.
+      if (alreadyReported.has(entryPath)) continue;
+
+      try {
+        await buildUpdatedSpec(update, changeName, { silent: true });
+      } catch (error) {
+        // Only the thrown preconditions, which carry no errno. A filesystem
+        // error says nothing about whether the delta applies, and `validate
+        // --all` reads six changes at once, so a transient EMFILE would report
+        // a collision that is not there - the same reason the scenario-loss
+        // check above reads only the codes that mean the file is unusable.
+        if ((error as NodeJS.ErrnoException)?.code !== undefined) continue;
+        issues.push({
+          level: 'INFO',
+          path: entryPath,
+          message: `Archive would refuse this delta: ${
+            error instanceof Error ? error.message : String(error)
+          }`,
+        });
+      }
+    }
+
+    return issues;
+  }
+
   private createReport(issues: ValidationIssue[]): ValidationReport {
     const errors = issues.filter(i => i.level === 'ERROR').length;
     const warnings = issues.filter(i => i.level === 'WARNING').length;

test/core/validation.archive-preflight.test.ts

@@ -0,0 +1,206 @@
+import { describe, it, expect, beforeEach, afterEach } from 'vitest';
+import { promises as fs } from 'fs';
+import os from 'os';
+import path from 'path';
+import { Validator } from '../../src/core/validation/validator.js';
+import { buildUpdatedSpec, findSpecUpdates } from '../../src/core/specs-apply.js';
+
+/**
+ * validate reports the deltas archive would refuse to apply (#1112).
+ *
+ * Every test here asserts parity in both directions: a warning validate emits
+ * must correspond to an error archive actually throws, and a change archive
+ * accepts must produce no warning. Reporting a change that archives cleanly
+ * would send an author to rewrite working work, which is worse than the gap
+ * this closes.
+ */
+describe('validate: deltas archive would refuse (#1112)', () => {
+  let testDir: string;
+  let changesDir: string;
+  let mainSpecsDir: string;
+
+  const REQUIREMENT = `### Requirement: Widget state\nThe system SHALL report the widget state.\n\n#### Scenario: Existing scenario\n- **WHEN** queried\n- **THEN** the state is reported`;
+
+  const mainSpec = (body: string) =>
+    `# widgets Specification\n\n## Purpose\nDefine widget behavior for these tests.\n\n## Requirements\n\n${body}\n`;
+
+  const writeMainSpec = async (id: string, body: string) => {
+    const file = path.join(mainSpecsDir, ...id.split('/'), 'spec.md');
+    await fs.mkdir(path.dirname(file), { recursive: true });
+    await fs.writeFile(file, mainSpec(body));
+  };
+
+  const writeChange = async (changeName: string, specId: string, delta: string) => {
+    const changeDir = path.join(changesDir, changeName);
+    const specDir = path.join(changeDir, 'specs', ...specId.split('/'));
+    await fs.mkdir(specDir, { recursive: true });
+    await fs.writeFile(path.join(specDir, 'spec.md'), delta);
+    return changeDir;
+  };
+
+  const validate = (changeDir: string, strict = false) =>
+    new Validator(strict).validateChangeDeltaSpecs(changeDir, { mainSpecsDir });
+
+  /** The preflight warning, so assertions cannot pass on an unrelated issue. */
+  const blocker = (report: { issues: Array<{ level: string; message: string }> }) =>
+    report.issues.find((i) => i.message.startsWith('Archive would refuse this delta:'));
+
+  /** What archive does with the same change: null when it applies cleanly. */
+  const archiveError = async (changeDir: string): Promise<string | null> => {
+    for (const update of await findSpecUpdates(changeDir, mainSpecsDir)) {
+      try {
+        await buildUpdatedSpec(update, path.basename(changeDir), { silent: true });
+      } catch (error) {
+        return error instanceof Error ? error.message : String(error);
+      }
+    }
+    return null;
+  };
+
+  beforeEach(async () => {
+    testDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-preflight-'));
+    changesDir = path.join(testDir, 'openspec', 'changes');
+    mainSpecsDir = path.join(testDir, 'openspec', 'specs');
+    await fs.mkdir(changesDir, { recursive: true });
+    await fs.mkdir(mainSpecsDir, { recursive: true });
+  });
+
+  afterEach(async () => {
+    await fs.rm(testDir, { recursive: true, force: true });
+  });
+
+  it('reports a MODIFIED naming a requirement the main spec does not have', async () => {
+    await writeMainSpec('widgets', REQUIREMENT);
+    const changeDir = await writeChange(
+      'c1',
+      'widgets',
+      `## MODIFIED Requirements\n\n### Requirement: Gadget state\nThe system SHALL report the gadget state.\n\n#### Scenario: Queried\n- **WHEN** queried\n- **THEN** reported\n`
+    );
+
+    const issue = blocker(await validate(changeDir));
+    expect(issue?.message).toContain('MODIFIED failed for header "### Requirement: Gadget state"');
+    expect(await archiveError(changeDir)).not.toBeNull();
+  });
+
+  it('reports an ADDED whose requirement already exists in the main spec', async () => {
+    await writeMainSpec('widgets', REQUIREMENT);
+    const changeDir = await writeChange(
+      'c1',
+      'widgets',
+      `## ADDED Requirements\n\n### Requirement: Widget state\nThe system SHALL report the widget state twice.\n\n#### Scenario: Queried\n- **WHEN** queried\n- **THEN** reported\n`
+    );
+
+    expect(blocker(await validate(changeDir))?.message).toContain('already exists');
+    expect(await archiveError(changeDir)).not.toBeNull();
+  });
+
+  it('reports a RENAMED whose source is not in the main spec', async () => {
+    await writeMainSpec('widgets', REQUIREMENT);
+    const changeDir = await writeChange(
+      'c1',
+      'widgets',
+      `## RENAMED Requirements\n\n- FROM: \`### Requirement: Gadget state\`\n- TO: \`### Requirement: Doodad state\`\n`
+    );
+
+    expect(blocker(await validate(changeDir))?.message).toContain('source not found');
+    expect(await archiveError(changeDir)).not.toBeNull();
+  });
+
+  it('stays silent on a delta that applies cleanly', async () => {
+    await writeMainSpec('widgets', REQUIREMENT);
+    const changeDir = await writeChange(
+      'c1',
+      'widgets',
+      `## ADDED Requirements\n\n### Requirement: Gadget state\nThe system SHALL report the gadget state.\n\n#### Scenario: Queried\n- **WHEN** queried\n- **THEN** reported\n`
+    );
+
+    expect(blocker(await validate(changeDir))).toBeUndefined();
+    expect(await archiveError(changeDir)).toBeNull();
+  });
+
+  it('stays silent on a rename the baseline already absorbed', async () => {
+    // Source gone, target present: specs-apply reads this as an early-synced
+    // rename and applies it as a no-op. A preflight with its own copy of the
+    // rules would call it a missing source and fail a change that archives.
+    await writeMainSpec('widgets', REQUIREMENT.replace('Widget state', 'Doodad state'));
+    const changeDir = await writeChange(
+      'c1',
+      'widgets',
+      `## RENAMED Requirements\n\n- FROM: \`### Requirement: Widget state\`\n- TO: \`### Requirement: Doodad state\`\n`
+    );
+
+    expect(blocker(await validate(changeDir))).toBeUndefined();
+    expect(await archiveError(changeDir)).toBeNull();
+  });
+
+  it('stays silent when the capability is new, so there is nothing to apply against', async () => {
+    const changeDir = await writeChange(
+      'c1',
+      'gizmos',
+      `## ADDED Requirements\n\n### Requirement: Gizmo state\nThe system SHALL report the gizmo state.\n\n#### Scenario: Queried\n- **WHEN** queried\n- **THEN** reported\n`
+    );
+
+    expect(blocker(await validate(changeDir))).toBeUndefined();
+    expect(await archiveError(changeDir)).toBeNull();
+  });
+
+  it('reports without changing the verdict, in strict mode too', async () => {
+    await writeMainSpec('widgets', REQUIREMENT);
+    const changeDir = await writeChange(
+      'c1',
+      'widgets',
+      `## MODIFIED Requirements\n\n### Requirement: Gadget state\nThe system SHALL report the gadget state.\n\n#### Scenario: Queried\n- **WHEN** queried\n- **THEN** reported\n`
+    );
+
+    // The same shape is a typo'd header and a change modifying a sibling's
+    // unarchived requirement, and validate stays valid for the second one
+    // today. Telling the two apart needs the opt-in marker #1112 asks for, so
+    // this reports the collision and leaves the verdict where it was.
+    for (const strict of [false, true]) {
+      const report = await validate(changeDir, strict);
+      expect(report.valid).toBe(true);
+      expect(blocker(report)?.level).toBe('INFO');
+    }
+  });
+
+  it('does not restate a delta with no parsed sections, reported after the loop', async () => {
+    // missingHeaderSpecs / emptySectionSpecs are collected inside the loop but
+    // their errors are pushed after it, so a preflight keyed on issues raised
+    // so far would not see them and would add a second finding for a file the
+    // validator is about to name properly.
+    await writeMainSpec('widgets', REQUIREMENT);
+    const changeDir = await writeChange('c1', 'widgets', '# notes\n\nNo delta headers here.\n');
+
+    const report = await validate(changeDir);
+    expect(report.issues.some((i) => i.message.startsWith('No delta sections found'))).toBe(true);
+    expect(blocker(report)).toBeUndefined();
+  });
+
+  it('does not restate a section that parsed no requirement entries', async () => {
+    await writeMainSpec('widgets', REQUIREMENT);
+    const changeDir = await writeChange('c1', 'widgets', '## ADDED Requirements\n\nNothing here.\n');
+
+    const report = await validate(changeDir);
+    expect(report.issues.some((i) => i.message.includes('no requirement entries parsed'))).toBe(true);
+    expect(blocker(report)).toBeUndefined();
+  });
+
+  it('does not restate a failure the delta checks already named', async () => {
+    // The scenario-loss check reports this one in wording that names the
+    // dropped scenario; buildUpdatedSpec throws on it too, a few steps later.
+    await writeMainSpec(
+      'widgets',
+      `${REQUIREMENT}\n\n#### Scenario: Second scenario\n- **WHEN** idle\n- **THEN** idle is reported`
+    );
+    const changeDir = await writeChange(
+      'c1',
+      'widgets',
+      `## MODIFIED Requirements\n\n### Requirement: Widget state\nThe system SHALL report the widget state.\n\n#### Scenario: Existing scenario\n- **WHEN** queried\n- **THEN** the state is reported\n`
+    );
+
+    const report = await validate(changeDir);
+    expect(report.issues.some((i) => i.level === 'ERROR')).toBe(true);
+    expect(blocker(report)).toBeUndefined();
+    expect(await archiveError(changeDir)).not.toBeNull();
+  });
+});