fix: locating plugin json file

[yordis]

No description provided.

[cursor]

PR Summary

Enhances plugin installation to resolve nested plugin paths via manifest or recursive search, skips validation in dry-run for non-directory marketplaces, improves error messaging, and adds tests for nested scenarios.

• CLI (src/commands/plugin-install.ts):
  • Add recursive plugin path resolution via resolvePluginPath (manifest-first, fallback search).
  • Skip validation in dry-run for non-directory marketplaces; keep for local directories.
  • Strengthen validation and error handling; ensure pluginPath before syncing.
• Helpers (src/helpers/marketplace.ts):
  • Introduce resolvePluginPath with manifest check, existence verification, and recursive discovery.
• Errors (src/errors.ts):
  • Improve PluginManifestNotFoundError message to include the resolved path.
• Tests (tests/commands/plugin-install.test.ts):
  • Add cases for nested plugin directories, incorrect manifest source paths, and duplicate base-name plugins.

^{Written by Cursor Bugbot for commit ec947ee. This will update automatically on new commits. Configure here.}

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds a manifest-aware plugin path resolution flow (new resolvePluginPath), conditional manifest loading for dry-run vs real install, defers existence checks until after resolution, moves plugin validation post-resolution, and updates error messaging for manifest-not-found formatting.

Changes

Cohort / File(s)	Summary
Plugin resolution helper src/helpers/marketplace.ts	Adds resolvePluginPath(marketplacePath, pluginName, manifest) which: prefers manifest-listed source if present and exists, falls back to recursive discovery via getAvailablePlugins (matching name or last path segment), and finally uses getPluginSourcePath as a last resort.
Install command flow src/commands/plugin-install.ts	Replaces previous manifest-existence checks with resolvePluginPath usage; conditionally loads marketplace manifest (skip in dry-run for directory marketplaces); defers runtime file-existence checks until after resolution; performs plugin structure validation after resolution and only when not skipping validation; guards for missing resolved path and preserves dry-run messaging and early returns.
Error message formatting src/errors.ts	Updates PluginManifestNotFoundError message to interpolate the provided pluginPath (uses ${pluginPath}/.claude-plugin/plugin.json) instead of a hardcoded path string.
Tests (nested plugins) tests/commands/plugin-install.test.ts	Adds a "nested plugin directories" test suite covering nested-directory installs, incorrect manifest paths, and multiple nested plugins with same base name — note: the new suite is duplicated in the diff.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as plugin-install
    participant FS as filesystem/marketplace
    participant Resolver as resolvePluginPath
    participant Validator as plugin validator

    CLI->>FS: (cond) Load marketplace manifest
    note right of CLI: skip manifest if dry-run and\nmarketplace is directory-based
    CLI->>Resolver: resolvePluginPath(marketplacePath, pluginName, manifest?)
    Resolver->>FS: if manifest path provided -> check manifest source path exists
    alt manifest source valid
        FS-->>Resolver: manifest source path
    else manifest missing/invalid
        Resolver->>FS: getAvailablePlugins(...) recursive search
        FS-->>Resolver: list of plugin paths
        Resolver->>Resolver: match by name or last path segment
        alt matched
            Resolver-->>CLI: resolved pluginPath
        else no match
            Resolver-->>CLI: fallback derived path (getPluginSourcePath)
        end
    end

    CLI->>FS: check resolved pluginPath exists (runtime file check)
    alt path missing
        FS-->>CLI: throw error (includes checked path + nested-dir guidance)
    else path exists
        CLI->>Validator: validate plugin structure (only if not skipping validation)
        alt valid
            Validator-->>CLI: ok -> proceed enable/sync (unless dry-run)
        else invalid
            Validator-->>CLI: standardized "Invalid plugin" error
        end
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Pay attention to resolvePluginPath matching logic and edge cases for same-base-name collisions.
• Verify conditional manifest loading behavior in plugin-install.ts (dry-run vs real install) and ensure no regressions in earlier flows.
• Review updated error message formatting and test duplication in tests/commands/plugin-install.test.ts.

Possibly related PRs

• fix: claude code marketplace schema #29 — Modifies src/helpers/marketplace.ts and src/commands/plugin-install.ts; likely overlaps with the new marketplace-aware resolution and install flow.
• fix: plugin discovery with recursive directory scanning #25 — Changes recursive plugin-scanning and canonicalization in src/helpers/marketplace.ts; relates to resolvePluginPath discovery behavior.
• fix: move config under .aipm directory #32 — Rewrites src/commands/plugin-install.ts flow; likely overlaps with conditional manifest loading and validation placement.

Poem

🐇 I hop through nested folders bright,

