feat: ship Claude Code plugin manifest to remove manual hook setup#18
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces Claude Code plugin marketplace support for tmux-agent-sidebar, replacing manual Changes
Sequence DiagramsequenceDiagram
participant App as App Startup
participant Registry as Claude Registry
participant Settings as Settings File
participant State as AppState
participant UI as Notice UI
App->>Registry: Read installed_plugins.json
Registry-->>App: Plugin version (if installed)
App->>Settings: Read ~/.claude/settings.json
Settings-->>App: Residual hooks present? (bool)
App->>State: Store plugin_version & residual_hooks
App->>State: Compute ClaudePluginNotice
State-->>App: Notice state (Recommend/Duplicate/Stale/None)
App->>UI: Render notices popup
UI-->>App: Display migration prompt or version alert
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
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 unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 149-151: Add language identifiers to the unlabeled fenced code
blocks that trigger markdownlint MD040: label the block containing "/plugin
install ~/.tmux/plugins/tmux-agent-sidebar" with sh (or bash) and label the
longer descriptive block starting "Run
~/.tmux/plugins/tmux-agent-sidebar/target/release/tmux-agent-sidebar setup
claude..." with text (or plain) so both fences are explicitly typed; update the
two occurrences referenced (the short install command and the longer instruction
block) to include these language identifiers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7860feeb-22c3-4f95-a3a8-3eed43bbfb3b
📒 Files selected for processing (7)
.claude-plugin/plugin.json.claude/skills/version-release/SKILL.mdCLAUDE.mdREADME.mdhook.shhooks/hooks.jsontests/plugin_hooks_tests.rs
451fd0c to
b3dd8f7
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
README.md (1)
149-151:⚠️ Potential issue | 🟡 MinorAdd language identifiers to fenced code blocks (MD040).
These three fences are unlabeled and trigger markdownlint warnings.
🛠️ Proposed fix
-``` +```sh /plugin install ~/.tmux/plugins/tmux-agent-sidebar-
+text
Run ~/.tmux/plugins/tmux-agent-sidebar/target/release/tmux-agent-sidebar setup claude (fall back to ~/.tmux/plugins/tmux-agent-sidebar/bin/tmux-agent-sidebar if that path is missing). Add these hooks to ~/.claude/settings.json. If hooks already exist, merge them without making destructive changes.-``` +```text Run ~/.tmux/plugins/tmux-agent-sidebar/target/release/tmux-agent-sidebar setup codex (fall back to ~/.tmux/plugins/tmux-agent-sidebar/bin/tmux-agent-sidebar if that path is missing). Add these hooks to ~/.codex/hooks.json. If hooks already exist, merge them without making destructive changes.</details> Also applies to: 162-164, 176-178 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@README.mdaround lines 149 - 151, Several fenced code blocks in README.md
(the/plugin install ~/.tmux/plugins/tmux-agent-sidebarline and the two "Run
~/.tmux/plugins/tmux-agent-sidebar/target/release/tmux-agent-sidebar setup ..."
blocks) lack language identifiers and trigger MD040; update each fence to
include an appropriate language tag (e.g.,sh for the plugin install command andtext or ```sh for the two setup/run paragraphs) so markdownlint no longer
flags them and the content renders with correct syntax highlighting.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In@README.md:
- Around line 149-151: Several fenced code blocks in README.md (the
/plugin install ~/.tmux/plugins/tmux-agent-sidebarline and the two "Run
~/.tmux/plugins/tmux-agent-sidebar/target/release/tmux-agent-sidebar setup ..."
blocks) lack language identifiers and trigger MD040; update each fence to
include an appropriate language tag (e.g.,sh for the plugin install command andtext or ```sh for the two setup/run paragraphs) so markdownlint no longer
flags them and the content renders with correct syntax highlighting.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `1df8d12a-88c6-41d1-84f6-f7cc8c123335` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 451fd0cdfb441ffa107a7c2bed8e55034a45f93e and 8fbdf0f07ce01ced53f995d8478519776c872a17. </details> <details> <summary>📒 Files selected for processing (7)</summary> * `.claude-plugin/plugin.json` * `.claude/skills/version-release/SKILL.md` * `CLAUDE.md` * `README.md` * `hook.sh` * `hooks/hooks.json` * `tests/plugin_hooks_tests.rs` </details> <details> <summary>✅ Files skipped from review due to trivial changes (4)</summary> * .claude-plugin/plugin.json * .claude/skills/version-release/SKILL.md * CLAUDE.md * hooks/hooks.json </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * tests/plugin_hooks_tests.rs </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli/plugin_state.rs`:
- Around line 96-110: The test helper with_env (and any callers like
record_no_op_when_env_missing) must not mutate process environment unsafely;
refactor to stop reading/writing CLAUDE_PLUGIN_ROOT and instead pass
plugin_root: &Path into the functions that need it (e.g., change
record_from_root to accept &Path and update tests to call
record_from_root(&plugin_root, &state_file) directly), remove uses of
std::env::set_var/remove_var and delete with_env entirely, and update all test
call sites to supply the plugin_root parameter so no process-global env mutation
is performed.
In `@src/state.rs`:
- Around line 499-518: The code unconditionally suppresses Claude missing-hook
warnings because compute_missing_hook_groups is fed notices_agents(&agent) which
always drops "claude"; change the call site to only remove/suppress "claude"
when we actually know a plugin is installed: compute current_installed_version()
into a variable (you already have installed), derive a boolean like let
claude_plugin_installed = installed.is_some(), then either (A) call a revised
notices_agents(&agent, claude_plugin_installed) or (B) call
notices_agents(&agent) and filter its result to remove "claude" only when
claude_plugin_installed is true before passing to compute_missing_hook_groups;
keep compute_claude_plugin_notice/claude_plugin_notice logic unchanged and
preserve debug_forced_display handling.
In `@src/ui/notices.rs`:
- Around line 35-52: The prompt_for_agent function currently calls
std::env::current_exe() up front which prevents the "claude" branch from working
when current_exe() fails and forces an unused exe_path into
build_claude_migration_prompt; instead, move current_exe() into the
agent-specific branches: compute exe_path only in the "codex" arm (format its
message there), and in the "claude" arm attempt to get
current_exe().ok().and_then(|e|
plugin_root_from_exe(&e).as_deref().map(String::from)) so that missing exe does
not disable the Claude prompt; update build_claude_migration_prompt to accept
only the optional plugin_root (remove the unused _exe_path parameter) and pass
that Option<&str> from prompt_for_agent, leaving other arms unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0ed13e5d-89a6-4bbf-b414-e16467f7f0b3
📒 Files selected for processing (5)
src/cli/hook.rssrc/cli/mod.rssrc/cli/plugin_state.rssrc/state.rssrc/ui/notices.rs
\`/plugin install <local-path>\` is not a supported syntax — Claude Code's slash command requires a marketplace-registered plugin name in the form \`<plugin>@<marketplace>\`. The README now matches what the sidebar's migration prompt actually generates and what was verified end-to-end in #18: /plugin marketplace add hiroppy/tmux-agent-sidebar /plugin install tmux-agent-sidebar@hiroppy Also notes the local-checkout variant for development and points users at the [prompt] copy button for automated settings.json migration. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The early README rewrite in this branch swapped Codex's "click [copy] in the sidebar Notices popup" walkthrough for an inferior "paste this LLM prompt" form. That walkthrough was the original recommended path and it still works (Codex CLI has no plugin mechanism, so the [copy] button keeps surfacing the legacy `setup codex` prompt). Restore it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Four issues raised by CodeRabbit, all addressed: 1. README.md (MD040): the new \`/plugin marketplace add\` install blocks were missing language tags. Added \`text\` so markdownlint stops complaining and the fence stays neutral. 2. src/cli/plugin_state.rs: the test helper \`with_env\` mutated \`CLAUDE_PLUGIN_ROOT\` via the now-unsafe \`set_var\`/\`remove_var\` and raced under cargo test's parallel execution despite a misleading SAFETY comment claiming serialization. Refactored \`record_from_env\` to read the env var once and delegate to a new pure \`record_from_root(plugin_root, state_path)\` helper. Tests call \`record_from_root\` directly, so no process-global env mutation happens at all. 3. src/state.rs: a previous refactor made \`compute_missing_hook_groups\` filter Claude unconditionally, which regressed the manual \`~/.claude/settings.json\` path — those users lost their concrete missing-hook diagnostics. Restored the \`claude_plugin_present\` parameter so Claude is only suppressed when the plugin install is actually known. Tests updated to cover both gates. 4. src/ui/notices.rs: \`prompt_for_agent\` resolved \`current_exe()\` up front and returned None on failure for *both* agents, even though the Claude branch only needs the resolved plugin root (which has its own canonical fallback). Moved \`current_exe()\` retrieval into the agent-specific arms so a Claude prompt still works when the exe path cannot be resolved. Dropped the unused \`_exe_path\` parameter from \`build_claude_migration_prompt\`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2a9cc8a to
4a34a61
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
README.md (1)
172-174:⚠️ Potential issue | 🟡 MinorAdd a language tag to this fenced block.
This still trips markdownlint
MD040.textwould fit here.📝 Suggested fix
-``` +```text Run ~/.tmux/plugins/tmux-agent-sidebar/target/release/tmux-agent-sidebar setup claude (fall back to ~/.tmux/plugins/tmux-agent-sidebar/bin/tmux-agent-sidebar if that path is missing). Add these hooks to ~/.claude/settings.json. If hooks already exist, merge them without making destructive changes.</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@README.mdaround lines 172 - 174, The fenced code block in README.md lacks a
language tag and triggers markdownlint MD040; update the triple-backtick fence
that surrounds the sentence starting "Run
~/.tmux/plugins/tmux-agent-sidebar/target/release/tmux-agent-sidebar setup
claude..." to include a language tag (e.g.,text) so the block becomestext ... ```, preserving the existing content exactly and not altering
formatting or wording.</details> </blockquote></details> <details> <summary>src/cli/plugin_state.rs (1)</summary><blockquote> `75-79`: _⚠️ Potential issue_ | _🟡 Minor_ **Treat missing `HOME` as “no Claude settings file.”** When `HOME` is unset, `unwrap_or_default()` makes this resolve to `./.claude/settings.json`. That means a process started from an arbitrary working directory can pick up an unrelated local file and raise a false duplicate-hooks notice. <details> <summary>🛠️ Suggested fix</summary> ```diff -fn claude_settings_path() -> PathBuf { - std::env::var_os("HOME") - .map(PathBuf::from) - .unwrap_or_default() - .join(".claude/settings.json") +fn claude_settings_path() -> Option<PathBuf> { + let home = std::env::var_os("HOME")?; + Some(PathBuf::from(home).join(".claude/settings.json")) } pub fn claude_settings_has_residual_hooks() -> bool { - claude_settings_has_residual_hooks_at(&claude_settings_path()) + let Some(path) = claude_settings_path() else { + return false; + }; + claude_settings_has_residual_hooks_at(&path) } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@src/cli/plugin_state.rs` around lines 75 - 79, The function claude_settings_path currently falls back to joining ".claude/settings.json" onto an empty PathBuf when HOME is unset, producing a relative "./.claude/settings.json"; change claude_settings_path to first check std::env::var_os("HOME") and only call .join(".claude/settings.json") when HOME is present, otherwise return an empty PathBuf (or another sentinel PathBuf that the caller treats as "no settings file") so a missing HOME will not resolve to a local relative file. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@src/cli/plugin_state.rs:
- Around line 53-57: The current logic only inspects
installs.as_array().and_then(|a| a.first()) which ignores subsequent install
records; change the lookup to iterate the installs array (e.g.,
installs.as_array().iter()) and for each element check
install.get("version").and_then(|v| v.as_str()) and !version.is_empty(),
returning the first valid version string; ensure you still handle installs being
non-array or missing gracefully and return None if no install record contains a
non-empty "version".In
@src/main.rs:
- Around line 108-117: The cached Claude plugin state assignment
(state.claude_plugin_installed_version and
state.claude_settings_has_residual_hooks) happens after the initial
state.refresh(), so the first rendered frame can show stale notices; fix by
either moving these assignments to run before the existing state.refresh() call
or by invoking state.refresh() again immediately after setting those two fields
(ensure you call the same state.refresh() method so notices are recomputed with
the updated values).In
@src/state.rs:
- Around line 536-544: The current claude_plugin_present boolean still requires
no residual legacy entries (self.claude_settings_has_residual_hooks) which
causes missing_hooks() to re-include Claude when any legacy entry remains;
change the guard so claude_plugin_present is true whenever the plugin is
installed by setting it to self.claude_plugin_installed_version.is_some()
(remove the residual-hooks check) so missing_hooks() no longer reports Claude as
missing; keep the DuplicateHooks diagnostic logic intact (so DuplicateHooks
still alerts when legacy entries exist).
Duplicate comments:
In@README.md:
- Around line 172-174: The fenced code block in README.md lacks a language tag
and triggers markdownlint MD040; update the triple-backtick fence that surrounds
the sentence starting "Run
~/.tmux/plugins/tmux-agent-sidebar/target/release/tmux-agent-sidebar setup
claude..." to include a language tag (e.g.,text) so the block becomestext ... ```, preserving the existing content exactly and not altering
formatting or wording.In
@src/cli/plugin_state.rs:
- Around line 75-79: The function claude_settings_path currently falls back to
joining ".claude/settings.json" onto an empty PathBuf when HOME is unset,
producing a relative "./.claude/settings.json"; change claude_settings_path to
first check std::env::var_os("HOME") and only call
.join(".claude/settings.json") when HOME is present, otherwise return an empty
PathBuf (or another sentinel PathBuf that the caller treats as "no settings
file") so a missing HOME will not resolve to a local relative file.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `a0c5e88a-5029-4c68-9847-432f3d032ca3` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 7cbdc053060dcfc291732f76788fdc8163c73c44 and f0ac8a555ff8c8f9927a4413cb94c5899c780f2a. </details> <details> <summary>📒 Files selected for processing (15)</summary> * `.claude-plugin/marketplace.json` * `.claude-plugin/plugin.json` * `.claude/skills/version-release/SKILL.md` * `CLAUDE.md` * `README.md` * `docs/state-management.md` * `hook.sh` * `hooks/hooks.json` * `src/cli/mod.rs` * `src/cli/plugin_state.rs` * `src/cli/setup.rs` * `src/main.rs` * `src/state.rs` * `src/ui/notices.rs` * `tests/plugin_hooks_tests.rs` </details> <details> <summary>✅ Files skipped from review due to trivial changes (5)</summary> * .claude-plugin/plugin.json * CLAUDE.md * .claude-plugin/marketplace.json * .claude/skills/version-release/SKILL.md * hooks/hooks.json </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * hook.sh </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
24b9b87 to
46fdcc0
Compare
Summary
.claude-plugin/plugin.json+hooks/hooks.jsonso Claude Code users can install all 16 hooks with a single/plugin install ~/.tmux/plugins/tmux-agent-sidebarinstead of pasting ~200 lines of JSON into~/.claude/settings.json.plugin.jsonversionin lockstep withCargo.tomlvia a drift-detection test, so Claude Code's/plugin updatemechanism actually fires for marketplace installs.hook.shwith two extra fallbacks (~/.tmux/plugins/tmux-agent-sidebar/{bin,target/release}/) so it can locate the binary even when invoked from${CLAUDE_PLUGIN_ROOT}(the plugin cache never contains the binary itself).settings.jsonflow stays as a collapsible fallback for locked-down environments.hook.shand the Rusthooksubcommand still serve both agents.Why
Manual hook setup was the single biggest install pain point — 16 JSON entries in
settings.jsonper user. Claude Code's plugin system supports bundled hooks via${CLAUDE_PLUGIN_ROOT}, so we can ship them in-repo and let/plugin installregister them in one step. Forgetting to bumpplugin.json's version would silently break update detection for marketplace users, so a unit test now enforces version parity withCargo.toml, and theversion-releaseskill grew a new step to bump both together.Test plan
cargo test— all 700+ tests pass, including 4 new tests intests/plugin_hooks_tests.rs:hooks/hooks.jsonmatchesClaudeAdapter::HOOK_REGISTRATIONS${CLAUDE_PLUGIN_ROOT}/hook.shplugin.jsondeclaresname,hooks, and aversionmatchingCargo.tomlcargo fmt --check,cargo clippy,cargo build --releaseall clean/plugin install ~/.tmux/plugins/tmux-agent-sidebarfrom a Claude Code session, then verify/tmp/tmux-agent-sidebar-*files get written forsession-start/user-prompt-submit/activity-log/stopsettings.jsonhooks present, confirm hooks fire twice (validates the README migration warning)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
cargo fmtbefore commits.Tests