fix(install): close the GitHub-marketplace broken-install path#133
Merged
Conversation
Installing claude-smart via `claude plugin marketplace add ReflexioAI/claude-smart` produced a plugin whose backend could never start: plugin/vendor/reflexio is generated at pack time and gitignored, so the GitHub tree carries no Reflexio runtime. The failure surfaced late, as a repeating `bundled Reflexio import preflight failed` in the backend log. Prevent it at the front door, and make the runtime checks safe: - Generate .claude-plugin/marketplace.json at pack time from a committed template (npm prepack) instead of committing it. The manifest is what makes a directory installable as a marketplace; absent from git, `claude plugin marketplace add ReflexioAI/claude-smart` now fails with "Marketplace file not found" rather than installing a plugin that cannot run. The npm path is unaffected: `npx claude-smart install` registers the package root, which ships both the manifest and the vendor bundle. - Repair an existing broken cache from the npm package, scoped to the cache directory matching the package's own version. Comparing against the highest-versioned cache dir reported a spurious mismatch on any downgrade install (each version vendors a different Reflexio), forcing a pointless reinstall and then failing the install outright. - Preflight the bundled runtime before stopping or replacing a backend, in both the SessionStart hook and `/claude-smart:restart`. The check sits on the boundary between accepting a healthy backend and tearing one down, so a broken bundle in this plugin root no longer reports "backend is not running" over a live, compatible backend served by another root. - Point the failure message at `npx claude-smart update`, which can repair a missing bundle; `/claude-smart:restart` cannot, since it preflights the same bundle. - Fail Setup loudly when a host plugin cache (*/plugins/cache/*) has no vendor bundle. That can only mean a GitHub-marketplace install; a source checkout legitimately has none and is left alone. Verified end to end against a packed tarball in an isolated HOME: clean install boots the backend from the vendored runtime; deleting the bundle makes the hook accept a live backend without crying wolf, makes restart refuse without stopping it, and `npx claude-smart update` restores the bundle and the backend.
📝 WalkthroughWalkthroughThe PR generates the Claude marketplace manifest during package creation, repairs incomplete cached plugin vendors, adds bundled Reflexio backend preflight checks, updates restart behavior, and documents npm-based installation and recovery flows with corresponding tests. ChangesMarketplace packaging and runtime repair
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant User
participant Installer
participant ClaudeCache
participant BackendService
participant ReflexioRuntime
User->>Installer: npx claude-smart install or update
Installer->>ClaudeCache: validate shipped-version vendor payload
Installer->>ClaudeCache: refresh incomplete cache when needed
Installer->>BackendService: bootstrap or restart backend
BackendService->>ReflexioRuntime: run bundled import preflight
ReflexioRuntime-->>BackendService: preflight result
BackendService-->>User: start backend or report repair guidance
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 4
🤖 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 `@bin/claude-smart.js`:
- Line 2286: Update the missing-vendor recovery around
refreshClaudeCodeCacheIfVendorMissing in bin/claude-smart.js to distinguish
cache-refresh failures from ordinary bootstrap failures and recommend npx
claude-smart update. In tests/test_install_scripts.py, update the expected
recovery command and corresponding smart-install.sh message to npx claude-smart
update.
- Around line 829-845: Update claudeCodePackageVersion to read the version from
package.json first, matching the generated marketplace version used for Claude’s
cache key. Retain the existing plugin.json and pyproject.toml lookups only as
explicit fallback sources for development fixtures, preserving their current
order and behavior after the package version lookup.
In `@plugin/scripts/backend-service.sh`:
- Around line 821-825: Update the preflight branch to skip only when
claude_smart_reflexio_url_is_remote is true; remove the
CLAUDE_SMART_BACKEND_AUTOSTART check from this condition so explicit preflight
still validates the bundle when autostart is disabled.
In `@tests/test_install_scripts.py`:
- Line 2347: Update the marker_text assertion in the relevant install-script
test and the corresponding smart-install.sh guidance to use the supported
missing-bundle recovery command, “npx claude-smart update,” instead of “npx
claude-smart install.”
🪄 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 Plus
Run ID: b53abf12-7cc7-4162-9339-cb64eb169052
📒 Files selected for processing (15)
.claude-plugin/marketplace.template.json.gitignoreDEVELOPER.mdMakefileREADME.mdTROUBLESHOOTING.mdbin/claude-smart.jspackage.jsonplugin/README.mdplugin/scripts/backend-service.shplugin/scripts/smart-install.shplugin/src/claude_smart/cli.pyscripts/generate-marketplace.jstests/test_cli_restart.pytests/test_install_scripts.py
💤 Files with no reviewable changes (1)
- .claude-plugin/marketplace.template.json
Comment on lines
+829
to
+845
| function claudeCodePackageVersion(sourceRoot = join(PACKAGE_ROOT, "plugin")) { | ||
| try { | ||
| const manifest = JSON.parse( | ||
| readFileSync(join(sourceRoot, ".claude-plugin", "plugin.json"), "utf8"), | ||
| ); | ||
| if (typeof manifest.version === "string" && manifest.version) return manifest.version; | ||
| } catch { | ||
| // Fall through to pyproject.toml. | ||
| } | ||
| try { | ||
| const match = readFileSync(join(sourceRoot, "pyproject.toml"), "utf8").match( | ||
| /^version\s*=\s*"([^"]+)"/m, | ||
| ); | ||
| if (match) return match[1]; | ||
| } catch { | ||
| // No readable version. | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use package.json as the Claude cache-key source.
The generated marketplace version—and therefore Claude’s cache directory—is sourced from package.json, but this helper uses plugin-local metadata. If those versions drift, Line 1021 inspects the wrong directory and silently skips the required repair. Read the package version first, retaining these sources only as explicit fallbacks for development fixtures.
🤖 Prompt for 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.
In `@bin/claude-smart.js` around lines 829 - 845, Update claudeCodePackageVersion
to read the version from package.json first, matching the generated marketplace
version used for Claude’s cache key. Retain the existing plugin.json and
pyproject.toml lookups only as explicit fallback sources for development
fixtures, preserving their current order and behavior after the package version
lookup.
| } | ||
|
|
||
| try { | ||
| await refreshClaudeCodeCacheIfVendorMissing(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use npx claude-smart update consistently for missing-bundle recovery.
The affected paths currently direct users toward commands that cannot consistently restore the packaged vendor bundle.
bin/claude-smart.js#L2286-L2286: distinguish cache-refresh failures from ordinary bootstrap failures and recommendnpx claude-smart update.tests/test_install_scripts.py#L2347-L2347: expectnpx claude-smart updateand update the correspondingsmart-install.shmessage.
📍 Affects 2 files
bin/claude-smart.js#L2286-L2286(this comment)tests/test_install_scripts.py#L2347-L2347
🤖 Prompt for 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.
In `@bin/claude-smart.js` at line 2286, Update the missing-vendor recovery around
refreshClaudeCodeCacheIfVendorMissing in bin/claude-smart.js to distinguish
cache-refresh failures from ordinary bootstrap failures and recommend npx
claude-smart update. In tests/test_install_scripts.py, update the expected
recovery command and corresponding smart-install.sh message to npx claude-smart
update.
Comment on lines
+821
to
+825
| preflight) | ||
| if claude_smart_reflexio_url_is_remote \ | ||
| || [ "${CLAUDE_SMART_BACKEND_AUTOSTART:-1}" = "0" ]; then | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not bypass explicit preflight when autostart is disabled.
CLAUDE_SMART_BACKEND_AUTOSTART=0 makes preflight report success without checking the bundle. Because restart uses this command before stopping services, it can cross that safety boundary with an unusable runtime. Only skip for remote Reflexio; callers that do not require preflight should avoid invoking it.
Proposed fix
- if claude_smart_reflexio_url_is_remote \
- || [ "${CLAUDE_SMART_BACKEND_AUTOSTART:-1}" = "0" ]; then
+ if claude_smart_reflexio_url_is_remote; then
exit 0
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| preflight) | |
| if claude_smart_reflexio_url_is_remote \ | |
| || [ "${CLAUDE_SMART_BACKEND_AUTOSTART:-1}" = "0" ]; then | |
| exit 0 | |
| fi | |
| preflight) | |
| if claude_smart_reflexio_url_is_remote; then | |
| exit 0 | |
| fi |
🤖 Prompt for 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.
In `@plugin/scripts/backend-service.sh` around lines 821 - 825, Update the
preflight branch to skip only when claude_smart_reflexio_url_is_remote is true;
remove the CLAUDE_SMART_BACKEND_AUTOSTART check from this condition so explicit
preflight still validates the bundle when autostart is disabled.
| assert marker.exists() | ||
| marker_text = marker.read_text() | ||
| assert "bundled Reflexio runtime is missing" in marker_text | ||
| assert "npx claude-smart install" in marker_text |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the supported cache-repair command.
This test locks Setup guidance to npx claude-smart install, while the supported missing-bundle recovery command elsewhere in this PR is npx claude-smart update. Update both the assertion and smart-install.sh output.
🤖 Prompt for 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.
In `@tests/test_install_scripts.py` at line 2347, Update the marker_text assertion
in the relevant install-script test and the corresponding smart-install.sh
guidance to use the supported missing-bundle recovery command, “npx claude-smart
update,” instead of “npx claude-smart install.”
The harness installs the checkout itself as a marketplace, so removing the committed .claude-plugin/marketplace.json broke it at `claude plugin marketplace add` with "Marketplace file not found". The manifest is a pack-time artifact, exactly like plugin/vendor/reflexio, which the harness already generates in setup. Generate the manifest there too, alongside prepare_vendored_reflexio, and add a guard test asserting it happens before the marketplace is registered. Corrects the DEVELOPER.md claim that local development never registers the checkout as a marketplace — it does.
plugin/vendor/reflexio is a pack-time artifact and is gitignored, so a fresh CI checkout has none. Three install-flow tests depended on it implicitly and passed locally only because a dev checkout has it lying around: - Two copied REPO_ROOT/plugin/vendor into a fake cache and blew up with FileNotFoundError when it was absent. - The tarball smoke test packed a vendorless tarball, which installs a plugin whose backend can never start (backend-service.sh requires the bundle) — a state Setup now correctly rejects. It was only "passing" before because smart-install.sh silently tolerated it. Vendor the locked commit in the pytest job so those tests exercise what users actually receive, and make the tests themselves resilient on an unvendored checkout: drop a copytree that the version-scoped cache repair made unnecessary, mirror the source payload only when the checkout has one, and skip the tarball smoke with a clear reason rather than asserting a state that cannot run. Verified both ways: 202 passed + 1 skipped with no vendor bundle (fresh clone), 203 passed with it (CI).
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/integration.yml (1)
63-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInline JSON parsing lacks clear error messages.
If
reflexio.lock.jsonis missing or lacks therepo/commitkey, the inlinepython3 -cwill fail with a rawKeyErrortraceback. The integration harness (tests/integration/integration.sh:195-237) usesread_reflexio_lock_fieldwhich raises a clearmissing field:message. Consider adding a guard or reusing the same pattern for consistency and debuggability in CI logs.♻️ Suggested improvement
repo=$(python3 -c "import json,sys; d=json.load(open('reflexio.lock.json')); v=d.get('repo'); print(v) if v else sys.exit('missing field: repo')") commit=$(python3 -c "import json,sys; d=json.load(open('reflexio.lock.json')); v=d.get('commit'); print(v) if v else sys.exit('missing field: commit')")🤖 Prompt for 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. In @.github/workflows/integration.yml around lines 63 - 64, Update the repo and commit extraction commands in the workflow to validate reflexio.lock.json and report clear missing-field errors, matching the read_reflexio_lock_field behavior used by the integration harness. Ensure missing repo or commit keys produce descriptive messages instead of raw KeyError tracebacks, while preserving the existing variable assignments.
🤖 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.
Nitpick comments:
In @.github/workflows/integration.yml:
- Around line 63-64: Update the repo and commit extraction commands in the
workflow to validate reflexio.lock.json and report clear missing-field errors,
matching the read_reflexio_lock_field behavior used by the integration harness.
Ensure missing repo or commit keys produce descriptive messages instead of raw
KeyError tracebacks, while preserving the existing variable assignments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 68b5e744-2a6b-4552-91e8-50ee695ab771
📒 Files selected for processing (2)
.github/workflows/integration.ymltests/test_install_scripts.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_install_scripts.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Installing via
claude plugin marketplace add ReflexioAI/claude-smartproduces a plugin whose backend can never start.plugin/vendor/reflexiois generated at pack time and gitignored, so the GitHub tree carries no Reflexio runtime. Users only find out later, from a backend log repeatingbundled Reflexio import preflight failed.What changed
Close the door at the front.
.claude-plugin/marketplace.jsonis now generated at pack time from a committed template (via npm'sprepack) instead of being committed. That manifest is what makes a directory installable as a Claude Code marketplace — with it absent from git,claude plugin marketplace add ReflexioAI/claude-smartfails immediately withMarketplace file not foundinstead of installing a plugin that cannot run. The supported path is untouched:npx claude-smart installregisters the npm package root, which ships both the manifest and the vendor bundle. Version can't drift either — the generator stamps it frompackage.json, somake bumphas nothing to bump.Repair anyone already stuck, scoped to the cache dir matching the package's own version. The first cut compared against the highest-versioned cache dir; since each plugin version vendors a different Reflexio, their vendor payloads legitimately differ, so any downgrade install was diagnosed as "stale", force-reinstalled, and then failed outright (
exit 1) when the repair couldn't converge. Reproduced, then fixed.Preflight the bundled runtime before stopping or replacing a backend — in both the SessionStart hook and
/claude-smart:restart. The check sits exactly on the boundary between accepting a healthy backend and tearing one down. This matters: a broken bundle in one plugin root no longer reports "backend is not running" over a live, compatible backend served by another root (a case the code explicitly supports).Point users at the command that can actually fix it. The failure message said to run
/claude-smart:restart, which preflights the same missing bundle and refuses — a dead end. It now saysnpx claude-smart update.Fail Setup loudly when a host plugin cache (
*/plugins/cache/*) has no vendor bundle. That can only mean a GitHub-marketplace install;smart-install.shpreviously skipped silently. A source checkout legitimately has no bundle and is left alone.Verification
Full end-to-end against a real
make packagetarball, installed into an isolatedHOMEon harness ports (production backend on 8071 untouched throughout):marketplace addMarketplace file not foundmarketplace add{"continue":true}— no false "backend is not running", backend survivesnpx claude-smart updateremedy/claude-smart:restart, bundle deletedrc=1; same pid still serving 200 — nothing stoppednpx claude-smart updateTests: 202 passing across
test_install_scripts.py,test_cli_restart.py,test_codex_support.py(7 new: manifest stays untracked, real pack ships a version-correct manifest, downgrade install doesn't touch a newer cache, dev-checkout exemption, Setup cache guard, multi-root acceptance, restart-preserves-services). Biome clean;bash -nclean.Note for release notes
Anyone who previously added the GitHub marketplace will find
claude plugin updatestops working once this ships. That's intended — it pushes them ontonpx claude-smart update, the only thing that can repair them — but it's a visible break.Summary by CodeRabbit
Bug Fixes
Documentation
npx claude-smart installflow and repair steps.Packaging