Skip to content

Fix Marketplace install PATH for macOS GUI launches#151

Merged
ryota-murakami merged 2 commits into
mainfrom
codex/fix-marketplace-install
May 10, 2026
Merged

Fix Marketplace install PATH for macOS GUI launches#151
ryota-murakami merged 2 commits into
mainfrom
codex/fix-marketplace-install

Conversation

@ryota-murakami

@ryota-murakami ryota-murakami commented May 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • extend the skills CLI child process PATH with common Node toolchain locations for Finder-launched macOS apps
  • keep FORCE_COLOR disabled while preserving the inherited environment
  • add unit and Electron E2E regression coverage for Marketplace Install with sparse GUI PATH

Verification

  • pnpm validate
  • pnpm test:e2e

Summary by CodeRabbit

バグ修正

  • macOS で Finder から起動した場合など、PATH が制限されている環境で Marketplace のスキル インストール機能が正常に動作するように改善しました。npx ツールチェーンのパスを自動的に補完し、インストール処理を確実に実行できるようになります。

Review Change Stack

@vercel

vercel Bot commented May 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
skills-desktop Ready Ready Preview, Comment May 10, 2026 0:47am

Request Review

@coderabbitai

coderabbitai Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@ryota-murakami has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 54 minutes and 11 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2bbec504-4a8e-46b7-b5ec-a518eb7b5685

📥 Commits

Reviewing files that changed from the base of the PR and between 7dc472a and 4a91e67.

📒 Files selected for processing (2)
  • e2e/spec/marketplace-install-regression.e2e.ts
  • src/main/services/skillsCliService.test.ts

ウォークスルー

macOS の Finder 起動 Electron アプリが限定された PATHnpx skills コマンドを実行できるよう、ツールチェーンパスフォールバック機構を実装。CLI 環境構築ヘルパー、環境変数単体テスト、隔離ホーム環境下での E2E レグレッション検証を追加。

変更点

macOS GUI起動時のPATH解決

レイヤー / ファイル 説明
CLI環境構築実装
src/main/services/skillsCliService.ts
CLI_PATH_FALLBACKS 定数、buildCliPath()buildCliEnv() ヘルパーを追加。execCli() で spawn に env: buildCliEnv() を適用し、/opt/homebrew/bin など標準ツールチェーン位置を注入。
環境変数単体テスト
src/main/services/skillsCliService.test.ts
PATH を最小値に設定して search() 実行し、spawnMockFORCE_COLOR: '0'/opt/homebrew/bin 付き PATH で呼び出されることを検証。afterEach で PATH 復元。
マーケットプレイスインストール E2E テスト
e2e/spec/marketplace-install-regression.e2e.ts
隔離ホーム内に fake npx をステージング。sparse PATH と E2E 環境フラグで Electron 起動。Redux ストア待機、マーケットプレイス UI 駆動、fake npx によるスキル CLI 呼び出し検証、インストール状態の UI 確認。

推定レビュー工数

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed タイトルは変更セットの主要な目的を正確に反映している。macOS GUI起動時のMarketplaceインストール用PATH問題の修正という核心的な変更を簡潔に説明している。
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-marketplace-install

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.

@codecov-commenter

codecov-commenter commented May 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 51.91%. Comparing base (1df5a12) to head (4a91e67).

Files with missing lines Patch % Lines
src/main/services/skillsCliService.ts 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #151      +/-   ##
==========================================
+ Coverage   51.64%   51.91%   +0.27%     
==========================================
  Files         173      173              
  Lines        4202     4207       +5     
  Branches      892      892              
==========================================
+ Hits         2170     2184      +14     
+ Misses       1843     1831      -12     
- Partials      189      192       +3     
Flag Coverage Δ
unittests 51.91% <80.00%> (+0.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ryota-murakami

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented May 10, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Full review triggered.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@e2e/spec/marketplace-install-regression.e2e.ts`:
- Line 99: The nullish coalescing expression assigned to variable store uses the
identical operands (window.__store__ ?? window.__store__), which is redundant
and likely a typo; update the assignment in the
marketplace-install-regression.e2e.ts file so that either you simply read
window.__store__ (const store = window.__store__) if only that property was
intended, or replace the second operand with the intended fallback (for example
window.__STORE__ or window.store) so the expression becomes window.__store__ ??
window.__STORE__ (or another correct fallback) to provide a meaningful fallback
path.

In `@src/main/services/skillsCliService.test.ts`:
- Around line 110-129: Update the test "adds common Node toolchain paths so
Finder-launched installs can find npx" to assert that the merged PATH contains
both the injected toolchain fallback (e.g. '/opt/homebrew/bin') and the original
PATH segments (use process.env.PATH or a canonical fragment like
'/usr/bin:/bin'), since buildCliPath merges the original PATH with fallbacks;
adjust the spawnMock expectation for skillsCliService.search (and the
SKILLS_CLI_VERSION usage) to check env.PATH includes both values (for example by
using expect.stringMatching(/\/opt\/homebrew\/bin.*\/usr\/bin/) or two
expect.stringContaining checks) so the test verifies both the fallback and
original PATH are preserved.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 883e2e36-eb7f-4d51-a0e4-0355e611235f

📥 Commits

Reviewing files that changed from the base of the PR and between 1df5a12 and 7dc472a.

📒 Files selected for processing (3)
  • e2e/spec/marketplace-install-regression.e2e.ts
  • src/main/services/skillsCliService.test.ts
  • src/main/services/skillsCliService.ts

Comment thread e2e/spec/marketplace-install-regression.e2e.ts Outdated
Comment thread src/main/services/skillsCliService.test.ts
@ryota-murakami ryota-murakami merged commit 215f017 into main May 10, 2026
9 checks passed
@ryota-murakami ryota-murakami deleted the codex/fix-marketplace-install branch May 10, 2026 12:49
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.

2 participants