feat: Add hierarchical specs change proposal#839
Open
victorhsb wants to merge 2 commits intoFission-AI:mainfrom
Open
feat: Add hierarchical specs change proposal#839victorhsb wants to merge 2 commits intoFission-AI:mainfrom
victorhsb wants to merge 2 commits intoFission-AI:mainfrom
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Contributor
📝 WalkthroughWalkthroughThis PR implements hierarchical spec support for OpenSpec, enabling nested spec organization through forward-slash-based IDs (e.g., Changes
Sequence Diagram(s)sequenceDiagram
actor User as User/CLI
participant CLI as CLI Handler
participant Discovery as getSpecIds()
participant Resolver as specIdToPath()
participant Parser as MarkdownParser
participant UI as Console Output
User->>CLI: openspec spec list [prefix]
CLI->>Discovery: getSpecIds(specsDir)
Discovery-->>CLI: [cli/show, cli/validate, domain/feature, ...]
CLI->>CLI: Filter by prefix (if provided)
CLI->>Resolver: specIdToPath(filtered IDs)
Resolver-->>CLI: File paths
CLI->>Parser: Parse spec.md for each ID
Parser-->>CLI: Requirement counts
CLI->>UI: Display table with hierarchical IDs
UI-->>User: Spec id | Requirement count
sequenceDiagram
actor User as User
participant CLI as validate CLI
participant TypeDetect as Type Detection
participant Resolver as specIdToPath()
participant FileSystem as File System
participant Validator as Validator
participant Output as Output
User->>CLI: openspec validate cli/show
CLI->>TypeDetect: Detect hierarchical spec ID
TypeDetect->>Resolver: Resolve cli/show to path
Resolver-->>TypeDetect: openspec/specs/cli/show/spec.md
TypeDetect->>FileSystem: Check file exists
FileSystem-->>TypeDetect: File found
TypeDetect->>Validator: Validate spec
Validator-->>Output: Validation result with hierarchical ID
Output-->>User: Spec cli/show valid/invalid
sequenceDiagram
actor User as User
participant Archive as Archive Command
participant Discovery as Glob Pattern
participant IdExtract as specId Extraction
participant Resolver as specIdToPath()
participant Source as Source File
participant Target as Target Directory
participant Confirm as Confirmation Prompt
User->>Archive: Run archive
Archive->>Discovery: Find spec.md under changes/*/specs/
Discovery-->>Archive: [cli/show/spec.md, domain/feature/spec.md, ...]
loop For each delta spec
Archive->>IdExtract: Extract specId from path
IdExtract-->>Archive: cli/show, domain/feature
Archive->>Resolver: Resolve target path
Resolver-->>Archive: openspec/specs/cli/show/spec.md
Archive->>Target: Create intermediate directories
Target-->>Archive: Dirs created
Archive->>Source: Read delta spec content
Source-->>Archive: Content
end
Archive->>Confirm: Display confirmation with hierarchical paths
Confirm-->>User: NEW/EXISTING specs with full hierarchical IDs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
openspec/changes/hierarchical-specs/specs/cli-list/spec.md (1)
19-21: Clarify subtree prefix matching semantics to avoid false positives.Please explicitly define segment-boundary matching for subtree filters (e.g.,
cli/should not matchclient/...). Without this, implementations may incorrectly use rawstartsWith("cli").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/hierarchical-specs/specs/cli-list/spec.md` around lines 19 - 21, The subtree filter semantics are ambiguous—implementations must perform segment-boundary matching rather than raw prefix matching for filters like the `--specs` argument; update the text around the examples for `openspec list --specs cli/` to state that a trailing slash denotes a segment boundary and that the filter should match only IDs whose next character after the prefix is a path separator (e.g., `cli/` matches `cli/foo` and `cli/bar/baz` but not `client/foo`), and add a short clarifying example showing a non-match (`client/...`) to prevent using startsWith("cli").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/changes/hierarchical-specs/proposal.md`:
- Line 13: The "**BREAKING**" label on the sentence about flat spec IDs
conflicts with the compatibility guarantee; change the label to a
compatibility/compat note or explicitly enumerate the exact breaking surface if
one exists (e.g., rename "**BREAKING**" to "**COMPATIBILITY**" or replace the
sentence with a note like "Compatibility: Existing flat spec IDs remain valid;
tools that treat spec IDs as opaque strings need no changes, but tools that
parse spec ID structure must be updated" and if there truly is a breaking
change, state the precise affected behavior and the conditions under which
migration is required, referencing the "**BREAKING**" token and the sentence
about flat spec IDs for context).
---
Nitpick comments:
In `@openspec/changes/hierarchical-specs/specs/cli-list/spec.md`:
- Around line 19-21: The subtree filter semantics are ambiguous—implementations
must perform segment-boundary matching rather than raw prefix matching for
filters like the `--specs` argument; update the text around the examples for
`openspec list --specs cli/` to state that a trailing slash denotes a segment
boundary and that the filter should match only IDs whose next character after
the prefix is a path separator (e.g., `cli/` matches `cli/foo` and `cli/bar/baz`
but not `client/foo`), and add a short clarifying example showing a non-match
(`client/...`) to prevent using startsWith("cli").
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f57b95c7-dffd-4ca2-a69b-486b0a736f0b
📒 Files selected for processing (12)
openspec/changes/hierarchical-specs/.openspec.yamlopenspec/changes/hierarchical-specs/design.mdopenspec/changes/hierarchical-specs/proposal.mdopenspec/changes/hierarchical-specs/specs/cli-archive/spec.mdopenspec/changes/hierarchical-specs/specs/cli-list/spec.mdopenspec/changes/hierarchical-specs/specs/cli-show/spec.mdopenspec/changes/hierarchical-specs/specs/cli-spec/spec.mdopenspec/changes/hierarchical-specs/specs/cli-validate/spec.mdopenspec/changes/hierarchical-specs/specs/cli-view/spec.mdopenspec/changes/hierarchical-specs/specs/hierarchical-spec-discovery/spec.mdopenspec/changes/hierarchical-specs/specs/hierarchical-spec-resolution/spec.mdopenspec/changes/hierarchical-specs/tasks.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/changes/hierarchical-specs/specs/cli-list/spec.md`:
- Around line 25-38: The spec currently mandates table output but the CLI
supports a --json mode, so add an explicit exception: update the "Displaying
change list (default)" and "Displaying spec list" scenarios in
specs/cli-list/spec.md to state that when the --json flag is present the command
SHALL emit machine-readable JSON (structured arrays/objects with fields for
change name, task progress, spec id, requirement count) instead of a
human-readable table; reference the CLI option --json (from the CLI entry point)
as the trigger and ensure the new scenario clarifies the JSON schema or refers
to a separate JSON output schema section.
- Around line 19-23: The CLI contract is ambiguous: choose a single canonical
form for the subtree filter (either make it a valued option like "--specs
<prefix>" or a positional argument "<prefix>") and update the CLI parser and
help text accordingly so "openspec list" unambiguously accepts a prefix value;
concretely, modify the CLI definition where the "--specs" flag is declared to
accept a string value (or add a positional parameter for the prefix) and update
the command handler for "openspec list" to read that value, enforce the existing
segment-boundary matching logic (prefix must end with '/' or be followed by a
path separator), and update any tests/docs that reference the old flag-only
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e218854e-ef51-46c4-9475-81c0f9cee2b5
📒 Files selected for processing (2)
openspec/changes/hierarchical-specs/proposal.mdopenspec/changes/hierarchical-specs/specs/cli-list/spec.md
Comment on lines
+25
to
+38
| The command SHALL display items in a clear, readable table format with mode-appropriate progress or counts. | ||
|
|
||
| #### Scenario: Displaying change list (default) | ||
| - **WHEN** displaying the list of changes | ||
| - **THEN** show a table with columns: | ||
| - Change name (directory name) | ||
| - Task progress (e.g., "3/5 tasks" or "✓ Complete") | ||
|
|
||
| #### Scenario: Displaying spec list | ||
| - **WHEN** displaying the list of specs | ||
| - **THEN** show a table with columns: | ||
| - Spec id (full hierarchical path, e.g., `cli/show` or `cli-show`) | ||
| - Requirement count (e.g., "requirements 12") | ||
|
|
Contributor
There was a problem hiding this comment.
Table-only output requirement conflicts with machine-readable JSON mode.
This section says output “SHALL” be table format, but the command surface includes --json (see src/cli/index.ts Lines 173-190). Add an explicit exception/scenario for JSON output to avoid contradictory requirements.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/hierarchical-specs/specs/cli-list/spec.md` around lines 25 -
38, The spec currently mandates table output but the CLI supports a --json mode,
so add an explicit exception: update the "Displaying change list (default)" and
"Displaying spec list" scenarios in specs/cli-list/spec.md to state that when
the --json flag is present the command SHALL emit machine-readable JSON
(structured arrays/objects with fields for change name, task progress, spec id,
requirement count) instead of a human-readable table; reference the CLI option
--json (from the CLI entry point) as the trigger and ensure the new scenario
clarifies the JSON schema or refers to a separate JSON output schema section.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/list.ts (1)
156-193:⚠️ Potential issue | 🟡 MinorMissing JSON output support for specs mode.
The
executemethod handlesjsonoutput for changes mode (lines 100-101 and 131-141) but the specs mode (lines 167-169 and 186-192) only outputs to console, ignoringoptions.json. This inconsistency breaks programmatic use cases where callers expect JSON output for both modes.Additionally, the empty-result case (line 168) should also respect the
jsonflag, matching the pattern used in changes mode (line 101).🐛 Proposed fix to add JSON output for specs mode
+ if (allIds.length === 0) { + if (json) { + console.log(JSON.stringify({ specs: [] })); + } else { + console.log('No specs found.'); + } + return; + }And after collecting specs:
console.log('Specs:'); const padding = ' '; const nameWidth = Math.max(...specs.map(s => s.id.length)); + + if (json) { + console.log(JSON.stringify({ specs }, null, 2)); + return; + } + for (const spec of specs) { const padded = spec.id.padEnd(nameWidth); console.log(`${padding}${padded} requirements ${spec.requirementCount}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/list.ts` around lines 156 - 193, The specs mode in execute currently always prints to console and ignores options.json; update the branch that handles specs (the block that builds SpecInfo[] `specs` from `allIds`, uses `specIdToPath`, `MarkdownParser.parseSpec`, and prints "No specs found.") to mirror changes mode JSON behavior: when `options.json` is true emit a JSON object (e.g., { specs: [{ id, requirementCount }, ...] } or an empty { specs: [] } for the no-specs case) to stdout and return, otherwise keep the existing human-readable console output; ensure you use the existing `specs` array and `SpecInfo` shape and handle the empty-result path by returning the JSON empty list when `options.json` is set.
🧹 Nitpick comments (5)
src/core/specs-apply.ts (1)
441-445: Consider reusingwriteUpdatedSpecto avoid write/log divergence.
applySpecsre-implements directory creation and file writes already encapsulated bywriteUpdatedSpec, which increases maintenance overhead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/specs-apply.ts` around lines 441 - 445, In applySpecs replace the manual directory creation and file write (the block using path.dirname(p.update.target), fs.mkdir(...), and fs.writeFile(...)) with a call to the existing writeUpdatedSpec function so all writes and logging stay centralized; locate the code in applySpecs that writes p.rebuilt to p.update.target and invoke writeUpdatedSpec(p.update.target, p.rebuilt) (or the matching signature) instead, removing the duplicated fs.mkdir/fs.writeFile logic and relying on writeUpdatedSpec’s behavior and logging.openspec/changes/archive/2026-03-13-hierarchical-specs/specs/hierarchical-spec-resolution/spec.md (1)
44-45: Optional: Consider hyphenating "full-path matching" for consistency.Static analysis suggests using a hyphen when "full path" modifies "matching" (i.e., "full-path matching"). This is a minor style nit and can be deferred.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/changes/archive/2026-03-13-hierarchical-specs/specs/hierarchical-spec-resolution/spec.md` around lines 44 - 45, Change the phrase "full path matching" to hyphenated "full-path matching" in the requirement text under the heading "### Requirement: Fuzzy matching for hierarchical IDs" so the compound modifier reads "full-path matching" (keep "leaf-segment matching" as-is) to maintain consistent compound-adjective style.src/commands/spec.ts (1)
153-175: Defensive existence check may indicate stale discovery.The
existsSync(specPath)check at line 166 is defensive, but ifgetSpecIds()returns an ID, the correspondingspec.mdshould exist. Returningnulland filtering suggests a potential race condition or stale cache scenario.Consider logging a warning when a discovered spec ID doesn't resolve to an existing file, as this could indicate a bug in discovery or a concurrent file deletion.
💡 Optional: Add diagnostic logging for missing specs
const specs = allIds.map(id => { const specPath = specIdToPath(id, SPECS_DIR); if (existsSync(specPath)) { try { const spec = parseSpecFromFile(specPath, id); return { id, title: spec.name, requirementCount: spec.requirements.length }; } catch { return { id, title: id, requirementCount: 0 }; } } + // Spec ID was discovered but file doesn't exist - possible race or stale discovery + console.warn(`Warning: Discovered spec '${id}' but file not found at ${specPath}`); return null; }).filter((spec): spec is { id: string; title: string; requirementCount: number } => spec !== null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/commands/spec.ts` around lines 153 - 175, The discovery code in the .action handler uses existsSync(specPath) defensively and silently drops missing specs, which can hide stale/corrupt discovery or concurrent-deletion issues; update the block that iterates allIds (references: getSpecIds, specIdToPath, SPECS_DIR, parseSpecFromFile) to emit a warning via the existing logger (or processLogger) when a spec ID resolves to a non-existent file (include the spec id and resolved specPath in the message), so missing files are visible for diagnostics while keeping the current behavior of skipping unparsable files (still catch parseSpecFromFile errors and log them as parse failures).src/cli/index.ts (1)
179-184: Consider warning when prefix is used with --changes mode.The
prefixargument is passed toListCommand.executeregardless of mode, but only used whenmode === 'specs'. If a user runsopenspec list cli/ --changes, the prefix is silently ignored, which could be confusing.💡 Optional: Warn about ignored prefix in changes mode
.action(async (prefix?: string, options?: { specs?: boolean; changes?: boolean; sort?: string; json?: boolean }) => { try { const listCommand = new ListCommand(); const mode: 'changes' | 'specs' = options?.specs ? 'specs' : 'changes'; + if (prefix && mode === 'changes') { + console.warn('Warning: Prefix filtering is only supported with --specs. Prefix ignored.'); + } const sort = options?.sort === 'name' ? 'name' : 'recent'; await listCommand.execute('.', mode, { sort, json: options?.json, specsPrefix: prefix });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/index.ts` around lines 179 - 184, The code passes prefix into ListCommand.execute unconditionally but execute only uses specsPrefix when mode === 'specs', so if a user supplies a prefix with --changes it is silently ignored; add a user-facing warning before calling new ListCommand().execute: detect if prefix is truthy and mode === 'changes' and emit a concise warning (e.g., console.warn or the CLI logger used in this file) stating the prefix will be ignored in changes mode; keep the rest of the call intact and reference ListCommand, execute, mode, and prefix when making the change.test/specs/hierarchical-specs.integration.test.ts (1)
101-115: Console mock should use try/finally to ensure restoration.If the test throws before line 109,
console.logremains overridden, potentially affecting subsequent tests.🛡️ Wrap in try/finally to ensure cleanup
it('ListCommand with specsPrefix filters correctly', async () => { const output: string[] = []; const origLog = console.log; console.log = (...args: any[]) => output.push(args.join(' ')); + try { const cmd = new ListCommand(); await cmd.execute(testDir, 'specs', { specsPrefix: 'cli/' }); - - console.log = origLog; + } finally { + console.log = origLog; + } const joined = output.join('\n'); expect(joined).toContain('cli/show'); expect(joined).toContain('cli/validate'); expect(joined).not.toContain('client/config'); expect(joined).not.toContain('auth'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/specs/hierarchical-specs.integration.test.ts` around lines 101 - 115, The test overrides console.log (origLog and output) before calling new ListCommand().execute — wrap the call and assertions in a try/finally so console.log is always restored: move the await cmd.execute(...) and subsequent assertions into a try block and restore console.log in a finally block (keeping origLog, output, and joined logic), referencing the ListCommand instantiation and execute(testDir, 'specs', { specsPrefix: 'cli/' }) call and the origLog/console.log override to ensure cleanup even if the test throws.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openspec/specs/cli/archive/spec.md`:
- Around line 77-82: Update the spec wording that currently implies global
pre-validation ("validate all operations before applying") to state per-delta,
non-transactional semantics: change phrasing around "recursively discover delta
specs" / "parse and apply delta changes" to specify that each delta is validated
and applied independently (validate each delta before applying it, apply deltas
atomically per-delta), and explicitly note that failures in one delta do not
trigger rollback of previously applied deltas and there is no global transaction
across all discovered deltas.
In `@src/core/archive.ts`:
- Around line 119-131: The empty catches around the probe logic can hide I/O
errors and cause hasDeltaSpecs to remain false, skipping
validateChangeDeltaSpecs(); change the error handling in the outer try (around
fg(...) producing matches) and the inner try (around fs.readFile(candidatePath,
...)) so that on any error you either rethrow or mark hasDeltaSpecs = true and
break/exit to force validation; specifically update the blocks that use matches,
specId (from specIdToPath) and content so errors are not silently swallowed—log
the error (or rethrow) and set hasDeltaSpecs = true before breaking/returning so
validateChangeDeltaSpecs() runs reliably.
In `@src/core/specs-apply.ts`:
- Around line 73-77: The loop deriving specId from each match can produce an
empty string for root spec files (match like 'spec.md'), so add a guard after
computing specId (variable specId in the for...of over matches) that checks if
specId === '' and if so throw or return a clear error describing the invalid
root spec delta (including the original match and directories
changeSpecsDir/mainSpecsDir) before calling specIdToPath; this ensures you fail
fast and avoid passing an empty ID into specIdToPath.
In `@src/core/validation/validator.ts`:
- Around line 364-379: The function extractNameFromPath currently only strips
'spec.md' from the tail, causing paths like
'openspec/changes/my-change/proposal.md' to resolve to 'my-change/proposal.md';
update the logic where you handle the tail (inside extractNameFromPath) to check
the anchor segment (parts[i]) and drop the appropriate trailing filename: if the
anchor is 'specs' remove a trailing 'spec.md', and if the anchor is 'changes'
remove a trailing 'proposal.md' (or generically drop either 'spec.md' or
'proposal.md' if present), then return the remaining joined tail so change IDs
become just 'my-change'.
In `@src/utils/item-discovery.ts`:
- Around line 34-43: getSpecIds currently only guards fs.access but calls
fg('**/spec.md', { cwd: specsPath, ... }) which can throw and cause hard
failures; wrap the fast-glob call (the fg invocation that produces matches from
specsPath) in a try/catch (or wrap the body of getSpecIds) and on any error
return an empty array [] so transient/unreadable-tree I/O errors are handled
consistently with the existing fs.access check; locate the fg call and the
matches variable inside getSpecIds to apply this change.
In `@src/utils/spec-paths.ts`:
- Around line 16-24: The function pathToSpecId currently unconditionally pops
the last path segment which corrupts IDs when filePath doesn't end with
"spec.md"; update pathToSpecId to normalize the relative path (variable
relative), inspect the final segment in parts (or use path.basename) and only
remove it if it equals "spec.md" — otherwise throw a clear Error (or return the
normalized relative path unchanged) so callers know the input shape is invalid;
adjust callers/tests accordingly to handle the explicit failure.
- Around line 7-10: The specIdToPath function currently joins raw specId
segments into a filesystem path, which allows traversal via '.'/''/'..'
segments; sanitize and validate the segments before joining: split specId,
filter out empty and '.' segments, reject or throw on any '..' segment (or
otherwise escape it), then use path.join(baseDir, ...cleanSegments, 'spec.md')
and finally assert the normalized result remains within baseDir (e.g., compare
path.resolve(baseDir) with path.resolve(result).startsWith) to prevent directory
traversal; update specIdToPath accordingly.
In `@test/commands/spec.test.ts`:
- Around line 314-327: The test "should not match on partial segment boundary
(client/ should not match cli/)" has a race where fs.mkdir(...).then(...) and
fs.writeFile are not awaited before running execSync; make the test reliably
synchronous by marking the test callback async and awaiting the directory
creation and file write (await fs.promises.mkdir and await fs.promises.writeFile
or await the promise chain) prior to calling execSync(`node ${openspecBin} spec
list cli/`); ensure you reference the same identifiers used here (the test name
string, specsDir, openspecBin) so the spec file exists before the execSync
assertion runs.
---
Outside diff comments:
In `@src/core/list.ts`:
- Around line 156-193: The specs mode in execute currently always prints to
console and ignores options.json; update the branch that handles specs (the
block that builds SpecInfo[] `specs` from `allIds`, uses `specIdToPath`,
`MarkdownParser.parseSpec`, and prints "No specs found.") to mirror changes mode
JSON behavior: when `options.json` is true emit a JSON object (e.g., { specs: [{
id, requirementCount }, ...] } or an empty { specs: [] } for the no-specs case)
to stdout and return, otherwise keep the existing human-readable console output;
ensure you use the existing `specs` array and `SpecInfo` shape and handle the
empty-result path by returning the JSON empty list when `options.json` is set.
---
Nitpick comments:
In
`@openspec/changes/archive/2026-03-13-hierarchical-specs/specs/hierarchical-spec-resolution/spec.md`:
- Around line 44-45: Change the phrase "full path matching" to hyphenated
"full-path matching" in the requirement text under the heading "### Requirement:
Fuzzy matching for hierarchical IDs" so the compound modifier reads "full-path
matching" (keep "leaf-segment matching" as-is) to maintain consistent
compound-adjective style.
In `@src/cli/index.ts`:
- Around line 179-184: The code passes prefix into ListCommand.execute
unconditionally but execute only uses specsPrefix when mode === 'specs', so if a
user supplies a prefix with --changes it is silently ignored; add a user-facing
warning before calling new ListCommand().execute: detect if prefix is truthy and
mode === 'changes' and emit a concise warning (e.g., console.warn or the CLI
logger used in this file) stating the prefix will be ignored in changes mode;
keep the rest of the call intact and reference ListCommand, execute, mode, and
prefix when making the change.
In `@src/commands/spec.ts`:
- Around line 153-175: The discovery code in the .action handler uses
existsSync(specPath) defensively and silently drops missing specs, which can
hide stale/corrupt discovery or concurrent-deletion issues; update the block
that iterates allIds (references: getSpecIds, specIdToPath, SPECS_DIR,
parseSpecFromFile) to emit a warning via the existing logger (or processLogger)
when a spec ID resolves to a non-existent file (include the spec id and resolved
specPath in the message), so missing files are visible for diagnostics while
keeping the current behavior of skipping unparsable files (still catch
parseSpecFromFile errors and log them as parse failures).
In `@src/core/specs-apply.ts`:
- Around line 441-445: In applySpecs replace the manual directory creation and
file write (the block using path.dirname(p.update.target), fs.mkdir(...), and
fs.writeFile(...)) with a call to the existing writeUpdatedSpec function so all
writes and logging stay centralized; locate the code in applySpecs that writes
p.rebuilt to p.update.target and invoke writeUpdatedSpec(p.update.target,
p.rebuilt) (or the matching signature) instead, removing the duplicated
fs.mkdir/fs.writeFile logic and relying on writeUpdatedSpec’s behavior and
logging.
In `@test/specs/hierarchical-specs.integration.test.ts`:
- Around line 101-115: The test overrides console.log (origLog and output)
before calling new ListCommand().execute — wrap the call and assertions in a
try/finally so console.log is always restored: move the await cmd.execute(...)
and subsequent assertions into a try block and restore console.log in a finally
block (keeping origLog, output, and joined logic), referencing the ListCommand
instantiation and execute(testDir, 'specs', { specsPrefix: 'cli/' }) call and
the origLog/console.log override to ensure cleanup even if the test throws.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6c357154-fd50-4692-951c-e84c3ba3b353
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (44)
openspec/AGENTS.mdopenspec/changes/archive/2026-03-13-hierarchical-specs/.openspec.yamlopenspec/changes/archive/2026-03-13-hierarchical-specs/design.mdopenspec/changes/archive/2026-03-13-hierarchical-specs/proposal.mdopenspec/changes/archive/2026-03-13-hierarchical-specs/specs/cli-archive/spec.mdopenspec/changes/archive/2026-03-13-hierarchical-specs/specs/cli-list/spec.mdopenspec/changes/archive/2026-03-13-hierarchical-specs/specs/cli-show/spec.mdopenspec/changes/archive/2026-03-13-hierarchical-specs/specs/cli-spec/spec.mdopenspec/changes/archive/2026-03-13-hierarchical-specs/specs/cli-validate/spec.mdopenspec/changes/archive/2026-03-13-hierarchical-specs/specs/cli-view/spec.mdopenspec/changes/archive/2026-03-13-hierarchical-specs/specs/hierarchical-spec-discovery/spec.mdopenspec/changes/archive/2026-03-13-hierarchical-specs/specs/hierarchical-spec-resolution/spec.mdopenspec/changes/archive/2026-03-13-hierarchical-specs/tasks.mdopenspec/specs/cli/archive/spec.mdopenspec/specs/cli/artifact-workflow/spec.mdopenspec/specs/cli/change/spec.mdopenspec/specs/cli/completion/spec.mdopenspec/specs/cli/config/spec.mdopenspec/specs/cli/feedback/spec.mdopenspec/specs/cli/init/spec.mdopenspec/specs/cli/list/spec.mdopenspec/specs/cli/show/spec.mdopenspec/specs/cli/spec/spec.mdopenspec/specs/cli/update/spec.mdopenspec/specs/cli/validate/spec.mdopenspec/specs/cli/view/spec.mdopenspec/specs/hierarchical-spec-discovery/spec.mdopenspec/specs/hierarchical-spec-resolution/spec.mdsrc/cli/index.tssrc/commands/spec.tssrc/commands/validate.tssrc/core/archive.tssrc/core/list.tssrc/core/specs-apply.tssrc/core/validation/validator.tssrc/core/view.tssrc/utils/item-discovery.tssrc/utils/match.tssrc/utils/spec-paths.tstest/commands/spec.test.tstest/core/archive.test.tstest/specs/hierarchical-specs.integration.test.tstest/utils/item-discovery.test.tstest/utils/match.test.ts
✅ Files skipped from review due to trivial changes (5)
- openspec/specs/cli/validate/spec.md
- openspec/changes/archive/2026-03-13-hierarchical-specs/.openspec.yaml
- openspec/changes/archive/2026-03-13-hierarchical-specs/tasks.md
- openspec/specs/hierarchical-spec-discovery/spec.md
- openspec/AGENTS.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hierarchical Specs
This change was proposed as an OpenSpec change — see openspec/changes/hierarchical-specs/ for the full proposal, design decisions, and task breakdown.
Problem
OpenSpec stores all specs flat under openspec/specs/. As projects grow, teams encode hierarchy in flat names (cli-show, cli-validate, schema-fork-command). With 38+ specs already, this is noisy, collision-prone, and hard to navigate.
What This Changes
Spec IDs become path-based, supporting arbitrary nesting depth:
Before
openspec/specs/cli-show/spec.md → ID: cli-show
After
openspec/specs/cli/show/spec.md → ID: cli/show
Backward Compatibility
No migration required. Flat and hierarchical specs coexist — cli-show and cli/show are distinct spec IDs. Existing changes and archives continue to work unchanged.
Scope
Generated with Claude Code using claude-opus-4-6
Summary by CodeRabbit
Release Notes
New Features
cli/show).openspec spec list cli/) to view only related specs.Documentation