fix(cli): reject missing roots for list and validate - #1612
Conversation
📝 WalkthroughWalkthroughThe change restricts implicit root resolution for bulk validation and listing without a root. Listing still uses the fallback for legacy OpenSpec projects. Tests cover human-readable errors, JSON diagnostics, empty roots, and direct validation. ChangesRoot resolution behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying openspec-docs with
|
| Latest commit: |
b15f1b5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://883e5778.openspec-docs.pages.dev |
| Branch Preview URL: | https://codex-fix-1611-root-validati.openspec-docs.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/commands/store-root-selection.test.ts (1)
564-587: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winVerify the legacy
listcompatibility path.The added tests cover a missing root and a modern nearest root. They do not exercise
listwhen the only marker isopenspec/project.md. Confirm that an existing test asserts success androot.sourceequal toimplicitfor this layout. If no test covers it, add that regression.Also applies to: 589-621
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/commands/store-root-selection.test.ts` around lines 564 - 587, Inspect the existing store-root selection tests for the legacy list layout where only openspec/project.md exists. Ensure a regression test runs list from that repository, expects success, and verifies the resolved root source is implicit; add the test if this behavior is not already covered, reusing the established root-selection assertion helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/commands/store-root-selection.test.ts`:
- Line 640: Update the assertion for json.root in the store-root selection test
to canonicalize json.root.path with the same realpath resolution used to create
appRepo before comparing paths. Preserve the expected source value of "nearest"
and keep the comparison path-safe for symlinked or resolved directories.
---
Nitpick comments:
In `@test/commands/store-root-selection.test.ts`:
- Around line 564-587: Inspect the existing store-root selection tests for the
legacy list layout where only openspec/project.md exists. Ensure a regression
test runs list from that repository, expects success, and verifies the resolved
root source is implicit; add the test if this behavior is not already covered,
reusing the established root-selection assertion helpers.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 82306e2b-2f86-4bd9-bddb-225c6844b8a1
📒 Files selected for processing (5)
docs/agent-contract.mdsrc/cli/index.tssrc/commands/validate.tssrc/core/root-selection.tstest/commands/store-root-selection.test.ts
Status
LGTM. Ready for human review.
What was wrong
From a directory with no OpenSpec root,
openspec validate --allandopenspec list --jsonresolved the current directory as an implicit root. They exited 0 and reported empty results, creating a silent false-pass for CI and agents.How it was fixed
--all,--changes, and--specs).openspec/project.md.Replication / proof
The regression tests first failed with exit code 0, then passed after the fix. Verified locally:
pnpm buildpnpm lintvitest run test/commands/store-root-selection.test.ts(40/40)vitest run test/commands/validate.test.ts(12/12)vitest run test/cli-e2e/view-store-resolution.test.ts(5/5)vitest run test/core/root-selection.test.ts(31/31)no_openspec_root; directvalidate missingstill reportsunknown_item.The full GitHub regression gate passes on Linux, macOS, and Windows, along with lint/type-check, release tracking, dependency review, security audit, CodeQL, and deployment checks.
Notes / nits
Closes #1611