From 2f6835fadde27fe5fa78e9953092f8da8f6aed6d Mon Sep 17 00:00:00 2001 From: atman-33 Date: Sun, 25 Jan 2026 21:32:46 +0900 Subject: [PATCH 1/3] feat: add task validation to Validator class --- .../changes/add-tasks-validation/proposal.md | 28 +++++++ .../specs/cli-validate/spec.md | 49 +++++++++++++ .../changes/add-tasks-validation/tasks.md | 33 +++++++++ src/commands/validate.ts | 21 +++++- src/core/validation/validator.ts | 61 ++++++++++++++++ src/utils/task-progress.ts | 4 +- test/commands/validate.test.ts | 29 ++++++++ test/core/validation.test.ts | 73 +++++++++++++++++++ .../tmp-init/openspec/changes/c1/tasks.md | 6 ++ 9 files changed, 300 insertions(+), 4 deletions(-) create mode 100644 openspec/changes/add-tasks-validation/proposal.md create mode 100644 openspec/changes/add-tasks-validation/specs/cli-validate/spec.md create mode 100644 openspec/changes/add-tasks-validation/tasks.md create mode 100644 test/fixtures/tmp-init/openspec/changes/c1/tasks.md diff --git a/openspec/changes/add-tasks-validation/proposal.md b/openspec/changes/add-tasks-validation/proposal.md new file mode 100644 index 000000000..d9d6681d0 --- /dev/null +++ b/openspec/changes/add-tasks-validation/proposal.md @@ -0,0 +1,28 @@ +# Change: Add tasks.md Validation + +## Why + +AI agents generate OpenSpec change proposals including `tasks.md` as implementation checklists. Currently, `openspec validate` checks `proposal.md` and spec deltas, but does not validate `tasks.md`. This can lead to: +- Missing or empty tasks.md files +- Tasks without checkboxes (making progress tracking impossible) +- Empty task descriptions that provide no implementation guidance + +Adding basic tasks.md validation ensures AI-generated proposals include properly formatted implementation checklists before approval. + +## What Changes + +- Add tasks.md validation to the `Validator` class +- Validate tasks.md when running `openspec validate ` +- Check three essential requirements: + 1. File exists + 2. At least one checkboxed task is present + 3. No empty task descriptions +- Use the same checkbox pattern as existing `task-progress.ts` (`/^[-*]\s+\[[xX\s]\]/`) +- Report validation errors with line numbers for easy fixing + +## Impact + +- **Affected specs**: cli-validate +- **Affected code**: `src/core/validation/validator.ts`, `src/commands/validate.ts` +- **Breaking changes**: None (only adds new validation checks) +- **Backward compatibility**: Existing changes may fail validation if tasks.md is missing or improperly formatted diff --git a/openspec/changes/add-tasks-validation/specs/cli-validate/spec.md b/openspec/changes/add-tasks-validation/specs/cli-validate/spec.md new file mode 100644 index 000000000..4e6770d2e --- /dev/null +++ b/openspec/changes/add-tasks-validation/specs/cli-validate/spec.md @@ -0,0 +1,49 @@ +# cli-validate Spec Delta + +## ADDED Requirements + +### Requirement: Validator SHALL validate tasks.md format in changes + +When validating changes, the validator SHALL check that tasks.md exists, contains at least one checkboxed task, and has no empty task descriptions. + +#### Scenario: Missing tasks.md file + +- **WHEN** validating a change that does not have tasks.md +- **THEN** report ERROR with message "tasks.md is required for OpenSpec changes" +- **AND** include path "tasks.md" in the issue + +#### Scenario: No checkboxed tasks found + +- **WHEN** validating a tasks.md file with no items matching pattern `/^[-*]\s+\[[xX\s]\]/` +- **THEN** report ERROR with message "tasks.md must contain at least one checkboxed task" +- **AND** include path "tasks.md" in the issue + +#### Scenario: Empty task description detected + +- **WHEN** validating a tasks.md file containing a line matching `/^[-*]\s+\[[xX\s]\]\s*$/` +- **THEN** report ERROR with message "Empty task description" +- **AND** include path "tasks.md:N" where N is the line number (1-indexed) + +#### Scenario: Valid tasks.md with checkboxed tasks + +- **WHEN** validating a tasks.md file with one or more properly formatted checkboxed tasks +- **THEN** report no errors for tasks.md +- **AND** include tasks.md in the validation summary + +#### Scenario: Tasks.md validation integrated with change validation + +- **WHEN** running `openspec validate ` +- **THEN** validate proposal.md, spec deltas, and tasks.md +- **AND** display results for all validated files +- **AND** exit with code 1 if any validation fails + +### Requirement: Checkbox pattern SHALL match task-progress utility + +The tasks.md validator SHALL use the same checkbox detection pattern as `src/utils/task-progress.ts` to ensure consistency across the codebase. + +#### Scenario: Checkbox pattern consistency + +- **WHEN** detecting checkboxed tasks in tasks.md +- **THEN** use pattern `/^[-*]\s+\[[xX\s]\]/` for checkbox detection +- **AND** accept both lowercase and uppercase X for completed tasks +- **AND** accept both `-` and `*` as list markers diff --git a/openspec/changes/add-tasks-validation/tasks.md b/openspec/changes/add-tasks-validation/tasks.md new file mode 100644 index 000000000..d11f51e31 --- /dev/null +++ b/openspec/changes/add-tasks-validation/tasks.md @@ -0,0 +1,33 @@ +# Tasks: Add tasks.md Validation + +## 1. Add Validation Method to Validator Class + +- [x] 1.1 Add `validateTasksFile(changeDir: string): Promise` method to Validator class +- [x] 1.2 Implement file existence check (ERROR if missing) +- [x] 1.3 Add `validateTasksContent(content: string): ValidationIssue[]` private method +- [x] 1.4 Implement checkbox task detection using pattern `/^[-*]\s+\[[xX\s]\]/` +- [x] 1.5 Check for at least one checkboxed task (ERROR if none found) +- [x] 1.6 Detect empty task descriptions with pattern `/^[-*]\s+\[[xX\s]\]\s*$/` (ERROR) +- [x] 1.7 Include line numbers in error messages for empty tasks + +## 2. Integrate with Validate Command + +- [x] 2.1 Update `validateDirectItem()` in ValidateCommand to call tasks.md validation for changes +- [x] 2.2 Update `validateByType()` to include tasks.md validation when type is 'change' +- [x] 2.3 Update `runBulkValidation()` to validate tasks.md for each change +- [x] 2.4 Ensure tasks.md validation results are displayed alongside proposal.md and spec results + +## 3. Testing + +- [x] 3.1 Add unit tests for `validateTasksContent()` with various checkbox formats +- [x] 3.2 Test error case: tasks.md file missing +- [x] 3.3 Test error case: no checkboxed tasks found +- [x] 3.4 Test error case: empty task descriptions +- [x] 3.5 Test valid case: properly formatted tasks.md with multiple tasks +- [x] 3.6 Test pattern compatibility with existing task-progress.ts regex +- [x] 3.7 Add integration test for `openspec validate ` including tasks.md + +## 4. Documentation + +- [x] 4.1 Update error messages to be clear and actionable +- [x] 4.2 Ensure validation output shows tasks.md status alongside other files diff --git a/src/commands/validate.ts b/src/commands/validate.ts index 9e59a4d48..ea6e4d19c 100644 --- a/src/commands/validate.ts +++ b/src/commands/validate.ts @@ -127,12 +127,29 @@ export class ValidateCommand { await this.validateByType(type, itemName, opts); } + private async validateChangeReports(changeDir: string, validator: Validator) { + const [deltaReport, tasksReport] = await Promise.all([ + validator.validateChangeDeltaSpecs(changeDir), + validator.validateTasksFile(changeDir), + ]); + + return { + valid: deltaReport.valid && tasksReport.valid, + issues: [...deltaReport.issues, ...tasksReport.issues], + summary: { + errors: deltaReport.summary.errors + tasksReport.summary.errors, + warnings: deltaReport.summary.warnings + tasksReport.summary.warnings, + info: deltaReport.summary.info + tasksReport.summary.info, + }, + }; + } + private async validateByType(type: ItemType, id: string, opts: { strict: boolean; json: boolean }): Promise { const validator = new Validator(opts.strict); if (type === 'change') { const changeDir = path.join(process.cwd(), 'openspec', 'changes', id); const start = Date.now(); - const report = await validator.validateChangeDeltaSpecs(changeDir); + const report = await this.validateChangeReports(changeDir, validator); const durationMs = Date.now() - start; this.printReport('change', id, report, durationMs, opts.json); // Non-zero exit if invalid (keeps enriched output test semantics) @@ -198,7 +215,7 @@ export class ValidateCommand { queue.push(async () => { const start = Date.now(); const changeDir = path.join(process.cwd(), 'openspec', 'changes', id); - const report = await validator.validateChangeDeltaSpecs(changeDir); + const report = await this.validateChangeReports(changeDir, validator); const durationMs = Date.now() - start; return { id, type: 'change' as const, valid: report.valid, issues: report.issues, durationMs }; }); diff --git a/src/core/validation/validator.ts b/src/core/validation/validator.ts index e6928cbda..9858c83e1 100644 --- a/src/core/validation/validator.ts +++ b/src/core/validation/validator.ts @@ -12,6 +12,10 @@ import { } from './constants.js'; import { parseDeltaSpec, normalizeRequirementName } from '../parsers/requirement-blocks.js'; import { FileSystemUtils } from '../../utils/file-system.js'; +import { TASK_PATTERN } from '../../utils/task-progress.js'; + +const TASK_CHECKBOX_PATTERN = TASK_PATTERN; +const EMPTY_TASK_PATTERN = /^[-*]\s+\[[xX\s]\]\s*$/; export class Validator { private strictMode: boolean; @@ -272,6 +276,63 @@ export class Validator { return this.createReport(issues); } + async validateTasksFile(changeDir: string): Promise { + const issues: ValidationIssue[] = []; + const tasksPath = path.join(changeDir, 'tasks.md'); + try { + const content = await fs.readFile(tasksPath, 'utf-8'); + issues.push(...this.validateTasksContent(content)); + } catch (error) { + const err = error as NodeJS.ErrnoException; + if (err?.code === 'ENOENT') { + issues.push({ + level: 'ERROR', + path: 'tasks.md', + message: 'tasks.md is required for OpenSpec changes', + }); + } else { + const baseMessage = err instanceof Error ? err.message : 'Unknown error'; + issues.push({ + level: 'ERROR', + path: 'tasks.md', + message: `Unable to read tasks.md: ${baseMessage}`, + }); + } + } + return this.createReport(issues); + } + + private validateTasksContent(content: string): ValidationIssue[] { + const issues: ValidationIssue[] = []; + const lines = content.split(/\r?\n/); + let hasCheckbox = false; + + lines.forEach((line, index) => { + if (TASK_CHECKBOX_PATTERN.test(line)) { + hasCheckbox = true; + if (EMPTY_TASK_PATTERN.test(line)) { + const lineNumber = index + 1; + issues.push({ + level: 'ERROR', + path: `tasks.md:${lineNumber}`, + message: 'Empty task description', + line: lineNumber, + }); + } + } + }); + + if (!hasCheckbox) { + issues.push({ + level: 'ERROR', + path: 'tasks.md', + message: 'tasks.md must contain at least one checkboxed task', + }); + } + + return issues; + } + private convertZodErrors(error: ZodError): ValidationIssue[] { return error.issues.map(err => { let message = err.message; diff --git a/src/utils/task-progress.ts b/src/utils/task-progress.ts index a14b866f0..3b1eef858 100644 --- a/src/utils/task-progress.ts +++ b/src/utils/task-progress.ts @@ -1,8 +1,8 @@ import { promises as fs } from 'fs'; import path from 'path'; -const TASK_PATTERN = /^[-*]\s+\[[\sx]\]/i; -const COMPLETED_TASK_PATTERN = /^[-*]\s+\[x\]/i; +export const TASK_PATTERN = /^[-*]\s+\[[\sx]\]/i; +export const COMPLETED_TASK_PATTERN = /^[-*]\s+\[x\]/i; export interface TaskProgress { total: number; diff --git a/test/commands/validate.test.ts b/test/commands/validate.test.ts index b94f72d35..f06f387ff 100644 --- a/test/commands/validate.test.ts +++ b/test/commands/validate.test.ts @@ -35,6 +35,8 @@ describe('top-level validate command', () => { const changeContent = `# Test Change\n\n## Why\nBecause reasons that are sufficiently long for validation.\n\n## What Changes\n- **alpha:** Add something`; await fs.mkdir(path.join(changesDir, 'c1'), { recursive: true }); await fs.writeFile(path.join(changesDir, 'c1', 'proposal.md'), changeContent, 'utf-8'); + const tasksContent = ['# Tasks', '', '- [ ] Initial task'].join('\n'); + await fs.writeFile(path.join(changesDir, 'c1', 'tasks.md'), tasksContent, 'utf-8'); const deltaContent = [ '## ADDED Requirements', '### Requirement: Validator SHALL support alpha change deltas', @@ -52,6 +54,7 @@ describe('top-level validate command', () => { // Duplicate name for ambiguity test await fs.mkdir(path.join(changesDir, 'dup'), { recursive: true }); await fs.writeFile(path.join(changesDir, 'dup', 'proposal.md'), changeContent, 'utf-8'); + await fs.writeFile(path.join(changesDir, 'dup', 'tasks.md'), tasksContent, 'utf-8'); const dupDeltaDir = path.join(changesDir, 'dup', 'specs', 'dup'); await fs.mkdir(dupDeltaDir, { recursive: true }); await fs.writeFile(path.join(dupDeltaDir, 'spec.md'), deltaContent, 'utf-8'); @@ -111,6 +114,7 @@ describe('top-level validate command', () => { await fs.mkdir(path.join(changesDir, changeId), { recursive: true }); await fs.writeFile(path.join(changesDir, changeId, 'proposal.md'), crlfContent, 'utf-8'); + await fs.writeFile(path.join(changesDir, changeId, 'tasks.md'), toCrlf(['# Tasks', '', '- [ ] Verify CRLF']), 'utf-8'); const deltaContent = toCrlf([ '## ADDED Requirements', @@ -131,6 +135,31 @@ describe('top-level validate command', () => { expect(result.exitCode).toBe(0); }); + it('fails validation when tasks.md is missing', async () => { + const changeId = 'missing-tasks'; + const changeContent = `# Missing Tasks\n\n## Why\nThis change intentionally lacks tasks.md for validation coverage.\n\n## What Changes\n- **alpha:** Add missing tasks case`; + await fs.mkdir(path.join(changesDir, changeId), { recursive: true }); + await fs.writeFile(path.join(changesDir, changeId, 'proposal.md'), changeContent, 'utf-8'); + + const deltaContent = [ + '## ADDED Requirements', + '### Requirement: Validator SHALL flag missing tasks', + 'The validator SHALL error when tasks.md is missing.', + '', + '#### Scenario: Missing tasks.md', + '- **GIVEN** a change without tasks.md', + '- **WHEN** validate runs', + '- **THEN** an error is reported', + ].join('\n'); + const deltaDir = path.join(changesDir, changeId, 'specs', 'alpha'); + await fs.mkdir(deltaDir, { recursive: true }); + await fs.writeFile(path.join(deltaDir, 'spec.md'), deltaContent, 'utf-8'); + + const result = await runCLI(['validate', changeId], { cwd: testDir }); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('tasks.md is required for OpenSpec changes'); + }); + it('respects --no-interactive flag passed via CLI', async () => { // This test ensures Commander.js --no-interactive flag is correctly parsed // and passed to the validate command. The flag sets options.interactive = false diff --git a/test/core/validation.test.ts b/test/core/validation.test.ts index f7323b36a..c09359b9f 100644 --- a/test/core/validation.test.ts +++ b/test/core/validation.test.ts @@ -288,6 +288,79 @@ We need to implement user authentication to secure the application and protect u }); }); + describe('validateTasksContent', () => { + it('accepts checkboxed tasks with dash or asterisk markers', () => { + const content = [ + '- [ ] First task', + '* [x] Second task', + '* [X] Third task', + ].join('\n'); + + const validator = new Validator(); + const issues = (validator as any).validateTasksContent(content); + + expect(issues.length).toBe(0); + }); + + it('reports an error when no checkboxed tasks are found', () => { + const content = [ + '- Task without checkbox', + '* Another task without checkbox', + ].join('\n'); + + const validator = new Validator(); + const issues = (validator as any).validateTasksContent(content); + + expect(issues.some((issue: any) => issue.message.includes('checkboxed task'))).toBe(true); + }); + + it('reports empty task descriptions with line numbers', () => { + const content = [ + '- [ ]', + '- [ ] Valid task', + ].join('\n'); + + const validator = new Validator(); + const issues = (validator as any).validateTasksContent(content); + + expect(issues.some((issue: any) => issue.path === 'tasks.md:1')).toBe(true); + expect(issues.some((issue: any) => issue.message === 'Empty task description')).toBe(true); + expect(issues.some((issue: any) => issue.line === 1)).toBe(true); + }); + }); + + describe('validateTasksFile', () => { + it('fails when tasks.md is missing', async () => { + const changeDir = path.join(testDir, 'missing-tasks-change'); + await fs.mkdir(changeDir, { recursive: true }); + + const validator = new Validator(); + const report = await validator.validateTasksFile(changeDir); + + expect(report.valid).toBe(false); + expect(report.issues.some(i => i.message.includes('tasks.md is required'))).toBe(true); + }); + + it('passes when tasks.md contains checkboxed tasks', async () => { + const changeDir = path.join(testDir, 'valid-tasks-change'); + await fs.mkdir(changeDir, { recursive: true }); + + const tasksContent = [ + '# Tasks', + '', + '- [ ] First task', + '- [x] Completed task', + ].join('\n'); + await fs.writeFile(path.join(changeDir, 'tasks.md'), tasksContent, 'utf-8'); + + const validator = new Validator(); + const report = await validator.validateTasksFile(changeDir); + + expect(report.valid).toBe(true); + expect(report.summary.errors).toBe(0); + }); + }); + describe('strict mode', () => { it('should fail on warnings in strict mode', async () => { const specContent = `# Test Spec diff --git a/test/fixtures/tmp-init/openspec/changes/c1/tasks.md b/test/fixtures/tmp-init/openspec/changes/c1/tasks.md new file mode 100644 index 000000000..5fd04e9ad --- /dev/null +++ b/test/fixtures/tmp-init/openspec/changes/c1/tasks.md @@ -0,0 +1,6 @@ +# Tasks + +## Implementation + +- [ ] Add fixture test case +- [ ] Validate fixture with CLI From c11457471b10fa63fbc9b49ce99c10c415080738 Mon Sep 17 00:00:00 2001 From: atman-33 Date: Sun, 25 Jan 2026 21:40:39 +0900 Subject: [PATCH 2/3] docs: archive add-tasks-validation change and update spec --- .../proposal.md | 0 .../specs/cli-validate/spec.md | 0 .../2026-01-25-add-tasks-validation}/tasks.md | 0 openspec/specs/cli-validate/spec.md | 46 +++++++++++++++++++ 4 files changed, 46 insertions(+) rename openspec/changes/{add-tasks-validation => archive/2026-01-25-add-tasks-validation}/proposal.md (100%) rename openspec/changes/{add-tasks-validation => archive/2026-01-25-add-tasks-validation}/specs/cli-validate/spec.md (100%) rename openspec/changes/{add-tasks-validation => archive/2026-01-25-add-tasks-validation}/tasks.md (100%) diff --git a/openspec/changes/add-tasks-validation/proposal.md b/openspec/changes/archive/2026-01-25-add-tasks-validation/proposal.md similarity index 100% rename from openspec/changes/add-tasks-validation/proposal.md rename to openspec/changes/archive/2026-01-25-add-tasks-validation/proposal.md diff --git a/openspec/changes/add-tasks-validation/specs/cli-validate/spec.md b/openspec/changes/archive/2026-01-25-add-tasks-validation/specs/cli-validate/spec.md similarity index 100% rename from openspec/changes/add-tasks-validation/specs/cli-validate/spec.md rename to openspec/changes/archive/2026-01-25-add-tasks-validation/specs/cli-validate/spec.md diff --git a/openspec/changes/add-tasks-validation/tasks.md b/openspec/changes/archive/2026-01-25-add-tasks-validation/tasks.md similarity index 100% rename from openspec/changes/add-tasks-validation/tasks.md rename to openspec/changes/archive/2026-01-25-add-tasks-validation/tasks.md diff --git a/openspec/specs/cli-validate/spec.md b/openspec/specs/cli-validate/spec.md index 5e543d230..cb3b6b7eb 100644 --- a/openspec/specs/cli-validate/spec.md +++ b/openspec/specs/cli-validate/spec.md @@ -216,3 +216,49 @@ The markdown parser SHALL correctly identify sections regardless of line ending - **WHEN** running `openspec validate ` - **THEN** validation SHALL recognize the sections and NOT raise parsing errors +### Requirement: Validator SHALL validate tasks.md format in changes + +When validating changes, the validator SHALL check that tasks.md exists, contains at least one checkboxed task, and has no empty task descriptions. + +#### Scenario: Missing tasks.md file + +- **WHEN** validating a change that does not have tasks.md +- **THEN** report ERROR with message "tasks.md is required for OpenSpec changes" +- **AND** include path "tasks.md" in the issue + +#### Scenario: No checkboxed tasks found + +- **WHEN** validating a tasks.md file with no items matching pattern `/^[-*]\s+\[[xX\s]\]/` +- **THEN** report ERROR with message "tasks.md must contain at least one checkboxed task" +- **AND** include path "tasks.md" in the issue + +#### Scenario: Empty task description detected + +- **WHEN** validating a tasks.md file containing a line matching `/^[-*]\s+\[[xX\s]\]\s*$/` +- **THEN** report ERROR with message "Empty task description" +- **AND** include path "tasks.md:N" where N is the line number (1-indexed) + +#### Scenario: Valid tasks.md with checkboxed tasks + +- **WHEN** validating a tasks.md file with one or more properly formatted checkboxed tasks +- **THEN** report no errors for tasks.md +- **AND** include tasks.md in the validation summary + +#### Scenario: Tasks.md validation integrated with change validation + +- **WHEN** running `openspec validate ` +- **THEN** validate proposal.md, spec deltas, and tasks.md +- **AND** display results for all validated files +- **AND** exit with code 1 if any validation fails + +### Requirement: Checkbox pattern SHALL match task-progress utility + +The tasks.md validator SHALL use the same checkbox detection pattern as `src/utils/task-progress.ts` to ensure consistency across the codebase. + +#### Scenario: Checkbox pattern consistency + +- **WHEN** detecting checkboxed tasks in tasks.md +- **THEN** use pattern `/^[-*]\s+\[[xX\s]\]/` for checkbox detection +- **AND** accept both lowercase and uppercase X for completed tasks +- **AND** accept both `-` and `*` as list markers + From 014a2d3ed39e5c35372e4dca920c4f97c0155395 Mon Sep 17 00:00:00 2001 From: atman-33 Date: Sun, 25 Jan 2026 22:04:06 +0900 Subject: [PATCH 3/3] docs: align tasks.md regex with TASK_PATTERN implementation --- openspec/specs/cli-validate/spec.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openspec/specs/cli-validate/spec.md b/openspec/specs/cli-validate/spec.md index cb3b6b7eb..96fcef666 100644 --- a/openspec/specs/cli-validate/spec.md +++ b/openspec/specs/cli-validate/spec.md @@ -228,7 +228,7 @@ When validating changes, the validator SHALL check that tasks.md exists, contain #### Scenario: No checkboxed tasks found -- **WHEN** validating a tasks.md file with no items matching pattern `/^[-*]\s+\[[xX\s]\]/` +- **WHEN** validating a tasks.md file with no items matching pattern `/^[-*]\s+\[[\sx]\]/i` - **THEN** report ERROR with message "tasks.md must contain at least one checkboxed task" - **AND** include path "tasks.md" in the issue