fix(release): address user-blocking setup issues#1012
Conversation
📝 WalkthroughWalkthroughThree unrelated fixes: Pi adapter deduplicates legacy subagent packages during settings merge; Claude Code Context7 MCP injection and uninstall now target ChangesPi Legacy Subagent Deduplication
Claude Code Context7 MCP Path Migration
OpenCode gentle-logo Plugin Theme Removal
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Installer as Install/Sync CLI
participant Inject as mcp.Inject
participant Settings as ~/.claude/settings.json
participant Legacy as ~/.claude/mcp/context7.json
Installer->>Inject: Inject(ComponentContext7, AgentClaudeCode)
Inject->>Settings: injectMergeIntoSettings (write mcpServers.context7)
Note over Inject,Legacy: Legacy file untouched by Inject
Installer->>Inject: Uninstall(ComponentContext7, AgentClaudeCode)
Inject->>Settings: remove mcpServers.context7 entry
Inject->>Legacy: read legacy file
alt matches managed npx/@upstash/context7-mcp shape
Inject->>Legacy: removeManagedContext7File deletes it
else custom content
Inject->>Legacy: leave file unchanged
end
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Pull request overview
This PR fixes several user-blocking setup and runtime issues across supported agents by improving migration/uninstall behavior, aligning Claude Code MCP injection with the paths Claude actually loads, and hardening an OpenCode TUI plugin against theme-swap crashes.
Changes:
- Pi: remove legacy subagent package entries during settings merge to avoid duplicate tool registration conflicts.
- Claude Code: inject Context7 via
~/.claude/settings.json(settings-based MCP) and make uninstall remove only managed legacy Context7 files while preserving user/custom files. - OpenCode: remove theme-state subscription from the Gentle Logo TUI plugin to mitigate theme-change TextBuffer crashes; add regression tests across affected areas.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/agents/pi/adapter.go | Filters known legacy Pi subagent package identities while appending the current MCP adapter package. |
| internal/agents/pi/adapter_test.go | Adds regression test ensuring legacy Pi subagent specs are removed during settings merge. |
| internal/components/mcp/inject.go | Routes Claude Code Context7 injection through settings merge instead of writing a standalone ~/.claude/mcp/context7.json. |
| internal/components/mcp/inject_test.go | Updates/extends tests to validate Claude Context7 is merged into settings and legacy files aren’t rewritten. |
| internal/cli/run.go | Adjusts Context7 “verification/backup paths” reporting for Claude to use settings.json. |
| internal/cli/run_component_paths_test.go | Adds regression test ensuring Claude Context7 uses settings.json and does not verify legacy file paths. |
| internal/components/uninstall/service.go | Adds Claude-specific Context7 uninstall behavior (remove settings entry + delete only managed legacy file). |
| internal/components/uninstall/service_test.go | Adds uninstall regression tests for Claude managed legacy file removal vs preserving custom legacy files. |
| internal/components/opencodeplugin/plugin.go | Removes theme dependency from the Gentle Logo plugin and hardcodes a non-theme color. |
| internal/components/opencodeplugin/plugin_test.go | Adds regression assertions ensuring the plugin does not reference theme state. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| apply: func(path string) (bool, bool, error) { | ||
| content, err := os.ReadFile(path) | ||
| if err != nil { | ||
| if os.IsNotExist(err) { | ||
| return false, false, nil | ||
| } | ||
| return false, false, err | ||
| } |
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)
internal/agents/pi/adapter.go (1)
267-291: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider consolidating the repeated identity-match pattern.
The
piMCPAdapterPackagecheck (Lines 276-278) and the legacy-loop check (Lines 279-284) both implement the same "exact match orX@prefix" pattern. Extracting a small helper (e.g.matchesPackage(source, candidate string) bool) would avoid duplicating this boundary logic and reduce risk of divergence if one copy is tweaked later.♻️ Proposed refactor
+func matchesPackage(source, candidate string) bool { + return source == candidate || strings.HasPrefix(source, candidate+"@") +} + func piPackageIdentity(pkg any) string { source, ok := pkg.(string) if !ok { object, isObject := pkg.(map[string]any) if !isObject { return "" } source, _ = object["source"].(string) } - if strings.HasPrefix(source, piMCPAdapterPackage+"@") || source == piMCPAdapterPackage { + if matchesPackage(source, piMCPAdapterPackage) { return piMCPAdapterPackage } for legacy := range legacyPiSubagentPackageIdentities { - if source == legacy || strings.HasPrefix(source, legacy+"@") { + if matchesPackage(source, legacy) { return legacy } } return source }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/agents/pi/adapter.go` around lines 267 - 291, The package identity matching in piPackageIdentity repeats the same exact-match-or-versioned-prefix logic for piMCPAdapterPackage and legacy entries, so consolidate that boundary check into a small helper and use it in both places. Introduce a shared matcher near piPackageIdentity/isLegacyPiSubagentPackage (for example, a helper that tests source against a candidate package name) and replace the duplicated string comparisons in piPackageIdentity with calls to it to keep the behavior consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@internal/agents/pi/adapter.go`:
- Around line 267-291: The package identity matching in piPackageIdentity
repeats the same exact-match-or-versioned-prefix logic for piMCPAdapterPackage
and legacy entries, so consolidate that boundary check into a small helper and
use it in both places. Introduce a shared matcher near
piPackageIdentity/isLegacyPiSubagentPackage (for example, a helper that tests
source against a candidate package name) and replace the duplicated string
comparisons in piPackageIdentity with calls to it to keep the behavior
consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3f7d9416-7bd9-4f20-a684-98fab18f0b41
📒 Files selected for processing (10)
internal/agents/pi/adapter.gointernal/agents/pi/adapter_test.gointernal/cli/run.gointernal/cli/run_component_paths_test.gointernal/components/mcp/inject.gointernal/components/mcp/inject_test.gointernal/components/opencodeplugin/plugin.gointernal/components/opencodeplugin/plugin_test.gointernal/components/uninstall/service.gointernal/components/uninstall/service_test.go
Closes #971
Closes #899
Closes #1011
PR Type
Summary
~/.claude/settings.jsonand makes uninstall clean managed legacy files without deleting custom files.Changes
internal/agents/pi/adapter.gointernal/components/mcp/inject.gointernal/cli/run.gosettings.json.internal/components/uninstall/service.gointernal/components/opencodeplugin/plugin.go*_test.goTest Plan
go test ./internal/components/uninstall ./internal/cli ./internal/components/mcp ./internal/agents/pi ./internal/components/opencodepluginContributor Checklist
type:*labelCo-Authored-BytrailersSummary by CodeRabbit
Bug Fixes
Style