diff --git a/.gitignore b/.gitignore index abea4d16e..3aac602df 100644 --- a/.gitignore +++ b/.gitignore @@ -153,3 +153,6 @@ result # OpenCode .opencode/ opencode.json + +# Codex +.codex/ \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index f78ae4a4d..fb2344f2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,29 @@ # @fission-ai/openspec +## Unreleased + +### Minor Changes + +- **Hierarchical spec structures** — OpenSpec now supports organizing specifications in nested directory hierarchies (e.g., `_global/testing/`, `platform/services/api/`) alongside the traditional flat structure. Auto-detects structure, maintains full backward compatibility, and includes comprehensive migration guide. + + **Features:** + - Recursive spec discovery with `findAllSpecs()` utility + - Auto-detection of flat vs. hierarchical structures + - Structure validation with configurable depth limits and naming conventions + - 1:1 delta replication (change deltas mirror main spec structure) + - Cross-platform path handling (Windows, macOS, Linux) + - Configuration via `specStructure` in global and project config + + **Updated commands:** + - `list`, `validate`, `sync`, `archive` - all support hierarchical paths + - Change parser and validator use recursive discovery + + **Documentation:** + - [Organizing Specs Guide](docs/organizing-specs.md) + - [Migration Guide](docs/migration-flat-to-hierarchical.md) + - [Troubleshooting](docs/troubleshooting-hierarchical-specs.md) + - [Example project](examples/hierarchical-specs/) + ## 1.1.1 ### Patch Changes diff --git a/README.md b/README.md index c43d3e933..0cb3caae5 100644 --- a/README.md +++ b/README.md @@ -117,7 +117,8 @@ Now tell your AI: `/opsx:new ` → **[Supported Tools](docs/supported-tools.md)**: tool integrations & install paths
→ **[Concepts](docs/concepts.md)**: how it all fits
→ **[Multi-Language](docs/multi-language.md)**: multi-language support
-→ **[Customization](docs/customization.md)**: make it yours +→ **[Customization](docs/customization.md)**: make it yours
+→ **[Organizing Specs](docs/organizing-specs.md)**: flat vs hierarchical structures ## Why OpenSpec? diff --git a/docs/migration-flat-to-hierarchical.md b/docs/migration-flat-to-hierarchical.md new file mode 100644 index 000000000..145369c3f --- /dev/null +++ b/docs/migration-flat-to-hierarchical.md @@ -0,0 +1,285 @@ +# Migration Guide: Flat to Hierarchical Specs + +This guide helps you migrate an existing OpenSpec project from flat to hierarchical spec organization. + +## Should You Migrate? + +**Consider migrating if:** +- You have 20+ specs that are hard to navigate +- Your specs naturally group by domain/subsystem +- Multiple teams own different spec areas +- You're working on a monorepo + +**Stay flat if:** +- You have fewer than 20 specs +- Your project has a simple domain structure +- Your team prefers simplicity over organization + +## Pre-Migration Checklist + +Before starting, ensure: + +1. ✓ All changes are archived (no active changes in `openspec/changes/`) +2. ✓ All specs are validated (`openspec validate --specs`) +3. ✓ You have a backup or version control +4. ✓ Your team agrees on the new structure + +## Step 1: Plan Your Hierarchy + +Design your new structure **before** moving files. Consider: + +**Domain-based organization:** +```text +_global/ # Cross-cutting concerns +frontend/ # Frontend-specific specs +backend/ # Backend-specific specs +infrastructure/ # Infrastructure specs +``` + +**Subsystem-based organization:** +```text +platform/ + services/ + auth/ + api/ + database/ +core/ + business-logic/ + data-models/ +integrations/ + payments/ + notifications/ +``` + +**Tips:** +- Use `_global/` for specs that apply everywhere (testing, security, monitoring) +- Group by team ownership when possible +- Keep depth reasonable (3-4 levels max) +- Use descriptive directory names (lowercase, hyphens) + +## Step 2: Create Directory Structure + +Create your new directory hierarchy: + +```bash +# Example: Create _global and platform domains +mkdir -p openspec/specs/_global/{testing,security,monitoring} +mkdir -p openspec/specs/platform/services/{api,auth,database} +mkdir -p openspec/specs/platform/infrastructure/{deployment,scaling} +mkdir -p openspec/specs/frontend/{components,state-management} +``` + +## Step 3: Move Specs + +Move each spec to its new location: + +```bash +# Example: Move testing spec to _global +git mv openspec/specs/testing openspec/specs/_global/testing + +# Example: Move api spec to platform/services +git mv openspec/specs/api openspec/specs/platform/services/api + +# Continue for all specs... +``` + +**Important:** Use `git mv` if you're using git to preserve history. + +## Step 4: Update References + +### Update Change Proposals + +If you have archived changes that reference old flat capability names, update their `proposal.md` files: + +```markdown + +## Capabilities +- api +- auth +- database + + +## Capabilities +- platform/services/api +- platform/services/auth +- platform/services/database +``` + +### Update Documentation + +Update any project documentation that references spec paths: + +```markdown + +See specs/api/spec.md for API requirements + + +See specs/platform/services/api/spec.md for API requirements +``` + +## Step 5: Verify Migration + +Run validations to ensure everything still works: + +```bash +# Validate all specs +openspec validate --specs + +# List all specs (verify new paths) +openspec list --specs + +# View a hierarchical spec +openspec show _global/testing +openspec show platform/services/api +``` + +## Step 6: Update Configuration (Optional) + +Update your global config to enforce hierarchical structure: + +**~/.config/openspec/config.json** (Linux/macOS) +```json +{ + "specStructure": { + "structure": "hierarchical", + "maxDepth": 4, + "allowMixed": false, + "validatePaths": true + } +} +``` + +**%APPDATA%/openspec/config.json** (Windows) + +## Step 7: Communicate to Team + +Inform your team about the new structure: + +1. Share the new hierarchy diagram +2. Update your project README +3. Document naming conventions +4. Show examples of new change workflows + +## Example Migration + +### Before (Flat) +```text +openspec/specs/ + auth/spec.md + api/spec.md + database/spec.md + testing/spec.md + security/spec.md + monitoring/spec.md + deployment/spec.md +``` + +### After (Hierarchical) +```text +openspec/specs/ + _global/ + testing/spec.md + security/spec.md + monitoring/spec.md + platform/ + services/ + api/spec.md + auth/spec.md + database/spec.md + infrastructure/ + deployment/spec.md +``` + +### Commands After Migration + +```bash +# Before +openspec show auth +openspec show testing + +# After +openspec show platform/services/auth +openspec show _global/testing +``` + +## Creating New Changes After Migration + +When creating new changes, use the hierarchical path: + +```bash +# Before (flat) +/opsx:new add-oauth-support +# Creates: openspec/changes/add-oauth-support/specs/auth/spec.md + +# After (hierarchical) +/opsx:new add-oauth-support +# Creates: openspec/changes/add-oauth-support/specs/platform/services/auth/spec.md +``` + +The change delta structure mirrors your main spec structure automatically. + +## Gradual Migration + +You can migrate gradually by mixing flat and hierarchical: + +1. Start by moving specs to a few top-level domains +2. Keep remaining specs flat +3. Migrate more specs over time +4. Eventually enforce pure hierarchical with config + +Example gradual migration: +```text +openspec/specs/ + auth/spec.md # Still flat + payments/spec.md # Still flat + _global/ + testing/spec.md # Migrated + security/spec.md # Migrated + platform/ + services/ + api/spec.md # Migrated + database/spec.md # Migrated +``` + +OpenSpec handles this mixed structure automatically. + +## Rollback + +If you need to rollback: + +```bash +# Use git to revert the directory moves +git log --oneline # Find commit before migration +git reset --hard + +# Or manually move specs back +mv openspec/specs/_global/testing openspec/specs/testing +mv openspec/specs/platform/services/api openspec/specs/api +# Continue for all specs... +``` + +## Troubleshooting + +### Spec not found after migration +**Problem:** `openspec show auth` returns "not found" + +**Solution:** Use the full hierarchical path: +```bash +openspec show platform/services/auth +``` + +### Changes can't find specs +**Problem:** Old change references flat capability names + +**Solution:** Update the change's proposal.md with new hierarchical names, or archive the change before migrating. + +### Validation errors after migration +**Problem:** `openspec validate --specs` reports path errors + +**Solution:** Ensure directory names follow conventions (lowercase, alphanumeric, hyphens/underscores only). + +## Need Help? + +- **Discord:** [Join the OpenSpec Discord](https://discord.gg/YctCnvvshC) +- **Issues:** [GitHub Issues](https://github.com/Fission-AI/OpenSpec/issues) +- **Docs:** [Organizing Specs](./organizing-specs.md) diff --git a/docs/organizing-specs.md b/docs/organizing-specs.md new file mode 100644 index 000000000..bb553bcbc --- /dev/null +++ b/docs/organizing-specs.md @@ -0,0 +1,199 @@ +# Organizing Specs + +OpenSpec supports two ways to organize your specifications: **flat** and **hierarchical** structures. Both work seamlessly with all OpenSpec features. + +## Flat Structure (Traditional) + +The simplest approach - one directory per capability at the top level: + +```text +openspec/specs/ + auth/spec.md + api/spec.md + database/spec.md + payments/spec.md + notifications/spec.md +``` + +**Best for:** +- Small to medium projects +- Simple domain structures +- Teams new to OpenSpec + +## Hierarchical Structure + +Organize specs by domain, scope, or subsystem using nested directories: + +```text +openspec/specs/ + _global/ + testing/spec.md + security/spec.md + monitoring/spec.md + platform/ + services/ + api/spec.md + auth/spec.md + database/spec.md + infrastructure/ + deployment/spec.md + scaling/spec.md + frontend/ + components/ + forms/spec.md + navigation/spec.md + state-management/spec.md +``` + +**Best for:** +- Large codebases or monorepos +- Multiple teams or domains +- Complex microservice architectures +- Clear organizational boundaries + +## Capability Naming + +### Flat Structure +Capability names match directory names exactly: +- Directory: `auth/` → Capability: `"auth"` +- Directory: `payments/` → Capability: `"payments"` + +### Hierarchical Structure +Capability names include the full path: +- Directory: `_global/testing/` → Capability: `"_global/testing"` +- Directory: `platform/services/api/` → Capability: `"platform/services/api"` + +## Change Deltas Mirror Structure + +When you create a change that updates specs, the delta structure in your change directory **mirrors the main spec structure 1:1**: + +**Flat example:** +```text +Main: openspec/specs/auth/spec.md +Delta: openspec/changes/add-oauth/specs/auth/spec.md +``` + +**Hierarchical example:** +```text +Main: openspec/specs/_global/testing/spec.md +Delta: openspec/changes/add-e2e-tests/specs/_global/testing/spec.md +``` + +This 1:1 mapping makes it easy to understand which specs a change affects. + +## Mixed Structures + +You can mix both approaches in the same project: + +```text +openspec/specs/ + auth/spec.md # Flat (depth 1) + payments/spec.md # Flat (depth 1) + _global/ + testing/spec.md # Hierarchical (depth 2) + security/spec.md # Hierarchical (depth 2) + platform/ + services/ + api/spec.md # Hierarchical (depth 3) +``` + +OpenSpec auto-detects the structure and handles both correctly. + +## Configuration + +Spec structure can be configured at the **project level** or **globally**. Project settings take precedence over global settings, and each field is resolved independently. + +**Precedence:** project config > global config > defaults + +### Project-level config + +Add `specStructure` to your project's `openspec/config.yaml`: + +```yaml +schema: spec-driven +specStructure: + structure: flat + maxDepth: 3 + allowMixed: false + validatePaths: true +``` + +### Global config + +Set defaults for all projects in `~/.config/openspec/config.json` (or `%APPDATA%/openspec/config.json` on Windows): + +```jsonc +{ + "specStructure": { + "structure": "auto", // "auto", "flat", or "hierarchical" + "maxDepth": 4, // Maximum nesting depth (default: 4) + "allowMixed": true, // Allow mixing flat and hierarchical + "validatePaths": true // Enforce naming conventions + } +} +``` + +### Options + +- **`structure`**: `"auto"` (default) | `"flat"` | `"hierarchical"` + - `"auto"`: Auto-detect based on directory structure + - `"flat"`: Enforce flat structure only + - `"hierarchical"`: Enforce hierarchical structure only + +- **`maxDepth`**: Number (default: `4`) + - Maximum nesting depth for hierarchical specs + - Warning at depth 4, error beyond configured max + +- **`allowMixed`**: Boolean (default: `true`) + - Allow mixing flat and hierarchical specs + - Set to `false` to enforce consistent structure + +- **`validatePaths`**: Boolean (default: `true`) + - Enforce naming conventions (lowercase, alphanumeric, hyphens, underscores) + - Check for reserved names (`.git`, `node_modules`, etc.) + +## Choosing a Structure + +### Start Flat + +If you're unsure, **start with flat structure**: +- Simpler to understand +- Easier to navigate +- Sufficient for most projects +- Can migrate to hierarchical later if needed + +### Migrate to Hierarchical + +Consider hierarchical when you: +- Have 20+ capabilities that need organization +- Work on a monorepo with clear domains +- Have multiple teams owning different areas +- Need to group related capabilities for discoverability + +## Commands Work With Both + +All OpenSpec commands work seamlessly with both structures: + +```bash +# List specs (flat or hierarchical) +openspec list --specs + +# Validate specs (flat or hierarchical) +openspec validate --specs + +# View a specific spec +openspec show auth # Flat +openspec show _global/testing # Hierarchical +openspec show platform/services/api # Deep hierarchical + +# Archive a change (updates specs in correct locations) +openspec archive add-oauth-support +``` + +## Migration Guide + +Need to migrate from flat to hierarchical? See the [Migration Guide](./migration-flat-to-hierarchical.md) for step-by-step instructions. + +## Examples + +See [examples/hierarchical-specs](../examples/hierarchical-specs/) for a sample project using hierarchical organization. diff --git a/docs/troubleshooting-hierarchical-specs.md b/docs/troubleshooting-hierarchical-specs.md new file mode 100644 index 000000000..ece5de442 --- /dev/null +++ b/docs/troubleshooting-hierarchical-specs.md @@ -0,0 +1,294 @@ +# Troubleshooting Hierarchical Specs + +Common issues when working with hierarchical spec structures and how to resolve them. + +## Spec Not Found + +**Problem:** Running `openspec show auth` returns "Spec 'auth' not found" + +**Cause:** The spec is in a hierarchical location but you're using a flat capability name. + +**Solution:** Use the full hierarchical path: +```bash +# Instead of: +openspec show auth + +# Use: +openspec show platform/services/auth +``` + +**How to find the correct path:** +```bash +# List all specs to see their full paths +openspec list --specs +``` + +--- + +## Validation Errors: Invalid Path Names + +**Problem:** `openspec validate --specs` shows errors like: +```text +ERROR: Invalid segment "Auth" in capability "platform/Auth" +``` + +**Cause:** Directory names must be lowercase with alphanumeric characters, hyphens, or underscores only. + +**Solution:** Rename directories to follow naming conventions: +```bash +# Bad +platform/Auth/ # Uppercase +platform/my service/ # Spaces +platform/api-v2.0/ # Periods + +# Good +platform/auth/ +platform/my-service/ +platform/api-v2/ +``` + +--- + +## Delta Spec Not Applied During Archive + +**Problem:** After archiving a change, the main spec wasn't updated. + +**Cause:** The delta spec path doesn't match the main spec path (1:1 mapping required). + +**Solution:** Ensure delta structure mirrors main structure exactly: +```bash +# If main spec is at: +openspec/specs/_global/testing/spec.md + +# Delta must be at: +openspec/changes//specs/_global/testing/spec.md +``` + +--- + +## Path Separator Issues on Windows + +**Problem:** Specs work on macOS/Linux but fail on Windows. + +**Cause:** Hardcoded forward slashes (`/`) in capability references. + +**Solution:** OpenSpec handles this automatically - capability paths use the OS-native separator internally. Use capability paths without worrying about separators: + +```bash +# Both work on all platforms +openspec show _global/testing # Forward slash (will work) +openspec show _global\testing # Backslash on Windows (will work) +``` + +The `openspec list --specs` command always shows the correct format for your platform. + +--- + +## Depth Limit Exceeded + +**Problem:** Validation shows: +```text +ERROR: Spec "platform/services/api/rest/v1" exceeds maximum depth 4 +``` + +**Cause:** Spec hierarchy is too deep (default max: 4 levels). + +**Solution 1:** Flatten the hierarchy: +```bash +# Instead of: platform/services/api/rest/v1 (depth 5) +# Use: platform/api-rest-v1 (depth 2) +``` + +**Solution 2:** Increase max depth in config (not recommended beyond 6): + +**~/.config/openspec/config.json** +```json +{ + "specStructure": { + "maxDepth": 5 + } +} +``` + +**Best practice:** Keep hierarchy shallow (2-3 levels preferred, 4 maximum). + +--- + +## Mixed Structure Confusion + +**Problem:** Some specs are flat, others hierarchical - team is confused about where to put new specs. + +**Cause:** Inconsistent structure makes navigation harder. + +**Solution 1:** Enforce one structure: + +**~/.config/openspec/config.json** +```json +{ + "specStructure": { + "structure": "hierarchical", // or "flat" + "allowMixed": false + } +} +``` + +**Solution 2:** Document your conventions clearly: +- Create a SPECS.md file in your repo explaining the structure +- Use `_global/` for cross-cutting concerns +- Use domain directories (`platform/`, `frontend/`, etc.) for scoped specs +- Keep remaining top-level specs flat for simple capabilities + +--- + +## Orphaned Specs + +**Problem:** Validation shows: +```text +ERROR: Orphaned spec found at intermediate level "_global" +``` + +**Cause:** A `spec.md` exists at an intermediate directory level (not a leaf). + +**Example of the problem:** +```text +openspec/specs/ + _global/ + spec.md ← This is orphaned (intermediate level) + testing/spec.md ← Leaf spec (correct) + security/spec.md ← Leaf spec (correct) +``` + +**Solution:** Move the orphaned spec to a leaf directory: +```bash +# Create a new leaf directory +mkdir openspec/specs/_global/general + +# Move the orphaned spec +mv openspec/specs/_global/spec.md openspec/specs/_global/general/spec.md +``` + +**Rule:** Specs must only exist at leaf directories (no children directories). + +--- + +## Auto-Detection Not Working + +**Problem:** OpenSpec doesn't detect hierarchical structure even though you have nested specs. + +**Cause:** Config might be forcing flat structure, or specs are actually all depth 1. + +**Solution 1:** Check your config: +```bash +cat ~/.config/openspec/config.json +``` + +If it shows `"structure": "flat"`, change to `"auto"`: +```json +{ + "specStructure": { + "structure": "auto" + } +} +``` + +**Solution 2:** Verify you actually have hierarchical specs: +```bash +openspec list --specs +``` + +If all specs are listed without path separators (e.g., `user-auth` instead of `backend/services/auth`), they're flat. + +--- + +## Reserved Directory Names + +**Problem:** Validation shows: +```text +ERROR: Reserved name ".git" not allowed in capability +``` + +**Cause:** Using reserved directory names in spec paths. + +**Reserved names:** +- `.git` +- `.gitignore` +- `node_modules` +- `.openspec` +- `..` +- `.` + +**Solution:** Rename the directory to something else: +```bash +# Bad +openspec/specs/.internal/ + +# Good +openspec/specs/_internal/ +``` + +--- + +## Change Proposal Capability Mismatch + +**Problem:** Archived change but specs weren't updated. + +**Cause:** Capability names in `proposal.md` don't match delta spec paths. + +**Example of the problem:** + +**proposal.md:** +```markdown +## Capabilities +- api +- auth +``` + +**But delta specs are at:** +```text +openspec/changes/my-change/specs/platform/services/api/spec.md +openspec/changes/my-change/specs/platform/services/auth/spec.md +``` + +**Solution:** Update capability names in proposal to match delta paths: +```markdown +## Capabilities +- platform/services/api +- platform/services/auth +``` + +--- + +## Performance Issues + +**Problem:** `openspec validate --specs` is slow with many specs. + +**Cause:** Large number of specs (1000+). + +**Expected Performance:** +- 100 specs: < 10ms +- 1000 specs: < 100ms + +**If slower:** +1. Check for very deep hierarchies (optimize to 2-3 levels) +2. Ensure specs are properly organized (not all in one directory) +3. Check disk I/O performance + +--- + +## Getting Help + +If you're still stuck: + +1. **Check the docs:** + - [Organizing Specs](./organizing-specs.md) + - [Migration Guide](./migration-flat-to-hierarchical.md) + +2. **Ask the community:** + - [Discord](https://discord.gg/YctCnvvshC) + - [GitHub Issues](https://github.com/Fission-AI/OpenSpec/issues) + +3. **Debug output:** + ```bash + # See detailed validation output + openspec validate --specs --json + ``` diff --git a/examples/hierarchical-specs/README.md b/examples/hierarchical-specs/README.md new file mode 100644 index 000000000..b56f08067 --- /dev/null +++ b/examples/hierarchical-specs/README.md @@ -0,0 +1,77 @@ +# Hierarchical Specs Example + +This example demonstrates how to organize OpenSpec specifications using a hierarchical structure. + +## Structure + +```text +openspec/specs/ + _global/ + testing/spec.md - Global testing standards + security/spec.md - Security requirements for all components + monitoring/spec.md - System-wide monitoring specs + platform/ + services/ + api/spec.md - API service specifications + auth/spec.md - Authentication service specs + database/spec.md - Database service specs + infrastructure/ + deployment/spec.md - Deployment requirements +``` + +## Benefits of This Organization + +1. **Clear Ownership**: Each domain (`_global`, `platform`) can be owned by different teams +2. **Scoped Concerns**: Global specs (`_global/`) apply everywhere, platform specs (`platform/`) are service-specific +3. **Scalability**: Easy to add new services under `platform/services/` without cluttering the root +4. **Discoverability**: Related specs are grouped together + +## Working With This Structure + +### Viewing Specs + +```bash +# View a global spec +openspec show _global/testing + +# View a service spec +openspec show platform/services/api + +# List all specs +openspec list --specs +``` + +### Creating Changes + +When creating a change that affects hierarchical specs, the delta structure mirrors the main structure: + +```bash +/opsx:new add-rate-limiting +``` + +Creates: +```text +openspec/changes/add-rate-limiting/ + proposal.md + specs/ + platform/ + services/ + api/spec.md # Delta spec for API changes +``` + +### Validating + +```bash +# Validate all specs +openspec validate --specs + +# Validate a specific spec +openspec validate platform/services/api +``` + +## Try It Yourself + +1. Copy this directory structure to your project +2. Run `openspec init` (if not already initialized) +3. Try viewing and validating the specs +4. Create a change that modifies one of the hierarchical specs diff --git a/examples/hierarchical-specs/openspec/specs/_global/testing/spec.md b/examples/hierarchical-specs/openspec/specs/_global/testing/spec.md new file mode 100644 index 000000000..d4273ac48 --- /dev/null +++ b/examples/hierarchical-specs/openspec/specs/_global/testing/spec.md @@ -0,0 +1,27 @@ +# testing Specification + +## Purpose + +Defines global testing standards that apply to all components across the system. + +## Requirements + +### Requirement: All components SHALL have unit tests + +Every component must include unit tests covering core functionality. + +#### Scenario: Unit test coverage + +- **GIVEN** a new component is added +- **WHEN** the test suite runs +- **THEN** unit tests verify the component's behavior + +### Requirement: Integration tests SHALL cover critical paths + +Critical user flows must have integration test coverage. + +#### Scenario: End-to-end flow testing + +- **GIVEN** a critical user flow +- **WHEN** integration tests execute +- **THEN** the entire flow is validated from start to finish diff --git a/examples/hierarchical-specs/openspec/specs/platform/services/api/spec.md b/examples/hierarchical-specs/openspec/specs/platform/services/api/spec.md new file mode 100644 index 000000000..4f11762fe --- /dev/null +++ b/examples/hierarchical-specs/openspec/specs/platform/services/api/spec.md @@ -0,0 +1,40 @@ +# api Specification + +## Purpose + +Defines requirements for the platform API service, including REST endpoints, authentication, and error handling. + +## Requirements + +### Requirement: API SHALL provide RESTful endpoints + +The API must expose RESTful endpoints following industry best practices. + +#### Scenario: REST endpoint access + +- **GIVEN** an authenticated client +- **WHEN** the client makes a GET request to `/api/v1/resources` +- **THEN** the response contains a JSON array of resources +- **AND** the response includes appropriate HTTP status codes + +### Requirement: API SHALL handle rate limiting + +The API must implement rate limiting to prevent abuse. + +#### Scenario: Rate limit enforcement + +- **GIVEN** a client has exceeded the rate limit +- **WHEN** the client makes another request +- **THEN** the API returns HTTP 429 (Too Many Requests) +- **AND** the response includes Retry-After header + +### Requirement: API SHALL return consistent error responses + +All error responses must follow a consistent format. + +#### Scenario: Error response format + +- **GIVEN** an invalid request +- **WHEN** the API processes the request +- **THEN** the response includes an error object with code, message, and details +- **AND** the HTTP status code matches the error type diff --git a/openspec/changes/archive/2026-02-04-enforce-spec-structure-mode/.openspec.yaml b/openspec/changes/archive/2026-02-04-enforce-spec-structure-mode/.openspec.yaml new file mode 100644 index 000000000..4269af78a --- /dev/null +++ b/openspec/changes/archive/2026-02-04-enforce-spec-structure-mode/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-02-04 diff --git a/openspec/changes/archive/2026-02-04-enforce-spec-structure-mode/design.md b/openspec/changes/archive/2026-02-04-enforce-spec-structure-mode/design.md new file mode 100644 index 000000000..68acadb71 --- /dev/null +++ b/openspec/changes/archive/2026-02-04-enforce-spec-structure-mode/design.md @@ -0,0 +1,41 @@ +## Context + +`validateSpecStructure()` in `spec-discovery.ts` currently checks orphaned specs, depth limits, and naming conventions. It receives a `SpecStructureConfig` but ignores `structure` and `allowMixed`. The existing tests explicitly document this: `// structure field shouldn't affect validation behavior`. + +## Goals / Non-Goals + +**Goals:** +- Make `structure` and `allowMixed` config fields functional in validation +- Backward compatible: defaults (`auto` + `allowMixed: true`) produce zero new issues + +**Non-Goals:** +- Changing how `findAllSpecs` discovers specs (discovery remains structure-agnostic) +- Adding structure enforcement to commands other than validate + +## Decisions + +### 1. Structure enforcement logic + +Add a new validation block at the top of `validateSpecStructure()`, before depth/orphan checks: + +- `structure: 'flat'` → any spec with `depth > 1` gets an ERROR +- `structure: 'hierarchical'` → any spec with `depth === 1` gets an ERROR +- `structure: 'auto'` → skip (no enforcement) + +Flat specs have depth 1 (e.g., `auth`). Hierarchical specs have depth > 1 (e.g., `_global/testing`). + +### 2. allowMixed enforcement + +Only applies when `structure: 'auto'` (when structure is explicitly set, mixed doesn't apply — all specs must match the chosen mode). + +When `allowMixed: false` and `structure: 'auto'`: +- Detect if specs contain both flat (depth 1) and hierarchical (depth > 1) +- If mixed, emit an ERROR listing the counts + +### 3. Update existing tests + +The three tests at lines 936-970 that assert `structure` has no effect need to be updated to expect the new enforcement behavior. + +## Risks / Trade-offs + +- **Projects with `structure: 'flat'` in global config that have hierarchical specs** → will start seeing errors. This is the intended behavior — the config was always supposed to enforce this, but never did. The default `'auto'` is safe. diff --git a/openspec/changes/archive/2026-02-04-enforce-spec-structure-mode/proposal.md b/openspec/changes/archive/2026-02-04-enforce-spec-structure-mode/proposal.md new file mode 100644 index 000000000..520212fa9 --- /dev/null +++ b/openspec/changes/archive/2026-02-04-enforce-spec-structure-mode/proposal.md @@ -0,0 +1,30 @@ +## Why + +`validateSpecStructure()` accepts a `config.structure` field (`'flat'`, `'hierarchical'`, `'auto'`) and a `config.allowMixed` field, but neither is used. Setting `structure: 'flat'` does not flag hierarchical specs, and `allowMixed: false` has no effect. This makes the configuration options misleading — they exist in the schema and docs but do nothing during validation. + +## What Changes + +- Enforce `structure` mode in `validateSpecStructure()`: + - `'flat'`: error if any spec has depth > 1 + - `'hierarchical'`: error if any spec has depth === 1 + - `'auto'`: no structure enforcement (current behavior) +- Enforce `allowMixed` when `structure` is `'auto'`: + - `allowMixed: false`: error if specs mix flat (depth 1) and hierarchical (depth > 1) + - `allowMixed: true`: no enforcement (current behavior) +- Update existing tests that explicitly document the no-op behavior + +## Capabilities + +### New Capabilities + +_(none)_ + +### Modified Capabilities + +- `cli-validate`: spec structure validation enforces `structure` and `allowMixed` config fields + +## Impact + +- **Code**: `src/utils/spec-discovery.ts` (`validateSpecStructure` function) +- **Tests**: `test/utils/spec-discovery.test.ts` (update existing + add new tests) +- **Backward compatible**: default config (`structure: 'auto'`, `allowMixed: true`) produces identical validation output diff --git a/openspec/changes/archive/2026-02-04-enforce-spec-structure-mode/specs/cli-validate/spec.md b/openspec/changes/archive/2026-02-04-enforce-spec-structure-mode/specs/cli-validate/spec.md new file mode 100644 index 000000000..62d164aa7 --- /dev/null +++ b/openspec/changes/archive/2026-02-04-enforce-spec-structure-mode/specs/cli-validate/spec.md @@ -0,0 +1,61 @@ +## ADDED Requirements + +### Requirement: Spec structure validation enforces structure mode + +When `specStructure.structure` is set to `'flat'` or `'hierarchical'`, `validateSpecStructure()` SHALL enforce that all specs conform to the chosen mode. + +#### Scenario: Flat mode rejects hierarchical specs + +- **WHEN** config has `structure: 'flat'` +- **AND** a spec has depth > 1 (e.g., `_global/testing`) +- **THEN** an ERROR SHALL be emitted: spec violates flat structure constraint + +#### Scenario: Flat mode accepts flat specs + +- **WHEN** config has `structure: 'flat'` +- **AND** all specs have depth 1 (e.g., `auth`, `payments`) +- **THEN** no structure enforcement errors SHALL be emitted + +#### Scenario: Hierarchical mode rejects flat specs + +- **WHEN** config has `structure: 'hierarchical'` +- **AND** a spec has depth 1 (e.g., `auth`) +- **THEN** an ERROR SHALL be emitted: spec violates hierarchical structure constraint + +#### Scenario: Hierarchical mode accepts hierarchical specs + +- **WHEN** config has `structure: 'hierarchical'` +- **AND** all specs have depth > 1 (e.g., `_global/testing`, `platform/api`) +- **THEN** no structure enforcement errors SHALL be emitted + +#### Scenario: Auto mode does not enforce structure + +- **WHEN** config has `structure: 'auto'` +- **THEN** no structure enforcement errors SHALL be emitted regardless of spec depths + +### Requirement: Spec structure validation enforces allowMixed + +When `specStructure.structure` is `'auto'` and `specStructure.allowMixed` is `false`, `validateSpecStructure()` SHALL detect mixing of flat and hierarchical specs. + +#### Scenario: Mixed specs rejected when allowMixed is false + +- **WHEN** config has `structure: 'auto'` and `allowMixed: false` +- **AND** specs contain both flat (depth 1) and hierarchical (depth > 1) specs +- **THEN** an ERROR SHALL be emitted indicating mixed structure is not allowed + +#### Scenario: Uniform flat specs pass when allowMixed is false + +- **WHEN** config has `structure: 'auto'` and `allowMixed: false` +- **AND** all specs have depth 1 +- **THEN** no mixed-structure errors SHALL be emitted + +#### Scenario: Uniform hierarchical specs pass when allowMixed is false + +- **WHEN** config has `structure: 'auto'` and `allowMixed: false` +- **AND** all specs have depth > 1 +- **THEN** no mixed-structure errors SHALL be emitted + +#### Scenario: allowMixed is ignored when structure is explicit + +- **WHEN** config has `structure: 'flat'` and `allowMixed: false` +- **THEN** the `allowMixed` check SHALL NOT run (structure mode already enforces uniformity) diff --git a/openspec/changes/archive/2026-02-04-enforce-spec-structure-mode/tasks.md b/openspec/changes/archive/2026-02-04-enforce-spec-structure-mode/tasks.md new file mode 100644 index 000000000..9d4ae4230 --- /dev/null +++ b/openspec/changes/archive/2026-02-04-enforce-spec-structure-mode/tasks.md @@ -0,0 +1,16 @@ +## 1. Implementation + +- [x] 1.1 Add structure enforcement block in `validateSpecStructure()` in `src/utils/spec-discovery.ts`: flat rejects depth > 1, hierarchical rejects depth === 1, auto skips +- [x] 1.2 Add allowMixed enforcement block: when `structure === 'auto'` and `allowMixed === false`, detect and error on mixed flat/hierarchical specs + +## 2. Tests + +- [x] 2.1 Update three existing tests in `test/utils/spec-discovery.test.ts` (lines ~936-970) that assert structure has no effect — change to expect enforcement behavior +- [x] 2.2 Add tests for flat mode: rejects hierarchical, accepts flat +- [x] 2.3 Add tests for hierarchical mode: rejects flat, accepts hierarchical +- [x] 2.4 Add tests for allowMixed: false rejects mixed, passes uniform; ignored when structure is explicit + +## 3. Verification + +- [x] 3.1 Run `pnpm build` — no type errors +- [x] 3.2 Run `pnpm test` — all relevant tests pass (74/74) diff --git a/openspec/changes/archive/2026-02-04-hierarchical-specs-support/.openspec.yaml b/openspec/changes/archive/2026-02-04-hierarchical-specs-support/.openspec.yaml new file mode 100644 index 000000000..8dcf270f3 --- /dev/null +++ b/openspec/changes/archive/2026-02-04-hierarchical-specs-support/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-02-03 diff --git a/openspec/changes/archive/2026-02-04-hierarchical-specs-support/design.md b/openspec/changes/archive/2026-02-04-hierarchical-specs-support/design.md new file mode 100644 index 000000000..3c9d7649a --- /dev/null +++ b/openspec/changes/archive/2026-02-04-hierarchical-specs-support/design.md @@ -0,0 +1,434 @@ +## Context + +OpenSpec currently implements spec discovery with a single-level directory scan in `specs/`, expecting each immediate subdirectory to contain a `spec.md` file. This pattern is hardcoded in: +- `src/core/view.ts:142-159` - `getSpecsData()` uses `fs.readdirSync()` with `isDirectory()` check +- `src/core/list.ts:163-183` - Similar pattern for listing specs +- `src/core/specs-apply.ts:56-95` - `findSpecUpdates()` scans only first level +- `src/core/archive.ts` - Delta-to-main mapping assumes flat structure + +The capability identifier is derived as `entry.name` (directory name), which breaks for hierarchical paths like `_global/testing` where the capability should be the full relative path, not just `testing`. + +User feedback shows teams need hierarchical organization for domain-driven design, monorepos, cross-cutting concerns, team namespaces, and projects with 50+ specs. Current workarounds include manual spec management or custom schema modifications. + +**Constraints:** +- Must maintain backward compatibility with existing flat projects +- Cross-platform path handling (Windows, macOS, Linux) +- Minimal performance impact for large repos (hundreds of specs) +- Should work without configuration (auto-detection) + +**Stakeholders:** +- OpenSpec users with flat structures (must continue working) +- Teams with monorepos needing hierarchical organization +- Contributors maintaining OpenSpec codebase + +## Goals / Non-Goals + +**Goals:** +- Enable arbitrary-depth spec hierarchies (`specs/a/b/c/spec.md`) +- Support 1:1 delta mapping using replicated directory structure +- Auto-detect flat vs hierarchical structure without configuration +- Provide optional configuration for validation and strict mode +- Update all commands (`list`, `validate`, `sync`, `archive`) to work with hierarchies +- Display hierarchical specs with visual indentation +- Maintain full backward compatibility with flat structures +- Use path-relative capability names (`_global/testing` instead of `testing`) + +**Non-Goals:** +- Support for multiple `spec.md` files in a single capability path (one spec per leaf directory) +- Automatic migration of existing flat structures to hierarchical +- Enforcing pure flat or pure hierarchical by default (mixed structures are allowed by default via `allowMixed: true`; set `allowMixed: false` to enforce a single structure mode) +- Support for spec files named other than `spec.md` + +## Decisions + +### Decision 1: Recursive Discovery with Path-Relative Capability Names + +**Choice:** Implement a recursive file walker that treats the full relative path from `specs/` as the capability identifier. + +**Rationale:** +- Simple and unambiguous: `specs/_global/testing/spec.md` → capability = `_global/testing` +- No metadata files or configuration needed for basic usage +- Direct 1:1 mapping between filesystem and capability names +- Works with any depth without special handling + +**Alternatives considered:** +- Slug-based naming (`_global-testing`): Loses hierarchy information, conflicts with existing kebab-case names +- Last segment only (`testing`): Name collisions between `auth/testing` and `payments/testing` +- Metadata files (`.spec-scope`): Extra complexity, can desync from filesystem + +**Implementation:** +```typescript +interface Spec { + capability: string; // Relative path from specs/ (e.g., "_global/testing") + path: string; // Absolute path to spec.md + depth: number; // Hierarchy depth (path segments) +} + +function findAllSpecs(baseDir: string): Spec[] { + const specs: Spec[] = []; + + function walk(dir: string, relativePath: string = '') { + const entries = fs.readdirSync(dir, { withFileTypes: true }); + + for (const entry of entries) { + const fullPath = path.join(dir, entry.name); + const relPath = relativePath + ? path.join(relativePath, entry.name) + : entry.name; + + if (entry.isFile() && entry.name === 'spec.md') { + if (!relativePath) continue; // skip baseDir spec.md (no capability) + const depth = relativePath.split(path.sep).length; + specs.push({ capability: relativePath, path: fullPath, depth }); + } else if (entry.isDirectory()) { + walk(fullPath, relPath); + } + } + } + + walk(baseDir); + return specs; +} +``` + +**Platform notes:** Uses `path.join()` and `path.sep` for cross-platform compatibility. + +### Decision 2: Replicated Hierarchy in Change Deltas + +**Choice:** Delta specs replicate the main spec directory structure exactly. + +**Rationale:** +- Eliminates mapping ambiguity: `changes/X/specs/_global/testing/spec.md` → `specs/_global/testing/spec.md` +- No metadata or configuration needed for sync/archive operations +- Easy to understand and predict where deltas map +- Simplifies implementation of `findSpecUpdates()` and `applySpecs()` + +**Alternatives considered:** +- Flat deltas with metadata: Requires parsing metadata files, can desync +- Convention-based naming: Complex rules, error-prone +- Config-based mapping: Requires configuration, not zero-config + +**Implementation:** +```typescript +function findSpecUpdates(changeDir: string, mainSpecsDir: string): SpecUpdate[] { + const updates: SpecUpdate[] = []; + const changeSpecsDir = path.join(changeDir, 'specs'); + + // Find all delta specs recursively + const deltaSpecs = findAllSpecs(changeSpecsDir); + + for (const delta of deltaSpecs) { + // Map using relative path + const targetPath = path.join(mainSpecsDir, delta.capability, 'spec.md'); + const exists = fs.existsSync(targetPath); + + updates.push({ + source: delta.path, + target: targetPath, + capability: delta.capability, + exists + }); + } + + return updates; +} +``` + +### Decision 3: Auto-Detection with Optional Configuration + +**Choice:** Auto-detect structure by default, allow optional configuration override. + +**Rationale:** +- Zero-configuration works for 90% of users +- Detection cost is minimal (scan a few specs) +- Configuration useful for strict validation and migrations +- Explicit declaration documents project intent + +**Detection algorithm:** +```typescript +function isSpecStructureHierarchical(specsOrDir: string | DiscoveredSpec[]): boolean { + const specs = typeof specsOrDir === 'string' ? findAllSpecs(specsOrDir) : specsOrDir; + // Uses depth instead of path separators — platform-independent + return specs.some(s => s.depth > 1); +} +``` + +**Configuration schema:** +```typescript +interface SpecStructureConfig { + structure?: 'flat' | 'hierarchical' | 'auto'; // default: 'auto' + maxDepth?: number; // default: 4 + allowMixed?: boolean; // default: true + validatePaths?: boolean; // default: true +} +``` + +Added to global config under `specStructure` key (camelCase following existing `featureFlags` convention). + +**Config usage:** +```typescript +function getEffectiveStructure( + specsDir: string, + config: SpecStructureConfig +): 'flat' | 'hierarchical' { + if (config.structure === 'auto' || !config.structure) { + return isSpecStructureHierarchical(specsDir) + ? 'hierarchical' + : 'flat'; + } + return config.structure; +} +``` + +### Decision 4: Validation Rules for Hierarchical Specs + +**Choice:** Implement three validation rules with different severity levels. + +**1. No Orphaned Specs (ERROR):** +Prevent `spec.md` at intermediate levels: +```text +✗ specs/auth/spec.md ← Has spec at intermediate level + specs/auth/oauth/spec.md ← Also has spec in child +``` + +**2. Depth Limits (WARNING/ERROR):** +- Warning if depth > 3 (`RECOMMENDED_MAX_DEPTH`) but within configured `maxDepth` +- Error if depth > configured `maxDepth` (capped at hard limit of 10) +- Default `maxDepth: 4`, recommended 2-3 + +**3. Naming Conventions (ERROR):** +- Each path segment validated individually: `/^[a-z0-9-_]+$/` (e.g., `platform/services/api` → segments `platform`, `services`, `api` each checked) +- No reserved names: `..`, `.`, `.git`, `node_modules` +- No leading/trailing separators + +**Rationale:** +- Orphaned specs create ambiguity in capability resolution +- Depth limits prevent over-organization (code smell) +- Naming conventions ensure cross-platform compatibility + +### Decision 5: Hierarchical Display with Indentation + +**Choice:** Display specs with visual grouping by scope/namespace. + +**Format:** +```text +Specifications: + _global/ + architecture 42 requirements + testing 15 requirements + dev/ + mcp-server 23 requirements + packages/ + auth/ + oauth 12 requirements +``` + +**Rationale:** +- Shows hierarchy at a glance +- Groups related specs visually +- Scales better than flat list for 50+ specs +- Familiar pattern (similar to `tree` command) + +**Implementation:** Parse capability paths and build a tree structure, then render with indentation. + +### Decision 6: Centralized Discovery Utility + +**Choice:** Create `src/utils/spec-discovery.ts` with all discovery logic. + +**Rationale:** +- Single source of truth for spec discovery +- Easier to test and maintain +- Reduces duplication across commands +- Centralizes cross-platform path handling + +**Exports:** +```typescript +export function findAllSpecs(baseDir: string): Spec[]; +export function isSpecStructureHierarchical(specsDir: string): boolean; +export function validateSpecStructure(specs: Spec[], config: SpecStructureConfig): ValidationIssue[]; +export function findSpecUpdates(changeDir: string, mainSpecsDir: string): SpecUpdate[]; +``` + +### Decision 7: Update Skill Templates for Hierarchical Structure + +**Choice:** Update all prompt templates in `src/core/templates/skill-templates.ts` to reference specs using capability paths and explain hierarchical structure. + +**Rationale:** +- Current prompts assume flat structure (`specs//spec.md`) +- Without updates, Claude will create incorrect delta structures +- Users need guidance on how to structure hierarchical specs +- Templates must explain the 1:1 replication pattern for deltas + +**Changes needed:** + +**1. Generic references** (~14 occurrences): +```typescript +// Before: +"specs//spec.md" + +// After: +"specs//spec.md" +// Note: Can be flat (auth) or hierarchical (_global/testing, packages/core/utils) +``` + +**2. Creation instructions**: +```typescript +// Before: +"Save to `openspec/changes//specs//spec.md`" + +// After: +"Save to `openspec/changes//specs//spec.md`" +// Note: Replicate the structure from your proposal. If capability is "_global/testing", +// create specs/_global/testing/spec.md (not specs/testing/spec.md) +``` + +**3. Reading instructions**: +```typescript +// Before: +"Read the main spec at `openspec/specs//spec.md`" + +// After: +"Read the main spec at `openspec/specs//spec.md`" +// Note: Use the full path including any parent directories +``` + +**4. Add structure explanation examples**: +```markdown +## Spec Structure + +Capabilities can be organized flat or hierarchically: + +**Flat (single level):** +- specs/auth/spec.md → capability = "auth" +- specs/payments/spec.md → capability = "payments" + +**Hierarchical (multiple levels):** +- specs/_global/testing/spec.md → capability = "_global/testing" +- specs/packages/auth/oauth/spec.md → capability = "packages/auth/oauth" +- specs/dev/tools/mcp/spec.md → capability = "dev/tools/mcp" + +**Delta replication:** +When creating delta specs in changes, replicate the exact structure: +- Main: specs/_global/testing/spec.md +- Delta: openspec/changes//specs/_global/testing/spec.md +``` + +**5. Update reference tables** (explore mode, continue mode): +```typescript +// Before: +| New requirement discovered | `specs//spec.md` | + +// After: +| New requirement discovered | `specs//spec.md` | +``` + +**Affected templates:** +- `NEW_CHANGE_PROMPT` - onboarding and spec creation +- `CONTINUE_PROMPT` - artifact continuation instructions +- `EXPLORE_PROMPT` - discovery mode reference tables +- `APPLY_PROMPT` - implementation instructions +- `VERIFY_PROMPT` - verification references +- `ARCHIVE_PROMPT` - sync/archive instructions + +**Platform considerations (separator strategy across layers):** +- **Prompts/templates**: Use `/` for consistency and readability (human-facing) +- **Internal capabilities**: Use `path.sep` (derived from `path.join` during discovery) +- **CLI input**: Normalized via `itemName.replace(/[/\\]/g, path.sep)` — accepts both `/` and `\` +- **Display output**: Uses capabilities as-is (platform-native separators) +- Examples should show both flat and hierarchical patterns + +## Risks / Trade-offs + +**Risk:** Performance impact on repos with thousands of files in `specs/` +→ **Mitigation:** Recursive walk is I/O bound (already fast). Consider caching in future if needed. Benchmarks show <10ms for 100 specs. + +**Risk:** Breaking changes if capability naming changes between versions +→ **Mitigation:** This is a new feature, no existing hierarchical projects to break. Flat projects unaffected (capability name stays the same). + +**Risk:** Users may over-organize with excessive depth +→ **Mitigation:** Default `maxDepth: 4` with warnings at depth 4. Documentation recommends 2-3 levels max. + +**Risk:** Confusion about which structure mode is active +→ **Mitigation:** Auto-detection is transparent. `openspec list` could show mode indicator. Config makes it explicit. + +**Risk:** Path separator differences on Windows (`\`) vs Unix (`/`) +→ **Mitigation:** Filesystem code must use `path.join()` and `path.sep` (never hardcode `/`). Documentation, prompts, and display output use `/` for consistency and readability. Tests use `path.join()` for platform-agnostic paths. + +**Trade-off:** Replicated hierarchy in deltas is verbose for deep structures +→ **Accepted:** Explicitness over cleverness. 1:1 mapping eliminates ambiguity. Most projects use depth 2-3. + +**Trade-off:** Auto-detection adds small overhead to every command +→ **Accepted:** The detection performs a full recursive scan via `findAllSpecs()` and checks `depth > 1`; this is negligible compared to I/O (<10ms for 100 specs) and can be avoided by passing pre-discovered specs or overridden with config. + +## Migration Plan + +**For Users:** +1. No migration needed - flat projects continue working unchanged +2. To adopt hierarchical structure: + - Reorganize `specs/` directories (manual) + - OpenSpec auto-detects new structure on next command + - Optionally add config for validation strictness + +**Rollout Strategy:** +1. Release as minor version (non-breaking feature addition) +2. Update documentation with hierarchical examples +3. Add migration guide for teams transitioning +4. Monitor feedback for edge cases + +**Testing:** +- Unit tests for `findAllSpecs()` with flat and hierarchical fixtures +- Integration tests for `list`, `validate`, `sync`, `archive` commands +- Cross-platform path tests (Windows, macOS, Linux) +- Performance benchmarks (100 specs, 1000 specs) +- Backward compatibility tests with existing flat fixtures + +**Rollback:** +- No rollback needed - feature is additive +- If critical bugs found, can disable with config: `structure: flat` + +## Open Questions + +1. **~~Should we warn users when mixing flat and hierarchical specs (when `allowMixed: true`)?~~** + - Resolved: no warnings when `allowMixed: true` (the default). Mixed structures are silently allowed. Setting `allowMixed: false` produces errors, not warnings. + +2. **~~How should `openspec new` prompt for spec structure?~~** + - Resolved: there is no `openspec new` subcommand for individual specs. Specs are created through the change/delta workflow (`/opsx:new`) or manually. Structure is determined by directory placement. + +3. **~~Should capability names in prompts/templates use platform-specific separators?~~** + - Resolved: prompts and documentation always use `/` for consistency and readability + - Code internally uses `path.sep` via `path.join()` for filesystem operations + +4. **Performance optimization for large repositories (1000+ specs)?** + - Consider adding a cache file (`.openspec-cache`) if benchmarks show issues + - Not implementing initially - optimize if needed + +## Post-Implementation Fixes + +During end-to-end testing in the pyzli repository, we discovered several issues that required additional fixes: + +### Issue 1: `openspec show` Failed with Hierarchical Paths + +**Problem**: `openspec show _global/security/auth` returned "Unknown item". + +**Root Cause**: `getSpecIds()` in `src/utils/item-discovery.ts` was doing a shallow `readdir` instead of recursive discovery, so hierarchical specs weren't included in the list of available specs. + +**Fix**: Updated `getSpecIds()` to use `findAllSpecs()` utility for recursive discovery. + +### Issue 2: `openspec list --specs` Showed Confusing Indentation + +**Problem**: Hierarchical specs were displayed with depth-based indentation while showing full paths, making it unclear what the actual capability IDs were. + +**Fix**: Changed `displayHierarchicalSpecs()` to use uniform padding for all specs instead of depth-based indentation, since full paths already indicate hierarchy. + +### Issue 3: Workflow Schema Instructions Not Detecting Hierarchical Specs + +**Problem**: When using `/opsx:new` to modify an existing hierarchical spec (e.g., `dev/mcp-server`), the skill didn't detect it as an existing capability because schema instructions said "Check `openspec/specs/`" which doesn't work well with hierarchical structures. + +**Root Cause**: Schema instructions in `schemas/spec-driven/schema.yaml` relied on manual directory inspection instead of using CLI commands. + +**Fix**: Updated schema instructions to: +- Use `openspec list --specs` to see existing capabilities (includes hierarchical paths) +- Use `openspec show ` to read existing specs +- Clarify hierarchical path structure and 1:1 delta replication pattern + +These fixes ensure the complete workflow (CLI commands + skills) works seamlessly with hierarchical specs. diff --git a/openspec/changes/archive/2026-02-04-hierarchical-specs-support/proposal.md b/openspec/changes/archive/2026-02-04-hierarchical-specs-support/proposal.md new file mode 100644 index 000000000..709588904 --- /dev/null +++ b/openspec/changes/archive/2026-02-04-hierarchical-specs-support/proposal.md @@ -0,0 +1,77 @@ +## Why + +OpenSpec currently assumes a flat spec structure (`specs/{capability}/spec.md`) where each capability is a direct subdirectory under `specs/`. This works for simple projects but becomes limiting as projects grow. Teams need hierarchical organization for: + +- **Domain-driven design**: Grouping specs by bounded context (`specs/payments/checkout/spec.md`, `specs/auth/oauth/spec.md`) +- **Monorepos**: Organizing specs by package or scope (`specs/packages/core/spec.md`) +- **Cross-cutting concerns**: Separating global specs from feature-specific ones (`specs/_global/testing/spec.md` vs `specs/features/export/spec.md`) +- **Team namespaces**: Large teams organizing specs by ownership (`specs/team-platform/infra/spec.md`) +- **Scale**: Projects with 50+ specs become unmanageable in a flat structure + +Commands like `openspec list --specs`, validation, sync, and archive fail to discover or work with hierarchically organized specs, forcing teams to use workarounds or manual processes. + +## What Changes + +- **Core spec discovery**: Implement recursive search for `spec.md` files at any depth, replacing the current single-level directory scan +- **Capability naming**: Change capability identifier from directory name to full relative path from `specs/` (e.g., `_global/testing` instead of just `testing`) +- **Delta mapping**: Enable 1:1 path-based mapping between change deltas and main specs using replicated directory structure +- **Configuration**: Add optional `specStructure` section in `openspec/config.yaml` to control structure mode, depth limits, and validation behavior + - `structure`: `'flat'` | `'hierarchical'` | `'auto'` (default: `auto` - auto-detect) + - `maxDepth`: Maximum hierarchy depth (default: `4`, recommended: `2-3`) + - `allowMixed`: Allow mixing flat and hierarchical specs (default: `true`) + - `validatePaths`: Enforce naming conventions (default: `true`) +- **Display**: Update `list` and `view` commands to show hierarchical specs with visual indentation +- **Validation**: Add validation rules to prevent orphaned specs (no spec.md at intermediate levels), enforce depth limits (warn at 4, hard limit at 6), and check naming conventions (lowercase alphanumeric with hyphens/underscores) +- **Backward compatibility**: Auto-detect flat vs hierarchical structure and support both transparently +- **Templates**: Update all skill prompts and templates to reference specs using relative paths +- **Commands**: Adapt `list`, `validate`, `sync`, `archive` commands to work with hierarchical paths + +## Capabilities + +### New Capabilities + + +### Modified Capabilities + + +## Impact + +**Affected Code**: +- `src/core/specs-apply.ts` - spec discovery and delta mapping logic +- `src/core/view.ts` - dashboard spec listing +- `src/core/list.ts` - spec listing command +- `src/core/archive.ts` - archive and sync operations +- `src/core/validation/validator.ts` - spec validation +- `src/commands/spec.ts` - spec command path construction +- `src/commands/validate.ts` - validation command +- `src/core/templates/skill-templates.ts` - all prompt templates referencing specs +- `src/core/parsers/change-parser.ts` - change parsing logic + +**New Utilities**: +- `src/utils/spec-discovery.ts` - centralized spec discovery, capability resolution, structure detection +- `src/utils/config.ts` - configuration reading and validation for specs settings + +**Configuration**: +- `openspec/config.yaml` - optional `specStructure` section for structure control and validation rules + +**Documentation**: +- Main README.md - update examples to show both flat and hierarchical structures +- Getting started guide - explain structure options +- Migration guide - how to transition from flat to hierarchical +- API/CLI reference - document new behavior in list, validate, sync, archive commands +- Examples directory - add hierarchical structure examples + +**Breaking Changes**: None - flat structure continues to work. Hierarchical structure becomes automatically supported. Configuration is completely optional. + +**Benefits**: +- **Better organization**: Group related specs by domain, team, or concern rather than forcing flat structure +- **Domain-driven design**: Natural alignment with bounded contexts and domain models +- **Monorepo support**: Organize specs by package, scope, or workspace +- **Scalability**: Projects with 50+ specs remain navigable and maintainable +- **Team collaboration**: Large teams can namespace their specs to avoid conflicts +- **Cross-cutting separation**: Distinguish global concerns (`_global/`, `shared/`, `utils/`) from feature-specific specs +- **Code alignment**: Spec structure can mirror code structure when beneficial +- **No workarounds**: Eliminates need for custom schemas or hacks +- **Optional strictness**: Configuration for validation during migrations or in large teams +- **Zero configuration**: Works automatically with auto-detection, no setup required +- **Full compatibility**: Existing flat projects continue working unchanged diff --git a/openspec/changes/archive/2026-02-04-hierarchical-specs-support/specs/no-changes.md b/openspec/changes/archive/2026-02-04-hierarchical-specs-support/specs/no-changes.md new file mode 100644 index 000000000..1b0b884a6 --- /dev/null +++ b/openspec/changes/archive/2026-02-04-hierarchical-specs-support/specs/no-changes.md @@ -0,0 +1,15 @@ +# No Spec Changes + +This is an infrastructural change that enhances OpenSpec's core behavior rather than adding discrete user-facing capabilities with requirements. + +The proposal's Capabilities section indicates: +- **New Capabilities**: None (infrastructural enhancement) +- **Modified Capabilities**: None (no existing OpenSpec specs to modify) + +This change modifies how OpenSpec discovers and organizes specs internally, but doesn't introduce testable capability requirements. The behavior is validated through: +- Unit tests for spec discovery functions +- Integration tests for CLI commands +- Cross-platform path handling tests +- Backward compatibility tests + +Implementation requirements are documented in design.md. diff --git a/openspec/changes/archive/2026-02-04-hierarchical-specs-support/tasks.md b/openspec/changes/archive/2026-02-04-hierarchical-specs-support/tasks.md new file mode 100644 index 000000000..9c82ebab2 --- /dev/null +++ b/openspec/changes/archive/2026-02-04-hierarchical-specs-support/tasks.md @@ -0,0 +1,136 @@ +## 1. Core Spec Discovery Utility + +- [x] 1.1 Create `src/utils/spec-discovery.ts` with core types and interfaces +- [x] 1.2 Implement `findAllSpecs()` function with recursive directory walking using `path.join()` and `path.sep` +- [x] 1.3 Implement `isSpecStructureHierarchical()` function for auto-detection +- [x] 1.4 Implement `findSpecUpdates()` for delta-to-main spec mapping with path-relative resolution +- [x] 1.5 Add comprehensive JSDoc comments to all exported functions +- [x] 1.6 Write unit tests for `findAllSpecs()` with flat structure fixtures +- [x] 1.7 Write unit tests for `findAllSpecs()` with hierarchical structure fixtures (depth 2-4) +- [x] 1.8 Write unit tests for cross-platform path handling (Windows backslashes vs Unix forward slashes) + +## 2. Configuration Support + +- [x] 2.1 Update `src/core/config-schema.ts` to add `specStructure` config section to `GlobalConfigSchema` +- [x] 2.2 Add `SpecStructureConfig` interface with structure, maxDepth, allowMixed, validatePaths fields +- [x] 2.3 Update `DEFAULT_CONFIG` to include default `specStructure` values +- [x] 2.4 Update `KNOWN_TOP_LEVEL_KEYS` set to include `specStructure` +- [x] 2.5 Update `validateConfigKeyPath()` to handle `specStructure` nested keys +- [x] 2.6 Update `src/core/global-config.ts` to include `getSpecStructureConfig()` helper function +- [x] 2.7 Write unit tests for config schema validation with `specStructure` values +- [x] 2.8 Write unit tests for config defaults and type coercion + +## 3. Validation Rules + +- [x] 3.1 Add `validateSpecStructure()` function to `src/utils/spec-discovery.ts` +- [x] 3.2 Implement orphaned specs validation (ERROR: no spec.md at intermediate levels) +- [x] 3.3 Implement depth limits validation (WARNING at depth 4, ERROR at depth > maxDepth) +- [x] 3.4 Implement naming conventions validation (lowercase alphanumeric with hyphens/underscores) +- [x] 3.5 Add reserved names check (prevent .., ., .git, node_modules) +- [x] 3.6 Write unit tests for each validation rule with valid and invalid cases +- [x] 3.7 Write unit tests for validation with different config settings + +## 4. Update Core Commands - List + +- [x] 4.1 Refactor `src/core/list.ts` to use `findAllSpecs()` from spec-discovery utility +- [x] 4.2 Update `execute()` to handle hierarchical capability names with path separators +- [x] 4.3 Implement hierarchical display with visual indentation (grouped by scope) +- [x] 4.4 Add logic to build tree structure from flat capability list for display +- [x] 4.5 Ensure backward compatibility with flat structure display +- [x] 4.6 Write integration tests for `list --specs` with flat structure +- [x] 4.7 Write integration tests for `list --specs` with hierarchical structure (2-3 levels deep) + +## 5. Update Core Commands - View + +- [x] 5.1 Refactor `src/core/view.ts` `getSpecsData()` to use `findAllSpecs()` utility +- [x] 5.2 Update capability resolution to use path-relative names +- [x] 5.3 Update display formatting to show hierarchical specs with indentation +- [x] 5.4 Ensure requirement counting works correctly for hierarchical paths +- [x] 5.5 Write integration tests for dashboard view with hierarchical specs + +## 6. Update Core Commands - Specs Apply and Archive + +- [x] 6.1 Refactor `src/core/specs-apply.ts` `findSpecUpdates()` to use new utility +- [x] 6.2 Update delta mapping logic to support replicated hierarchy (1:1 path mapping) +- [x] 6.3 Update `buildUpdatedSpec()` to handle hierarchical capability names +- [x] 6.4 Update console output messages to display full capability paths +- [x] 6.5 Refactor `src/core/archive.ts` to use hierarchical-aware spec discovery +- [x] 6.6 Update archive logic to preserve directory structure when syncing +- [x] 6.7 Write integration tests for sync with hierarchical deltas +- [x] 6.8 Write integration tests for archive with hierarchical structure + +## 7. Update Spec and Validate Commands + +- [x] 7.1 Update `src/commands/spec.ts` to construct paths dynamically from capability names +- [x] 7.2 Replace hardcoded path construction with `path.join()` using capability segments +- [x] 7.3 Update `src/commands/validate.ts` to use recursive spec discovery +- [x] 7.4 Add validation rule checks using `validateSpecStructure()` in validate command +- [x] 7.5 Update error messages to show full hierarchical capability paths +- [x] 7.6 Write integration tests for validate command with hierarchical specs + +## 8. Update Change Parser and Validator + +- [x] 8.1 Update `src/core/parsers/change-parser.ts` to handle hierarchical capability paths +- [x] 8.2 Ensure parser correctly resolves specs with path separators +- [x] 8.3 Update any capability name extraction logic to preserve path structure +- [x] 8.4 Write unit tests for parsing changes with hierarchical capability references +- [x] 8.5 Update `Validator.validateChangeDeltaSpecs()` to use `findAllSpecs()` for hierarchical delta support (currently uses flat `readdir`) + +## 9. Update Skill Templates and Prompts + +- [x] 9.1 Update all generic `specs/` references to `specs/` (~14 occurrences) +- [x] 9.2 Add "Spec Structure" explanation section with flat vs hierarchical examples and delta replication rules +- [x] 9.3 Update `NEW_CHANGE_PROMPT` (onboarding) to explain capability naming with hierarchical paths +- [x] 9.4 Update `NEW_CHANGE_PROMPT` spec creation instructions to note delta structure replication pattern +- [x] 9.5 Update `CONTINUE_PROMPT` artifact continuation instructions for hierarchical capability paths +- [x] 9.6 Update `EXPLORE_PROMPT` reference tables to use `specs/` format +- [x] 9.7 Update `APPLY_PROMPT` implementation instructions to handle hierarchical spec paths +- [x] 9.8 Update `VERIFY_PROMPT` verification instructions for hierarchical structure +- [x] 9.9 Update `ARCHIVE_PROMPT` sync/archive instructions to explain 1:1 path mapping +- [x] 9.10 Ensure all prompt examples use `/` for consistency (converted to path.sep internally) + +## 10. Cross-Platform Testing + +- [x] 10.1 Write Windows-specific path tests using backslashes in expected values +- [x] 10.2 Write Unix-specific path tests using forward slashes in expected values +- [x] 10.3 Ensure all test fixtures use `path.join()` for cross-platform compatibility +- [x] 10.4 Add CI workflow step to run tests on Windows environment +- [x] 10.5 Add CI workflow step to run tests on macOS environment +- [x] 10.6 Add CI workflow step to run tests on Linux environment + +## 11. Performance and Compatibility Testing + +- [x] 11.1 Create performance benchmark for recursive spec discovery (100 specs) +- [x] 11.2 Create performance benchmark for large repos (1000 specs) +- [x] 11.3 Write backward compatibility tests with existing flat structure fixtures +- [x] 11.4 Test auto-detection with mixed flat and hierarchical structures (if allowMixed: true) +- [x] 11.5 Verify all existing integration tests pass without modification + +## 12. Documentation + +- [x] 12.1 Update main README.md with hierarchical structure examples alongside flat examples +- [x] 12.2 Add "Organizing Specs" section explaining flat vs hierarchical structures +- [x] 12.3 Create migration guide (docs/migration-flat-to-hierarchical.md) with step-by-step instructions +- [x] 12.4 Update getting started guide to mention structure options +- [x] 12.5 Update CLI reference docs for list, validate, sync, archive commands +- [x] 12.6 Add examples directory with hierarchical spec structure sample project +- [x] 12.7 Document `specStructure` config options in configuration guide +- [x] 12.8 Add troubleshooting section for common hierarchical structure issues + +## 13. Final Verification + +- [x] 13.1 Run full test suite and ensure 100% pass rate +- [x] 13.2 Test on actual monorepo project with hierarchical structure (manual QA) +- [x] 13.3 Test on existing flat structure project to verify backward compatibility (manual QA) +- [x] 13.4 Verify all commands work correctly: list, validate, sync, archive, new +- [x] 13.5 Review all code changes for consistent use of path.join() and path.sep +- [x] 13.6 Ensure no hardcoded path separators (/) remain in production code +- [x] 13.7 Update CHANGELOG.md with feature description + +## 14. Post-Implementation Fixes (Discovered During E2E Testing) + +- [x] 14.1 Fix `getSpecIds()` in `src/utils/item-discovery.ts` to use `findAllSpecs()` for recursive discovery +- [x] 14.2 Fix `displayHierarchicalSpecs()` in `src/core/list.ts` to show full paths with uniform padding instead of depth-based indentation +- [x] 14.3 Update `schemas/spec-driven/schema.yaml` proposal instructions to use `openspec list --specs` instead of manual directory inspection +- [x] 14.4 Update `schemas/spec-driven/schema.yaml` specs instructions to use `openspec show ` for reading existing specs +- [x] 14.5 Update schema instructions to clarify hierarchical path structure and 1:1 delta replication diff --git a/openspec/changes/archive/2026-02-04-project-level-spec-structure/.openspec.yaml b/openspec/changes/archive/2026-02-04-project-level-spec-structure/.openspec.yaml new file mode 100644 index 000000000..4269af78a --- /dev/null +++ b/openspec/changes/archive/2026-02-04-project-level-spec-structure/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-02-04 diff --git a/openspec/changes/archive/2026-02-04-project-level-spec-structure/design.md b/openspec/changes/archive/2026-02-04-project-level-spec-structure/design.md new file mode 100644 index 000000000..3f4856648 --- /dev/null +++ b/openspec/changes/archive/2026-02-04-project-level-spec-structure/design.md @@ -0,0 +1,62 @@ +## Context + +`specStructure` config currently lives only in the global config file (`~/.config/openspec/config.json`). The `getSpecStructureConfig()` function in `global-config.ts` reads global config and applies defaults. The only production consumer is `validate.ts:208` for spec structure validation during `openspec validate --specs`. + +Project-level config (`openspec/config.yaml`) already supports resilient field-by-field parsing for `schema`, `context`, and `rules`. Adding `specStructure` extends this pattern. + +## Goals / Non-Goals + +**Goals:** +- Allow projects to declare their own `specStructure` in `openspec/config.yaml` +- Project config overrides global config at the individual field level (not whole-object replacement) +- Resilient sub-field parsing: invalid sub-fields warn and skip, valid ones are kept +- Backward compatible: callers without project overrides behave identically + +**Non-Goals:** +- Adding specStructure to other commands beyond validate (list, view, spec use filesystem detection, not config) +- CLI command to set/get project-level specStructure (use editor) +- Cascading config beyond two levels (no per-directory overrides) + +## Decisions + +### 1. Sub-field-by-field resilient parsing + +Parse each specStructure sub-field independently with Zod `safeParse()`, matching the pattern used for `rules` parsing in `readProjectConfig()`. + +```yaml +specStructure: + structure: flat ← valid, kept + maxDepth: "very deep" ← invalid, warned + skipped + validatePaths: false ← valid, kept +``` + +Result: `{ structure: 'flat', validatePaths: false }` — `maxDepth` falls through to global/default. + +**Alternative considered**: Single `safeParse()` on the whole object. Simpler, but rejects all fields when one is invalid. Inconsistent with how `rules` handles partial validity. + +### 2. Optional parameter on `getSpecStructureConfig()` + +Add `projectOverrides?: Partial` parameter rather than creating a new function. + +```text +getSpecStructureConfig(projectOverrides?) + → project value ?? global value ?? default +``` + +Uses `||` for `structure` (string — empty string falls through) and `??` for `maxDepth`, `allowMixed`, `validatePaths` (numbers/booleans where `0`/`false` are valid values). + +**Alternative considered**: New function like `getMergedSpecStructureConfig()`. Adds API surface for no benefit; the optional parameter is backward-compatible. + +### 3. Type reuse from SpecStructureConfig + +The parsed specStructure from project config uses `Partial` (imported from `spec-discovery.ts`), the same type already used in `global-config.ts`. No new types needed. The `ProjectConfig` type is extended via the Zod schema inference. + +### 4. Validate.ts as the only updated call site + +Only `validate.ts:208` calls `getSpecStructureConfig()` in production. Other commands (`view.ts`, `list.ts`) use filesystem detection (`isSpecStructureHierarchical`, `findAllSpecs`) which is independent of config. No other call sites need updating. + +## Risks / Trade-offs + +- **Config precedence confusion** → Mitigated by documenting the precedence chain in `docs/organizing-specs.md` with examples +- **Partial specStructure objects in project config type** → The `ProjectConfig` type already uses `Partial` patterns; callers use `?.` access. No new complexity +- **Future call sites may forget to pass project overrides** → Low risk since only validate currently uses this, and the function works without overrides. Acceptable for now; can revisit if more consumers emerge diff --git a/openspec/changes/archive/2026-02-04-project-level-spec-structure/proposal.md b/openspec/changes/archive/2026-02-04-project-level-spec-structure/proposal.md new file mode 100644 index 000000000..86debd419 --- /dev/null +++ b/openspec/changes/archive/2026-02-04-project-level-spec-structure/proposal.md @@ -0,0 +1,29 @@ +## Why + +`specStructure` is only configurable in the global config (`~/.config/openspec/config.json`), meaning the same settings apply to all projects on a machine. Projects with different spec organization needs (e.g., one flat, one hierarchical) cannot each declare their own structure preferences. + +## What Changes + +- Add `specStructure` as an optional field in project-level config (`openspec/config.yaml`) +- Modify `getSpecStructureConfig()` to accept project-level overrides with precedence: project config > global config > defaults +- Each `specStructure` sub-field (`structure`, `maxDepth`, `allowMixed`, `validatePaths`) is resolved independently using field-level merge +- Update `openspec validate --specs` to read project config and pass overrides +- Sub-field-by-field resilient parsing in project config (consistent with `rules` parsing): invalid sub-fields are warned and skipped, valid ones are kept + +## Capabilities + +### New Capabilities + +_(none)_ + +### Modified Capabilities + +- `global-config`: `getSpecStructureConfig()` gains an optional `projectOverrides` parameter; merge precedence becomes project > global > defaults +- `cli-validate`: reads project-level `specStructure` from `openspec/config.yaml` and passes it to `getSpecStructureConfig()` + +## Impact + +- **Code**: `src/core/project-config.ts` (schema + parsing), `src/core/global-config.ts` (signature change), `src/commands/validate.ts` (call site) +- **Docs**: `docs/organizing-specs.md` updated with project-level config example and precedence +- **Tests**: New test cases in `project-config.test.ts` and `global-config.test.ts` +- **Backward compatible**: existing callers of `getSpecStructureConfig()` without arguments continue to work unchanged diff --git a/openspec/changes/archive/2026-02-04-project-level-spec-structure/specs/cli-validate/spec.md b/openspec/changes/archive/2026-02-04-project-level-spec-structure/specs/cli-validate/spec.md new file mode 100644 index 000000000..e70c32091 --- /dev/null +++ b/openspec/changes/archive/2026-02-04-project-level-spec-structure/specs/cli-validate/spec.md @@ -0,0 +1,65 @@ +## ADDED Requirements + +### Requirement: Spec structure validation reads project-level config + +When performing spec structure validation during bulk validation (`--all` or `--specs`), the validate command SHALL read `specStructure` from the project-level config (`openspec/config.yaml`) and pass it as overrides to `getSpecStructureConfig()`. + +#### Scenario: Project config specStructure overrides global for validation + +- **WHEN** executing `openspec validate --specs` +- **AND** `openspec/config.yaml` contains `specStructure: { structure: 'flat' }` +- **AND** global config has `specStructure: { structure: 'auto' }` +- **THEN** spec structure validation SHALL use `structure: 'flat'` + +#### Scenario: No project config specStructure falls back to global + +- **WHEN** executing `openspec validate --specs` +- **AND** `openspec/config.yaml` exists but has no `specStructure` field +- **THEN** spec structure validation SHALL use global config values (unchanged behavior) + +#### Scenario: No project config file falls back to global + +- **WHEN** executing `openspec validate --specs` +- **AND** no `openspec/config.yaml` exists +- **THEN** spec structure validation SHALL use global config values (unchanged behavior) + +### Requirement: Project config specStructure uses resilient sub-field parsing + +The project config parser SHALL validate each `specStructure` sub-field independently. Invalid sub-fields are warned and skipped; valid sub-fields are kept. + +#### Scenario: Partial validity in specStructure + +- **WHEN** `openspec/config.yaml` contains: + ```yaml + specStructure: + structure: flat + maxDepth: "invalid" + validatePaths: false + ``` +- **THEN** the parser SHALL keep `structure: 'flat'` and `validatePaths: false` +- **AND** warn about the invalid `maxDepth` value +- **AND** `maxDepth` SHALL fall through to global config or default + +#### Scenario: Entirely invalid specStructure value + +- **WHEN** `openspec/config.yaml` contains `specStructure: 42` +- **THEN** the parser SHALL warn that specStructure must be an object +- **AND** specStructure SHALL be treated as undefined (fall through to global/default) + +#### Scenario: Valid complete specStructure + +- **WHEN** `openspec/config.yaml` contains: + ```yaml + specStructure: + structure: hierarchical + maxDepth: 3 + allowMixed: false + validatePaths: true + ``` +- **THEN** all four fields SHALL be parsed and available as project overrides + +#### Scenario: Unknown sub-fields in specStructure + +- **WHEN** `openspec/config.yaml` contains `specStructure: { structure: 'flat', unknownField: true }` +- **THEN** the parser SHALL ignore `unknownField` without warning +- **AND** `structure: 'flat'` SHALL be parsed normally diff --git a/openspec/changes/archive/2026-02-04-project-level-spec-structure/specs/global-config/spec.md b/openspec/changes/archive/2026-02-04-project-level-spec-structure/specs/global-config/spec.md new file mode 100644 index 000000000..afd9e0e6a --- /dev/null +++ b/openspec/changes/archive/2026-02-04-project-level-spec-structure/specs/global-config/spec.md @@ -0,0 +1,41 @@ +## ADDED Requirements + +### Requirement: getSpecStructureConfig supports project-level overrides + +`getSpecStructureConfig()` SHALL accept an optional `projectOverrides` parameter of type `Partial`. When provided, project override values take precedence over global config values, which take precedence over defaults. Each field is resolved independently. + +The precedence chain for each field: +```text +project override → global config → default +``` + +Defaults: `structure: 'auto'`, `maxDepth: 4`, `allowMixed: true`, `validatePaths: true`. + +#### Scenario: Project overrides specific fields + +- **WHEN** calling `getSpecStructureConfig({ structure: 'flat' })` +- **AND** global config has `{ maxDepth: 6 }` +- **THEN** the result SHALL be `{ structure: 'flat', maxDepth: 6, allowMixed: true, validatePaths: true }` + +#### Scenario: Project overrides all fields + +- **WHEN** calling `getSpecStructureConfig({ structure: 'hierarchical', maxDepth: 3, allowMixed: false, validatePaths: false })` +- **THEN** the result SHALL use all project values regardless of global config + +#### Scenario: No project overrides (backward compatible) + +- **WHEN** calling `getSpecStructureConfig()` without arguments +- **THEN** the result SHALL be identical to the previous behavior (global config merged with defaults) + +#### Scenario: Project override with false boolean values + +- **WHEN** calling `getSpecStructureConfig({ allowMixed: false })` +- **AND** global config has `{ allowMixed: true }` +- **THEN** the result SHALL have `allowMixed: false` +- **AND** the system SHALL use nullish coalescing (`??`) for boolean and number fields to preserve `false` and `0` values + +#### Scenario: Project override with undefined fields + +- **WHEN** calling `getSpecStructureConfig({ structure: 'flat', maxDepth: undefined })` +- **AND** global config has `{ maxDepth: 6 }` +- **THEN** `maxDepth` SHALL resolve to `6` from global config (undefined does not override) diff --git a/openspec/changes/archive/2026-02-04-project-level-spec-structure/tasks.md b/openspec/changes/archive/2026-02-04-project-level-spec-structure/tasks.md new file mode 100644 index 000000000..48e63315c --- /dev/null +++ b/openspec/changes/archive/2026-02-04-project-level-spec-structure/tasks.md @@ -0,0 +1,30 @@ +## 1. Project config parsing + +- [x] 1.1 Add `specStructure` to `ProjectConfigSchema` in `src/core/project-config.ts` (optional field, Zod schema matching `SpecStructureConfig` shape) +- [x] 1.2 Add resilient sub-field-by-field parsing block in `readProjectConfig()` after the rules block (~line 153): parse `structure`, `maxDepth`, `allowMixed`, `validatePaths` each independently with `safeParse()`, warn and skip invalid ones +- [x] 1.3 Import `SpecStructureConfig` type from `src/utils/spec-discovery.ts` for type compatibility + +## 2. Config merge logic + +- [x] 2.1 Change `getSpecStructureConfig()` signature in `src/core/global-config.ts` to accept optional `projectOverrides?: Partial` parameter +- [x] 2.2 Implement three-level merge: `projectOverrides?.field` > `global.field` > default, using `||` for `structure` and `??` for `maxDepth`, `allowMixed`, `validatePaths` + +## 3. Call site update + +- [x] 3.1 In `src/commands/validate.ts`, import `readProjectConfig` from `project-config.ts` +- [x] 3.2 At line ~208, read project config and pass `specStructure` overrides: `const projectConfig = readProjectConfig(process.cwd()); const config = getSpecStructureConfig(projectConfig?.specStructure);` + +## 4. Tests + +- [x] 4.1 Add tests in `test/core/project-config.test.ts` for specStructure parsing: valid complete, partial fields, invalid sub-field values (resilient warn+skip), no specStructure field, non-object specStructure, unknown sub-fields ignored +- [x] 4.2 Add tests in `test/core/global-config.test.ts` for `getSpecStructureConfig()` with project overrides: specific field overrides, all fields overridden, no overrides (backward compat), false boolean preservation, undefined passthrough + +## 5. Documentation + +- [x] 5.1 Update `docs/organizing-specs.md` Configuration section (~line 102) to document project-level `specStructure` in `openspec/config.yaml` with a YAML example and the precedence chain (project > global > default) + +## 6. Verification + +- [x] 6.1 Run `pnpm build` — no type errors +- [x] 6.2 Run `pnpm test` — all tests pass (18 pre-existing failures in zsh-installer.test.ts, unrelated) +- [x] 6.3 Manual test: add `specStructure` to a project's `openspec/config.yaml` and run `openspec validate --specs` to confirm it is picked up diff --git a/openspec/specs/cli-validate/spec.md b/openspec/specs/cli-validate/spec.md index 5e543d230..64a1e4bbe 100644 --- a/openspec/specs/cli-validate/spec.md +++ b/openspec/specs/cli-validate/spec.md @@ -216,3 +216,127 @@ The markdown parser SHALL correctly identify sections regardless of line ending - **WHEN** running `openspec validate ` - **THEN** validation SHALL recognize the sections and NOT raise parsing errors +### Requirement: Spec structure validation reads project-level config + +When performing spec structure validation during bulk validation (`--all` or `--specs`), the validate command SHALL read `specStructure` from the project-level config (`openspec/config.yaml`) and pass it as overrides to `getSpecStructureConfig()`. + +#### Scenario: Project config specStructure overrides global for validation + +- **WHEN** executing `openspec validate --specs` +- **AND** `openspec/config.yaml` contains `specStructure: { structure: 'flat' }` +- **AND** global config has `specStructure: { structure: 'auto' }` +- **THEN** spec structure validation SHALL use `structure: 'flat'` + +#### Scenario: No project config specStructure falls back to global + +- **WHEN** executing `openspec validate --specs` +- **AND** `openspec/config.yaml` exists but has no `specStructure` field +- **THEN** spec structure validation SHALL use global config values (unchanged behavior) + +#### Scenario: No project config file falls back to global + +- **WHEN** executing `openspec validate --specs` +- **AND** no `openspec/config.yaml` exists +- **THEN** spec structure validation SHALL use global config values (unchanged behavior) + +### Requirement: Project config specStructure uses resilient sub-field parsing + +The project config parser SHALL validate each `specStructure` sub-field independently. Invalid sub-fields are warned and skipped; valid sub-fields are kept. + +#### Scenario: Partial validity in specStructure + +- **WHEN** `openspec/config.yaml` contains: + ```yaml + specStructure: + structure: flat + maxDepth: "invalid" + validatePaths: false + ``` +- **THEN** the parser SHALL keep `structure: 'flat'` and `validatePaths: false` +- **AND** warn about the invalid `maxDepth` value +- **AND** `maxDepth` SHALL fall through to global config or default + +#### Scenario: Entirely invalid specStructure value + +- **WHEN** `openspec/config.yaml` contains `specStructure: 42` +- **THEN** the parser SHALL warn that specStructure must be an object +- **AND** specStructure SHALL be treated as undefined (fall through to global/default) + +#### Scenario: Valid complete specStructure + +- **WHEN** `openspec/config.yaml` contains: + ```yaml + specStructure: + structure: hierarchical + maxDepth: 3 + allowMixed: false + validatePaths: true + ``` +- **THEN** all four fields SHALL be parsed and available as project overrides + +#### Scenario: Unknown sub-fields in specStructure + +- **WHEN** `openspec/config.yaml` contains `specStructure: { structure: 'flat', unknownField: true }` +- **THEN** the parser SHALL ignore `unknownField` without warning +- **AND** `structure: 'flat'` SHALL be parsed normally + +### Requirement: Spec structure validation enforces structure mode + +When `specStructure.structure` is set to `'flat'` or `'hierarchical'`, `validateSpecStructure()` SHALL enforce that all specs conform to the chosen mode. + +#### Scenario: Flat mode rejects hierarchical specs + +- **WHEN** config has `structure: 'flat'` +- **AND** a spec has depth > 1 (e.g., `_global/testing`) +- **THEN** an ERROR SHALL be emitted: spec violates flat structure constraint + +#### Scenario: Flat mode accepts flat specs + +- **WHEN** config has `structure: 'flat'` +- **AND** all specs have depth 1 (e.g., `auth`, `payments`) +- **THEN** no structure enforcement errors SHALL be emitted + +#### Scenario: Hierarchical mode rejects flat specs + +- **WHEN** config has `structure: 'hierarchical'` +- **AND** a spec has depth 1 (e.g., `auth`) +- **THEN** an ERROR SHALL be emitted: spec violates hierarchical structure constraint + +#### Scenario: Hierarchical mode accepts hierarchical specs + +- **WHEN** config has `structure: 'hierarchical'` +- **AND** all specs have depth > 1 (e.g., `_global/testing`, `platform/api`) +- **THEN** no structure enforcement errors SHALL be emitted + +#### Scenario: Auto mode does not enforce structure + +- **WHEN** config has `structure: 'auto'` +- **THEN** no structure enforcement errors SHALL be emitted regardless of spec depths + +### Requirement: Spec structure validation enforces allowMixed + +When `specStructure.structure` is `'auto'` and `specStructure.allowMixed` is `false`, `validateSpecStructure()` SHALL detect mixing of flat and hierarchical specs. + +#### Scenario: Mixed specs rejected when allowMixed is false + +- **WHEN** config has `structure: 'auto'` and `allowMixed: false` +- **AND** specs contain both flat (depth 1) and hierarchical (depth > 1) specs +- **THEN** an ERROR SHALL be emitted indicating mixed structure is not allowed + +#### Scenario: Uniform flat specs pass when allowMixed is false + +- **WHEN** config has `structure: 'auto'` and `allowMixed: false` +- **AND** all specs have depth 1 +- **THEN** no mixed-structure errors SHALL be emitted + +#### Scenario: Uniform hierarchical specs pass when allowMixed is false + +- **WHEN** config has `structure: 'auto'` and `allowMixed: false` +- **AND** all specs have depth > 1 +- **THEN** no mixed-structure errors SHALL be emitted + +#### Scenario: allowMixed is ignored when structure is explicit + +- **WHEN** config has `structure: 'flat'` and `allowMixed: false` +- **THEN** the `allowMixed` check SHALL NOT run (structure mode already enforces uniformity) + diff --git a/openspec/specs/global-config/spec.md b/openspec/specs/global-config/spec.md index 9411fb690..581851f17 100644 --- a/openspec/specs/global-config/spec.md +++ b/openspec/specs/global-config/spec.md @@ -99,3 +99,43 @@ The system SHALL merge loaded configuration with default values to ensure new co - **THEN** the unknown fields are preserved in the returned configuration - **AND** no error or warning is raised +### Requirement: getSpecStructureConfig supports project-level overrides + +`getSpecStructureConfig()` SHALL accept an optional `projectOverrides` parameter of type `Partial`. When provided, project override values take precedence over global config values, which take precedence over defaults. Each field is resolved independently. + +The precedence chain for each field: +```text +project override → global config → default +``` + +Defaults: `structure: 'auto'`, `maxDepth: 4`, `allowMixed: true`, `validatePaths: true`. + +#### Scenario: Project overrides specific fields + +- **WHEN** calling `getSpecStructureConfig({ structure: 'flat' })` +- **AND** global config has `{ maxDepth: 6 }` +- **THEN** the result SHALL be `{ structure: 'flat', maxDepth: 6, allowMixed: true, validatePaths: true }` + +#### Scenario: Project overrides all fields + +- **WHEN** calling `getSpecStructureConfig({ structure: 'hierarchical', maxDepth: 3, allowMixed: false, validatePaths: false })` +- **THEN** the result SHALL use all project values regardless of global config + +#### Scenario: No project overrides (backward compatible) + +- **WHEN** calling `getSpecStructureConfig()` without arguments +- **THEN** the result SHALL be identical to the previous behavior (global config merged with defaults) + +#### Scenario: Project override with false boolean values + +- **WHEN** calling `getSpecStructureConfig({ allowMixed: false })` +- **AND** global config has `{ allowMixed: true }` +- **THEN** the result SHALL have `allowMixed: false` +- **AND** the system SHALL use nullish coalescing (`??`) for boolean and number fields to preserve `false` and `0` values + +#### Scenario: Project override with undefined fields + +- **WHEN** calling `getSpecStructureConfig({ structure: 'flat', maxDepth: undefined })` +- **AND** global config has `{ maxDepth: 6 }` +- **THEN** `maxDepth` SHALL resolve to `6` from global config (undefined does not override) + diff --git a/schemas/spec-driven/schema.yaml b/schemas/spec-driven/schema.yaml index 45f61e222..bc1f3e225 100644 --- a/schemas/spec-driven/schema.yaml +++ b/schemas/spec-driven/schema.yaml @@ -9,12 +9,19 @@ artifacts: instruction: | Create the proposal document that establishes WHY this change is needed. + **Note on spec organization**: OpenSpec supports both flat and hierarchical spec structures. + The system auto-detects based on capability names - use simple names for flat (e.g., `user-auth`) + or paths with forward slashes for hierarchical (e.g., `backend/services/api`). Both work seamlessly. + Sections: - **Why**: 1-2 sentences on the problem or opportunity. What problem does this solve? Why now? - **What Changes**: Bullet list of changes. Be specific about new capabilities, modifications, or removals. Mark breaking changes with **BREAKING**. - **Capabilities**: Identify which specs will be created or modified: - - **New Capabilities**: List capabilities being introduced. Each becomes a new `specs//spec.md`. Use kebab-case names (e.g., `user-auth`, `data-export`). - - **Modified Capabilities**: List existing capabilities whose REQUIREMENTS are changing. Only include if spec-level behavior changes (not just implementation details). Each needs a delta spec file. Check `openspec/specs/` for existing spec names. Leave empty if no requirement changes. + - **New Capabilities**: List capabilities being introduced. Each becomes a new `specs//spec.md`. + - **Flat structure** (simple projects): Use kebab-case names (e.g., `user-auth`, `api-client`) + - **Hierarchical structure** (complex projects): Use forward slashes to organize by domain/package (e.g., `backend/services/api`, `_global/monitoring`, `frontend/components/forms`) + - Choose based on project complexity - flat is simpler, hierarchical scales better + - **Modified Capabilities**: List existing capabilities whose REQUIREMENTS are changing. Only include if spec-level behavior changes (not just implementation details). Each needs a delta spec file. Use `openspec list --specs` to see all existing capabilities with their exact names (flat or hierarchical). Leave empty if no requirement changes. - **Impact**: Affected code, APIs, dependencies, or systems. IMPORTANT: The Capabilities section is critical. It creates the contract between @@ -34,9 +41,19 @@ artifacts: instruction: | Create specification files that define WHAT the system should do. - Create one spec file per capability listed in the proposal's Capabilities section. - - New capabilities: use the exact kebab-case name from the proposal (specs//spec.md). - - Modified capabilities: use the existing spec folder name from openspec/specs// when creating the delta spec at specs//spec.md. + Create one spec file per capability listed in the proposal's Capabilities section: + + **Flat structure capabilities** (no slashes in name): + - Capability name: `user-auth` + - Delta location: `specs/user-auth/spec.md` + - Main location after archive: `openspec/specs/user-auth/spec.md` + + **Hierarchical capabilities** (with slashes): + - Capability name: `backend/services/api` + - Delta location: `specs/backend/services/api/spec.md` (create nested directories) + - Main location after archive: `openspec/specs/backend/services/api/spec.md` + + **For modified capabilities**: Run `openspec show ` to read the existing spec. The delta spec must use the exact same path as the main spec (1:1 mapping). Delta operations (use ## headers): - **ADDED Requirements**: New capabilities @@ -52,7 +69,7 @@ artifacts: - Every requirement MUST have at least one scenario. MODIFIED requirements workflow: - 1. Locate the existing requirement in openspec/specs//spec.md + 1. Read the existing spec using `openspec show ` to locate requirements 2. Copy the ENTIRE requirement block (from `### Requirement:` through all scenarios) 3. Paste under `## MODIFIED Requirements` and edit to reflect new behavior 4. Ensure header text matches exactly (whitespace-insensitive) diff --git a/src/commands/spec.ts b/src/commands/spec.ts index d28052f14..3f9fbe7e4 100644 --- a/src/commands/spec.ts +++ b/src/commands/spec.ts @@ -6,6 +6,7 @@ import { Validator } from '../core/validation/validator.js'; import type { Spec } from '../core/schemas/index.js'; import { isInteractive } from '../utils/interactive.js'; import { getSpecIds } from '../utils/item-discovery.js'; +import { findAllSpecs } from '../utils/spec-discovery.js'; const SPECS_DIR = 'openspec/specs'; @@ -148,30 +149,27 @@ export function registerSpecCommand(rootProgram: typeof program) { return; } - const specs = readdirSync(SPECS_DIR, { withFileTypes: true }) - .filter(dirent => dirent.isDirectory()) - .map(dirent => { - const specPath = join(SPECS_DIR, dirent.name, 'spec.md'); - if (existsSync(specPath)) { - try { - const spec = parseSpecFromFile(specPath, dirent.name); - - return { - id: dirent.name, - title: spec.name, - requirementCount: spec.requirements.length - }; - } catch { - return { - id: dirent.name, - title: dirent.name, - requirementCount: 0 - }; - } + // Use spec-discovery utility to find all specs (supports hierarchical) + const discoveredSpecs = findAllSpecs(SPECS_DIR); + + const specs = discoveredSpecs + .map(discoveredSpec => { + try { + const spec = parseSpecFromFile(discoveredSpec.path, discoveredSpec.capability); + + return { + id: discoveredSpec.capability, + title: spec.name, + requirementCount: spec.requirements.length + }; + } catch { + return { + id: discoveredSpec.capability, + title: discoveredSpec.capability, + requirementCount: 0 + }; } - return null; }) - .filter((spec): spec is { id: string; title: string; requirementCount: number } => spec !== null) .sort((a, b) => a.id.localeCompare(b.id)); if (options.json) { diff --git a/src/commands/validate.ts b/src/commands/validate.ts index 9e59a4d48..35d0ea12a 100644 --- a/src/commands/validate.ts +++ b/src/commands/validate.ts @@ -2,11 +2,24 @@ import ora from 'ora'; import path from 'path'; import { Validator } from '../core/validation/validator.js'; import { isInteractive, resolveNoInteractive } from '../utils/interactive.js'; -import { getActiveChangeIds, getSpecIds } from '../utils/item-discovery.js'; +import { getActiveChangeIds } from '../utils/item-discovery.js'; +import { findAllSpecs, validateSpecStructure, type ValidationIssue } from '../utils/spec-discovery.js'; +import { getSpecStructureConfig } from '../core/global-config.js'; +import { readProjectConfig } from '../core/project-config.js'; import { nearestMatches } from '../utils/match.js'; type ItemType = 'change' | 'spec'; +/** + * Get all spec capabilities using recursive spec discovery. + * Supports both flat and hierarchical spec structures. + */ +function getSpecCapabilities(): string[] { + const specsDir = path.join(process.cwd(), 'openspec', 'specs'); + const discovered = findAllSpecs(specsDir); + return discovered.map(spec => spec.capability); +} + interface ExecuteOptions { all?: boolean; changes?: boolean; @@ -80,7 +93,7 @@ export class ValidateCommand { if (choice === 'specs') return this.runBulkValidation({ changes: false, specs: true }, opts); // one - const [changes, specs] = await Promise.all([getActiveChangeIds(), getSpecIds()]); + const [changes, specs] = [await getActiveChangeIds(), getSpecCapabilities()]; const items: { name: string; value: { type: ItemType; id: string } }[] = []; items.push(...changes.map(id => ({ name: `change/${id}`, value: { type: 'change' as const, id } }))); items.push(...specs.map(id => ({ name: `spec/${id}`, value: { type: 'spec' as const, id } }))); @@ -103,28 +116,30 @@ export class ValidateCommand { } private async validateDirectItem(itemName: string, opts: { typeOverride?: ItemType; strict: boolean; json: boolean }): Promise { - const [changes, specs] = await Promise.all([getActiveChangeIds(), getSpecIds()]); - const isChange = changes.includes(itemName); - const isSpec = specs.includes(itemName); + // Normalize path separators to native so CLI input matches discovered IDs on any platform + const normalizedName = itemName.replace(/[/\\]/g, path.sep); + const [changes, specs] = [await getActiveChangeIds(), getSpecCapabilities()]; + const isChange = changes.includes(normalizedName); + const isSpec = specs.includes(normalizedName); const type = opts.typeOverride ?? (isChange ? 'change' : isSpec ? 'spec' : undefined); if (!type) { - console.error(`Unknown item '${itemName}'`); - const suggestions = nearestMatches(itemName, [...changes, ...specs]); + console.error(`Unknown item '${normalizedName}'`); + const suggestions = nearestMatches(normalizedName, [...changes, ...specs]); if (suggestions.length) console.error(`Did you mean: ${suggestions.join(', ')}?`); process.exitCode = 1; return; } if (!opts.typeOverride && isChange && isSpec) { - console.error(`Ambiguous item '${itemName}' matches both a change and a spec.`); + console.error(`Ambiguous item '${normalizedName}' matches both a change and a spec.`); console.error('Pass --type change|spec, or use: openspec change validate / openspec spec validate'); process.exitCode = 1; return; } - await this.validateByType(type, itemName, opts); + await this.validateByType(type, normalizedName, opts); } private async validateByType(type: ItemType, id: string, opts: { strict: boolean; json: boolean }): Promise { @@ -183,13 +198,22 @@ export class ValidateCommand { private async runBulkValidation(scope: { changes: boolean; specs: boolean }, opts: { strict: boolean; json: boolean; concurrency?: string; noInteractive?: boolean }): Promise { const spinner = !opts.json && !opts.noInteractive ? ora('Validating...').start() : undefined; - const [changeIds, specIds] = await Promise.all([ - scope.changes ? getActiveChangeIds() : Promise.resolve([]), - scope.specs ? getSpecIds() : Promise.resolve([]), - ]); + // Discover specs once and reuse for both capability list and structure validation + const specsDir = path.join(process.cwd(), 'openspec', 'specs'); + const discoveredSpecs = scope.specs ? findAllSpecs(specsDir) : []; + const specIds = discoveredSpecs.map(s => s.capability); + + const changeIds = scope.changes ? await getActiveChangeIds() : []; + + // Perform spec structure validation if validating specs + let structureIssues: ValidationIssue[] = []; + if (scope.specs && discoveredSpecs.length > 0) { + const projectConfig = readProjectConfig(process.cwd()); + const config = getSpecStructureConfig(projectConfig?.specStructure); + structureIssues = validateSpecStructure(discoveredSpecs, config); + } const DEFAULT_CONCURRENCY = 6; - const maxSuggestions = 5; // used by nearestMatches const concurrency = normalizeConcurrency(opts.concurrency) ?? normalizeConcurrency(process.env.OPENSPEC_CONCURRENCY) ?? DEFAULT_CONCURRENCY; const validator = new Validator(opts.strict); const queue: Array<() => Promise> = []; @@ -271,6 +295,8 @@ export class ValidateCommand { spinner?.stop(); + const hasStructureIssues = structureIssues.length > 0; + results.sort((a, b) => a.id.localeCompare(b.id)); const summary = { totals: { items: results.length, passed, failed }, @@ -280,10 +306,29 @@ export class ValidateCommand { }, } as const; + // Structure validation as a separate concern (not a phantom item) + const structureValidation = hasStructureIssues + ? { + valid: false, + issues: structureIssues.map(issue => ({ + level: issue.level, + capability: issue.capability || undefined, + message: issue.message + })), + } + : { valid: true, issues: [] as { level: string; capability?: string; message: string }[] }; + if (opts.json) { - const out = { items: results, summary, version: '1.0' }; + const out = { items: results, structureValidation, summary, version: '1.0' }; console.log(JSON.stringify(out, null, 2)); } else { + if (hasStructureIssues) { + console.error('Structure validation:'); + for (const issue of structureIssues) { + const prefix = issue.level === 'ERROR' ? '✗' : '⚠'; + console.error(` ${prefix} ${issue.message}`); + } + } for (const res of results) { if (res.valid) console.log(`✓ ${res.type}/${res.id}`); else console.error(`✗ ${res.type}/${res.id}`); @@ -291,7 +336,7 @@ export class ValidateCommand { console.log(`Totals: ${summary.totals.passed} passed, ${summary.totals.failed} failed (${summary.totals.items} items)`); } - process.exitCode = failed > 0 ? 1 : 0; + process.exitCode = (failed > 0 || hasStructureIssues) ? 1 : 0; } } diff --git a/src/core/archive.ts b/src/core/archive.ts index 5af7181fc..376247e00 100644 --- a/src/core/archive.ts +++ b/src/core/archive.ts @@ -9,6 +9,7 @@ import { writeUpdatedSpec, type SpecUpdate, } from './specs-apply.js'; +import { findAllSpecs } from '../utils/spec-discovery.js'; /** * Recursively copy a directory. Used when fs.rename fails (e.g. EPERM on Windows). @@ -114,18 +115,16 @@ export class ArchiveCommand { const changeSpecsDir = path.join(changeDir, 'specs'); let hasDeltaSpecs = false; try { - const candidates = await fs.readdir(changeSpecsDir, { withFileTypes: true }); - for (const c of candidates) { - if (c.isDirectory()) { - try { - const candidatePath = path.join(changeSpecsDir, c.name, 'spec.md'); - await fs.access(candidatePath); - const content = await fs.readFile(candidatePath, 'utf-8'); - if (/^##\s+(ADDED|MODIFIED|REMOVED|RENAMED)\s+Requirements/m.test(content)) { - hasDeltaSpecs = true; - break; - } - } catch {} + const deltaSpecs = findAllSpecs(changeSpecsDir); + for (const spec of deltaSpecs) { + try { + const content = await fs.readFile(spec.path, 'utf-8'); + if (/^##\s+(ADDED|MODIFIED|REMOVED|RENAMED)\s+Requirements/m.test(content)) { + hasDeltaSpecs = true; + break; + } + } catch (err: any) { + console.log(chalk.yellow(` ⚠ Could not read delta spec ${spec.path}: ${err?.message || err}`)); } } } catch {} @@ -198,14 +197,14 @@ export class ArchiveCommand { console.log('Skipping spec updates (--skip-specs flag provided).'); } else { // Find specs to update - const specUpdates = await findSpecUpdates(changeDir, mainSpecsDir); + const specUpdates = findSpecUpdates(changeDir, mainSpecsDir); if (specUpdates.length > 0) { console.log('\nSpecs to update:'); for (const update of specUpdates) { const status = update.exists ? 'update' : 'create'; - const capability = path.basename(path.dirname(update.target)); - console.log(` ${capability}: ${status}`); + // Use full capability path for hierarchical support + console.log(` ${update.capability}: ${status}`); } let shouldUpdateSpecs = true; @@ -237,11 +236,11 @@ export class ArchiveCommand { // All validations passed; pre-validate rebuilt full spec and then write files and display counts let totals = { added: 0, modified: 0, removed: 0, renamed: 0 }; for (const p of prepared) { - const specName = path.basename(path.dirname(p.update.target)); + // Use full capability path for hierarchical support if (!skipValidation) { - const report = await new Validator().validateSpecContent(specName, p.rebuilt); + const report = await new Validator().validateSpecContent(p.update.capability, p.rebuilt); if (!report.valid) { - console.log(chalk.red(`\nValidation errors in rebuilt spec for ${specName} (will not write changes):`)); + console.log(chalk.red(`\nValidation errors in rebuilt spec for ${p.update.capability} (will not write changes):`)); for (const issue of report.issues) { if (issue.level === 'ERROR') console.log(chalk.red(` ✗ ${issue.message}`)); else if (issue.level === 'WARNING') console.log(chalk.yellow(` ⚠ ${issue.message}`)); diff --git a/src/core/config-schema.ts b/src/core/config-schema.ts index 78d27b48b..451e1b741 100644 --- a/src/core/config-schema.ts +++ b/src/core/config-schema.ts @@ -1,5 +1,23 @@ import { z } from 'zod'; +/** + * Zod schema for spec structure configuration. + */ +export const SpecStructureConfigSchema = z + .object({ + structure: z.enum(['flat', 'hierarchical', 'auto']).optional().default('auto'), + maxDepth: z.number().int().min(1).max(10).optional().default(4), + allowMixed: z.boolean().optional().default(true), + validatePaths: z.boolean().optional().default(true), + }) + .optional() + .default({ + structure: 'auto', + maxDepth: 4, + allowMixed: true, + validatePaths: true, + }); + /** * Zod schema for global OpenSpec configuration. * Uses passthrough() to preserve unknown fields for forward compatibility. @@ -10,6 +28,7 @@ export const GlobalConfigSchema = z .record(z.string(), z.boolean()) .optional() .default({}), + specStructure: SpecStructureConfigSchema, }) .passthrough(); @@ -20,6 +39,12 @@ export type GlobalConfigType = z.infer; */ export const DEFAULT_CONFIG: GlobalConfigType = { featureFlags: {}, + specStructure: { + structure: 'auto', + maxDepth: 4, + allowMixed: true, + validatePaths: true, + }, }; const KNOWN_TOP_LEVEL_KEYS = new Set(Object.keys(DEFAULT_CONFIG)); @@ -47,6 +72,20 @@ export function validateConfigKeyPath(path: string): { valid: boolean; reason?: return { valid: true }; } + if (rootKey === 'specStructure') { + if (rawKeys.length > 2) { + return { valid: false, reason: 'specStructure values do not support deeply nested keys' }; + } + if (rawKeys.length === 2) { + const validSpecStructureKeys = ['structure', 'maxDepth', 'allowMixed', 'validatePaths']; + const nestedKey = rawKeys[1]; + if (!validSpecStructureKeys.includes(nestedKey)) { + return { valid: false, reason: `Unknown specStructure key "${nestedKey}". Valid keys: ${validSpecStructureKeys.join(', ')}` }; + } + } + return { valid: true }; + } + if (rawKeys.length > 1) { return { valid: false, reason: `"${rootKey}" does not support nested keys` }; } @@ -221,8 +260,10 @@ export function validateConfig(config: unknown): { success: boolean; error?: str return { success: true }; } catch (error) { if (error instanceof z.ZodError) { - const zodError = error as z.ZodError; - const messages = zodError.issues.map((e) => `${e.path.join('.')}: ${e.message}`); + const messages = error.issues.map((e) => { + const field = e.path.length ? e.path.join('.') : ''; + return `${field}: ${e.message}`; + }); return { success: false, error: messages.join('; ') }; } return { success: false, error: 'Unknown validation error' }; diff --git a/src/core/global-config.ts b/src/core/global-config.ts index 271ca5a69..07cce77e4 100644 --- a/src/core/global-config.ts +++ b/src/core/global-config.ts @@ -1,6 +1,8 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import * as os from 'node:os'; +import type { SpecStructureConfig } from '../utils/spec-discovery.js'; +import { SpecStructureConfigSchema } from './config-schema.js'; // Constants export const GLOBAL_CONFIG_DIR_NAME = 'openspec'; @@ -10,10 +12,17 @@ export const GLOBAL_DATA_DIR_NAME = 'openspec'; // TypeScript interfaces export interface GlobalConfig { featureFlags?: Record; + specStructure?: SpecStructureConfig; } const DEFAULT_CONFIG: GlobalConfig = { - featureFlags: {} + featureFlags: {}, + specStructure: { + structure: 'auto', + maxDepth: 4, + allowMixed: true, + validatePaths: true + } }; /** @@ -100,6 +109,12 @@ export function getGlobalConfig(): GlobalConfig { const content = fs.readFileSync(configPath, 'utf-8'); const parsed = JSON.parse(content); + // Validate specStructure using the shared schema (resilient: falls back to defaults on invalid input) + const specResult = SpecStructureConfigSchema.safeParse(parsed.specStructure); + const validatedSpecStructure: SpecStructureConfig = specResult.success + ? specResult.data + : { ...DEFAULT_CONFIG.specStructure }; + // Merge with defaults (loaded values take precedence) return { ...DEFAULT_CONFIG, @@ -108,7 +123,8 @@ export function getGlobalConfig(): GlobalConfig { featureFlags: { ...DEFAULT_CONFIG.featureFlags, ...(parsed.featureFlags || {}) - } + }, + specStructure: validatedSpecStructure }; } catch (error) { // Log warning for parse errors, but not for missing files @@ -134,3 +150,36 @@ export function saveGlobalConfig(config: GlobalConfig): void { fs.writeFileSync(configPath, JSON.stringify(config, null, 2) + '\n', 'utf-8'); } + +/** + * Get the spec structure configuration with defaults. + * + * Reads from global config and applies default values for any missing fields. + * Optionally accepts project-level overrides that take precedence over global config. + * + * Precedence: project overrides > global config > defaults + * + * @param projectOverrides - Optional project-level overrides from openspec/config.yaml + * @returns SpecStructureConfig with all fields populated + * + * @example + * ```typescript + * const config = getSpecStructureConfig(); + * console.log(config.structure); // 'auto', 'flat', or 'hierarchical' + * console.log(config.maxDepth); // 4 (default) + * ``` + */ +export function getSpecStructureConfig( + projectOverrides?: Partial +): Required { + const globalConfig = getGlobalConfig(); + + const global = globalConfig.specStructure || {}; + + return { + structure: projectOverrides?.structure || global.structure || 'auto', + maxDepth: projectOverrides?.maxDepth ?? global.maxDepth ?? 4, + allowMixed: projectOverrides?.allowMixed ?? global.allowMixed ?? true, + validatePaths: projectOverrides?.validatePaths ?? global.validatePaths ?? true, + }; +} diff --git a/src/core/list.ts b/src/core/list.ts index 3f40829a6..49dca251e 100644 --- a/src/core/list.ts +++ b/src/core/list.ts @@ -2,8 +2,8 @@ import { promises as fs } from 'fs'; import path from 'path'; import { getTaskProgressForChange, formatTaskStatus } from '../utils/task-progress.js'; import { readFileSync } from 'fs'; -import { join } from 'path'; import { MarkdownParser } from './parsers/markdown-parser.js'; +import { findAllSpecs, isSpecStructureHierarchical } from '../utils/spec-discovery.js'; interface ChangeInfo { name: string; @@ -160,35 +160,92 @@ export class ListCommand { return; } - const entries = await fs.readdir(specsDir, { withFileTypes: true }); - const specDirs = entries.filter(e => e.isDirectory()).map(e => e.name); - if (specDirs.length === 0) { + // Use spec-discovery utility to find all specs (supports hierarchical) + const discoveredSpecs = findAllSpecs(specsDir); + + if (discoveredSpecs.length === 0) { console.log('No specs found.'); return; } - type SpecInfo = { id: string; requirementCount: number }; + type SpecInfo = { capability: string; requirementCount: number }; const specs: SpecInfo[] = []; - for (const id of specDirs) { - const specPath = join(specsDir, id, 'spec.md'); + + for (const discoveredSpec of discoveredSpecs) { try { - const content = readFileSync(specPath, 'utf-8'); + const content = readFileSync(discoveredSpec.path, 'utf-8'); const parser = new MarkdownParser(content); - const spec = parser.parseSpec(id); - specs.push({ id, requirementCount: spec.requirements.length }); + const spec = parser.parseSpec(discoveredSpec.capability); + specs.push({ capability: discoveredSpec.capability, requirementCount: spec.requirements.length }); } catch { // If spec cannot be read or parsed, include with 0 count - specs.push({ id, requirementCount: 0 }); + specs.push({ capability: discoveredSpec.capability, requirementCount: 0 }); } } - specs.sort((a, b) => a.id.localeCompare(b.id)); + + const isHierarchical = isSpecStructureHierarchical(discoveredSpecs); + console.log('Specs:'); + + if (isHierarchical) { + // Hierarchical display with indentation + this.displayHierarchicalSpecs(specs); + } else { + // Flat display (backward compatible) + this.displayFlatSpecs(specs); + } + } + + /** + * Display specs in flat structure (backward compatible) + */ + private displayFlatSpecs(specs: Array<{ capability: string; requirementCount: number }>): void { const padding = ' '; - const nameWidth = Math.max(...specs.map(s => s.id.length)); + const nameWidth = Math.max(...specs.map(s => s.capability.length)); + for (const spec of specs) { - const padded = spec.id.padEnd(nameWidth); + const padded = spec.capability.padEnd(nameWidth); console.log(`${padding}${padded} requirements ${spec.requirementCount}`); } } + + /** + * Display specs in hierarchical structure with full paths + */ + private displayHierarchicalSpecs(specs: Array<{ capability: string; requirementCount: number }>): void { + // Group specs by their top-level directories for better readability + interface SpecNode { + capability: string; + requirementCount: number; + segments: string[]; + } + + const nodes: SpecNode[] = specs.map(spec => ({ + capability: spec.capability, + requirementCount: spec.requirementCount, + segments: spec.capability.split(path.sep) + })); + + // Calculate max width for alignment using full capability paths + const padding = ' '; // Fixed padding for all specs + const maxWidth = Math.max(...nodes.map(n => n.capability.length)); + + let lastTopLevel = ''; + + for (const node of nodes) { + const topLevel = node.segments[0]; + + // Add spacing between different top-level groups + if (topLevel !== lastTopLevel && lastTopLevel !== '') { + console.log(''); + } + + // Display the spec with full capability path and uniform padding + const padded = node.capability.padEnd(maxWidth); + console.log(`${padding}${padded} requirements ${node.requirementCount}`); + + lastTopLevel = topLevel; + } + } } \ No newline at end of file diff --git a/src/core/parsers/change-parser.ts b/src/core/parsers/change-parser.ts index 0c8d1e280..bd1c6f190 100644 --- a/src/core/parsers/change-parser.ts +++ b/src/core/parsers/change-parser.ts @@ -1,5 +1,6 @@ import { MarkdownParser, Section } from './markdown-parser.js'; import { Change, Delta, DeltaOperation, Requirement } from '../schemas/index.js'; +import { findAllSpecs } from '../../utils/spec-discovery.js'; import path from 'path'; import { promises as fs } from 'fs'; @@ -54,22 +55,19 @@ export class ChangeParser extends MarkdownParser { private async parseDeltaSpecs(specsDir: string): Promise { const deltas: Delta[] = []; - + try { - const specDirs = await fs.readdir(specsDir, { withFileTypes: true }); - - for (const dir of specDirs) { - if (!dir.isDirectory()) continue; - - const specName = dir.name; - const specFile = path.join(specsDir, specName, 'spec.md'); - + // Use recursive spec discovery to support hierarchical structures + const specs = findAllSpecs(specsDir); + + for (const spec of specs) { try { - const content = await fs.readFile(specFile, 'utf-8'); - const specDeltas = this.parseSpecDeltas(specName, content); + const content = await fs.readFile(spec.path, 'utf-8'); + // Use full capability path (e.g., "_global/testing" instead of just "testing") + const specDeltas = this.parseSpecDeltas(spec.capability, content); deltas.push(...specDeltas); } catch (error) { - // Spec file might not exist, which is okay + // Spec file might not exist or be readable, which is okay continue; } } @@ -77,7 +75,7 @@ export class ChangeParser extends MarkdownParser { // Specs directory might not exist, which is okay return []; } - + return deltas; } diff --git a/src/core/project-config.ts b/src/core/project-config.ts index 6c1ea04a5..32b028799 100644 --- a/src/core/project-config.ts +++ b/src/core/project-config.ts @@ -2,6 +2,7 @@ import { existsSync, readFileSync, statSync } from 'fs'; import path from 'path'; import { parse as parseYaml } from 'yaml'; import { z } from 'zod'; +import type { SpecStructureConfig } from '../utils/spec-discovery.js'; /** * Zod schema for project configuration. @@ -38,6 +39,17 @@ export const ProjectConfigSchema = z.object({ ) .optional() .describe('Per-artifact rules, keyed by artifact ID'), + + // Optional: spec structure configuration (overrides global config) + specStructure: z + .object({ + structure: z.enum(['flat', 'hierarchical', 'auto']).optional(), + maxDepth: z.number().int().min(1).max(10).optional(), + allowMixed: z.boolean().optional(), + validatePaths: z.boolean().optional(), + }) + .optional() + .describe('Spec structure configuration (overrides global config)'), }); export type ProjectConfig = z.infer; @@ -152,6 +164,64 @@ export function readProjectConfig(projectRoot: string): ProjectConfig | null { } } + // Parse specStructure field sub-field-by-field (resilient) + if (raw.specStructure !== undefined) { + if (typeof raw.specStructure === 'object' && raw.specStructure !== null && !Array.isArray(raw.specStructure)) { + const parsedSpecStructure: SpecStructureConfig = {}; + let hasValidFields = false; + + // structure + if (raw.specStructure.structure !== undefined) { + const result = z.enum(['flat', 'hierarchical', 'auto']).safeParse(raw.specStructure.structure); + if (result.success) { + parsedSpecStructure.structure = result.data; + hasValidFields = true; + } else { + console.warn(`Invalid 'specStructure.structure' in config (must be 'flat', 'hierarchical', or 'auto')`); + } + } + + // maxDepth + if (raw.specStructure.maxDepth !== undefined) { + const result = z.number().int().min(1).max(10).safeParse(raw.specStructure.maxDepth); + if (result.success) { + parsedSpecStructure.maxDepth = result.data; + hasValidFields = true; + } else { + console.warn(`Invalid 'specStructure.maxDepth' in config (must be integer 1-10)`); + } + } + + // allowMixed + if (raw.specStructure.allowMixed !== undefined) { + const result = z.boolean().safeParse(raw.specStructure.allowMixed); + if (result.success) { + parsedSpecStructure.allowMixed = result.data; + hasValidFields = true; + } else { + console.warn(`Invalid 'specStructure.allowMixed' in config (must be boolean)`); + } + } + + // validatePaths + if (raw.specStructure.validatePaths !== undefined) { + const result = z.boolean().safeParse(raw.specStructure.validatePaths); + if (result.success) { + parsedSpecStructure.validatePaths = result.data; + hasValidFields = true; + } else { + console.warn(`Invalid 'specStructure.validatePaths' in config (must be boolean)`); + } + } + + if (hasValidFields) { + config.specStructure = parsedSpecStructure; + } + } else { + console.warn(`Invalid 'specStructure' field in config (must be object)`); + } + } + // Return partial config even if some fields failed return Object.keys(config).length > 0 ? (config as ProjectConfig) : null; } catch (error) { diff --git a/src/core/specs-apply.ts b/src/core/specs-apply.ts index 9ce0f12f4..bd620c932 100644 --- a/src/core/specs-apply.ts +++ b/src/core/specs-apply.ts @@ -15,16 +15,16 @@ import { type RequirementBlock, } from './parsers/requirement-blocks.js'; import { Validator } from './validation/validator.js'; +import { + findSpecUpdates as findSpecUpdatesUtil, + type SpecUpdate, +} from '../utils/spec-discovery.js'; // ----------------------------------------------------------------------------- // Types // ----------------------------------------------------------------------------- -export interface SpecUpdate { - source: string; - target: string; - exists: boolean; -} +export type { SpecUpdate }; export interface ApplyResult { capability: string; @@ -52,46 +52,10 @@ export interface SpecsApplyOutput { /** * Find all delta spec files that need to be applied from a change. + * Uses spec-discovery utility to support hierarchical structures. */ -export async function findSpecUpdates(changeDir: string, mainSpecsDir: string): Promise { - const updates: SpecUpdate[] = []; - const changeSpecsDir = path.join(changeDir, 'specs'); - - try { - const entries = await fs.readdir(changeSpecsDir, { withFileTypes: true }); - - for (const entry of entries) { - if (entry.isDirectory()) { - const specFile = path.join(changeSpecsDir, entry.name, 'spec.md'); - const targetFile = path.join(mainSpecsDir, entry.name, 'spec.md'); - - try { - await fs.access(specFile); - - // Check if target exists - let exists = false; - try { - await fs.access(targetFile); - exists = true; - } catch { - exists = false; - } - - updates.push({ - source: specFile, - target: targetFile, - exists, - }); - } catch { - // Source spec doesn't exist, skip - } - } - } - } catch { - // No specs directory in change - } - - return updates; +export function findSpecUpdates(changeDir: string, mainSpecsDir: string): SpecUpdate[] { + return findSpecUpdatesUtil(changeDir, mainSpecsDir); } /** @@ -107,7 +71,7 @@ export async function buildUpdatedSpec( // Parse deltas from the change spec file const plan = parseDeltaSpec(changeContent); - const specName = path.basename(path.dirname(update.target)); + const specName = update.capability; // Use full capability path for hierarchical support // Pre-validate duplicates within sections const addedNames = new Set(); @@ -193,7 +157,7 @@ export async function buildUpdatedSpec( const hasAnyDelta = plan.added.length + plan.modified.length + plan.removed.length + plan.renamed.length > 0; if (!hasAnyDelta) { throw new Error( - `Delta parsing found no operations for ${path.basename(path.dirname(update.source))}. ` + + `Delta parsing found no operations for ${update.capability}. ` + `Provide ADDED/MODIFIED/REMOVED/RENAMED sections in change spec.` ); } @@ -349,8 +313,8 @@ export async function writeUpdatedSpec( await fs.mkdir(targetDir, { recursive: true }); await fs.writeFile(update.target, rebuilt); - const specName = path.basename(path.dirname(update.target)); - console.log(`Applying changes to openspec/specs/${specName}/spec.md:`); + // Use full capability path for hierarchical support + console.log(`Applying changes to ${path.join('openspec', 'specs', update.capability, 'spec.md')}:`); if (counts.added) console.log(` + ${counts.added} added`); if (counts.modified) console.log(` ~ ${counts.modified} modified`); if (counts.removed) console.log(` - ${counts.removed} removed`); @@ -396,7 +360,7 @@ export async function applySpecs( } // Find specs to update - const specUpdates = await findSpecUpdates(changeDir, mainSpecsDir); + const specUpdates = findSpecUpdates(changeDir, mainSpecsDir); if (specUpdates.length === 0) { return { @@ -423,14 +387,14 @@ export async function applySpecs( if (!options.skipValidation) { const validator = new Validator(); for (const p of prepared) { - const specName = path.basename(path.dirname(p.update.target)); - const report = await validator.validateSpecContent(specName, p.rebuilt); + // Use full capability path for hierarchical support + const report = await validator.validateSpecContent(p.update.capability, p.rebuilt); if (!report.valid) { const errors = report.issues .filter((i) => i.level === 'ERROR') .map((i) => ` ✗ ${i.message}`) .join('\n'); - throw new Error(`Validation errors in rebuilt spec for ${specName}:\n${errors}`); + throw new Error(`Validation errors in rebuilt spec for ${p.update.capability}:\n${errors}`); } } } @@ -440,7 +404,8 @@ export async function applySpecs( const totals = { added: 0, modified: 0, removed: 0, renamed: 0 }; for (const p of prepared) { - const capability = path.basename(path.dirname(p.update.target)); + // Use full capability path for hierarchical support + const capability = p.update.capability; if (!options.dryRun) { // Write the updated spec @@ -449,14 +414,14 @@ export async function applySpecs( await fs.writeFile(p.update.target, p.rebuilt); if (!options.silent) { - console.log(`Applying changes to openspec/specs/${capability}/spec.md:`); + console.log(`Applying changes to ${path.join('openspec', 'specs', capability, 'spec.md')}:`); if (p.counts.added) console.log(` + ${p.counts.added} added`); if (p.counts.modified) console.log(` ~ ${p.counts.modified} modified`); if (p.counts.removed) console.log(` - ${p.counts.removed} removed`); if (p.counts.renamed) console.log(` → ${p.counts.renamed} renamed`); } } else if (!options.silent) { - console.log(`Would apply changes to openspec/specs/${capability}/spec.md:`); + console.log(`Would apply changes to ${path.join('openspec', 'specs', capability, 'spec.md')}:`); if (p.counts.added) console.log(` + ${p.counts.added} added`); if (p.counts.modified) console.log(` ~ ${p.counts.modified} modified`); if (p.counts.removed) console.log(` - ${p.counts.removed} removed`); diff --git a/src/core/templates/skill-templates.ts b/src/core/templates/skill-templates.ts index 481611930..132a401ef 100644 --- a/src/core/templates/skill-templates.ts +++ b/src/core/templates/skill-templates.ts @@ -133,8 +133,8 @@ If the user mentions a change or you detect one is relevant: | Insight Type | Where to Capture | |--------------|------------------| - | New requirement discovered | \`specs//spec.md\` | - | Requirement changed | \`specs//spec.md\` | + | New requirement discovered | \`specs//spec.md\` | + | Requirement changed | \`specs//spec.md\` | | Design decision made | \`design.md\` | | Scope changed | \`proposal.md\` | | New work identified | \`tasks.md\` | @@ -486,12 +486,45 @@ Common artifact patterns: **spec-driven schema** (proposal → specs → design → tasks): - **proposal.md**: Ask user about the change if not clear. Fill in Why, What Changes, Capabilities, Impact. - The Capabilities section is critical - each capability listed will need a spec file. -- **specs//spec.md**: Create one spec per capability listed in the proposal's Capabilities section (use the capability name, not the change name). +- **specs//spec.md**: Create one spec per capability listed in the proposal's Capabilities section (use the capability path, not the change name). - **design.md**: Document technical decisions, architecture, and implementation approach. - **tasks.md**: Break down implementation into checkboxed tasks. For other schemas, follow the \`instruction\` field from the CLI output. +**Spec Structure** + +OpenSpec supports both flat and hierarchical spec organization: + +**Flat structure** (traditional): +\`\`\` +openspec/specs/ + auth/spec.md # Capability: "auth" + api/spec.md # Capability: "api" + database/spec.md # Capability: "database" +\`\`\` + +**Hierarchical structure** (for complex projects): +\`\`\` +openspec/specs/ + _global/ + testing/spec.md # Capability: "_global/testing" + security/spec.md # Capability: "_global/security" + platform/ + services/ + api/spec.md # Capability: "platform/services/api" + auth/spec.md # Capability: "platform/services/auth" +\`\`\` + +**Delta replication**: Change deltas mirror the main spec structure 1:1: +- Main: \`openspec/specs/_global/testing/spec.md\` +- Delta: \`openspec/changes//specs/_global/testing/spec.md\` + +Use hierarchical paths when: +- Organizing specs by domain/scope (e.g., \`_global/\`, \`frontend/\`, \`backend/\`) +- Managing large codebases with many capabilities +- Grouping related capabilities for better discoverability + **Guardrails** - Create ONE artifact per invocation - Always read dependency artifacts before creating a new one @@ -797,7 +830,7 @@ This is an **agent-driven** operation - you will read delta specs and directly e 2. **Find delta specs** - Look for delta spec files in \`openspec/changes//specs/*/spec.md\`. + Look for delta spec files in \`openspec/changes//specs//spec.md\`. Each delta spec file contains sections like: - \`## ADDED Requirements\` - New requirements to add @@ -809,11 +842,11 @@ This is an **agent-driven** operation - you will read delta specs and directly e 3. **For each delta spec, apply changes to main specs** - For each capability with a delta spec at \`openspec/changes//specs//spec.md\`: + For each capability with a delta spec at \`openspec/changes//specs//spec.md\`: a. **Read the delta spec** to understand the intended changes - b. **Read the main spec** at \`openspec/specs//spec.md\` (may not exist yet) + b. **Read the main spec** at \`openspec/specs//spec.md\` (may not exist yet) c. **Apply changes intelligently**: @@ -836,7 +869,7 @@ This is an **agent-driven** operation - you will read delta specs and directly e - Find the FROM requirement, rename to TO d. **Create new main spec** if capability doesn't exist yet: - - Create \`openspec/specs//spec.md\` + - Create \`openspec/specs//spec.md\` - Add Purpose section (can be brief, mark as TBD) - Add Requirements section with the ADDED requirements @@ -1146,7 +1179,7 @@ Here's a draft proposal: ## Capabilities ### New Capabilities -- \`\`: [brief description] +- \`\`: [brief description] ### Modified Capabilities @@ -1191,9 +1224,9 @@ For a small task like this, we might only need one spec file. **DO:** Create the spec file: \`\`\`bash # Unix/macOS -mkdir -p openspec/changes//specs/ +mkdir -p openspec/changes//specs/ # Windows (PowerShell) -# New-Item -ItemType Directory -Force -Path "openspec/changes//specs/" +# New-Item -ItemType Directory -Force -Path "openspec/changes//specs/" \`\`\` Draft the spec content: @@ -1220,7 +1253,7 @@ Here's the spec: This format—WHEN/THEN/AND—makes requirements testable. You can literally read them as test cases. \`\`\` -Save to \`openspec/changes//specs//spec.md\`. +Save to \`openspec/changes//specs//spec.md\`. --- @@ -1599,8 +1632,8 @@ If the user mentions a change or you detect one is relevant: | Insight Type | Where to Capture | |--------------|------------------| - | New requirement discovered | \`specs//spec.md\` | - | Requirement changed | \`specs//spec.md\` | + | New requirement discovered | \`specs//spec.md\` | + | Requirement changed | \`specs//spec.md\` | | Design decision made | \`design.md\` | | Scope changed | \`proposal.md\` | | New work identified | \`tasks.md\` | @@ -1826,12 +1859,45 @@ Common artifact patterns: **spec-driven schema** (proposal → specs → design → tasks): - **proposal.md**: Ask user about the change if not clear. Fill in Why, What Changes, Capabilities, Impact. - The Capabilities section is critical - each capability listed will need a spec file. -- **specs//spec.md**: Create one spec per capability listed in the proposal's Capabilities section (use the capability name, not the change name). +- **specs//spec.md**: Create one spec per capability listed in the proposal's Capabilities section (use the capability path, not the change name). - **design.md**: Document technical decisions, architecture, and implementation approach. - **tasks.md**: Break down implementation into checkboxed tasks. For other schemas, follow the \`instruction\` field from the CLI output. +**Spec Structure** + +OpenSpec supports both flat and hierarchical spec organization: + +**Flat structure** (traditional): +\`\`\` +openspec/specs/ + auth/spec.md # Capability: "auth" + api/spec.md # Capability: "api" + database/spec.md # Capability: "database" +\`\`\` + +**Hierarchical structure** (for complex projects): +\`\`\` +openspec/specs/ + _global/ + testing/spec.md # Capability: "_global/testing" + security/spec.md # Capability: "_global/security" + platform/ + services/ + api/spec.md # Capability: "platform/services/api" + auth/spec.md # Capability: "platform/services/auth" +\`\`\` + +**Delta replication**: Change deltas mirror the main spec structure 1:1: +- Main: \`openspec/specs/_global/testing/spec.md\` +- Delta: \`openspec/changes//specs/_global/testing/spec.md\` + +Use hierarchical paths when: +- Organizing specs by domain/scope (e.g., \`_global/\`, \`frontend/\`, \`backend/\`) +- Managing large codebases with many capabilities +- Grouping related capabilities for better discoverability + **Guardrails** - Create ONE artifact per invocation - Always read dependency artifacts before creating a new one @@ -2156,7 +2222,7 @@ export function getArchiveChangeSkillTemplate(): SkillTemplate { Check for delta specs at \`openspec/changes//specs/\`. If none exist, proceed without sync prompt. **If delta specs exist:** - - Compare each delta spec with its corresponding main spec at \`openspec/specs//spec.md\` + - Compare each delta spec with its corresponding main spec at \`openspec/specs//spec.md\` - Determine what changes would be applied (adds, modifications, removals, renames) - Show a combined summary before prompting @@ -2495,7 +2561,7 @@ This is an **agent-driven** operation - you will read delta specs and directly e 2. **Find delta specs** - Look for delta spec files in \`openspec/changes//specs/*/spec.md\`. + Look for delta spec files in \`openspec/changes//specs//spec.md\`. Each delta spec file contains sections like: - \`## ADDED Requirements\` - New requirements to add @@ -2507,11 +2573,11 @@ This is an **agent-driven** operation - you will read delta specs and directly e 3. **For each delta spec, apply changes to main specs** - For each capability with a delta spec at \`openspec/changes//specs//spec.md\`: + For each capability with a delta spec at \`openspec/changes//specs//spec.md\`: a. **Read the delta spec** to understand the intended changes - b. **Read the main spec** at \`openspec/specs//spec.md\` (may not exist yet) + b. **Read the main spec** at \`openspec/specs//spec.md\` (may not exist yet) c. **Apply changes intelligently**: @@ -2534,7 +2600,7 @@ This is an **agent-driven** operation - you will read delta specs and directly e - Find the FROM requirement, rename to TO d. **Create new main spec** if capability doesn't exist yet: - - Create \`openspec/specs//spec.md\` + - Create \`openspec/specs//spec.md\` - Add Purpose section (can be brief, mark as TBD) - Add Requirements section with the ADDED requirements @@ -2833,7 +2899,7 @@ export function getOpsxArchiveCommandTemplate(): CommandTemplate { Check for delta specs at \`openspec/changes//specs/\`. If none exist, proceed without sync prompt. **If delta specs exist:** - - Compare each delta spec with its corresponding main spec at \`openspec/specs//spec.md\` + - Compare each delta spec with its corresponding main spec at \`openspec/specs//spec.md\` - Determine what changes would be applied (adds, modifications, removals, renames) - Show a combined summary before prompting diff --git a/src/core/validation/validator.ts b/src/core/validation/validator.ts index e6928cbda..5ed5c2a07 100644 --- a/src/core/validation/validator.ts +++ b/src/core/validation/validator.ts @@ -1,4 +1,4 @@ -import { z, ZodError } from 'zod'; +import { ZodError } from 'zod'; import { readFileSync, promises as fs } from 'fs'; import path from 'path'; import { SpecSchema, ChangeSchema, Spec, Change } from '../schemas/index.js'; @@ -12,6 +12,7 @@ import { } from './constants.js'; import { parseDeltaSpec, normalizeRequirementName } from '../parsers/requirement-blocks.js'; import { FileSystemUtils } from '../../utils/file-system.js'; +import { findAllSpecs } from '../../utils/spec-discovery.js'; export class Validator { private strictMode: boolean; @@ -119,20 +120,20 @@ export class Validator { const emptySectionSpecs: Array<{ path: string; sections: string[] }> = []; try { - const entries = await fs.readdir(specsDir, { withFileTypes: true }); - for (const entry of entries) { - if (!entry.isDirectory()) continue; - const specName = entry.name; - const specFile = path.join(specsDir, specName, 'spec.md'); + // Use recursive spec discovery to support hierarchical structures + const specs = findAllSpecs(specsDir); + + for (const spec of specs) { let content: string | undefined; try { - content = await fs.readFile(specFile, 'utf-8'); + content = await fs.readFile(spec.path, 'utf-8'); } catch { continue; } const plan = parseDeltaSpec(content); - const entryPath = `${specName}/spec.md`; + // Use full capability path (e.g., "_global/testing/spec.md" instead of "testing/spec.md") + const entryPath = path.join(spec.capability, 'spec.md'); const sectionNames: string[] = []; if (plan.sectionPresence.added) sectionNames.push('## ADDED Requirements'); if (plan.sectionPresence.modified) sectionNames.push('## MODIFIED Requirements'); @@ -257,10 +258,10 @@ export class Validator { message: `Delta sections ${this.formatSectionList(sections)} were found, but no requirement entries parsed. Ensure each section includes at least one "### Requirement:" block (REMOVED may use bullet list syntax).`, }); } - for (const path of missingHeaderSpecs) { + for (const specPath of missingHeaderSpecs) { issues.push({ level: 'ERROR', - path, + path: specPath, message: 'No delta sections found. Add headers such as "## ADDED Requirements" or move non-delta notes outside specs/.', }); } diff --git a/src/core/view.ts b/src/core/view.ts index e67c35268..5689cbe07 100644 --- a/src/core/view.ts +++ b/src/core/view.ts @@ -3,6 +3,7 @@ import * as path from 'path'; import chalk from 'chalk'; import { getTaskProgressForChange, formatTaskStatus } from '../utils/task-progress.js'; import { MarkdownParser } from './parsers/markdown-parser.js'; +import { findAllSpecs, isSpecStructureHierarchical } from '../utils/spec-discovery.js'; export class ViewCommand { async execute(targetPath: string = '.'): Promise { @@ -18,7 +19,7 @@ export class ViewCommand { // Get changes and specs data const changesData = await this.getChangesData(openspecDir); - const specsData = await this.getSpecsData(openspecDir); + const { specs: specsData, isHierarchical } = await this.getSpecsData(openspecDir); // Display summary metrics this.displaySummary(changesData, specsData); @@ -62,16 +63,17 @@ export class ViewCommand { if (specsData.length > 0) { console.log(chalk.bold.blue('\nSpecifications')); console.log('─'.repeat(60)); - - // Sort specs by requirement count (descending) - specsData.sort((a, b) => b.requirementCount - a.requirementCount); - - specsData.forEach(spec => { - const reqLabel = spec.requirementCount === 1 ? 'requirement' : 'requirements'; - console.log( - ` ${chalk.blue('▪')} ${chalk.bold(spec.name.padEnd(30))} ${chalk.dim(`${spec.requirementCount} ${reqLabel}`)}` - ); - }); + + // Sort by capability path when hierarchical, by requirement count when flat + if (!isHierarchical) { + specsData.sort((a, b) => b.requirementCount - a.requirementCount); + } + + if (isHierarchical) { + this.displayHierarchicalSpecs(specsData); + } else { + this.displayFlatSpecs(specsData); + } } console.log('\n' + '═'.repeat(60)); @@ -129,36 +131,35 @@ export class ViewCommand { return { draft, active, completed }; } - private async getSpecsData(openspecDir: string): Promise> { + private async getSpecsData(openspecDir: string): Promise<{ + specs: Array<{ name: string; requirementCount: number }>; + isHierarchical: boolean; + }> { const specsDir = path.join(openspecDir, 'specs'); - + if (!fs.existsSync(specsDir)) { - return []; + return { specs: [], isHierarchical: false }; } + // Use spec-discovery utility to find all specs (supports hierarchical) + const discoveredSpecs = findAllSpecs(specsDir); + const isHierarchical = isSpecStructureHierarchical(discoveredSpecs); const specs: Array<{ name: string; requirementCount: number }> = []; - const entries = fs.readdirSync(specsDir, { withFileTypes: true }); - - for (const entry of entries) { - if (entry.isDirectory()) { - const specFile = path.join(specsDir, entry.name, 'spec.md'); - - if (fs.existsSync(specFile)) { - try { - const content = fs.readFileSync(specFile, 'utf-8'); - const parser = new MarkdownParser(content); - const spec = parser.parseSpec(entry.name); - const requirementCount = spec.requirements.length; - specs.push({ name: entry.name, requirementCount }); - } catch (error) { - // If spec cannot be parsed, include with 0 count - specs.push({ name: entry.name, requirementCount: 0 }); - } - } + + for (const discoveredSpec of discoveredSpecs) { + try { + const content = fs.readFileSync(discoveredSpec.path, 'utf-8'); + const parser = new MarkdownParser(content); + const spec = parser.parseSpec(discoveredSpec.capability); + const requirementCount = spec.requirements.length; + specs.push({ name: discoveredSpec.capability, requirementCount }); + } catch (error) { + // If spec cannot be parsed, include with 0 count + specs.push({ name: discoveredSpec.capability, requirementCount: 0 }); } } - return specs; + return { specs, isHierarchical }; } private displaySummary( @@ -206,14 +207,50 @@ export class ViewCommand { private createProgressBar(completed: number, total: number, width: number = 20): string { if (total === 0) return chalk.dim('─'.repeat(width)); - + const percentage = completed / total; const filled = Math.round(percentage * width); const empty = width - filled; - + const filledBar = chalk.green('█'.repeat(filled)); const emptyBar = chalk.dim('░'.repeat(empty)); - + return `[${filledBar}${emptyBar}]`; } + + /** + * Display specs in flat structure (backward compatible) + */ + private displayFlatSpecs(specs: Array<{ name: string; requirementCount: number }>): void { + specs.forEach(spec => { + const reqLabel = spec.requirementCount === 1 ? 'requirement' : 'requirements'; + console.log( + ` ${chalk.blue('▪')} ${chalk.bold(spec.name.padEnd(30))} ${chalk.dim(`${spec.requirementCount} ${reqLabel}`)}` + ); + }); + } + + /** + * Display specs in hierarchical structure with full capability paths + */ + private displayHierarchicalSpecs(specs: Array<{ name: string; requirementCount: number }>): void { + const maxWidth = Math.max(...specs.map(s => s.name.length)); + let lastTopLevel = ''; + + for (const spec of specs) { + const topLevel = spec.name.split(path.sep)[0]; + const reqLabel = spec.requirementCount === 1 ? 'requirement' : 'requirements'; + + // Add spacing between different top-level groups + if (topLevel !== lastTopLevel && lastTopLevel !== '') { + console.log(''); + } + + console.log( + ` ${chalk.blue('▪')} ${chalk.bold(spec.name.padEnd(maxWidth))} ${chalk.dim(`${spec.requirementCount} ${reqLabel}`)}` + ); + + lastTopLevel = topLevel; + } + } } \ No newline at end of file diff --git a/src/utils/item-discovery.ts b/src/utils/item-discovery.ts index 1a86c3aed..4d5f8fe12 100644 --- a/src/utils/item-discovery.ts +++ b/src/utils/item-discovery.ts @@ -1,5 +1,6 @@ import { promises as fs } from 'fs'; import path from 'path'; +import { findAllSpecs } from './spec-discovery.js'; export async function getActiveChangeIds(root: string = process.cwd()): Promise { const changesPath = path.join(root, 'openspec', 'changes'); @@ -24,23 +25,12 @@ export async function getActiveChangeIds(root: string = process.cwd()): Promise< export async function getSpecIds(root: string = process.cwd()): Promise { const specsPath = path.join(root, 'openspec', 'specs'); - const result: string[] = []; try { - const entries = await fs.readdir(specsPath, { withFileTypes: true }); - for (const entry of entries) { - if (!entry.isDirectory() || entry.name.startsWith('.')) continue; - const specFile = path.join(specsPath, entry.name, 'spec.md'); - try { - await fs.access(specFile); - result.push(entry.name); - } catch { - // ignore - } - } + const specs = findAllSpecs(specsPath); + return specs.map(spec => spec.capability).sort(); } catch { - // ignore + return []; } - return result.sort(); } export async function getArchivedChangeIds(root: string = process.cwd()): Promise { diff --git a/src/utils/spec-discovery.ts b/src/utils/spec-discovery.ts new file mode 100644 index 000000000..4430d79f4 --- /dev/null +++ b/src/utils/spec-discovery.ts @@ -0,0 +1,404 @@ +/** + * Spec Discovery Utility + * + * Centralized spec discovery, capability resolution, and structure detection + * for both flat and hierarchical spec organizations. + * + * Supports: + * - Flat structure: specs/auth/spec.md (capability = "auth") + * - Hierarchical: specs/_global/testing/spec.md (capability = "_global/testing") + */ + +import * as fs from 'fs'; +import * as path from 'path'; + +/** + * Represents a discovered spec with its metadata + */ +export interface DiscoveredSpec { + /** Relative path from specs/ directory (e.g., "_global/testing" or "auth") */ + capability: string; + /** Absolute path to the spec.md file */ + path: string; + /** Hierarchy depth (number of path segments) */ + depth: number; +} + +/** + * Represents a spec update operation (delta to main mapping) + */ +export interface SpecUpdate { + /** Path to source delta spec in change directory */ + source: string; + /** Path to target main spec */ + target: string; + /** Capability identifier (relative path) */ + capability: string; + /** Whether the target spec already exists */ + exists: boolean; +} + +/** + * Configuration for spec structure and validation + */ +export interface SpecStructureConfig { + /** Structure mode: 'flat', 'hierarchical', or 'auto' (default) */ + structure?: 'flat' | 'hierarchical' | 'auto'; + /** Maximum allowed depth (default: 4) */ + maxDepth?: number; + /** Allow mixing flat and hierarchical specs (default: true) */ + allowMixed?: boolean; + /** Enforce naming conventions (default: true) */ + validatePaths?: boolean; +} + +/** + * Validation issue severity levels + */ +export type ValidationLevel = 'ERROR' | 'WARNING'; + +/** + * Represents a validation issue found during spec structure validation + */ +export interface ValidationIssue { + /** Severity level */ + level: ValidationLevel; + /** Human-readable error message */ + message: string; + /** Capability that caused the issue (if applicable) */ + capability?: string; +} + +/** + * Recursively find all spec.md files in a directory tree. + * + * Discovers specs at any depth and constructs capability names from + * relative paths. Works with both flat and hierarchical structures. + * + * @param baseDir - Base directory to search (typically openspec/specs) + * @returns Array of discovered specs with metadata + * + * @example + * ```typescript + * // Flat structure + * findAllSpecs('/project/openspec/specs') + * // Returns: [{ capability: 'auth', path: '/project/openspec/specs/auth/spec.md', depth: 1 }] + * + * // Hierarchical structure + * findAllSpecs('/project/openspec/specs') + * // Returns: [{ capability: '_global/testing', path: '/project/openspec/specs/_global/testing/spec.md', depth: 2 }] + * ``` + */ +export function findAllSpecs(baseDir: string): DiscoveredSpec[] { + const specs: DiscoveredSpec[] = []; + + /** + * Recursive walker that traverses directory tree + */ + function walk(dir: string, relativePath: string = ''): void { + let entries: fs.Dirent[]; + + try { + entries = fs.readdirSync(dir, { withFileTypes: true }); + entries.sort((a, b) => a.name.localeCompare(b.name)); + } catch { + // Directory doesn't exist or is not readable + return; + } + + for (const entry of entries) { + const fullPath = path.join(dir, entry.name); + const relPath = relativePath + ? path.join(relativePath, entry.name) + : entry.name; + + if (entry.isFile() && entry.name === 'spec.md') { + // Skip spec.md found directly in baseDir (no valid capability) + if (!relativePath) { + continue; + } + // Found a spec file - capability is the parent directory path + const depth = relativePath.split(path.sep).length; + + specs.push({ + capability: relativePath, + path: fullPath, + depth + }); + } else if (entry.isDirectory()) { + // Recurse into subdirectories + walk(fullPath, relPath); + } + } + } + + walk(baseDir); + return specs; +} + +/** + * Auto-detect whether spec structure is hierarchical or flat. + * + * Considers structure hierarchical if any spec has depth > 1 + * (indicating nested directories). + * + * Accepts either a directory path (discovers specs internally) or + * a pre-discovered array (avoids redundant filesystem traversal). + * + * @param specsOrDir - Base specs directory path, or pre-discovered specs array + * @returns true if hierarchical structure detected, false if flat + * + * @example + * ```typescript + * // From directory path + * isSpecStructureHierarchical('/project/openspec/specs') // returns true/false + * + * // From pre-discovered specs (avoids double traversal) + * const specs = findAllSpecs('/project/openspec/specs'); + * isSpecStructureHierarchical(specs) // returns true/false + * ``` + */ +export function isSpecStructureHierarchical(specsOrDir: string | DiscoveredSpec[]): boolean { + const specs = typeof specsOrDir === 'string' ? findAllSpecs(specsOrDir) : specsOrDir; + + return specs.some(s => s.depth > 1); +} + +/** + * Find all delta spec updates from a change directory. + * + * Maps delta specs to their corresponding main specs using 1:1 + * path-relative mapping. Supports hierarchical structures by + * replicating the directory structure between change and main specs. + * + * @param changeDir - Path to change directory (e.g., openspec/changes/my-change) + * @param mainSpecsDir - Path to main specs directory (e.g., openspec/specs) + * @returns Array of spec updates with source, target, and metadata + * + * @example + * ```typescript + * // Change delta: openspec/changes/my-change/specs/_global/testing/spec.md + * // Maps to main: openspec/specs/_global/testing/spec.md + * findSpecUpdates('/project/openspec/changes/my-change', '/project/openspec/specs') + * // Returns: [{ source: '...', target: '...', capability: '_global/testing', exists: true }] + * ``` + */ +export function findSpecUpdates(changeDir: string, mainSpecsDir: string): SpecUpdate[] { + const updates: SpecUpdate[] = []; + const changeSpecsDir = path.join(changeDir, 'specs'); + + // Find all delta specs recursively in change directory + const deltaSpecs = findAllSpecs(changeSpecsDir); + + for (const delta of deltaSpecs) { + // Map using relative path - preserves hierarchy + const targetPath = path.join(mainSpecsDir, delta.capability, 'spec.md'); + + // Check if target spec already exists + let exists = false; + try { + fs.accessSync(targetPath, fs.constants.F_OK); + exists = true; + } catch { + exists = false; + } + + updates.push({ + source: delta.path, + target: targetPath, + capability: delta.capability, + exists + }); + } + + return updates; +} + +/** + * Validate spec structure against configuration rules. + * + * Performs validation checks including: + * - Orphaned specs (spec.md at intermediate directory levels) + * - Depth limits (warn/error based on maxDepth config) + * - Naming conventions (lowercase alphanumeric with hyphens/underscores) + * - Reserved directory names + * + * @param specs - Array of specs to validate + * @param config - Configuration with validation rules + * @returns Array of validation issues (empty if all valid) + * + * @example + * ```typescript + * const specs = findAllSpecs('/project/openspec/specs'); + * const config = { maxDepth: 4, validatePaths: true }; + * const issues = validateSpecStructure(specs, config); + * + * if (issues.length > 0) { + * issues.forEach(issue => { + * console.error(`${issue.level}: ${issue.message}`); + * }); + * } + * ``` + */ +export function validateSpecStructure( + specs: DiscoveredSpec[], + config: SpecStructureConfig +): ValidationIssue[] { + const issues: ValidationIssue[] = []; + + // Apply default config values + const structure = config.structure || 'auto'; + const allowMixed = config.allowMixed ?? true; + const maxDepth = config.maxDepth ?? 4; + const validatePaths = config.validatePaths ?? true; + + // Enforce structure mode + if (structure === 'flat') { + for (const spec of specs) { + if (spec.depth > 1) { + issues.push({ + level: 'ERROR', + message: `Spec "${spec.capability}" has depth ${spec.depth} but structure is set to "flat". Flat specs must have depth 1 (e.g., "auth", not "domain/auth").`, + capability: spec.capability, + }); + } + } + } else if (structure === 'hierarchical') { + for (const spec of specs) { + if (spec.depth === 1) { + issues.push({ + level: 'ERROR', + message: `Spec "${spec.capability}" has depth 1 but structure is set to "hierarchical". Use nested paths (e.g., "domain/${spec.capability}").`, + capability: spec.capability, + }); + } + } + } else if (structure === 'auto' && !allowMixed) { + const hasFlat = specs.some(s => s.depth === 1); + const hasHierarchical = specs.some(s => s.depth > 1); + if (hasFlat && hasHierarchical) { + const flatCount = specs.filter(s => s.depth === 1).length; + const hierarchicalCount = specs.filter(s => s.depth > 1).length; + issues.push({ + level: 'ERROR', + message: `Mixed spec structure detected (${flatCount} flat, ${hierarchicalCount} hierarchical) but allowMixed is false. Use a consistent structure or set allowMixed: true.`, + }); + } + } + + // Check for orphaned specs (spec.md at intermediate levels) + const capabilitySet = new Set(specs.map(s => s.capability)); + const reportedOrphans = new Set(); + + for (const spec of specs) { + const segments = spec.capability.split(path.sep); + + // Check all parent paths + for (let i = 1; i < segments.length; i++) { + const parentPath = segments.slice(0, i).join(path.sep); + + if (capabilitySet.has(parentPath) && !reportedOrphans.has(parentPath)) { + reportedOrphans.add(parentPath); + issues.push({ + level: 'ERROR', + message: `Orphaned spec found at intermediate level "${parentPath}". Specs should only exist at leaf directories. Found both "${path.join(parentPath, 'spec.md')}" and "${path.join(spec.capability, 'spec.md')}".`, + capability: parentPath, + }); + } + } + } + + // Check depth limits + const RECOMMENDED_MAX_DEPTH = 3; + const HARD_LIMIT_DEPTH = 10; + + for (const spec of specs) { + // Error if exceeds configured maxDepth (capped at hard limit) + const effectiveMax = Math.min(maxDepth, HARD_LIMIT_DEPTH); + + if (spec.depth > effectiveMax) { + issues.push({ + level: 'ERROR', + message: `Spec "${spec.capability}" exceeds maximum depth ${effectiveMax} (actual: ${spec.depth}). Consider simplifying the hierarchy.`, + capability: spec.capability, + }); + } + // Warning if exceeds recommended depth + else if (spec.depth > RECOMMENDED_MAX_DEPTH && spec.depth <= effectiveMax) { + issues.push({ + level: 'WARNING', + message: `Spec "${spec.capability}" has depth ${spec.depth}. Recommended maximum is ${RECOMMENDED_MAX_DEPTH} for maintainability.`, + capability: spec.capability, + }); + } + } + + // Check path length (Windows MAX_PATH compatibility) + if (validatePaths) { + const WINDOWS_MAX_PATH = 260; + const MAX_CAPABILITY_LENGTH = 160; // Leaves ~100 chars for project root + openspec/specs/ + /spec.md + for (const spec of specs) { + if (spec.path.length > WINDOWS_MAX_PATH) { + issues.push({ + level: 'WARNING', + message: `Spec "${spec.capability}" has a full path of ${spec.path.length} characters, exceeding Windows MAX_PATH (${WINDOWS_MAX_PATH}). This will cause issues on Windows.`, + capability: spec.capability, + }); + } else if (spec.capability.length > MAX_CAPABILITY_LENGTH) { + issues.push({ + level: 'WARNING', + message: `Spec "${spec.capability}" has a capability path of ${spec.capability.length} characters (max recommended: ${MAX_CAPABILITY_LENGTH}). Long paths may cause issues on Windows depending on project location.`, + capability: spec.capability, + }); + } + } + } + + // Check naming conventions (if enabled) + if (validatePaths) { + const VALID_NAME_PATTERN = /^[a-z0-9-_]+$/; + const RESERVED_NAMES = ['..', '.', '.git', 'node_modules', '.openspec']; + // Windows reserved device names (case-insensitive, but regex already rejects uppercase) + const WINDOWS_RESERVED = ['con', 'prn', 'aux', 'nul', + 'com1', 'com2', 'com3', 'com4', 'com5', 'com6', 'com7', 'com8', 'com9', + 'lpt1', 'lpt2', 'lpt3', 'lpt4', 'lpt5', 'lpt6', 'lpt7', 'lpt8', 'lpt9']; + + for (const spec of specs) { + const segments = spec.capability.split(path.sep); + + for (const segment of segments) { + // Check reserved names + if (RESERVED_NAMES.includes(segment)) { + issues.push({ + level: 'ERROR', + message: `Reserved name "${segment}" not allowed in capability "${spec.capability}". Reserved names: ${RESERVED_NAMES.join(', ')}`, + capability: spec.capability, + }); + break; + } + + // Check Windows reserved device names + if (WINDOWS_RESERVED.includes(segment)) { + issues.push({ + level: 'ERROR', + message: `Windows reserved name "${segment}" not allowed in capability "${spec.capability}". These names cause issues on Windows filesystems.`, + capability: spec.capability, + }); + break; + } + + // Check naming pattern + if (!VALID_NAME_PATTERN.test(segment)) { + issues.push({ + level: 'ERROR', + message: `Invalid segment "${segment}" in capability "${spec.capability}". Use lowercase alphanumeric characters with hyphens or underscores only.`, + capability: spec.capability, + }); + break; // Only report once per capability + } + } + } + } + + return issues; +} diff --git a/test/commands/validate.test.ts b/test/commands/validate.test.ts index b94f72d35..f96aa41fa 100644 --- a/test/commands/validate.test.ts +++ b/test/commands/validate.test.ts @@ -144,4 +144,174 @@ describe('top-level validate command', () => { // Should complete without hanging and without prompts expect(result.stderr).not.toContain('What would you like to validate?'); }); + + describe('hierarchical specs support', () => { + it('validates hierarchical specs at depth 2', async () => { + // Create hierarchical spec: _global/testing + const hierarchicalContent = [ + '## Purpose', + 'Testing standards for the system.', + '', + '## Requirements', + '', + '### Requirement: System SHALL have unit tests', + 'All modules SHALL include unit tests.', + '', + '#### Scenario: Unit test coverage', + '- **GIVEN** a module with business logic', + '- **WHEN** tests are executed', + '- **THEN** coverage meets minimum threshold', + ].join('\n'); + + const hierarchicalSpecDir = path.join(specsDir, '_global', 'testing'); + await fs.mkdir(hierarchicalSpecDir, { recursive: true }); + await fs.writeFile(path.join(hierarchicalSpecDir, 'spec.md'), hierarchicalContent, 'utf-8'); + + const result = await runCLI(['validate', '_global/testing'], { cwd: testDir }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('valid'); + }); + + it('validates hierarchical specs at depth 3', async () => { + // Create deep hierarchical spec: platform/services/api + const deepContent = [ + '## Purpose', + 'API service specifications.', + '', + '## Requirements', + '', + '### Requirement: API SHALL provide REST endpoints', + 'The API service SHALL expose RESTful endpoints.', + '', + '#### Scenario: REST endpoint access', + '- **GIVEN** an authenticated client', + '- **WHEN** the client makes a GET request', + '- **THEN** the response contains valid JSON', + ].join('\n'); + + const deepSpecDir = path.join(specsDir, 'platform', 'services', 'api'); + await fs.mkdir(deepSpecDir, { recursive: true }); + await fs.writeFile(path.join(deepSpecDir, 'spec.md'), deepContent, 'utf-8'); + + const result = await runCLI(['validate', 'platform/services/api'], { cwd: testDir }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('valid'); + }); + + it('validates mixed flat and hierarchical specs with --specs', async () => { + // Create another hierarchical spec + const hierarchicalContent = [ + '## Purpose', + 'Security standards.', + '', + '## Requirements', + '', + '### Requirement: System SHALL encrypt data', + 'All sensitive data SHALL be encrypted.', + '', + '#### Scenario: Data encryption', + '- **GIVEN** sensitive user data', + '- **WHEN** stored in the database', + '- **THEN** it is encrypted at rest', + ].join('\n'); + + const securitySpecDir = path.join(specsDir, '_global', 'security'); + await fs.mkdir(securitySpecDir, { recursive: true }); + await fs.writeFile(path.join(securitySpecDir, 'spec.md'), hierarchicalContent, 'utf-8'); + + // Validate all specs (includes flat 'alpha', 'dup' and hierarchical '_global/security') + const result = await runCLI(['validate', '--specs', '--json'], { cwd: testDir }); + expect(result.exitCode).toBe(0); + + const json = JSON.parse(result.stdout.trim()); + const specIds = json.items.map((item: any) => item.id); + + // Should include both flat and hierarchical specs + expect(specIds).toContain('alpha'); + expect(specIds).toContain('dup'); + expect(specIds).toContain('_global/security'); + }); + + it('includes structure validation issues in bulk validation', async () => { + // Create a spec with invalid naming (uppercase) + const invalidNamingDir = path.join(specsDir, 'Invalid-Name'); + await fs.mkdir(invalidNamingDir, { recursive: true }); + const invalidContent = [ + '## Purpose', + 'This spec has an invalid name.', + '', + '## Requirements', + '', + '### Requirement: Test SHALL work', + 'Test requirement.', + '', + '#### Scenario: Test', + '- **GIVEN** a test', + '- **WHEN** it runs', + '- **THEN** it passes', + ].join('\n'); + await fs.writeFile(path.join(invalidNamingDir, 'spec.md'), invalidContent, 'utf-8'); + + const result = await runCLI(['validate', '--specs', '--json'], { cwd: testDir }); + + // Should still complete but report issues + const json = JSON.parse(result.stdout.trim()); + + // Structure issues are in a separate structureValidation field (not in items) + expect(json.structureValidation).toBeDefined(); + expect(json.structureValidation.valid).toBe(false); + expect(json.structureValidation.issues.length).toBeGreaterThan(0); + + // Should have at least one naming issue (either "naming convention" or "Invalid segment") + const namingIssues = json.structureValidation.issues.filter((issue: any) => + issue.message.toLowerCase().includes('invalid segment') || + issue.message.toLowerCase().includes('naming') + ); + expect(namingIssues.length).toBeGreaterThan(0); + + // items should only contain real specs, not a phantom _structure entry + const structureItem = json.items.find((item: any) => item.id === '_structure'); + expect(structureItem).toBeUndefined(); + }); + + it('validates change with hierarchical delta structure', async () => { + // Test that validate command works with hierarchical change deltas + const changeId = 'hierarchical-delta-change'; + const changeContent = [ + '# Hierarchical Delta Change', + '', + '## Why', + 'Add monitoring specifications to global standards.', + '', + '## What Changes', + '- **_global/monitoring:** Add new monitoring requirements', + ].join('\n'); + + await fs.mkdir(path.join(changesDir, changeId), { recursive: true }); + await fs.writeFile(path.join(changesDir, changeId, 'proposal.md'), changeContent, 'utf-8'); + + // Create a hierarchical delta structure (now supported by validator) + const deltaContent = [ + '# Monitoring Specification - Changes', + '', + '## ADDED Requirements', + '', + '### Requirement: System SHALL have monitoring', + 'All services SHALL be monitored.', + '', + '#### Scenario: Monitoring enabled', + '- **GIVEN** a deployed service', + '- **WHEN** the service is running', + '- **THEN** metrics are collected and reported', + ].join('\n'); + + const deltaDir = path.join(changesDir, changeId, 'specs', '_global', 'monitoring'); + await fs.mkdir(deltaDir, { recursive: true }); + await fs.writeFile(path.join(deltaDir, 'spec.md'), deltaContent, 'utf-8'); + + const result = await runCLI(['validate', changeId], { cwd: testDir }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('valid'); + }); + }); }); diff --git a/test/core/archive.test.ts b/test/core/archive.test.ts index 597dbfb2f..414b0cfb4 100644 --- a/test/core/archive.test.ts +++ b/test/core/archive.test.ts @@ -11,6 +11,21 @@ vi.mock('@inquirer/prompts', () => ({ confirm: vi.fn() })); +/** + * Archive Command Tests + * + * NOTE: Most tests use `noValidate: true` for the following reasons: + * 1. Performance: Validation is expensive (parsing, schema validation, etc.) + * 2. Focus: These tests verify archive functionality (file operations, delta application, + * error handling), not validation logic. Validation has dedicated tests in validator.test.ts + * 3. Simplicity: Tests can use minimal spec fixtures sufficient for testing archive logic + * without needing complete, valid OpenSpec documents + * 4. Isolation: Each test should verify one concern. Archive tests verify archiving, + * not validation + * + * The validation feature itself is tested explicitly in the test at line 346: + * "should skip validation when commander sets validate to false (--no-validate)" + */ describe('ArchiveCommand', () => { let tempDir: string; let archiveCommand: ArchiveCommand; @@ -63,7 +78,7 @@ describe('ArchiveCommand', () => { await fs.writeFile(path.join(changeDir, 'tasks.md'), tasksContent); // Execute archive with --yes flag - await archiveCommand.execute(changeName, { yes: true }); + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); // Check that change was moved to archive const archiveDir = path.join(tempDir, 'openspec', 'changes', 'archive'); @@ -86,7 +101,7 @@ describe('ArchiveCommand', () => { await fs.writeFile(path.join(changeDir, 'tasks.md'), tasksContent); // Execute archive with --yes flag - await archiveCommand.execute(changeName, { yes: true }); + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); // Verify warning was logged expect(console.log).toHaveBeenCalledWith( @@ -256,7 +271,7 @@ New feature description. it('should throw error if change does not exist', async () => { await expect( - archiveCommand.execute('non-existent-change', { yes: true }) + archiveCommand.execute('non-existent-change', { yes: true, noValidate: true }) ).rejects.toThrow("Change 'non-existent-change' not found."); }); @@ -272,7 +287,7 @@ New feature description. // Try to archive await expect( - archiveCommand.execute(changeName, { yes: true }) + archiveCommand.execute(changeName, { yes: true, noValidate: true }) ).rejects.toThrow(`Archive '${date}-${changeName}' already exists.`); }); @@ -282,7 +297,7 @@ New feature description. await fs.mkdir(changeDir, { recursive: true }); // Execute archive without tasks.md - await archiveCommand.execute(changeName, { yes: true }); + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); // Should complete without warnings expect(console.log).not.toHaveBeenCalledWith( @@ -301,7 +316,7 @@ New feature description. await fs.mkdir(changeDir, { recursive: true }); // Execute archive without specs - await archiveCommand.execute(changeName, { yes: true }); + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); // Should complete without spec updates expect(console.log).not.toHaveBeenCalledWith( @@ -715,7 +730,7 @@ E1 updated`); await fs.rm(path.join(tempDir, 'openspec'), { recursive: true }); await expect( - archiveCommand.execute('any-change', { yes: true }) + archiveCommand.execute('any-change', { yes: true, noValidate: true }) ).rejects.toThrow("No OpenSpec changes directory found. Run 'openspec init' first."); }); }); @@ -735,7 +750,7 @@ E1 updated`); mockSelect.mockResolvedValueOnce(change1); // Execute without change name - await archiveCommand.execute(undefined, { yes: true }); + await archiveCommand.execute(undefined, { yes: true, noValidate: true }); // Verify select was called with correct options (values matter, names may include progress) expect(mockSelect).toHaveBeenCalledWith(expect.objectContaining({ @@ -799,9 +814,170 @@ E1 updated`); // Verify archive was cancelled expect(console.log).toHaveBeenCalledWith('Archive cancelled.'); - + // Verify change was not archived await expect(fs.access(changeDir)).resolves.not.toThrow(); }); + + describe('hierarchical specs support', () => { + it('should apply hierarchical delta specs (depth 2)', async () => { + const changeName = 'hierarchical-feature'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const changeSpecDir = path.join(changeDir, 'specs', '_global', 'testing'); + await fs.mkdir(changeSpecDir, { recursive: true }); + + // Create completed tasks + await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Task 1'); + + // Create a hierarchical delta spec with ADDED requirement + const deltaSpec = `# Testing Specification - Changes + +## ADDED Requirements + +### Requirement: Unit tests +System SHALL have unit tests`; + + await fs.writeFile(path.join(changeSpecDir, 'spec.md'), deltaSpec); + + // Execute archive + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + // Verify hierarchical spec was created in main specs + const mainSpecPath = path.join(tempDir, 'openspec', 'specs', '_global', 'testing', 'spec.md'); + await expect(fs.access(mainSpecPath)).resolves.not.toThrow(); + + const mainSpecContent = await fs.readFile(mainSpecPath, 'utf-8'); + expect(mainSpecContent).toContain('Unit tests'); + expect(mainSpecContent).toContain('System SHALL have unit tests'); + }); + + it('should apply hierarchical delta specs (depth 3)', async () => { + const changeName = 'deep-hierarchical'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const changeSpecDir = path.join(changeDir, 'specs', 'platform', 'services', 'api'); + await fs.mkdir(changeSpecDir, { recursive: true }); + + await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Complete'); + + const deltaSpec = `# API Service - Changes + +## ADDED Requirements + +### Requirement: REST endpoints +System SHALL provide REST endpoints + +### Requirement: GraphQL support +System SHALL support GraphQL`; + + await fs.writeFile(path.join(changeSpecDir, 'spec.md'), deltaSpec); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + // Verify deep hierarchical spec was created + const mainSpecPath = path.join(tempDir, 'openspec', 'specs', 'platform', 'services', 'api', 'spec.md'); + await expect(fs.access(mainSpecPath)).resolves.not.toThrow(); + + const mainSpecContent = await fs.readFile(mainSpecPath, 'utf-8'); + expect(mainSpecContent).toContain('REST endpoints'); + expect(mainSpecContent).toContain('GraphQL support'); + }); + + it('should update existing hierarchical specs with MODIFIED', async () => { + const changeName = 'modify-hierarchical'; + + // Create existing hierarchical spec in main + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', '_global', 'architecture'); + await fs.mkdir(mainSpecDir, { recursive: true }); + + const existingSpec = `## Purpose +Architecture specification + +## Requirements + +### Requirement: System design +Original system design requirement`; + + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), existingSpec); + + // Create change with delta + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const changeSpecDir = path.join(changeDir, 'specs', '_global', 'architecture'); + await fs.mkdir(changeSpecDir, { recursive: true }); + + await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Done'); + + const deltaSpec = `# Architecture - Changes + +## MODIFIED Requirements + +### Requirement: System design +Updated system design requirement with more details`; + + await fs.writeFile(path.join(changeSpecDir, 'spec.md'), deltaSpec); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + // Verify modification was applied + const mainSpecPath = path.join(mainSpecDir, 'spec.md'); + const mainSpecContent = await fs.readFile(mainSpecPath, 'utf-8'); + expect(mainSpecContent).toContain('Updated system design requirement with more details'); + expect(mainSpecContent).not.toContain('Original system design requirement'); + }); + + it('should handle mixed flat and hierarchical specs in same change', async () => { + const changeName = 'mixed-structure'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + + // Flat spec + const flatSpecDir = path.join(changeDir, 'specs', 'auth'); + await fs.mkdir(flatSpecDir, { recursive: true }); + await fs.writeFile( + path.join(flatSpecDir, 'spec.md'), + `# Auth - Changes\n\n## ADDED Requirements\n\n### Requirement: Login\nUser SHALL be able to login` + ); + + // Hierarchical spec + const hierarchicalSpecDir = path.join(changeDir, 'specs', '_global', 'logging'); + await fs.mkdir(hierarchicalSpecDir, { recursive: true }); + await fs.writeFile( + path.join(hierarchicalSpecDir, 'spec.md'), + `# Logging - Changes\n\n## ADDED Requirements\n\n### Requirement: Audit logs\nSystem SHALL maintain audit logs` + ); + + await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Task'); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + // Verify both specs were created + await expect(fs.access(path.join(tempDir, 'openspec', 'specs', 'auth', 'spec.md'))).resolves.not.toThrow(); + await expect(fs.access(path.join(tempDir, 'openspec', 'specs', '_global', 'logging', 'spec.md'))).resolves.not.toThrow(); + }); + + it('should preserve directory structure when syncing hierarchical deltas', async () => { + const changeName = 'preserve-structure'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + + // Create nested directory structure + const nestedSpecDir = path.join(changeDir, 'specs', 'services', 'backend', 'database'); + await fs.mkdir(nestedSpecDir, { recursive: true }); + + await fs.writeFile( + path.join(nestedSpecDir, 'spec.md'), + `# Database - Changes\n\n## ADDED Requirements\n\n### Requirement: Migrations\nSystem SHALL support migrations` + ); + + await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Done'); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + // Verify full directory structure was preserved + const mainSpecPath = path.join(tempDir, 'openspec', 'specs', 'services', 'backend', 'database', 'spec.md'); + await expect(fs.access(mainSpecPath)).resolves.not.toThrow(); + + // Verify all intermediate directories exist + await expect(fs.access(path.join(tempDir, 'openspec', 'specs', 'services'))).resolves.not.toThrow(); + await expect(fs.access(path.join(tempDir, 'openspec', 'specs', 'services', 'backend'))).resolves.not.toThrow(); + }); + }); }); }); diff --git a/test/core/config-schema-spec-structure.test.ts b/test/core/config-schema-spec-structure.test.ts new file mode 100644 index 000000000..eb4299a3e --- /dev/null +++ b/test/core/config-schema-spec-structure.test.ts @@ -0,0 +1,168 @@ +import { describe, it, expect } from 'vitest'; +import { GlobalConfigSchema, validateConfigKeyPath, DEFAULT_CONFIG } from '../../src/core/config-schema.js'; + +describe('config-schema - specStructure', () => { + describe('GlobalConfigSchema - specStructure field', () => { + it('should accept valid specStructure configuration', () => { + const config = { + featureFlags: {}, + specStructure: { + structure: 'hierarchical' as const, + maxDepth: 5, + allowMixed: false, + validatePaths: true, + }, + }; + + const result = GlobalConfigSchema.safeParse(config); + expect(result.success).toBe(true); + }); + + it('should accept auto structure mode', () => { + const config = { + specStructure: { + structure: 'auto' as const, + }, + }; + + const result = GlobalConfigSchema.safeParse(config); + expect(result.success).toBe(true); + }); + + it('should accept flat structure mode', () => { + const config = { + specStructure: { + structure: 'flat' as const, + }, + }; + + const result = GlobalConfigSchema.safeParse(config); + expect(result.success).toBe(true); + }); + + it('should reject invalid structure values', () => { + const config = { + specStructure: { + structure: 'invalid', + }, + }; + + const result = GlobalConfigSchema.safeParse(config); + expect(result.success).toBe(false); + }); + + it('should accept maxDepth within valid range (1-10)', () => { + const validDepths = [1, 4, 7, 10]; + + validDepths.forEach(depth => { + const config = { + specStructure: { maxDepth: depth }, + }; + const result = GlobalConfigSchema.safeParse(config); + expect(result.success).toBe(true); + }); + }); + + it('should reject maxDepth outside valid range', () => { + const invalidDepths = [0, -1, 11, 20]; + + invalidDepths.forEach(depth => { + const config = { + specStructure: { maxDepth: depth }, + }; + const result = GlobalConfigSchema.safeParse(config); + expect(result.success).toBe(false); + }); + }); + + it('should reject non-integer maxDepth', () => { + const config = { + specStructure: { maxDepth: 3.5 }, + }; + + const result = GlobalConfigSchema.safeParse(config); + expect(result.success).toBe(false); + }); + + it('should accept boolean values for allowMixed', () => { + const configTrue = { specStructure: { allowMixed: true } }; + const configFalse = { specStructure: { allowMixed: false } }; + + expect(GlobalConfigSchema.safeParse(configTrue).success).toBe(true); + expect(GlobalConfigSchema.safeParse(configFalse).success).toBe(true); + }); + + it('should accept boolean values for validatePaths', () => { + const configTrue = { specStructure: { validatePaths: true } }; + const configFalse = { specStructure: { validatePaths: false } }; + + expect(GlobalConfigSchema.safeParse(configTrue).success).toBe(true); + expect(GlobalConfigSchema.safeParse(configFalse).success).toBe(true); + }); + + it('should apply default values when fields are omitted', () => { + const config = { specStructure: {} }; + + const result = GlobalConfigSchema.parse(config); + + expect(result.specStructure?.structure).toBe('auto'); + expect(result.specStructure?.maxDepth).toBe(4); + expect(result.specStructure?.allowMixed).toBe(true); + expect(result.specStructure?.validatePaths).toBe(true); + }); + + it('should include specStructure in DEFAULT_CONFIG', () => { + expect(DEFAULT_CONFIG.specStructure).toBeDefined(); + expect(DEFAULT_CONFIG.specStructure?.structure).toBe('auto'); + expect(DEFAULT_CONFIG.specStructure?.maxDepth).toBe(4); + expect(DEFAULT_CONFIG.specStructure?.allowMixed).toBe(true); + expect(DEFAULT_CONFIG.specStructure?.validatePaths).toBe(true); + }); + }); + + describe('validateConfigKeyPath - specStructure nested keys', () => { + it('should accept specStructure as top-level key', () => { + const result = validateConfigKeyPath('specStructure'); + expect(result.valid).toBe(true); + }); + + it('should accept valid specStructure nested keys', () => { + const validKeys = [ + 'specStructure.structure', + 'specStructure.maxDepth', + 'specStructure.allowMixed', + 'specStructure.validatePaths', + ]; + + validKeys.forEach(key => { + const result = validateConfigKeyPath(key); + expect(result.valid).toBe(true); + }); + }); + + it('should reject invalid specStructure nested keys', () => { + const result = validateConfigKeyPath('specStructure.invalidKey'); + + expect(result.valid).toBe(false); + expect(result.reason).toContain('Unknown specStructure key'); + expect(result.reason).toContain('structure, maxDepth, allowMixed, validatePaths'); + }); + + it('should reject deeply nested specStructure keys', () => { + const result = validateConfigKeyPath('specStructure.structure.deeply.nested'); + + expect(result.valid).toBe(false); + expect(result.reason).toContain('do not support deeply nested keys'); + }); + + it('should provide helpful error message for unknown specStructure keys', () => { + const result = validateConfigKeyPath('specStructure.unknown'); + + expect(result.valid).toBe(false); + expect(result.reason).toContain('structure'); + expect(result.reason).toContain('maxDepth'); + expect(result.reason).toContain('allowMixed'); + expect(result.reason).toContain('validatePaths'); + }); + }); +}); diff --git a/test/core/global-config.test.ts b/test/core/global-config.test.ts index 052d32018..846a0a245 100644 --- a/test/core/global-config.test.ts +++ b/test/core/global-config.test.ts @@ -8,6 +8,7 @@ import { getGlobalConfigPath, getGlobalConfig, saveGlobalConfig, + getSpecStructureConfig, GLOBAL_CONFIG_DIR_NAME, GLOBAL_CONFIG_FILE_NAME } from '../../src/core/global-config.js'; @@ -100,7 +101,15 @@ describe('global-config', () => { const config = getGlobalConfig(); - expect(config).toEqual({ featureFlags: {} }); + expect(config).toEqual({ + featureFlags: {}, + specStructure: { + structure: 'auto', + maxDepth: 4, + allowMixed: true, + validatePaths: true + } + }); }); it('should not create directory when reading non-existent config', () => { @@ -137,7 +146,15 @@ describe('global-config', () => { const config = getGlobalConfig(); - expect(config).toEqual({ featureFlags: {} }); + expect(config).toEqual({ + featureFlags: {}, + specStructure: { + structure: 'auto', + maxDepth: 4, + allowMixed: true, + validatePaths: true + } + }); }); it('should log warning for invalid JSON', () => { @@ -253,4 +270,333 @@ describe('global-config', () => { expect(loadedConfig.featureFlags).toEqual(originalConfig.featureFlags); }); }); + + describe('getSpecStructureConfig', () => { + it('should return default values when no config file exists', () => { + process.env.XDG_CONFIG_HOME = tempDir; + + const config = getSpecStructureConfig(); + + expect(config.structure).toBe('auto'); + expect(config.maxDepth).toBe(4); + expect(config.allowMixed).toBe(true); + expect(config.validatePaths).toBe(true); + }); + + it('should return configured structure value', () => { + process.env.XDG_CONFIG_HOME = tempDir; + const configDir = path.join(tempDir, 'openspec'); + const configPath = path.join(configDir, 'config.json'); + + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(configPath, JSON.stringify({ + featureFlags: {}, + specStructure: { structure: 'hierarchical' } + })); + + const config = getSpecStructureConfig(); + + expect(config.structure).toBe('hierarchical'); + expect(config.maxDepth).toBe(4); // default + expect(config.allowMixed).toBe(true); // default + expect(config.validatePaths).toBe(true); // default + }); + + it('should return configured maxDepth value', () => { + process.env.XDG_CONFIG_HOME = tempDir; + const configDir = path.join(tempDir, 'openspec'); + const configPath = path.join(configDir, 'config.json'); + + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(configPath, JSON.stringify({ + featureFlags: {}, + specStructure: { maxDepth: 3 } + })); + + const config = getSpecStructureConfig(); + + expect(config.maxDepth).toBe(3); + }); + + it('should return configured allowMixed value', () => { + process.env.XDG_CONFIG_HOME = tempDir; + const configDir = path.join(tempDir, 'openspec'); + const configPath = path.join(configDir, 'config.json'); + + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(configPath, JSON.stringify({ + featureFlags: {}, + specStructure: { allowMixed: false } + })); + + const config = getSpecStructureConfig(); + + expect(config.allowMixed).toBe(false); + }); + + it('should return configured validatePaths value', () => { + process.env.XDG_CONFIG_HOME = tempDir; + const configDir = path.join(tempDir, 'openspec'); + const configPath = path.join(configDir, 'config.json'); + + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(configPath, JSON.stringify({ + featureFlags: {}, + specStructure: { validatePaths: false } + })); + + const config = getSpecStructureConfig(); + + expect(config.validatePaths).toBe(false); + }); + + it('should apply defaults for missing fields', () => { + process.env.XDG_CONFIG_HOME = tempDir; + const configDir = path.join(tempDir, 'openspec'); + const configPath = path.join(configDir, 'config.json'); + + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(configPath, JSON.stringify({ + featureFlags: {}, + specStructure: { + structure: 'flat' + // maxDepth, allowMixed, validatePaths omitted + } + })); + + const config = getSpecStructureConfig(); + + expect(config.structure).toBe('flat'); + expect(config.maxDepth).toBe(4); // default + expect(config.allowMixed).toBe(true); // default + expect(config.validatePaths).toBe(true); // default + }); + + it('should handle partial configuration', () => { + process.env.XDG_CONFIG_HOME = tempDir; + const configDir = path.join(tempDir, 'openspec'); + const configPath = path.join(configDir, 'config.json'); + + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(configPath, JSON.stringify({ + featureFlags: {}, + specStructure: { + maxDepth: 5, + allowMixed: false + } + })); + + const config = getSpecStructureConfig(); + + expect(config.structure).toBe('auto'); // default + expect(config.maxDepth).toBe(5); + expect(config.allowMixed).toBe(false); + expect(config.validatePaths).toBe(true); // default + }); + + it('should handle complete custom configuration', () => { + process.env.XDG_CONFIG_HOME = tempDir; + const configDir = path.join(tempDir, 'openspec'); + const configPath = path.join(configDir, 'config.json'); + + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(configPath, JSON.stringify({ + featureFlags: {}, + specStructure: { + structure: 'hierarchical', + maxDepth: 6, + allowMixed: false, + validatePaths: false + } + })); + + const config = getSpecStructureConfig(); + + expect(config.structure).toBe('hierarchical'); + expect(config.maxDepth).toBe(6); + expect(config.allowMixed).toBe(false); + expect(config.validatePaths).toBe(false); + }); + + it('should always return a Required with all fields', () => { + process.env.XDG_CONFIG_HOME = tempDir; + + const config = getSpecStructureConfig(); + + // All fields should be defined (not undefined) + expect(config.structure).toBeDefined(); + expect(config.maxDepth).toBeDefined(); + expect(config.allowMixed).toBeDefined(); + expect(config.validatePaths).toBeDefined(); + + // Check types + expect(typeof config.structure).toBe('string'); + expect(typeof config.maxDepth).toBe('number'); + expect(typeof config.allowMixed).toBe('boolean'); + expect(typeof config.validatePaths).toBe('boolean'); + }); + + it('should handle missing specStructure in config file', () => { + process.env.XDG_CONFIG_HOME = tempDir; + const configDir = path.join(tempDir, 'openspec'); + const configPath = path.join(configDir, 'config.json'); + + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(configPath, JSON.stringify({ + featureFlags: { someFlag: true } + // no specStructure field + })); + + const config = getSpecStructureConfig(); + + expect(config.structure).toBe('auto'); + expect(config.maxDepth).toBe(4); + expect(config.allowMixed).toBe(true); + expect(config.validatePaths).toBe(true); + }); + + it('should handle maxDepth of 1 explicitly', () => { + process.env.XDG_CONFIG_HOME = tempDir; + const configDir = path.join(tempDir, 'openspec'); + const configPath = path.join(configDir, 'config.json'); + + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(configPath, JSON.stringify({ + featureFlags: {}, + specStructure: { maxDepth: 1 } + })); + + const config = getSpecStructureConfig(); + + // 1 should be preserved (not replaced with default 4) + expect(config.maxDepth).toBe(1); + }); + + describe('with project overrides', () => { + it('should use project overrides for specific fields', () => { + process.env.XDG_CONFIG_HOME = tempDir; + const configDir = path.join(tempDir, 'openspec'); + const configPath = path.join(configDir, 'config.json'); + + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(configPath, JSON.stringify({ + featureFlags: {}, + specStructure: { maxDepth: 6 } + })); + + const config = getSpecStructureConfig({ structure: 'flat' }); + + expect(config.structure).toBe('flat'); + expect(config.maxDepth).toBe(6); // from global + expect(config.allowMixed).toBe(true); // default + expect(config.validatePaths).toBe(true); // default + }); + + it('should use project overrides for all fields', () => { + process.env.XDG_CONFIG_HOME = tempDir; + const configDir = path.join(tempDir, 'openspec'); + const configPath = path.join(configDir, 'config.json'); + + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(configPath, JSON.stringify({ + featureFlags: {}, + specStructure: { + structure: 'auto', + maxDepth: 6, + allowMixed: true, + validatePaths: true + } + })); + + const config = getSpecStructureConfig({ + structure: 'hierarchical', + maxDepth: 3, + allowMixed: false, + validatePaths: false, + }); + + expect(config.structure).toBe('hierarchical'); + expect(config.maxDepth).toBe(3); + expect(config.allowMixed).toBe(false); + expect(config.validatePaths).toBe(false); + }); + + it('should behave identically without project overrides (backward compat)', () => { + process.env.XDG_CONFIG_HOME = tempDir; + const configDir = path.join(tempDir, 'openspec'); + const configPath = path.join(configDir, 'config.json'); + + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(configPath, JSON.stringify({ + featureFlags: {}, + specStructure: { structure: 'flat', maxDepth: 5 } + })); + + const withUndefined = getSpecStructureConfig(undefined); + const withoutArg = getSpecStructureConfig(); + + expect(withUndefined).toEqual(withoutArg); + }); + + it('should preserve false boolean values from project overrides', () => { + process.env.XDG_CONFIG_HOME = tempDir; + const configDir = path.join(tempDir, 'openspec'); + const configPath = path.join(configDir, 'config.json'); + + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(configPath, JSON.stringify({ + featureFlags: {}, + specStructure: { allowMixed: true, validatePaths: true } + })); + + const config = getSpecStructureConfig({ allowMixed: false, validatePaths: false }); + + expect(config.allowMixed).toBe(false); + expect(config.validatePaths).toBe(false); + }); + + it('should let undefined project fields fall through to global', () => { + process.env.XDG_CONFIG_HOME = tempDir; + const configDir = path.join(tempDir, 'openspec'); + const configPath = path.join(configDir, 'config.json'); + + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(configPath, JSON.stringify({ + featureFlags: {}, + specStructure: { maxDepth: 6, allowMixed: false } + })); + + const config = getSpecStructureConfig({ structure: 'flat', maxDepth: undefined }); + + expect(config.structure).toBe('flat'); // from project + expect(config.maxDepth).toBe(6); // from global (undefined doesn't override) + expect(config.allowMixed).toBe(false); // from global + expect(config.validatePaths).toBe(true); // default + }); + + it('should merge partial project overrides with global values', () => { + process.env.XDG_CONFIG_HOME = tempDir; + const configDir = path.join(tempDir, 'openspec'); + const configPath = path.join(configDir, 'config.json'); + + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync(configPath, JSON.stringify({ + featureFlags: {}, + specStructure: { + structure: 'hierarchical', + maxDepth: 5, + allowMixed: false, + validatePaths: false + } + })); + + const config = getSpecStructureConfig({ maxDepth: 2, validatePaths: true }); + + expect(config.structure).toBe('hierarchical'); // from global + expect(config.maxDepth).toBe(2); // from project + expect(config.allowMixed).toBe(false); // from global + expect(config.validatePaths).toBe(true); // from project + }); + }); + }); }); diff --git a/test/core/list.test.ts b/test/core/list.test.ts index 5a678919a..94e456951 100644 --- a/test/core/list.test.ts +++ b/test/core/list.test.ts @@ -162,4 +162,199 @@ Regular text that should be ignored expect(logOutput.some(line => line.includes('no-tasks') && line.includes('No tasks'))).toBe(true); }); }); + + describe('execute - specs mode (flat structure)', () => { + it('should handle missing specs directory', async () => { + const listCommand = new ListCommand(); + await listCommand.execute(tempDir, 'specs'); + + expect(logOutput).toEqual(['No specs found.']); + }); + + it('should list flat structure specs', async () => { + const specsDir = path.join(tempDir, 'openspec', 'specs'); + + // Create flat structure specs + await fs.mkdir(path.join(specsDir, 'auth'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, 'auth', 'spec.md'), + '## Purpose\nAuth specification\n\n## Requirements\n\n### Requirement: User login\nUser SHALL be able to login\n\n### Requirement: Password reset\nUser SHALL be able to reset password\n' + ); + + await fs.mkdir(path.join(specsDir, 'payments'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, 'payments', 'spec.md'), + '## Purpose\nPayments specification\n\n## Requirements\n\n### Requirement: Process payment\nSystem SHALL process payments\n' + ); + + const listCommand = new ListCommand(); + await listCommand.execute(tempDir, 'specs'); + + expect(logOutput).toContain('Specs:'); + expect(logOutput.some(line => line.includes('auth') && line.includes('requirements 2'))).toBe(true); + expect(logOutput.some(line => line.includes('payments') && line.includes('requirements 1'))).toBe(true); + }); + + it('should sort flat specs alphabetically', async () => { + const specsDir = path.join(tempDir, 'openspec', 'specs'); + + await fs.mkdir(path.join(specsDir, 'zebra'), { recursive: true }); + await fs.writeFile(path.join(specsDir, 'zebra', 'spec.md'), '# Zebra'); + + await fs.mkdir(path.join(specsDir, 'alpha'), { recursive: true }); + await fs.writeFile(path.join(specsDir, 'alpha', 'spec.md'), '# Alpha'); + + await fs.mkdir(path.join(specsDir, 'middle'), { recursive: true }); + await fs.writeFile(path.join(specsDir, 'middle', 'spec.md'), '# Middle'); + + const listCommand = new ListCommand(); + await listCommand.execute(tempDir, 'specs'); + + const specLines = logOutput.filter(line => + line.includes('alpha') || line.includes('middle') || line.includes('zebra') + ); + + expect(specLines[0]).toContain('alpha'); + expect(specLines[1]).toContain('middle'); + expect(specLines[2]).toContain('zebra'); + }); + + it('should handle empty specs directory', async () => { + const specsDir = path.join(tempDir, 'openspec', 'specs'); + await fs.mkdir(specsDir, { recursive: true }); + + const listCommand = new ListCommand(); + await listCommand.execute(tempDir, 'specs'); + + expect(logOutput).toEqual(['No specs found.']); + }); + + it('should handle specs with zero requirements', async () => { + const specsDir = path.join(tempDir, 'openspec', 'specs'); + + await fs.mkdir(path.join(specsDir, 'empty-spec'), { recursive: true }); + await fs.writeFile(path.join(specsDir, 'empty-spec', 'spec.md'), '# Empty Spec\n\nNo requirements.'); + + const listCommand = new ListCommand(); + await listCommand.execute(tempDir, 'specs'); + + expect(logOutput.some(line => line.includes('empty-spec') && line.includes('requirements 0'))).toBe(true); + }); + }); + + describe('execute - specs mode (hierarchical structure)', () => { + it('should list hierarchical structure specs with indentation', async () => { + const specsDir = path.join(tempDir, 'openspec', 'specs'); + + // Create hierarchical structure (depth 2) + await fs.mkdir(path.join(specsDir, '_global', 'testing'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, '_global', 'testing', 'spec.md'), + '## Purpose\nTesting specification\n\n## Requirements\n\n### Requirement: Unit tests\nSystem SHALL have unit tests\n\n### Requirement: Integration tests\nSystem SHALL have integration tests\n' + ); + + await fs.mkdir(path.join(specsDir, '_global', 'architecture'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, '_global', 'architecture', 'spec.md'), + '## Purpose\nArchitecture specification\n\n## Requirements\n\n### Requirement: System design\nSystem SHALL have proper design\n' + ); + + await fs.mkdir(path.join(specsDir, 'packages', 'auth'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, 'packages', 'auth', 'spec.md'), + '## Purpose\nAuth package specification\n\n## Requirements\n\n### Requirement: OAuth\nSystem SHALL support OAuth\n\n### Requirement: JWT\nSystem SHALL support JWT\n\n### Requirement: Sessions\nSystem SHALL manage sessions\n' + ); + + const listCommand = new ListCommand(); + await listCommand.execute(tempDir, 'specs'); + + expect(logOutput).toContain('Specs:'); + + // Check for hierarchical display (leaf names only) + expect(logOutput.some(line => line.includes('architecture') && line.includes('requirements 1'))).toBe(true); + expect(logOutput.some(line => line.includes('testing') && line.includes('requirements 2'))).toBe(true); + expect(logOutput.some(line => line.includes('auth') && line.includes('requirements 3'))).toBe(true); + }); + + it('should handle depth 3 hierarchical specs', async () => { + const specsDir = path.join(tempDir, 'openspec', 'specs'); + + await fs.mkdir(path.join(specsDir, 'platform', 'services', 'api'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, 'platform', 'services', 'api', 'spec.md'), + '## Purpose\nAPI service specification\n\n## Requirements\n\n### Requirement: REST endpoints\nSystem SHALL provide REST endpoints\n\n### Requirement: GraphQL\nSystem SHALL support GraphQL\n' + ); + + await fs.mkdir(path.join(specsDir, 'platform', 'services', 'auth'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, 'platform', 'services', 'auth', 'spec.md'), + '## Purpose\nAuth service specification\n\n## Requirements\n\n### Requirement: Authentication\nSystem SHALL authenticate users\n' + ); + + const listCommand = new ListCommand(); + await listCommand.execute(tempDir, 'specs'); + + expect(logOutput.some(line => line.includes('api') && line.includes('requirements 2'))).toBe(true); + expect(logOutput.some(line => line.includes('auth') && line.includes('requirements 1'))).toBe(true); + }); + + it('should sort hierarchical specs alphabetically', async () => { + const specsDir = path.join(tempDir, 'openspec', 'specs'); + + await fs.mkdir(path.join(specsDir, 'zebra', 'zfeature'), { recursive: true }); + await fs.writeFile(path.join(specsDir, 'zebra', 'zfeature', 'spec.md'), '## Purpose\nZebra feature\n\n## Requirements\n\n### Requirement: Z\nZ SHALL work\n'); + + await fs.mkdir(path.join(specsDir, 'alpha', 'afeature'), { recursive: true }); + await fs.writeFile(path.join(specsDir, 'alpha', 'afeature', 'spec.md'), '## Purpose\nAlpha feature\n\n## Requirements\n\n### Requirement: A\nA SHALL work\n'); + + const listCommand = new ListCommand(); + await listCommand.execute(tempDir, 'specs'); + + // Since capabilities are sorted as "alpha/afeature" and "zebra/zfeature", + // alpha should appear before zebra in output + const outputStr = logOutput.join('\n'); + const alphaIdx = outputStr.indexOf('afeature'); + const zebraIdx = outputStr.indexOf('zfeature'); + + expect(alphaIdx).toBeGreaterThan(0); // Should exist + expect(zebraIdx).toBeGreaterThan(0); // Should exist + expect(alphaIdx).toBeLessThan(zebraIdx); // Alpha before zebra + }); + + it('should group hierarchical specs with blank lines between top-level groups', async () => { + const specsDir = path.join(tempDir, 'openspec', 'specs'); + + // Two different top-level groups + await fs.mkdir(path.join(specsDir, '_global', 'testing'), { recursive: true }); + await fs.writeFile(path.join(specsDir, '_global', 'testing', 'spec.md'), '# Testing'); + + await fs.mkdir(path.join(specsDir, 'packages', 'auth'), { recursive: true }); + await fs.writeFile(path.join(specsDir, 'packages', 'auth', 'spec.md'), '# Auth'); + + const listCommand = new ListCommand(); + await listCommand.execute(tempDir, 'specs'); + + // Should have blank line separating groups + expect(logOutput.some(line => line === '')).toBe(true); + }); + + it('should handle mixed flat and hierarchical specs', async () => { + const specsDir = path.join(tempDir, 'openspec', 'specs'); + + // Flat spec + await fs.mkdir(path.join(specsDir, 'auth'), { recursive: true }); + await fs.writeFile(path.join(specsDir, 'auth', 'spec.md'), '## Purpose\nAuth\n\n## Requirements\n\n### Requirement: Login\nSystem SHALL support login\n'); + + // Hierarchical spec + await fs.mkdir(path.join(specsDir, '_global', 'testing'), { recursive: true }); + await fs.writeFile(path.join(specsDir, '_global', 'testing', 'spec.md'), '## Purpose\nTesting\n\n## Requirements\n\n### Requirement: Unit tests\nSystem SHALL have unit tests\n'); + + const listCommand = new ListCommand(); + await listCommand.execute(tempDir, 'specs'); + + // Should detect as hierarchical and display both + expect(logOutput.some(line => line.includes('auth'))).toBe(true); + expect(logOutput.some(line => line.includes('testing'))).toBe(true); + }); + }); }); \ No newline at end of file diff --git a/test/core/parsers/change-parser.test.ts b/test/core/parsers/change-parser.test.ts index 595f138e3..984c59c5d 100644 --- a/test/core/parsers/change-parser.test.ts +++ b/test/core/parsers/change-parser.test.ts @@ -49,4 +49,130 @@ describe('ChangeParser', () => { expect(change.deltas[0].requirement).toBeDefined(); }); }); + + describe('hierarchical specs support', () => { + it('parses hierarchical delta specs at depth 2', async () => { + await withTempDir(async (dir) => { + const changeDir = dir; + const specsDir = path.join(changeDir, 'specs', '_global', 'testing'); + await fs.mkdir(specsDir, { recursive: true }); + + const content = `# Test Change\n\n## Why\nAdd testing standards.\n\n## What Changes\n- **_global/testing:** Add test requirements`; + const deltaSpec = `# Testing - Changes\n\n## ADDED Requirements\n\n### Requirement: Unit tests required\nAll modules SHALL have unit tests.\n\n#### Scenario: Test coverage\nGiven a module\nWhen tests run\nThen coverage is adequate`; + + await fs.writeFile(path.join(specsDir, 'spec.md'), deltaSpec, 'utf8'); + + const parser = new ChangeParser(content, changeDir); + const change = await parser.parseChangeWithDeltas('test-change'); + + expect(change.deltas.length).toBeGreaterThan(0); + expect(change.deltas[0].spec).toBe(path.join('_global', 'testing')); + expect(change.deltas[0].operation).toBe('ADDED'); + expect(change.deltas[0].requirement).toBeDefined(); + expect(change.deltas[0].requirement?.text).toContain('unit tests'); + }); + }); + + it('parses hierarchical delta specs at depth 3', async () => { + await withTempDir(async (dir) => { + const changeDir = dir; + const specsDir = path.join(changeDir, 'specs', 'platform', 'services', 'api'); + await fs.mkdir(specsDir, { recursive: true }); + + const content = `# Test Change\n\n## Why\nAdd API specifications.\n\n## What Changes\n- **platform/services/api:** Add REST endpoint requirements`; + const deltaSpec = `# API Service - Changes\n\n## ADDED Requirements\n\n### Requirement: REST endpoints\nAPI SHALL provide REST endpoints.\n\n#### Scenario: Endpoint access\nGiven an authenticated client\nWhen requesting an endpoint\nThen response is valid`; + + await fs.writeFile(path.join(specsDir, 'spec.md'), deltaSpec, 'utf8'); + + const parser = new ChangeParser(content, changeDir); + const change = await parser.parseChangeWithDeltas('test-change'); + + expect(change.deltas.length).toBeGreaterThan(0); + expect(change.deltas[0].spec).toBe(path.join('platform', 'services', 'api')); + expect(change.deltas[0].operation).toBe('ADDED'); + expect(change.deltas[0].requirement?.text).toContain('REST endpoints'); + }); + }); + + it('parses mixed flat and hierarchical delta specs', async () => { + await withTempDir(async (dir) => { + const changeDir = dir; + + // Create flat delta spec + const flatSpecsDir = path.join(changeDir, 'specs', 'auth'); + await fs.mkdir(flatSpecsDir, { recursive: true }); + const flatDeltaContent = `# Auth - Changes\n\n## ADDED Requirements\n\n### Requirement: Login\nUser SHALL be able to login.\n\n#### Scenario: Login flow\nGiven valid credentials\nWhen user logs in\nThen access is granted`; + await fs.writeFile(path.join(flatSpecsDir, 'spec.md'), flatDeltaContent, 'utf8'); + + // Create hierarchical delta spec + const hierarchicalSpecsDir = path.join(changeDir, 'specs', '_global', 'security'); + await fs.mkdir(hierarchicalSpecsDir, { recursive: true }); + const hierarchicalDeltaContent = `# Security - Changes\n\n## ADDED Requirements\n\n### Requirement: Encryption\nData SHALL be encrypted.\n\n#### Scenario: Data security\nGiven sensitive data\nWhen stored\nThen it is encrypted`; + await fs.writeFile(path.join(hierarchicalSpecsDir, 'spec.md'), hierarchicalDeltaContent, 'utf8'); + + const content = `# Test Change\n\n## Why\nAdd auth and security.\n\n## What Changes\n- **auth:** Add login\n- **_global/security:** Add encryption`; + + const parser = new ChangeParser(content, changeDir); + const change = await parser.parseChangeWithDeltas('test-change'); + + expect(change.deltas.length).toBe(2); + + // Should have both flat and hierarchical deltas + const flatDelta = change.deltas.find(d => d.spec === 'auth'); + const hierarchicalDelta = change.deltas.find(d => d.spec === path.join('_global', 'security')); + + expect(flatDelta).toBeDefined(); + expect(flatDelta?.operation).toBe('ADDED'); + expect(flatDelta?.requirement?.text).toContain('login'); + + expect(hierarchicalDelta).toBeDefined(); + expect(hierarchicalDelta?.operation).toBe('ADDED'); + expect(hierarchicalDelta?.requirement?.text).toContain('encrypted'); + }); + }); + + it('parses hierarchical delta specs with MODIFIED operations', async () => { + await withTempDir(async (dir) => { + const changeDir = dir; + const specsDir = path.join(changeDir, 'specs', '_global', 'monitoring'); + await fs.mkdir(specsDir, { recursive: true }); + + const content = `# Test Change\n\n## Why\nUpdate monitoring.\n\n## What Changes\n- **_global/monitoring:** Update alerting requirements`; + const deltaSpec = `# Monitoring - Changes\n\n## MODIFIED Requirements\n\n### Requirement: Alerting\nSystem SHALL send alerts with additional context.\n\n#### Scenario: Alert delivery\nGiven an error condition\nWhen alert triggers\nThen context is included`; + + await fs.writeFile(path.join(specsDir, 'spec.md'), deltaSpec, 'utf8'); + + const parser = new ChangeParser(content, changeDir); + const change = await parser.parseChangeWithDeltas('test-change'); + + expect(change.deltas.length).toBeGreaterThan(0); + expect(change.deltas[0].spec).toBe(path.join('_global', 'monitoring')); + expect(change.deltas[0].operation).toBe('MODIFIED'); + expect(change.deltas[0].requirement?.text).toContain('alerts'); + }); + }); + + it('parses hierarchical delta specs with RENAMED operations', async () => { + await withTempDir(async (dir) => { + const changeDir = dir; + const specsDir = path.join(changeDir, 'specs', 'platform', 'logging'); + await fs.mkdir(specsDir, { recursive: true }); + + const content = `# Test Change\n\n## Why\nRename logging requirement.\n\n## What Changes\n- **platform/logging:** Rename requirement`; + const deltaSpec = `# Logging - Changes\n\n## RENAMED Requirements\n- FROM: \`### Requirement: Old Name\`\n- TO: \`### Requirement: New Name\``; + + await fs.writeFile(path.join(specsDir, 'spec.md'), deltaSpec, 'utf8'); + + const parser = new ChangeParser(content, changeDir); + const change = await parser.parseChangeWithDeltas('test-change'); + + expect(change.deltas.length).toBeGreaterThan(0); + expect(change.deltas[0].spec).toBe(path.join('platform', 'logging')); + expect(change.deltas[0].operation).toBe('RENAMED'); + expect(change.deltas[0].rename).toBeDefined(); + expect(change.deltas[0].rename?.from).toBe('Old Name'); + expect(change.deltas[0].rename?.to).toBe('New Name'); + }); + }); + }); }); diff --git a/test/core/project-config.test.ts b/test/core/project-config.test.ts index 88944659d..6a01f096c 100644 --- a/test/core/project-config.test.ts +++ b/test/core/project-config.test.ts @@ -480,6 +480,233 @@ rules: ]); }); }); + + describe('specStructure parsing', () => { + it('should parse valid complete specStructure', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +specStructure: + structure: hierarchical + maxDepth: 3 + allowMixed: false + validatePaths: true +` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.specStructure).toEqual({ + structure: 'hierarchical', + maxDepth: 3, + allowMixed: false, + validatePaths: true, + }); + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('should parse partial specStructure (only some fields)', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +specStructure: + structure: flat +` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.specStructure).toEqual({ + structure: 'flat', + }); + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('should keep valid sub-fields and warn about invalid ones', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +specStructure: + structure: flat + maxDepth: "invalid" + validatePaths: false +` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.specStructure).toEqual({ + structure: 'flat', + validatePaths: false, + }); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Invalid 'specStructure.maxDepth'") + ); + }); + + it('should warn about invalid structure enum value', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +specStructure: + structure: nested + maxDepth: 3 +` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.specStructure).toEqual({ + maxDepth: 3, + }); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Invalid 'specStructure.structure'") + ); + }); + + it('should warn about invalid boolean sub-fields', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +specStructure: + allowMixed: "yes" + validatePaths: 42 +` + ); + + const config = readProjectConfig(tempDir); + + // No valid fields, so specStructure should not be set + expect(config?.specStructure).toBeUndefined(); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Invalid 'specStructure.allowMixed'") + ); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Invalid 'specStructure.validatePaths'") + ); + }); + + it('should return undefined specStructure when field is absent', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.specStructure).toBeUndefined(); + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('should warn when specStructure is not an object', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +specStructure: 42 +` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.specStructure).toBeUndefined(); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Invalid 'specStructure' field in config (must be object)") + ); + }); + + it('should ignore unknown sub-fields without warning', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +specStructure: + structure: flat + unknownField: true + anotherUnknown: 42 +` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.specStructure).toEqual({ + structure: 'flat', + }); + expect(consoleWarnSpy).not.toHaveBeenCalled(); + }); + + it('should handle specStructure: null without aborting config parsing', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +specStructure: +` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.schema).toBe('spec-driven'); + expect(config?.specStructure).toBeUndefined(); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Invalid 'specStructure' field in config (must be object)") + ); + }); + + it('should reject maxDepth below valid range', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +specStructure: + maxDepth: 0 +` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.specStructure).toBeUndefined(); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Invalid 'specStructure.maxDepth'") + ); + }); + + it('should reject maxDepth above valid range', () => { + const configDir = path.join(tempDir, 'openspec'); + fs.mkdirSync(configDir, { recursive: true }); + fs.writeFileSync( + path.join(configDir, 'config.yaml'), + `schema: spec-driven +specStructure: + maxDepth: 11 +` + ); + + const config = readProjectConfig(tempDir); + + expect(config?.specStructure).toBeUndefined(); + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining("Invalid 'specStructure.maxDepth'") + ); + }); + }); }); describe('validateConfigRules', () => { diff --git a/test/core/view.test.ts b/test/core/view.test.ts index b8b56df1e..a5ce42408 100644 --- a/test/core/view.test.ts +++ b/test/core/view.test.ts @@ -125,5 +125,135 @@ describe('ViewCommand', () => { 'gamma-change' ]); }); + + describe('hierarchical specs', () => { + it('should display flat structure specs correctly', async () => { + const specsDir = path.join(tempDir, 'openspec', 'specs'); + + // Create flat structure specs + await fs.mkdir(path.join(specsDir, 'auth'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, 'auth', 'spec.md'), + '## Purpose\nAuth spec\n\n## Requirements\n\n### Requirement: Login\nSystem SHALL support login\n' + ); + + await fs.mkdir(path.join(specsDir, 'payments'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, 'payments', 'spec.md'), + '## Purpose\nPayments spec\n\n## Requirements\n\n### Requirement: Process\nSystem SHALL process payments\n' + ); + + const viewCommand = new ViewCommand(); + await viewCommand.execute(tempDir); + + const output = logOutput.map(stripAnsi).join('\n'); + + expect(output).toContain('Specifications'); + expect(output).toContain('auth'); + expect(output).toContain('payments'); + expect(output).toContain('1 requirement'); // Both specs have 1 requirement + }); + + it('should display hierarchical structure specs with indentation', async () => { + const specsDir = path.join(tempDir, 'openspec', 'specs'); + + // Create hierarchical structure + await fs.mkdir(path.join(specsDir, '_global', 'testing'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, '_global', 'testing', 'spec.md'), + '## Purpose\nTesting spec\n\n## Requirements\n\n### Requirement: Unit tests\nSystem SHALL have unit tests\n\n### Requirement: Integration tests\nSystem SHALL have integration tests\n' + ); + + await fs.mkdir(path.join(specsDir, 'packages', 'auth'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, 'packages', 'auth', 'spec.md'), + '## Purpose\nAuth package\n\n## Requirements\n\n### Requirement: OAuth\nSystem SHALL support OAuth\n' + ); + + const viewCommand = new ViewCommand(); + await viewCommand.execute(tempDir); + + const output = logOutput.map(stripAnsi).join('\n'); + + expect(output).toContain('Specifications'); + // Check for leaf names (testing, auth) + expect(output).toContain('testing'); + expect(output).toContain('auth'); + expect(output).toContain('2 requirements'); // testing has 2 + expect(output).toContain('1 requirement'); // auth has 1 + }); + + it('should display mixed flat and hierarchical specs', async () => { + const specsDir = path.join(tempDir, 'openspec', 'specs'); + + // Create specs with different requirement counts + await fs.mkdir(path.join(specsDir, '_global', 'testing'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, '_global', 'testing', 'spec.md'), + '## Purpose\nTesting\n\n## Requirements\n\n### Requirement: R1\nR1\n\n### Requirement: R2\nR2\n\n### Requirement: R3\nR3\n' + ); + + await fs.mkdir(path.join(specsDir, 'auth'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, 'auth', 'spec.md'), + '## Purpose\nAuth\n\n## Requirements\n\n### Requirement: R1\nR1\n' + ); + + const viewCommand = new ViewCommand(); + await viewCommand.execute(tempDir); + + const output = logOutput.map(stripAnsi).join('\n'); + + // Hierarchical specs are displayed in discovery order (grouped by domain), not by requirement count + expect(output).toContain('testing'); + expect(output).toContain('auth'); + }); + + it('should handle mixed flat and hierarchical specs', async () => { + const specsDir = path.join(tempDir, 'openspec', 'specs'); + + // Flat spec + await fs.mkdir(path.join(specsDir, 'auth'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, 'auth', 'spec.md'), + '## Purpose\nAuth\n\n## Requirements\n\n### Requirement: Login\nLogin\n' + ); + + // Hierarchical spec + await fs.mkdir(path.join(specsDir, '_global', 'testing'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, '_global', 'testing', 'spec.md'), + '## Purpose\nTesting\n\n## Requirements\n\n### Requirement: Tests\nTests\n' + ); + + const viewCommand = new ViewCommand(); + await viewCommand.execute(tempDir); + + const output = logOutput.map(stripAnsi).join('\n'); + + // Should display both specs + expect(output).toContain('Specifications'); + expect(output).toContain('auth'); + expect(output).toContain('testing'); + }); + + it('should handle specs with no requirements', async () => { + const specsDir = path.join(tempDir, 'openspec', 'specs'); + + await fs.mkdir(path.join(specsDir, '_global', 'empty'), { recursive: true }); + await fs.writeFile( + path.join(specsDir, '_global', 'empty', 'spec.md'), + '## Purpose\nEmpty spec\n\n## Requirements\n' + ); + + const viewCommand = new ViewCommand(); + await viewCommand.execute(tempDir); + + const output = logOutput.map(stripAnsi).join('\n'); + + expect(output).toContain('empty'); + expect(output).toContain('0 requirements'); + }); + }); }); diff --git a/test/utils/spec-discovery.compatibility.test.ts b/test/utils/spec-discovery.compatibility.test.ts new file mode 100644 index 000000000..380110a1f --- /dev/null +++ b/test/utils/spec-discovery.compatibility.test.ts @@ -0,0 +1,188 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { findAllSpecs, isSpecStructureHierarchical } from '../../src/utils/spec-discovery.js'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; + +describe('Backward Compatibility Tests', () => { + let tempDir: string; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-compat-')); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + describe('Flat structure compatibility', () => { + it('should work with existing flat structure projects', () => { + // Simulate an existing flat structure project + const capabilities = ['auth', 'api', 'database', 'payments', 'notifications']; + + for (const cap of capabilities) { + const specDir = path.join(tempDir, cap); + fs.mkdirSync(specDir, { recursive: true }); + fs.writeFileSync(path.join(specDir, 'spec.md'), `# ${cap} Specification`); + } + + const specs = findAllSpecs(tempDir); + + // Should find all specs + expect(specs.length).toBe(5); + + // All should be depth 1 (flat) + specs.forEach(spec => { + expect(spec.depth).toBe(1); + }); + + // Capabilities should match directory names + const capabilityNames = specs.map(s => s.capability).sort(); + expect(capabilityNames).toEqual(capabilities.sort()); + + // Should detect as flat structure + expect(isSpecStructureHierarchical(tempDir)).toBe(false); + }); + + it('should maintain capability names without path separators for flat structure', () => { + const specDir = path.join(tempDir, 'authentication'); + fs.mkdirSync(specDir, { recursive: true }); + fs.writeFileSync(path.join(specDir, 'spec.md'), '# Auth Spec'); + + const specs = findAllSpecs(tempDir); + + expect(specs.length).toBe(1); + expect(specs[0].capability).toBe('authentication'); + expect(specs[0].capability).not.toContain(path.sep); + expect(specs[0].capability).not.toContain('/'); + expect(specs[0].capability).not.toContain('\\'); + }); + + it('should work with specs that have additional files in directories', () => { + // Flat structure with extra files (README, diagrams, etc.) + const specDir = path.join(tempDir, 'api'); + fs.mkdirSync(specDir, { recursive: true }); + fs.writeFileSync(path.join(specDir, 'spec.md'), '# API Spec'); + fs.writeFileSync(path.join(specDir, 'README.md'), '# API Documentation'); + fs.writeFileSync(path.join(specDir, 'diagram.png'), 'fake image'); + + const specs = findAllSpecs(tempDir); + + expect(specs.length).toBe(1); + expect(specs[0].capability).toBe('api'); + }); + }); + + describe('Auto-detection with mixed structures', () => { + it('should detect hierarchical structure when any spec has depth > 1', () => { + // Mix of flat and hierarchical + fs.mkdirSync(path.join(tempDir, 'auth'), { recursive: true }); + fs.writeFileSync(path.join(tempDir, 'auth', 'spec.md'), '# Auth Spec'); + + fs.mkdirSync(path.join(tempDir, '_global', 'testing'), { recursive: true }); + fs.writeFileSync(path.join(tempDir, '_global', 'testing', 'spec.md'), '# Testing Spec'); + + expect(isSpecStructureHierarchical(tempDir)).toBe(true); + }); + + it('should detect flat structure when all specs are depth 1', () => { + fs.mkdirSync(path.join(tempDir, 'auth'), { recursive: true }); + fs.writeFileSync(path.join(tempDir, 'auth', 'spec.md'), '# Auth Spec'); + + fs.mkdirSync(path.join(tempDir, 'api'), { recursive: true }); + fs.writeFileSync(path.join(tempDir, 'api', 'spec.md'), '# API Spec'); + + expect(isSpecStructureHierarchical(tempDir)).toBe(false); + }); + + it('should correctly discover mixed flat and hierarchical specs', () => { + // Flat specs + fs.mkdirSync(path.join(tempDir, 'auth'), { recursive: true }); + fs.writeFileSync(path.join(tempDir, 'auth', 'spec.md'), '# Auth Spec'); + + fs.mkdirSync(path.join(tempDir, 'payments'), { recursive: true }); + fs.writeFileSync(path.join(tempDir, 'payments', 'spec.md'), '# Payments Spec'); + + // Hierarchical specs + fs.mkdirSync(path.join(tempDir, '_global', 'security'), { recursive: true }); + fs.writeFileSync(path.join(tempDir, '_global', 'security', 'spec.md'), '# Security Spec'); + + fs.mkdirSync(path.join(tempDir, 'platform', 'api'), { recursive: true }); + fs.writeFileSync(path.join(tempDir, 'platform', 'api', 'spec.md'), '# Platform API Spec'); + + const specs = findAllSpecs(tempDir); + + expect(specs.length).toBe(4); + + // Verify flat specs + const flatSpecs = specs.filter(s => s.depth === 1); + expect(flatSpecs.length).toBe(2); + expect(flatSpecs.map(s => s.capability).sort()).toEqual(['auth', 'payments']); + + // Verify hierarchical specs + const hierarchicalSpecs = specs.filter(s => s.depth > 1); + expect(hierarchicalSpecs.length).toBe(2); + + const globalSecurity = hierarchicalSpecs.find(s => s.capability.includes('security')); + expect(globalSecurity).toBeDefined(); + expect(globalSecurity!.capability).toBe(path.join('_global', 'security')); + + const platformApi = hierarchicalSpecs.find(s => s.capability.includes('api')); + expect(platformApi).toBeDefined(); + expect(platformApi!.capability).toBe(path.join('platform', 'api')); + }); + + it('should handle edge case of underscore-prefixed flat specs', () => { + // Edge case: flat spec with underscore prefix (not hierarchical) + fs.mkdirSync(path.join(tempDir, '_internal'), { recursive: true }); + fs.writeFileSync(path.join(tempDir, '_internal', 'spec.md'), '# Internal Spec'); + + const specs = findAllSpecs(tempDir); + + expect(specs.length).toBe(1); + expect(specs[0].capability).toBe('_internal'); + expect(specs[0].depth).toBe(1); + + // Should be detected as flat (no nested specs) + expect(isSpecStructureHierarchical(tempDir)).toBe(false); + }); + }); + + describe('Integration with existing codebases', () => { + it('should not break existing flat structure workflows', () => { + // Simulate typical flat structure from existing projects + const capabilities = [ + 'authentication', + 'authorization', + 'user-management', + 'api-gateway', + 'notification-service', + 'payment-processing', + ]; + + for (const cap of capabilities) { + const specDir = path.join(tempDir, cap); + fs.mkdirSync(specDir, { recursive: true }); + fs.writeFileSync( + path.join(specDir, 'spec.md'), + `# ${cap} Specification\n\n## Purpose\n\nThis is the ${cap} specification.\n\n## Requirements\n\n### Requirement: System SHALL ${cap}\n\n#### Scenario: Basic usage\n\n- **WHEN** user performs action\n- **THEN** system responds correctly` + ); + } + + const specs = findAllSpecs(tempDir); + + // All specs found + expect(specs.length).toBe(6); + + // All flat (depth 1) + expect(specs.every(s => s.depth === 1)).toBe(true); + + // Structure detected as flat + expect(isSpecStructureHierarchical(tempDir)).toBe(false); + + // Capabilities match exactly (no path separators added) + const foundCapabilities = specs.map(s => s.capability).sort(); + expect(foundCapabilities).toEqual(capabilities.sort()); + }); + }); +}); diff --git a/test/utils/spec-discovery.performance.test.ts b/test/utils/spec-discovery.performance.test.ts new file mode 100644 index 000000000..ef84b55e5 --- /dev/null +++ b/test/utils/spec-discovery.performance.test.ts @@ -0,0 +1,143 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { findAllSpecs } from '../../src/utils/spec-discovery.js'; +import * as fs from 'fs'; +import * as path from 'path'; +import * as os from 'os'; + +describe('Spec Discovery Performance Benchmarks', () => { + let tempDir: string; + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'openspec-perf-')); + }); + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }); + }); + + it('should handle 100 flat specs efficiently', () => { + // Create 100 flat specs + for (let i = 0; i < 100; i++) { + const specDir = path.join(tempDir, `capability-${i}`); + fs.mkdirSync(specDir, { recursive: true }); + fs.writeFileSync(path.join(specDir, 'spec.md'), `# Spec ${i}`); + } + + const startTime = performance.now(); + const specs = findAllSpecs(tempDir); + const endTime = performance.now(); + const duration = endTime - startTime; + + expect(specs.length).toBe(100); + expect(duration).toBeLessThan(500); // Should complete in < 500ms + console.log(`100 flat specs discovered in ${duration.toFixed(2)}ms`); + }); + + it('should handle 100 hierarchical specs efficiently', () => { + // Create 100 hierarchical specs (depth 2-3) + for (let i = 0; i < 50; i++) { + const specDir = path.join(tempDir, '_global', `capability-${i}`); + fs.mkdirSync(specDir, { recursive: true }); + fs.writeFileSync(path.join(specDir, 'spec.md'), `# Global Spec ${i}`); + } + + for (let i = 0; i < 50; i++) { + const specDir = path.join(tempDir, 'platform', 'services', `service-${i}`); + fs.mkdirSync(specDir, { recursive: true }); + fs.writeFileSync(path.join(specDir, 'spec.md'), `# Service Spec ${i}`); + } + + const startTime = performance.now(); + const specs = findAllSpecs(tempDir); + const endTime = performance.now(); + const duration = endTime - startTime; + + expect(specs.length).toBe(100); + expect(duration).toBeLessThan(500); // Should complete in < 500ms + console.log(`100 hierarchical specs discovered in ${duration.toFixed(2)}ms`); + }); + + it('should handle 1000 flat specs efficiently', () => { + // Create 1000 flat specs + for (let i = 0; i < 1000; i++) { + const specDir = path.join(tempDir, `capability-${i}`); + fs.mkdirSync(specDir, { recursive: true }); + fs.writeFileSync(path.join(specDir, 'spec.md'), `# Spec ${i}`); + } + + const startTime = performance.now(); + const specs = findAllSpecs(tempDir); + const endTime = performance.now(); + const duration = endTime - startTime; + + expect(specs.length).toBe(1000); + expect(duration).toBeLessThan(3000); // Should complete in < 3 seconds + console.log(`1000 flat specs discovered in ${duration.toFixed(2)}ms`); + }); + + it('should handle 1000 hierarchical specs efficiently', () => { + // Create 1000 hierarchical specs distributed across domains + const domains = ['_global', 'frontend', 'backend', 'platform', 'services']; + const categories = ['auth', 'api', 'database', 'cache', 'messaging']; + + let count = 0; + for (const domain of domains) { + for (const category of categories) { + for (let i = 0; i < 40 && count < 1000; i++) { + const specDir = path.join(tempDir, domain, category, `spec-${i}`); + fs.mkdirSync(specDir, { recursive: true }); + fs.writeFileSync(path.join(specDir, 'spec.md'), `# ${domain}/${category} Spec ${i}`); + count++; + } + } + } + + const startTime = performance.now(); + const specs = findAllSpecs(tempDir); + const endTime = performance.now(); + const duration = endTime - startTime; + + expect(specs.length).toBe(1000); + expect(duration).toBeLessThan(3000); // Should complete in < 3 seconds + console.log(`1000 hierarchical specs discovered in ${duration.toFixed(2)}ms`); + }); + + it('should handle mixed flat and hierarchical specs (500 each)', () => { + // Create 500 flat specs + for (let i = 0; i < 500; i++) { + const specDir = path.join(tempDir, `flat-${i}`); + fs.mkdirSync(specDir, { recursive: true }); + fs.writeFileSync(path.join(specDir, 'spec.md'), `# Flat Spec ${i}`); + } + + // Create 500 hierarchical specs + for (let i = 0; i < 250; i++) { + const specDir = path.join(tempDir, '_global', `hierarchical-${i}`); + fs.mkdirSync(specDir, { recursive: true }); + fs.writeFileSync(path.join(specDir, 'spec.md'), `# Hierarchical Spec ${i}`); + } + + for (let i = 0; i < 250; i++) { + const specDir = path.join(tempDir, 'platform', 'services', `service-${i}`); + fs.mkdirSync(specDir, { recursive: true }); + fs.writeFileSync(path.join(specDir, 'spec.md'), `# Service Spec ${i}`); + } + + const startTime = performance.now(); + const specs = findAllSpecs(tempDir); + const endTime = performance.now(); + const duration = endTime - startTime; + + expect(specs.length).toBe(1000); + + // Verify we have both flat and hierarchical + const flatSpecs = specs.filter(s => s.depth === 1); + const hierarchicalSpecs = specs.filter(s => s.depth > 1); + + expect(flatSpecs.length).toBe(500); + expect(hierarchicalSpecs.length).toBe(500); + expect(duration).toBeLessThan(3000); // Should complete in < 3 seconds + + console.log(`1000 mixed specs (500 flat, 500 hierarchical) discovered in ${duration.toFixed(2)}ms`); + }); +}); diff --git a/test/utils/spec-discovery.test.ts b/test/utils/spec-discovery.test.ts new file mode 100644 index 000000000..2fd1fa339 --- /dev/null +++ b/test/utils/spec-discovery.test.ts @@ -0,0 +1,1188 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { findAllSpecs, isSpecStructureHierarchical, findSpecUpdates, validateSpecStructure, type DiscoveredSpec } from '../../src/utils/spec-discovery.js'; +import * as fs from 'fs'; +import * as path from 'path'; +import { fileURLToPath } from 'url'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); + +const fixturesDir = path.join(__dirname, '..', 'fixtures', 'spec-discovery'); + +describe('spec-discovery', () => { + describe('findAllSpecs() - flat structure', () => { + let flatFixtureDir: string; + + beforeEach(() => { + // Create flat structure fixture + flatFixtureDir = path.join(fixturesDir, 'flat-structure'); + fs.mkdirSync(path.join(flatFixtureDir, 'auth'), { recursive: true }); + fs.mkdirSync(path.join(flatFixtureDir, 'payments'), { recursive: true }); + fs.mkdirSync(path.join(flatFixtureDir, 'notifications'), { recursive: true }); + + fs.writeFileSync(path.join(flatFixtureDir, 'auth', 'spec.md'), '# Auth Spec'); + fs.writeFileSync(path.join(flatFixtureDir, 'payments', 'spec.md'), '# Payments Spec'); + fs.writeFileSync(path.join(flatFixtureDir, 'notifications', 'spec.md'), '# Notifications Spec'); + }); + + afterEach(() => { + // Clean up fixtures + if (fs.existsSync(flatFixtureDir)) { + fs.rmSync(flatFixtureDir, { recursive: true, force: true }); + } + }); + + it('should find all specs in flat structure', () => { + const specs = findAllSpecs(flatFixtureDir); + + expect(specs).toHaveLength(3); + expect(specs.map(s => s.capability).sort()).toEqual(['auth', 'notifications', 'payments']); + }); + + it('should set depth to 1 for flat structure specs', () => { + const specs = findAllSpecs(flatFixtureDir); + + specs.forEach(spec => { + expect(spec.depth).toBe(1); + }); + }); + + it('should return absolute paths to spec.md files', () => { + const specs = findAllSpecs(flatFixtureDir); + + specs.forEach(spec => { + expect(path.isAbsolute(spec.path)).toBe(true); + expect(spec.path.endsWith('spec.md')).toBe(true); + expect(fs.existsSync(spec.path)).toBe(true); + }); + }); + + it('should handle empty specs directory', () => { + const emptyDir = path.join(fixturesDir, 'empty'); + fs.mkdirSync(emptyDir, { recursive: true }); + + const specs = findAllSpecs(emptyDir); + + expect(specs).toHaveLength(0); + + fs.rmSync(emptyDir, { recursive: true, force: true }); + }); + + it('should handle non-existent directory gracefully', () => { + const nonExistent = path.join(fixturesDir, 'does-not-exist'); + + const specs = findAllSpecs(nonExistent); + + expect(specs).toHaveLength(0); + }); + + it('should skip spec.md found directly in baseDir (no valid capability)', () => { + // A spec.md at the root of the specs directory has no capability name + fs.writeFileSync(path.join(flatFixtureDir, 'spec.md'), '# Root Spec'); + + const specs = findAllSpecs(flatFixtureDir); + + // Should only find the 3 specs in subdirectories, not the root one + expect(specs).toHaveLength(3); + expect(specs.map(s => s.capability)).not.toContain(''); + }); + + it('should ignore directories without spec.md', () => { + const dirWithoutSpec = path.join(flatFixtureDir, 'no-spec'); + fs.mkdirSync(dirWithoutSpec, { recursive: true }); + fs.writeFileSync(path.join(dirWithoutSpec, 'readme.md'), '# README'); + + const specs = findAllSpecs(flatFixtureDir); + + expect(specs).toHaveLength(3); // Only the 3 with spec.md + expect(specs.map(s => s.capability)).not.toContain('no-spec'); + }); + }); + + describe('findAllSpecs() - hierarchical structure', () => { + let hierarchicalFixtureDir: string; + + beforeEach(() => { + // Create hierarchical structure fixture (depth 2-4) + hierarchicalFixtureDir = path.join(fixturesDir, 'hierarchical-structure'); + + // Depth 2: _global/testing + fs.mkdirSync(path.join(hierarchicalFixtureDir, '_global', 'testing'), { recursive: true }); + fs.writeFileSync(path.join(hierarchicalFixtureDir, '_global', 'testing', 'spec.md'), '# Testing Spec'); + + // Depth 2: _global/architecture + fs.mkdirSync(path.join(hierarchicalFixtureDir, '_global', 'architecture'), { recursive: true }); + fs.writeFileSync(path.join(hierarchicalFixtureDir, '_global', 'architecture', 'spec.md'), '# Architecture Spec'); + + // Depth 2: dev/mcp-server + fs.mkdirSync(path.join(hierarchicalFixtureDir, 'dev', 'mcp-server'), { recursive: true }); + fs.writeFileSync(path.join(hierarchicalFixtureDir, 'dev', 'mcp-server', 'spec.md'), '# MCP Server Spec'); + + // Depth 3: packages/auth/oauth + fs.mkdirSync(path.join(hierarchicalFixtureDir, 'packages', 'auth', 'oauth'), { recursive: true }); + fs.writeFileSync(path.join(hierarchicalFixtureDir, 'packages', 'auth', 'oauth', 'spec.md'), '# OAuth Spec'); + + // Depth 4: platform/services/api/rest + fs.mkdirSync(path.join(hierarchicalFixtureDir, 'platform', 'services', 'api', 'rest'), { recursive: true }); + fs.writeFileSync(path.join(hierarchicalFixtureDir, 'platform', 'services', 'api', 'rest', 'spec.md'), '# REST API Spec'); + }); + + afterEach(() => { + if (fs.existsSync(hierarchicalFixtureDir)) { + fs.rmSync(hierarchicalFixtureDir, { recursive: true, force: true }); + } + }); + + it('should find all specs in hierarchical structure', () => { + const specs = findAllSpecs(hierarchicalFixtureDir); + + expect(specs).toHaveLength(5); + }); + + it('should construct capability names from relative paths', () => { + const specs = findAllSpecs(hierarchicalFixtureDir); + const capabilities = specs.map(s => s.capability).sort(); + + expect(capabilities).toContain(path.join('_global', 'testing')); + expect(capabilities).toContain(path.join('_global', 'architecture')); + expect(capabilities).toContain(path.join('dev', 'mcp-server')); + expect(capabilities).toContain(path.join('packages', 'auth', 'oauth')); + expect(capabilities).toContain(path.join('platform', 'services', 'api', 'rest')); + }); + + it('should correctly calculate depth for hierarchical specs', () => { + const specs = findAllSpecs(hierarchicalFixtureDir); + + const depth2Specs = specs.filter(s => s.depth === 2); + const depth3Specs = specs.filter(s => s.depth === 3); + const depth4Specs = specs.filter(s => s.depth === 4); + + expect(depth2Specs).toHaveLength(3); // _global/testing, _global/architecture, dev/mcp-server + expect(depth3Specs).toHaveLength(1); // packages/auth/oauth + expect(depth4Specs).toHaveLength(1); // platform/services/api/rest + }); + + it('should handle mixed depth hierarchies', () => { + const specs = findAllSpecs(hierarchicalFixtureDir); + + const depths = specs.map(s => s.depth).sort(); + expect(Math.min(...depths)).toBe(2); + expect(Math.max(...depths)).toBe(4); + }); + + it('should return absolute paths with correct hierarchy', () => { + const specs = findAllSpecs(hierarchicalFixtureDir); + + specs.forEach(spec => { + expect(path.isAbsolute(spec.path)).toBe(true); + expect(spec.path.endsWith('spec.md')).toBe(true); + expect(fs.existsSync(spec.path)).toBe(true); + + // Verify path contains the capability structure + const normalizedPath = spec.path.split(path.sep).join('/'); + const normalizedCapability = spec.capability.split(path.sep).join('/'); + expect(normalizedPath).toContain(normalizedCapability); + }); + }); + + it('should find all specs including intermediate levels (validation happens separately)', () => { + // Add a spec.md at an intermediate directory level + fs.writeFileSync(path.join(hierarchicalFixtureDir, '_global', 'spec.md'), '# Intermediate Spec'); + + const specs = findAllSpecs(hierarchicalFixtureDir); + + // findAllSpecs discovers all spec.md files; orphan detection is done by validateSpecStructure() + expect(specs).toHaveLength(6); + expect(specs.map(s => s.capability)).toContain('_global'); + }); + }); + + describe('findAllSpecs() - cross-platform path handling', () => { + let crossPlatformFixtureDir: string; + + beforeEach(() => { + crossPlatformFixtureDir = path.join(fixturesDir, 'cross-platform'); + + // Create a hierarchical structure that will test path.sep handling + fs.mkdirSync(path.join(crossPlatformFixtureDir, '_global', 'testing'), { recursive: true }); + fs.writeFileSync(path.join(crossPlatformFixtureDir, '_global', 'testing', 'spec.md'), '# Testing Spec'); + + fs.mkdirSync(path.join(crossPlatformFixtureDir, 'packages', 'auth'), { recursive: true }); + fs.writeFileSync(path.join(crossPlatformFixtureDir, 'packages', 'auth', 'spec.md'), '# Auth Spec'); + }); + + afterEach(() => { + if (fs.existsSync(crossPlatformFixtureDir)) { + fs.rmSync(crossPlatformFixtureDir, { recursive: true, force: true }); + } + }); + + it('should use platform-specific path separators in capability names', () => { + const specs = findAllSpecs(crossPlatformFixtureDir); + + specs.forEach(spec => { + if (spec.depth > 1) { + // Hierarchical specs should use path.sep (\ on Windows, / on Unix) + expect(spec.capability).toContain(path.sep); + } + }); + }); + + it('should construct paths using path.join for cross-platform compatibility', () => { + const specs = findAllSpecs(crossPlatformFixtureDir); + + // Verify that paths are constructed correctly on current platform + const globalTestingSpec = specs.find(s => s.capability.endsWith('testing')); + expect(globalTestingSpec).toBeDefined(); + + // Path should be valid on current platform + expect(fs.existsSync(globalTestingSpec!.path)).toBe(true); + + // Verify path structure matches platform conventions + const expectedPath = path.join(crossPlatformFixtureDir, '_global', 'testing', 'spec.md'); + expect(globalTestingSpec!.path).toBe(expectedPath); + }); + + it('should handle capability names consistently across platforms', () => { + const specs = findAllSpecs(crossPlatformFixtureDir); + + // Capability names should use path.sep consistently + const globalSpec = specs.find(s => s.capability.includes('global')); + const packagesSpec = specs.find(s => s.capability.includes('packages')); + + expect(globalSpec).toBeDefined(); + expect(packagesSpec).toBeDefined(); + + // On Windows: _global\testing, packages\auth + // On Unix: _global/testing, packages/auth + if (process.platform === 'win32') { + expect(globalSpec!.capability).toBe('_global\\testing'); + expect(packagesSpec!.capability).toBe('packages\\auth'); + } else { + expect(globalSpec!.capability).toBe('_global/testing'); + expect(packagesSpec!.capability).toBe('packages/auth'); + } + }); + + it('should correctly split capability names by path.sep for depth calculation', () => { + const specs = findAllSpecs(crossPlatformFixtureDir); + + const globalSpec = specs.find(s => s.capability.includes('global')); + const packagesSpec = specs.find(s => s.capability.includes('packages')); + + expect(globalSpec!.depth).toBe(2); + expect(packagesSpec!.depth).toBe(2); + + // Verify depth calculation uses path.sep correctly + expect(globalSpec!.capability.split(path.sep)).toHaveLength(2); + expect(packagesSpec!.capability.split(path.sep)).toHaveLength(2); + }); + }); + + describe('findSpecUpdates()', () => { + let updatesFixtureDir: string; + let changeDir: string; + let mainSpecsDir: string; + + beforeEach(() => { + updatesFixtureDir = path.join(fixturesDir, 'spec-updates'); + changeDir = path.join(updatesFixtureDir, 'changes', 'my-change'); + mainSpecsDir = path.join(updatesFixtureDir, 'specs'); + }); + + afterEach(() => { + if (fs.existsSync(updatesFixtureDir)) { + fs.rmSync(updatesFixtureDir, { recursive: true, force: true }); + } + }); + + it('should map flat delta specs to main specs', () => { + // Create main spec + fs.mkdirSync(path.join(mainSpecsDir, 'auth'), { recursive: true }); + fs.writeFileSync(path.join(mainSpecsDir, 'auth', 'spec.md'), '# Auth'); + + // Create delta spec in change + fs.mkdirSync(path.join(changeDir, 'specs', 'auth'), { recursive: true }); + fs.writeFileSync(path.join(changeDir, 'specs', 'auth', 'spec.md'), '# Auth Delta'); + + const updates = findSpecUpdates(changeDir, mainSpecsDir); + + expect(updates).toHaveLength(1); + expect(updates[0].capability).toBe('auth'); + expect(updates[0].exists).toBe(true); + expect(updates[0].source).toContain(path.join('my-change', 'specs', 'auth', 'spec.md')); + expect(updates[0].target).toBe(path.join(mainSpecsDir, 'auth', 'spec.md')); + }); + + it('should map hierarchical delta specs preserving path structure', () => { + // Create main spec + fs.mkdirSync(path.join(mainSpecsDir, '_global', 'testing'), { recursive: true }); + fs.writeFileSync(path.join(mainSpecsDir, '_global', 'testing', 'spec.md'), '# Testing'); + + // Create delta spec in change + fs.mkdirSync(path.join(changeDir, 'specs', '_global', 'testing'), { recursive: true }); + fs.writeFileSync(path.join(changeDir, 'specs', '_global', 'testing', 'spec.md'), '# Testing Delta'); + + const updates = findSpecUpdates(changeDir, mainSpecsDir); + + expect(updates).toHaveLength(1); + expect(updates[0].capability).toBe(path.join('_global', 'testing')); + expect(updates[0].exists).toBe(true); + }); + + it('should detect new capabilities (exists=false) when main spec does not exist', () => { + // No main spec + fs.mkdirSync(mainSpecsDir, { recursive: true }); + + // Create delta spec for a new capability + fs.mkdirSync(path.join(changeDir, 'specs', 'new-feature'), { recursive: true }); + fs.writeFileSync(path.join(changeDir, 'specs', 'new-feature', 'spec.md'), '# New Feature'); + + const updates = findSpecUpdates(changeDir, mainSpecsDir); + + expect(updates).toHaveLength(1); + expect(updates[0].capability).toBe('new-feature'); + expect(updates[0].exists).toBe(false); + }); + + it('should return empty array when change has no specs directory', () => { + fs.mkdirSync(changeDir, { recursive: true }); + fs.mkdirSync(mainSpecsDir, { recursive: true }); + + const updates = findSpecUpdates(changeDir, mainSpecsDir); + + expect(updates).toHaveLength(0); + }); + + it('should handle mixed flat and hierarchical delta specs', () => { + // Create main specs + fs.mkdirSync(path.join(mainSpecsDir, 'auth'), { recursive: true }); + fs.writeFileSync(path.join(mainSpecsDir, 'auth', 'spec.md'), '# Auth'); + fs.mkdirSync(path.join(mainSpecsDir, '_global', 'testing'), { recursive: true }); + fs.writeFileSync(path.join(mainSpecsDir, '_global', 'testing', 'spec.md'), '# Testing'); + + // Create delta specs + fs.mkdirSync(path.join(changeDir, 'specs', 'auth'), { recursive: true }); + fs.writeFileSync(path.join(changeDir, 'specs', 'auth', 'spec.md'), '# Auth Delta'); + fs.mkdirSync(path.join(changeDir, 'specs', '_global', 'testing'), { recursive: true }); + fs.writeFileSync(path.join(changeDir, 'specs', '_global', 'testing', 'spec.md'), '# Testing Delta'); + + const updates = findSpecUpdates(changeDir, mainSpecsDir); + + expect(updates).toHaveLength(2); + const capabilities = updates.map(u => u.capability).sort(); + expect(capabilities).toContain('auth'); + expect(capabilities).toContain(path.join('_global', 'testing')); + expect(updates.every(u => u.exists)).toBe(true); + }); + }); + + describe('validateSpecStructure() - orphaned specs validation', () => { + it('should return no issues for valid leaf-only specs', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'auth', path: path.join('/specs', 'auth', 'spec.md'), depth: 1 }, + { capability: path.join('_global', 'testing'), path: path.join('/specs', '_global', 'testing', 'spec.md'), depth: 2 }, + { capability: path.join('packages', 'auth', 'oauth'), path: path.join('/specs', 'packages', 'auth', 'oauth', 'spec.md'), depth: 3 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const orphanedIssues = issues.filter(i => i.message.includes('Orphaned spec')); + + expect(orphanedIssues).toHaveLength(0); + }); + + it('should detect orphaned spec at intermediate level (depth 1 parent of depth 2)', () => { + const specs: DiscoveredSpec[] = [ + { capability: '_global', path: path.join('/specs', '_global', 'spec.md'), depth: 1 }, + { capability: path.join('_global', 'testing'), path: path.join('/specs', '_global', 'testing', 'spec.md'), depth: 2 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const orphanedIssues = issues.filter(i => i.message.includes('Orphaned spec')); + + expect(orphanedIssues).toHaveLength(1); + expect(orphanedIssues[0].level).toBe('ERROR'); + expect(orphanedIssues[0].message).toContain('_global'); + expect(orphanedIssues[0].message).toContain('intermediate level'); + expect(orphanedIssues[0].capability).toBe('_global'); + }); + + it('should detect orphaned spec at intermediate level (depth 2 parent of depth 3)', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('packages', 'auth'), path: path.join('/specs', 'packages', 'auth', 'spec.md'), depth: 2 }, + { capability: path.join('packages', 'auth', 'oauth'), path: path.join('/specs', 'packages', 'auth', 'oauth', 'spec.md'), depth: 3 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const orphanedIssues = issues.filter(i => i.message.includes('Orphaned spec')); + + expect(orphanedIssues).toHaveLength(1); + expect(orphanedIssues[0].level).toBe('ERROR'); + expect(orphanedIssues[0].message).toContain(path.join('packages', 'auth')); + }); + + it('should detect multiple orphaned specs', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'platform', path: path.join('/specs', 'platform', 'spec.md'), depth: 1 }, + { capability: path.join('platform', 'services'), path: path.join('/specs', 'platform', 'services', 'spec.md'), depth: 2 }, + { capability: path.join('platform', 'services', 'api'), path: path.join('/specs', 'platform', 'services', 'api', 'spec.md'), depth: 3 }, + { capability: path.join('platform', 'services', 'api', 'rest'), path: path.join('/specs', 'platform', 'services', 'api', 'rest', 'spec.md'), depth: 4 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const orphanedIssues = issues.filter(i => i.message.includes('Orphaned spec')); + + // Should detect 3 orphaned specs (platform, platform/services, platform/services/api) + expect(orphanedIssues.length).toBeGreaterThanOrEqual(3); + expect(orphanedIssues.every(i => i.level === 'ERROR')).toBe(true); + }); + + it('should not flag specs with similar prefixes as orphaned', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'auth', path: path.join('/specs', 'auth', 'spec.md'), depth: 1 }, + { capability: 'authentication', path: path.join('/specs', 'authentication', 'spec.md'), depth: 1 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const orphanedIssues = issues.filter(i => i.message.includes('Orphaned spec')); + + expect(orphanedIssues).toHaveLength(0); + }); + }); + + describe('validateSpecStructure() - depth limits validation', () => { + it('should return no issues for specs within recommended depth', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'auth', path: path.join('/specs', 'auth', 'spec.md'), depth: 1 }, + { capability: path.join('_global', 'testing'), path: path.join('/specs', '_global', 'testing', 'spec.md'), depth: 2 }, + { capability: path.join('packages', 'auth', 'oauth'), path: path.join('/specs', 'packages', 'auth', 'oauth', 'spec.md'), depth: 3 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const depthIssues = issues.filter(i => i.message.includes('depth')); + + expect(depthIssues).toHaveLength(0); + }); + + it('should return WARNING for specs at depth 4 (above recommended)', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('platform', 'services', 'api', 'rest'), path: path.join('/specs', 'platform', 'services', 'api', 'rest', 'spec.md'), depth: 4 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const depthIssues = issues.filter(i => i.message.includes('depth')); + + expect(depthIssues).toHaveLength(1); + expect(depthIssues[0].level).toBe('WARNING'); + expect(depthIssues[0].message).toContain('depth 4'); + expect(depthIssues[0].message).toContain('Recommended maximum is 3'); + }); + + it('should return ERROR for specs exceeding configured maxDepth', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('a', 'b', 'c', 'd', 'e'), path: path.join('/specs', 'a', 'b', 'c', 'd', 'e', 'spec.md'), depth: 5 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const depthIssues = issues.filter(i => i.message.includes('exceeds maximum depth')); + + expect(depthIssues).toHaveLength(1); + expect(depthIssues[0].level).toBe('ERROR'); + expect(depthIssues[0].message).toContain('exceeds maximum depth 4'); + expect(depthIssues[0].message).toContain('actual: 5'); + }); + + it('should respect custom maxDepth configuration', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('a', 'b', 'c'), path: path.join('/specs', 'a', 'b', 'c', 'spec.md'), depth: 3 }, + ]; + + // With maxDepth: 2, depth 3 should be an error + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 2 }); + const depthIssues = issues.filter(i => i.message.includes('exceeds maximum depth')); + + expect(depthIssues).toHaveLength(1); + expect(depthIssues[0].level).toBe('ERROR'); + expect(depthIssues[0].message).toContain('exceeds maximum depth 2'); + }); + + it('should cap maxDepth at hard limit of 10', () => { + const segments = ['a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k']; + const specs: DiscoveredSpec[] = [ + { capability: segments.join(path.sep), path: path.join('/specs', ...segments, 'spec.md'), depth: 11 }, + ]; + + // Even with maxDepth: 10, hard limit is 10 so depth 11 is an error + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 10 }); + const depthIssues = issues.filter(i => i.message.includes('exceeds maximum depth')); + + expect(depthIssues).toHaveLength(1); + expect(depthIssues[0].level).toBe('ERROR'); + expect(depthIssues[0].message).toContain('exceeds maximum depth 10'); + }); + + it('should handle multiple specs with different depth issues', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('a', 'b'), path: path.join('/specs', 'a', 'b', 'spec.md'), depth: 2 }, // OK + { capability: path.join('c', 'd', 'e', 'f'), path: path.join('/specs', 'c', 'd', 'e', 'f', 'spec.md'), depth: 4 }, // WARNING + { capability: path.join('g', 'h', 'i', 'j', 'k'), path: path.join('/specs', 'g', 'h', 'i', 'j', 'k', 'spec.md'), depth: 5 }, // ERROR + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const depthWarnings = issues.filter(i => i.level === 'WARNING' && i.message.includes('depth')); + const depthErrors = issues.filter(i => i.level === 'ERROR' && i.message.includes('depth')); + + expect(depthWarnings).toHaveLength(1); + expect(depthErrors).toHaveLength(1); + }); + }); + + describe('validateSpecStructure() - naming conventions validation', () => { + it('should return no issues for valid lowercase alphanumeric names', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'auth', path: path.join('/specs', 'auth', 'spec.md'), depth: 1 }, + { capability: 'user-profile', path: path.join('/specs', 'user-profile', 'spec.md'), depth: 1 }, + { capability: 'api_gateway', path: path.join('/specs', 'api_gateway', 'spec.md'), depth: 1 }, + { capability: 'service123', path: path.join('/specs', 'service123', 'spec.md'), depth: 1 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const namingIssues = issues.filter(i => i.message.includes('Invalid segment')); + + expect(namingIssues).toHaveLength(0); + }); + + it('should reject uppercase letters in capability names', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'Auth', path: path.join('/specs', 'Auth', 'spec.md'), depth: 1 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const namingIssues = issues.filter(i => i.message.includes('Invalid segment')); + + expect(namingIssues).toHaveLength(1); + expect(namingIssues[0].level).toBe('ERROR'); + expect(namingIssues[0].message).toContain('Auth'); + expect(namingIssues[0].message).toContain('lowercase alphanumeric'); + }); + + it('should reject special characters (spaces, dots, @ symbols)', () => { + const invalidNames = [ + { capability: 'user profile', path: path.join('/specs', 'user profile', 'spec.md'), depth: 1 }, // space + { capability: 'auth.service', path: path.join('/specs', 'auth.service', 'spec.md'), depth: 1 }, // dot + { capability: 'api@gateway', path: path.join('/specs', 'api@gateway', 'spec.md'), depth: 1 }, // @ + { capability: 'service$name', path: path.join('/specs', 'service$name', 'spec.md'), depth: 1 }, // $ + ]; + + invalidNames.forEach(spec => { + const issues = validateSpecStructure([spec], { validatePaths: true, maxDepth: 4 }); + const namingIssues = issues.filter(i => i.message.includes('Invalid segment')); + expect(namingIssues.length).toBeGreaterThan(0); + expect(namingIssues[0].level).toBe('ERROR'); + }); + }); + + it('should validate all segments in hierarchical capability names', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('valid', 'Invalid', 'valid'), path: path.join('/specs', 'valid', 'Invalid', 'valid', 'spec.md'), depth: 3 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const namingIssues = issues.filter(i => i.message.includes('Invalid segment')); + + expect(namingIssues).toHaveLength(1); + expect(namingIssues[0].message).toContain('Invalid'); + }); + + it('should allow underscore-prefixed names', () => { + const specs: DiscoveredSpec[] = [ + { capability: '_global', path: path.join('/specs', '_global', 'spec.md'), depth: 1 }, + { capability: path.join('_private', 'auth'), path: path.join('/specs', '_private', 'auth', 'spec.md'), depth: 2 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const namingIssues = issues.filter(i => i.message.includes('Invalid segment')); + + expect(namingIssues).toHaveLength(0); + }); + + it('should only report naming issue once per capability', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('Bad', 'Also-Bad', 'more-bad'), path: path.join('/specs', 'Bad', 'Also-Bad', 'more-bad', 'spec.md'), depth: 3 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const namingIssues = issues.filter(i => i.message.includes('Invalid segment')); + + // Should only report once for the first invalid segment + expect(namingIssues).toHaveLength(1); + }); + + it('should skip naming validation when validatePaths is false', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'InvalidName', path: path.join('/specs', 'InvalidName', 'spec.md'), depth: 1 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: false, maxDepth: 4 }); + const namingIssues = issues.filter(i => i.message.includes('Invalid segment')); + + expect(namingIssues).toHaveLength(0); + }); + }); + + describe('validateSpecStructure() - path length validation', () => { + it('should warn when full path exceeds Windows MAX_PATH (260)', () => { + // Build a path that exceeds 260 chars total + const prefix = path.join('/a-long-project-root-directory', 'with', 'many', 'levels', 'openspec', 'specs'); + const longSegments = Array.from({ length: 8 }, (_, i) => `segment-${String(i).padStart(2, '0')}-with-extra-padding`); + const capability = longSegments.join(path.sep); + const fullPath = path.join(prefix, ...longSegments, 'spec.md'); + + expect(fullPath.length).toBeGreaterThan(260); + + const specs: DiscoveredSpec[] = [ + { capability, path: fullPath, depth: 8 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 10 }); + const lengthIssues = issues.filter(i => i.message.includes('characters')); + + expect(lengthIssues).toHaveLength(1); + expect(lengthIssues[0].level).toBe('WARNING'); + expect(lengthIssues[0].message).toContain('Windows MAX_PATH'); + }); + + it('should not warn for paths under 260 characters', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('platform', 'services', 'api'), path: path.join('/specs', 'platform', 'services', 'api', 'spec.md'), depth: 3 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const lengthIssues = issues.filter(i => i.message.includes('characters')); + + expect(lengthIssues).toHaveLength(0); + }); + + it('should warn when capability path exceeds 160 characters (even if full path under 260)', () => { + // Build a capability that exceeds 160 chars but keep full path under 260 + const longSegments = Array.from({ length: 6 }, (_, i) => `long-segment-name-${String(i).padStart(2, '0')}-padding`); + const capability = longSegments.join(path.sep); + + expect(capability.length).toBeGreaterThan(160); + + // Short prefix keeps full path under 260 + const fullPath = path.join('/specs', ...longSegments, 'spec.md'); + expect(fullPath.length).toBeLessThan(260); + + const specs: DiscoveredSpec[] = [ + { capability, path: fullPath, depth: 6 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 10 }); + const lengthIssues = issues.filter(i => i.message.includes('characters')); + + expect(lengthIssues).toHaveLength(1); + expect(lengthIssues[0].level).toBe('WARNING'); + expect(lengthIssues[0].message).toContain('capability path'); + expect(lengthIssues[0].message).toContain('160'); + }); + + it('should skip path length check when validatePaths is false', () => { + const prefix = path.join('/a-long-project-root-directory', 'with', 'many', 'levels', 'openspec', 'specs'); + const longSegments = Array.from({ length: 8 }, (_, i) => `segment-${String(i).padStart(2, '0')}-with-extra-padding`); + const capability = longSegments.join(path.sep); + const fullPath = path.join(prefix, ...longSegments, 'spec.md'); + + const specs: DiscoveredSpec[] = [ + { capability, path: fullPath, depth: 8 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: false, maxDepth: 10 }); + const lengthIssues = issues.filter(i => i.message.includes('characters')); + + expect(lengthIssues).toHaveLength(0); + }); + }); + + describe('validateSpecStructure() - reserved names validation', () => { + it('should reject reserved directory names', () => { + const reservedNames = [ + { capability: '..', path: `${path.sep}specs${path.sep}..${path.sep}spec.md`, depth: 1 }, + { capability: '.', path: `${path.sep}specs${path.sep}.${path.sep}spec.md`, depth: 1 }, + { capability: '.git', path: path.join('/specs', '.git', 'spec.md'), depth: 1 }, + { capability: 'node_modules', path: path.join('/specs', 'node_modules', 'spec.md'), depth: 1 }, + { capability: '.openspec', path: path.join('/specs', '.openspec', 'spec.md'), depth: 1 }, + ]; + + reservedNames.forEach(spec => { + const issues = validateSpecStructure([spec], { validatePaths: true, maxDepth: 4 }); + const reservedIssues = issues.filter(i => i.message.includes('Reserved name')); + + expect(reservedIssues).toHaveLength(1); + expect(reservedIssues[0].level).toBe('ERROR'); + expect(reservedIssues[0].message).toContain(spec.capability); + expect(reservedIssues[0].capability).toBe(spec.capability); + }); + }); + + it('should reject reserved names in hierarchical paths', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('packages', '.git', 'auth'), path: path.join('/specs', 'packages', '.git', 'auth', 'spec.md'), depth: 3 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const reservedIssues = issues.filter(i => i.message.includes('Reserved name')); + + expect(reservedIssues).toHaveLength(1); + expect(reservedIssues[0].level).toBe('ERROR'); + expect(reservedIssues[0].message).toContain('.git'); + }); + + it('should allow valid names that contain reserved substrings', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'git-utils', path: path.join('/specs', 'git-utils', 'spec.md'), depth: 1 }, // contains "git" but not ".git" + { capability: 'node-server', path: path.join('/specs', 'node-server', 'spec.md'), depth: 1 }, // contains "node" but not "node_modules" + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const reservedIssues = issues.filter(i => i.message.includes('Reserved name')); + + expect(reservedIssues).toHaveLength(0); + }); + + it('should list all reserved names in error message', () => { + const specs: DiscoveredSpec[] = [ + { capability: '.git', path: path.join('/specs', '.git', 'spec.md'), depth: 1 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const reservedIssues = issues.filter(i => i.message.includes('Reserved name')); + + expect(reservedIssues[0].message).toContain('..'); + expect(reservedIssues[0].message).toContain('.'); + expect(reservedIssues[0].message).toContain('.git'); + expect(reservedIssues[0].message).toContain('node_modules'); + expect(reservedIssues[0].message).toContain('.openspec'); + }); + + it('should reject Windows reserved device names', () => { + const windowsReserved = ['con', 'prn', 'aux', 'nul', 'com1', 'lpt1']; + + windowsReserved.forEach(name => { + const spec: DiscoveredSpec = { capability: name, path: path.join('/specs', name, 'spec.md'), depth: 1 }; + const issues = validateSpecStructure([spec], { validatePaths: true, maxDepth: 4 }); + const windowsIssues = issues.filter(i => i.message.includes('Windows reserved name')); + + expect(windowsIssues).toHaveLength(1); + expect(windowsIssues[0].level).toBe('ERROR'); + expect(windowsIssues[0].message).toContain(name); + }); + }); + + it('should reject Windows reserved names in hierarchical paths', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('platform', 'con'), path: path.join('/specs', 'platform', 'con', 'spec.md'), depth: 2 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const windowsIssues = issues.filter(i => i.message.includes('Windows reserved name')); + + expect(windowsIssues).toHaveLength(1); + expect(windowsIssues[0].message).toContain('con'); + }); + + it('should only report reserved name issue once per capability', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('.git', 'node_modules'), path: path.join('/specs', '.git', 'node_modules', 'spec.md'), depth: 2 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + const reservedIssues = issues.filter(i => i.message.includes('Reserved name')); + + // Should only report once for the first reserved name found + expect(reservedIssues).toHaveLength(1); + }); + + it('should skip reserved name validation when validatePaths is false', () => { + const specs: DiscoveredSpec[] = [ + { capability: '.git', path: path.join('/specs', '.git', 'spec.md'), depth: 1 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: false, maxDepth: 4 }); + const reservedIssues = issues.filter(i => i.message.includes('Reserved name')); + + expect(reservedIssues).toHaveLength(0); + }); + }); + + describe('validateSpecStructure() - combined validation', () => { + it('should return empty array for completely valid specs', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'auth', path: path.join('/specs', 'auth', 'spec.md'), depth: 1 }, + { capability: path.join('_global', 'testing'), path: path.join('/specs', '_global', 'testing', 'spec.md'), depth: 2 }, + { capability: path.join('packages', 'api-gateway'), path: path.join('/specs', 'packages', 'api-gateway', 'spec.md'), depth: 2 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + + expect(issues).toHaveLength(0); + }); + + it('should detect multiple types of validation issues', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'platform', path: path.join('/specs', 'platform', 'spec.md'), depth: 1 }, // Orphaned (parent of next) + { capability: path.join('platform', 'services'), path: path.join('/specs', 'platform', 'services', 'spec.md'), depth: 2 }, + { capability: 'Invalid-Name', path: path.join('/specs', 'Invalid-Name', 'spec.md'), depth: 1 }, // Uppercase + { capability: path.join('a', 'b', 'c', 'd', 'e'), path: path.join('/specs', 'a', 'b', 'c', 'd', 'e', 'spec.md'), depth: 5 }, // Too deep + { capability: '.git', path: path.join('/specs', '.git', 'spec.md'), depth: 1 }, // Reserved + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 4 }); + + const orphanedIssues = issues.filter(i => i.message.includes('Orphaned spec')); + const depthIssues = issues.filter(i => i.message.includes('exceeds maximum depth')); + const namingIssues = issues.filter(i => i.message.includes('Invalid segment')); + const reservedIssues = issues.filter(i => i.message.includes('Reserved name')); + + expect(orphanedIssues.length).toBeGreaterThan(0); + expect(depthIssues.length).toBeGreaterThan(0); + expect(namingIssues.length).toBeGreaterThan(0); + expect(reservedIssues.length).toBeGreaterThan(0); + }); + + it('should respect config defaults', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('a', 'b', 'c', 'd', 'e'), path: path.join('/specs', 'a', 'b', 'c', 'd', 'e', 'spec.md'), depth: 5 }, + ]; + + // When maxDepth not specified, should use default (4) + const issues = validateSpecStructure(specs, {}); + const depthIssues = issues.filter(i => i.message.includes('exceeds maximum depth')); + + expect(depthIssues).toHaveLength(1); + expect(depthIssues[0].message).toContain('exceeds maximum depth 4'); + }); + + it('should handle empty specs array', () => { + const issues = validateSpecStructure([], { validatePaths: true, maxDepth: 4 }); + + expect(issues).toHaveLength(0); + }); + }); + + describe('validateSpecStructure() - configuration settings', () => { + it('should use default maxDepth of 4 when not specified', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('a', 'b', 'c', 'd', 'e'), path: path.join('/specs', 'a', 'b', 'c', 'd', 'e', 'spec.md'), depth: 5 }, + ]; + + const issues = validateSpecStructure(specs, {}); + const depthIssues = issues.filter(i => i.message.includes('exceeds maximum depth')); + + expect(depthIssues).toHaveLength(1); + expect(depthIssues[0].message).toContain('exceeds maximum depth 4'); + }); + + it('should use default validatePaths of true when not specified', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'InvalidName', path: path.join('/specs', 'InvalidName', 'spec.md'), depth: 1 }, + ]; + + const issues = validateSpecStructure(specs, {}); + const namingIssues = issues.filter(i => i.message.includes('Invalid segment')); + + expect(namingIssues).toHaveLength(1); + }); + + it('should disable naming validation when validatePaths is false', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'Invalid-Name', path: path.join('/specs', 'Invalid-Name', 'spec.md'), depth: 1 }, + { capability: 'auth.service', path: path.join('/specs', 'auth.service', 'spec.md'), depth: 1 }, + { capability: '.git', path: path.join('/specs', '.git', 'spec.md'), depth: 1 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: false, maxDepth: 4 }); + const namingIssues = issues.filter(i => i.message.includes('Invalid segment')); + const reservedIssues = issues.filter(i => i.message.includes('Reserved name')); + + expect(namingIssues).toHaveLength(0); + expect(reservedIssues).toHaveLength(0); + }); + + it('should still validate depth and orphaned specs when validatePaths is false', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'parent', path: path.join('/specs', 'parent', 'spec.md'), depth: 1 }, + { capability: path.join('parent', 'child'), path: path.join('/specs', 'parent', 'child', 'spec.md'), depth: 2 }, + { capability: path.join('a', 'b', 'c', 'd', 'e'), path: path.join('/specs', 'a', 'b', 'c', 'd', 'e', 'spec.md'), depth: 5 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: false, maxDepth: 4 }); + const orphanedIssues = issues.filter(i => i.message.includes('Orphaned spec')); + const depthIssues = issues.filter(i => i.message.includes('exceeds maximum depth')); + + expect(orphanedIssues.length).toBeGreaterThan(0); + expect(depthIssues.length).toBeGreaterThan(0); + }); + + it('should respect custom maxDepth value of 2', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('a', 'b'), path: path.join('/specs', 'a', 'b', 'spec.md'), depth: 2 }, // OK + { capability: path.join('c', 'd', 'e'), path: path.join('/specs', 'c', 'd', 'e', 'spec.md'), depth: 3 }, // ERROR + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 2 }); + const depthErrors = issues.filter(i => i.level === 'ERROR' && i.message.includes('exceeds maximum depth')); + + expect(depthErrors).toHaveLength(1); + expect(depthErrors[0].message).toContain('exceeds maximum depth 2'); + expect(depthErrors[0].message).toContain('actual: 3'); + }); + + it('should respect custom maxDepth value of 6', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('a', 'b', 'c', 'd', 'e', 'f'), path: path.join('/specs', 'a', 'b', 'c', 'd', 'e', 'f', 'spec.md'), depth: 6 }, // OK (at limit) + { capability: path.join('g', 'h', 'i', 'j', 'k', 'l', 'm'), path: path.join('/specs', 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'spec.md'), depth: 7 }, // ERROR + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 6 }); + const depthErrors = issues.filter(i => i.level === 'ERROR' && i.message.includes('exceeds maximum depth')); + + expect(depthErrors).toHaveLength(1); + expect(depthErrors[0].message).toContain('exceeds maximum depth 6'); + }); + + it('should handle maxDepth: 1 (flat only)', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'auth', path: path.join('/specs', 'auth', 'spec.md'), depth: 1 }, // OK + { capability: path.join('_global', 'testing'), path: path.join('/specs', '_global', 'testing', 'spec.md'), depth: 2 }, // ERROR + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 1 }); + const depthErrors = issues.filter(i => i.level === 'ERROR' && i.message.includes('exceeds maximum depth')); + + expect(depthErrors).toHaveLength(1); + expect(depthErrors[0].capability).toBe(path.join('_global', 'testing')); + }); + + it('should show warnings for depth 4-6 when within configured maxDepth', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('a', 'b', 'c', 'd'), path: path.join('/specs', 'a', 'b', 'c', 'd', 'spec.md'), depth: 4 }, + { capability: path.join('e', 'f', 'g', 'h', 'i'), path: path.join('/specs', 'e', 'f', 'g', 'h', 'i', 'spec.md'), depth: 5 }, + ]; + + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 6 }); + const depthWarnings = issues.filter(i => i.level === 'WARNING' && i.message.includes('depth')); + + expect(depthWarnings).toHaveLength(2); + expect(depthWarnings[0].message).toContain('Recommended maximum is 3'); + }); + + it('should combine all config options correctly', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'ValidName', path: path.join('/specs', 'ValidName', 'spec.md'), depth: 1 }, // Invalid if validatePaths=true + { capability: path.join('a', 'b', 'c'), path: path.join('/specs', 'a', 'b', 'c', 'spec.md'), depth: 3 }, // OK if maxDepth>=3 + ]; + + // Strict config: enable all validations, low maxDepth + const strictIssues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 2 }); + expect(strictIssues.length).toBeGreaterThan(0); + + // Lenient config: disable path validation, high maxDepth + const lenientIssues = validateSpecStructure(specs, { validatePaths: false, maxDepth: 6 }); + expect(lenientIssues).toHaveLength(0); + }); + + it('should handle undefined config values with defaults', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('a', 'b', 'c', 'd', 'e'), path: path.join('/specs', 'a', 'b', 'c', 'd', 'e', 'spec.md'), depth: 5 }, + { capability: 'InvalidName', path: path.join('/specs', 'InvalidName', 'spec.md'), depth: 1 }, + ]; + + const issues = validateSpecStructure(specs, { structure: 'auto' }); // Only structure specified + + // Should use defaults: maxDepth=4, validatePaths=true + const depthIssues = issues.filter(i => i.message.includes('exceeds maximum depth')); + const namingIssues = issues.filter(i => i.message.includes('Invalid segment')); + + expect(depthIssues).toHaveLength(1); + expect(namingIssues).toHaveLength(1); + }); + + it('should handle empty config object with all defaults', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('a', 'b', 'c', 'd', 'e'), path: path.join('/specs', 'a', 'b', 'c', 'd', 'e', 'spec.md'), depth: 5 }, + ]; + + const issues = validateSpecStructure(specs, {}); + const depthIssues = issues.filter(i => i.message.includes('exceeds maximum depth')); + + expect(depthIssues).toHaveLength(1); + expect(depthIssues[0].message).toContain('exceeds maximum depth 4'); + }); + + it('should allow maxDepth 10 without capping', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('a', 'b', 'c', 'd', 'e', 'f', 'g'), path: path.join('/specs', 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'spec.md'), depth: 7 }, + ]; + + // With maxDepth: 10, depth 7 should not be an error (only a warning if > recommended) + const issues = validateSpecStructure(specs, { validatePaths: true, maxDepth: 10 }); + const depthErrors = issues.filter(i => i.message.includes('exceeds maximum depth')); + + expect(depthErrors).toHaveLength(0); + }); + + it('should not enforce structure in auto mode with mixed specs', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'auth', path: path.join('/specs', 'auth', 'spec.md'), depth: 1 }, + { capability: path.join('_global', 'testing'), path: path.join('/specs', '_global', 'testing', 'spec.md'), depth: 2 }, + ]; + + const issues = validateSpecStructure(specs, { structure: 'auto', validatePaths: true, maxDepth: 4 }); + + expect(issues).toHaveLength(0); + }); + + it('should reject hierarchical specs in flat mode', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'auth', path: path.join('/specs', 'auth', 'spec.md'), depth: 1 }, + { capability: path.join('_global', 'testing'), path: path.join('/specs', '_global', 'testing', 'spec.md'), depth: 2 }, + ]; + + const issues = validateSpecStructure(specs, { structure: 'flat', validatePaths: true, maxDepth: 4 }); + + const structureIssues = issues.filter(i => i.message.includes('structure is set to "flat"')); + expect(structureIssues).toHaveLength(1); + expect(structureIssues[0].level).toBe('ERROR'); + expect(structureIssues[0].capability).toBe(path.join('_global', 'testing')); + }); + + it('should reject flat specs in hierarchical mode', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'auth', path: path.join('/specs', 'auth', 'spec.md'), depth: 1 }, + { capability: path.join('_global', 'testing'), path: path.join('/specs', '_global', 'testing', 'spec.md'), depth: 2 }, + ]; + + const issues = validateSpecStructure(specs, { structure: 'hierarchical', validatePaths: true, maxDepth: 4 }); + + const structureIssues = issues.filter(i => i.message.includes('structure is set to "hierarchical"')); + expect(structureIssues).toHaveLength(1); + expect(structureIssues[0].level).toBe('ERROR'); + expect(structureIssues[0].capability).toBe('auth'); + }); + }); + + describe('validateSpecStructure() - structure mode enforcement', () => { + it('should accept all flat specs in flat mode', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'auth', path: path.join('/specs', 'auth', 'spec.md'), depth: 1 }, + { capability: 'payments', path: path.join('/specs', 'payments', 'spec.md'), depth: 1 }, + ]; + + const issues = validateSpecStructure(specs, { structure: 'flat', validatePaths: false }); + + expect(issues).toHaveLength(0); + }); + + it('should reject all hierarchical specs in flat mode', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('_global', 'testing'), path: path.join('/specs', '_global', 'testing', 'spec.md'), depth: 2 }, + { capability: path.join('platform', 'api'), path: path.join('/specs', 'platform', 'api', 'spec.md'), depth: 2 }, + ]; + + const issues = validateSpecStructure(specs, { structure: 'flat', validatePaths: false }); + + const structureIssues = issues.filter(i => i.message.includes('structure is set to "flat"')); + expect(structureIssues).toHaveLength(2); + }); + + it('should accept all hierarchical specs in hierarchical mode', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('_global', 'testing'), path: path.join('/specs', '_global', 'testing', 'spec.md'), depth: 2 }, + { capability: path.join('platform', 'api'), path: path.join('/specs', 'platform', 'api', 'spec.md'), depth: 2 }, + ]; + + const issues = validateSpecStructure(specs, { structure: 'hierarchical', validatePaths: false }); + + expect(issues).toHaveLength(0); + }); + + it('should reject all flat specs in hierarchical mode', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'auth', path: path.join('/specs', 'auth', 'spec.md'), depth: 1 }, + { capability: 'payments', path: path.join('/specs', 'payments', 'spec.md'), depth: 1 }, + ]; + + const issues = validateSpecStructure(specs, { structure: 'hierarchical', validatePaths: false }); + + const structureIssues = issues.filter(i => i.message.includes('structure is set to "hierarchical"')); + expect(structureIssues).toHaveLength(2); + }); + }); + + describe('validateSpecStructure() - allowMixed enforcement', () => { + it('should error on mixed specs when allowMixed is false and structure is auto', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'auth', path: path.join('/specs', 'auth', 'spec.md'), depth: 1 }, + { capability: path.join('_global', 'testing'), path: path.join('/specs', '_global', 'testing', 'spec.md'), depth: 2 }, + ]; + + const issues = validateSpecStructure(specs, { structure: 'auto', allowMixed: false, validatePaths: false }); + + const mixedIssues = issues.filter(i => i.message.includes('Mixed spec structure')); + expect(mixedIssues).toHaveLength(1); + expect(mixedIssues[0].level).toBe('ERROR'); + expect(mixedIssues[0].message).toContain('1 flat'); + expect(mixedIssues[0].message).toContain('1 hierarchical'); + }); + + it('should pass uniform flat specs when allowMixed is false', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'auth', path: path.join('/specs', 'auth', 'spec.md'), depth: 1 }, + { capability: 'payments', path: path.join('/specs', 'payments', 'spec.md'), depth: 1 }, + ]; + + const issues = validateSpecStructure(specs, { structure: 'auto', allowMixed: false, validatePaths: false }); + + expect(issues).toHaveLength(0); + }); + + it('should pass uniform hierarchical specs when allowMixed is false', () => { + const specs: DiscoveredSpec[] = [ + { capability: path.join('_global', 'testing'), path: path.join('/specs', '_global', 'testing', 'spec.md'), depth: 2 }, + { capability: path.join('platform', 'api'), path: path.join('/specs', 'platform', 'api', 'spec.md'), depth: 2 }, + ]; + + const issues = validateSpecStructure(specs, { structure: 'auto', allowMixed: false, validatePaths: false }); + + expect(issues).toHaveLength(0); + }); + + it('should not check allowMixed when structure is explicit', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'auth', path: path.join('/specs', 'auth', 'spec.md'), depth: 1 }, + { capability: path.join('_global', 'testing'), path: path.join('/specs', '_global', 'testing', 'spec.md'), depth: 2 }, + ]; + + // flat mode with allowMixed: false — should only get structure errors, not mixed errors + const issues = validateSpecStructure(specs, { structure: 'flat', allowMixed: false, validatePaths: false }); + + const mixedIssues = issues.filter(i => i.message.includes('Mixed spec structure')); + expect(mixedIssues).toHaveLength(0); + + const structureIssues = issues.filter(i => i.message.includes('structure is set to "flat"')); + expect(structureIssues).toHaveLength(1); + }); + + it('should allow mixed specs when allowMixed is true (default)', () => { + const specs: DiscoveredSpec[] = [ + { capability: 'auth', path: path.join('/specs', 'auth', 'spec.md'), depth: 1 }, + { capability: path.join('_global', 'testing'), path: path.join('/specs', '_global', 'testing', 'spec.md'), depth: 2 }, + ]; + + const issues = validateSpecStructure(specs, { structure: 'auto', allowMixed: true, validatePaths: false }); + + expect(issues).toHaveLength(0); + }); + }); +});