-
Notifications
You must be signed in to change notification settings - Fork 993
fix: auto-detect and prevent conflicts with external DCP/ELF plugins #558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
- Add plugin compatibility detection system - Auto-disable conflicting hooks when opencode-elf/opencode-dcp detected - Display clear warning messages about auto-disabled hooks - Add Plugin Compatibility section to README - Resolves #555 The issue was that oh-my-opencode's built-in DCP and compaction features were interfering with external opencode-elf and opencode-dcp plugins. Now the plugin automatically detects these conflicts and disables its own hooks to allow external plugins to work properly. Users can still manually override via disabled_hooks config if needed.
Greptile SummaryThis PR implements automatic plugin conflict detection to resolve issues where oh-my-opencode's built-in DCP and context management features interfere with external Major Changes:
Issues Found:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant OpenCode
participant OhMyOpenCode as oh-my-opencode Plugin
participant PluginCompat as plugin-compatibility.ts
participant ConfigLoader as loadPluginConfig
participant Hooks as Hook System
User->>OpenCode: Start OpenCode
OpenCode->>OhMyOpenCode: Initialize Plugin (ctx)
Note over OhMyOpenCode: startTmuxCheck()
OhMyOpenCode->>ConfigLoader: loadPluginConfig(directory)
ConfigLoader-->>OhMyOpenCode: pluginConfig (with user disabled_hooks)
OhMyOpenCode->>PluginCompat: detectConflictingPlugins(directory)
PluginCompat->>PluginCompat: loadOpencodeConfig(directory)
Note over PluginCompat: Check project/user opencode.json<br/>for plugin list
PluginCompat->>PluginCompat: Check for DCP plugins<br/>(opencode-dcp, @opencode/dcp)
PluginCompat->>PluginCompat: Check for ELF plugins<br/>(opencode-elf, @opencode/elf)
PluginCompat->>PluginCompat: Check for Compaction plugins
alt Conflicts Found
PluginCompat->>PluginCompat: Build hooksToDisable array<br/>(preemptive-compaction, etc)
PluginCompat-->>OhMyOpenCode: {hasConflicts: true, conflicts, hooksToDisable}
OhMyOpenCode->>OhMyOpenCode: Combine autoDisabledHooks + userDisabledHooks
Note over OhMyOpenCode: Issue: User cannot override<br/>auto-disabled hooks
OhMyOpenCode->>User: warnAboutConflicts() - Display warning
Note over User: ⚠️ Plugin Conflict Detection<br/>Auto-disabling N hooks
else No Conflicts
PluginCompat-->>OhMyOpenCode: {hasConflicts: false, conflicts: [], hooksToDisable: []}
end
OhMyOpenCode->>Hooks: Initialize hooks with isHookEnabled() checks
Note over Hooks: Disabled hooks are not created/registered
OhMyOpenCode-->>OpenCode: Plugin initialized
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (1)
-
src/shared/plugin-compatibility.ts, line 1-161 (link)style: Missing tests for new plugin-compatibility feature. Per
AGENTS.md:42-64, TDD is MANDATORY for new features. Should include test cases for:- Exact plugin name match
- Versioned plugin match (e.g.,
[email protected]) - False positives from overly-broad matching
- Multiple conflicting plugins
- No conflicts scenario
- Config file not found
- Invalid config file
Context Used: Context from
dashboard- AGENTS.md (source)
4 files reviewed, 3 comments
| for (const pluginName of pluginNames) { | ||
| const hasPlugin = installedPlugins.some( | ||
| (p) => | ||
| p === pluginName || p.startsWith(`${pluginName}@`) || p.includes(pluginName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: The p.includes(pluginName) check is too broad - will match unrelated plugins. For example, if pluginName is "dcp", this will match plugins like "my-dcp-custom", "cdcp-handler", or "opencode-backup-dcp". Should use more precise matching.
| p === pluginName || p.startsWith(`${pluginName}@`) || p.includes(pluginName) | |
| p === pluginName || p.startsWith(`${pluginName}@`) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/shared/plugin-compatibility.ts
Line: 110:110
Comment:
**logic:** The `p.includes(pluginName)` check is too broad - will match unrelated plugins. For example, if `pluginName` is `"dcp"`, this will match plugins like `"my-dcp-custom"`, `"cdcp-handler"`, or `"opencode-backup-dcp"`. Should use more precise matching.
```suggestion
p === pluginName || p.startsWith(`${pluginName}@`)
```
How can I resolve this? If you propose a fix, please make it concise.| const conflictDetection = detectConflictingPlugins(ctx.directory); | ||
| const autoDisabledHooks = new Set(conflictDetection.hooksToDisable); | ||
| const userDisabledHooks = new Set(pluginConfig.disabled_hooks ?? []); | ||
| const disabledHooks = new Set([...autoDisabledHooks, ...userDisabledHooks]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: User cannot re-enable hooks that were auto-disabled. The current logic combines both sets, but users might want to override auto-detection (e.g., they know their DCP plugin is compatible). Consider implementing priority: if user explicitly lists a hook in disabled_hooks, respect auto-detection; but provide a separate enabled_hooks_override config to force-enable specific hooks despite conflicts.
Current behavior:
- Auto-disabled:
["preemptive-compaction"] - User config:
disabled_hooks: [] - Result:
preemptive-compactionis still disabled (user cannot override)
The README states "Override auto-detection by explicitly configuring disabled_hooks" but this doesn't actually allow re-enabling auto-disabled hooks.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/index.ts
Line: 78:78
Comment:
**logic:** User cannot re-enable hooks that were auto-disabled. The current logic combines both sets, but users might want to override auto-detection (e.g., they know their DCP plugin is compatible). Consider implementing priority: if user explicitly lists a hook in `disabled_hooks`, respect auto-detection; but provide a separate `enabled_hooks_override` config to force-enable specific hooks despite conflicts.
Current behavior:
- Auto-disabled: `["preemptive-compaction"]`
- User config: `disabled_hooks: []`
- Result: `preemptive-compaction` is still disabled (user cannot override)
The README states "Override auto-detection by explicitly configuring `disabled_hooks`" but this doesn't actually allow re-enabling auto-disabled hooks.
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 4 files
Confidence score: 3/5
- Auto-disabling hooks in
src/shared/plugin-compatibility.tsuses.includes(pluginName), so unrelated package names containing the substring will falsely match and deactivate hooks users expect to run. src/index.tsalways applies previously auto-disabled hooks even whendisabled_hooksis empty, meaning developers can’t re-enable them through config and remain stuck without those hooks.- Pay close attention to
src/shared/plugin-compatibility.ts,src/index.ts- both contain logic that can unnecessarily disable hooks and prevent re-enabling them.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/shared/plugin-compatibility.ts">
<violation number="1" location="src/shared/plugin-compatibility.ts:110">
P1: The `.includes(pluginName)` check is overly broad and will cause false positives. Plugins like `"my-opencode-dcp-wrapper"` or `"not-opencode-elf-plugin"` would incorrectly match, auto-disabling hooks unnecessarily. Consider removing `.includes()` or replacing with a more specific check like `p.endsWith('/' + pluginName)` for scoped packages with different orgs.</violation>
</file>
<file name="src/index.ts">
<violation number="1" location="src/index.ts:78">
P2: Logic error: Users cannot re-enable hooks that were auto-disabled. The current implementation merges both sets (auto-disabled + user-disabled), so `disabled_hooks: []` in user config won't override auto-detection. This contradicts the README which states users can 'Override auto-detection by explicitly configuring disabled_hooks'. Consider adding an `enabled_hooks_override` config option or changing the logic so that explicitly omitting a hook from `disabled_hooks` re-enables it.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| for (const pluginName of pluginNames) { | ||
| const hasPlugin = installedPlugins.some( | ||
| (p) => | ||
| p === pluginName || p.startsWith(`${pluginName}@`) || p.includes(pluginName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: The .includes(pluginName) check is overly broad and will cause false positives. Plugins like "my-opencode-dcp-wrapper" or "not-opencode-elf-plugin" would incorrectly match, auto-disabling hooks unnecessarily. Consider removing .includes() or replacing with a more specific check like p.endsWith('/' + pluginName) for scoped packages with different orgs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/shared/plugin-compatibility.ts, line 110:
<comment>The `.includes(pluginName)` check is overly broad and will cause false positives. Plugins like `"my-opencode-dcp-wrapper"` or `"not-opencode-elf-plugin"` would incorrectly match, auto-disabling hooks unnecessarily. Consider removing `.includes()` or replacing with a more specific check like `p.endsWith('/' + pluginName)` for scoped packages with different orgs.</comment>
<file context>
@@ -0,0 +1,160 @@
+ for (const pluginName of pluginNames) {
+ const hasPlugin = installedPlugins.some(
+ (p) =>
+ p === pluginName || p.startsWith(`${pluginName}@`) || p.includes(pluginName)
+ );
+
</file context>
| const conflictDetection = detectConflictingPlugins(ctx.directory); | ||
| const autoDisabledHooks = new Set(conflictDetection.hooksToDisable); | ||
| const userDisabledHooks = new Set(pluginConfig.disabled_hooks ?? []); | ||
| const disabledHooks = new Set([...autoDisabledHooks, ...userDisabledHooks]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: Logic error: Users cannot re-enable hooks that were auto-disabled. The current implementation merges both sets (auto-disabled + user-disabled), so disabled_hooks: [] in user config won't override auto-detection. This contradicts the README which states users can 'Override auto-detection by explicitly configuring disabled_hooks'. Consider adding an enabled_hooks_override config option or changing the logic so that explicitly omitting a hook from disabled_hooks re-enables it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/index.ts, line 78:
<comment>Logic error: Users cannot re-enable hooks that were auto-disabled. The current implementation merges both sets (auto-disabled + user-disabled), so `disabled_hooks: []` in user config won't override auto-detection. This contradicts the README which states users can 'Override auto-detection by explicitly configuring disabled_hooks'. Consider adding an `enabled_hooks_override` config option or changing the logic so that explicitly omitting a hook from `disabled_hooks` re-enables it.</comment>
<file context>
@@ -62,17 +62,25 @@ import {
+ const conflictDetection = detectConflictingPlugins(ctx.directory);
+ const autoDisabledHooks = new Set(conflictDetection.hooksToDisable);
+ const userDisabledHooks = new Set(pluginConfig.disabled_hooks ?? []);
+ const disabledHooks = new Set([...autoDisabledHooks, ...userDisabledHooks]);
+
+ if (conflictDetection.hasConflicts) {
</file context>
Summary
Fixes #555 - oh-my-opencode now automatically detects and prevents conflicts with external opencode-elf and opencode-dcp plugins.
Problem
Users reported that after installing oh-my-opencode, their existing opencode-elf and opencode-dcp plugins stopped working. The root cause was:
preemptive-compactionhook (enabled by default)anthropic-context-window-limit-recoveryhook (enabled by default)compaction-context-injectorhook (enabled by default)experimental.dcp_for_compactionBoth oh-my-opencode and external plugins were trying to manage the same session state, causing interference.
Solution
Implemented automatic plugin conflict detection:
Auto-Detection: On startup, scans
opencode.jsonfor conflicting plugins:opencode-dcp,@opencode/dcpopencode-elf,@opencode/elfopencode-compaction,@opencode/compactionAuto-Disable: Automatically disables oh-my-opencode hooks that would conflict:
anthropic-context-window-limit-recovery,preemptive-compaction,compaction-context-injectorcontext-window-monitor,directory-agents-injector,directory-readme-injectorpreemptive-compaction,anthropic-context-window-limit-recovery,compaction-context-injectorUser Notification: Displays clear warning messages showing which hooks were auto-disabled
Manual Override: Users can still manually configure via
disabled_hooksconfig if neededChanges
src/shared/plugin-compatibility.ts- Plugin conflict detection systemsrc/index.ts- Integrate detection at plugin initializationREADME.md- Add Plugin Compatibility section with documentationTesting
Next Steps
Users with opencode-elf/opencode-dcp installed will automatically see warning messages and have conflicting hooks disabled. No manual configuration required, but users can override if needed via
disabled_hooksconfig.Summary by cubic
Automatically detect external DCP/ELF/compaction plugins and auto-disable conflicting oh-my-opencode hooks to prevent clashes. Fixes #555 so opencode-elf/opencode-dcp keep working without manual setup.
Written for commit 3609bb0. Summary will update on new commits.