resolving paths by day and night,
if manifests hide or lead astray,
I sniff the trails and find your way,
with tidy errors, soft and light.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Description check	❓ Inconclusive	No pull request description was provided by the author, making it impossible to assess whether the description relates to the changeset.	Provide a description explaining the changes, such as the new resolvePluginPath flow, manifest loading strategy, and file existence checks.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: fixing the logic for locating the plugin.json file, which is the core focus of the changes across plugin-install.ts, marketplace.ts, and errors.ts.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch yordis/fix-1

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

tests/commands/plugin-install.test.ts (1)

435-603: Nested directory tests align well with the new resolution logic

The three tests here do a good job exercising the new behavior: simple nested install, manifest with a bad source still succeeding via recursive discovery, and multiple nested plugins sharing a base name.

One thing to be aware of: in the “multiple nested plugins with same base name” case you only assert that my-plugin@local is enabled, not which of the two variants was picked. That keeps the behavior intentionally unspecified and dependent on filesystem traversal order. If you ever want deterministic selection (e.g., prefer a specific category or a sorted path), you may want to either codify that in resolvePluginPath or strengthen this test to assert the chosen path/version.

src/helpers/marketplace.ts (1)

92-193: resolvePluginPath + recursive discovery correctly handle nested and mis-declared plugins

The new resolution flow looks solid:

• resolvePluginPath now prefers the manifest source when it exists and the path is present, but crucially falls back to recursive discovery when the manifest path is missing or when no manifest exists. This directly fixes the earlier issue where a bad manifest source prevented discovery of the real plugin location.
• findPluginsRecursively is defensive:
  • realpath-based canonicalization plus the startsWith(canonicalRoot + sep) checks prevent escaping the marketplace root.
  • the visited set stops symlink-driven cycles.
  • hidden directories are skipped, so .claude-plugin itself isn’t traversed.
• Using getAvailablePlugins(marketplacePath, null) inside resolvePluginPath intentionally ignores any manifest when doing the fallback search, which is what you want when the manifest may itself be wrong.

If you ever notice performance issues on very large marketplace trees, one micro-optimization would be to compute the canonical marketplace root once and thread it through instead of calling realpath(marketplaceRoot) on every recursion, but that’s not necessary for correctness.

Also applies to: 203-216, 218-255

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9993a54 and ec947ee.

📒 Files selected for processing (4)

• src/commands/plugin-install.ts (3 hunks)
• src/errors.ts (1 hunks)
• src/helpers/marketplace.ts (1 hunks)
• tests/commands/plugin-install.test.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (3)

src/helpers/marketplace.ts (2)

src/schema.ts (1)

• MarketplaceManifest (98-98)

src/helpers/fs.ts (1)

• fileExists (26-33)

tests/commands/plugin-install.test.ts (2)

src/commands/plugin-install.ts (1)

• pluginInstall (22-135)

src/helpers/fs.ts (1)

• fileExists (26-33)

src/commands/plugin-install.ts (3)

src/helpers/marketplace.ts (3)

• loadMarketplaceManifest (71-79)
• getMarketplaceType (13-15)
• resolvePluginPath (230-256)

src/helpers/fs.ts (1)

• fileExists (26-33)

src/helpers/io.ts (1)

• defaultIO (79-79)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Cursor Bugbot

🔇 Additional comments (2)

src/errors.ts (1)

9-16: More precise PluginManifestNotFoundError message looks good

Including ${pluginPath}/.claude-plugin/plugin.json in the error text makes it much clearer what concrete path was expected for the manifest, especially now that plugins can live in nested directories. No issues here.

src/commands/plugin-install.ts (1)

10-11: Dry-run gating and plugin path resolution look correct and address prior issues

• The shouldSkipValidation flag (cmd.dryRun && marketplace.source !== 'directory') cleanly restores the old behavior for git/url marketplaces in dry-run: you still resolve the marketplace path, but you no longer attempt to load a manifest, resolve a plugin path, or validate structure against an uncached checkout before the dry-run early return. Directory-based marketplaces are still validated, which matches local expectations.
• The new flow:
  • loads the appropriate manifest via loadMarketplaceManifest(marketplacePath, getMarketplaceType(marketplaceName)),
  • resolves pluginPath through resolvePluginPath (manifest first, then recursive search, then fallback),
  • and only then checks fileExists(pluginPath) with a helpful error message that includes the checked path and hints about nested directories.
    This aligns with the new nested-directory tests.
• The post-install guard

  if (!pluginPath) {
    throw new Error(`Plugin path not resolved for '${pluginName}'`);
  }

  is a good defensive check and is indeed unreachable under the current control flow, since any non-dry-run path always sets pluginPath or throws earlier.

Overall, the changes here tie together the new marketplace helper logic and dry-run semantics without introducing new edge-case regressions.

Also applies to: 63-76, 77-95, 118-122