refactor(lsp): remove standalone lsp plugins and lsp hooks from me plugin#583
refactor(lsp): remove standalone lsp plugins and lsp hooks from me plugin#583
Conversation
📝 WalkthroughWalkthroughThe PR removes LSP (Language Server Protocol) plugin support by eliminating five standalone LSP plugins ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Repository rule violations found
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 (1)
plugins/me/.claude-plugin/plugin.json (1)
4-20:⚠️ Potential issue | 🟠 MajorLSP removal is incomplete: SessionStart still invokes deleted hook scripts.
plugins/me/hooks/hooks.json(Line 1-50) still callshandoff-session-start.shplus multiplelsp-*-check-install.shcommands that were removed. This will produce command failures during session start.Proposed follow-up diff (`plugins/me/hooks/hooks.json`)
"hooks": { "SessionStart": [ { "matcher": "*", "hooks": [ - { - "type": "command", - "command": "bash ${CLAUDE_PLUGIN_ROOT}/hooks/handoff-session-start.sh", - "timeout": 10 - }, - { - "type": "command", - "command": "${CLAUDE_PLUGIN_ROOT}/hooks/lsp-bash-check-install.sh &>/dev/null &" - }, - { - "type": "command", - "command": "${CLAUDE_PLUGIN_ROOT}/hooks/lsp-typescript-check-install.sh &>/dev/null &" - }, - { - "type": "command", - "command": "${CLAUDE_PLUGIN_ROOT}/hooks/lsp-python-check-install.sh &>/dev/null &" - }, - { - "type": "command", - "command": "${CLAUDE_PLUGIN_ROOT}/hooks/lsp-go-check-install.sh &>/dev/null &" - }, - { - "type": "command", - "command": "${CLAUDE_PLUGIN_ROOT}/hooks/lsp-kotlin-check-install.sh &>/dev/null &" - }, - { - "type": "command", - "command": "${CLAUDE_PLUGIN_ROOT}/hooks/lsp-lua-check-install.sh &>/dev/null &" - }, - { - "type": "command", - "command": "${CLAUDE_PLUGIN_ROOT}/hooks/lsp-terraform-check-install.sh &>/dev/null &" - }, - { - "type": "command", - "command": "${CLAUDE_PLUGIN_ROOT}/hooks/lsp-nix-check-install.sh &>/dev/null &" - } + // keep only non-LSP startup hooks that still exist ] } ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/me/.claude-plugin/plugin.json` around lines 4 - 20, SessionStart in plugins/me/hooks/hooks.json is still invoking removed scripts (handoff-session-start.sh and several lsp-*-check-install.sh commands), causing failures; edit the SessionStart hook to remove any calls to "handoff-session-start.sh" and all "lsp-*-check-install.sh" entries and replace them with the new, valid startup actions or leave the hook empty if no startup script is required, ensuring the JSON array only references existing scripts; search for the exact symbol "SessionStart" and filenames "handoff-session-start.sh" and patterns "lsp-*-check-install.sh" to locate and remove/update the stale entries.
🧹 Nitpick comments (1)
plugins/me/.claude-plugin/plugin.json (1)
19-20: Please update manifest validation to stop allowinglspServers.
tests/helpers/bats_helper.bash(Line 124-151) still treatslspServersas an allowed field, which weakens enforcement of this cleanup in future manifests.Proposed follow-up diff (`tests/helpers/bats_helper.bash`)
- local allowed_fields="name description author version license homepage repository keywords lspServers" + local allowed_fields="name description author version license homepage repository keywords" ... - echo "Allowed fields: name, description, author, version, license, homepage, repository, keywords, lspServers" + echo "Allowed fields: name, description, author, version, license, homepage, repository, keywords"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/me/.claude-plugin/plugin.json` around lines 19 - 20, The manifest validation in the test helper still treats the deprecated key `lspServers` as allowed; update tests/helpers/bats_helper.bash to remove `lspServers` from the allowed manifest fields list (the array or variable that enumerates permitted manifest keys, e.g., the allowed_fields/ALLOWED_MANIFEST_KEYS constant used in the validation function), and update any assertions that expect `lspServers` to be permitted so the helper will fail when manifests include that key.
🤖 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 `@plugins/me/.claude-plugin/plugin.json`:
- Around line 4-20: SessionStart in plugins/me/hooks/hooks.json is still
invoking removed scripts (handoff-session-start.sh and several
lsp-*-check-install.sh commands), causing failures; edit the SessionStart hook
to remove any calls to "handoff-session-start.sh" and all
"lsp-*-check-install.sh" entries and replace them with the new, valid startup
actions or leave the hook empty if no startup script is required, ensuring the
JSON array only references existing scripts; search for the exact symbol
"SessionStart" and filenames "handoff-session-start.sh" and patterns
"lsp-*-check-install.sh" to locate and remove/update the stale entries.
---
Nitpick comments:
In `@plugins/me/.claude-plugin/plugin.json`:
- Around line 19-20: The manifest validation in the test helper still treats the
deprecated key `lspServers` as allowed; update tests/helpers/bats_helper.bash to
remove `lspServers` from the allowed manifest fields list (the array or variable
that enumerates permitted manifest keys, e.g., the
allowed_fields/ALLOWED_MANIFEST_KEYS constant used in the validation function),
and update any assertions that expect `lspServers` to be permitted so the helper
will fail when manifests include that key.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: db0713c5-55f7-4859-86c6-13353847239c
📒 Files selected for processing (22)
.claude-plugin/marketplace.jsonplugins/lsp-clangd/.claude-plugin/plugin.jsonplugins/lsp-clangd/.lsp.jsonplugins/lsp-gopls/.claude-plugin/plugin.jsonplugins/lsp-gopls/.lsp.jsonplugins/lsp-lua/.claude-plugin/plugin.jsonplugins/lsp-lua/.lsp.jsonplugins/lsp-pyright/.claude-plugin/plugin.jsonplugins/lsp-pyright/.lsp.jsonplugins/lsp-typescript/.claude-plugin/plugin.jsonplugins/lsp-typescript/.lsp.jsonplugins/me/.claude-plugin/plugin.jsonplugins/me/hooks/handoff-session-start.shplugins/me/hooks/lsp-bash-check-install.shplugins/me/hooks/lsp-go-check-install.shplugins/me/hooks/lsp-kotlin-check-install.shplugins/me/hooks/lsp-lua-check-install.shplugins/me/hooks/lsp-nix-check-install.shplugins/me/hooks/lsp-python-check-install.shplugins/me/hooks/lsp-terraform-check-install.shplugins/me/hooks/lsp-typescript-check-install.shtests/me/me-specific.bats
💤 Files with no reviewable changes (20)
- plugins/lsp-pyright/.lsp.json
- plugins/lsp-gopls/.lsp.json
- plugins/me/hooks/lsp-typescript-check-install.sh
- plugins/lsp-clangd/.lsp.json
- plugins/me/hooks/lsp-bash-check-install.sh
- plugins/lsp-lua/.lsp.json
- plugins/me/hooks/lsp-go-check-install.sh
- plugins/lsp-pyright/.claude-plugin/plugin.json
- plugins/me/hooks/lsp-terraform-check-install.sh
- plugins/me/hooks/lsp-python-check-install.sh
- plugins/lsp-typescript/.lsp.json
- plugins/lsp-clangd/.claude-plugin/plugin.json
- plugins/lsp-typescript/.claude-plugin/plugin.json
- plugins/me/hooks/lsp-kotlin-check-install.sh
- plugins/me/hooks/lsp-nix-check-install.sh
- plugins/me/hooks/lsp-lua-check-install.sh
- plugins/lsp-gopls/.claude-plugin/plugin.json
- tests/me/me-specific.bats
- plugins/me/hooks/handoff-session-start.sh
- plugins/lsp-lua/.claude-plugin/plugin.json
Summary
Remove 5 standalone LSP plugins (clangd, gopls, lua, pyright, typescript) and associated LSP-related hooks from the
meplugin.Changes
plugins/lsp-clangd,lsp-gopls,lsp-lua,lsp-pyright,lsp-typescriptplugin directorieshandoff-session-start.shfromplugins/me/hooks/marketplace.json: removed 5 lsp plugin entries, removedlsptag and description reference frommeplugins/me/plugin.json: removedlspServerssection, lsp-related keywords and description referenceTests
Verified
marketplace.jsonandplugins/me/plugin.jsonreflect the deletions correctly.Summary by CodeRabbit
Release Notes