From df44f23b1674b14d71c0d0ba468da37d0d88509a Mon Sep 17 00:00:00 2001 From: alectimison-maker Date: Thu, 6 Aug 2026 14:32:54 +0800 Subject: [PATCH 1/2] fix(validate): warn on ambiguous task numbering --- src/commands/change.ts | 1 + src/commands/validate.ts | 10 +- src/core/validation/task-numbering.ts | 57 +++++ src/core/validation/validator.ts | 49 ++++- src/utils/task-progress.ts | 36 +-- test/cli-e2e/validate-task-numbering.test.ts | 218 +++++++++++++++++++ test/core/task-numbering.test.ts | 60 +++++ 7 files changed, 409 insertions(+), 22 deletions(-) create mode 100644 src/core/validation/task-numbering.ts create mode 100644 test/cli-e2e/validate-task-numbering.test.ts create mode 100644 test/core/task-numbering.test.ts diff --git a/src/commands/change.ts b/src/commands/change.ts index 849fadea6d..4c58af7892 100644 --- a/src/commands/change.ts +++ b/src/commands/change.ts @@ -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) { diff --git a/src/commands/validate.ts b/src/commands/validate.ts index 0e74722493..7c474cd0c2 100644 --- a/src/commands/validate.ts +++ b/src/commands/validate.ts @@ -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) @@ -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 }; }); diff --git a/src/core/validation/task-numbering.ts b/src/core/validation/task-numbering.ts new file mode 100644 index 0000000000..bcf9f67115 --- /dev/null +++ b/src/core/validation/task-numbering.ts @@ -0,0 +1,57 @@ +import { parseTaskLines } from '../../utils/task-progress.js'; + +export interface TaskNumberingIssue { + line: number; + message: string; +} + +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 without imposing a contiguous numbering + * scheme. Unnumbered tasks and tasks outside a `## N.` group are intentionally + * ignored because both forms already exist in real projects. + */ +export function findTaskNumberingIssues(content: string): TaskNumberingIssue[] { + const lines = content.split('\n'); + if (!lines.some((line) => NUMBERED_GROUP_HEADING.test(line))) return []; + + const issues: TaskNumberingIssue[] = []; + const firstLineById = new Map(); + let currentGroup: string | undefined; + + lines.forEach((line, index) => { + if (LEVEL_TWO_HEADING.test(line)) { + currentGroup = line.match(NUMBERED_GROUP_HEADING)?.[1]; + } + + 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 (normalizedCurrentGroup !== undefined && normalizedTaskGroup !== normalizedCurrentGroup) { + issues.push({ + 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 firstLine = firstLineById.get(id); + if (firstLine !== undefined) { + issues.push({ + line: lineNumber, + message: `Task ID "${id}" is duplicated; it was first declared on line ${firstLine}.`, + }); + } else { + firstLineById.set(id, lineNumber); + } + }); + + return issues; +} diff --git a/src/core/validation/validator.ts b/src/core/validation/validator.ts index 24bd7fe7eb..87d4a332b5 100644 --- a/src/core/validation/validator.ts +++ b/src/core/validation/validator.ts @@ -27,6 +27,8 @@ 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 { resolveTaskFilesForChange } from '../../utils/task-progress.js'; +import { findTaskNumberingIssues } from './task-numbering.js'; export class Validator { private strictMode: boolean; @@ -144,12 +146,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 { const issues: ValidationIssue[] = []; const specsDir = path.join(changeDir, 'specs'); @@ -450,9 +453,49 @@ 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 { + let taskFiles: string[]; + try { + taskFiles = resolveTaskFilesForChange(changeDir, projectRoot); + } catch { + return []; + } + if (taskFiles.length === 0) { + taskFiles = [path.join(changeDir, 'tasks.md')]; + } + + const issues: ValidationIssue[] = []; + for (const taskFile of taskFiles) { + let content: string; + try { + content = await fs.readFile(taskFile, 'utf-8'); + } catch { + continue; + } + + const entryPath = FileSystemUtils.toPosixPath(path.relative(changeDir, taskFile)); + for (const issue of findTaskNumberingIssues(content)) { + issues.push({ + level: 'WARNING', + path: entryPath, + line: issue.line, + message: issue.message, + }); + } + } + return issues; + } + /** * Report MODIFIED requirements whose block omits a scenario the main spec * still carries. Uses the same comparison archive applies, so validate can diff --git a/src/utils/task-progress.ts b/src/utils/task-progress.ts index 21f3452ac9..a9d755488f 100644 --- a/src/utils/task-progress.ts +++ b/src/utils/task-progress.ts @@ -105,6 +105,12 @@ async function countSingleTopLevelTasksFile(changeDir: string): Promise { 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); diff --git a/test/cli-e2e/validate-task-numbering.test.ts b/test/cli-e2e/validate-task-numbering.test.ts new file mode 100644 index 0000000000..f768d03812 --- /dev/null +++ b/test/cli-e2e/validate-task-numbering.test.ts @@ -0,0 +1,218 @@ +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { promises as fs } from 'fs'; +import path from 'path'; +import { tmpdir } from 'os'; +import { runCLI } from '../helpers/run-cli.js'; + +describe('openspec validate checks task numbering (#1520)', () => { + let projectDir: string; + + const write = async (relative: string, content: string) => { + const file = path.join(projectDir, relative); + await fs.mkdir(path.dirname(file), { recursive: true }); + await fs.writeFile(file, content, 'utf-8'); + }; + + const validDelta = [ + '## ADDED Requirements', + '', + '### Requirement: Task validation SHALL preserve planning references', + 'The validator SHALL preserve unambiguous task references.', + '', + '#### Scenario: Validate a task list', + '- **WHEN** strict validation runs', + '- **THEN** inconsistent task numbering is reported', + '', + ].join('\n'); + + const globTasksSchema = [ + 'name: glob-tasks', + 'version: 1', + 'description: tasks artifact uses a nested glob', + 'artifacts:', + ' - id: proposal', + ' generates: proposal.md', + ' description: Proposal', + ' template: proposal.md', + ' requires: []', + ' - id: tasks', + ' generates: "**/tasks.md"', + ' description: Nested tasks', + ' template: tasks.md', + ' requires: [proposal]', + 'apply:', + ' requires: [tasks]', + ' tracks: "**/tasks.md"', + '', + ].join('\n'); + + beforeAll(async () => { + projectDir = await fs.mkdtemp(path.join(tmpdir(), 'openspec-task-numbering-e2e-')); + + await write( + 'openspec/changes/bad-numbering/specs/tasks/spec.md', + validDelta + ); + await write( + 'openspec/changes/bad-numbering/tasks.md', + [ + '## 10. First release', + '', + '- [x] 10.1 do a thing', + '- [x] 10.6 do another thing', + '', + '## 11. Register corrections', + '', + '- [x] 10.7 belongs to group 10', + '- [x] 10.8 also belongs to group 10', + '- [ ] 11.1 a real group-11 task', + '- [ ] 11.1 a duplicate id', + '', + ].join('\n') + ); + + await write( + 'openspec/changes/valid-numbering/specs/tasks/spec.md', + validDelta + ); + await write( + 'openspec/changes/valid-numbering/tasks.md', + [ + '# Tasks', + '- [ ] an unnumbered task before any numbered group', + '', + '## 3. Implementation', + '- [ ] 3.2a an inserted task', + ' - [ ] 3.2.1 a nested task', + '- [ ] 3.5 a numbering gap is allowed', + '', + '## Notes', + '- [ ] an unnumbered task under an unnumbered heading', + '', + ].join('\n') + ); + + await write('openspec/schemas/glob-tasks/schema.yaml', globTasksSchema); + await write( + 'openspec/changes/nested-numbering/.openspec.yaml', + 'schema: glob-tasks\n' + ); + await write( + 'openspec/changes/nested-numbering/specs/tasks/spec.md', + validDelta + ); + await write( + 'openspec/changes/nested-numbering/backend/tasks.md', + '## 2. Backend\n- [ ] 3.1 wrong group\n' + ); + await write( + 'openspec/changes/nested-numbering/frontend/tasks.md', + '## 4. Frontend\n- [ ] 4.1 correct group\n' + ); + }); + + afterAll(async () => { + await fs.rm(projectDir, { recursive: true, force: true }); + }); + + it('reports duplicate full ids and group mismatches under --strict', async () => { + const result = await runCLI( + ['validate', '--type', 'change', 'bad-numbering', '--strict', '--json'], + { cwd: projectDir } + ); + + expect(result.exitCode).toBe(1); + const report = JSON.parse(result.stdout); + const issues = report.items[0].issues.filter( + (issue: { path: string }) => issue.path === 'tasks.md' + ); + expect(issues).toEqual([ + expect.objectContaining({ + level: 'WARNING', + line: 8, + message: expect.stringContaining('10.7'), + }), + expect.objectContaining({ + level: 'WARNING', + line: 9, + message: expect.stringContaining('10.8'), + }), + expect.objectContaining({ + level: 'WARNING', + line: 11, + message: expect.stringMatching(/11\.1.*duplicate/i), + }), + ]); + }); + + it('keeps warnings non-blocking without --strict', async () => { + const result = await runCLI( + ['validate', '--type', 'change', 'bad-numbering', '--json'], + { cwd: projectDir } + ); + + expect(result.exitCode).toBe(0); + const report = JSON.parse(result.stdout); + expect(report.items[0].valid).toBe(true); + expect( + report.items[0].issues.filter((issue: { level: string }) => issue.level === 'WARNING') + ).toHaveLength(3); + }); + + it('allows full-depth ids, suffixes, gaps, and unnumbered sections', async () => { + const result = await runCLI( + ['validate', '--type', 'change', 'valid-numbering', '--strict'], + { cwd: projectDir } + ); + + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain("Change 'valid-numbering' is valid"); + }); + + it('applies the same warnings to bulk validation', async () => { + const result = await runCLI(['validate', '--changes', '--strict', '--json'], { + cwd: projectDir, + }); + + expect(result.exitCode).toBe(1); + const report = JSON.parse(result.stdout); + const byId = Object.fromEntries( + report.items.map((item: { id: string; valid: boolean }) => [item.id, item.valid]) + ); + expect(byId['bad-numbering']).toBe(false); + expect(byId['valid-numbering']).toBe(true); + expect(byId['nested-numbering']).toBe(false); + }); + + it('validates every task file selected by the schema glob', async () => { + const result = await runCLI( + ['validate', '--type', 'change', 'nested-numbering', '--strict', '--json'], + { cwd: projectDir } + ); + + expect(result.exitCode).toBe(1); + const report = JSON.parse(result.stdout); + const taskIssues = report.items[0].issues.filter( + (issue: { path: string }) => issue.path.endsWith('tasks.md') + ); + expect(taskIssues).toEqual([ + expect.objectContaining({ + level: 'WARNING', + path: 'backend/tasks.md', + line: 2, + message: expect.stringContaining('3.1'), + }), + ]); + }); + + it('applies the same warnings to the deprecated change validate command', async () => { + const result = await runCLI( + ['change', 'validate', 'bad-numbering', '--strict'], + { cwd: projectDir } + ); + + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('Task "10.7" is under group 11'); + expect(result.stderr).toContain('Task ID "11.1" is duplicated'); + }); +}); diff --git a/test/core/task-numbering.test.ts b/test/core/task-numbering.test.ts new file mode 100644 index 0000000000..aa3f36a941 --- /dev/null +++ b/test/core/task-numbering.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it } from 'vitest'; +import { findTaskNumberingIssues } from '../../src/core/validation/task-numbering.js'; + +describe('findTaskNumberingIssues', () => { + it('matches duplicate ids at full depth', () => { + const issues = findTaskNumberingIssues( + [ + '## 3. Work', + '- [ ] 3.2.1 first child', + '- [ ] 3.2.2 second child', + '- [ ] 3.2.1 duplicate child', + '', + ].join('\n') + ); + + expect(issues).toEqual([ + { + line: 4, + message: 'Task ID "3.2.1" is duplicated; it was first declared on line 2.', + }, + ]); + }); + + it('accepts alphabetic suffixes and numbering gaps', () => { + const issues = findTaskNumberingIssues( + ['## 4. Work', '- [ ] 4.2a inserted', '- [ ] 4.2b another', '- [ ] 4.7 gap'].join( + '\r\n' + ) + ); + + expect(issues).toEqual([]); + }); + + it('resets group context at an unnumbered level-two heading', () => { + const issues = findTaskNumberingIssues( + ['## 1. Work', '- [ ] 1.1 task', '## Notes', '- [ ] 9.1 external note'].join('\n') + ); + + expect(issues).toEqual([]); + }); + + it('skips every check in files without numbered groups', () => { + const issues = findTaskNumberingIssues( + [ + '# Tasks', + '- [ ] plain task', + '- [ ] 7.1 numbered but ungrouped', + '- [ ] 7.1 duplicate but still ungrouped', + ].join('\n') + ); + + expect(issues).toEqual([]); + }); + + it('compares group prefixes as integers', () => { + const issues = findTaskNumberingIssues('## 01. Work\n- [ ] 1.1 task\n'); + + expect(issues).toEqual([]); + }); +}); From 668eaae783ac89dda3cdf893883ebb10fd1c7b0c Mon Sep 17 00:00:00 2001 From: alectimison-maker Date: Thu, 6 Aug 2026 15:58:51 +0800 Subject: [PATCH 2/2] fix(validate): honor task numbering review boundaries --- src/core/validation/task-numbering.ts | 103 ++++++++++++------- src/core/validation/validator.ts | 50 ++++++--- test/cli-e2e/validate-task-numbering.test.ts | 15 +-- test/core/task-numbering.test.ts | 43 ++++++-- 4 files changed, 142 insertions(+), 69 deletions(-) diff --git a/src/core/validation/task-numbering.ts b/src/core/validation/task-numbering.ts index bcf9f67115..a77767a260 100644 --- a/src/core/validation/task-numbering.ts +++ b/src/core/validation/task-numbering.ts @@ -1,57 +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 without imposing a contiguous numbering - * scheme. Unnumbered tasks and tasks outside a `## N.` group are intentionally - * ignored because both forms already exist in real projects. + * 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(content: string): TaskNumberingIssue[] { - const lines = content.split('\n'); - if (!lines.some((line) => NUMBERED_GROUP_HEADING.test(line))) return []; - +export function findTaskNumberingIssues( + documents: readonly TaskNumberingDocument[] +): TaskNumberingIssue[] { const issues: TaskNumberingIssue[] = []; - const firstLineById = new Map(); - let currentGroup: string | undefined; - - lines.forEach((line, index) => { - if (LEVEL_TWO_HEADING.test(line)) { - currentGroup = line.match(NUMBERED_GROUP_HEADING)?.[1]; - } - - 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 (normalizedCurrentGroup !== undefined && normalizedTaskGroup !== normalizedCurrentGroup) { - issues.push({ - 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 firstLine = firstLineById.get(id); - if (firstLine !== undefined) { - issues.push({ - line: lineNumber, - message: `Task ID "${id}" is duplicated; it was first declared on line ${firstLine}.`, - }); - } else { - firstLineById.set(id, lineNumber); - } - }); + const firstLocationById = new Map(); + + 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; } diff --git a/src/core/validation/validator.ts b/src/core/validation/validator.ts index 87d4a332b5..56f771e1a8 100644 --- a/src/core/validation/validator.ts +++ b/src/core/validation/validator.ts @@ -26,9 +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; @@ -464,6 +469,25 @@ export class Validator { changeDir: string, projectRoot: string ): Promise { + 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); @@ -474,7 +498,7 @@ export class Validator { taskFiles = [path.join(changeDir, 'tasks.md')]; } - const issues: ValidationIssue[] = []; + const documents: Array<{ path: string; content: string }> = []; for (const taskFile of taskFiles) { let content: string; try { @@ -483,17 +507,19 @@ export class Validator { continue; } - const entryPath = FileSystemUtils.toPosixPath(path.relative(changeDir, taskFile)); - for (const issue of findTaskNumberingIssues(content)) { - issues.push({ - level: 'WARNING', - path: entryPath, - line: issue.line, - message: issue.message, - }); - } + documents.push({ + path: FileSystemUtils.toPosixPath(path.relative(changeDir, taskFile)), + content, + }); } - return issues; + + 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, + })); } /** diff --git a/test/cli-e2e/validate-task-numbering.test.ts b/test/cli-e2e/validate-task-numbering.test.ts index f768d03812..2d6a133b12 100644 --- a/test/cli-e2e/validate-task-numbering.test.ts +++ b/test/cli-e2e/validate-task-numbering.test.ts @@ -181,28 +181,21 @@ describe('openspec validate checks task numbering (#1520)', () => { ); expect(byId['bad-numbering']).toBe(false); expect(byId['valid-numbering']).toBe(true); - expect(byId['nested-numbering']).toBe(false); + expect(byId['nested-numbering']).toBe(true); }); - it('validates every task file selected by the schema glob', async () => { + it('does not apply the built-in numbering grammar to a custom schema', async () => { const result = await runCLI( ['validate', '--type', 'change', 'nested-numbering', '--strict', '--json'], { cwd: projectDir } ); - expect(result.exitCode).toBe(1); + expect(result.exitCode).toBe(0); const report = JSON.parse(result.stdout); const taskIssues = report.items[0].issues.filter( (issue: { path: string }) => issue.path.endsWith('tasks.md') ); - expect(taskIssues).toEqual([ - expect.objectContaining({ - level: 'WARNING', - path: 'backend/tasks.md', - line: 2, - message: expect.stringContaining('3.1'), - }), - ]); + expect(taskIssues).toEqual([]); }); it('applies the same warnings to the deprecated change validate command', async () => { diff --git a/test/core/task-numbering.test.ts b/test/core/task-numbering.test.ts index aa3f36a941..709bede411 100644 --- a/test/core/task-numbering.test.ts +++ b/test/core/task-numbering.test.ts @@ -1,9 +1,12 @@ import { describe, expect, it } from 'vitest'; import { findTaskNumberingIssues } from '../../src/core/validation/task-numbering.js'; +const findInSingleFile = (content: string) => + findTaskNumberingIssues([{ path: 'tasks.md', content }]).map(({ path: _path, ...issue }) => issue); + describe('findTaskNumberingIssues', () => { it('matches duplicate ids at full depth', () => { - const issues = findTaskNumberingIssues( + const issues = findInSingleFile( [ '## 3. Work', '- [ ] 3.2.1 first child', @@ -22,7 +25,7 @@ describe('findTaskNumberingIssues', () => { }); it('accepts alphabetic suffixes and numbering gaps', () => { - const issues = findTaskNumberingIssues( + const issues = findInSingleFile( ['## 4. Work', '- [ ] 4.2a inserted', '- [ ] 4.2b another', '- [ ] 4.7 gap'].join( '\r\n' ) @@ -32,15 +35,21 @@ describe('findTaskNumberingIssues', () => { }); it('resets group context at an unnumbered level-two heading', () => { - const issues = findTaskNumberingIssues( - ['## 1. Work', '- [ ] 1.1 task', '## Notes', '- [ ] 9.1 external note'].join('\n') + const issues = findInSingleFile( + [ + '## 1. Work', + '- [ ] 1.1 task', + '## Notes', + '- [ ] 9.1 external note', + '- [ ] 9.1 repeated external note', + ].join('\n') ); expect(issues).toEqual([]); }); it('skips every check in files without numbered groups', () => { - const issues = findTaskNumberingIssues( + const issues = findInSingleFile( [ '# Tasks', '- [ ] plain task', @@ -53,8 +62,30 @@ describe('findTaskNumberingIssues', () => { }); it('compares group prefixes as integers', () => { - const issues = findTaskNumberingIssues('## 01. Work\n- [ ] 1.1 task\n'); + const issues = findInSingleFile('## 01. Work\n- [ ] 1.1 task\n'); expect(issues).toEqual([]); }); + + it('detects duplicate ids across task files', () => { + const issues = findTaskNumberingIssues([ + { + path: 'backend/tasks.md', + content: '## 2. Backend\n- [ ] 2.1 shared task\n', + }, + { + path: 'frontend/tasks.md', + content: '## 2. Frontend\n- [ ] 2.1 duplicate task\n', + }, + ]); + + expect(issues).toEqual([ + { + path: 'frontend/tasks.md', + line: 2, + message: + 'Task ID "2.1" is duplicated; it was first declared in backend/tasks.md on line 2.', + }, + ]); + }); });