Skip to content

Conversation

@yha9806
Copy link

@yha9806 yha9806 commented Nov 5, 2025

Summary

Fixes #269: Git doesn't track empty directories, which causes the specs/, changes/, and changes/archive/ directories created by openspec init to disappear after commit. This breaks the workflow for new team members who clone the repository.

Changes

  • Modified src/core/init.ts createDirectoryStructure method to create .gitkeep files in empty directories
  • Added 3 comprehensive test cases:
    • Verify .gitkeep files exist in all three directories
    • Verify .gitkeep files are empty
    • Ensure directory structure is maintained
  • Created OpenSpec proposal following the framework: add-gitkeep-empty-folders

Testing

✅ All 3 new tests pass
✅ No regressions in existing test suite (267/268 tests passing, 1 pre-existing Windows-specific failure unrelated to changes)
✅ Manually tested with openspec init --tools none - verified .gitkeep files created
✅ OpenSpec proposal validated with --strict mode

Impact

  • Users Affected: All users who run openspec init
  • Breaking Changes: None
  • Migration: Not required - existing projects work as-is, but benefit from re-running openspec init in extend mode

Result

After this change:

  • Empty directories persist through Git operations
  • openspec list works immediately after cloning
  • No manual directory creation required by team members

OpenSpec Proposal

Created a complete OpenSpec proposal at openspec/changes/add-gitkeep-empty-folders/ following the project's spec-driven development workflow.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Init now creates .gitkeep placeholders so empty specs, changes, and archive folders are preserved in Git and usable after clones.
  • Tests

    • Added tests verifying those placeholder files are created and are empty after initialization.
  • Documentation

    • CLI Init docs updated to describe empty-directory persistence and expected post-clone behavior.
  • Chores

    • Adjusted install/build-related behavior and changed repository ignore behavior for build output.

Git doesn't track empty directories, causing workflow issues when team
members clone a repository initialized with OpenSpec. The specs/,
changes/, and changes/archive/ directories would disappear, breaking
commands like 'openspec list'.

Changes:
- Modified src/core/init.ts to create .gitkeep files in empty directories
- Added 3 test cases verifying .gitkeep file creation and content
- Created OpenSpec proposal: add-gitkeep-empty-folders

All 3 new tests pass. The change ensures that empty directories persist
through Git operations, allowing 'openspec list' to work immediately
after cloning.

Fixes Fission-AI#269

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@yha9806 yha9806 requested a review from TabishB as a code owner November 5, 2025 16:11
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 5, 2025

Walkthrough

Creates three empty .gitkeep files (specs/.gitkeep, changes/.gitkeep, changes/archive/.gitkeep) during openspec initialization; adds tests and documentation describing these placeholder files. Also removes the prepare script from package.json and changes the .gitignore entry for dist/ to a commented explanatory line.

Changes

Cohort / File(s) Summary
Implementation
src/core/init.ts
Adds writes for specs/.gitkeep, changes/.gitkeep, and changes/archive/.gitkeep as part of the init directory creation flow.
Tests
test/core/init.test.ts
Adds assertions that the three .gitkeep files exist and contain empty-string content after running InitCommand.
Proposal & Feature Spec
openspec/changes/archive/2025-11-05-add-gitkeep-empty-folders/proposal.md, openspec/changes/archive/2025-11-05-add-gitkeep-empty-folders/specs/cli-init/spec.md, openspec/specs/cli-init/spec.md
Documents the rationale, required directory layout including .gitkeep placeholders, acceptance criteria, and migration guidance (re-run init in extend mode).
Tasks
openspec/changes/archive/2025-11-05-add-gitkeep-empty-folders/tasks.md
Adds implementation and validation checklist for .gitkeep creation and tests.
Repo metadata
package.json, .gitignore
Removes the prepare script from package.json. Replaces dist/ ignore with a commented explanatory line in .gitignore (no functional dist/ ignore).

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant InitCmd as InitCommand
  participant FS as FileSystem
  rect rgba(221,235,247,0.6)
    Note over InitCmd,FS: init directory creation (new .gitkeep writes)
    User->>InitCmd: run "openspec init"
    InitCmd->>FS: create directories (openspec/, specs/, changes/, changes/archive/)
    InitCmd->>FS: write `specs/.gitkeep` (empty)
    InitCmd->>FS: write `changes/.gitkeep` (empty)
    InitCmd->>FS: write `changes/archive/.gitkeep` (empty)
    FS-->>InitCmd: success
    InitCmd-->>User: initialization complete
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Small, focused code changes plus tests and documentation.
  • Review attention points:
    • Confirm exact file paths and filenames (specs/.gitkeep, changes/.gitkeep, changes/archive/.gitkeep) match tests and docs.
    • Verify .gitkeep files are written as empty strings and error handling remains unchanged.
    • Check that removal of the prepare script in package.json is intentional and that .gitignore comment for dist/ is correct.

