Skip to content

fix: OpenSpec CLI install scope#179

Merged
benym merged 4 commits into
rpamis:masterfrom
ddddddddwp:fix/issue-175-openspec-no-node-modules
Jul 12, 2026
Merged

fix: OpenSpec CLI install scope#179
benym merged 4 commits into
rpamis:masterfrom
ddddddddwp:fix/issue-175-openspec-no-node-modules

Conversation

@ddddddddwp

@ddddddddwp ddddddddwp commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Install or upgrade the OpenSpec CLI globally even when comet init is run with project scope.
  • Preserve project-scoped openspec init behavior so OpenSpec skills/artifacts still initialize under the selected Comet scope.
  • Add a regression test covering project-scope OpenSpec CLI install arguments.
  • Bump the package version to 0.4.0-beta.4 and add a changelog entry.

Root Cause

Project-scope initialization reused Comet's install scope for the OpenSpec CLI npm install command. That made Comet run npm install @fission-ai/openspec@latest from the target project directory, which is npm's local install mode and can create a project node_modules/ directory. The CLI is later resolved from PATH and executed as openspec, so the CLI install should be global regardless of Comet's project/global skill scope.

Fixes #175.

Validation

  • npx vitest run test/domains/integrations/openspec.test.ts test/app/init.test.ts
  • pnpm lint
  • pnpm build
  • git diff --check

Notes

A full local pnpm test run was also attempted: 1047 tests passed and 1 existing environment-sensitive doctor auto-scope assertion failed because this machine has global Comet skills installed. The failure is unrelated to this OpenSpec install-scope change.

Summary by Sourcery

Ensure the OpenSpec CLI is always installed globally, regardless of Comet project/global skill scope, to avoid creating project-local node_modules directories.

Bug Fixes:

  • Fix OpenSpec CLI installation so project-scoped initialization no longer performs a local npm install that can create a project node_modules directory.

Tests:

  • Add a regression test verifying that project-scoped OpenSpec initialization installs the OpenSpec CLI with global npm flags.

Summary by CodeRabbit

  • Bug Fixes
    • OpenSpec CLI installation now consistently uses a global npm installation, including when project scope is selected.
    • This improves installation reliability and ensures the CLI is available for subsequent OpenSpec operations.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6b4945a9-8d9b-42bf-9e37-db94086361a7

📥 Commits

Reviewing files that changed from the base of the PR and between f003caa and d8b8613.

📒 Files selected for processing (1)
  • test/domains/integrations/openspec.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/domains/integrations/openspec.test.ts

📝 Walkthrough

Walkthrough

Changes

OpenSpec Global Install Fix

Layer / File(s) Summary
Global install behavior
domains/integrations/openspec.ts
ensureOpenSpecCli no longer accepts installation scope and always invokes npm with global install arguments; installOpenSpec matches the updated signature.
Regression coverage
test/domains/integrations/openspec.test.ts
Adds project-scope coverage asserting the global OpenSpec CLI installation command and installed result.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: adjusting OpenSpec CLI install scope.
Linked Issues check ✅ Passed The changes address #175 by forcing global OpenSpec CLI installs and adding regression coverage.
Out of Scope Changes check ✅ Passed The implementation and test changes stay focused on the OpenSpec install-scope fix.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@sourcery-ai

sourcery-ai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Reviewer's Guide

OpenSpec CLI installation is refactored to always use global npm install regardless of Comet skill scope, a regression test is added to assert the global install behavior for project scope, and the installOpenSpec wiring is updated accordingly.

Sequence diagram for global OpenSpec CLI installation regardless of scope

sequenceDiagram
  participant CometInit
  participant installOpenSpec
  participant ensureOpenSpecCli
  participant npm as npm_execFileSync

  CometInit->>installOpenSpec: installOpenSpec(scope, projectPath, shouldInstallCli)
  installOpenSpec->>ensureOpenSpecCli: ensureOpenSpecCli(projectPath, shouldInstallCli)
  ensureOpenSpecCli->>npm: execFileSync(getNpmExecutable(), [install, -g, @fission-ai/openspec@latest])
  npm-->>ensureOpenSpecCli: CLI installed_or_upgraded
  ensureOpenSpecCli-->>installOpenSpec: 'ready' | 'missing' | 'failed'
  installOpenSpec-->>CometInit: 'installed' | 'failed' | 'skipped'
Loading

File-Level Changes

Change Details Files
Make OpenSpec CLI installation always use global npm install irrespective of Comet project/global scope.
  • Removed the scope parameter from ensureOpenSpecCli and its call sites.
  • Simplified npm argument selection to always use ['install', '-g', '@fission-ai/openspec@latest'].
  • Kept cwd pointing at the project path while forcing a global CLI install via npm.
domains/integrations/openspec.ts
Add regression coverage confirming project-scope OpenSpec initialization still performs a global CLI install.
  • Extended the OpenSpec integration test suite with a case that runs installOpenSpec using project scope.
  • Configured mockedExecFileSync return values to simulate CLI path detection and upgrade flows.
  • Asserted that the npm call includes the global install arguments for @fission-ai/openspec@latest.
test/domains/integrations/openspec.test.ts

Assessment against linked issues

Issue Objective Addressed Explanation
#175 Prevent comet init with OpenSpec selected from creating a project-level node_modules directory (i.e., avoid local OpenSpec CLI installation in the target project).

Possibly linked issues

  • #unknown: PR fixes OpenSpec CLI install to always be global, preventing extra project node_modules described in issue.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@ddddddddwp ddddddddwp changed the title Fix OpenSpec CLI install scope fix: OpenSpec CLI install scope Jul 9, 2026
@ddddddddwp ddddddddwp marked this pull request as ready for review July 9, 2026 15:52

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Around line 11-14: Remove the entire `### Tests` changelog entry from
CHANGELOG.md, since the new OpenSpec install regression coverage is a normal
unit/regression test and should not be documented there. Keep the changelog
focused on user-runnable release capabilities only; no other content in this
section should remain.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5f46e235-ad71-47a3-8951-a3683517fa6d

📥 Commits

Reviewing files that changed from the base of the PR and between da58122 and e007b05.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (4)
  • CHANGELOG.md
  • domains/integrations/openspec.ts
  • package.json
  • test/domains/integrations/openspec.test.ts

Comment thread CHANGELOG.md Outdated
@ddddddddwp

Copy link
Copy Markdown
Contributor Author

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Around line 11-14: Remove the entire `### Tests` changelog entry from
CHANGELOG.md, since the new OpenSpec install regression coverage is a normal
unit/regression test and should not be documented there. Keep the changelog
focused on user-runnable release capabilities only; no other content in this
section should remain.

🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5f46e235-ad71-47a3-8951-a3683517fa6d

📥 Commits
Reviewing files that changed from the base of the PR and between da58122 and e007b05.

⛔ Files ignored due to path filters (1)

  • package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (4)

@ddddddddwp ddddddddwp closed this Jul 10, 2026
@ddddddddwp

Copy link
Copy Markdown
Contributor Author

@coderabbitai
review

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

@ddddddddwp Got it, I'll review the latest changes now.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ddddddddwp ddddddddwp reopened this Jul 10, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread test/domains/integrations/openspec.test.ts Outdated
@benym

benym commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

LGTM

@benym benym merged commit 7cd530c into rpamis:master Jul 12, 2026
14 checks passed
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.

fix: comet init时选择openspec会多出来node moudle

2 participants