refactor: consolidate plugins to hybrid structure with root everything-agent - #494
Conversation
Update plugin descriptions to match their actual plugin.json files: - LSP plugins: clarified integration details and specific features - databricks-devtools: updated to reflect Unity Catalog focus - jira: removed Confluence mention, focused on Jira workflows - me: added auto-install behavior note - ralph-loop: added loop execution details Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove plugins and features that are being consolidated into everything-agent: - plugins/me/ - plugins/suggest-compacting/ - plugins/ralph-loop/ - scripts/ralph/ - skills/suggest-compacting/ - src/suggest-compacting/ - commands/cancel-ralph.md, ralph-*.md - agents/ralph/config.sh.example - tests for ralph-loop and suggest-compacting - .gitignore entry for suggest-compacting/dist/ Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update test files to validate root everything-agent plugin alongside plugins directory entries: - tests/marketplace_json.bats: Add test for root plugin in marketplace - tests/plugin_json.bats: Include root manifest in all validation loops - tests/plugin_validation_common.bats: Add root plugin checks - tests/validate_plugin_manifest.bats: Include root manifest validation - tests/performance/benchmarks.bats: Include root plugin in benchmarks Note: Some marketplace tests will fail until marketplace.json is updated in the next task to include the root plugin entry. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add everything-agent root plugin entry (source: "./") - Remove deprecated plugin entries: me, suggest-compacting, ralph-loop - Keep core plugins: git-guard, handoff, jira, databricks-devtools, lsp-* This completes the marketplace restructuring for the hybrid plugin model. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove all references to deleted plugins (ralph-loop, me, strategic-compact) - Add new plugins to available plugins list (databricks-devtools, handoff, lsp-*) - Update project structure to reflect hybrid model (root + plugins) - Update test count from 178 to 163 (actual count) - Fix duplicate performance benchmarks section in TESTING.md - Add canonical plugin notice for everything-agent Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These changes were part of Task 2 (test/release assumptions adjustment) but were not included in the previous commits: - .releaserc.js: add root plugin discovery and version sync - tests/helpers/bats_helper.bash: include root manifest in iteration - tests/helpers/marketplace_helper.bash: support root plugin validation - tests/helpers/test_utils.bash: handle root plugin in checks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR consolidates the plugin architecture by promoting suggest-compacting to a root-level "everything-agent" plugin, removing the "me" and "ralph-loop" plugins entirely, updating marketplace discovery to handle both root and per-plugin manifests, and removing Ralph-specific documentation and tests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
tests/helpers/marketplace_helper.bash (1)
137-174:⚠️ Potential issue | 🟡 MinorInconsistent name-matching strategy: root plugin uses
.namefrom JSON, per-plugin uses directory basename.For the root plugin (lines 144–145), the name is read from
plugin.jsonviajq -r '.name'. For per-plugin directories (line 162), the name is derived frombasename "$plugin_dir". If a plugin's directory name ever differs from its.namefield inplugin.json, the per-plugin check will compare the wrong value against marketplace entries.Consider reading
.namefrom each plugin'splugin.jsonas well, for consistency with the root plugin logic:Proposed fix
# Skip if no plugin.json if [ ! -f "${plugin_dir}/.claude-plugin/plugin.json" ]; then continue fi - # Check if plugin is in marketplace.json by name - if ! echo "$marketplace_plugin_names" | grep -q "^${plugin_name}$"; then + # Read the actual plugin name from plugin.json for consistency + local manifest_name + manifest_name=$($JQ_BIN -r '.name' "${plugin_dir}/.claude-plugin/plugin.json" 2>/dev/null) + if [ -z "$manifest_name" ] || [ "$manifest_name" = "null" ]; then + manifest_name="$plugin_name" + fi + + # Check if plugin is in marketplace.json by name + if ! echo "$marketplace_plugin_names" | grep -q "^${manifest_name}$"; then - echo "Error: Plugin '$plugin_name' not listed in marketplace.json" >&2 + echo "Error: Plugin '$manifest_name' (dir: $plugin_name) not listed in marketplace.json" >&2 ((missing++)) fitests/helpers/test_utils.bash (1)
777-810:⚠️ Potential issue | 🟠 MajorRoot manifest is not included in
check_all_plugin_manifests(root cause of downstream gap).
check_all_plugin_manifests(and similarlycount_valid_pluginsat line 821,get_invalid_pluginsat line 853) only iterates overfind_all_plugins(), which searchesplugins/*/. The root canonical manifest at${PROJECT_ROOT}/.claude-plugin/plugin.jsonis never validated by these functions, even though the rest of this PR treats it as a first-class manifest.This is the root cause of the inconsistency in
tests/plugin_json.batswhere the individual tests cover the root manifest but the comprehensive/count/invalid tests do not.Suggested approach for check_all_plugin_manifests
check_all_plugin_manifests() { local output_file="${1:-/dev/stderr}" local plugin_dirs local total=0 local valid=0 local invalid=0 + # Validate root canonical plugin manifest + local root_manifest="${PROJECT_ROOT}/.claude-plugin/plugin.json" + if [ -f "$root_manifest" ]; then + ((total++)) + local result_file="${TEST_TEMP_DIR}/validation_$$_root.txt" + if validate_plugin_manifest_comprehensive "$root_manifest" "$result_file" 2>&1; then + ((valid++)) + else + ((invalid++)) + cat "$result_file" >> "$output_file" + fi + rm -f "$result_file" + fi + plugin_dirs=$(find_all_plugins)Apply the same pattern to
count_valid_pluginsandget_invalid_plugins.tests/plugin_validation_common.bats (1)
221-241:⚠️ Potential issue | 🟡 Minor
count_valid_pluginsdoesn't include root manifest, making this assertion fragile.
count_valid_pluginsonly counts plugins underplugins/, whiletotal_pluginsat line 233 also only counts plugin directories. So the math works out today, but the test is silently excluding the root canonical plugin from both sides. Ifcount_valid_pluginsis later fixed to include the root manifest (as suggested), this test will break becausetotal_pluginswon't include it.Consider adding a note or TODO, or proactively adjusting the total count to include the root manifest.
tests/plugin_json.bats (1)
176-204:⚠️ Potential issue | 🟠 Major
check_all_plugin_manifests,count_valid_plugins, andget_invalid_pluginsexclude the root manifest.These functions delegate to
find_all_plugins()(line 28 in test_utils.bash), which searches onlyfind "$PROJECT_ROOT/plugins" -mindepth 1 -maxdepth 1, and therefore iterate overplugins/*/directories only. The root manifest at${PROJECT_ROOT}/.claude-plugin/plugin.jsonis silently excluded from comprehensive validation, plugin counting, and invalid-plugin detection. A broken root manifest would be caught by individual tests but pass the "all plugin.json files pass comprehensive validation" test.
🤖 Fix all issues with AI agents
In @.claude-plugin/marketplace.json:
- Line 27: Update the plugin version string for the everything-agent entry in
.claude-plugin/marketplace.json from "5.23.1" to "5.23.3" so it matches the rest
of the marketplace plugins; locate the JSON key "version" (currently "5.23.1")
for the everything-agent and replace its value with "5.23.3", then save and run
any local validation (lint/JSON parse) to ensure the file remains valid.
In `@tests/plugin_validation_common.bats`:
- Around line 86-95: The current recursive find (plugins_manifests=$(find
"$PROJECT_ROOT/plugins" -name "plugin.json" -type f ...)) can capture unintended
files; narrow the search to only the expected plugin manifest locations by
changing the find criteria to target the specific plugin layout (e.g., only
match "$PROJECT_ROOT/plugins/*/.claude-plugin/plugin.json" or use a
-path/-maxdepth combination) so that variables like plugins_manifests,
plugins_count and manifest_files only include true plugin manifests and not
node_modules or nested fixtures.
🧹 Nitpick comments (7)
tests/run-all-tests.sh (1)
10-10: Pre-existing:set -edefeats the failure-collection design.
run_testsreturns 1 on failure (Line 41), but it's invoked as a simple statement at Lines 55 and 66. Withset -eactive, the script will exit on the first failure, never reaching the summary block. Consider prefixing calls withrun_tests ... || true(failures are already tracked inFAILED_TESTS).Not introduced by this PR, so just flagging for awareness.
Also applies to: 55-55, 66-66
tests/helpers/bun.ts (2)
448-454: Fragile error discrimination via string matching on the message.The re-throw logic relies on
error.message.includes('missing from')to distinguish validation errors from JSON parse errors. If the error message wording changes, this will silently swallow validation failures.Consider using a custom error class or a typed property instead:
♻️ Use a dedicated error type for validation failures
+class MarketplaceValidationError extends Error { + constructor(message: string) { + super(message) + this.name = 'MarketplaceValidationError' + } +} + // ...inside validateMarketplaceIncludesAllPlugins: if (rootManifest.name && !marketplacePluginNames.has(rootManifest.name)) { - throw new Error( + throw new MarketplaceValidationError( `Root plugin '${rootManifest.name}' missing from ${marketplacePath}` ) } } catch (error) { - // Re-throw if it's our validation error - if (error instanceof Error && error.message.includes('missing from')) { + if (error instanceof MarketplaceValidationError) { throw error } // Ignore other parse errors }
404-436: Root manifest read twice — once for marketplace name extraction, once for validation.When the marketplace entry for the root plugin uses
source: "./"without anamefield, the root manifest is parsed at Line 420. It's then parsed again at Line 442 for the explicit root validation check. This is a minor redundancy.♻️ Cache the root plugin name to avoid double-parsing
+ // Pre-read root plugin name for reuse + let rootPluginName: string | undefined + const rootPluginJsonPath = join(PROJECT_ROOT, '.claude-plugin', 'plugin.json') + if (existsSync(rootPluginJsonPath)) { + try { + const rootManifest = validateJson<PluginManifest>(rootPluginJsonPath) + rootPluginName = rootManifest.name + } catch { + // Ignore parse errors + } + } + // Extract plugin names from marketplace ... for (const p of marketplace.plugins) { if (p.name) { marketplacePluginNames.add(p.name) } else { if (p.source === './') { - const rootManifestPath = join(PROJECT_ROOT, '.claude-plugin', 'plugin.json') - if (existsSync(rootManifestPath)) { - try { - const manifest = validateJson<PluginManifest>(rootManifestPath) - if (manifest.name) { - marketplacePluginNames.add(manifest.name) - } - } catch { } + if (rootPluginName) { + marketplacePluginNames.add(rootPluginName) } } else { ... } } } // Check for root canonical plugin - const rootPluginJsonPath = join(PROJECT_ROOT, '.claude-plugin', 'plugin.json') - if (existsSync(rootPluginJsonPath)) { - try { - const rootManifest = validateJson<PluginManifest>(rootPluginJsonPath) - if (rootManifest.name && !marketplacePluginNames.has(rootManifest.name)) { - throw new Error(...) - } - } catch (error) { ... } + if (rootPluginName && !marketplacePluginNames.has(rootPluginName)) { + throw new Error( + `Root plugin '${rootPluginName}' missing from ${marketplacePath}` + ) }Also applies to: 438-455
tests/validate_plugin_manifest.bats (2)
128-137: Inconsistent pattern:foundis incremented on Line 128 but overwritten on Line 137.Since
foundis only used as a boolean (-gt 0), this works correctly, but it's confusing to mix$((found + 1))withfound=1.♻️ Use consistent assignment
if [ -f "$root_manifest" ]; then manifest_files="$root_manifest"$'\n' - found=$((found + 1)) + found=1 fi
131-133: Samefinddepth concern asbats_helper.bash— unrestricted recursive search.This
findcommand will match anyplugin.jsonanywhere underplugins/, not just at the expected.claude-plugin/plugin.jsonpath. Consider applying the same-pathfilter suggested forfor_each_plugin_manifestinbats_helper.bash.♻️ Restrict find to expected manifest paths
local plugins_manifests - plugins_manifests=$(find "$PROJECT_ROOT/plugins" -name "plugin.json" -type f 2>/dev/null) + plugins_manifests=$(find "$PROJECT_ROOT/plugins" -maxdepth 3 -path "*/.claude-plugin/plugin.json" -type f 2>/dev/null)tests/helpers/bats_helper.bash (1)
194-195: Align plugin discovery with other implementations to prevent inconsistency.The
findcommand uses a broad pattern that doesn't match howbun.tsand.releaserc.jsdiscover plugins. Both restrict toplugins/<name>/.claude-plugin/plugin.json, while this code searches for anyplugin.jsonin the entire tree. Apply the same pattern restriction:♻️ Restrict find to match established .claude-plugin pattern
# Find all plugin manifests in plugins directory local plugins_manifests - plugins_manifests=$(find "$PROJECT_ROOT/plugins" -name "plugin.json" -type f 2>/dev/null) + plugins_manifests=$(find "$PROJECT_ROOT/plugins" -maxdepth 3 -path "*/.claude-plugin/plugin.json" -type f 2>/dev/null)tests/plugin_json.bats (1)
16-20: Consider extracting the root manifest check into a shared helper to reduce duplication.The same root-manifest-check block is copy-pasted across six test functions:
local root_manifest="${PROJECT_ROOT}/.claude-plugin/plugin.json" if [ -f "$root_manifest" ]; then # validation logic fiA helper that collects all manifest paths (root + per-plugin) into an array would let each test iterate a single list, eliminating ~60 lines of duplicated boilerplate. The helpers file (
bats_helper.bash) already has patterns likeget_all_plugins; a similarget_all_plugin_manifeststhat returns root + per-plugin manifest paths would centralize this.Also applies to: 36-42, 61-70, 91-100, 122-133, 156-162
| "mcp" | ||
| ], | ||
| "version": "5.23.3" | ||
| "version": "5.23.1" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat .claude-plugin/marketplace.json | head -50Repository: baleen37/everything-agent
Length of output: 1362
🏁 Script executed:
fd -t f 'plugin.json' --exec cat {} \;Repository: baleen37/everything-agent
Length of output: 9837
🏁 Script executed:
fd -t f '.releaserc.js' --exec cat {} \;Repository: baleen37/everything-agent
Length of output: 51
🏁 Script executed:
# Get version of everything-agent and other plugins from marketplace.json
rg '"version":\s*"[^"]+' .claude-plugin/marketplace.json -oRepository: baleen37/everything-agent
Length of output: 296
🏁 Script executed:
cat .claude-plugin/plugin.json 2>/dev/null || echo "File not found"Repository: baleen37/everything-agent
Length of output: 1714
🏁 Script executed:
cat package.json | head -30Repository: baleen37/everything-agent
Length of output: 947
🏁 Script executed:
# Check git history to see if this version change was intentional
git log --oneline -n 20 -- .claude-plugin/marketplace.jsonRepository: baleen37/everything-agent
Length of output: 133
🏁 Script executed:
fd -t f '\.releaserc' -o '.releaserc.json' -o 'release.config.js' -o 'release.config.mjs'Repository: baleen37/everything-agent
Length of output: 218
🏁 Script executed:
# Check if there's a script that manages/syncs versions
rg 'marketplace|5\.23\.' --type json --type js -A 2 -B 2 | head -100Repository: baleen37/everything-agent
Length of output: 2175
🏁 Script executed:
# Look for release or version-related scripts
fd -t f -e 'sh' -e 'js' -e 'mjs' | xargs rg 'marketplace|version.*sync' -lRepository: baleen37/everything-agent
Length of output: 104
🏁 Script executed:
cat scripts/sync-marketplace-version.shRepository: baleen37/everything-agent
Length of output: 509
🏁 Script executed:
cat .releaserc.jsRepository: baleen37/everything-agent
Length of output: 5889
Synchronize everything-agent version to match all other plugins.
The root plugin is at version 5.23.1 while all other plugins in the marketplace are at 5.23.3. The semantic-release configuration explicitly detects and warns about these mismatches and synchronizes all plugin versions during releases. Update the version to 5.23.3.
Proposed fix
- "version": "5.23.1"
+ "version": "5.23.3"📝 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.
| "version": "5.23.1" | |
| "version": "5.23.3" |
🤖 Prompt for AI Agents
In @.claude-plugin/marketplace.json at line 27, Update the plugin version string
for the everything-agent entry in .claude-plugin/marketplace.json from "5.23.1"
to "5.23.3" so it matches the rest of the marketplace plugins; locate the JSON
key "version" (currently "5.23.1") for the everything-agent and replace its
value with "5.23.3", then save and run any local validation (lint/JSON parse) to
ensure the file remains valid.
| # Find all plugin manifests in plugins directory | ||
| local plugins_manifests | ||
| plugins_manifests=$(find "$PROJECT_ROOT/plugins" -name "plugin.json" -type f 2>/dev/null) | ||
|
|
||
| # Count manifests found - use file count instead of loop | ||
| local count | ||
| count=$(echo "$manifest_files" | grep -c '^') | ||
| if [ -n "$plugins_manifests" ]; then | ||
| manifest_files="${manifest_files}${plugins_manifests}" | ||
| local plugins_count | ||
| plugins_count=$(echo "$plugins_manifests" | grep -c '^') | ||
| count=$((count + plugins_count)) | ||
| fi |
There was a problem hiding this comment.
Broader find scope may pick up unintended plugin.json files.
Line 88 uses find "$PROJECT_ROOT/plugins" -name "plugin.json" -type f, which recursively searches all depths. Other tests in this PR use a stricter glob (plugins/*/.claude-plugin/plugin.json). This broader find could match files in node_modules/, nested test fixtures, or other non-standard locations.
Consider constraining the path:
Suggested fix
- plugins_manifests=$(find "$PROJECT_ROOT/plugins" -name "plugin.json" -type f 2>/dev/null)
+ plugins_manifests=$(find "$PROJECT_ROOT/plugins" -path "*/.claude-plugin/plugin.json" -type f 2>/dev/null)📝 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.
| # Find all plugin manifests in plugins directory | |
| local plugins_manifests | |
| plugins_manifests=$(find "$PROJECT_ROOT/plugins" -name "plugin.json" -type f 2>/dev/null) | |
| # Count manifests found - use file count instead of loop | |
| local count | |
| count=$(echo "$manifest_files" | grep -c '^') | |
| if [ -n "$plugins_manifests" ]; then | |
| manifest_files="${manifest_files}${plugins_manifests}" | |
| local plugins_count | |
| plugins_count=$(echo "$plugins_manifests" | grep -c '^') | |
| count=$((count + plugins_count)) | |
| fi | |
| # Find all plugin manifests in plugins directory | |
| local plugins_manifests | |
| plugins_manifests=$(find "$PROJECT_ROOT/plugins" -path "*/.claude-plugin/plugin.json" -type f 2>/dev/null) | |
| if [ -n "$plugins_manifests" ]; then | |
| manifest_files="${manifest_files}${plugins_manifests}" | |
| local plugins_count | |
| plugins_count=$(echo "$plugins_manifests" | grep -c '^') | |
| count=$((count + plugins_count)) | |
| fi |
🤖 Prompt for AI Agents
In `@tests/plugin_validation_common.bats` around lines 86 - 95, The current
recursive find (plugins_manifests=$(find "$PROJECT_ROOT/plugins" -name
"plugin.json" -type f ...)) can capture unintended files; narrow the search to
only the expected plugin manifest locations by changing the find criteria to
target the specific plugin layout (e.g., only match
"$PROJECT_ROOT/plugins/*/.claude-plugin/plugin.json" or use a -path/-maxdepth
combination) so that variables like plugins_manifests, plugins_count and
manifest_files only include true plugin manifests and not node_modules or nested
fixtures.
1 participant
Summary
everything-agent중심의 하이브리드 모델로 재구성everything-agent, source:./) 추가me,suggest-compacting,ralph-loopChanges
Marketplace & Structure
.claude-plugin/marketplace.json: rooteverything-agent엔트리 추가, deprecated 플러그인 제거plugins/me/,plugins/suggest-compacting/,plugins/ralph-loop/디렉터리 삭제Test Infrastructure
.releaserc.js: root 플러그인 버전 동기화 지원tests/helpers/*.bash,tests/helpers/bun.ts: root 플러그인 검증 지원tests/*.bats: root manifest 검증 추가Documentation
README.md: 삭제된 플러그인 참조 제거, 하이브리드 구조 설명 추가docs/TESTING.md: 테스트 경로 및 개수(163개) 업데이트Test Plan
bats tests/- 163 tests passbash tests/run-all-tests.sh- All tests passbash tests/run-unit-tests.sh- Unit tests passpre-commit run --all-files- Passes (existing markdownlint warnings in plan files)🤖 Generated with Claude Code
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com
Summary by CodeRabbit
Release Notes
New Features
Plugin Updates
Removed
Improvements