Suggested reviewers

  • TabishB

Poem

🐇
I hopped where folders once were thin,
Planted three small files so they’d stay in.
Specs and changes now proudly keep,
Little .gitkeep seeds in a Git-tracked heap. 🥕📁

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Changes to package.json (removed prepare script) and .gitignore (commented out dist/) appear unrelated to the .gitkeep feature and may represent scope creep requiring clarification. Clarify whether package.json and .gitignore changes are intentional for this PR or should be reverted to separate commits; confirm they don't introduce unintended side effects.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately and concisely summarizes the main change: adding .gitkeep files to preserve empty directories, directly addressing the linked issue #269.
Linked Issues check ✅ Passed All coding requirements from issue #269 are met: .gitkeep files are created in specs/, changes/, and changes/archive/ directories, ensuring Git tracks empty folders and they persist through commits and clones.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2f715d1 and ca78a3b.

⛔ Files ignored due to path filters (264)
  • dist/cli/index.d.ts is excluded by !**/dist/**
  • dist/cli/index.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/cli/index.js is excluded by !**/dist/**
  • dist/cli/index.js.map is excluded by !**/dist/**, !**/*.map
  • dist/commands/change.d.ts is excluded by !**/dist/**
  • dist/commands/change.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/commands/change.js is excluded by !**/dist/**
  • dist/commands/change.js.map is excluded by !**/dist/**, !**/*.map
  • dist/commands/show.d.ts is excluded by !**/dist/**
  • dist/commands/show.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/commands/show.js is excluded by !**/dist/**
  • dist/commands/show.js.map is excluded by !**/dist/**, !**/*.map
  • dist/commands/spec.d.ts is excluded by !**/dist/**
  • dist/commands/spec.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/commands/spec.js is excluded by !**/dist/**
  • dist/commands/spec.js.map is excluded by !**/dist/**, !**/*.map
  • dist/commands/validate.d.ts is excluded by !**/dist/**
  • dist/commands/validate.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/commands/validate.js is excluded by !**/dist/**
  • dist/commands/validate.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/archive.d.ts is excluded by !**/dist/**
  • dist/core/archive.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/archive.js is excluded by !**/dist/**
  • dist/core/archive.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/config.d.ts is excluded by !**/dist/**
  • dist/core/config.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/config.js is excluded by !**/dist/**
  • dist/core/config.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/agents.d.ts is excluded by !**/dist/**
  • dist/core/configurators/agents.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/agents.js is excluded by !**/dist/**
  • dist/core/configurators/agents.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/base.d.ts is excluded by !**/dist/**
  • dist/core/configurators/base.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/base.js is excluded by !**/dist/**
  • dist/core/configurators/base.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/claude.d.ts is excluded by !**/dist/**
  • dist/core/configurators/claude.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/claude.js is excluded by !**/dist/**
  • dist/core/configurators/claude.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/cline.d.ts is excluded by !**/dist/**
  • dist/core/configurators/cline.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/cline.js is excluded by !**/dist/**
  • dist/core/configurators/cline.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/codebuddy.d.ts is excluded by !**/dist/**
  • dist/core/configurators/codebuddy.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/codebuddy.js is excluded by !**/dist/**
  • dist/core/configurators/codebuddy.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/costrict.d.ts is excluded by !**/dist/**
  • dist/core/configurators/costrict.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/costrict.js is excluded by !**/dist/**
  • dist/core/configurators/costrict.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/qoder.d.ts is excluded by !**/dist/**
  • dist/core/configurators/qoder.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/qoder.js is excluded by !**/dist/**
  • dist/core/configurators/qoder.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/qwen.d.ts is excluded by !**/dist/**
  • dist/core/configurators/qwen.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/qwen.js is excluded by !**/dist/**
  • dist/core/configurators/qwen.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/registry.d.ts is excluded by !**/dist/**
  • dist/core/configurators/registry.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/registry.js is excluded by !**/dist/**
  • dist/core/configurators/registry.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/amazon-q.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/amazon-q.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/amazon-q.js is excluded by !**/dist/**
  • dist/core/configurators/slash/amazon-q.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/auggie.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/auggie.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/auggie.js is excluded by !**/dist/**
  • dist/core/configurators/slash/auggie.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/base.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/base.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/base.js is excluded by !**/dist/**
  • dist/core/configurators/slash/base.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/claude.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/claude.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/claude.js is excluded by !**/dist/**
  • dist/core/configurators/slash/claude.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/cline.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/cline.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/cline.js is excluded by !**/dist/**
  • dist/core/configurators/slash/cline.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/codebuddy.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/codebuddy.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/codebuddy.js is excluded by !**/dist/**
  • dist/core/configurators/slash/codebuddy.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/codex.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/codex.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/codex.js is excluded by !**/dist/**
  • dist/core/configurators/slash/codex.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/costrict.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/costrict.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/costrict.js is excluded by !**/dist/**
  • dist/core/configurators/slash/costrict.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/crush.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/crush.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/crush.js is excluded by !**/dist/**
  • dist/core/configurators/slash/crush.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/cursor.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/cursor.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/cursor.js is excluded by !**/dist/**
  • dist/core/configurators/slash/cursor.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/factory.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/factory.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/factory.js is excluded by !**/dist/**
  • dist/core/configurators/slash/factory.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/github-copilot.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/github-copilot.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/github-copilot.js is excluded by !**/dist/**
  • dist/core/configurators/slash/github-copilot.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/kilocode.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/kilocode.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/kilocode.js is excluded by !**/dist/**
  • dist/core/configurators/slash/kilocode.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/opencode.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/opencode.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/opencode.js is excluded by !**/dist/**
  • dist/core/configurators/slash/opencode.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/qoder.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/qoder.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/qoder.js is excluded by !**/dist/**
  • dist/core/configurators/slash/qoder.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/qwen.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/qwen.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/qwen.js is excluded by !**/dist/**
  • dist/core/configurators/slash/qwen.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/registry.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/registry.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/registry.js is excluded by !**/dist/**
  • dist/core/configurators/slash/registry.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/windsurf.d.ts is excluded by !**/dist/**
  • dist/core/configurators/slash/windsurf.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/configurators/slash/windsurf.js is excluded by !**/dist/**
  • dist/core/configurators/slash/windsurf.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/converters/json-converter.d.ts is excluded by !**/dist/**
  • dist/core/converters/json-converter.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/converters/json-converter.js is excluded by !**/dist/**
  • dist/core/converters/json-converter.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/index.d.ts is excluded by !**/dist/**
  • dist/core/index.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/index.js is excluded by !**/dist/**
  • dist/core/index.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/init.d.ts is excluded by !**/dist/**
  • dist/core/init.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/init.js is excluded by !**/dist/**
  • dist/core/init.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/list.d.ts is excluded by !**/dist/**
  • dist/core/list.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/list.js is excluded by !**/dist/**
  • dist/core/list.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/parsers/change-parser.d.ts is excluded by !**/dist/**
  • dist/core/parsers/change-parser.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/parsers/change-parser.js is excluded by !**/dist/**
  • dist/core/parsers/change-parser.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/parsers/markdown-parser.d.ts is excluded by !**/dist/**
  • dist/core/parsers/markdown-parser.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/parsers/markdown-parser.js is excluded by !**/dist/**
  • dist/core/parsers/markdown-parser.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/parsers/requirement-blocks.d.ts is excluded by !**/dist/**
  • dist/core/parsers/requirement-blocks.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/parsers/requirement-blocks.js is excluded by !**/dist/**
  • dist/core/parsers/requirement-blocks.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/schemas/base.schema.d.ts is excluded by !**/dist/**
  • dist/core/schemas/base.schema.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/schemas/base.schema.js is excluded by !**/dist/**
  • dist/core/schemas/base.schema.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/schemas/change.schema.d.ts is excluded by !**/dist/**
  • dist/core/schemas/change.schema.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/schemas/change.schema.js is excluded by !**/dist/**
  • dist/core/schemas/change.schema.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/schemas/index.d.ts is excluded by !**/dist/**
  • dist/core/schemas/index.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/schemas/index.js is excluded by !**/dist/**
  • dist/core/schemas/index.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/schemas/spec.schema.d.ts is excluded by !**/dist/**
  • dist/core/schemas/spec.schema.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/schemas/spec.schema.js is excluded by !**/dist/**
  • dist/core/schemas/spec.schema.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/styles/palette.d.ts is excluded by !**/dist/**
  • dist/core/styles/palette.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/styles/palette.js is excluded by !**/dist/**
  • dist/core/styles/palette.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/agents-root-stub.d.ts is excluded by !**/dist/**
  • dist/core/templates/agents-root-stub.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/agents-root-stub.js is excluded by !**/dist/**
  • dist/core/templates/agents-root-stub.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/agents-template.d.ts is excluded by !**/dist/**
  • dist/core/templates/agents-template.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/agents-template.js is excluded by !**/dist/**
  • dist/core/templates/agents-template.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/claude-template.d.ts is excluded by !**/dist/**
  • dist/core/templates/claude-template.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/claude-template.js is excluded by !**/dist/**
  • dist/core/templates/claude-template.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/cline-template.d.ts is excluded by !**/dist/**
  • dist/core/templates/cline-template.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/cline-template.js is excluded by !**/dist/**
  • dist/core/templates/cline-template.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/costrict-template.d.ts is excluded by !**/dist/**
  • dist/core/templates/costrict-template.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/costrict-template.js is excluded by !**/dist/**
  • dist/core/templates/costrict-template.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/index.d.ts is excluded by !**/dist/**
  • dist/core/templates/index.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/index.js is excluded by !**/dist/**
  • dist/core/templates/index.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/project-template.d.ts is excluded by !**/dist/**
  • dist/core/templates/project-template.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/project-template.js is excluded by !**/dist/**
  • dist/core/templates/project-template.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/slash-command-templates.d.ts is excluded by !**/dist/**
  • dist/core/templates/slash-command-templates.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/templates/slash-command-templates.js is excluded by !**/dist/**
  • dist/core/templates/slash-command-templates.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/update.d.ts is excluded by !**/dist/**
  • dist/core/update.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/update.js is excluded by !**/dist/**
  • dist/core/update.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/validation/constants.d.ts is excluded by !**/dist/**
  • dist/core/validation/constants.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/validation/constants.js is excluded by !**/dist/**
  • dist/core/validation/constants.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/validation/types.d.ts is excluded by !**/dist/**
  • dist/core/validation/types.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/validation/types.js is excluded by !**/dist/**
  • dist/core/validation/types.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/validation/validator.d.ts is excluded by !**/dist/**
  • dist/core/validation/validator.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/validation/validator.js is excluded by !**/dist/**
  • dist/core/validation/validator.js.map is excluded by !**/dist/**, !**/*.map
  • dist/core/view.d.ts is excluded by !**/dist/**
  • dist/core/view.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/core/view.js is excluded by !**/dist/**
  • dist/core/view.js.map is excluded by !**/dist/**, !**/*.map
  • dist/index.d.ts is excluded by !**/dist/**
  • dist/index.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/index.js is excluded by !**/dist/**
  • dist/index.js.map is excluded by !**/dist/**, !**/*.map
  • dist/utils/file-system.d.ts is excluded by !**/dist/**
  • dist/utils/file-system.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/utils/file-system.js is excluded by !**/dist/**
  • dist/utils/file-system.js.map is excluded by !**/dist/**, !**/*.map
  • dist/utils/index.d.ts is excluded by !**/dist/**
  • dist/utils/index.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/utils/index.js is excluded by !**/dist/**
  • dist/utils/index.js.map is excluded by !**/dist/**, !**/*.map
  • dist/utils/interactive.d.ts is excluded by !**/dist/**
  • dist/utils/interactive.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/utils/interactive.js is excluded by !**/dist/**
  • dist/utils/interactive.js.map is excluded by !**/dist/**, !**/*.map
  • dist/utils/item-discovery.d.ts is excluded by !**/dist/**
  • dist/utils/item-discovery.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/utils/item-discovery.js is excluded by !**/dist/**
  • dist/utils/item-discovery.js.map is excluded by !**/dist/**, !**/*.map
  • dist/utils/match.d.ts is excluded by !**/dist/**
  • dist/utils/match.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/utils/match.js is excluded by !**/dist/**
  • dist/utils/match.js.map is excluded by !**/dist/**, !**/*.map
  • dist/utils/task-progress.d.ts is excluded by !**/dist/**
  • dist/utils/task-progress.d.ts.map is excluded by !**/dist/**, !**/*.map
  • dist/utils/task-progress.js is excluded by !**/dist/**
  • dist/utils/task-progress.js.map is excluded by !**/dist/**, !**/*.map
