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
5 changes: 5 additions & 0 deletions .changeset/validate-archived-tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@fission-ai/openspec": minor
---

Add `openspec validate --archived`: an opt-in check that every change under `changes/archive/` has all of its `tasks.md` checkboxes ticked, exiting non-zero if any are unchecked. This surfaces changes that were archived with unfinished work — which the normal validate flow never catches, because it only looks at active changes — and is meant for a pre-commit or CI hook (#205). It is a standalone scope: it does not alter any existing `validate` invocation and does not re-validate already-applied spec deltas.
6 changes: 6 additions & 0 deletions docs/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -553,12 +553,15 @@ A change with zero spec deltas fails validation unless its `.openspec.yaml` decl
| `--all` | Validate all changes and specs |
| `--changes` | Validate all changes |
| `--specs` | Validate all specs |
| `--archived` | Validate that archived changes have all tasks completed (for pre-commit linting) |
| `--type <type>` | Specify type when name is ambiguous: `change` or `spec` |
| `--strict` | Enable strict validation mode |
| `--json` | Output as JSON |
| `--concurrency <n>` | Max parallel validations (default: 6, or `OPENSPEC_CONCURRENCY` env) |
| `--no-interactive` | Disable prompts |

`--archived` is its own scope: it does not validate spec deltas (already applied at archive time), it verifies that every change under `changes/archive/` has all of its `tasks.md` checkboxes ticked, exiting non-zero if any are unchecked. This catches changes that were archived with unfinished work — handy in a pre-commit hook.

**Examples:**

```bash
Expand All @@ -576,6 +579,9 @@ openspec validate --all --json

# Strict validation with increased parallelism
openspec validate --all --strict --concurrency 12

# Fail if any archived change still has unchecked tasks
openspec validate --archived
```

**Output (text):**
Expand Down
1 change: 1 addition & 0 deletions docs/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ Validation checks your specs and changes for structural problems. Read the messa
openspec validate <name> # validate one item
openspec validate --all # validate everything
openspec validate --all --strict # stricter checks, good for CI
openspec validate --archived # fail if archived changes have unchecked tasks
```

Common causes are a missing required section (like a spec with no scenarios) or a malformed delta header. Fix the file and re-run. The [CLI reference](cli.md#openspec-validate) documents the output format.
Expand Down
3 changes: 2 additions & 1 deletion src/cli/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -439,14 +439,15 @@ program
.option('--all', 'Validate all changes and specs')
.option('--changes', 'Validate all changes')
.option('--specs', 'Validate all specs')
.option('--archived', 'Validate that archived changes have all tasks completed (for pre-commit linting)')
.option('--type <type>', 'Specify item type when ambiguous: change|spec')
.option('--strict', 'Enable strict validation mode')
.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')
.option('--store <id>', STORE_OPTION_DESCRIPTION)
.addOption(hiddenStorePathOption())
.action(async (itemName?: string, options?: { all?: boolean; changes?: boolean; specs?: boolean; type?: string; strict?: boolean; json?: boolean; noInteractive?: boolean; concurrency?: string; store?: string; storePath?: string }) => {
.action(async (itemName?: string, options?: { all?: boolean; changes?: boolean; specs?: boolean; archived?: boolean; type?: string; strict?: boolean; json?: boolean; noInteractive?: boolean; concurrency?: string; store?: string; storePath?: string }) => {
try {
const validateCommand = new ValidateCommand();
await validateCommand.execute(itemName, options);
Expand Down
140 changes: 140 additions & 0 deletions src/commands/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,17 @@ import { isInteractive, resolveNoInteractive } from '../utils/interactive.js';
import { getSpecIds } from '../utils/item-discovery.js';
import { getAvailableChanges } from './workflow/shared.js';
import { nearestMatches } from '../utils/match.js';
import { promises as fs } from 'fs';
import { getTaskProgressDetailForChange, type SchemaGlobCache } from '../utils/task-progress.js';
import { FileSystemUtils } from '../utils/file-system.js';

type ItemType = 'change' | 'spec';

interface ExecuteOptions {
all?: boolean;
changes?: boolean;
specs?: boolean;
archived?: boolean;
type?: string;
strict?: boolean;
json?: boolean;
Expand Down Expand Up @@ -47,6 +51,18 @@ export class ValidateCommand {

const interactive = isInteractive(options);

// Archived-task linting is its own scope: it checks task completion of
// already-archived changes, not delta specs (whose operations are already
// applied). Handled before the other bulk flags so `--archived` is explicit
// and never alters an existing invocation's behavior (#205).
if (options.archived) {
await this.runArchivedTaskValidation(root, {
json: !!options.json,
noInteractive: resolveNoInteractive(options),
});
return;
}

// Handle bulk flags first
if (options.all || options.changes || options.specs) {
await this.runBulkValidation(root, {
Expand Down Expand Up @@ -387,6 +403,130 @@ export class ValidateCommand {

process.exitCode = failed > 0 ? 1 : 0;
}

/**
* Lists archived change ids from the resolved root's archive directory,
* mirroring `getArchivedChangeIds` but store-aware (uses `root.archiveDir`
* rather than a cwd-relative path). Directories only, hidden entries skipped.
*
* Only a missing archive directory (ENOENT) is an empty list; a permission
* error, an I/O error, or an `archive` path that is a file (ENOTDIR) is a real
* failure and must not read as "no archived changes" — that would let a
* pre-commit lint pass without inspecting anything (#205).
*/
private async listArchivedChangeIds(root: ResolvedOpenSpecRoot): Promise<string[]> {
try {
const entries = await fs.readdir(root.archiveDir, { withFileTypes: true });
return entries
.filter((entry) => entry.isDirectory() && !entry.name.startsWith('.'))
.map((entry) => entry.name)
.sort();
} catch (error: any) {
if (error?.code === 'ENOENT') return [];
throw error;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

/**
* Validates that every archived change has all of its tasks completed.
*
* An archived change is expected to be finished; an archived change with
* unchecked tasks is a real integrity problem the normal validate flow never
* surfaces, because active-change discovery excludes the archive directory
* (#205). Reuses the same task-progress counting `status`, `list`, and
* `archive` rely on, so what counts as a task never forks. Changes with no
* tasks pass (nothing to complete).
*/
private async runArchivedTaskValidation(
root: ResolvedOpenSpecRoot,
opts: { json: boolean; noInteractive?: boolean }
): Promise<void> {
// List first (may throw on a real archive-read failure), then start the
// spinner so a thrown error never leaves a spinner spinning.
const ids = await this.listArchivedChangeIds(root);
const spinner = !opts.json && !opts.noInteractive ? ora('Validating archived changes...').start() : undefined;

// The archive is append-only and can hold thousands of changes; a single
// run resolves them all under one constant projectRoot (root.path), so
// memoize the schema→glob lookup to avoid re-parsing the same schema.yaml
// once per change. The loop is intentionally sequential: the per-change work
// is dominated by synchronous schema/config resolution, which a promise pool
// cannot overlap on Node's single thread — a pool would add complexity for
// no real gain here.
const schemaGlobCache: SchemaGlobCache = new Map();
const results: BulkItemResult[] = [];
let passed = 0;
let failed = 0;
for (const id of ids) {
const start = Date.now();
const issues: BulkItemResult['issues'] = [];
try {
// The explicit root.path override is load-bearing: an archived change
// lives one directory deeper (changes/archive/<id>), so the default
// "../../.." projectRoot derivation would be wrong without it.
const progress = await getTaskProgressDetailForChange(root.archiveDir, id, root.path, schemaGlobCache);
// A tasks file that exists but cannot be read must fail loudly, not be
// silently counted as "no tasks" and pass. Report one issue per file,
// pathed like every other validate issue (POSIX, root-relative).
for (const file of progress.unreadable) {
issues.push({
level: 'ERROR',
path: FileSystemUtils.toPosixPath(path.relative(root.path, file)),
message: 'could not read task file',
});
}
const incomplete = Math.max(progress.total - progress.completed, 0);
if (incomplete > 0) {
issues.push({
level: 'ERROR',
path: 'tasks.md',
message: `${incomplete} incomplete task${incomplete === 1 ? '' : 's'} (${progress.completed}/${progress.total} completed)`,
});
}
} catch (error: any) {
issues.push({ level: 'ERROR', path: 'tasks.md', message: error?.message || 'Unknown error' });
}
const valid = issues.length === 0;
if (valid) passed++; else failed++;
results.push({ id, type: 'change', valid, issues, durationMs: Date.now() - start });
}

spinner?.stop();

const summary = {
totals: { items: results.length, passed, failed },
byType: { change: summarizeType(results, 'change') },
} as const;

if (opts.json) {
const out = { items: results, summary, version: '1.0', root: toRootOutput(root) };
console.log(JSON.stringify(out, null, 2));
process.exitCode = failed > 0 ? 1 : 0;
return;
}

if (results.length === 0) {
console.log('No archived changes found.');
process.exitCode = 0;
return;
}

// Use the same `<type>/<id>` prefix bulk validation prints, so the plain
// output maps to the JSON `type` ('change') and stays greppable the same way.
for (const res of results) {
if (res.valid) {
console.log(`✓ change/${res.id}`);
} else {
console.error(`✗ change/${res.id}`);
for (const issue of res.issues) {
const prefix = issue.level === 'ERROR' ? '✗' : issue.level === 'WARNING' ? '⚠' : 'ℹ';
console.error(` ${prefix} ${issue.message}`);
}
}
}
console.log(`Totals: ${summary.totals.passed} passed, ${summary.totals.failed} failed (${summary.totals.items} items)`);
process.exitCode = failed > 0 ? 1 : 0;
}
}

function summarizeType(results: BulkItemResult[], type: ItemType) {
Expand Down
4 changes: 4 additions & 0 deletions src/core/completions/command-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ export const COMMAND_REGISTRY: CommandDefinition[] = [
name: 'specs',
description: 'Validate all specs',
},
{
name: 'archived',
description: 'Validate that archived changes have all tasks completed (for pre-commit linting)',
},
COMMON_FLAGS.type,
COMMON_FLAGS.strict,
COMMON_FLAGS.jsonValidation,
Expand Down
123 changes: 91 additions & 32 deletions src/utils/task-progress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,71 +79,130 @@ function findTrackedTasksArtifact(schema: SchemaYaml): Artifact | undefined {
return schema.artifacts.find((a) => a.id === 'tasks');
}

/**
* Run-scoped memo mapping a schema name to its tracked-tasks `generates` glob.
* When one command resolves progress for many changes under a constant
* `projectRoot` — e.g. `validate --archived` over an append-only archive — this
* avoids re-reading and re-parsing (YAML + Zod) the same `schema.yaml` once per
* change. Keyed by schema name alone, which is safe *only* because a single run
* holds `projectRoot` constant; never reuse one cache across differing roots.
*/
export type SchemaGlobCache = Map<string, string | undefined>;

/**
* Resolves the tracked-tasks artifact's output glob for a change, or undefined
* when the schema cannot be resolved or no tracked-tasks artifact exists.
* `resolveSchema` throws on an unresolvable/misnamed schema; we swallow that so
* the caller falls back to a single top-level `tasks.md` and never crashes.
* A `schemaGlobCache`, when supplied, memoizes the schema-name → glob lookup for
* the duration of one run.
*/
function resolveTrackedTasksGlob(changeDir: string, projectRoot: string): string | undefined {
function resolveTrackedTasksGlob(
changeDir: string,
projectRoot: string,
schemaGlobCache?: SchemaGlobCache
): string | undefined {
try {
const schemaName = resolveSchemaForChange(changeDir, undefined, projectRoot);
if (schemaGlobCache?.has(schemaName)) return schemaGlobCache.get(schemaName);
const schema = resolveSchema(schemaName, projectRoot);
return findTrackedTasksArtifact(schema)?.generates;
const generates = findTrackedTasksArtifact(schema)?.generates;
schemaGlobCache?.set(schemaName, generates);
return generates;
} catch {
return undefined;
}
}

async function countSingleTopLevelTasksFile(changeDir: string): Promise<TaskProgress> {
const tasksPath = path.join(changeDir, 'tasks.md');
/** Resolves the task files selected by the schema's apply tracking rule. */
export function resolveTaskFilesForChange(
changeDir: string,
projectRoot: string,
schemaGlobCache?: SchemaGlobCache
): string[] {
const generates = resolveTrackedTasksGlob(changeDir, projectRoot, schemaGlobCache);
return generates ? resolveArtifactOutputs(changeDir, generates) : [];
}

export interface TaskProgressDetail extends TaskProgress {
/**
* Task files that exist but could not be read (any error other than ENOENT).
* `getTaskProgressForChange` discards this list to preserve its behavior;
* callers that must fail loudly on an unreadable tasks file — e.g.
* `openspec validate --archived` — read it so an unreadable file is never
* silently counted as "no tasks" (#205).
*/
unreadable: string[];
}

/**
* Reads one task file and counts its checkboxes. ENOENT (a glob file that
* vanished between resolve and read, or the absent single top-level `tasks.md`)
* means zero tasks, exactly as before. Any other error (permissions, I/O,
* ENOTDIR) is recorded in `unreadable` so a caller can surface it; the count
* still contributes zero, so existing callers see no change.
*/
async function countTaskFile(file: string, unreadable: string[]): Promise<TaskProgress> {
try {
const content = await fs.readFile(tasksPath, 'utf-8');
const content = await fs.readFile(file, 'utf-8');
return countTasksFromContent(content);
} catch {
} catch (error: any) {
if (error?.code !== 'ENOENT') unreadable.push(file);
return { total: 0, completed: 0 };
}
}

/** 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`
* glob — the same file-resolution `openspec status` uses to detect the tasks
* artifact (`resolveArtifactOutputs`) — so progress is no longer blind to nested
* `tasks.md` files (#1202). Falls back to a single top-level `tasks.md` (exactly
* as before) when the schema is unresolvable, no tracked-tasks artifact is found,
* or the glob matches no file. Never throws.
* or the glob matches no file. Also reports task files that exist but could not
* be read. Per-file read errors are captured (never thrown); the only throw path
* is a malformed/unsafe schema whose glob resolution rejects (path traversal or
* a linked-directory cycle in `resolveArtifactOutputs`). Pass `schemaGlobCache`
* to memoize schema→glob resolution across many changes in one run.
*/
export async function getTaskProgressForChange(
export async function getTaskProgressDetailForChange(
changesDir: string,
changeName: string,
projectRoot: string
): Promise<TaskProgress> {
projectRoot: string,
schemaGlobCache?: SchemaGlobCache
): Promise<TaskProgressDetail> {
const changeDir = path.join(changesDir, changeName);
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 };
const files = resolveTaskFilesForChange(changeDir, projectRoot, schemaGlobCache);
const targets = files.length > 0 ? files : [path.join(changeDir, 'tasks.md')];
const unreadable: string[] = [];
let total = 0;
let completed = 0;
for (const file of targets) {
const progress = await countTaskFile(file, unreadable);
total += progress.total;
completed += progress.completed;
}
return { total, completed, unreadable };
}

return countSingleTopLevelTasksFile(changeDir);
/**
* The task-completion counter `status`, `list`, and `archive` share. Delegates
* to `getTaskProgressDetailForChange` and drops the `unreadable` detail, so its
* returned totals are unchanged. Throws only on the same malformed/unsafe-schema
* glob-resolution path as that function (existing behavior; callers guard it as
* they did before).
*/
export async function getTaskProgressForChange(
changesDir: string,
changeName: string,
projectRoot: string
): Promise<TaskProgress> {
const { total, completed } = await getTaskProgressDetailForChange(
changesDir,
changeName,
projectRoot
);
return { total, completed };
}

export function formatTaskStatus(progress: TaskProgress): string {
Expand Down
Loading
Loading