fix: correct typo marketplace to marketplaces#1552
fix: correct typo marketplace to marketplaces#1552fangzhengjin wants to merge 1 commit intoaffaan-m:mainfrom
Conversation
📝 WalkthroughWalkthroughRenamed inspected plugin directory segment from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Greptile SummaryThis PR corrects a typo across all plugin path resolution logic, renaming the directory segment Confidence Score: 5/5Safe to merge — all remaining findings are P2 style suggestions that do not affect runtime behavior. The fix is a straightforward, consistent rename of a directory segment string across all consumers (resolver, bootstrap hook, hooks.json, command docs, and tests). No logic is changed; the only outstanding note is the previously-flagged P2 about stale test description strings in resolve-ecc-root.test.js. tests/lib/resolve-ecc-root.test.js — test description strings on lines 161 and 172 still say 'marketplace' (singular). Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[resolveEccRoot / resolvePluginRoot] --> B{CLAUDE_PLUGIN_ROOT set?}
B -- yes --> C[Return env var value]
B -- no --> D{~/.claude/ has probe file?}
D -- yes --> E[Return ~/.claude/]
D -- no --> F[Try known plugin paths]
F --> G["~/.claude/plugins/ecc"]
F --> H["~/.claude/plugins/ecc@ecc"]
F --> I["~/.claude/plugins/marketplaces/ecc ✓ fixed"]
F --> J["~/.claude/plugins/everything-claude-code"]
F --> K["~/.claude/plugins/everything-claude-code@everything-claude-code"]
F --> L["~/.claude/plugins/marketplaces/everything-claude-code ✓ fixed"]
G & H & I & J & K & L --> M{Probe file found?}
M -- yes --> N[Return matched path]
M -- no --> O[Scan plugin cache]
O --> P["~/.claude/plugins/cache/ecc/org/version/"]
O --> Q["~/.claude/plugins/cache/everything-claude-code/org/version/"]
P & Q --> R{Probe file found?}
R -- yes --> S[Return cache path]
R -- no --> T[Fallback: return ~/.claude/]
Reviews (3): Last reviewed commit: "fix: correct typo `marketplace` to `mark..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
tests/lib/command-plugin-root.test.js (1)
40-43:⚠️ Potential issue | 🔴 CriticalStale assertions — this test will fail after the rename.
INLINE_RESOLVEis generated fromPLUGIN_ROOT_SEGMENTSinscripts/lib/resolve-ecc-root.js, which now only emits the plural"marketplaces"form. These twoassert.ok(INLINE_RESOLVE.includes('"marketplace",...'))checks (singular) will therefore never match, sonode tests/lib/command-plugin-root.test.jswill fail.Either update the expectations to the plural form, or (if you want to keep asserting the legacy singular path is still discoverable) also re-add the singular segments to
PLUGIN_ROOT_SEGMENTSfor backward compatibility. The former is the most likely intent given the PR is a pure rename:🔧 Proposed fix — match the new plural directory name
test('inline resolver covers current and legacy marketplace plugin roots', () => { - assert.ok(INLINE_RESOLVE.includes('"marketplace","ecc"')); - assert.ok(INLINE_RESOLVE.includes('"marketplace","everything-claude-code"')); + assert.ok(INLINE_RESOLVE.includes('"marketplaces","ecc"')); + assert.ok(INLINE_RESOLVE.includes('"marketplaces","everything-claude-code"')); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/lib/command-plugin-root.test.js` around lines 40 - 43, The test's assertions are stale because INLINE_RESOLVE (generated from PLUGIN_ROOT_SEGMENTS in resolve-ecc-root.js) now emits the plural "marketplaces"; update the two assertions in command-plugin-root.test.js that check INLINE_RESOLVE.includes('"marketplace",...') to use the new plural '"marketplaces",...' instead (or, if backward compatibility is desired, restore the legacy singular segment in PLUGIN_ROOT_SEGMENTS so both forms are emitted).tests/lib/resolve-ecc-root.test.js (1)
161-193:⚠️ Potential issue | 🟡 MinorNit: test names still say
marketplace/(singular) while fixtures now usemarketplaces.The setup calls were updated to
['marketplaces', ...]but thetest(...)labels on lines 161, 172, and (implicitly) the block above line 186 still reference~/.claude/plugins/marketplace/eccand.../plugins/marketplace/everything-claude-code. These names will be misleading in test output.📝 Proposed label updates
- if (test('finds marketplace current plugin install at ~/.claude/plugins/marketplace/ecc', () => { + if (test('finds marketplace current plugin install at ~/.claude/plugins/marketplaces/ecc', () => { @@ - if (test('finds marketplace legacy plugin install at ~/.claude/plugins/marketplace/everything-claude-code', () => { + if (test('finds marketplace legacy plugin install at ~/.claude/plugins/marketplaces/everything-claude-code', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/lib/resolve-ecc-root.test.js` around lines 161 - 193, Update the three test description strings passed to test(...) that currently say "marketplace" to use the plural "marketplaces" so they match the fixture calls (setupLegacyPluginInstall(..., ['marketplaces', ...]) and setupPluginCache usage) and avoid misleading output; locate the tests that call setupLegacyPluginInstall and setupPluginCache and change their labels like "finds marketplace current plugin install at ~/.claude/plugins/marketplace/ecc" to "finds marketplaces current plugin install at ~/.claude/plugins/marketplaces/ecc" (and similarly for "everywhere-claude-code" and the "prefers exact legacy plugin..." case) while leaving resolveEccRoot, setupLegacyPluginInstall, and setupPluginCache usage unchanged.hooks/hooks.json (1)
10-230:⚠️ Potential issue | 🔴 CriticalUpdate all embedded node -e resolvers in PreToolUse, PreCompact, SessionStart, PostToolUse, and PostToolUseFailure hooks to use plural
marketplacespath.Hooks at lines 10, 21, 32, 43, 56, 68, 80, 91, 105, 118, 131, 144, 157, 169, 180, 191, 203, 215, and 230 still contain embedded resolvers with singular
"marketplace"paths:["marketplace","ecc"] ... ["marketplace","everything-claude-code"]Meanwhile, Stop and SessionEnd hooks use a newer resolver implementation with correct plural paths (
marketplaces). This inconsistency breaks plugin discovery at runtime: when installed via marketplace to~/.claude/plugins/marketplaces/, the majority of hooks will fail to locate the plugin and fall back silently. Update all old-style inline resolvers to match the plural path used in Stop/SessionEnd.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/hooks.json` around lines 10 - 230, The embedded node -e resolver strings inside the command fields (the inline resolver that builds CLAUDE_PLUGIN_ROOT) still reference the singular path segments ["marketplace", ...] — update every such inline resolver in the PreToolUse, PreCompact, SessionStart, PostToolUse, and PostToolUseFailure hook command strings to use the plural "marketplaces" segments (e.g., replace ["marketplace","ecc"] and ["marketplace","everything-claude-code"] with ["marketplaces","ecc"] and ["marketplaces","everything-claude-code"]) so the resolver matches the newer implementation used by Stop/SessionEnd; target the command strings for hook IDs like pre:bash:dispatcher, pre:write:doc-file-warning, pre:edit-write:suggest-compact, pre:observe:continuous-learning, pre:governance-capture, pre:config-protection, pre:mcp-health-check, pre:edit-write:gateguard-fact-force, pre:compact, session:start, post:bash:dispatcher, post:quality-gate, post:edit:design-quality-check, post:edit:accumulator, post:edit:console-warn, post:governance-capture, post:session-activity-tracker, and post:observe:continuous-learning to ensure consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@hooks/hooks.json`:
- Around line 10-230: The embedded node -e resolver strings inside the command
fields (the inline resolver that builds CLAUDE_PLUGIN_ROOT) still reference the
singular path segments ["marketplace", ...] — update every such inline resolver
in the PreToolUse, PreCompact, SessionStart, PostToolUse, and PostToolUseFailure
hook command strings to use the plural "marketplaces" segments (e.g., replace
["marketplace","ecc"] and ["marketplace","everything-claude-code"] with
["marketplaces","ecc"] and ["marketplaces","everything-claude-code"]) so the
resolver matches the newer implementation used by Stop/SessionEnd; target the
command strings for hook IDs like pre:bash:dispatcher,
pre:write:doc-file-warning, pre:edit-write:suggest-compact,
pre:observe:continuous-learning, pre:governance-capture, pre:config-protection,
pre:mcp-health-check, pre:edit-write:gateguard-fact-force, pre:compact,
session:start, post:bash:dispatcher, post:quality-gate,
post:edit:design-quality-check, post:edit:accumulator, post:edit:console-warn,
post:governance-capture, post:session-activity-tracker, and
post:observe:continuous-learning to ensure consistency.
In `@tests/lib/command-plugin-root.test.js`:
- Around line 40-43: The test's assertions are stale because INLINE_RESOLVE
(generated from PLUGIN_ROOT_SEGMENTS in resolve-ecc-root.js) now emits the
plural "marketplaces"; update the two assertions in command-plugin-root.test.js
that check INLINE_RESOLVE.includes('"marketplace",...') to use the new plural
'"marketplaces",...' instead (or, if backward compatibility is desired, restore
the legacy singular segment in PLUGIN_ROOT_SEGMENTS so both forms are emitted).
In `@tests/lib/resolve-ecc-root.test.js`:
- Around line 161-193: Update the three test description strings passed to
test(...) that currently say "marketplace" to use the plural "marketplaces" so
they match the fixture calls (setupLegacyPluginInstall(..., ['marketplaces',
...]) and setupPluginCache usage) and avoid misleading output; locate the tests
that call setupLegacyPluginInstall and setupPluginCache and change their labels
like "finds marketplace current plugin install at
~/.claude/plugins/marketplace/ecc" to "finds marketplaces current plugin install
at ~/.claude/plugins/marketplaces/ecc" (and similarly for
"everywhere-claude-code" and the "prefers exact legacy plugin..." case) while
leaving resolveEccRoot, setupLegacyPluginInstall, and setupPluginCache usage
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0ac0863e-bf72-4ae1-bb63-d12619090264
📒 Files selected for processing (7)
commands/sessions.mdcommands/skill-health.mdhooks/hooks.jsonscripts/hooks/session-start-bootstrap.jsscripts/lib/resolve-ecc-root.jstests/lib/command-plugin-root.test.jstests/lib/resolve-ecc-root.test.js
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="hooks/hooks.json">
<violation number="1" location="hooks/hooks.json:243">
P2: Partial rename from `marketplace` to `marketplaces` created inconsistent plugin-root resolution across hooks, which can make some hook phases skip execution.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
1 similar comment
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
1 participant
What Changed
Link: #1357
correct typo
marketplacetomarketplacesWhy This Change
Summary by cubic
Fixes a path typo by switching
marketplacetomarketplacesacross resolvers, hooks, docs, and tests so installs under~/.claude/plugins/marketplaces/...(foreccand legacyeverything-claude-code) are correctly detected and used.scripts/lib/resolve-ecc-root.jsandscripts/hooks/session-start-bootstrap.jsto usemarketplaces.hooks/hooks.jsontomarketplaces.commands/sessions.md,commands/skill-health.md, and tests to reference themarketplacespath.Written for commit f5209b3. Summary will update on new commits.
Summary by CodeRabbit
Refactoring
marketplacetomarketplacesacross configuration, hooks, and scripts to align with standardized naming conventions.Tests