Skip to content

fix: keep opencode hook out of repo #1870

Open
janburzinski wants to merge 2 commits intogeneralaction:mainfrom
janburzinski:emdash/opencode-plugin-ib0rp
Open

fix: keep opencode hook out of repo #1870
janburzinski wants to merge 2 commits intogeneralaction:mainfrom
janburzinski:emdash/opencode-plugin-ib0rp

Conversation

@janburzinski
Copy link
Copy Markdown
Collaborator

summary

instead of having the opencode plugin inside of the repo, this makes it a custom emdash global config so you dont have to add it to your gitignore and the config just gets loaded to your already existing opencode config

also updates the provider docs to describe the new OpenCode hook setup

@janburzinski janburzinski changed the title Keep OpenCode hooks out of repos fix: keep opencode hook out of repo May 4, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 4, 2026

Greptile Summary

This PR moves the OpenCode notifications plugin out of the repository worktree and into an Emdash-owned directory (app.getPath('userData')/opencode), then points OpenCode at it via OPENCODE_CONFIG_DIR. The approach is clean: a secondary FileSystemProvider is injected into HookConfigWriter, .gitignore management for OpenCode is removed, and the env-var injection uses the existing customVars path in buildAgentEnv.

Confidence Score: 4/5

Safe to merge; only a P2 style issue found in a test assertion.

All findings are P2 (style). Logic, env-var injection, and test coverage are correct. One vestigial assertion in the test checks a now-obsolete path but does not mask any real bug.

hook-config.test.ts — line 104 has a stale path assertion that should be updated.

Important Files Changed

Filename Overview
src/main/core/agent-hooks/hook-config.ts Moves OpenCode plugin write target to an injected openCodeConfigFs, falls back to project fs when not provided; removes gitignore entry logic for OpenCode.
src/main/core/agent-hooks/hook-config.test.ts Tests updated to use dual-fs setup; one vestigial assertion checks the old .opencode/ path in the project fs (vacuously true, not meaningful).
src/main/core/conversations/impl/local-conversation.ts Wires openCodeConfigFs pointing at app.getPath('userData')/opencode; injects OPENCODE_CONFIG_DIR env var for opencode sessions so OpenCode loads the emdash-owned plugin directory.
agents/integrations/providers.md Documentation updated to reflect the new service filename and the OpenCode config-dir strategy.

Sequence Diagram

sequenceDiagram
    participant LCP as LocalConversationProvider
    participant HCW as HookConfigWriter
    participant ProjFS as LocalFileSystem (taskPath)
    participant OCConfigFS as LocalFileSystem (userData/opencode)
    participant OpenCode

    LCP->>HCW: new HookConfigWriter(projFs, ctx, { openCodeConfigFs })
    LCP->>LCP: startSession(conversation)
    LCP->>HCW: writeForProvider('opencode')
    HCW->>HCW: resolveCommandPath('opencode')
    HCW->>OCConfigFS: write('plugins/emdash-notifications.js')
    Note over ProjFS: No writes to project dir
    LCP->>OpenCode: spawn with OPENCODE_CONFIG_DIR=userData/opencode
    OpenCode->>OCConfigFS: loads plugins/emdash-notifications.js
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
src/main/core/agent-hooks/hook-config.test.ts:104
**Stale path assertion**

This assertion checks for `.opencode/plugins/emdash-notifications.js` in the project filesystem, which is the old pre-PR path that was never going to be written there after the refactor. It is vacuously true and doesn't verify any meaningful invariant. The corresponding new check (`openCodeConfigFs.has('plugins/emdash-notifications.js')`) already follows on line 105. Consider replacing this with `expect(fs.files.has('plugins/emdash-notifications.js')).toBe(false)` to verify the new relative path is also not written into the project fs.

Reviews (1): Last reviewed commit: "Keep OpenCode hooks out of repos" | Re-trigger Greptile


await writer.writeForProvider('opencode');

expect(fs.files.has('.opencode/plugins/emdash-notifications.js')).toBe(false);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Stale path assertion

This assertion checks for .opencode/plugins/emdash-notifications.js in the project filesystem, which is the old pre-PR path that was never going to be written there after the refactor. It is vacuously true and doesn't verify any meaningful invariant. The corresponding new check (openCodeConfigFs.has('plugins/emdash-notifications.js')) already follows on line 105. Consider replacing this with expect(fs.files.has('plugins/emdash-notifications.js')).toBe(false) to verify the new relative path is also not written into the project fs.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/main/core/agent-hooks/hook-config.test.ts
Line: 104

Comment:
**Stale path assertion**

This assertion checks for `.opencode/plugins/emdash-notifications.js` in the project filesystem, which is the old pre-PR path that was never going to be written there after the refactor. It is vacuously true and doesn't verify any meaningful invariant. The corresponding new check (`openCodeConfigFs.has('plugins/emdash-notifications.js')`) already follows on line 105. Consider replacing this with `expect(fs.files.has('plugins/emdash-notifications.js')).toBe(false)` to verify the new relative path is also not written into the project fs.

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant