Clarify scaffold proposal #310
Conversation
WalkthroughThe scaffold command behavior is specified: it validates change IDs, copies a default template bundle (proposal, tasks, design) into Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as openspec scaffold
participant FS as FileSystem
participant Validator
User->>CLI: run scaffold <change-id>
CLI->>Validator: validate change-id
Validator-->>CLI: valid / invalid
alt invalid
CLI-->>User: exit with error (invalid id)
else valid
CLI->>FS: check if openspec/changes/<id>/ exists
FS-->>CLI: exists / not exists
alt exists
CLI-->>User: exit with error (directory exists)
else not exists
CLI->>FS: copy template bundle -> openspec/changes/<id>/
CLI->>FS: create openspec/changes/<id>/specs/
CLI->>Validator: run openspec validate --strict on new change
Validator-->>CLI: pass / fail
CLI-->>User: success / validation error
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🪛 LanguageToolopenspec/changes/add-scaffold-command/specs/cli-scaffold/spec.md[style] ~28-~28: Consider using the synonym “brief” (= concise, using a few words, not lasting long) to strengthen your wording. (QUICK_BRIEF) 🔇 Additional comments (1)
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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
openspec/changes/add-scaffold-command/specs/cli-scaffold/spec.md (1)
12-12: Clarify the phrasing of "directories" when describing the change workspace structure.Line 12 states "proposal, tasks, optional design, and
specs/directories" which is grammatically awkward and unclear. It reads as if there are multiple directories at the root level, when the intent is to describe file types plus onespecs/directory.Recommend: "The scaffold command SHALL create the standard change workspace with proposal, tasks, and optional design files, plus a
specs/directory, all laid out according to OpenSpec conventions."
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
openspec/changes/add-scaffold-command/proposal.md(1 hunks)openspec/changes/add-scaffold-command/specs/cli-scaffold/spec.md(2 hunks)openspec/changes/add-scaffold-command/tasks.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
openspec/changes/add-scaffold-command/specs/cli-scaffold/spec.md
[style] ~28-~28: Consider using the synonym “brief” (= concise, using a few words, not lasting long) to strengthen your wording.
Context: ...keep or delete them - AND include a short reminder inside specs/README.md (or s...
(QUICK_BRIEF)
🔇 Additional comments (3)
openspec/changes/add-scaffold-command/proposal.md (1)
5-7: LGTM conditional on resolving thespecs/directory inconsistency.The proposal clearly describes the scaffold command behavior, documentation updates, and test coverage. However, the proposal should align with the resolution of the specs/README.md inconsistency flagged in tasks.md (line 3 vs. spec.md line 18). Once that is clarified, this proposal accurately summarizes the feature.
Verify that proposal.md aligns with the final decision on whether
specs/README.mdis included in the template bundle.openspec/changes/add-scaffold-command/specs/cli-scaffold/spec.md (1)
30-36: Directory collision handling requirement is clear and well‑specified.The error handling scenario (lines 30–36) is concrete, testable, and correctly specifies that no files should be modified when the target directory exists. This protects against accidental overwrites.
openspec/changes/add-scaffold-command/tasks.md (1)
3-4: The claimed inconsistency does not exist in the codebase.Both
tasks.mdandspec.mdare consistent:tasks.mdline 1.2 listsspecs/README.mdas part of the copied template bundle, andspec.mdconfirms this by requiring "a short reminder insidespecs/README.md" in the template. The word "empty" inspec.mdrefers to the absence of capability-specific deltas (which users add later), not the absence of files. The template bundle includesspecs/README.mdwith guidance text, and thespecs/directory starts with no user deltas—both interpretations align correctly.Likely an incorrect or invalid review comment.
| - **AND** copy the default template bundle (proposal, tasks, design placeholders) into that directory in a single operation | ||
| - **AND** create an empty `openspec/changes/add-user-notifications/specs/` directory ready for capability-specific deltas that will be authored later |
There was a problem hiding this comment.
Resolve inconsistency: Does the template bundle include specs/README.md or is specs/ created empty?
- Line 18 states: "create an empty
openspec/changes/add-user-notifications/specs/directory ready for capability-specific deltas" - Line 28 states: "include a short reminder inside
specs/README.md(or similar) instructing authors to add deltas"
These two statements conflict. If specs/ is truly empty (line 18), where does specs/README.md come from (line 28)?
Options to resolve:
- If the template includes
specs/README.md: Update line 18 to remove "empty" and clarify that the directory is created with a README file. - If
specs/is truly empty: Move thespecs/README.mdguidance to a separate manual step (after scaffolding), or update the requirement scope.
Also, per static analysis hint: Consider updating "short reminder" (line 28) to "brief reminder" for stronger wording.
Also applies to: 28-28
🤖 Prompt for AI Agents
In openspec/changes/add-scaffold-command/specs/cli-scaffold/spec.md around lines
17–18 there is a conflict: the scaffold is described as creating an "empty"
specs/ directory but later (line 28) references a specs/README.md; resolve by
choosing one of two fixes — Option A: if the template bundle should include a
README, change line 18 to remove "empty" and state the specs/ directory is
created with a brief README (also change "short reminder" on line 28 to "brief
reminder"); Option B: if the specs/ directory is truly empty, remove or relocate
the specs/README.md guidance from line 28 to a separate post-scaffold/manual
step and update line 28 wording to "brief reminder" if that guidance remains
elsewhere.
|
LGTM |
* clarify scaffold proposal * Update scaffold command proposal to support idempotent execution
* clarify scaffold proposal * Update scaffold command proposal to support idempotent execution
Align the add-scaffold-command proposal/spec/tasks with the latest decisions: command copies the template bundle, errors when the change directory exists, and defers spec deltas while documentation and tests cover the new workflow.
Summary by CodeRabbit
New Features
Documentation
Tests