📒 Files selected for processing (2)
  • .gitignore (1 hunks)
  • package.json (0 hunks)
💤 Files with no reviewable changes (1)
  • package.json
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/changes/*/{proposal.md,tasks.md,design.md,specs/**/spec.md} : Scaffold each change with proposal.md, tasks.md, optional design.md, and delta spec files under openspec/changes/<change-id>/specs/<capability>/spec.md
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/specs/**/spec.md : Keep current, deployed specs in openspec/specs/<capability>/spec.md (one focused capability per directory)
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/changes/archive/** : Archive completed changes under openspec/changes/archive/YYYY-MM-DD-<name>/ and run 'openspec validate --strict' post-archive; update specs/ if capabilities changed
🔇 Additional comments (1)
.gitignore (1)

84-85: The review comment is incorrect—dist/ tracking is an intentional, standard npm distribution pattern.

The git history confirms this with commit message "feat: commit dist directory for npm install from git". The package.json explicitly controls distribution via the "files" field: ["dist", "bin", "!dist//*.test.js", "!dist//tests", "!dist/**/*.map"]. This allows users installing from GitHub directly to get pre-compiled code without rebuilding, which is standard practice for npm packages. The change is not scope creep.

Likely an incorrect or invalid review comment.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md (1)

11-21: Minor: Add language specifier to fenced code block.

The fenced code block is missing a language identifier. While the directory structure doesn't require syntax highlighting, adding a language specifier satisfies markdown linting rules.

Apply this diff to address the static analysis hint:

-```
+```text
 openspec/
 ├── project.md
 ├── AGENTS.md
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d32e50f and a359909.

📒 Files selected for processing (5)
  • openspec/changes/add-gitkeep-empty-folders/proposal.md (1 hunks)
  • openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md (1 hunks)
  • openspec/changes/add-gitkeep-empty-folders/tasks.md (1 hunks)
  • src/core/init.ts (1 hunks)
  • test/core/init.test.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
openspec/changes/*/{proposal.md,tasks.md,design.md,specs/**/spec.md}

📄 CodeRabbit inference engine (openspec/AGENTS.md)

Scaffold each change with proposal.md, tasks.md, optional design.md, and delta spec files under openspec/changes//specs//spec.md

Files:

  • openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md
  • openspec/changes/add-gitkeep-empty-folders/proposal.md
  • openspec/changes/add-gitkeep-empty-folders/tasks.md
openspec/changes/*/**

📄 CodeRabbit inference engine (openspec/AGENTS.md)

Choose a unique kebab-case, verb-led change-id (add-, update-, remove-, refactor-) for openspec/changes//

Files:

  • openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md
  • openspec/changes/add-gitkeep-empty-folders/proposal.md
  • openspec/changes/add-gitkeep-empty-folders/tasks.md
openspec/{specs/**/spec.md,changes/*/specs/**/spec.md}

