diff --git a/.changeset/bright-widgets-validate.md b/.changeset/bright-widgets-validate.md new file mode 100644 index 0000000000..9a689ee8b5 --- /dev/null +++ b/.changeset/bright-widgets-validate.md @@ -0,0 +1,7 @@ +--- +"@fission-ai/openspec": patch +--- + +Report delta merge conflicts during validation as informational findings, including in successful text reports, without changing validation exit codes. Preserve filesystem read errors so unreadable main specs are not mistaken for missing specs. + +Keep the validation report intact when the advisory merge preflight cannot resolve its inputs. diff --git a/docs-lab/reference/cli.md b/docs-lab/reference/cli.md index 65abd0b4b3..7bb72ac6f0 100644 --- a/docs-lab/reference/cli.md +++ b/docs-lab/reference/cli.md @@ -657,7 +657,7 @@ With no name and no bulk flag, validate prompts you to pick items. Outside an in **Output** -One line per item. Bulk runs end with totals: +Bulk runs print one status line per item, followed by any findings, and end with totals: ``` ✓ change/add-rate-limit @@ -665,6 +665,20 @@ One line per item. Bulk runs end with totals: Totals: 2 passed, 0 failed (2 items) ``` +**Archive merge findings** + +For changes, validate runs archive's merge builder against the current main specs without writing files. It reports merge conflicts, such as a missing `MODIFIED` target or a conflicting `ADDED` requirement, as `INFO`: + +```text +ℹ [INFO] api/spec.md: Archive would refuse this delta: api MODIFIED failed for header "### Requirement: Rate limiting" - not found +``` + +These findings appear even when validation passes, in both text and JSON output. `INFO` never changes the exit code, including under `--strict`: a missing target may belong to a sibling change that has not archived yet. Deltas already synced into the main specs follow archive's existing merge rules. + +This check does not run archive's later merged-spec validation or retirement checks. A clean report does not guarantee that archive will succeed. + +If the merge preflight cannot start, an `INFO` finding explains why. Existing validation findings and the exit code stay unchanged. + A failing item lists each issue and the fix: ``` diff --git a/src/commands/change.ts b/src/commands/change.ts index 437e2ce8e0..8cd23b3030 100644 --- a/src/commands/change.ts +++ b/src/commands/change.ts @@ -545,11 +545,12 @@ export class ChangeCommand { console.log(`Change "${changeName}" is valid`); } else { console.error(`Change "${changeName}" has issues`); - report.issues.forEach(issue => { - const label = issue.level === 'ERROR' ? 'ERROR' : 'WARNING'; - const prefix = issue.level === 'ERROR' ? '✗' : '⚠'; - console.error(`${prefix} [${label}] ${issue.path}: ${issue.message}`); - }); + } + report.issues.forEach(issue => { + const prefix = issue.level === 'ERROR' ? '✗' : issue.level === 'WARNING' ? '⚠' : 'ℹ'; + console.error(`${prefix} [${issue.level}] ${issue.path}: ${issue.message}`); + }); + if (!report.valid) { // Next steps footer to guide fixing issues this.printNextSteps(report.issues); if (!options?.json) { diff --git a/src/commands/validate.ts b/src/commands/validate.ts index 8f7428e647..d1320a8fb5 100644 --- a/src/commands/validate.ts +++ b/src/commands/validate.ts @@ -245,11 +245,12 @@ export class ValidateCommand { console.log(`${type === 'change' ? 'Change' : 'Specification'} '${id}' is valid`); } else { console.error(`${type === 'change' ? 'Change' : 'Specification'} '${id}' has issues`); - for (const issue of report.issues) { - const label = issue.level === 'ERROR' ? 'ERROR' : issue.level; - const prefix = issue.level === 'ERROR' ? '✗' : issue.level === 'WARNING' ? '⚠' : 'ℹ'; - console.error(`${prefix} [${label}] ${issue.path}: ${issue.message}`); - } + } + for (const issue of report.issues) { + const prefix = issue.level === 'ERROR' ? '✗' : issue.level === 'WARNING' ? '⚠' : 'ℹ'; + console.error(`${prefix} [${issue.level}] ${issue.path}: ${issue.message}`); + } + if (!report.valid) { this.printNextSteps(type, id, root, report.issues); } } @@ -394,6 +395,10 @@ export class ValidateCommand { for (const res of results) { if (res.valid) console.log(`✓ ${res.type}/${res.id}`); else console.error(`✗ ${res.type}/${res.id}`); + for (const issue of res.issues) { + const prefix = issue.level === 'ERROR' ? '✗' : issue.level === 'WARNING' ? '⚠' : 'ℹ'; + console.error(` ${prefix} [${issue.level}] ${issue.path}: ${issue.message}`); + } } console.log(`Totals: ${summary.totals.passed} passed, ${summary.totals.failed} failed (${summary.totals.items} items)`); const firstFailure = results.find((res) => !res.valid); diff --git a/src/core/specs-apply.ts b/src/core/specs-apply.ts index 81d5516cf4..821a7b2ca6 100644 --- a/src/core/specs-apply.ts +++ b/src/core/specs-apply.ts @@ -324,7 +324,11 @@ export async function buildUpdatedSpec( ); } } - } catch { + } catch (error) { + // An unreadable target is not a new spec. Preserve the filesystem error + // for callers, rather than synthesizing a baseline or a missing-target finding. + const code = (error as NodeJS.ErrnoException)?.code; + if (code !== 'ENOENT' && code !== 'ENOTDIR') throw error; // Target spec does not exist; MODIFIED and RENAMED are not allowed for new specs // REMOVED will be ignored with a warning since there's nothing to remove if (plan.modified.length > 0 || plan.renamed.length > 0) { diff --git a/src/core/validation/validator.ts b/src/core/validation/validator.ts index 2bcb75cc57..a36e4e75c8 100644 --- a/src/core/validation/validator.ts +++ b/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, @@ -151,9 +152,10 @@ export class Validator { * - No duplicates within sections; no cross-section conflicts per spec * * When `options.mainSpecsDir` is given, MODIFIED blocks are also checked - * against the current main specs for the scenario loss archive refuses to - * apply (#1477). When `options.projectRoot` is given, the schema's tracked - * task files are checked for ambiguous numbering (#1520). Omitting either + * against the current main specs for scenario loss (#1477), and merge + * conflicts are reported as INFO without changing the verdict (#1112). + * When `options.projectRoot` is given, the schema's tracked task files are + * checked for ambiguous numbering (#1520). Omitting either * option keeps existing library and archive callers behaving as before. */ async validateChangeDeltaSpecs( @@ -395,6 +397,23 @@ export class Validator { } } } + + // Reuse archive's merge builder to report conflicts with the main specs. + // Keep structural errors and scenario loss in their existing diagnostics. + 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 @@ -779,6 +798,70 @@ export class Validator { return dotIndex > 0 ? fileName.slice(0, dotIndex) : fileName; } + /** + * Dry-run archive's merge builder without writing its result. Reusing the + * builder preserves its already-synced delta rules instead of duplicating them. + * INFO leaves the verdict unchanged: a missing target can be a typo or a + * requirement introduced by a sibling change that has not archived yet. + * This does not run archive's later merged-spec validation or retirement checks. + */ + private async findArchiveBlockers( + changeDir: string, + mainSpecsDir: string, + alreadyReportedPaths: string[] + ): Promise { + 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[] = []; + + let updates: Awaited>; + try { + updates = await findSpecUpdates(changeDir, mainSpecsDir); + } catch (error) { + // An incomplete advisory check must not discard the validation report. + // Source discovery already ran above; archive retains its own path guards. + return [{ + level: 'INFO', + path: 'specs', + message: `Could not check archive merge conflicts: ${ + error instanceof Error ? error.message : String(error) + }`, + }]; + } + + for (const update of updates) { + // discoverSpecFiles builds both this id and the entryPath the checks + // above report under, from the same walk. + const entryPath = FileSystemUtils.toPosixPath(`${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; diff --git a/test/commands/validate.enriched-output.test.ts b/test/commands/validate.enriched-output.test.ts index 5ecb7c4903..6586203549 100644 --- a/test/commands/validate.enriched-output.test.ts +++ b/test/commands/validate.enriched-output.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { promises as fs } from 'fs'; import path from 'path'; import { execFileSync } from 'child_process'; +import { runCLI } from '../helpers/run-cli.js'; describe('validate command enriched human output', () => { const projectRoot = process.cwd(); @@ -18,6 +19,143 @@ describe('validate command enriched human output', () => { await fs.rm(testDir, { recursive: true, force: true }); }); + const writeArchiveBlocker = async () => { + const mainDir = path.join(testDir, 'openspec', 'specs', 'widgets'); + const changeDir = path.join(changesDir, 'c-archive'); + const deltaDir = path.join(changeDir, 'specs', 'widgets'); + await fs.mkdir(mainDir, { recursive: true }); + await fs.mkdir(deltaDir, { recursive: true }); + await fs.writeFile(path.join(mainDir, 'spec.md'), `# Widgets Specification + +## Purpose +Define how widgets report their existing state consistently to all callers. + +## Requirements + +### Requirement: Existing state +The system SHALL report the existing state. + +#### Scenario: Query state +- **WHEN** queried +- **THEN** the state is reported +`); + await fs.writeFile( + path.join(changeDir, 'proposal.md'), + '# Widget update\n\n## Why\nUpdate widgets.\n\n## What Changes\n- Update state reporting\n' + ); + await fs.writeFile(path.join(deltaDir, 'spec.md'), `## MODIFIED Requirements + +### Requirement: Future state +The system SHALL report the future state. + +#### Scenario: Query state +- **WHEN** queried +- **THEN** the state is reported +`); + }; + + const entryPoints = [ + ['validate', 'c-archive'], + ['change', 'validate', 'c-archive'], + ['validate', '--changes'], + ['validate', '--all'], + ]; + + for (const strict of [false, true]) { + for (const args of entryPoints) { + const invocation = [...args, ...(strict ? ['--strict'] : [])]; + + it(`shows non-blocking archive advice for ${invocation.join(' ')}`, async () => { + await writeArchiveBlocker(); + + const result = await runCLI([...invocation, '--no-interactive'], { cwd: testDir }); + + expect(result.exitCode).toBe(0); + expect(result.stderr).toContain('ℹ [INFO] widgets/spec.md: Archive would refuse this delta:'); + expect(result.stderr).toContain('Future state'); + expect(result.stderr).not.toContain('Next steps:'); + expect(result.stdout).toMatch(/is valid|0 failed/); + }); + + it(`keeps archive advice structured and non-blocking for ${invocation.join(' ')} --json`, async () => { + await writeArchiveBlocker(); + + const result = await runCLI([...invocation, '--json', '--no-interactive'], { cwd: testDir }); + + expect(result.exitCode).toBe(0); + const output = JSON.parse(result.stdout); + const report = args[0] === 'change' + ? output + : output.items.find((item: { id: string }) => item.id === 'c-archive'); + expect(report.valid).toBe(true); + expect(report.issues).toContainEqual(expect.objectContaining({ + level: 'INFO', + path: 'widgets/spec.md', + message: expect.stringContaining('Archive would refuse this delta:'), + })); + expect(result.stderr).not.toContain('Archive would refuse this delta:'); + if (args[0] !== 'change') expect(output.summary.totals.failed).toBe(0); + }); + } + } + + for (const args of [['validate', 'c-archive'], ['validate', '--changes']]) { + for (const json of [false, true]) { + it.skipIf(process.platform === 'win32')( + `reports an incomplete archive check without failing ${args.join(' ')}${json ? ' --json' : ''}`, + async () => { + await writeArchiveBlocker(); + const deltaFile = path.join(changesDir, 'c-archive', 'specs', 'widgets', 'spec.md'); + const delta = await fs.readFile(deltaFile, 'utf-8'); + await fs.writeFile(deltaFile, delta.replace('## MODIFIED Requirements', '## ADDED Requirements')); + const mainFile = path.join(testDir, 'openspec', 'specs', 'widgets', 'spec.md'); + const missingFile = path.join(testDir, 'missing-spec.md'); + await fs.unlink(mainFile); + await fs.symlink(missingFile, mainFile); + + const result = await runCLI( + [...args, '--strict', '--no-interactive', ...(json ? ['--json'] : [])], + { cwd: testDir } + ); + + if (json) { + const output = JSON.parse(result.stdout); + expect(output.items).toHaveLength(1); + expect(output.items[0].valid).toBe(true); + expect(output.items[0].issues).toContainEqual(expect.objectContaining({ + level: 'INFO', + path: 'specs', + message: expect.stringContaining('Could not check archive merge conflicts:'), + })); + expect(output.summary.totals).toEqual({ items: 1, passed: 1, failed: 0 }); + } else { + expect(result.stdout).toMatch(/is valid|0 failed/); + expect(result.stderr).toContain('ℹ [INFO] specs: Could not check archive merge conflicts:'); + expect(result.stderr).not.toContain('Next steps:'); + } + expect(result.exitCode).toBe(0); + } + ); + } + } + + it('preserves INFO severity in the deprecated command when another delta is invalid', async () => { + await writeArchiveBlocker(); + const invalidDir = path.join(changesDir, 'c-archive', 'specs', 'broken'); + await fs.mkdir(invalidDir, { recursive: true }); + await fs.writeFile( + path.join(invalidDir, 'spec.md'), + '## ADDED Requirements\n\n### Requirement: Missing scenario\nThe system SHALL do something.\n' + ); + + const result = await runCLI(['change', 'validate', 'c-archive', '--no-interactive'], { cwd: testDir }); + + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('ℹ [INFO] widgets/spec.md: Archive would refuse this delta:'); + expect(result.stderr).toContain('[ERROR]'); + expect(result.stderr).toContain('Next steps:'); + }); + it('prints Next steps footer and guidance on invalid change', async () => { const changeContent = `# Test Change\n\n## Why\nThis is a sufficiently long explanation to pass the why length requirement for validation purposes.\n\n## What Changes\nThere are changes proposed, but no delta specs provided yet.`; const changeId = 'c-next-steps'; diff --git a/test/core/validation.archive-preflight.test.ts b/test/core/validation.archive-preflight.test.ts new file mode 100644 index 0000000000..525fd86475 --- /dev/null +++ b/test/core/validation.archive-preflight.test.ts @@ -0,0 +1,383 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { promises as fs, realpathSync } 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). + * + * Compare findings against archive's merge builder, not its later validation + * and retirement checks. A delta the builder accepts must produce no finding. + * Reporting a change that merges 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 finding, 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's merge builder does: null when the delta applies cleanly. */ + const archiveError = async (changeDir: string): Promise => { + 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 () => { + vi.restoreAllMocks(); + await fs.rm(testDir, { recursive: true, force: true }); + }); + + it('keeps strict validation valid when advisory discovery encounters a filesystem error', async () => { + const changeDir = await writeChange('c1', 'widgets', `## ADDED Requirements\n\n${REQUIREMENT}\n`); + const specsDir = realpathSync.native(path.join(changeDir, 'specs')); + const readdir = fs.readdir; + let discoveries = 0; + vi.spyOn(fs, 'readdir').mockImplementation(async (dir, ...rest) => { + if (realpathSync.native(String(dir)) === specsDir && ++discoveries === 2) { + throw Object.assign(new Error('EIO: cannot discover archive inputs'), { code: 'EIO' }); + } + return readdir(dir, ...(rest as [])); + }); + + const report = await validate(changeDir, true); + expect(report.valid).toBe(true); + expect(report.issues).toContainEqual({ + level: 'INFO', + path: 'specs', + message: 'Could not check archive merge conflicts: EIO: cannot discover archive inputs', + }); + expect(blocker(report)).toBeUndefined(); + }); + + it.skipIf(process.platform === 'win32').each([ + ['outside', true], + ['outside', false], + ['dangling', true], + ['dangling', false], + ] as const)('preserves the validation report for a %s target link (valid delta: %s)', async (link, validDelta) => { + const body = validDelta ? REQUIREMENT : '### Requirement: Widget state\nThe system SHALL report the widget state.'; + const changeDir = await writeChange('c1', 'widgets', `## ADDED Requirements\n\n${body}\n`); + const target = path.join(mainSpecsDir, 'widgets', 'spec.md'); + const outside = path.join(testDir, 'outside.md'); + if (link === 'outside') await fs.writeFile(outside, mainSpec(REQUIREMENT)); + await fs.mkdir(path.dirname(target), { recursive: true }); + await fs.symlink(outside, target); + + // Advisory discovery must not weaken the merge path's security checks. + await expect(findSpecUpdates(changeDir, mainSpecsDir)).rejects.toThrow(); + for (const strict of [false, true]) { + const report = await validate(changeDir, strict); + expect(report.valid).toBe(validDelta); + expect(report.issues).toContainEqual(expect.objectContaining({ + level: 'INFO', + path: 'specs', + message: expect.stringContaining('Could not check archive merge conflicts:'), + })); + if (!validDelta) { + expect(report.issues).toContainEqual(expect.objectContaining({ + level: 'ERROR', path: 'widgets/spec.md', message: expect.stringContaining('must include at least one scenario'), + })); + } + } + if (link === 'outside') expect(await fs.readFile(outside, 'utf8')).toBe(mainSpec(REQUIREMENT)); + else await expect(fs.stat(outside)).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it.skipIf(process.platform === 'win32')('still refuses an unsafe delta source before the advisory check', async () => { + const changeDir = await writeChange('c1', 'widgets', `## ADDED Requirements\n\n${REQUIREMENT}\n`); + const delta = path.join(changeDir, 'specs', 'widgets', 'spec.md'); + const outside = path.join(testDir, 'outside-delta.md'); + await fs.rename(delta, outside); + await fs.symlink(outside, delta); + await expect(validate(changeDir)).rejects.toThrow('Path is outside the allowed directory'); + }); + + it.each(['EMFILE', 'EIO', 'EACCES'])( + 'does not misreport a target read failure (%s) as a missing requirement', + async (code) => { + await writeMainSpec('widgets', REQUIREMENT); + const changeDir = await writeChange('c1', 'widgets', `## MODIFIED Requirements\n\n${REQUIREMENT}\n`); + const [update] = await findSpecUpdates(changeDir, mainSpecsDir); + const target = realpathSync.native(update.target); + const readFile = fs.readFile; + const failure = Object.assign(new Error(`${code}: cannot read target`), { code }); + const spy = vi.spyOn(fs, 'readFile').mockImplementation(async (file, ...rest) => { + if (realpathSync.native(String(file)) === target) throw failure; + return readFile(file, ...(rest as [])); + }); + + const report = await validate(changeDir); + expect(spy.mock.calls.some(([file]) => realpathSync.native(String(file)) === target)).toBe(true); + expect(blocker(report)).toBeUndefined(); + await expect(buildUpdatedSpec(update, 'c1', { silent: true })).rejects.toBe(failure); + } + ); + + it.each([ + ['already-synced addition', `## ADDED Requirements\n\n${REQUIREMENT}\n`], + ['already-synced removal', '## REMOVED Requirements\n\n### Requirement: Gone\n'], + ])('stays silent on an %s', async (_name, delta) => { + await writeMainSpec('widgets', REQUIREMENT); + const changeDir = await writeChange('c1', 'widgets', delta); + expect(blocker(await validate(changeDir))).toBeUndefined(); + expect(await archiveError(changeDir)).toBeNull(); + }); + + it('reports a rename target collision', async () => { + await writeMainSpec('widgets', `${REQUIREMENT}\n\n${REQUIREMENT.replace('Widget state', 'Gadget state')}`); + const changeDir = await writeChange('c1', 'widgets', '## RENAMED Requirements\n\n- FROM: `### Requirement: Widget state`\n- TO: `### Requirement: Gadget state`\n'); + const error = await archiveError(changeDir); + expect(error).toContain('already exists'); + expect(blocker(await validate(changeDir))?.message).toBe(`Archive would refuse this delta: ${error}`); + }); + + it.each([ + ['MODIFIED', `## MODIFIED Requirements\n\n${REQUIREMENT}\n`], + ['RENAMED', '## RENAMED Requirements\n\n- FROM: `### Requirement: Widget state`\n- TO: `### Requirement: Gadget state`\n'], + ])('reports %s against a capability that does not exist', async (_operation, delta) => { + const changeDir = await writeChange('c1', 'new-capability', delta); + const error = await archiveError(changeDir); + expect(error).toContain('target spec does not exist'); + expect(blocker(await validate(changeDir))?.message).toBe(`Archive would refuse this delta: ${error}`); + }); + + it('keeps library validation unchanged when mainSpecsDir is omitted', async () => { + const changeDir = await writeChange('c1', 'widgets', `## MODIFIED Requirements\n\n${REQUIREMENT}\n`); + const report = await new Validator(true).validateChangeDeltaSpecs(changeDir); + expect(report.valid).toBe(true); + expect(blocker(report)).toBeUndefined(); + }); + + it('does not synthesize a new baseline for ADDED when the existing spec cannot be read through an alias or canonical path', async () => { + await writeMainSpec('widgets', REQUIREMENT); + await fs.symlink( + path.join(mainSpecsDir, 'widgets'), + path.join(mainSpecsDir, 'widgets-alias'), + process.platform === 'win32' ? 'junction' : 'dir' + ); + const changeDir = await writeChange('c1', 'widgets-alias', `## ADDED Requirements\n\n${REQUIREMENT.replace('Widget state', 'Gadget state')}\n`); + const [update] = await findSpecUpdates(changeDir, mainSpecsDir); + const target = realpathSync.native(update.target); + // Keep distinct path spellings so this exercises both reads of the same file. + expect(update.target).not.toBe(target); + const readFile = fs.readFile; + const failure = Object.assign(new Error('EIO: cannot read target'), { code: 'EIO' }); + vi.spyOn(fs, 'readFile').mockImplementation(async (file, ...rest) => { + if (realpathSync.native(String(file)) === target) throw failure; + return readFile(file, ...(rest as [])); + }); + await expect(buildUpdatedSpec(update, 'c1', { silent: true })).rejects.toBe(failure); + await expect(buildUpdatedSpec({ ...update, target }, 'c1', { silent: true })).rejects.toBe(failure); + }); + + it('reports a nested capability without suppressing findings for other files', async () => { + await writeMainSpec('area/widgets', REQUIREMENT); + const changeDir = await writeChange('c1', 'area/widgets', `## MODIFIED Requirements\n\n${REQUIREMENT.replace('Widget state', 'Missing')}\n`); + await writeChange('c1', 'invalid', '## ADDED Requirements\n\nNo entries.\n'); + const report = await validate(changeDir); + expect(report.issues.filter((issue) => issue.level === 'INFO')).toEqual([ + expect.objectContaining({ path: 'area/widgets/spec.md', message: `Archive would refuse this delta: ${await archiveError(changeDir)}` }), + ]); + }); + + it('does not create or rewrite spec files or print merge warnings', async () => { + await writeMainSpec('widgets', REQUIREMENT); + const changeDir = await writeChange('c1', 'widgets', `## ADDED Requirements\n\n${REQUIREMENT}\n`); + await writeChange('c1', 'new-capability', `## ADDED Requirements\n\n${REQUIREMENT}\n`); + const mainFile = path.join(mainSpecsDir, 'widgets', 'spec.md'); + const deltaFile = path.join(changeDir, 'specs', 'widgets', 'spec.md'); + const before = await Promise.all([fs.readFile(mainFile, 'utf8'), fs.readFile(deltaFile, 'utf8')]); + const log = vi.spyOn(console, 'log').mockImplementation(() => {}); + await validate(changeDir); + expect(await Promise.all([fs.readFile(mainFile, 'utf8'), fs.readFile(deltaFile, 'utf8')])).toEqual(before); + await expect(fs.stat(path.join(mainSpecsDir, 'new-capability'))).rejects.toMatchObject({ code: 'ENOENT' }); + expect(log).not.toHaveBeenCalled(); + }); + + 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.skipIf(process.platform === 'win32')('uses the same display path when suppressing malformed deltas with a literal backslash', async () => { + const changeDir = await writeChange('c1', 'area\\widgets', '# Notes\n\nNo delta headers.\n'); + const report = await validate(changeDir); + expect(report.issues).toContainEqual(expect.objectContaining({ + level: 'ERROR', path: 'area/widgets/spec.md', message: expect.stringContaining('No delta sections found'), + })); + 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(); + }); +});