feat: list cmd support json for specs and archive#700
feat: list cmd support json for specs and archive#700RandyZ wants to merge 5 commits intoFission-AI:mainfrom
Conversation
ignore cursor
Enable JSON output for specs and add an archive listing mode to `openspec list`. Updated CLI to accept `--archive` and compute mode precedence (archive > specs > changes). ListCommand (src/core/list.ts) now supports mode `'archive'`, emits `{ "specs": [...] }` for `--specs --json` and `{ "archivedChanges": [...] }` for `--archive --json` (archived items reuse change fields: name, completedTasks, totalTasks, lastModified, status). Documentation and specs were added/updated (docs/cli.md, openspec/specs/cli-list/spec.md) along with design/proposal/tasks metadata. Tests (test/core/list.test.ts) were extended to cover specs JSON, archive listing, JSON shapes, empty states, and mode precedence.
Move the add-list-specs-json-and-archive change files into a dated archive folder at openspec/changes/archive/2026-02-12-add-list-specs-json-and-archive/. Files moved: .openspec.yaml, design.md, proposal.md, specs/cli-list/spec.md, and tasks.md. This archives the change entry to keep active changes cleaned up.
📝 WalkthroughWalkthroughThis PR adds a new --archive mode and expands the openspec list command to support JSON output for specs (including an optional --detail) and archived changes, introduces mode precedence (archive > specs > changes), updates CLI flags and ListCommand.execute signature, and adds tests and docs for these behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User (CLI)"
participant CLI as "src/cli/index.ts"
participant List as "ListCommand (src/core/list.ts)"
participant FS as "Filesystem (openspec/changes, openspec/specs)"
participant Output as "Stdout / JSON"
User->>CLI: run `openspec list [--specs|--archive] [--json] [--detail]`
CLI->>List: execute(path='.', mode, {sort, json, detail})
List->>FS: read openspec/changes or openspec/specs or openspec/changes/archive
FS-->>List: return dirs/files + tasks.md + timestamps
List->>List: compute progress, lastModified, status, (spec titles if specs mode)
List->>Output: render table or emit JSON { specs: [...] } / { archivedChanges: [...] }
Output-->>User: display results / exit 0
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 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)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
Greptile OverviewGreptile SummaryThis PR adds JSON output support for Major changes:
Critical issue:
Confidence Score: 1/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI
participant ListCommand
participant FileSystem
User->>CLI: openspec list --archive --json
CLI->>CLI: Parse flags (archive > specs > changes)
CLI->>ListCommand: execute(path, 'archive', {json: true, sort: 'recent'})
ListCommand->>FileSystem: access(openspec/changes/archive/)
alt Directory exists
FileSystem-->>ListCommand: Success
ListCommand->>FileSystem: readdir(archive/)
FileSystem-->>ListCommand: [dir1, dir2, ...]
loop For each directory
ListCommand->>FileSystem: Read tasks.md
FileSystem-->>ListCommand: Task content
ListCommand->>ListCommand: Parse task progress
ListCommand->>FileSystem: Get last modified time
FileSystem-->>ListCommand: Timestamp
end
ListCommand->>ListCommand: Sort by recent/name
ListCommand->>ListCommand: Format JSON output
ListCommand->>User: {"archivedChanges": [...]}
else Directory missing
FileSystem-->>ListCommand: Error
ListCommand->>User: {"archivedChanges": []}
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -183,12 +191,83 @@ export class ListCommand { | |||
| } | |||
|
|
|||
| specs.sort((a, b) => a.id.localeCompare(b.id)); | |||
| if (json) { | |||
| console.log(JSON.stringify({ specs }, null, 2)); | |||
| return; | |||
| } | |||
| console.log('Specs:'); | |||
| const padding = ' '; | |||
| const nameWidth = Math.max(...specs.map(s => s.id.length)); | |||
| for (const spec of specs) { | |||
| const padded = spec.id.padEnd(nameWidth); | |||
| console.log(`${padding}${padded} requirements ${spec.requirementCount}`); | |||
| } | |||
| return; | |||
| } | |||
|
|
|||
| // archive mode | |||
| const archiveDir = path.join(targetPath, 'openspec', 'changes', 'archive'); | |||
| try { | |||
| await fs.access(archiveDir); | |||
| } catch { | |||
| if (json) { | |||
| console.log(JSON.stringify({ archivedChanges: [] }, null, 2)); | |||
| } else { | |||
| console.log('No archived changes found.'); | |||
| } | |||
| return; | |||
| } | |||
|
|
|||
| const entries = await fs.readdir(archiveDir, { withFileTypes: true }); | |||
| const archiveDirs = entries.filter(e => e.isDirectory()).map(e => e.name); | |||
| if (archiveDirs.length === 0) { | |||
| if (json) { | |||
| console.log(JSON.stringify({ archivedChanges: [] }, null, 2)); | |||
| } else { | |||
| console.log('No archived changes found.'); | |||
| } | |||
| return; | |||
| } | |||
|
|
|||
| const changes: ChangeInfo[] = []; | |||
| for (const changeDir of archiveDirs) { | |||
| const progress = await getTaskProgressForChange(archiveDir, changeDir); | |||
| const changePath = path.join(archiveDir, changeDir); | |||
| const lastModified = await getLastModified(changePath); | |||
| changes.push({ | |||
| name: changeDir, | |||
| completedTasks: progress.completed, | |||
| totalTasks: progress.total, | |||
| lastModified | |||
| }); | |||
| } | |||
|
|
|||
| if (sort === 'recent') { | |||
| changes.sort((a, b) => b.lastModified.getTime() - a.lastModified.getTime()); | |||
| } else { | |||
| changes.sort((a, b) => a.name.localeCompare(b.name)); | |||
| } | |||
|
|
|||
| if (json) { | |||
| const jsonOutput = changes.map(c => ({ | |||
| name: c.name, | |||
| completedTasks: c.completedTasks, | |||
| totalTasks: c.totalTasks, | |||
| lastModified: c.lastModified.toISOString(), | |||
| status: c.totalTasks === 0 ? 'no-tasks' : c.completedTasks === c.totalTasks ? 'complete' : 'in-progress' | |||
| })); | |||
| console.log(JSON.stringify({ archivedChanges: jsonOutput }, null, 2)); | |||
| return; | |||
| } | |||
|
|
|||
| console.log('Archived changes:'); | |||
| const padding = ' '; | |||
| const nameWidth = Math.max(...changes.map(c => c.name.length)); | |||
| for (const change of changes) { | |||
| const paddedName = change.name.padEnd(nameWidth); | |||
| const status = formatTaskStatus({ total: change.totalTasks, completed: change.completedTasks }); | |||
| const timeAgo = formatRelativeTime(change.lastModified); | |||
| console.log(`${padding}${paddedName} ${status.padEnd(12)} ${timeAgo}`); | |||
| } | |||
| } | |||
There was a problem hiding this comment.
Indentation broken for specs and archive mode blocks - these should be at the same level as the changes mode block, not nested
| if (mode === 'specs') { | |
| const specsDir = path.join(targetPath, 'openspec', 'specs'); | |
| try { | |
| await fs.access(specsDir); | |
| } catch { | |
| if (json) { | |
| console.log(JSON.stringify({ specs: [] }, null, 2)); | |
| } else { | |
| console.log('No specs found.'); | |
| } | |
| return; | |
| } | |
| const entries = await fs.readdir(specsDir, { withFileTypes: true }); | |
| const specDirs = entries.filter(e => e.isDirectory()).map(e => e.name); | |
| if (specDirs.length === 0) { | |
| if (json) { | |
| console.log(JSON.stringify({ specs: [] }, null, 2)); | |
| } else { | |
| console.log('No specs found.'); | |
| } | |
| return; | |
| } | |
| type SpecInfo = { id: string; requirementCount: number }; | |
| const specs: SpecInfo[] = []; | |
| for (const id of specDirs) { | |
| const specPath = join(specsDir, id, 'spec.md'); | |
| try { | |
| const content = readFileSync(specPath, 'utf-8'); | |
| const parser = new MarkdownParser(content); | |
| const spec = parser.parseSpec(id); | |
| specs.push({ id, requirementCount: spec.requirements.length }); | |
| } catch { | |
| // If spec cannot be read or parsed, include with 0 count | |
| specs.push({ id, requirementCount: 0 }); | |
| } | |
| } | |
| specs.sort((a, b) => a.id.localeCompare(b.id)); | |
| if (json) { | |
| console.log(JSON.stringify({ specs }, null, 2)); | |
| return; | |
| } | |
| console.log('Specs:'); | |
| const padding = ' '; | |
| const nameWidth = Math.max(...specs.map(s => s.id.length)); | |
| for (const spec of specs) { | |
| const padded = spec.id.padEnd(nameWidth); | |
| console.log(`${padding}${padded} requirements ${spec.requirementCount}`); | |
| } | |
| return; | |
| } | |
| // archive mode | |
| const archiveDir = path.join(targetPath, 'openspec', 'changes', 'archive'); | |
| try { | |
| await fs.access(archiveDir); | |
| } catch { | |
| if (json) { | |
| console.log(JSON.stringify({ archivedChanges: [] }, null, 2)); | |
| } else { | |
| console.log('No archived changes found.'); | |
| } | |
| return; | |
| } | |
| const entries = await fs.readdir(archiveDir, { withFileTypes: true }); | |
| const archiveDirs = entries.filter(e => e.isDirectory()).map(e => e.name); | |
| if (archiveDirs.length === 0) { | |
| if (json) { | |
| console.log(JSON.stringify({ archivedChanges: [] }, null, 2)); | |
| } else { | |
| console.log('No archived changes found.'); | |
| } | |
| return; | |
| } | |
| const changes: ChangeInfo[] = []; | |
| for (const changeDir of archiveDirs) { | |
| const progress = await getTaskProgressForChange(archiveDir, changeDir); | |
| const changePath = path.join(archiveDir, changeDir); | |
| const lastModified = await getLastModified(changePath); | |
| changes.push({ | |
| name: changeDir, | |
| completedTasks: progress.completed, | |
| totalTasks: progress.total, | |
| lastModified | |
| }); | |
| } | |
| if (sort === 'recent') { | |
| changes.sort((a, b) => b.lastModified.getTime() - a.lastModified.getTime()); | |
| } else { | |
| changes.sort((a, b) => a.name.localeCompare(b.name)); | |
| } | |
| if (json) { | |
| const jsonOutput = changes.map(c => ({ | |
| name: c.name, | |
| completedTasks: c.completedTasks, | |
| totalTasks: c.totalTasks, | |
| lastModified: c.lastModified.toISOString(), | |
| status: c.totalTasks === 0 ? 'no-tasks' : c.completedTasks === c.totalTasks ? 'complete' : 'in-progress' | |
| })); | |
| console.log(JSON.stringify({ archivedChanges: jsonOutput }, null, 2)); | |
| return; | |
| } | |
| console.log('Archived changes:'); | |
| const padding = ' '; | |
| const nameWidth = Math.max(...changes.map(c => c.name.length)); | |
| for (const change of changes) { | |
| const paddedName = change.name.padEnd(nameWidth); | |
| const status = formatTaskStatus({ total: change.totalTasks, completed: change.completedTasks }); | |
| const timeAgo = formatRelativeTime(change.lastModified); | |
| console.log(`${padding}${paddedName} ${status.padEnd(12)} ${timeAgo}`); | |
| } | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/core/list.ts (3)
1-6: 🛠️ Refactor suggestion | 🟠 MajorDuplicate and mixed imports:
path/joinand sync / asyncfs.
pathis imported as a default on Line 2, andjoinis destructured from the same module on Line 5. Similarly, asyncfs(promises) is imported on Line 1 while syncreadFileSyncis imported on Line 4. This creates unnecessary duplication and mixes I/O paradigms.Use
path.joinconsistently (already used everywhere except Line 181) and drop the redundant{ join }import. Preferawait fs.readFileoverreadFileSyncto avoid blocking (see related comment on Line 183).♻️ Proposed import cleanup
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';
180-191:⚠️ Potential issue | 🟠 MajorSynchronous file read inside an
asyncmethod blocks the event loop.
readFileSyncon Line 183 is called inside anasync execute()method that otherwise uses async I/O throughout. When iterating over many spec directories, this blocks the thread for each file read. Switch toawait fs.readFileand update Line 181 to usepath.joinfor consistency.🔧 Proposed fix
for (const id of specDirs) { - const specPath = join(specsDir, id, 'spec.md'); + const specPath = path.join(specsDir, id, 'spec.md'); try { - const content = readFileSync(specPath, 'utf-8'); + const content = await fs.readFile(specPath, 'utf-8'); const parser = new MarkdownParser(content); const spec = parser.parseSpec(id); specs.push({ id, requirementCount: spec.requirements.length });
154-206: 🛠️ Refactor suggestion | 🟠 MajorInconsistent indentation in
specsandarchiveblocks.The
specsblock (Lines 156–205) drops to 4-space indent despite being inside theif (mode === 'specs')block, and thearchiveblock (Lines 210–271) drops to 2-space indent. This makes the code hard to follow and fragile—an editor auto-format could silently shift brace matching.Please re-indent the body of both blocks to align consistently with the containing
if/ method scope.Also applies to: 208-271
🤖 Fix all issues with AI agents
In `@docs/cli.md`:
- Around line 197-203: The fenced code block under the "Output (text):" section
lacks a language specifier which triggers markdownlint MD040; update the opening
fence to include a language identifier (e.g., change ``` to ```text) for the
block containing "Changes: add-dark-mode ..." so the snippet is explicitly
marked as text and the linter error is resolved.
In `@src/core/list.ts`:
- Around line 208-230: The archive mode currently checks only archiveDir
(variable archiveDir) and falls back to "No archived changes found" when the
parent openspec/changes directory is missing; update the logic to first validate
the parent changes directory (e.g., a changesDir derived from targetPath +
'openspec/changes') using fs.access and, if that check fails, emit the same
descriptive init prompt used in changes mode instead of the empty-archive
message; only if changesDir exists should you then check archiveDir and keep the
existing behavior of returning an empty archivedChanges response when the
archive subdirectory is absent.
🧹 Nitpick comments (6)
test/core/list.test.ts (1)
285-302: Consider adding a sort test for archive mode.The precedence test is a great addition. You might also want a test that verifies
--sort nameworks for archive mode (alphabetical ordering of archived changes), similar to the existingsort=nametest for changes at line 119. This would confirm the documented sort behavior for archive mode.openspec/changes/archive/2026-02-12-add-list-specs-json-and-archive/specs/cli-list/spec.md (1)
134-136: Specs mode sorting is implemented and correct, but tests lack explicit ordering verification.The implementation at line 193 of
src/core/list.tscorrectly sorts specs alphabetically by id usinglocaleCompare, which satisfies the requirement. However, the test suite does not include an explicit test case that verifies this alphabetical ordering for specs mode. Consider adding a test similar to the one at line 119-135 (which verifies alphabetical ordering for changes mode) to explicitly confirm specs ordering is maintained.openspec/specs/cli-list/spec.md (2)
22-25: Implementation detail in spec:path.joinreference.Line 24 prescribes a specific Node.js API (
path.join) for path resolution. Specs typically describe what (cross-platform path resolution) rather than how (which library call). Consider rewording to "using platform-appropriate path joining" so the spec stays implementation-agnostic.
67-68: Duplicated precedence rule.The mode precedence rule is stated identically in the "Flags" requirement (Line 68) and again as a standalone "Mode precedence" scenario (Lines 82-84). Having it in one place and referencing it from the other would reduce drift risk if the rule ever changes.
Also applies to: 82-84
src/core/list.ts (2)
128-138: Duplicated status derivation and JSON-mapping logic betweenchangesandarchivemodes.Lines 130–136 (changes JSON) and Lines 252–258 (archive JSON) contain identical mapping logic including the same ternary for
status. The human-readable display (Lines 142–150 vs 263–271) is also nearly identical. Consider extracting a shared helper (e.g.,toJsonOutput(changes)andprintChangeTable(label, changes)) to keep these in sync and reduce copy-paste drift.♻️ Example helper extraction
function toChangeJsonOutput(changes: ChangeInfo[]) { return changes.map(c => ({ name: c.name, completedTasks: c.completedTasks, totalTasks: c.totalTasks, lastModified: c.lastModified.toISOString(), status: c.totalTasks === 0 ? 'no-tasks' : c.completedTasks === c.totalTasks ? 'complete' : 'in-progress', })); } function printChangeTable(heading: string, changes: ChangeInfo[]) { console.log(`${heading}:`); const padding = ' '; const nameWidth = Math.max(...changes.map(c => c.name.length)); for (const change of changes) { const paddedName = change.name.padEnd(nameWidth); const status = formatTaskStatus({ total: change.totalTasks, completed: change.completedTasks }); const timeAgo = formatRelativeTime(change.lastModified); console.log(`${padding}${paddedName} ${status.padEnd(12)} ${timeAgo}`); } }Also applies to: 251-261
24-51:getLastModifiedperforms unbounded recursive traversal.This function walks every file in the directory tree to find the latest
mtime. For archive entries with many files this could become slow. Not a blocking issue for this PR, but worth noting for future consideration—a shallow scan (e.g., justtasks.mdmtime or directory mtime) may suffice.
| **Output (text):** | ||
|
|
||
| ``` | ||
| Active changes: | ||
| add-dark-mode UI theme switching support | ||
| fix-login-bug Session timeout handling | ||
| Changes: | ||
| add-dark-mode 2/5 tasks 2h ago | ||
| fix-login-bug ✓ Complete 1d ago | ||
| ``` |
There was a problem hiding this comment.
Fenced code block missing language specifier.
The static analysis tool (markdownlint MD040) flags this code block. Add a language identifier for consistency with the rest of the file.
Proposed fix
**Output (text):**
-```
+```text
Changes:
add-dark-mode 2/5 tasks 2h ago
fix-login-bug ✓ Complete 1d ago</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.20.0)</summary>
[warning] 199-199: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @docs/cli.md around lines 197 - 203, The fenced code block under the "Output
(text):" section lacks a language specifier which triggers markdownlint MD040;
update the opening fence to include a language identifier (e.g., change ``` to
explicitly marked as text and the linter error is resolved.
| // archive mode | ||
| const archiveDir = path.join(targetPath, 'openspec', 'changes', 'archive'); | ||
| try { | ||
| await fs.access(archiveDir); | ||
| } catch { | ||
| if (json) { | ||
| console.log(JSON.stringify({ archivedChanges: [] }, null, 2)); | ||
| } else { | ||
| console.log('No archived changes found.'); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| const entries = await fs.readdir(archiveDir, { withFileTypes: true }); | ||
| const archiveDirs = entries.filter(e => e.isDirectory()).map(e => e.name); | ||
| if (archiveDirs.length === 0) { | ||
| if (json) { | ||
| console.log(JSON.stringify({ archivedChanges: [] }, null, 2)); | ||
| } else { | ||
| console.log('No archived changes found.'); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
Archive mode doesn't validate that the changes directory exists first.
In changes mode (Line 82–89), if openspec/changes/ doesn't exist, a descriptive error is thrown prompting the user to run openspec init. In archive mode, the code jumps straight to checking the archive subdirectory. If openspec/changes/ itself doesn't exist, fs.access(archiveDir) will fail and silently report "No archived changes found" instead of the more helpful init prompt. Consider checking the parent changes directory first, or at minimum documenting why the behavior diverges.
🤖 Prompt for AI Agents
In `@src/core/list.ts` around lines 208 - 230, The archive mode currently checks
only archiveDir (variable archiveDir) and falls back to "No archived changes
found" when the parent openspec/changes directory is missing; update the logic
to first validate the parent changes directory (e.g., a changesDir derived from
targetPath + 'openspec/changes') using fs.access and, if that check fails, emit
the same descriptive init prompt used in changes mode instead of the
empty-archive message; only if changesDir exists should you then check
archiveDir and keep the existing behavior of returning an empty archivedChanges
response when the archive subdirectory is absent.
|
I was actually working on a change to do exactly the same thing, but after seeing this PR I’ll leave it to you. Would you consider adding a This would make it easier for the LLM to discover and select relevant specs. |
Good idea, I will support this in my PR. The source of the title is not clearly specified in openspec at the moment. The name used during parseSpec is actually the file ID. This is indeed somewhat unreasonable. |
It would be great, just one consideration. Specs might use a custom schema that doesn’t include the title in the first “#” (i.e., # Spec Title), so having a fallback to the spec ID in that case would be helpful. The same applies to the description section — it might not use ## Purpose, but something like ## Overview, ## Description, or even just the paragraphs immediately following the title. |
Introduce a required Spec.title field sourced from the document's first level-1 heading (H1) with fallback to the spec id when missing. Update MarkdownParser.parseSpec to extract the H1 and return title, add SPEC_TITLE_EMPTY validation message and enforce title in SpecSchema. Add a new --detail flag to the list CLI and ListCommand so that openspec list --specs --json remains unchanged by default but openspec list --specs --json --detail includes title and overview for each spec. Update deprecated spec command outputs to use parsed title, adjust tests to cover H1/fallback behavior and the new detail JSON, and add design/proposal/specs documentation for the change.
The new commit is already supported, but I merged two parameters. Using |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
test/core/validation.test.ts (1)
87-87: Consider adding a test case for empty/missingtitlevalidation.The
titlefield is now required and validated withSPEC_TITLE_EMPTY, but there's no test that asserts a spec with an empty or missing title is rejected bySpecSchema. The existing tests only add a valid title to passing and other-failure cases.💡 Suggested test case
it('should reject spec with empty title', () => { const spec = { name: 'user-auth', title: '', overview: 'This spec defines user authentication requirements', requirements: [ { text: 'The system SHALL provide user authentication', scenarios: [ { rawText: 'Given a user with valid credentials\nWhen they submit the login form\nThen they are authenticated', }, ], }, ], }; const result = SpecSchema.safeParse(spec); expect(result.success).toBe(false); if (!result.success) { expect(result.error.issues[0].message).toBe('Spec title cannot be empty'); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/core/validation.test.ts` at line 87, Add a unit test verifying SpecSchema rejects empty or missing title: create a spec object with name 'user-auth' and title set to '' (and optionally another test omitting title entirely), call SpecSchema.safeParse(spec), assert result.success is false, and verify the validation error message/issue corresponds to SPEC_TITLE_EMPTY (or message 'Spec title cannot be empty'); reference SpecSchema and SPEC_TITLE_EMPTY in the assertion to ensure the correct validation path is tested.test/commands/spec.test.ts (1)
94-104: This test largely duplicates the test at lines 74–92.Both tests invoke
spec show auth --jsonand assertjson.title === 'auth'andjson.id === 'auth'. The only difference is thecwdoption vsprocess.chdirpattern. Consider consolidating or removing the duplicate to keep the test suite focused.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/commands/spec.test.ts` around lines 94 - 104, The test "should output title from parsed spec (spec show --json)" duplicates an earlier test that runs `spec show auth --json` and asserts title/id; remove or consolidate them so only one authoritative test remains (either keep the earlier test that uses process.chdir or keep this one that uses execSync with cwd), and if consolidating merge any unique assertions into the kept test; locate the duplicate by the test name and the use of execSync, openspecBin and testDir, then delete the redundant `it(...)` block or combine its assertions into the other `it(...)` block.src/core/list.ts (2)
182-196: Mixing sync and async file I/O in an async method.
readFileSync(Line 184) is used inside anasyncmethod, while the rest of the method (and the changes/archive modes) consistently usesawait fs.readFile(...). For a small number of spec files this won't be noticeable, but it blocks the event loop during each read and is inconsistent with the pattern used elsewhere in this method.♻️ Suggested change
- const content = readFileSync(specPath, 'utf-8'); + const content = await fs.readFile(specPath, 'utf-8');This also lets you drop the
import { readFileSync } from 'fs'andimport { join } from 'path'(use the already-importedpath.join).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/list.ts` around lines 182 - 196, The code is blocking the event loop by using readFileSync inside an async method; replace the synchronous read with an asynchronous read (use await fs.readFile(specPath, 'utf-8')) when building specPath (still using path.join(specsDir, id, 'spec.md')), then pass the resulting content into new MarkdownParser(content) and parseSpec(id) as before; keep the existing try/catch and the fallback specs.push({ id, requirementCount: 0 }), and remove the readFileSync import since it will no longer be used.
238-277: Significant duplication between archive and changes modes.The archive block (Lines 238–277) is nearly identical to the changes block (Lines 108–151): gather
ChangeInfo[]viagetTaskProgressForChange+getLastModified, sort, format JSON with the same status derivation, and render the same table layout. The only differences are the directory path, the JSON root key (archivedChangesvschanges), and the header string.Consider extracting a shared helper, e.g.:
async function gatherChanges(dir: string, dirNames: string[]): Promise<ChangeInfo[]> { ... } function formatChangesJson(changes: ChangeInfo[], key: string): string { ... } function printChangesTable(changes: ChangeInfo[], header: string): void { ... }This would eliminate ~40 duplicated lines and make future changes (e.g., adding fields) apply in one place.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/list.ts` around lines 238 - 277, Extract the duplicated logic into three helpers and replace both the archive and changes blocks with calls to them: implement async function gatherChanges(dir: string, dirNames: string[]): Promise<ChangeInfo[]> which iterates dirNames, calls getTaskProgressForChange and getLastModified and returns the ChangeInfo[]; implement function formatChangesJson(changes: ChangeInfo[], rootKey: string): string which maps status exactly as current code does and returns JSON.stringify({ [rootKey]: mapped }, null, 2); and implement function printChangesTable(changes: ChangeInfo[], header: string): void which sorts (by recent or name based on an argument you pass), computes nameWidth, formats status via formatTaskStatus and prints the header and rows. Replace the existing archive-specific block and the changes-specific block to call gatherChanges(...) then either console.log(formatChangesJson(changes, 'archivedChanges'|'changes')) or printChangesTable(changes, 'Archived changes:'|'Changes:') as appropriate so only the directory path, JSON root key, and header differ.test/core/list.test.ts (1)
309-326: Consider adding a symmetric precedence test for specs > changes.The test validates
archive > specsprecedence via CLI. For completeness, consider adding a test that verifiesspecs > changeswhen both--specsand--changesflags are passed, to fully exercise the documented precedence chain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/core/list.test.ts` around lines 309 - 326, Add a symmetric CLI test that verifies specs take precedence over changes when both flags are passed: create a new it(...) in the same test suite (mirroring the existing "should apply mode precedence so archive wins over specs when both flags passed (CLI)" test) that sets up a specs folder with a spec (e.g., under tempDir/openspec/specs/some-spec/spec.md) and a changes archive folder (e.g., tempDir/openspec/changes/archive/some-change), then call runCLI(['list','--specs','--changes','--json'], { cwd: tempDir }) and assert exitCode === 0 and that the parsed JSON contains the specs result (e.g., check parsed.specs or that the spec name exists) rather than changes; use the same helpers/variables (runCLI, tempDir) and naming convention as the existing test to keep consistency.
🤖 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/archive/2025-02-12-add-spec-title-and-list-json-fields/specs/openspec-conventions/spec.md`:
- Around line 1-17: Update the archive directory name used in this change from
"2025-02-12-add-spec-title-and-list-json-fields" to
"2026-02-12-add-spec-title-and-list-json-fields" so it matches the PR creation
year; rename the directory and adjust any references (e.g., links or index
entries) that mention the old directory name to the new one to preserve
consistency with the YYYY-MM-DD-[name] convention.
In `@openspec/specs/cli-list/spec.md`:
- Around line 54-64: The new list path produces JSON without title unless
--detail, while the deprecated spec list always includes title; to avoid
surprising migrations either document the difference or make the new command
match the old behavior — to align behavior, update the JSON construction in
src/core/list.ts (the block that builds the specs array for openspec list
--specs --json) to include the same display title extraction used by
src/commands/spec.ts (reuse its title/overview extraction helper) so each spec
object always contains id, requirementCount and title (and include overview only
under --detail if you want to keep that difference); alternately add a clear
note in the CLI help/docs explaining the intentional difference between openspec
spec list --json and openspec list --specs --json.
In `@src/core/list.ts`:
- Around line 155-212: The block guarded by if (mode === 'specs') has its inner
statements (the try/catch around specsDir access, dir reading, loop over
specDirs, sorting and printing) incorrectly unindented; re-indent all code
inside the if (mode === 'specs') { ... } so the try/catch, const entries = await
fs.readdir(...), the for (const id of specDirs) loop that reads spec.md, and the
final return are visually nested under that if and not at top level—apply the
same re-indentation fix to the archive block so its contents are nested under
its mode check (look for the archive-mode conditional and re-indent its
try/catch and processing code accordingly).
---
Duplicate comments:
In `@src/core/list.ts`:
- Around line 214-225: The archive mode only checks archiveDir (archiveDir =
path.join(targetPath, 'openspec', 'changes', 'archive')) and falls back to "No
archived changes found." instead of validating the parent changes directory;
update the logic to first check changesDir = path.join(targetPath, 'openspec',
'changes') and if that parent is missing, emit the same init prompt/response
used by changes mode (the behavior at the changes-mode initialization) rather
than the generic "No archived changes found." message; if changesDir exists then
continue to check archiveDir as currently implemented.
---
Nitpick comments:
In `@src/core/list.ts`:
- Around line 182-196: The code is blocking the event loop by using readFileSync
inside an async method; replace the synchronous read with an asynchronous read
(use await fs.readFile(specPath, 'utf-8')) when building specPath (still using
path.join(specsDir, id, 'spec.md')), then pass the resulting content into new
MarkdownParser(content) and parseSpec(id) as before; keep the existing try/catch
and the fallback specs.push({ id, requirementCount: 0 }), and remove the
readFileSync import since it will no longer be used.
- Around line 238-277: Extract the duplicated logic into three helpers and
replace both the archive and changes blocks with calls to them: implement async
function gatherChanges(dir: string, dirNames: string[]): Promise<ChangeInfo[]>
which iterates dirNames, calls getTaskProgressForChange and getLastModified and
returns the ChangeInfo[]; implement function formatChangesJson(changes:
ChangeInfo[], rootKey: string): string which maps status exactly as current code
does and returns JSON.stringify({ [rootKey]: mapped }, null, 2); and implement
function printChangesTable(changes: ChangeInfo[], header: string): void which
sorts (by recent or name based on an argument you pass), computes nameWidth,
formats status via formatTaskStatus and prints the header and rows. Replace the
existing archive-specific block and the changes-specific block to call
gatherChanges(...) then either console.log(formatChangesJson(changes,
'archivedChanges'|'changes')) or printChangesTable(changes, 'Archived
changes:'|'Changes:') as appropriate so only the directory path, JSON root key,
and header differ.
In `@test/commands/spec.test.ts`:
- Around line 94-104: The test "should output title from parsed spec (spec show
--json)" duplicates an earlier test that runs `spec show auth --json` and
asserts title/id; remove or consolidate them so only one authoritative test
remains (either keep the earlier test that uses process.chdir or keep this one
that uses execSync with cwd), and if consolidating merge any unique assertions
into the kept test; locate the duplicate by the test name and the use of
execSync, openspecBin and testDir, then delete the redundant `it(...)` block or
combine its assertions into the other `it(...)` block.
In `@test/core/list.test.ts`:
- Around line 309-326: Add a symmetric CLI test that verifies specs take
precedence over changes when both flags are passed: create a new it(...) in the
same test suite (mirroring the existing "should apply mode precedence so archive
wins over specs when both flags passed (CLI)" test) that sets up a specs folder
with a spec (e.g., under tempDir/openspec/specs/some-spec/spec.md) and a changes
archive folder (e.g., tempDir/openspec/changes/archive/some-change), then call
runCLI(['list','--specs','--changes','--json'], { cwd: tempDir }) and assert
exitCode === 0 and that the parsed JSON contains the specs result (e.g., check
parsed.specs or that the spec name exists) rather than changes; use the same
helpers/variables (runCLI, tempDir) and naming convention as the existing test
to keep consistency.
In `@test/core/validation.test.ts`:
- Line 87: Add a unit test verifying SpecSchema rejects empty or missing title:
create a spec object with name 'user-auth' and title set to '' (and optionally
another test omitting title entirely), call SpecSchema.safeParse(spec), assert
result.success is false, and verify the validation error message/issue
corresponds to SPEC_TITLE_EMPTY (or message 'Spec title cannot be empty');
reference SpecSchema and SPEC_TITLE_EMPTY in the assertion to ensure the correct
validation path is tested.
...es/archive/2025-02-12-add-spec-title-and-list-json-fields/specs/openspec-conventions/spec.md
Show resolved
Hide resolved
| #### Scenario: JSON output for specs | ||
| - **WHEN** `openspec list --specs --json` is executed without `--detail` | ||
| - **THEN** output a JSON object with key `specs` and an array of objects with `id` and `requirementCount` only | ||
| - **AND** output `{ "specs": [] }` when no specs exist | ||
|
|
||
| #### Scenario: JSON output for specs with detail | ||
| - **WHEN** `openspec list --specs --json --detail` is executed | ||
| - **THEN** output a JSON object with key `specs` and an array of objects with `id`, `requirementCount`, `title`, and `overview` | ||
| - **AND** `title` SHALL be the spec's display title (from document H1 or spec id) | ||
| - **AND** `overview` SHALL be the spec's Purpose section content | ||
| - **AND** output `{ "specs": [] }` when no specs exist |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the test file to see what the deprecated spec list --json outputs
find . -name "spec.test.ts" -type f 2>/dev/null | head -5Repository: Fission-AI/OpenSpec
Length of output: 91
🏁 Script executed:
# Check the implementation files
fd -t f "list\.(ts|js)" src/ 2>/dev/null | head -10Repository: Fission-AI/OpenSpec
Length of output: 79
🏁 Script executed:
# Also look for the spec command implementation
fd -t f "spec\.(ts|js)" src/commands/ 2>/dev/nullRepository: Fission-AI/OpenSpec
Length of output: 83
🏁 Script executed:
# Let's examine the test file around lines 203-205
cat -n test/commands/spec.test.ts | sed -n '195,215p'Repository: Fission-AI/OpenSpec
Length of output: 968
🏁 Script executed:
# Now let's check the list.ts implementation to see how it constructs the output
cat -n src/core/list.ts | head -150Repository: Fission-AI/OpenSpec
Length of output: 5984
🏁 Script executed:
# Also check spec.ts for the deprecated command handling
cat -n src/commands/spec.ts | head -100Repository: Fission-AI/OpenSpec
Length of output: 4252
🏁 Script executed:
# Let's see the specs mode implementation in list.ts
cat -n src/core/list.ts | sed -n '150,300p'Repository: Fission-AI/OpenSpec
Length of output: 5109
🏁 Script executed:
# Let's also check if there's a deprecated spec list handler in spec.ts
cat -n src/commands/spec.ts | sed -n '100,200p'Repository: Fission-AI/OpenSpec
Length of output: 4385
🏁 Script executed:
# Search for where the deprecated spec list --json is actually implemented
rg -n "spec list" src/commands/ -A 5 -B 5Repository: Fission-AI/OpenSpec
Length of output: 45
Clarify the intentional output difference between deprecated spec list --json and new openspec list --specs --json commands.
The deprecated openspec spec list --json command (in src/commands/spec.ts lines 162) always includes title in the output. The new openspec list --specs --json command (in src/core/list.ts lines 188-191) includes title only when --detail is passed. This is an intentional design difference between the two command paths, but it may confuse users migrating from the deprecated to the new command. Consider documenting this distinction explicitly or aligning the behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/specs/cli-list/spec.md` around lines 54 - 64, The new list path
produces JSON without title unless --detail, while the deprecated spec list
always includes title; to avoid surprising migrations either document the
difference or make the new command match the old behavior — to align behavior,
update the JSON construction in src/core/list.ts (the block that builds the
specs array for openspec list --specs --json) to include the same display title
extraction used by src/commands/spec.ts (reuse its title/overview extraction
helper) so each spec object always contains id, requirementCount and title (and
include overview only under --detail if you want to keep that difference);
alternately add a clear note in the CLI help/docs explaining the intentional
difference between openspec spec list --json and openspec list --specs --json.
| if (mode === 'specs') { | ||
| const specsDir = path.join(targetPath, 'openspec', 'specs'); | ||
| try { | ||
| await fs.access(specsDir); | ||
| } catch { | ||
| console.log('No specs found.'); | ||
| if (json) { | ||
| console.log(JSON.stringify({ specs: [] }, null, 2)); | ||
| } else { | ||
| console.log('No specs found.'); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| const entries = await fs.readdir(specsDir, { withFileTypes: true }); | ||
| const specDirs = entries.filter(e => e.isDirectory()).map(e => e.name); | ||
| if (specDirs.length === 0) { | ||
| console.log('No specs found.'); | ||
| if (json) { | ||
| console.log(JSON.stringify({ specs: [] }, null, 2)); | ||
| } else { | ||
| console.log('No specs found.'); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| type SpecInfo = { id: string; requirementCount: number }; | ||
| type SpecInfo = { id: string; requirementCount: number; title?: string; overview?: string }; | ||
| const specs: SpecInfo[] = []; | ||
| for (const id of specDirs) { | ||
| const specPath = join(specsDir, id, 'spec.md'); | ||
| try { | ||
| const content = readFileSync(specPath, 'utf-8'); | ||
| const parser = new MarkdownParser(content); | ||
| const spec = parser.parseSpec(id); | ||
| specs.push({ id, requirementCount: spec.requirements.length }); | ||
| const entry: SpecInfo = { id, requirementCount: spec.requirements.length }; | ||
| if (json && detail) { | ||
| entry.title = spec.title; | ||
| entry.overview = spec.overview; | ||
| } | ||
| specs.push(entry); | ||
| } catch { | ||
| // If spec cannot be read or parsed, include with 0 count | ||
| specs.push({ id, requirementCount: 0 }); | ||
| } | ||
| } | ||
|
|
||
| specs.sort((a, b) => a.id.localeCompare(b.id)); | ||
| if (json) { | ||
| console.log(JSON.stringify({ specs }, null, 2)); | ||
| return; | ||
| } | ||
| console.log('Specs:'); | ||
| const padding = ' '; | ||
| const nameWidth = Math.max(...specs.map(s => s.id.length)); | ||
| for (const spec of specs) { | ||
| const padded = spec.id.padEnd(nameWidth); | ||
| console.log(`${padding}${padded} requirements ${spec.requirementCount}`); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inconsistent indentation inside the specs mode block.
The try/catch at Line 157 and the code through Line 211 are indented at the wrong level — they sit at the method-body level rather than inside the if (mode === 'specs') block. The closing } at Line 212 and the return at Line 211 restore control flow correctness, but the visual misalignment makes it hard to follow the nesting and will trip up future maintainers.
The same issue affects the archive block (Lines 216–277).
♻️ Suggested fix: re-indent specs and archive blocks
Indent all code within if (mode === 'specs') { ... } and the archive block by one additional level (2 or 4 spaces, matching project convention) so nesting is visually consistent with the changes block above.
if (mode === 'specs') {
const specsDir = path.join(targetPath, 'openspec', 'specs');
- try {
- await fs.access(specsDir);
- } catch {
- if (json) {
- console.log(JSON.stringify({ specs: [] }, null, 2));
- } else {
- console.log('No specs found.');
- }
- return;
- }
+ try {
+ await fs.access(specsDir);
+ } catch {
+ if (json) {
+ console.log(JSON.stringify({ specs: [] }, null, 2));
+ } else {
+ console.log('No specs found.');
+ }
+ return;
+ }
// ... (apply same re-indentation to the rest of the specs block)
}Apply the same correction to the archive block (Lines 216–277).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/core/list.ts` around lines 155 - 212, The block guarded by if (mode ===
'specs') has its inner statements (the try/catch around specsDir access, dir
reading, loop over specDirs, sorting and printing) incorrectly unindented;
re-indent all code inside the if (mode === 'specs') { ... } so the try/catch,
const entries = await fs.readdir(...), the for (const id of specDirs) loop that
reads spec.md, and the final return are visually nested under that if and not at
top level—apply the same re-indentation fix to the archive block so its contents
are nested under its mode check (look for the archive-mode conditional and
re-indent its try/catch and processing code accordingly).
2 participants
Suggested English PR description:
Add JSON output for
list --specsandlist --archivefor archived changesSummary
openspec list --specs --jsonnow returns JSON:{ "specs": [ { "id", "requirementCount" } ] }instead of the human-readable table. Empty result is{ "specs": [] }.openspec list --archivelists archived changes (directories underopenspec/changes/archive/). It supports--sort(recent | name) and--json; with--jsonit outputs{ "archivedChanges": [...] }with the same shape as active changes (name, completedTasks, totalTasks, lastModified, status).--changes,--specs, or--archiveis passed, the effective mode is:--archiveover--specsover default (changes).Motivation
list --specs --jsonwas ignored and only the table was printed.openspec/changes/archive/manually.Changes
--archiveoption onopenspec list; mode resolution with precedence (archive > specs > changes).ListCommandinsrc/core/list.ts: specs branch outputs JSON whenoptions.jsonis true; newarchivemode scansopenspec/changes/archive/, reuses task progress and last-modified, supports--sortand--json.openspec/specs/cli-list/spec.mdupdated with JSON output for specs, archive mode, precedence, and empty-archive behavior.test/core/list.test.tsforlist --specs --json(shape, empty),list --archive/list --archive --json(shape, empty state), and CLI precedence (--specs --archive→ archive list).docs/cli.md: list section updated with--archive, mode/JSON note, and examples for--archive,--specs --json, and--archive --json.Testing
pnpm run build && pnpm exec vitest run test/core/list.test.ts— all 17 list tests pass.openspec list --specs --json,openspec list --archive --json, andopenspec list --archiveproduce the expected output.Summary by CodeRabbit
New Features
Documentation
Tests
Chores