Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,10 +307,11 @@ program
.option('--specs', 'Validate all specs')
.option('--type <type>', 'Specify item type when ambiguous: change|spec')
.option('--strict', 'Enable strict validation mode')
.option('--accept-cross-change-base', 'Allow MODIFIED/REMOVED/RENAMED-from in a change delta to reference a Requirement that lives in a sister-pending change (`openspec/changes/<other>/specs/<cap>/spec.md`) rather than the canonical spec. Default: false (matches archive-time strictness). When set, the user opts into cross-change authoring; they must archive the sister change first OR fold this change into it before this change can itself be archived (archive remains strict; this flag affects ONLY write-time validate).')
.option('--json', 'Output validation results as JSON')
.option('--concurrency <n>', 'Max concurrent validations (defaults to env OPENSPEC_CONCURRENCY or 6)')
.option('--no-interactive', 'Disable interactive prompts')
.action(async (itemName?: string, options?: { all?: boolean; changes?: boolean; specs?: boolean; type?: string; strict?: boolean; json?: boolean; noInteractive?: boolean; concurrency?: string }) => {
.action(async (itemName?: string, options?: { all?: boolean; changes?: boolean; specs?: boolean; type?: string; strict?: boolean; acceptCrossChangeBase?: boolean; json?: boolean; noInteractive?: boolean; concurrency?: string }) => {
try {
const validateCommand = new ValidateCommand();
await validateCommand.execute(itemName, options);
Expand Down
31 changes: 22 additions & 9 deletions src/commands/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,19 @@ interface ExecuteOptions {
specs?: boolean;
type?: string;
strict?: boolean;
/**
* Opt-in: allow `MODIFIED` / `REMOVED` / `RENAMED-from` in a delta
* spec to reference a Requirement that lives in a sister-pending
* change (`openspec/changes/<other>/specs/<cap>/spec.md`) rather
* than the canonical spec (`openspec/specs/<cap>/spec.md`).
*
* Default: false — validate matches archive-time semantics.
*
* Archive (`openspec archive`) remains strict regardless of this
* flag; you must archive the sister change first OR fold this
* change into it before this change's own archive succeeds.
*/
acceptCrossChangeBase?: boolean;
json?: boolean;
noInteractive?: boolean;
interactive?: boolean; // Commander sets this to false when --no-interactive is used
Expand All @@ -36,14 +49,14 @@ export class ValidateCommand {
await this.runBulkValidation({
changes: !!options.all || !!options.changes,
specs: !!options.all || !!options.specs,
}, { strict: !!options.strict, json: !!options.json, concurrency: options.concurrency, noInteractive: resolveNoInteractive(options) });
}, { strict: !!options.strict, acceptCrossChangeBase: !!options.acceptCrossChangeBase, json: !!options.json, concurrency: options.concurrency, noInteractive: resolveNoInteractive(options) });
return;
}

// No item and no flags
if (!itemName) {
if (interactive) {
await this.runInteractiveSelector({ strict: !!options.strict, json: !!options.json, concurrency: options.concurrency });
await this.runInteractiveSelector({ strict: !!options.strict, acceptCrossChangeBase: !!options.acceptCrossChangeBase, json: !!options.json, concurrency: options.concurrency });
return;
}
this.printNonInteractiveHint();
Expand All @@ -53,7 +66,7 @@ export class ValidateCommand {

// Direct item validation with type detection or override
const typeOverride = this.normalizeType(options.type);
await this.validateDirectItem(itemName, { typeOverride, strict: !!options.strict, json: !!options.json });
await this.validateDirectItem(itemName, { typeOverride, strict: !!options.strict, acceptCrossChangeBase: !!options.acceptCrossChangeBase, json: !!options.json });
}

private normalizeType(value?: string): ItemType | undefined {
Expand All @@ -63,7 +76,7 @@ export class ValidateCommand {
return undefined;
}

private async runInteractiveSelector(opts: { strict: boolean; json: boolean; concurrency?: string }): Promise<void> {
private async runInteractiveSelector(opts: { strict: boolean; acceptCrossChangeBase: boolean; json: boolean; concurrency?: string }): Promise<void> {
const { select } = await import('@inquirer/prompts');
const choice = await select({
message: 'What would you like to validate?',
Expand Down Expand Up @@ -102,7 +115,7 @@ export class ValidateCommand {
console.error('Or run in an interactive terminal.');
}

private async validateDirectItem(itemName: string, opts: { typeOverride?: ItemType; strict: boolean; json: boolean }): Promise<void> {
private async validateDirectItem(itemName: string, opts: { typeOverride?: ItemType; strict: boolean; acceptCrossChangeBase: boolean; json: boolean }): Promise<void> {
const [changes, specs] = await Promise.all([getActiveChangeIds(), getSpecIds()]);
const isChange = changes.includes(itemName);
const isSpec = specs.includes(itemName);
Expand All @@ -127,8 +140,8 @@ export class ValidateCommand {
await this.validateByType(type, itemName, opts);
}

private async validateByType(type: ItemType, id: string, opts: { strict: boolean; json: boolean }): Promise<void> {
const validator = new Validator(opts.strict);
private async validateByType(type: ItemType, id: string, opts: { strict: boolean; acceptCrossChangeBase?: boolean; json: boolean }): Promise<void> {
const validator = new Validator({ strictMode: opts.strict, acceptCrossChangeBase: !!opts.acceptCrossChangeBase });
if (type === 'change') {
const changeDir = path.join(process.cwd(), 'openspec', 'changes', id);
const start = Date.now();
Expand Down Expand Up @@ -181,7 +194,7 @@ export class ValidateCommand {
bullets.forEach(b => console.error(` ${b}`));
}

private async runBulkValidation(scope: { changes: boolean; specs: boolean }, opts: { strict: boolean; json: boolean; concurrency?: string; noInteractive?: boolean }): Promise<void> {
private async runBulkValidation(scope: { changes: boolean; specs: boolean }, opts: { strict: boolean; acceptCrossChangeBase?: boolean; json: boolean; concurrency?: string; noInteractive?: boolean }): Promise<void> {
const spinner = !opts.json && !opts.noInteractive ? ora('Validating...').start() : undefined;
const [changeIds, specIds] = await Promise.all([
scope.changes ? getActiveChangeIds() : Promise.resolve<string[]>([]),
Expand All @@ -191,7 +204,7 @@ export class ValidateCommand {
const DEFAULT_CONCURRENCY = 6;
const maxSuggestions = 5; // used by nearestMatches
const concurrency = normalizeConcurrency(opts.concurrency) ?? normalizeConcurrency(process.env.OPENSPEC_CONCURRENCY) ?? DEFAULT_CONCURRENCY;
const validator = new Validator(opts.strict);
const validator = new Validator({ strictMode: opts.strict, acceptCrossChangeBase: !!opts.acceptCrossChangeBase });
const queue: Array<() => Promise<BulkItemResult>> = [];

for (const id of changeIds) {
Expand Down
180 changes: 178 additions & 2 deletions src/core/validation/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,51 @@ import { parseDeltaSpec, normalizeRequirementName } from '../parsers/requirement
import { findMainSpecStructureIssues } from '../parsers/spec-structure.js';
import { FileSystemUtils } from '../../utils/file-system.js';

export interface ValidatorOptions {
/**
* Strict mode promotes structural-completeness WARNINGs to ERRORs (no
* change to MODIFIED-base cross-references — see acceptCrossChangeBase).
*/
strictMode?: boolean;
/**
* Accept a MODIFIED / REMOVED / RENAMED-from reference whose target
* Requirement is NOT present in the canonical spec (`openspec/specs/
* <cap>/spec.md`) IF the same header appears in a sister change at
* `openspec/changes/<other>/specs/<cap>/spec.md`.
*
* Default false: write-time validate matches archive-time semantics
* (the canonical spec is the only legitimate base) so authoring bugs
* surface immediately instead of at archive time.
*
* Set to true (typically via `openspec validate --accept-cross-change-base`)
* to opt into cross-change MODIFIED authoring. The user is signalling
* they will either archive the sister change first OR fold the work
* into that sister change before this change's own archive. Archive-
* time checking remains strict; this flag affects ONLY write-time
* validate.
*/
acceptCrossChangeBase?: boolean;
}

export class Validator {
private strictMode: boolean;
private acceptCrossChangeBase: boolean;

constructor(strictMode: boolean = false) {
this.strictMode = strictMode;
/**
* @param strictModeOrOptions Either a boolean (legacy, equivalent to
* `{ strictMode }`) or a full `ValidatorOptions` object. The two-
* argument constructor signature is preserved for backward
* compatibility with existing callers that pass just a strictMode
* boolean.
*/
constructor(strictModeOrOptions: boolean | ValidatorOptions = false) {
if (typeof strictModeOrOptions === 'boolean') {
this.strictMode = strictModeOrOptions;
this.acceptCrossChangeBase = false;
} else {
this.strictMode = strictModeOrOptions.strictMode ?? false;
this.acceptCrossChangeBase = strictModeOrOptions.acceptCrossChangeBase ?? false;
}
}

async validateSpec(filePath: string): Promise<ValidationReport> {
Expand Down Expand Up @@ -222,6 +262,29 @@ export class Validator {
}
}

// Canonical-base cross-reference check.
// MODIFIED / REMOVED / RENAMED-from must point at a Requirement
// that already exists in the canonical spec at
// `openspec/specs/<specName>/spec.md`. Mirrors the strict check
// `specs-apply.ts` runs at archive time, but here so authoring
// bugs surface immediately rather than days later when the
// change is being archived.
//
// With `acceptCrossChangeBase` set (CLI flag
// `--accept-cross-change-base`), the check also looks in
// sister-pending changes (`openspec/changes/<other>/specs/
// <specName>/spec.md`). The user opts into cross-change
// authoring; they must either archive the sister change first
// OR fold this change into it before this change's own archive
// succeeds (archive remains strict, no override).
await this.checkDeltaAgainstCanonicalBase({
changeDir,
specName,
entryPath,
plan,
issues,
});

// Cross-section conflicts (within the same spec file)
for (const n of modifiedNames) {
if (removedNames.has(n)) {
Expand Down Expand Up @@ -456,4 +519,117 @@ export class Validator {
const last = sections[sections.length - 1];
return `${head.join(', ')} and ${last}`;
}

/**
* Walk a spec file's content and collect normalized `### Requirement:
* <name>` header names. Used to build the cross-reference set for
* MODIFIED / REMOVED / RENAMED-from checks. Works on both canonical
* specs and delta specs since both use the same `### Requirement:`
* header syntax.
*/
private extractRequirementHeaderNames(content: string): Set<string> {
const names = new Set<string>();
const re = /^###\s+Requirement:\s+(.+?)\s*$/gm;
let m: RegExpExecArray | null;
while ((m = re.exec(content)) !== null) {
names.add(normalizeRequirementName(m[1]));
}
return names;
}

/**
* Cross-reference MODIFIED / REMOVED / RENAMED-from delta operations
* against the canonical spec (and optionally sister-pending changes
* when `acceptCrossChangeBase` is set). Pushes ERROR-level issues
* into the provided `issues` array.
*/
private async checkDeltaAgainstCanonicalBase(args: {
changeDir: string;
specName: string;
entryPath: string;
plan: ReturnType<typeof parseDeltaSpec>;
issues: ValidationIssue[];
}): Promise<void> {
const { changeDir, specName, entryPath, plan, issues } = args;

// Targets that must reference an existing-in-base Requirement.
type Target = { kind: 'MODIFIED' | 'REMOVED' | 'RENAMED'; raw: string; normalized: string };
const targets: Target[] = [
...plan.modified.map(b => ({ kind: 'MODIFIED' as const, raw: b.name, normalized: normalizeRequirementName(b.name) })),
...plan.removed.map(n => ({ kind: 'REMOVED' as const, raw: n, normalized: normalizeRequirementName(n) })),
...plan.renamed.map(r => ({ kind: 'RENAMED' as const, raw: r.from, normalized: normalizeRequirementName(r.from) })),
];
if (targets.length === 0) return;

// Canonical lookup.
const openspecRoot = path.resolve(changeDir, '..', '..');
const canonicalSpecPath = path.join(openspecRoot, 'specs', specName, 'spec.md');
let canonicalNames: Set<string> | null = null;
try {
const c = await fs.readFile(canonicalSpecPath, 'utf-8');
canonicalNames = this.extractRequirementHeaderNames(c);
} catch {
canonicalNames = null;
}

// Sister-pending lookup (lazy — only built if needed by acceptCrossChangeBase).
let sisterNames: Set<string> | null = null;
const buildSisterNames = async (): Promise<Set<string>> => {
if (sisterNames !== null) return sisterNames;
const acc = new Set<string>();
const selfChange = path.basename(changeDir);
const changesDir = path.dirname(changeDir);
try {
const entries = await fs.readdir(changesDir, { withFileTypes: true });
for (const entry of entries) {
if (!entry.isDirectory()) continue;
if (entry.name === selfChange) continue;
if (entry.name === 'archive') continue;
const p = path.join(changesDir, entry.name, 'specs', specName, 'spec.md');
try {
const c = await fs.readFile(p, 'utf-8');
for (const n of this.extractRequirementHeaderNames(c)) acc.add(n);
} catch { /* sister doesn't touch this capability */ }
}
} catch { /* no changes dir (unlikely from within validateChangeDeltaSpecs) */ }
sisterNames = acc;
return sisterNames;
};

// CREATE shape: canonical spec does not exist.
if (canonicalNames === null) {
for (const t of targets) {
if (this.acceptCrossChangeBase) {
const sis = await buildSisterNames();
if (sis.has(t.normalized)) continue;
}
const fix = this.acceptCrossChangeBase
? 'No canonical spec, and no sister-pending change defines this Requirement either. Either change this to ADDED, or author the sister change first.'
: `${t.kind} requires an existing canonical spec at \`openspec/specs/${specName}/spec.md\`. Either change this to ADDED, or — if a sister change is creating this capability — re-run with \`--accept-cross-change-base\` (note: archive remains strict; you must archive the sister change first OR fold this change into it before archive succeeds).`;
issues.push({
level: 'ERROR',
path: entryPath,
message: `${t.kind} "${t.raw}": canonical spec \`openspec/specs/${specName}/spec.md\` does not exist. ${fix}`,
});
}
return;
}

// Canonical exists: each target must be present by exact header match.
for (const t of targets) {
if (canonicalNames.has(t.normalized)) continue;
if (this.acceptCrossChangeBase) {
const sis = await buildSisterNames();
if (sis.has(t.normalized)) continue;
}
const fix = this.acceptCrossChangeBase
? 'Not found in canonical AND not found in any sister-pending change. Did you mean ADDED with a new name? Or did you typo the header?'
: `Not found in canonical \`openspec/specs/${specName}/spec.md\`. Common causes: (a) the Requirement is genuinely new — use ADDED instead; (b) you typo'd the header (whitespace-insensitive match); (c) a sister change in \`openspec/changes/<other>/\` defines it and hasn't been archived — re-run with \`--accept-cross-change-base\` to opt into cross-change MODIFIED (archive remains strict, no override).`;
issues.push({
level: 'ERROR',
path: entryPath,
message: `${t.kind} "${t.raw}": header not found in base. ${fix}`,
});
}
}
}
Loading