📄 CodeRabbit inference engine (openspec/AGENTS.md)

openspec/{specs/**/spec.md,changes/*/specs/**/spec.md}: Write requirements using SHALL/MUST for normative wording
Every requirement must include at least one scenario, formatted exactly with a '#### Scenario: ' header (no bullets or bold)
Under ADDED, introduce new capabilities as standalone requirements rather than altering existing ones

Files:

  • openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md
openspec/changes/*/specs/**/spec.md

📄 CodeRabbit inference engine (openspec/AGENTS.md)

openspec/changes/*/specs/**/spec.md: Use ADDED, MODIFIED, REMOVED, or RENAMED sections when authoring deltas; place changed requirements under the correct operation header
For MODIFIED requirements, paste the full existing requirement block (header through scenarios) and ensure the header text matches exactly (whitespace-insensitive)
Use RENAMED when only the requirement name changes; if behavior changes too, use RENAMED plus MODIFIED referencing the new name

Files:

  • openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md
openspec/changes/*/tasks.md

📄 CodeRabbit inference engine (openspec/AGENTS.md)

Track implementation steps in tasks.md and mark each completed item with '- [x]' once finished

Files:

  • openspec/changes/add-gitkeep-empty-folders/tasks.md
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/changes/*/{proposal.md,tasks.md,design.md,specs/**/spec.md} : Scaffold each change with proposal.md, tasks.md, optional design.md, and delta spec files under openspec/changes/<change-id>/specs/<capability>/spec.md
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/specs/**/spec.md : Keep current, deployed specs in openspec/specs/<capability>/spec.md (one focused capability per directory)
📚 Learning: 2025-10-22T06:05:06.615Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/changes/*/{proposal.md,tasks.md,design.md,specs/**/spec.md} : Scaffold each change with proposal.md, tasks.md, optional design.md, and delta spec files under openspec/changes/<change-id>/specs/<capability>/spec.md

Applied to files:

  • openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md
  • openspec/changes/add-gitkeep-empty-folders/proposal.md
  • openspec/changes/add-gitkeep-empty-folders/tasks.md
  • src/core/init.ts
📚 Learning: 2025-10-22T06:05:06.615Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/specs/**/spec.md : Keep current, deployed specs in openspec/specs/<capability>/spec.md (one focused capability per directory)

Applied to files:

  • openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md
  • openspec/changes/add-gitkeep-empty-folders/proposal.md
  • openspec/changes/add-gitkeep-empty-folders/tasks.md
📚 Learning: 2025-10-22T06:05:06.615Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/{specs/**/spec.md,changes/*/specs/**/spec.md} : Under ADDED, introduce new capabilities as standalone requirements rather than altering existing ones

Applied to files:

  • openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md
  • openspec/changes/add-gitkeep-empty-folders/proposal.md
  • openspec/changes/add-gitkeep-empty-folders/tasks.md
📚 Learning: 2025-10-22T06:05:06.615Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : Use ADDED, MODIFIED, REMOVED, or RENAMED sections when authoring deltas; place changed requirements under the correct operation header

Applied to files:

  • openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md
📚 Learning: 2025-10-22T06:05:06.615Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/changes/archive/** : Archive completed changes under openspec/changes/archive/YYYY-MM-DD-<name>/ and run 'openspec validate --strict' post-archive; update specs/ if capabilities changed

Applied to files:

  • openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md
  • openspec/changes/add-gitkeep-empty-folders/proposal.md
  • openspec/changes/add-gitkeep-empty-folders/tasks.md
📚 Learning: 2025-10-22T06:05:06.615Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : For MODIFIED requirements, paste the full existing requirement block (header through scenarios) and ensure the header text matches exactly (whitespace-insensitive)

Applied to files:

  • openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md
📚 Learning: 2025-10-22T06:05:06.615Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/{specs/**/spec.md,changes/*/specs/**/spec.md} : Every requirement must include at least one scenario, formatted exactly with a '#### Scenario: <name>' header (no bullets or bold)

