mirror of
https://github.com/Fission-AI/OpenSpec.git
synced 2026-10-02 05:24:34 +08:00
fix(cli): reject missing roots for list and validate (#1612)
* fix(cli): reject missing roots for list and validate * test(cli): cover legacy list root fallback
This commit is contained in:
@@ -33,7 +33,7 @@ All root-resolving commands (`list`, `show`, `validate`, `status`, `instructions
|
||||
2. Otherwise, nearest ancestor with `openspec/`: planning shape → `source: "nearest"` (a `store:` pointer is ignored with a stderr warning); config-only dir with a valid `store:` pointer → that store, `source: "declared"`.
|
||||
3. No nearest root + global `defaultStore` set (`openspec config set defaultStore <id>`) → that store, `source: "global_default"`; a stale id fails with the underlying store error and a `fix` naming `openspec config unset defaultStore`.
|
||||
4. No nearest root, no default + registered stores exist → error `no_root_with_registered_stores`.
|
||||
5. No root, no default, no stores: scaffolding commands treat the cwd as `source: "implicit"`; diagnostic commands (`doctor`, `context`) fail with `no_openspec_root` instead — they inspect, never scaffold.
|
||||
5. No root, no default, no stores: commands may treat the cwd as `source: "implicit"`; `doctor`, `context`, `list`, and bulk `validate` instead fail with `no_openspec_root`. `list` preserves the implicit fallback for legacy projects with `openspec/project.md`.
|
||||
|
||||
Successful JSON payloads embed the root:
|
||||
|
||||
|
||||
+4
-1
@@ -4,7 +4,7 @@ import { createRequire } from 'module';
|
||||
import ora from 'ora';
|
||||
import path from 'path';
|
||||
import { fileURLToPath } from 'url';
|
||||
import { promises as fs } from 'fs';
|
||||
import { existsSync, promises as fs } from 'fs';
|
||||
import { AI_TOOLS, TOOL_ID_ALIASES } from '../core/config.js';
|
||||
import { UpdateCommand } from '../core/update.js';
|
||||
import {
|
||||
@@ -299,6 +299,9 @@ program
|
||||
const root = await resolveRootForCommand(options ?? {}, {
|
||||
json: options?.json,
|
||||
failurePayload: options?.specs ? { specs: [], root: null } : { changes: [], root: null },
|
||||
// Preserve the cwd fallback for pre-config.yaml projects. The resolver
|
||||
// still lets a registered/default store take precedence over it.
|
||||
allowImplicitRoot: existsSync(path.join(process.cwd(), 'openspec', 'project.md')),
|
||||
});
|
||||
if (!root) {
|
||||
return;
|
||||
|
||||
@@ -44,7 +44,11 @@ interface BulkItemResult {
|
||||
|
||||
export class ValidateCommand {
|
||||
async execute(itemName: string | undefined, options: ExecuteOptions = {}): Promise<void> {
|
||||
const root = await resolveRootForCommand(options, { json: options.json });
|
||||
const bulk = options.all || options.changes || options.specs;
|
||||
const root = await resolveRootForCommand(options, {
|
||||
json: options.json,
|
||||
...(bulk ? { allowImplicitRoot: false } : {}),
|
||||
});
|
||||
if (!root) {
|
||||
return;
|
||||
}
|
||||
@@ -64,7 +68,7 @@ export class ValidateCommand {
|
||||
}
|
||||
|
||||
// Handle bulk flags first
|
||||
if (options.all || options.changes || options.specs) {
|
||||
if (bulk) {
|
||||
await this.runBulkValidation(root, {
|
||||
changes: !!options.all || !!options.changes,
|
||||
specs: !!options.all || !!options.specs,
|
||||
|
||||
@@ -529,7 +529,7 @@ export async function resolveRootForCommand(
|
||||
output: {
|
||||
json?: boolean;
|
||||
failurePayload?: Record<string, unknown>;
|
||||
/** Diagnostic commands inspect what exists; they never scaffold. */
|
||||
/** Commands that require an existing root set this to false. */
|
||||
allowImplicitRoot?: boolean;
|
||||
} = {}
|
||||
): Promise<ResolvedOpenSpecRoot | null> {
|
||||
|
||||
@@ -560,6 +560,123 @@ operations:
|
||||
expect(json.changes).toEqual([]);
|
||||
expect(json.root.source).toBe('implicit');
|
||||
});
|
||||
|
||||
it('keeps list working for a legacy project.md root when no stores are registered', async () => {
|
||||
const isolatedEnv = {
|
||||
...env,
|
||||
XDG_DATA_HOME: path.join(tempDir, 'data-empty'),
|
||||
};
|
||||
fs.mkdirSync(path.join(appRepo, 'openspec'), { recursive: true });
|
||||
fs.writeFileSync(path.join(appRepo, 'openspec', 'project.md'), '# Project\n');
|
||||
|
||||
const result = await runCLI(['list', '--json'], { cwd: appRepo, env: isolatedEnv });
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.stderr).toBe('');
|
||||
|
||||
const json = parseJson(result);
|
||||
expect(json.changes).toEqual([]);
|
||||
expect({
|
||||
...json.root,
|
||||
path: fs.realpathSync.native(json.root.path),
|
||||
}).toEqual({ path: fs.realpathSync.native(appRepo), source: 'implicit' });
|
||||
});
|
||||
|
||||
it('rejects implicit roots for bulk validation and listing', async () => {
|
||||
const isolatedEnv = {
|
||||
...env,
|
||||
XDG_DATA_HOME: path.join(tempDir, 'data-empty'),
|
||||
};
|
||||
|
||||
for (const args of [
|
||||
['validate', '--all'],
|
||||
['validate', '--changes'],
|
||||
['validate', '--specs'],
|
||||
['list'],
|
||||
['list', '--specs'],
|
||||
]) {
|
||||
const result = await runCLI(args, { cwd: appRepo, env: isolatedEnv });
|
||||
expect(result.exitCode).toBe(1);
|
||||
expect(result.stdout).toBe('');
|
||||
expect(result.stderr).toContain(
|
||||
'Error: No OpenSpec root found from the current directory.'
|
||||
);
|
||||
expect(result.stderr).not.toContain('No items found to validate.');
|
||||
expect(result.stderr).not.toContain('No active changes found.');
|
||||
expect(result.stderr).not.toContain('No specs found.');
|
||||
}
|
||||
});
|
||||
|
||||
it('reports missing roots as JSON instead of fabricating an implicit root', async () => {
|
||||
const isolatedEnv = {
|
||||
...env,
|
||||
XDG_DATA_HOME: path.join(tempDir, 'data-empty'),
|
||||
};
|
||||
|
||||
for (const args of [
|
||||
['validate', '--all', '--json'],
|
||||
['validate', '--changes', '--json'],
|
||||
['validate', '--specs', '--json'],
|
||||
['list', '--json'],
|
||||
['list', '--specs', '--json'],
|
||||
]) {
|
||||
const result = await runCLI(args, { cwd: appRepo, env: isolatedEnv });
|
||||
expect(result.exitCode).toBe(1);
|
||||
expect(result.stderr).toBe('');
|
||||
|
||||
const json = parseJson(result);
|
||||
if (args[0] === 'validate') {
|
||||
expect(json).not.toHaveProperty('root');
|
||||
} else {
|
||||
expect(json.root).toBeNull();
|
||||
expect(json[args.includes('--specs') ? 'specs' : 'changes']).toEqual([]);
|
||||
}
|
||||
expect(json.status[0]).toEqual(
|
||||
expect.objectContaining({
|
||||
severity: 'error',
|
||||
code: 'no_openspec_root',
|
||||
message: 'No OpenSpec root found from the current directory.',
|
||||
})
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it('still accepts an existing root with no items', async () => {
|
||||
const isolatedEnv = {
|
||||
...env,
|
||||
XDG_DATA_HOME: path.join(tempDir, 'data-empty'),
|
||||
};
|
||||
createOpenSpecRoot(appRepo);
|
||||
|
||||
const result = await runCLI(['validate', '--all', '--json'], {
|
||||
cwd: appRepo,
|
||||
env: isolatedEnv,
|
||||
});
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.stderr).toBe('');
|
||||
|
||||
const json = parseJson(result);
|
||||
expect(json.items).toEqual([]);
|
||||
expect(json.summary.totals).toEqual({ items: 0, passed: 0, failed: 0 });
|
||||
expect({
|
||||
...json.root,
|
||||
path: fs.realpathSync.native(json.root.path),
|
||||
}).toEqual({ path: fs.realpathSync.native(appRepo), source: 'nearest' });
|
||||
});
|
||||
|
||||
it('preserves direct validation behavior without a root', async () => {
|
||||
const isolatedEnv = {
|
||||
...env,
|
||||
XDG_DATA_HOME: path.join(tempDir, 'data-empty'),
|
||||
};
|
||||
|
||||
const result = await runCLI(['validate', 'missing'], {
|
||||
cwd: appRepo,
|
||||
env: isolatedEnv,
|
||||
});
|
||||
expect(result.exitCode).toBe(1);
|
||||
expect(result.stderr).toContain("Unknown item 'missing'.");
|
||||
expect(result.stderr).not.toContain('No OpenSpec root found');
|
||||
});
|
||||
});
|
||||
|
||||
describe('archive --json is non-interactive', () => {
|
||||
|
||||
Reference in New Issue
Block a user