From b15f1b5e03182c231786927b04df89f9c40ec654 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 7 Aug 2026 17:15:46 -0500 Subject: [PATCH 1/2] fix(cli): reject missing roots for list and validate --- docs/agent-contract.md | 2 +- src/cli/index.ts | 5 +- src/commands/validate.ts | 8 +- src/core/root-selection.ts | 2 +- test/commands/store-root-selection.test.ts | 94 ++++++++++++++++++++++ 5 files changed, 106 insertions(+), 5 deletions(-) diff --git a/docs/agent-contract.md b/docs/agent-contract.md index 63e469e48f..f11e490042 100644 --- a/docs/agent-contract.md +++ b/docs/agent-contract.md @@ -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 `) → 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: diff --git a/src/cli/index.ts b/src/cli/index.ts index 619f958ecc..b87ae49afb 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -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; diff --git a/src/commands/validate.ts b/src/commands/validate.ts index 7c474cd0c2..8fa8122b4e 100644 --- a/src/commands/validate.ts +++ b/src/commands/validate.ts @@ -40,7 +40,11 @@ interface BulkItemResult { export class ValidateCommand { async execute(itemName: string | undefined, options: ExecuteOptions = {}): Promise { - 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; } @@ -48,7 +52,7 @@ export class ValidateCommand { const interactive = isInteractive(options); // 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, diff --git a/src/core/root-selection.ts b/src/core/root-selection.ts index 21108f5967..333d24f721 100644 --- a/src/core/root-selection.ts +++ b/src/core/root-selection.ts @@ -529,7 +529,7 @@ export async function resolveRootForCommand( output: { json?: boolean; failurePayload?: Record; - /** Diagnostic commands inspect what exists; they never scaffold. */ + /** Commands that require an existing root set this to false. */ allowImplicitRoot?: boolean; } = {} ): Promise { diff --git a/test/commands/store-root-selection.test.ts b/test/commands/store-root-selection.test.ts index 190276dc35..ba879deccc 100644 --- a/test/commands/store-root-selection.test.ts +++ b/test/commands/store-root-selection.test.ts @@ -560,6 +560,100 @@ operations: expect(json.changes).toEqual([]); expect(json.root.source).toBe('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).toEqual({ path: 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', () => { From 315cd6d9ba9873faa7e8333b0db8d0ee4e92e441 Mon Sep 17 00:00:00 2001 From: Clay Good Date: Fri, 7 Aug 2026 17:27:32 -0500 Subject: [PATCH 2/2] test(cli): cover legacy list root fallback --- test/commands/store-root-selection.test.ts | 25 +++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/test/commands/store-root-selection.test.ts b/test/commands/store-root-selection.test.ts index ba879deccc..334c05830f 100644 --- a/test/commands/store-root-selection.test.ts +++ b/test/commands/store-root-selection.test.ts @@ -561,6 +561,26 @@ operations: 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, @@ -637,7 +657,10 @@ operations: const json = parseJson(result); expect(json.items).toEqual([]); expect(json.summary.totals).toEqual({ items: 0, passed: 0, failed: 0 }); - expect(json.root).toEqual({ path: appRepo, source: 'nearest' }); + 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 () => {