diff --git a/.changeset/reject-unread-delta-files.md b/.changeset/reject-unread-delta-files.md new file mode 100644 index 0000000000..3051a95145 --- /dev/null +++ b/.changeset/reject-unread-delta-files.md @@ -0,0 +1,5 @@ +--- +'@fission-ai/openspec': patch +--- + +Stop archiving a change whose delta was written somewhere `archive` never reads. `validate` and `archive` read a change's deltas only from `specs//spec.md`, but the spec-driven artifact graph counts any markdown file under `specs/` as the specs being written, so a delta at `specs/user-auth.md`, or in a second file beside a capability's `spec.md`, was reported done by `status` and ready by `instructions apply` with no warning, rejected by `validate` only as "no deltas found", and then archived with exit 0 and nothing merged into `openspec/specs/`. A markdown file that carries delta sections but is not a capability's `spec.md` is now a validation error naming the file and the `spec.md` its requirements belong in; `archive` runs that validation and refuses the change instead of archiving it unmerged, and `instructions apply` lists each such file in its `warnings`. `--no-validate` still archives as before, a change with no spec files still archives, and notes without delta sections under `specs/` are not affected. diff --git a/docs-lab/reference/schemas/spec-driven/index.md b/docs-lab/reference/schemas/spec-driven/index.md index be7f1c4c61..e16dec2dc8 100644 --- a/docs-lab/reference/schemas/spec-driven/index.md +++ b/docs-lab/reference/schemas/spec-driven/index.md @@ -122,6 +122,8 @@ This is the foundation - specs, design, and tasks all build on this. Defines what behavior changes, with one delta spec per capability the proposal lists. +Each delta spec is the `spec.md` inside its capability folder. `openspec validate` and `openspec archive` reject delta sections written in any other file under `specs/`, such as `specs/user-auth.md`, because archive never merges them. + ### Structure The template the agent receives as the output format ([templates/spec.md](https://github.com/Fission-AI/OpenSpec/blob/main/schemas/spec-driven/templates/spec.md)): diff --git a/src/commands/workflow/instructions.ts b/src/commands/workflow/instructions.ts index cc2b143bb2..aff8968c1f 100644 --- a/src/commands/workflow/instructions.ts +++ b/src/commands/workflow/instructions.ts @@ -17,6 +17,7 @@ import { type ArtifactInstructions, } from '../../core/artifact-graph/index.js'; import { isSpecsArtifactPath } from '../../core/artifact-graph/outputs.js'; +import { findUnreadDeltaFiles } from '../../utils/spec-discovery.js'; import { getChangeDir, resolveCurrentPlanningHomeSync, @@ -436,14 +437,18 @@ function collectMissingPrerequisites(input: { * reached tasks yet, the missing specs are the next step rather than a warning. * Schemas that declare no spec-producing artifact carry `skip_specs` from * creation, so this never fires on them. + * + * A delta file the merge path never reads (specs/.md, a note + * beside spec.md) still satisfies the specs glob, so it reads as written here + * while validate rejects it and archive would drop it. Each one is named. */ -function collectApplyWarnings(input: { +async function collectApplyWarnings(input: { state: ApplyInstructions['state']; schema: { artifacts: { id: string; generates: string }[] }; changeDir: string; changeName: string; skippedArtifacts?: Set; -}): string[] { +}): Promise { const { state, schema, changeDir, changeName, skippedArtifacts } = input; if (state === 'blocked') return []; @@ -452,10 +457,15 @@ function collectApplyWarnings(input: { ); if (specArtifacts.length === 0) return []; if (specArtifacts.some((artifact) => skippedArtifacts?.has(artifact.id))) return []; + const warnings = (await findUnreadDeltaFiles(path.join(changeDir, 'specs'))).map( + (file) => + `specs/${file.path} is not a capability's spec.md, so \`openspec validate ${changeName}\` rejects it and archive never merges it. ` + + `Move its requirements into specs/${file.expected}.` + ); const hasDeltas = specArtifacts.some( (artifact) => resolveArtifactOutputs(changeDir, artifact.generates).length > 0 ); - if (hasDeltas) return []; + if (hasDeltas) return warnings; const metadataPath = path.join(changeDir, METADATA_FILENAME); // The command names the artifact this schema actually declares, never the @@ -466,6 +476,7 @@ function collectApplyWarnings(input: { // a placeholder rather than a guess. const specTarget = specArtifacts.length === 1 ? specArtifacts[0].id : ''; return [ + ...warnings, `This change has no delta specs and does not declare \`skip_specs: true\`, so \`openspec validate ${changeName}\` fails on it. ` + `Write the delta specs before implementing (\`openspec instructions ${specTarget} --change ${changeName}\`), ` + `or add \`skip_specs: true\` to ${metadataPath} if this change really changes no specified behavior.`, @@ -608,7 +619,7 @@ export async function generateApplyInstructions( instruction = schemaInstruction?.trim() ?? 'Read context files, work through pending tasks, mark complete as you go.\nPause if you hit blockers or need clarification.'; } - const warnings = collectApplyWarnings({ + const warnings = await collectApplyWarnings({ state, schema, changeDir, diff --git a/src/core/archive.ts b/src/core/archive.ts index 888a6135a6..61c0921ec6 100644 --- a/src/core/archive.ts +++ b/src/core/archive.ts @@ -23,7 +23,7 @@ import { finalizeRetiredSpec, type SpecUpdate, } from './specs-apply.js'; -import { discoverSpecFiles, hasAnyFileUnder } from '../utils/spec-discovery.js'; +import { discoverSpecFiles, findUnreadDeltaFiles, hasAnyFileUnder } from '../utils/spec-discovery.js'; import { METADATA_FILENAME, readRetireCapabilitiesMarker, readSkipSpecsMarker } from '../utils/change-metadata.js'; import { confirmPrompt, isNonInteractivePromptError } from '../utils/interactive.js'; import { FileSystemUtils } from '../utils/file-system.js'; @@ -1225,6 +1225,13 @@ export class ArchiveCommand { // folder, so only a regular file counts. const rootSpecStat = await fs.stat(path.join(changeSpecsDir, 'spec.md')).catch(() => null); let hasDeltaSpecs = rootSpecStat?.isFile() === true; + // Likewise for delta sections in any other file the merge path does not + // read (specs/.md, a note beside spec.md): without this the + // zero-delta leniency below archives the change as done with nothing + // merged, although validate rejects it. + if (!hasDeltaSpecs) { + hasDeltaSpecs = (await findUnreadDeltaFiles(changeSpecsDir)).length > 0; + } // A change that declares skip_specs must not carry any file under // specs/ — validate reports that as a conflict, so archive has to run // the same check instead of skipping validation because the files diff --git a/src/core/validation/validator.ts b/src/core/validation/validator.ts index a36e4e75c8..f4e63f8933 100644 --- a/src/core/validation/validator.ts +++ b/src/core/validation/validator.ts @@ -26,7 +26,7 @@ import { } from '../parsers/requirement-text.js'; import { findMainSpecStructureIssues } from '../parsers/spec-structure.js'; import { FileSystemUtils } from '../../utils/file-system.js'; -import { discoverSpecFiles, hasAnyFileUnder } from '../../utils/spec-discovery.js'; +import { discoverSpecFiles, findUnreadDeltaFiles, hasAnyFileUnder } from '../../utils/spec-discovery.js'; import { METADATA_FILENAME, readSkipSpecsMarker, @@ -426,6 +426,18 @@ export class Validator { } } + // The same drop happens to delta sections in any other file the merge + // path does not read (specs/.md, a note beside spec.md), + // while the artifact graph's specs/**/*.md glob counts it as written. + const unreadDeltaFiles = await findUnreadDeltaFiles(specsDir); + for (const file of unreadDeltaFiles) { + issues.push({ + level: 'ERROR', + path: file.path, + message: `Delta spec found at specs/${file.path}. Delta specs must be a spec.md inside a capability folder — this file is ignored when the change is applied or archived. Move its requirements into specs/${file.expected}.`, + }); + } + for (const { path: specPath, sections } of emptySectionSpecs) { issues.push({ level: 'ERROR', @@ -467,10 +479,10 @@ export class Validator { issues.push({ level: 'ERROR', path: 'file', message: VALIDATION_MESSAGES.CHANGE_SKIP_SPECS_CONFLICT }); } - // The root-level error already names the file and the fix; adding "No - // deltas found" on top would contradict it, since the deltas are sitting in - // the file just reported. - if (totalDeltas === 0 && !hasRootLevelSpec) { + // The root-level and unread-file errors already name the file and the fix; + // adding "No deltas found" on top would contradict them, since the deltas + // are sitting in the files just reported. + if (totalDeltas === 0 && !hasRootLevelSpec && unreadDeltaFiles.length === 0) { if (skipSpecs && !specsDirHasFiles) { issues.push({ level: 'INFO', path: 'file', message: VALIDATION_MESSAGES.CHANGE_SKIP_SPECS_ACCEPTED }); } else if (!skipSpecs) { diff --git a/src/utils/spec-discovery.ts b/src/utils/spec-discovery.ts index ab6b8a5eb3..d1d2d35d2a 100644 --- a/src/utils/spec-discovery.ts +++ b/src/utils/spec-discovery.ts @@ -1,6 +1,7 @@ import { promises as fs } from 'fs'; import path from 'path'; import { FileSystemUtils } from './file-system.js'; +import { parseDeltaSpec } from '../core/parsers/requirement-blocks.js'; export interface DiscoveredSpec { /** Spec id relative to the specs root, forward-slash separated on every platform (e.g. "web" or "platform/session-layout"). */ @@ -75,6 +76,64 @@ export async function discoverSpecFiles(specsRoot: string): Promise (a.id < b.id ? -1 : a.id > b.id ? 1 : 0)); } +export interface UnreadDeltaFile { + /** File path relative to the specs root, forward-slash separated. */ + path: string; + /** The spec.md the merge path reads for it, relative to the specs root. */ + expected: string; +} + +/** + * Markdown files under a change's specs/ that carry delta sections but are not + * a capability's `spec.md`, so discoverSpecFiles, and with it validate and + * archive, never reads them: `specs/user-auth.md`, or `specs/user-auth/delta.md` + * beside or instead of the capability's spec.md. The artifact graph's + * recursive specs/ markdown glob does match them, so status and apply report + * the specs as written while archive has nothing to merge. A `spec.md` at the + * specs/ root has its own check (#1385) and is not repeated here. Notes with + * no delta section are not deltas and are not reported. The walk matches + * discoverSpecFiles: dot entries are skipped, symlinked directories are not + * followed, and a dangling link is skipped. A missing root yields an empty + * list; any other read failure is thrown. Results are sorted by path. + */ +export async function findUnreadDeltaFiles(specsRoot: string): Promise { + const results: UnreadDeltaFile[] = []; + const walk = async (dir: string, segments: string[]): Promise => { + let entries; + try { + entries = await fs.readdir(dir, { withFileTypes: true }); + } catch (err: any) { + if (err?.code === 'ENOENT' || err?.code === 'ENOTDIR') return; + throw err; + } + for (const entry of entries) { + if (entry.name.startsWith('.')) continue; + if (entry.isDirectory()) { + await walk(path.join(dir, entry.name), [...segments, entry.name]); + continue; + } + if (!entry.isFile() && !entry.isSymbolicLink()) continue; + if (entry.name === 'spec.md' || !entry.name.toLowerCase().endsWith('.md')) continue; + const filePath = path.join(dir, entry.name); + let content: string; + try { + if (entry.isSymbolicLink() && !(await fs.stat(filePath)).isFile()) continue; + content = await fs.readFile(filePath, 'utf-8'); + } catch (err: any) { + // A dangling link is not content; anything else fails loudly. + if (err?.code === 'ENOENT') continue; + throw err; + } + if (!Object.values(parseDeltaSpec(content).sectionPresence).some(Boolean)) continue; + const capability = + segments.length > 0 ? segments.join('/') : entry.name.slice(0, -'.md'.length); + results.push({ path: [...segments, entry.name].join('/'), expected: `${capability}/spec.md` }); + } + }; + await walk(specsRoot, []); + return results.sort((a, b) => (a.path < b.path ? -1 : a.path > b.path ? 1 : 0)); +} + /** * True when any regular non-dot file exists anywhere under the given * directory. Used by validate/archive to detect content under a change's diff --git a/test/core/misplaced-delta-files.test.ts b/test/core/misplaced-delta-files.test.ts new file mode 100644 index 0000000000..39eb200f37 --- /dev/null +++ b/test/core/misplaced-delta-files.test.ts @@ -0,0 +1,427 @@ +import { describe, it, expect, beforeEach, afterEach, afterAll, vi } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import os from 'os'; +import { findUnreadDeltaFiles } from '../../src/utils/spec-discovery.js'; +import { Validator } from '../../src/core/validation/validator.js'; +import { ArchiveCommand } from '../../src/core/archive.js'; +import { generateApplyInstructions } from '../../src/commands/workflow/instructions.js'; +import { runCLI } from '../helpers/run-cli.js'; + +vi.mock('@inquirer/prompts', () => ({ + select: vi.fn(), + confirm: vi.fn(), +})); + +vi.mock('../../src/utils/interactive.js', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, confirmPrompt: vi.fn() }; +}); + +/** + * validate and archive read a change's deltas only from + * specs//spec.md, but the spec-driven artifact graph counts + * any specs/**.md as the specs being written. A delta written anywhere else, + * such as specs/user-auth.md, was reported done by status and ready by apply, + * rejected by validate as "no deltas", and then archived with exit 0 and + * nothing merged into openspec/specs/. + */ +const DELTA = [ + '## ADDED Requirements', + '', + '### Requirement: Password Login', + 'The system SHALL let a user sign in with a password.', + '', + '#### Scenario: Valid password', + '- **WHEN** a user submits a valid password', + '- **THEN** a session is created', + '', +].join('\n'); + +async function write(root: string, segments: string[], content: string): Promise { + const file = path.join(root, ...segments); + await fs.mkdir(path.dirname(file), { recursive: true }); + await fs.writeFile(file, content); +} + +const exists = (p: string) => fs.access(p).then(() => true, () => false); + +describe('findUnreadDeltaFiles', () => { + let tempDir: string; + let specsDir: string; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-unread-deltas-')); + specsDir = path.join(tempDir, 'specs'); + }); + + afterEach(async () => { + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + it('reports a delta written as specs/.md', async () => { + await write(specsDir, ['user-auth.md'], DELTA); + expect(await findUnreadDeltaFiles(specsDir)).toEqual([ + { path: 'user-auth.md', expected: 'user-auth/spec.md' }, + ]); + }); + + it('reports a delta in a capability folder under another name', async () => { + await write(specsDir, ['user-auth', 'delta.md'], DELTA); + expect(await findUnreadDeltaFiles(specsDir)).toEqual([ + { path: 'user-auth/delta.md', expected: 'user-auth/spec.md' }, + ]); + }); + + it('reports a stray delta beside a capability spec.md', async () => { + await write(specsDir, ['user-auth', 'spec.md'], DELTA); + await write(specsDir, ['user-auth', 'more.md'], DELTA); + expect(await findUnreadDeltaFiles(specsDir)).toEqual([ + { path: 'user-auth/more.md', expected: 'user-auth/spec.md' }, + ]); + }); + + it('reports one inside a nested area folder', async () => { + await write(specsDir, ['platform', 'session', 'changes.md'], DELTA); + expect(await findUnreadDeltaFiles(specsDir)).toEqual([ + { path: 'platform/session/changes.md', expected: 'platform/session/spec.md' }, + ]); + }); + + it('reports a spec file whose name differs only in case', async () => { + await write(specsDir, ['user-auth', 'SPEC.md'], DELTA); + expect(await findUnreadDeltaFiles(specsDir)).toEqual([ + { path: 'user-auth/SPEC.md', expected: 'user-auth/spec.md' }, + ]); + }); + + it('ignores the flat and nested layouts the merge path reads', async () => { + await write(specsDir, ['user-auth', 'spec.md'], DELTA); + await write(specsDir, ['platform', 'session', 'spec.md'], DELTA); + expect(await findUnreadDeltaFiles(specsDir)).toEqual([]); + }); + + it('leaves a specs/-root spec.md to its own check', async () => { + await write(specsDir, ['spec.md'], DELTA); + expect(await findUnreadDeltaFiles(specsDir)).toEqual([]); + }); + + it('ignores notes with no delta section', async () => { + await write(specsDir, ['README.md'], '# Notes\n\nWhy these specs are organized this way.\n'); + await write(specsDir, ['user-auth', 'spec.md'], DELTA); + await write(specsDir, ['user-auth', 'notes.md'], '# Notes\n\nOpen questions for review.\n'); + expect(await findUnreadDeltaFiles(specsDir)).toEqual([]); + }); + + it('ignores delta headers that only appear inside a code fence', async () => { + await write( + specsDir, + ['guide.md'], + '# How to write a delta\n\n```markdown\n## ADDED Requirements\n### Requirement: Example\n```\n' + ); + expect(await findUnreadDeltaFiles(specsDir)).toEqual([]); + }); + + it('skips dot entries and non-markdown files', async () => { + await write(specsDir, ['.drafts', 'user-auth.md'], DELTA); + await write(specsDir, ['.hidden.md'], DELTA); + await write(specsDir, ['user-auth.txt'], DELTA); + expect(await findUnreadDeltaFiles(specsDir)).toEqual([]); + }); + + it('returns results sorted by path', async () => { + await write(specsDir, ['zeta.md'], DELTA); + await write(specsDir, ['alpha', 'delta.md'], DELTA); + expect((await findUnreadDeltaFiles(specsDir)).map((file) => file.path)).toEqual([ + 'alpha/delta.md', + 'zeta.md', + ]); + }); + + it('returns nothing for a change with no specs folder', async () => { + expect(await findUnreadDeltaFiles(specsDir)).toEqual([]); + }); + + it.skipIf(process.platform === 'win32')('skips a dangling symlink', async () => { + await fs.mkdir(specsDir, { recursive: true }); + await fs.symlink(path.join(tempDir, 'missing.md'), path.join(specsDir, 'ghost.md')); + expect(await findUnreadDeltaFiles(specsDir)).toEqual([]); + }); +}); + +describe('validate with an unread delta file', () => { + let tempDir: string; + let changeDir: string; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-unread-validate-')); + changeDir = path.join(tempDir, 'change'); + }); + + afterEach(async () => { + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + it('rejects specs/.md, naming the file and where it belongs', async () => { + await write(changeDir, ['specs', 'user-auth.md'], DELTA); + + const report = await new Validator(true).validateChangeDeltaSpecs(changeDir); + + expect(report.valid).toBe(false); + const issue = report.issues.find((i) => i.path === 'user-auth.md'); + expect(issue?.level).toBe('ERROR'); + expect(issue?.message).toContain('specs/user-auth.md'); + expect(issue?.message).toContain('specs/user-auth/spec.md'); + // The precise error replaces the generic one, which would say "No deltas + // found" about deltas sitting in the file it just named. + expect(report.issues.some((i) => i.message.includes('No deltas found'))).toBe(false); + }); + + it('rejects a stray delta file even when the capability spec.md is valid', async () => { + await write(changeDir, ['specs', 'user-auth', 'spec.md'], DELTA); + await write(changeDir, ['specs', 'user-auth', 'more.md'], DELTA); + + const report = await new Validator(true).validateChangeDeltaSpecs(changeDir); + + expect(report.valid).toBe(false); + expect(report.issues.find((i) => i.path === 'user-auth/more.md')?.level).toBe('ERROR'); + }); + + it('control: accepts the nested layout specs///spec.md', async () => { + await write(changeDir, ['specs', 'platform', 'session', 'spec.md'], DELTA); + + const report = await new Validator(true).validateChangeDeltaSpecs(changeDir); + + expect(report.valid).toBe(true); + }); + + it('control: accepts notes beside a valid delta', async () => { + await write(changeDir, ['specs', 'user-auth', 'spec.md'], DELTA); + await write(changeDir, ['specs', 'README.md'], '# Notes\n\nPlain notes.\n'); + + const report = await new Validator(true).validateChangeDeltaSpecs(changeDir); + + expect(report.valid).toBe(true); + }); +}); + +describe('archive with an unread delta file', () => { + let tempDir: string; + let archiveCommand: ArchiveCommand; + const originalCwd = process.cwd(); + const originalConsoleLog = console.log; + const originalExitCode = process.exitCode; + const originalXdgDataHome = process.env.XDG_DATA_HOME; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-unread-archive-')); + process.chdir(tempDir); + process.env.XDG_DATA_HOME = path.join(tempDir, 'xdg-data'); + await fs.mkdir(path.join(tempDir, 'openspec', 'specs'), { recursive: true }); + await fs.mkdir(path.join(tempDir, 'openspec', 'changes', 'archive'), { recursive: true }); + console.log = vi.fn(); + process.exitCode = undefined; + archiveCommand = new ArchiveCommand(); + }); + + afterEach(async () => { + console.log = originalConsoleLog; + process.exitCode = originalExitCode; + if (originalXdgDataHome === undefined) { + delete process.env.XDG_DATA_HOME; + } else { + process.env.XDG_DATA_HOME = originalXdgDataHome; + } + process.chdir(originalCwd); + vi.clearAllMocks(); + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + async function change(name: string, specs: Array<[string[], string]>): Promise { + const changeDir = path.join(tempDir, 'openspec', 'changes', name); + await write(changeDir, ['tasks.md'], '- [x] Task 1\n'); + for (const [segments, content] of specs) { + await write(changeDir, ['specs', ...segments], content); + } + return changeDir; + } + + const archived = async (name: string) => + (await fs.readdir(path.join(tempDir, 'openspec', 'changes', 'archive'))).some((entry) => + entry.endsWith(name) + ); + const mainSpec = () => path.join(tempDir, 'openspec', 'specs', 'user-auth', 'spec.md'); + + it('refuses to archive when the only delta is specs/.md', async () => { + const changeDir = await change('flat-delta', [[['user-auth.md'], DELTA]]); + + await archiveCommand.execute('flat-delta', { yes: true }); + + expect(process.exitCode).toBe(1); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining('Validation failed')); + expect(console.log).toHaveBeenCalledWith(expect.stringContaining('specs/user-auth/spec.md')); + expect(await archived('flat-delta')).toBe(false); + expect(await exists(changeDir)).toBe(true); + expect(await exists(mainSpec())).toBe(false); + }); + + it('refuses to archive when a stray delta sits beside a valid spec.md', async () => { + await change('stray-delta', [ + [['user-auth', 'spec.md'], DELTA], + [['user-auth', 'more.md'], DELTA.replace('Password Login', 'Passkey Login')], + ]); + + await archiveCommand.execute('stray-delta', { yes: true }); + + expect(process.exitCode).toBe(1); + expect(await archived('stray-delta')).toBe(false); + expect(await exists(mainSpec())).toBe(false); + }); + + it('still archives with --no-validate, the documented escape hatch', async () => { + await change('flat-no-validate', [[['user-auth.md'], DELTA]]); + + await archiveCommand.execute('flat-no-validate', { yes: true, noValidate: true }); + + expect(process.exitCode).toBeUndefined(); + expect(await archived('flat-no-validate')).toBe(true); + }); + + it('control: a change with no spec files still archives', async () => { + await change('tooling-only', []); + + await archiveCommand.execute('tooling-only', { yes: true }); + + expect(process.exitCode).toBeUndefined(); + expect(await archived('tooling-only')).toBe(true); + }); + + it('control: specs//spec.md archives and merges', async () => { + await change('nested-delta', [[['user-auth', 'spec.md'], DELTA]]); + + await archiveCommand.execute('nested-delta', { yes: true }); + + expect(process.exitCode).toBeUndefined(); + expect(await archived('nested-delta')).toBe(true); + expect(await fs.readFile(mainSpec(), 'utf-8')).toContain('Password Login'); + }); +}); + +describe('instructions apply with an unread delta file', () => { + let tempDir: string; + let changeDir: string; + + beforeEach(async () => { + tempDir = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-unread-apply-')); + changeDir = path.join(tempDir, 'openspec', 'changes', 'my-change'); + await write(changeDir, ['.openspec.yaml'], 'schema: spec-driven\n'); + await write(changeDir, ['proposal.md'], '## Why\nx\n'); + await write(changeDir, ['tasks.md'], '## 1. Implementation\n- [x] 1.1 Write the code\n'); + }); + + afterEach(async () => { + await fs.rm(tempDir, { recursive: true, force: true }); + }); + + it('warns about specs/.md, exactly when the validator rejects it', async () => { + await write(changeDir, ['specs', 'user-auth.md'], DELTA); + + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + const report = await new Validator().validateChangeDeltaSpecs(changeDir); + + expect(instructions.state).toBe('all_done'); + expect(instructions.warnings).toHaveLength(1); + expect(instructions.warnings?.[0]).toContain('specs/user-auth.md'); + expect(instructions.warnings?.[0]).toContain('specs/user-auth/spec.md'); + expect(instructions.warnings?.[0]).toContain('openspec validate my-change'); + expect(report.valid).toBe(false); + }); + + it('names a stray delta beside a valid spec.md', async () => { + await write(changeDir, ['specs', 'user-auth', 'spec.md'], DELTA); + await write(changeDir, ['specs', 'user-auth', 'more.md'], DELTA); + + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + + expect(instructions.warnings).toHaveLength(1); + expect(instructions.warnings?.[0]).toContain('specs/user-auth/more.md'); + }); + + it('control: stays quiet for specs//spec.md', async () => { + await write(changeDir, ['specs', 'user-auth', 'spec.md'], DELTA); + + const instructions = await generateApplyInstructions(tempDir, 'my-change'); + const report = await new Validator().validateChangeDeltaSpecs(changeDir); + + expect(instructions.warnings).toBeUndefined(); + expect(report.valid).toBe(true); + }); +}); + +describe('end to end: a delta written as specs/.md', () => { + const temps: string[] = []; + afterAll(async () => { + await Promise.all(temps.map((dir) => fs.rm(dir, { recursive: true, force: true }))); + }); + const T = 120_000; + + /** A fully planned change created through the CLI, its delta at the given path. */ + async function loginChange(deltaPath: string[]) { + const base = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-unread-e2e-')); + temps.push(base); + const home = path.join(base, 'home'); + const project = path.join(base, 'project'); + await fs.mkdir(home, { recursive: true }); + await fs.mkdir(project, { recursive: true }); + const env = { + HOME: home, + XDG_CONFIG_HOME: path.join(home, '.config'), + XDG_DATA_HOME: path.join(home, '.local', 'share'), + OPENSPEC_NO_ANIMATION: '1', + }; + const cli = (args: string[]) => runCLI(args, { cwd: project, env, timeoutMs: 60_000 }); + expect((await cli(['init', '--tools', 'claude'])).exitCode).toBe(0); + expect((await cli(['new', 'change', 'add-login'])).exitCode).toBe(0); + const dir = path.join(project, 'openspec', 'changes', 'add-login'); + await write( + dir, + ['proposal.md'], + '# Add login\n\n## Why\nUsers need to sign in so that their data is private to them and auditable.\n\n## What Changes\n- **user-auth**: adds login\n' + ); + await write(dir, ['design.md'], '# Design\n\nSession cookies.\n'); + await write(dir, ['tasks.md'], '## 1. Work\n- [x] 1.1 Implement login\n'); + await write(dir, ['specs', ...deltaPath], DELTA); + const mainSpec = path.join(project, 'openspec', 'specs', 'user-auth', 'spec.md'); + return { cli, dir, mainSpec }; + } + + it('apply warns, validate rejects, and archive refuses instead of archiving it unmerged', async () => { + const c = await loginChange(['user-auth.md']); + + const apply = JSON.parse( + (await c.cli(['instructions', 'apply', '--change', 'add-login', '--json'])).stdout + ); + expect(apply.warnings?.join('\n')).toContain('specs/user-auth/spec.md'); + + const validated = await c.cli(['validate', 'add-login']); + expect(validated.exitCode).not.toBe(0); + expect(validated.stdout + validated.stderr).toContain('specs/user-auth.md'); + + const archived = await c.cli(['archive', 'add-login', '--yes']); + expect(archived.exitCode).not.toBe(0); + expect(await exists(c.dir)).toBe(true); + expect(await exists(c.mainSpec)).toBe(false); + }, T); + + it('control: specs//spec.md validates, archives, and merges', async () => { + const c = await loginChange(['user-auth', 'spec.md']); + + const apply = JSON.parse( + (await c.cli(['instructions', 'apply', '--change', 'add-login', '--json'])).stdout + ); + expect(apply.warnings).toBeUndefined(); + expect((await c.cli(['validate', 'add-login'])).exitCode).toBe(0); + expect((await c.cli(['archive', 'add-login', '--yes'])).exitCode).toBe(0); + expect(await fs.readFile(c.mainSpec, 'utf-8')).toContain('Password Login'); + }, T); +});