mirror of
https://github.com/Fission-AI/OpenSpec.git
synced 2026-10-02 05:24:34 +08:00
feat(validate): add --archived to lint task completion of archived changes (#1604)
* feat(validate): add --archived to lint task completion of archived changes `openspec validate --archived` scans every change under changes/archive/ and fails (exit 1) if any has unchecked tasks in tasks.md. This catches changes archived with unfinished work — which the normal validate flow never sees, since it only looks at active changes — and is meant for a pre-commit or CI hook. It is a standalone, opt-in scope: it returns before any existing bulk path, so no current `validate` invocation changes behavior, and it does not re-validate already-applied spec deltas. Reuses getTaskProgressForChange (the same counter status/list/archive use) so task counting never forks, and reads root.archiveDir so it is store-aware. Closes #205 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(validate): fail loudly on archive read errors and unreadable task files Address adversarial + CodeRabbit review of `validate --archived`: - listArchivedChangeIds now returns [] only for ENOENT (missing archive dir) and rethrows permission/I/O/ENOTDIR errors, so a real archive-read failure exits 1 instead of silently reading as "no archived changes". - Add getTaskProgressDetailForChange, which reports task files that exist but cannot be read; --archived turns those into an ERROR (naming the file) rather than silently counting them as zero tasks. The shared getTaskProgressForChange now wraps it and drops the detail, so status/list/archive totals are byte-identical. - Start the spinner after listing so a thrown listing error never leaves a spinner running. Adds regression tests (archive path is a file; archived tasks.md is unreadable) and unit tests for the new detail variant. Docs: align the --archived table verb and add a troubleshooting one-liner. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(validate): address CodeRabbit nits on archived-task tests - Build the unreadable-fixture path from separate path.join components instead of a hard-coded Unix-separator string. - Assert the reported unreadable path (canonicalized with realpathSync.native), not just the count, so a wrong path can't pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(validate): address round-3 review of --archived From three fresh adversarial reviews (scale/perf, flag/output-shape, filesystem/security): - perf: memoize schema→glob resolution across archived changes via a run-scoped SchemaGlobCache, so the same schema.yaml isn't re-parsed once per change (the archive is append-only and can hold thousands). Threaded as an optional arg; existing callers are unchanged. Loop stays sequential by design (per-change work is synchronous) — now documented. - output shape: issue `path` now follows validate's convention — 'tasks.md' for incomplete tasks, and the POSIX root-relative file path for an unreadable file (one issue per file) instead of the bare 'tasks'. - plain output: print `change/<id>` (matching the JSON `type` and bulk validation) instead of `archived/<id>`. - docs: correct the "Never throws" docstrings (glob resolution can throw on a malformed/unsafe schema; the caller guards it) and note the load-bearing projectRoot override for the archive path depth. Store-mode resolution confirmed correct by review. Tests updated + a memo regression test added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
59c16a4461
commit
83be9d113e
@@ -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.
|
||||
@@ -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
|
||||
@@ -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):**
|
||||
|
||||
@@ -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.
|
||||
|
||||
+2
-1
@@ -439,6 +439,7 @@ 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')
|
||||
@@ -446,7 +447,7 @@ program
|
||||
.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);
|
||||
|
||||
@@ -13,6 +13,9 @@ 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';
|
||||
|
||||
@@ -20,6 +23,7 @@ interface ExecuteOptions {
|
||||
all?: boolean;
|
||||
changes?: boolean;
|
||||
specs?: boolean;
|
||||
archived?: boolean;
|
||||
type?: string;
|
||||
strict?: boolean;
|
||||
json?: boolean;
|
||||
@@ -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, {
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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) {
|
||||
|
||||
@@ -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,
|
||||
|
||||
+93
-34
@@ -79,36 +79,77 @@ 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');
|
||||
try {
|
||||
const content = await fs.readFile(tasksPath, 'utf-8');
|
||||
return countTasksFromContent(content);
|
||||
} catch {
|
||||
return { total: 0, completed: 0 };
|
||||
}
|
||||
/** 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) : [];
|
||||
}
|
||||
|
||||
/** 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) : [];
|
||||
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(file, 'utf-8');
|
||||
return countTasksFromContent(content);
|
||||
} catch (error: any) {
|
||||
if (error?.code !== 'ENOENT') unreadable.push(file);
|
||||
return { total: 0, completed: 0 };
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -118,32 +159,50 @@ export function resolveTaskFilesForChange(changeDir: string, projectRoot: string
|
||||
* 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 getTaskProgressDetailForChange(
|
||||
changesDir: string,
|
||||
changeName: string,
|
||||
projectRoot: string,
|
||||
schemaGlobCache?: SchemaGlobCache
|
||||
): Promise<TaskProgressDetail> {
|
||||
const changeDir = path.join(changesDir, changeName);
|
||||
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 };
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 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 };
|
||||
}
|
||||
|
||||
return countSingleTopLevelTasksFile(changeDir);
|
||||
const { total, completed } = await getTaskProgressDetailForChange(
|
||||
changesDir,
|
||||
changeName,
|
||||
projectRoot
|
||||
);
|
||||
return { total, completed };
|
||||
}
|
||||
|
||||
export function formatTaskStatus(progress: TaskProgress): string {
|
||||
|
||||
@@ -0,0 +1,160 @@
|
||||
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 --archived checks archived task completion (#205)', () => {
|
||||
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');
|
||||
};
|
||||
|
||||
beforeAll(async () => {
|
||||
projectDir = await fs.mkdtemp(path.join(tmpdir(), 'openspec-archived-tasks-e2e-'));
|
||||
|
||||
// Fully completed archived change.
|
||||
await write(
|
||||
'openspec/changes/archive/2026-01-01-done-change/tasks.md',
|
||||
['# Tasks', '', '- [x] 1.1 do a', '- [x] 1.2 do b', ''].join('\n')
|
||||
);
|
||||
|
||||
// Archived change with an unchecked nested sub-task.
|
||||
await write(
|
||||
'openspec/changes/archive/2026-01-02-incomplete-change/tasks.md',
|
||||
[
|
||||
'# Tasks',
|
||||
'',
|
||||
'- [x] 1.1 do a',
|
||||
'- [ ] 1.2 do b',
|
||||
' - [ ] 1.2.1 nested unfinished work',
|
||||
'',
|
||||
].join('\n')
|
||||
);
|
||||
|
||||
// An active change with unchecked tasks must NOT be scanned by --archived.
|
||||
await write(
|
||||
'openspec/changes/active-change/tasks.md',
|
||||
['# Tasks', '', '- [ ] 1.1 still in progress', ''].join('\n')
|
||||
);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await fs.rm(projectDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('fails when an archived change has unchecked tasks and passes the complete one', async () => {
|
||||
const result = await runCLI(['validate', '--archived', '--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])
|
||||
);
|
||||
|
||||
// Only archived changes are considered; the active change is absent.
|
||||
expect(byId['active-change']).toBeUndefined();
|
||||
expect(byId['2026-01-01-done-change']).toBe(true);
|
||||
expect(byId['2026-01-02-incomplete-change']).toBe(false);
|
||||
|
||||
const incomplete = report.items.find(
|
||||
(item: { id: string }) => item.id === '2026-01-02-incomplete-change'
|
||||
);
|
||||
// The unchecked nested sub-task counts, so 2 of 3 tasks are open.
|
||||
expect(incomplete.issues[0]).toEqual(
|
||||
expect.objectContaining({
|
||||
level: 'ERROR',
|
||||
path: 'tasks.md',
|
||||
message: expect.stringContaining('2 incomplete tasks (1/3 completed)'),
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it('exits 0 when every archived change is complete', async () => {
|
||||
await fs.rm(
|
||||
path.join(
|
||||
projectDir,
|
||||
'openspec/changes/archive/2026-01-02-incomplete-change'
|
||||
),
|
||||
{ recursive: true, force: true }
|
||||
);
|
||||
|
||||
const result = await runCLI(['validate', '--archived'], { cwd: projectDir });
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.stdout).toContain('✓ change/2026-01-01-done-change');
|
||||
});
|
||||
|
||||
it('exits 0 with a friendly message when there is no archive directory', async () => {
|
||||
const emptyDir = await fs.mkdtemp(path.join(tmpdir(), 'openspec-no-archive-e2e-'));
|
||||
await fs.mkdir(path.join(emptyDir, 'openspec', 'changes'), { recursive: true });
|
||||
await fs.mkdir(path.join(emptyDir, 'openspec', 'specs'), { recursive: true });
|
||||
|
||||
const result = await runCLI(['validate', '--archived'], { cwd: emptyDir });
|
||||
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.stdout).toContain('No archived changes found.');
|
||||
await fs.rm(emptyDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('fails instead of passing silently when the archive path is not a directory', async () => {
|
||||
const dir = await fs.mkdtemp(path.join(tmpdir(), 'openspec-archive-notdir-e2e-'));
|
||||
await fs.mkdir(path.join(dir, 'openspec', 'changes'), { recursive: true });
|
||||
await fs.mkdir(path.join(dir, 'openspec', 'specs'), { recursive: true });
|
||||
// A real read failure (ENOTDIR) must not read as "no archived changes".
|
||||
await fs.writeFile(
|
||||
path.join(dir, 'openspec', 'changes', 'archive'),
|
||||
'not a directory\n'
|
||||
);
|
||||
|
||||
const result = await runCLI(['validate', '--archived'], { cwd: dir });
|
||||
|
||||
expect(result.exitCode).toBe(1);
|
||||
expect(result.stdout).not.toContain('No archived changes found.');
|
||||
await fs.rm(dir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('fails when an archived tasks file exists but cannot be read', async () => {
|
||||
const dir = await fs.mkdtemp(path.join(tmpdir(), 'openspec-archive-unreadable-e2e-'));
|
||||
await fs.mkdir(path.join(dir, 'openspec', 'specs'), { recursive: true });
|
||||
// A tasks.md that is a directory triggers a non-ENOENT read error (EISDIR)
|
||||
// on every platform, standing in for a genuinely unreadable file. It must
|
||||
// be reported, not silently counted as "no tasks".
|
||||
await fs.mkdir(
|
||||
path.join(
|
||||
dir,
|
||||
'openspec',
|
||||
'changes',
|
||||
'archive',
|
||||
'unreadable-change',
|
||||
'tasks.md'
|
||||
),
|
||||
{ recursive: true }
|
||||
);
|
||||
|
||||
const result = await runCLI(['validate', '--archived', '--json'], {
|
||||
cwd: dir,
|
||||
});
|
||||
|
||||
expect(result.exitCode).toBe(1);
|
||||
const report = JSON.parse(result.stdout);
|
||||
const item = report.items.find(
|
||||
(i: { id: string }) => i.id === 'unreadable-change'
|
||||
);
|
||||
expect(item.valid).toBe(false);
|
||||
expect(item.issues[0]).toEqual(
|
||||
expect.objectContaining({
|
||||
level: 'ERROR',
|
||||
// Pathed like every other validate issue: POSIX, root-relative.
|
||||
path: 'openspec/changes/archive/unreadable-change/tasks.md',
|
||||
message: 'could not read task file',
|
||||
})
|
||||
);
|
||||
await fs.rm(dir, { recursive: true, force: true });
|
||||
});
|
||||
});
|
||||
@@ -1,10 +1,11 @@
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { promises as fs } from 'fs';
|
||||
import { promises as fs, realpathSync } from 'fs';
|
||||
import path from 'path';
|
||||
import os from 'os';
|
||||
import {
|
||||
countTasksFromContent,
|
||||
getTaskProgressForChange,
|
||||
getTaskProgressDetailForChange,
|
||||
parseTaskLines,
|
||||
} from '../../src/utils/task-progress.js';
|
||||
import { resolveArtifactOutputs } from '../../src/core/artifact-graph/index.js';
|
||||
@@ -115,6 +116,25 @@ describe('getTaskProgressForChange (#1202 tracked-tasks resolution)', () => {
|
||||
expect(progress).toEqual({ total: 2, completed: 1 });
|
||||
});
|
||||
|
||||
it('memoizes schema→glob resolution across changes via the shared cache (#205)', async () => {
|
||||
await writeGlobSchema();
|
||||
await writeChange('c1', { 'backend/tasks.md': '- [x] a\n' });
|
||||
await writeChange('c2', { 'backend/tasks.md': '- [ ] b\n' });
|
||||
|
||||
const cache = new Map<string, string | undefined>();
|
||||
const d1 = await getTaskProgressDetailForChange(changesDir, 'c1', projectRoot, cache);
|
||||
// The schema→glob lookup is now cached under the resolved schema name.
|
||||
expect(cache.get('glob-tasks')).toBe('**/tasks.md');
|
||||
expect(cache.size).toBe(1);
|
||||
|
||||
// A second change on the same schema reuses the entry (no new key added),
|
||||
// and results are still correct.
|
||||
const d2 = await getTaskProgressDetailForChange(changesDir, 'c2', projectRoot, cache);
|
||||
expect(cache.size).toBe(1);
|
||||
expect(d1).toEqual({ total: 1, completed: 1, unreadable: [] });
|
||||
expect(d2).toEqual({ total: 1, completed: 0, unreadable: [] });
|
||||
});
|
||||
|
||||
it('identifies the tracked artifact by apply.tracks even when it is not named "tasks"', async () => {
|
||||
const schemaDir = path.join(projectRoot, 'openspec', 'schemas', 'custom-track');
|
||||
await fs.mkdir(schemaDir, { recursive: true });
|
||||
@@ -305,3 +325,56 @@ describe('countTasksFromContent', () => {
|
||||
expect(countTasksFromContent(content)).toEqual({ total: 6, completed: 3 });
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* #205 — `getTaskProgressDetailForChange` mirrors `getTaskProgressForChange`
|
||||
* but also reports task files that exist yet cannot be read, so a lint can fail
|
||||
* loudly instead of silently counting an unreadable file as "no tasks".
|
||||
*/
|
||||
describe('getTaskProgressDetailForChange (#205 unreadable reporting)', () => {
|
||||
let root: string;
|
||||
let changesDir: string;
|
||||
|
||||
beforeEach(async () => {
|
||||
root = await fs.mkdtemp(path.join(os.tmpdir(), 'openspec-taskdetail-'));
|
||||
changesDir = path.join(root, 'openspec', 'changes');
|
||||
await fs.mkdir(changesDir, { recursive: true });
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
it('reports no unreadable files for a normal tasks.md and counts as before', async () => {
|
||||
const changeDir = path.join(changesDir, 'ok');
|
||||
await fs.mkdir(changeDir, { recursive: true });
|
||||
await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] a\n- [ ] b\n', 'utf-8');
|
||||
|
||||
const detail = await getTaskProgressDetailForChange(changesDir, 'ok', root);
|
||||
expect(detail).toEqual({ total: 2, completed: 1, unreadable: [] });
|
||||
});
|
||||
|
||||
it('records a task file that exists but cannot be read (EISDIR)', async () => {
|
||||
// A tasks.md that is a directory yields a non-ENOENT read error on every
|
||||
// platform, standing in for a genuinely unreadable file.
|
||||
const badTasks = path.join(changesDir, 'bad', 'tasks.md');
|
||||
await fs.mkdir(badTasks, { recursive: true });
|
||||
|
||||
const detail = await getTaskProgressDetailForChange(changesDir, 'bad', root);
|
||||
expect(detail.total).toBe(0);
|
||||
expect(detail.completed).toBe(0);
|
||||
expect(detail.unreadable).toHaveLength(1);
|
||||
// The reported path is the file that could not be read (both canonicalized
|
||||
// so a /var vs /private/var symlink difference does not fail the identity).
|
||||
expect(realpathSync.native(detail.unreadable[0])).toBe(
|
||||
realpathSync.native(badTasks)
|
||||
);
|
||||
});
|
||||
|
||||
it('treats a missing tasks.md as zero tasks, not unreadable', async () => {
|
||||
await fs.mkdir(path.join(changesDir, 'empty'), { recursive: true });
|
||||
|
||||
const detail = await getTaskProgressDetailForChange(changesDir, 'empty', root);
|
||||
expect(detail).toEqual({ total: 0, completed: 0, unreadable: [] });
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user