Applied to files:

  • openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md
📚 Learning: 2025-10-22T06:05:06.615Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/{specs/**/spec.md,changes/*/specs/**/spec.md} : Write requirements using SHALL/MUST for normative wording

Applied to files:

  • openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md
📚 Learning: 2025-10-22T06:05:06.615Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/project.md : Document project conventions in openspec/project.md and review it before starting work

Applied to files:

  • openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md
📚 Learning: 2025-10-22T06:05:06.615Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/changes/*/specs/**/spec.md : Use RENAMED when only the requirement name changes; if behavior changes too, use RENAMED plus MODIFIED referencing the new name

Applied to files:

  • openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md
📚 Learning: 2025-10-22T06:05:06.615Z
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/changes/*/tasks.md : Track implementation steps in tasks.md and mark each completed item with '- [x]' once finished

Applied to files:

  • openspec/changes/add-gitkeep-empty-folders/tasks.md
🧬 Code graph analysis (3)
openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md (3)
test/cli-e2e/basic.test.ts (1)
  • projectDir (102-115)
test/commands/change.interactive-validate.test.ts (1)
  • fs (16-21)
test/commands/spec.test.ts (1)
  • originalCwd (160-175)
test/core/init.test.ts (1)
src/utils/file-system.ts (1)
  • fileExists (76-86)
src/core/init.ts (1)
src/utils/file-system.ts (1)
  • FileSystemUtils (44-187)
🪛 markdownlint-cli2 (0.18.1)
openspec/changes/add-gitkeep-empty-folders/specs/cli-init/spec.md

11-11: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🔇 Additional comments (3)
test/core/init.test.ts (1)

82-106: LGTM! Excellent test coverage for .gitkeep files.

The two new test cases comprehensively verify both the existence and content of .gitkeep files in all three target directories. The tests follow the existing test patterns and integrate well with the test suite.

src/core/init.ts (1)

719-729: LGTM! Clean implementation of .gitkeep file creation.

The implementation correctly creates .gitkeep files in all three target directories using the existing FileSystemUtils.writeFile utility. The placement within createDirectoryStructure ensures these files are created both during fresh initialization and extend mode, which is appropriate behavior.

openspec/changes/add-gitkeep-empty-folders/proposal.md (1)

1-31: LGTM! Well-articulated proposal.

The proposal clearly identifies the problem (Git not tracking empty directories), provides a standard solution (.gitkeep files), and accurately assesses the impact. The document structure follows OpenSpec conventions and provides sufficient detail for implementation.

Comment on lines +1 to +19
# Tasks

## Implementation
- [ ] Modify `src/core/init.ts` `createDirectoryStructure` method to create `.gitkeep` files in empty directories
- [ ] Add `.gitkeep` file creation for `openspec/specs/`
- [ ] Add `.gitkeep` file creation for `openspec/changes/`
- [ ] Add `.gitkeep` file creation for `openspec/changes/archive/`

## Testing
- [ ] Add test case verifying `.gitkeep` exists in `specs/` after init
- [ ] Add test case verifying `.gitkeep` exists in `changes/` after init
- [ ] Add test case verifying `.gitkeep` exists in `changes/archive/` after init
- [ ] Run existing test suite to ensure no regressions

## Validation
- [ ] Run `openspec validate add-gitkeep-empty-folders --strict` and resolve all issues
- [ ] Test `openspec init` manually in a fresh directory
- [ ] Verify Git tracks the empty directories with `.gitkeep` files
- [ ] Test `openspec list` works immediately after clone in a new repository
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Mark completed tasks with [x] checkboxes.

Based on the PR description, all implementation, testing, and validation work is complete:

  • Implementation: src/core/init.ts modified to create .gitkeep files
  • Testing: Three new tests added and passing in test/core/init.test.ts
  • Validation: Manual verification confirmed, proposal validated with --strict mode

All tasks should be marked as completed using - [x] syntax to accurately reflect the work status.

As per coding guidelines

Apply this diff to mark all completed tasks:

 ## Implementation
-- [ ] Modify `src/core/init.ts` `createDirectoryStructure` method to create `.gitkeep` files in empty directories
-- [ ] Add `.gitkeep` file creation for `openspec/specs/`
-- [ ] Add `.gitkeep` file creation for `openspec/changes/`
-- [ ] Add `.gitkeep` file creation for `openspec/changes/archive/`
+- [x] Modify `src/core/init.ts` `createDirectoryStructure` method to create `.gitkeep` files in empty directories
+- [x] Add `.gitkeep` file creation for `openspec/specs/`
+- [x] Add `.gitkeep` file creation for `openspec/changes/`
+- [x] Add `.gitkeep` file creation for `openspec/changes/archive/`
 
 ## Testing
-- [ ] Add test case verifying `.gitkeep` exists in `specs/` after init
-- [ ] Add test case verifying `.gitkeep` exists in `changes/` after init
-- [ ] Add test case verifying `.gitkeep` exists in `changes/archive/` after init
-- [ ] Run existing test suite to ensure no regressions
+- [x] Add test case verifying `.gitkeep` exists in `specs/` after init
+- [x] Add test case verifying `.gitkeep` exists in `changes/` after init
+- [x] Add test case verifying `.gitkeep` exists in `changes/archive/` after init
+- [x] Run existing test suite to ensure no regressions
 
 ## Validation
-- [ ] Run `openspec validate add-gitkeep-empty-folders --strict` and resolve all issues
-- [ ] Test `openspec init` manually in a fresh directory
-- [ ] Verify Git tracks the empty directories with `.gitkeep` files
-- [ ] Test `openspec list` works immediately after clone in a new repository
+- [x] Run `openspec validate add-gitkeep-empty-folders --strict` and resolve all issues
+- [x] Test `openspec init` manually in a fresh directory
+- [x] Verify Git tracks the empty directories with `.gitkeep` files
+- [x] Test `openspec list` works immediately after clone in a new repository
🤖 Prompt for AI Agents
In openspec/changes/add-gitkeep-empty-folders/tasks.md around lines 1 to 19, the
checklist still shows unfinished tasks; update the file by changing each "- [ ]"
checkbox for the Implementation, Testing, and Validation items to "- [x]" to
mark them completed, ensuring spacing and markdown syntax remain intact so the
file correctly reflects that implementation, tests, and validation are done.

yha9806 and others added 4 commits November 5, 2025 16:26
Archived the completed change following OpenSpec workflow:
- Moved change to archive/2025-11-05-add-gitkeep-empty-folders/
- Updated cli-init spec with new Directory Creation requirement
- Added 3 new scenarios:
  1. Creating OpenSpec structure (with .gitkeep files)
  2. Preserving empty directories in Git
  3. Working with fresh clones

All specs validate successfully with --strict mode.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
The prepare script runs before dependencies are installed, causing
build failures when installing from GitHub. Changed to postinstall
which runs after pnpm install completes.

This allows: npm install -g git+https://github.com/yha9806/OpenSpec.git
Enable direct installation from GitHub by committing built dist files:
- Uncommented dist/ in .gitignore to allow tracking build output
- Removed prepare script (causes build failures during git install)
- Kept prepublishOnly for npm registry releases

This allows users to install directly from GitHub:
npm install -g git+https://github.com/yha9806/OpenSpec.git

Build artifacts are now included so no compilation is needed during install.
Fixes npm/pnpm interaction issues on Windows.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 90596db and 2f715d1.

📒 Files selected for processing (1)
  • package.json (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/changes/*/{proposal.md,tasks.md,design.md,specs/**/spec.md} : Scaffold each change with proposal.md, tasks.md, optional design.md, and delta spec files under openspec/changes/<change-id>/specs/<capability>/spec.md
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/specs/**/spec.md : Keep current, deployed specs in openspec/specs/<capability>/spec.md (one focused capability per directory)
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-21T03:05:07.856Z
Learning: Keep the OPENSPEC managed block so 'openspec update' can refresh the instructions
Learnt from: CR
Repo: Fission-AI/OpenSpec PR: 0
File: openspec/AGENTS.md:0-0
Timestamp: 2025-10-22T06:05:06.615Z
Learning: Applies to openspec/changes/archive/** : Archive completed changes under openspec/changes/archive/YYYY-MM-DD-<name>/ and run 'openspec validate --strict' post-archive; update specs/ if capabilities changed
🔇 Additional comments (1)
package.json (1)

47-48: Clarify the relationship between package.json script changes and the .gitkeep PR objective.

The package.json changes (removing prepare and adding preinstall/postinstall scripts) appear disconnected from the stated PR objective of adding .gitkeep files to preserve empty directories. The PR description makes no mention of script lifecycle changes, and the preinstall echo statement has no functional purpose.

Can you confirm:

  1. Is the move from prepare to postinstall intentional and necessary for this PR?
  2. What problem does the preinstall echo statement solve?

package.json Outdated
"test:coverage": "vitest --coverage",
"prepare": "pnpm run build",
"preinstall": "echo Installing dependencies...",
"postinstall": "pnpm run build",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Moving build from prepare to postinstall may negatively impact development workflows.

The change from prepare: pnpm run build to postinstall: pnpm run build is significant:

  • prepare runs once per git dependency and is the standard hook for build steps (suitable for CI and git-based installs).
  • postinstall runs on every pnpm install invocation, which would trigger a full build each time dependencies are reinstalled during development.

This could slow down common development workflows (e.g., pnpm install, switching branches, or updating dependencies). Consider whether this behavioral change is necessary or if prepare should be retained.


I'll verify the claims in this review comment by checking the npm/pnpm lifecycle hook behavior and examining the repository for context.





🏁 Script executed:

#!/bin/bash
# Check git history or current package.json for the change from prepare to postinstall
git log -p --all -- package.json | grep -A5 -B5 "prepare\|postinstall" | head -80

Length of output: 59


🌐 Web query:

pnpm prepare vs postinstall lifecycle hooks behavior when to use

💡 Result:

Short answer

  • prepare: meant to build or prepare a package before it is packed/published and when installing from a Git repository (devDependencies are installed for git installs). Use it for producing artifacts that must exist in the published package (e.g. transpile/build before pack/publish or when consumers install from git). [1][2]
  • postinstall: runs after a package is installed into node_modules. It's appropriate for runtime setup that must run on every install of an installed package (e.g. install native binaries, run post‑install hooks), but is not guaranteed to run in all package manager configurations. [1]

pnpm-specific behavior (important)

  • pnpm caches/isolates side effects and (since pnpm v10) blocks dependency lifecycle scripts by default — postinstall (and other install hooks) of dependencies may not run unless explicitly allowed via pnpm configuration (onlyBuiltDependencies / neverBuiltDependencies / side‑effects settings). Because of this, relying on postinstall for producing published package artifacts or workspace rebuild triggers is fragile with pnpm. [3][4]

When to prefer which

  • Use prepare when:
    • You need built files included in the published tarball (prepare runs before pack/publish and for git installs).
    • You want consistent behavior across npm/pnpm/Yarn for source (git) installs.
  • Use postinstall when:
    • You need a script to run on end-user installs for runtime setup and you accept that some managers (or pnpm default settings) may block/ cache those scripts unless configured to allow them.
  • With pnpm, prefer prepare for build/artifact generation and avoid relying on postinstall for essential build steps; if you must use postinstall for a dependency, configure pnpm to allow that dependency’s scripts. [1][3][4]

Sources

  • npm lifecycle docs (prepare vs postinstall behavior). [1]
  • pnpm v10 lifecycle-scripts default blocking (security/change in behavior). [3]
  • pnpm package.json / onlyBuiltDependencies / neverBuiltDependencies docs. [4]
  • Example community migration away from postinstall to prepare (SvelteKit PR). [2]

Let me check the current package.json and repository context more carefully:





🏁 Script executed:

#!/bin/bash
# View the current package.json around the scripts section
cat package.json | grep -A20 '"scripts"'

Length of output: 777


🏁 Script executed:

#!/bin/bash
# Check if there's a .npmrc or pnpm-workspace.yaml indicating workspace/published package
ls -la | grep -E "npmrc|pnpm-workspace|package-lock"
# Also check git status to see if package.json was changed in this PR
git status package.json 2>/dev/null || echo "Git status not available"
# Try a simpler git diff
git diff HEAD~ package.json 2>/dev/null | head -50 || echo "Git diff not available"

Length of output: 564


Revert postinstall to prepare for git-based installs and CI workflows.

The change from prepare to postinstall is confirmed in the git diff and will negatively impact development workflows. prepare runs once per git dependency and is the standard hook for build steps, while postinstall runs on every pnpm install invocation. For this published package (which has prepublishOnly and release scripts), prepare is the correct lifecycle hook. Switching to postinstall will trigger unnecessary rebuilds on every install, slowing common development workflows like dependency updates and branch switching. The package already handles pre-publish builds via prepublishOnly, making postinstall redundant and inappropriate here.

Restore prepare: pnpm run build to align with npm best practices for published packages.

🤖 Prompt for AI Agents
In package.json around line 48, restore the lifecycle hook from "postinstall":
"pnpm run build" back to "prepare": "pnpm run build" because prepare runs once
for git-based installs and is the correct hook for published packages; change
the script key to "prepare" with the existing build command so CI and developer
installs don't trigger rebuilds on every pnpm install.

@yha9806 yha9806 mentioned this pull request Nov 6, 2025

# Build output
dist/
# dist/ - Commented out to allow git installation from GitHub
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too keen on check in /dist since in my experience this would get outdated pretty quickly and we would have to ensure this always up to date.


// Create .gitkeep files in empty directories to ensure Git tracks them
const emptyDirectories = [
path.join(openspecPath, 'specs', '.gitkeep'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there's a different way to solve this. This makes sense but I'm a reluctant to add in .gitkeep files that users will most likely forget to delete.

Maybe we can look into making init self-healing or finding another way to always ensure we re-create these folders if these are ever missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Empty folders

2 participants