add OpenCode CLI provider support#67
Conversation
Adds OpenCode (https://opencode.ai) as a new CLI provider following the established provider pattern (matching PR elirantutia#27 / Codex). Closes elirantutia#59. - opencode-config.ts: reads MCP servers, agents (JSON + markdown), skills, and commands from opencode.json and .opencode/ directories - opencode-hooks.ts: installs a JS plugin at .opencode/plugins/vibeyard-status.js to emit hook events (session lifecycle, tool use, permissions) - opencode-provider.ts: full CliProvider implementation with binary resolution, env vars (OPENCODE_CLIENT=vibeyard), args (--session/--prompt/--agent), and capability flags - Registered in registry.ts; ProviderId union updated to include 'opencode' - 55 new tests (30 provider + 9 config + 16 hooks), all passing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Security Review — PR #67: Add OpenCode CLI Provider
Reviewed files: opencode-config.ts, opencode-hooks.ts, opencode-provider.ts, registry.ts, types.ts, and associated test files.
Summary
No exploitable vulnerabilities found. The PR follows the same patterns established by the existing Claude, Codex, Copilot, and Gemini providers. Two low/informational items are noted below for awareness.
Findings
1. STATUS_DIR embedded in single-quoted JS template without quote escaping — Low / Informational
Location: opencode-hooks.ts — buildPluginContent():
const statusDir = STATUS_DIR.replace(/\\/g, '/');
return `...
const STATUS_DIR = '${statusDir}';
...`;STATUS_DIR is path.join(os.tmpdir(), 'vibeyard') (e.g., /tmp/vibeyard). Only backslashes are escaped before interpolation. If os.tmpdir() ever returns a path containing a single quote (e.g., /var/folders/tmp is fine, but a hypothetical /Users/O'Brien/tmp would not be), the generated plugin file becomes syntactically invalid JavaScript.
Practical risk is very low — the temp directory path is OS-controlled and essentially never contains single quotes. The same unescaped-path pattern already exists in hook-status.ts for the Python and shell hook scripts (e.g., status_dir=r'${STATUS_DIR}', 2>>${STATUS_DIR}/statusline.log), so this is consistent with the existing codebase approach.
Recommendation (non-blocking): Consider escaping single quotes in the interpolated path (statusDir.replace(/'/g, "\\'")), or switching to a JSON.stringify-based string literal so any path character is safely encoded: `const STATUS_DIR = ${JSON.stringify(statusDir)};`. This would also make hook-status.ts's analogous scripts more robust.
2. extraArgs whitespace-split passed directly to spawn args — Informational
Location: opencode-provider.ts — buildArgs():
if (opts.extraArgs) {
args.push(...opts.extraArgs.split(/\s+/).filter(Boolean));
}Any string in extraArgs is tokenised on whitespace and appended verbatim to the CLI argument array, so a value like --session evil-id would inject additional flags. This is identical to how all other providers (claude-provider.ts, copilot-provider.ts, gemini-provider.ts) handle extraArgs, and extraArgs comes from user-configured session settings, so the "attacker" is the local user themselves. No privilege escalation is possible. Noted for completeness only.
Checks Passed
| Area | Result |
|---|---|
| Code injection into generated plugin JS | No (STATUS_DIR is OS-controlled; same pattern as existing scripts) |
| Shell/command injection in PTY spawn | No (args passed as array, not shell string) |
| Path traversal via config file reads | No meaningful risk (local app, same-user privilege) |
| Sensitive data leaked via env vars | No (OPENCODE_CLIENT=vibeyard, VIBEYARD_SESSION_ID are intentional) |
| Untrusted IPC / renderer inputs | No (projectPath flows from trusted main-process store) |
| Plugin file written to arbitrary paths | No (path is always <projectPath>/.opencode/plugins/vibeyard-status.js) |
--prompt arg injection |
No (passed as proper array element, not shell-interpolated) |
| New provider registered with correct capabilities | Yes (costTracking/contextWindow false, hookStatus/configReading/sessionResume correct) |
There was a problem hiding this comment.
PR #67 Review — OpenCode CLI provider
Overall this is a well-structured addition that follows the established provider pattern. The test coverage (55 tests) is solid and the code is readable. A few issues worth addressing before merge.
Issues
1. permission_asked is not a valid InspectorEventType
File: src/main/opencode-hooks.ts (plugin content, ~line 589)
The plugin appends:
appendEvt({ type: 'permission_asked', timestamp: t, hookEvent: 'permission.asked' });But InspectorEventType in shared/types.ts includes 'permission_request', not 'permission_asked'. Events with unrecognized types will be passed to the renderer but won't match any known branch in the inspector, silently disappearing. Either add 'permission_asked' to the union, or change the plugin to emit 'permission_request'.
2. session.status → user_prompt is a semantic mismatch
File: src/main/opencode-hooks.ts (plugin content, ~line 586–588)
} else if (event.type === 'session.status') {
writeStatus('session.status', 'working');
appendEvt({ type: 'user_prompt', timestamp: t, hookEvent: 'session.status' });
}In OpenCode, session.status fires on any internal session state change — not just when a user submits a prompt. Mapping it to the user_prompt inspector event will create spurious entries in the session inspector (e.g., every model "thinking" update appears as a user message). Consider either omitting the appendEvt call here, or using a different event type such as 'status_update' which is already in the InspectorEventType union.
3. session.error → tool_failure is a semantic mismatch
File: src/main/opencode-hooks.ts (plugin content, ~line 583–585)
} else if (event.type === 'session.error') {
writeStatus('session.error', 'working');
appendEvt({ type: 'tool_failure', timestamp: t, hookEvent: 'session.error' });
}session.error is a session-level failure, not a specific tool failure. Using tool_failure will cause confusing inspector entries that lack a tool name. 'stop_failure' (already in InspectorEventType) is a better fit — it signals that the session terminated with an error.
4. Plugin file written into the project directory will show up in git status
File: src/main/opencode-hooks.ts — pluginFilePath()
The plugin is installed at .opencode/plugins/vibeyard-status.js inside the project, not in a global user config directory like Codex (~/.codex/hooks.json) or Claude (~/.vibeyard/). This means every project using OpenCode will get a generated file in its working tree that appears in git status. Users will need to manually add it to .gitignore.
A few options:
- Add
.opencode/plugins/vibeyard-status.jsto the project's.gitignorefrominstallOpenCodeHooks()(same pattern used by other tools). - Document this in the UI (settings screen / hook status tooltip).
- Check whether OpenCode supports a global plugin directory (
~/.config/opencode/plugins/) — if so, installing there avoids the per-project pollution entirely.
Additionally, the plugin embeds the machine-specific absolute STATUS_DIR path (/tmp/vibeyard or Windows equivalent), so the file is non-portable. Committing it accidentally would break other team members' vibeyard installations.
Minor / Informational
MCP override direction differs from agents/skills
In opencode-config.ts, MCP servers use project-wins-over-user semantics (later entries in the serverMap loop win), while agents, skills, and commands use user-wins-over-project semantics (dedupeByName keeps first occurrence). The comments document this intentional difference, but it may surprise users who expect consistent behavior across all config types.
tool.execute.after writes no status
The tool.execute.after handler only calls appendEvt, not writeStatus. This means the session's displayed status stays working from the tool.execute.before call through the end of execution, which is reasonable behavior — just confirming this is intentional.
What's working well
- Clean separation into
opencode-config.ts,opencode-hooks.ts,opencode-provider.tsmatching the Codex pattern - Idempotent plugin install (byte-compare skip) avoids redundant disk writes on every spawn
_resetForTesting()/_resetCachedPath()pattern is consistent with other modulesdedupeByNameandreadSkillsFromDirre-use fromprovider-config-utils— good reuse- Binary resolution uses the shared
resolveBinary/validateBinaryExistshelpers - Platform path handling (
replace(/\\/g, '/')inbuildPluginContent) is correct
…dd-opencode-provider
There was a problem hiding this comment.
PR Review: Add OpenCode CLI provider support
Overall this is a solid, well-structured addition. The code follows the established provider pattern closely (matching CopilotProvider almost 1:1), the test coverage is good, and the three-layer split (config / hooks / provider) is clean. A few issues need attention before merge.
Bugs
1. permission_asked is not a valid InspectorEventType
File: src/main/opencode-hooks.ts (in buildPluginContent)
The generated plugin calls:
appendEvt({ type: 'permission_asked', timestamp: t, hookEvent: 'permission.asked' });But InspectorEventType in src/shared/types.ts has permission_request (not permission_asked). The renderer won't recognize this event type, so permission events will silently be ignored by the inspector timeline. Should be:
appendEvt({ type: 'permission_request', timestamp: t, hookEvent: 'permission.asked' });The tests don't catch this because they only verify that certain hook-event strings appear in the plugin content, not that the type values are valid.
2. Session ID is never captured → sessionResume: true won't work
File: src/main/providers/opencode-provider.ts
The provider declares sessionResume: true and buildArgs uses opts.cliSessionId to pass --session <id> on resume. But cliSessionId in SessionRecord is populated by hook-status.ts reading .sessionid files from STATUS_DIR. The OpenCode plugin (buildPluginContent) never writes a .sessionid file — only .status and .events files.
Without a .sessionid write, cliSessionId will always stay null, isResume will never produce the --session arg, and the resume capability flag will cause the UI to show an option that silently has no effect. Either:
- Add a
.sessionidwrite to the plugin when the session ID is available (e.g., fromsession.createdpayload if OpenCode includes one), or - Set
sessionResume: falseuntil this is wired up.
Concerns
3. session.error mapped to 'working' status
File: src/main/opencode-hooks.ts (in buildPluginContent)
} else if (event.type === 'session.error') {
writeStatus('session.error', 'working');A session.error should end the session (or at minimum signal a failure), but 'working' tells the activity tracker the session is still actively doing something. 'completed' or a stop-class event seems more appropriate. Compare with Copilot's errorOccurred → status: 'working' (which is debatable there too, but at least Copilot re-transitions via subsequent events).
4. MCP server merge semantics are opposite to agents/skills
File: src/main/opencode-config.ts
// MCP: project overrides user on name collision
for (const server of userMcp) serverMap.set(server.name, server);
for (const server of projectMcp) serverMap.set(server.name, server); // project wins
// Skills/agents: user wins on name collision
const agents = dedupeByName(
readAgentsFromJson(userConfigFile, 'user'), // user listed first → wins
readAgentsFromJson(projectConfigFile, 'project'),
);Project-level MCP servers override user-level ones, but for agents and skills the user-level wins. The asymmetry is surprising and may be intentional (matching OpenCode's own behavior for MCP vs. agents), but if so it should be documented with a comment explaining why.
Minor
5. Unnecessary mockStatSync in skills tests
File: src/main/opencode-config.test.ts
The "reads skills from .opencode/skills subdirectories" and "deduplicates skills" tests set up a mockStatSync, but readSkillsFromDir (from provider-config-utils.ts) uses readFileSafe → readFileSync, not statSync. The mock is dead code in those tests and can be removed.
6. Generated plugin file will appear in git status without guidance
The plugin is installed at .opencode/plugins/vibeyard-status.js inside the project directory. This file is auto-generated on every session spawn and shouldn't be committed. Worth adding a comment in opencode-hooks.ts noting that users should add this path to .gitignore, or having the install step write a .gitignore entry (Copilot's hooks go under .github/hooks/ which is typically already committed, but this is different).
Change permission event type from 'permission_asked' to 'permission_request' to match the InspectorEventType union in shared/types.ts. Also add .gitignore entry for the generated plugin file so it does not appear in git status on every project using OpenCode. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Set sessionResume: false until the OpenCode plugin writes a .sessionid file to STATUS_DIR. Currently cliSessionId stays null, so the UI would show a resume option that silently has no effect. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- opencode-config.ts: add comment explaining why MCP uses project-wins semantics while agents/skills/commands use user-wins semantics - opencode-config.test.ts: remove dead mockStatSync.mockImplementation calls from skills tests (readSkillsFromDir uses readFileSync, not statSync) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR Fix SummaryReviewed and addressed comments from @github-actions[bot]: Fixed Issues
Deferred Issues
Generated by gen-pr-resolver skill. |
There was a problem hiding this comment.
Good implementation — the provider follows the established Copilot/Codex pattern closely and the 55-test suite is thorough. A few things worth addressing before merge.
Bugs / Inconsistencies
1. sessionResume: false contradicts the working resume implementation
opencode-provider.ts declares capabilities.sessionResume = false, but buildArgs fully implements it:
if (opts.isResume && opts.cliSessionId) {
args.push('--session', opts.cliSessionId);
}There's even an explicit test:
it('returns ["--session", id] when isResume=true with cliSessionId', ...)
Every other provider (claude, codex, copilot, gemini) sets sessionResume: true. The false flag means the UI will never surface session-resume for OpenCode even though both the CLI and the code support it. Either flip the flag to true or remove the --session path (and its test) if resume isn't ready to ship.
2. session.error writes 'completed' as its status field
In the generated plugin (buildPluginContent):
} else if (event.type === 'session.error') {
writeStatus('session.error', 'completed');The .status file format is eventName:status, so this produces session.error:completed. Downstream readers (and humans) will see an error event reported as "completed". Compare with session.idle → 'completed' which makes sense semantically. For errors, something like 'error' or 'failed' would be more accurate.
Minor Issues
3. readCommandsFromDir uses filename as fallback name; readAgentsFromDir does not
opencode-config.ts:246:
const name = fm.name || file.slice(0, -3); // commands: file basename as fallbackopencode-config.ts:229:
if (!fm.name) continue; // agents: skip if no nameThis asymmetry means a command file without a name frontmatter key silently uses its filename, while an agent file in the same situation is silently dropped. The behaviour may be intentional (commands are simpler), but it's worth a comment or making both behave consistently.
4. User-level agent markdown directory (~/.config/opencode/agents/) — does OpenCode support this?
getOpenCodeConfig reads agents from readAgentsFromDir(path.join(userConfigDir, 'agents'), 'user'), but the OpenCode docs and the PR description only mention opencode.json (user JSON config) and .opencode/ (project-level directory). If ~/.config/opencode/agents/ isn't a real OpenCode path, this read is harmless (the dir won't exist) but misleading. Worth verifying against the OpenCode spec before merge.
Nits
- The
EXPECTED_HOOK_EVENTSlist inopencode-hooks.tscoverssession.created/session.idle/tool.execute.*, but the plugin also handlessession.error,session.status, andpermission.asked. These extra events won't cause false-partialresults, but they won't be validated either. If they matter for activity tracking, add them to the list. cleanupOpenCodeHooksleaveslastProjectPathset after removing the plugin, unlike a full teardown. This is consistent withcleanupCopilotHooks, but it means a subsequentreinstallSettings()call after cleanup will re-create the plugin file. Probably fine given the call sites, just flagging for awareness.
…lementation buildArgs had a working --session <id> path but sessionResume: false meant the UI would never surface it. Removed the --session branch and its tests to eliminate the contradiction. When OpenCode session ID writing is wired up in the plugin, both can be restored together. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tatus - Add session.error, session.status, permission.asked to EXPECTED_HOOK_EVENTS so hook validation reflects all handled events (previously only 4 of 7 were validated, causing false 'complete' results) - Add comment explaining why 'completed' is used for session.error: the hook-status.ts layer only accepts working/waiting/completed/input, so there is no separate error terminal status Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ir convention - Add comment explaining why agents without a 'name' frontmatter key are skipped (vs commands which use filename as fallback) - Add comment noting the ~/.config/opencode/agents/ dir follows OpenCode's project-level convention; harmless if absent Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR Fix SummaryReviewed and addressed new comments from @github-actions[bot] (review #4095296830): Fixed Issues
Deferred Issues
Generated by gen-pr-resolver skill. |
There was a problem hiding this comment.
This is a solid, well-structured provider implementation that follows the established patterns from Codex and Copilot. Tests are thorough and the hook installation/validation lifecycle is clean. One functional gap blocks correct operation; the rest are smaller items.
Bug: config-watcher.ts not updated for 'opencode'
This is the most significant issue. config-watcher.ts:startConfigWatcher has a provider dispatch chain:
if (providerId === 'codex') {
setupCodexWatchers(projectPath);
} else if (providerId === 'gemini') {
setupGeminiWatchers(projectPath);
} else if (providerId === 'copilot') {
setupCopilotWatchers(projectPath);
} else {
setupClaudeWatchers(projectPath); // ← opencode falls here
}opencode is now a valid ProviderId but has no case, so it silently falls through to setupClaudeWatchers. Consequences:
- Config-change notifications won't fire when the user edits
opencode.json,.opencode/agents/,.opencode/skills/, or.opencode/commands/— the config panel will appear stale until a manual reload. - Config-change events will fire spuriously for OpenCode sessions when Claude-related files (
.claude/settings.json,.mcp.json, etc.) are modified.
A setupOpenCodeWatchers function needs to be added and wired in. Based on the files opencode-config.ts reads, the watcher would cover:
~/.config/opencode/opencode.json
~/.config/opencode/agents/ (dir)
~/.config/opencode/skills/ (dir)
~/.config/opencode/commands/ (dir)
<project>/opencode.json
<project>/.opencode/agents/ (dir)
<project>/.opencode/skills/ (dir)
<project>/.opencode/commands/(dir)
Plugin module format
buildPluginContent() emits ESM syntax (import * as fs from 'fs', export const VibeyardStatus), which is written to .opencode/plugins/vibeyard-status.js. Whether OpenCode's plugin loader handles .js as ESM (vs. requiring .mjs) depends on OpenCode internals that aren't documented in this diff. If OpenCode uses require() to load plugins, the import statement will throw at runtime. This is worth a note in the PR or a quick verification against the OpenCode plugin spec before merging.
Minor: two separate fs-utils import lines in opencode-config.ts
import { readDirSafe } from './fs-utils'; // line 3
import { readJsonSafe } from './fs-utils'; // line 5These can be merged into one import. Not a functional issue.
Nits
-
planModeArg: '--agent plan'— the split-on-whitespace path inbuildArgswill correctly yield['--agent', 'plan']. Fine mechanically, but worth a manual smoke-test to confirmopencode --agent planis the correct invocation for plan mode. -
OPENCODE_CLIENT=vibeyard— good for telemetry/identification. If OpenCode changes the env var name or semantics, this will silently stop working; a link to the relevant OpenCode docs in a comment would help future maintainers. -
The
session.error→'completed'status mapping is well-commented (hook-status.tshas no error state, so completed is the right approximation). No change needed; the comment is sufficient.
What's working well
- The provider cleanly follows the Codex/Copilot pattern with no novel abstractions needed.
- Hook installation is idempotent (byte-comparison skip) and self-cleaning (gitignore entry,
cleanupOpenCodeHooks). dedupeByNamecorrectly implements user-wins semantics for agents/skills/commands and the intentional project-wins exception for MCP servers is clearly commented.- 55 tests cover the lifecycle thoroughly including edge cases (no-op without path, partial validation,
.gitignoreidempotency). _resetForTesting()/_resetCachedPath()pattern is consistent with the rest of the codebase.
The config-watcher gap is the only thing that needs to land before this is fully functional end-to-end.
OpenCode sessions were falling through to setupClaudeWatchers, causing
config-change notifications to fire for wrong files and miss opencode.json
and .opencode/ dir changes entirely.
Add setupOpenCodeWatchers watching:
- ~/.config/opencode/opencode.json
- <project>/opencode.json
- ~/.config/opencode/{agents,skills,commands}/
- <project>/.opencode/{agents,skills,commands}/
Wire into the provider dispatch chain with a dedicated 'opencode' case.
Add tests covering both initial setup and provider-switching.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- opencode-config.ts: merge two separate import lines from ./fs-utils into one - opencode-provider.ts: add comment explaining OPENCODE_CLIENT=vibeyard purpose Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
PR Fix SummaryReviewed and addressed new comments from @github-actions[bot] (review #4095334323): Fixed Issues
Acknowledged / No Code Change
Generated by gen-pr-resolver skill. |
There was a problem hiding this comment.
Review: Add OpenCode CLI Provider Support
Overall this is a clean, well-structured addition that follows the established provider pattern closely. The 55-test suite is thorough and the separation of concerns (config, hooks, provider) mirrors the Codex/Copilot implementations well. A few things worth addressing:
Issues
ESM imports in the generated JS plugin
opencode-hooks.ts (buildPluginContent) generates a .js file containing:
import * as fs from 'fs';
import * as path from 'path';
// ...
export const VibeyardStatus = async () => ({These are ES module static imports and a named ESM export. Node.js treats .js files as CommonJS by default unless a package.json with "type": "module" is present in the plugin directory — which won't exist for a freshly created .opencode/plugins/ folder. If OpenCode loads plugins via Node's require() (or any CJS-based runtime), this will throw a SyntaxError at load time and hooks will silently never fire.
If OpenCode uses Bun or a loader that explicitly handles ESM, this is fine — but that assumption should be confirmed. Otherwise the plugin should use require() / module.exports instead, consistent with how Claude's hook scripts are generated (they use CommonJS).
readMcpFromJson silently drops entries that have args but no command or url
const url = (config?.url as string) || (config?.command as string) || '';
if (url) {
servers.push(...)
}OpenCode's stdio MCP type can look like { type: 'stdio', command: 'npx', args: ['-y', '@my/server'] }. In this case command is 'npx' and would be captured, but a config with only args and no explicit command key would be silently skipped. This probably reflects real-world OpenCode configs, but a comment noting the omission (or a test for the args-only case returning empty) would prevent future confusion.
Stray double blank line in config-watcher.ts
for (const d of dirs) watchDir(d);
}
function setupGeminiWatchersTwo blank lines between setupOpenCodeWatchers and setupGeminiWatchers — every other function boundary in the file uses one.
Minor Observations
getOpenCodeConfig is declared async but contains no await
All the underlying helpers (readMcpFromJson, readAgentsFromDir, etc.) are synchronous. The async satisfies the Promise<ProviderConfig> interface contract, but the implementation body reads as sync code. A short comment like // returns a Promise to satisfy the CliProvider interface; all reads are sync would make the intent clear to the next reader.
No explicit test that isResume: true is a no-op in buildArgs
Since sessionResume: false, the isResume flag is intentionally ignored in buildArgs, and the comment in the implementation explains why. Adding a one-line test (expect(provider.buildArgs({ ..., isResume: true })).toEqual([])) would make this contract explicit and guard against a future provider that accidentally enables session resume.
What looks good
- Hook lifecycle is complete:
install/validate/cleanup/reinstallall delegate cleanly, thelastProjectPathpattern matches the existing Claude hooks pattern, and_resetForTesting()is properly exported. .gitignorehygiene: The machine-specific plugin file is automatically excluded from version control; the logic handles missing/existing.gitignorecorrectly.- MCP project-wins semantics: The intentional reversal of deduplication priority for MCP (project over user) compared to agents/skills (user over project) is clearly documented and tested.
OPENCODE_CLIENT=vibeyardenv var: Good integration touch for telemetry.planModeArg: '--agent plan': Correctly set and tested; will split properly through the existingextraArgswhitespace-split path inbuildArgs.- Config watcher coverage: Both
watchFile(opencode.json) andwatchDir(agents/skills/commands) are registered for both user and project scopes, consistent with Gemini/Copilot.
Adds OpenCode (https://opencode.ai) as a new CLI provider following the established provider pattern (matching PR #27 / Codex). Closes #59.
Summary
Brief description of what this PR does.
Test plan
npm run buildpassesnpm testpassesNotes
Any additional context or trade-offs worth mentioning.