Skip to content
Merged
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
1 change: 1 addition & 0 deletions src/commands/change.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ export class ChangeCommand {
// Derived from changesPath so the main specs come from the same root the
// change itself was resolved against.
mainSpecsDir: path.join(path.dirname(changesPath), 'specs'),
projectRoot: path.dirname(path.dirname(changesPath)),
});

if (options?.json) {
Expand Down
10 changes: 8 additions & 2 deletions src/commands/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,10 @@ export class ValidateCommand {
if (type === 'change') {
const changeDir = path.join(root.changesDir, id);
const start = Date.now();
const report = await validator.validateChangeDeltaSpecs(changeDir, { mainSpecsDir: root.specsDir });
const report = await validator.validateChangeDeltaSpecs(changeDir, {
mainSpecsDir: root.specsDir,
projectRoot: root.path,
});
const durationMs = Date.now() - start;
this.printReport('change', id, report, durationMs, opts.json, root);
// Non-zero exit if invalid (keeps enriched output test semantics)
Expand Down Expand Up @@ -279,7 +282,10 @@ export class ValidateCommand {
queue.push(async () => {
const start = Date.now();
const changeDir = path.join(root.changesDir, id);
const report = await validator.validateChangeDeltaSpecs(changeDir, { mainSpecsDir: root.specsDir });
const report = await validator.validateChangeDeltaSpecs(changeDir, {
mainSpecsDir: root.specsDir,
projectRoot: root.path,
});
const durationMs = Date.now() - start;
return { id, type: 'change' as const, valid: report.valid, issues: report.issues, durationMs };
});
Expand Down
80 changes: 80 additions & 0 deletions src/core/validation/task-numbering.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
import { parseTaskLines } from '../../utils/task-progress.js';

export interface TaskNumberingDocument {
path: string;
content: string;
}

export interface TaskNumberingIssue {
path: string;
line: number;
message: string;
}

interface TaskLocation {
path: string;
line: number;
}

const LEVEL_TWO_HEADING = /^ {0,3}##(?!#)(?:[ \t]+|[ \t]*\r?$)/;
const NUMBERED_GROUP_HEADING = /^ {0,3}##[ \t]+(\d+)\.(?:[ \t]|\r?$)/;
const TASK_ID = /^(\d+(?:\.\d+)+(?:[A-Za-z]+)?)(?=\s|$)/;

/**
* Finds ambiguous task references across the task files tracked by a change.
* Numbering is interpreted only inside `## N.` groups. Unnumbered sections,
* unnumbered tasks, and files without numbered groups are intentionally ignored.
*/
export function findTaskNumberingIssues(
documents: readonly TaskNumberingDocument[]
): TaskNumberingIssue[] {
const issues: TaskNumberingIssue[] = [];
const firstLocationById = new Map<string, TaskLocation>();

for (const document of documents) {
const lines = document.content.split('\n');
if (!lines.some((line) => NUMBERED_GROUP_HEADING.test(line))) continue;

let currentGroup: string | undefined;

lines.forEach((line, index) => {
if (LEVEL_TWO_HEADING.test(line)) {
currentGroup = line.match(NUMBERED_GROUP_HEADING)?.[1];
}
if (currentGroup === undefined) return;

const task = parseTaskLines(line)[0];
const id = task?.description.match(TASK_ID)?.[1];
if (!id) return;

const lineNumber = index + 1;
const taskGroup = id.split('.')[0];
const normalizedTaskGroup = taskGroup.replace(/^0+(?=\d)/, '');
const normalizedCurrentGroup = currentGroup.replace(/^0+(?=\d)/, '');
if (normalizedTaskGroup !== normalizedCurrentGroup) {
issues.push({
path: document.path,
line: lineNumber,
message: `Task "${id}" is under group ${currentGroup}, but its leading number points to group ${taskGroup}. Move it to group ${taskGroup} or renumber it.`,
});
}

const firstLocation = firstLocationById.get(id);
if (firstLocation !== undefined) {
const firstDeclaration =
firstLocation.path === document.path
? `on line ${firstLocation.line}`
: `in ${firstLocation.path} on line ${firstLocation.line}`;
issues.push({
path: document.path,
line: lineNumber,
message: `Task ID "${id}" is duplicated; it was first declared ${firstDeclaration}.`,
});
} else {
firstLocationById.set(id, { path: document.path, line: lineNumber });
}
});
}

return issues;
}
77 changes: 73 additions & 4 deletions src/core/validation/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,14 @@ import {
import { findMainSpecStructureIssues } from '../parsers/spec-structure.js';
import { FileSystemUtils } from '../../utils/file-system.js';
import { discoverSpecFiles, hasAnyFileUnder } from '../../utils/spec-discovery.js';
import { METADATA_FILENAME, readSkipSpecsMarker } from '../../utils/change-metadata.js';
import {
METADATA_FILENAME,
readSkipSpecsMarker,
resolveSchemaForChange,
} from '../../utils/change-metadata.js';
import { resolveTaskFilesForChange } from '../../utils/task-progress.js';
import { findTaskNumberingIssues } from './task-numbering.js';
import { getPackageSchemasDir, getSchemaDir } from '../artifact-graph/index.js';

export class Validator {
private strictMode: boolean;
Expand Down Expand Up @@ -144,12 +151,13 @@ export class Validator {
*
* When `options.mainSpecsDir` is given, MODIFIED blocks are also checked
* against the current main specs for the scenario loss archive refuses to
* apply (#1477). Omitting it keeps the change-only checks, so callers with
* no main specs root (and existing library callers) behave as before.
* apply (#1477). 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(
changeDir: string,
options: { mainSpecsDir?: string } = {}
options: { mainSpecsDir?: string; projectRoot?: string } = {}
): Promise<ValidationReport> {
const issues: ValidationIssue[] = [];
const specsDir = path.join(changeDir, 'specs');
Expand Down Expand Up @@ -450,9 +458,70 @@ export class Validator {
}
}

if (options.projectRoot) {
issues.push(...await this.collectTaskNumberingIssues(changeDir, options.projectRoot));
}

return this.createReport(issues);
}

private async collectTaskNumberingIssues(
changeDir: string,
projectRoot: string
): Promise<ValidationIssue[]> {
try {
const schemaName = resolveSchemaForChange(changeDir, undefined, projectRoot).replace(
/\.ya?ml$/,
''
);
const schemaDir = getSchemaDir(schemaName, projectRoot);
const builtInSchemaDir = path.join(getPackageSchemasDir(), 'spec-driven');
if (
schemaName !== 'spec-driven' ||
schemaDir === null ||
FileSystemUtils.canonicalizeExistingPath(schemaDir) !==
FileSystemUtils.canonicalizeExistingPath(builtInSchemaDir)
) {
return [];
}
} catch {
return [];
}

let taskFiles: string[];
try {
taskFiles = resolveTaskFilesForChange(changeDir, projectRoot);
} catch {
return [];
}
if (taskFiles.length === 0) {
taskFiles = [path.join(changeDir, 'tasks.md')];
}

const documents: Array<{ path: string; content: string }> = [];
for (const taskFile of taskFiles) {
let content: string;
try {
content = await fs.readFile(taskFile, 'utf-8');
} catch {
continue;
}

documents.push({
path: FileSystemUtils.toPosixPath(path.relative(changeDir, taskFile)),
content,
});
}

documents.sort((left, right) => left.path.localeCompare(right.path));
return findTaskNumberingIssues(documents).map((issue) => ({
level: 'WARNING',
path: issue.path,
line: issue.line,
message: issue.message,
}));
}

/**
* Report MODIFIED requirements whose block omits a scenario the main spec
* still carries. Uses the same comparison archive applies, so validate can
Expand Down
36 changes: 19 additions & 17 deletions src/utils/task-progress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,12 @@ async function countSingleTopLevelTasksFile(changeDir: string): Promise<TaskProg
}
}

/** Resolves the task files selected by the schema's apply tracking rule. */
export function resolveTaskFilesForChange(changeDir: string, projectRoot: string): string[] {
const generates = resolveTrackedTasksGlob(changeDir, projectRoot);
return generates ? resolveArtifactOutputs(changeDir, generates) : [];
}

/**
* Computes a change's task progress by resolving its tracked-tasks artifact and
* counting checkboxes across every file matched by that artifact's `generates`
Expand All @@ -120,25 +126,21 @@ export async function getTaskProgressForChange(
projectRoot: string
): Promise<TaskProgress> {
const changeDir = path.join(changesDir, changeName);

const generates = resolveTrackedTasksGlob(changeDir, projectRoot);
if (generates) {
const files = resolveArtifactOutputs(changeDir, generates);
if (files.length > 0) {
let total = 0;
let completed = 0;
for (const file of files) {
try {
const content = await fs.readFile(file, 'utf-8');
const progress = countTasksFromContent(content);
total += progress.total;
completed += progress.completed;
} catch {
// Swallow files that vanish between glob and read, as before.
}
const files = resolveTaskFilesForChange(changeDir, projectRoot);
if (files.length > 0) {
let total = 0;
let completed = 0;
for (const file of files) {
try {
const content = await fs.readFile(file, 'utf-8');
const progress = countTasksFromContent(content);
total += progress.total;
completed += progress.completed;
} catch {
// Swallow files that vanish between glob and read, as before.
}
return { total, completed };
}
return { total, completed };
}

return countSingleTopLevelTasksFile(changeDir);
Expand Down
Loading
Loading