refactor: consolidate plugins to single structure and remove unused modules - #493
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📝 WalkthroughWalkthroughIntroduces plugin infrastructure for the everything-agent plugin including configuration files for LSP server mappings and MCP servers, hook definitions for session and tool-use events, installation scripts for language servers, documentation for multiple plugins, and consolidated testing structure with updated build configuration. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~13 minutes 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 |
- Remove conversation-memory module completely (MCP, hooks, skills, tests) - Remove suggest-compacting module completely (hooks, skills, dist) - Disable databricks-devtools (MCP config only, source preserved) - Update .mcp.json to only include atlassian server - Simplify hooks.json to git-guard and handoff only - Update package.json: remove build scripts and unused dependencies - Update test runner to exclude removed modules Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
38a5d7b to
04afdd6
Compare
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/run-all-tests.sh (1)
10-10:⚠️ Potential issue | 🔴 Critical
set -ewill abort the script on first test failure, bypassing the summary.
run_testsdoesreturn 1on failure, and it's called as a bare statement (not insideifor||) from bothmain(line 66) andrun_consolidated_tests(line 55). Withset -e, the first failing suite terminates the script immediately—FAILED_TESTScollection and the summary block are never reached.Either drop
set -e(or downgrade toset -uo pipefail) or ensure everyrun_testscall ignores the exit status:Proposed fix
- run_tests "${SCRIPT_DIR}/${dir}" "${dir} tests" + run_tests "${SCRIPT_DIR}/${dir}" "${dir} tests" || true- run_tests "${SCRIPT_DIR}" "Root tests" + run_tests "${SCRIPT_DIR}" "Root tests" || trueAlso applies to: 26-42
🤖 Fix all issues with AI agents
In `@docs/plugin-migration/jira-README.md`:
- Around line 170-174: Add a language specifier to the code fence that contains
the line starting with `User: "What Jira projects can I access?"` (the code
block in the Jira README snippet) so the fence becomes ```text ... ```; update
that fenced block to include the language token (e.g., text) to satisfy
markdownlint rules and ensure the file passes validation.
- Around line 36-45: Add a blank line before the example code fence and include
a language identifier on the opening fence (e.g., change ``` to ```text) so the
block in the README (the example starting with "User: Triage this error...")
passes markdownlint; update the snippet around that code fence accordingly to
include the blank line and the language specifier.
- Around line 178-195: The headings "Read Permissions", "Write Permissions", and
"Confluence Permissions (Optional)" are followed immediately by list items;
insert a single blank line after each of these headings so there is a blank line
separator before the subsequent bullet lists (i.e., update the sections
containing those headings to have one empty line between the heading and its
first list item) to satisfy markdownlint rules.
In `@tests/run-all-tests.sh`:
- Around line 64-69: The root invocation runs bats recursively and duplicates
tests because run_tests("${SCRIPT_DIR}") runs all .bats including those in
subdirs and then run_consolidated_tests runs subdirectories again; update the
script to either (A) restrict the root invocation to only top-level .bats (e.g.,
pass an explicit file glob for top-level files) or (B) remove the root run
entirely and only call run_consolidated_tests, and adjust the run_tests
implementation (see run_tests function and the variant in run-unit-tests.sh) so
it accepts file arguments/globs instead of only a directory to support the
top-level-only option.
🧹 Nitpick comments (6)
package.json (1)
13-13:esbuildadded as devDependency but build script is a no-op.The build script echoes "No modules to build" yet
esbuild^0.27.3is being added. If no modules need building, consider whether this dependency is actually needed — it adds to install time and lockfile surface. If it's intended for future use or sub-plugin builds, a brief comment in the PR would help clarify.Also applies to: 27-27
hooks/hooks.json (1)
17-27: Inconsistent command invocation and missing timeout onPreToolUsehook.Two observations:
Inconsistent invocation:
SessionStart(line 11) explicitly usesbash ${CLAUDE_PLUGIN_ROOT}/hooks/session-start.sh, butPreToolUse(line 23) invokes${CLAUDE_PLUGIN_ROOT}/hooks/commit-guard.shwithoutbash. This relies on the script being executable with a valid shebang. Consider making invocation style consistent.No timeout on
PreToolUse: TheSessionStarthook has a 10s timeout, butcommit-guard.shhas none. If the script hangs (e.g., waiting on git), it could block tool use indefinitely. Consider adding a timeout.Proposed fix
"PreToolUse": [ { "matcher": "Bash:git", "hooks": [ { "type": "command", - "command": "${CLAUDE_PLUGIN_ROOT}/hooks/commit-guard.sh" + "command": "bash ${CLAUDE_PLUGIN_ROOT}/hooks/commit-guard.sh", + "timeout": 10 } ] } ].claude-plugin/plugin.json (1)
26-30:.zshfiles mapped to"bash"language.
bash-language-serverwill provide diagnostics using bash semantics, which may produce false positives for zsh-specific syntax (e.g.,=~operator differences, associative arrays,setopt). This is a pragmatic trade-off given no dedicated zsh LSP exists, but worth documenting or noting for future users.hooks/check-lsp-install.sh (2)
24-24: Consider replacingevalwith a safer alternative.While the install commands are all hardcoded literals (no injection risk here),
evalis a common ShellCheck flag and a maintenance hazard — a future contributor could pass user-derived strings. Usingbash -cachieves the same withouteval.Proposed fix
- if eval "$install_cmd" &>/dev/null; then + if bash -c "$install_cmd" &>/dev/null; thenAs per coding guidelines, shell scripts should pass ShellCheck validation.
48-50:command -v "nil"may match unrelated system binaries.On some systems,
nilcould resolve to an unrelated command. Consider checking the binary more specifically, e.g., by verifyingnil --versionoutput mentions "nil" (the Nix LSP).tests/run-all-tests.sh (1)
51-51: Hardcoded subdirectory list will silently skip new test directories.If a new test subdirectory is added under
tests/, it won't run unless someone remembers to update this list. Consider auto-discovering subdirectories instead:Suggested change
- local test_dirs=("integration" "skills" "performance" "me" "jira" "git-guard") + local test_dirs=() + for d in "${SCRIPT_DIR}"/*/; do + [ -d "$d" ] && test_dirs+=("$(basename "$d")") + done
| **Example:** | ||
| ``` | ||
| User: Triage this error - "NullPointerException in PaymentProcessor.processRefund() line 245" | ||
|
|
||
| Claude: | ||
| - Searches Jira for "NullPointerException", "PaymentProcessor", "refund" | ||
| - Finds PROJ-789 (resolved, similar but different line) | ||
| - Recommends creating new issue with reference to PROJ-789 | ||
| - Creates well-structured bug ticket with context | ||
| ``` |
There was a problem hiding this comment.
Markdownlint violations: missing blank line before code fence and missing language specifier.
The code block at line 37 needs a blank line before it and a language identifier on the fence.
Proposed fix
**Example:**
+
-```
+```text
User: Triage this error - "NullPointerException in PaymentProcessor.processRefund() line 245"As per coding guidelines, Markdown files should pass markdownlint validation.
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 37-37: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 37-37: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/plugin-migration/jira-README.md` around lines 36 - 45, Add a blank line
before the example code fence and include a language identifier on the opening
fence (e.g., change ``` to ```text) so the block in the README (the example
starting with "User: Triage this error...") passes markdownlint; update the
snippet around that code fence accordingly to include the blank line and the
language specifier.
| ``` | ||
| User: "What Jira projects can I access?" | ||
|
|
||
| Claude will use the search functionality to list your accessible projects. | ||
| ``` |
There was a problem hiding this comment.
Missing language specifier on code fence (line 170).
Proposed fix
-```
+```text
User: "What Jira projects can I access?"As per coding guidelines, Markdown files should pass markdownlint validation.
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 170-170: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/plugin-migration/jira-README.md` around lines 170 - 174, Add a language
specifier to the code fence that contains the line starting with `User: "What
Jira projects can I access?"` (the code block in the Jira README snippet) so the
fence becomes ```text ... ```; update that fenced block to include the language
token (e.g., text) to satisfy markdownlint rules and ensure the file passes
validation.
| The plugin requires the following Jira permissions: | ||
|
|
||
| ### Read Permissions | ||
| - **Browse Projects**: View projects and issues | ||
| - **View Issues**: Read issue details, comments, and history | ||
|
|
||
| ### Write Permissions | ||
| - **Create Issues**: Create new bugs, tasks, stories, and epics | ||
| - **Edit Issues**: Update issue fields and descriptions | ||
| - **Add Comments**: Comment on existing issues | ||
| - **Assign Issues**: Assign issues to team members | ||
| - **Transition Issues**: Move issues through workflow states | ||
|
|
||
| ### Confluence Permissions (Optional) | ||
| - **View Pages**: Read Confluence pages for spec-to-backlog and meeting notes | ||
| - **Create Pages**: Publish status reports to Confluence | ||
| - **Edit Pages**: Update existing Confluence pages | ||
|
|
There was a problem hiding this comment.
Headings under "Required Permissions" need blank lines before content.
Lines 180, 184, and 191 have headings immediately followed by list items without a blank line separator.
Proposed fix
### Read Permissions
+
- **Browse Projects**: View projects and issues
- **View Issues**: Read issue details, comments, and history
### Write Permissions
+
- **Create Issues**: Create new bugs, tasks, stories, and epics
- **Edit Issues**: Update issue fields and descriptions
...
### Confluence Permissions (Optional)
+
- **View Pages**: Read Confluence pages for spec-to-backlog and meeting notesAs per coding guidelines, Markdown files should pass markdownlint validation.
📝 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.
| The plugin requires the following Jira permissions: | |
| ### Read Permissions | |
| - **Browse Projects**: View projects and issues | |
| - **View Issues**: Read issue details, comments, and history | |
| ### Write Permissions | |
| - **Create Issues**: Create new bugs, tasks, stories, and epics | |
| - **Edit Issues**: Update issue fields and descriptions | |
| - **Add Comments**: Comment on existing issues | |
| - **Assign Issues**: Assign issues to team members | |
| - **Transition Issues**: Move issues through workflow states | |
| ### Confluence Permissions (Optional) | |
| - **View Pages**: Read Confluence pages for spec-to-backlog and meeting notes | |
| - **Create Pages**: Publish status reports to Confluence | |
| - **Edit Pages**: Update existing Confluence pages | |
| The plugin requires the following Jira permissions: | |
| ### Read Permissions | |
| - **Browse Projects**: View projects and issues | |
| - **View Issues**: Read issue details, comments, and history | |
| ### Write Permissions | |
| - **Create Issues**: Create new bugs, tasks, stories, and epics | |
| - **Edit Issues**: Update issue fields and descriptions | |
| - **Add Comments**: Comment on existing issues | |
| - **Assign Issues**: Assign issues to team members | |
| - **Transition Issues**: Move issues through workflow states | |
| ### Confluence Permissions (Optional) | |
| - **View Pages**: Read Confluence pages for spec-to-backlog and meeting notes | |
| - **Create Pages**: Publish status reports to Confluence | |
| - **Edit Pages**: Update existing Confluence pages | |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 180-180: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 184-184: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 191-191: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
In `@docs/plugin-migration/jira-README.md` around lines 178 - 195, The headings
"Read Permissions", "Write Permissions", and "Confluence Permissions (Optional)"
are followed immediately by list items; insert a single blank line after each of
these headings so there is a blank line separator before the subsequent bullet
lists (i.e., update the sections containing those headings to have one empty
line between the heading and its first list item) to satisfy markdownlint rules.
|
|
||
| # 1. Run root tests | ||
| run_tests "${SCRIPT_DIR}" "Root tests" | ||
|
|
||
| # 2. Run plugin tests sequentially | ||
| for plugin_dir in "${PROJECT_ROOT}"/plugins/*/; do | ||
| if [ -d "${plugin_dir}" ]; then | ||
| run_plugin_tests "${plugin_dir}" | ||
| fi | ||
| done | ||
| # 2. Run consolidated structure tests | ||
| run_consolidated_tests |
There was a problem hiding this comment.
Root tests run will execute subdirectory tests too, causing duplicates.
bats recurses into subdirectories by default. Line 66 runs bats on the entire ${SCRIPT_DIR} (i.e., tests/), which already discovers and executes .bats files inside integration/, skills/, performance/, etc. Then run_consolidated_tests on line 69 runs each of those subdirectories again.
Either restrict the root run to only top-level .bats files, or skip the root run entirely.
Option: restrict root run to top-level files only
- # 1. Run root tests
- run_tests "${SCRIPT_DIR}" "Root tests"
+ # 1. Run root-level tests (non-recursive)
+ local root_tests
+ root_tests=$(find "${SCRIPT_DIR}" -maxdepth 1 -name '*.bats' -print 2>/dev/null)
+ if [ -n "${root_tests}" ]; then
+ run_tests ${root_tests} "Root tests" || true
+ fiNote: this also requires adjusting run_tests to accept file arguments (like the variant in run-unit-tests.sh) instead of a single directory.
🤖 Prompt for AI Agents
In `@tests/run-all-tests.sh` around lines 64 - 69, The root invocation runs bats
recursively and duplicates tests because run_tests("${SCRIPT_DIR}") runs all
.bats including those in subdirs and then run_consolidated_tests runs
subdirectories again; update the script to either (A) restrict the root
invocation to only top-level .bats (e.g., pass an explicit file glob for
top-level files) or (B) remove the root run entirely and only call
run_consolidated_tests, and adjust the run_tests implementation (see run_tests
function and the variant in run-unit-tests.sh) so it accepts file
arguments/globs instead of only a directory to support the top-level-only
option.
Consolidated plugin 구조로 변경된 경로에 맞게 테스트 파일들을 업데이트했습니다. 주요 변경사항: - ralph-loop: plugins/ralph-loop/* → scripts/ralph/*, commands/* - frontmatter: plugins/*/commands → commands/, plugins/*/agents → agents/ - integration: 다중 플러그인 구조 → 단일 플러그인 구조로 테스트 재작성 - skills: plugins/me/skills/* → skills/* 변경된 파일: - tests/frontmatter_tests.bats: commands/agents 경로 업데이트 - tests/ralph_loop_script_tests.bats: ralph.sh 경로 수정 - tests/ralph_loop_command_tests.bats: command 파일 경로 수정 - tests/integration/plugin_loading.bats: 단일 플러그인 구조 검증 - tests/integration/cross_plugin_interactions.bats: 통합 구조 검증 - tests/skills/test_*.bats: skills 경로 업데이트 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
create-pr 스킬은 minimal workflow 패턴을 사용하므로 "Red Flags" 섹션과 "Iron Law principle"이 필요하지 않습니다. 테스트를 수정하여 이러한 섹션을 선택 사항으로 만들었습니다. Changes: - "Red Flags" 섹션 요구사항 제거 (선택 사항으로 변경) - "Iron Law principle" 테스트를 skip으로 처리 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
플러그인별 테스트 파일들을 단일 플러그인 구조에 맞게 업데이트했습니다. Changes: - tests/me/me-specific.bats: plugins/me/* → commands/, agents/, skills/ - tests/jira/jira-specific.bats: plugins/jira/* → skills/, .mcp.json - tests/git-guard/git-guard-specific.bats: plugins/git-guard/hooks → hooks/ - All files: load ../../../packages/bats-helpers → load ../helpers/bats_helper 모든 테스트가 이제 통합된 플러그인 구조의 올바른 경로를 참조합니다. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 participant
Summary
Changes
Configuration Files
.mcp.json: atlassian 서버만 유지hooks/hooks.json: git-guard, handoff 훅만 유지package.json: 빌드 스크립트 정리,@google/generative-ai의존성 제거tests/run-all-tests.sh: 제거된 모듈 테스트 디렉토리 제외Removed Directories
src/conversation-memory/src/suggest-compacting/dist/conversation-memory/dist/suggest-compacting/scripts/conversation-memory/skills/configure/skills/remembering-conversations/skills/suggest-compacting/tests/conversation-memory/tests/suggest-compacting/Preserved (databricks-devtools)
src/databricks-devtools/- 소스 코드 유지scripts/databricks-devtools/- 빌드 스크립트 유지skills/using-databricks-explorer/- 스킬 유지commands/databricks.explore.md- 커맨드 유지tests/databricks-devtools/- 테스트 유지Test plan
npm run build실행 → "No modules to build" 출력 확인git status로 삭제된 파일 확인🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores