Skip to content

add OpenCode CLI provider support#67

Open
lauferism wants to merge 10 commits into
elirantutia:mainfrom
lauferism:add-opencode-provider
Open

add OpenCode CLI provider support#67
lauferism wants to merge 10 commits into
elirantutia:mainfrom
lauferism:add-opencode-provider

Conversation

@lauferism

Copy link
Copy Markdown
Collaborator

Adds OpenCode (https://opencode.ai) as a new CLI provider following the established provider pattern (matching PR #27 / Codex). Closes #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

Summary

Brief description of what this PR does.

Test plan

  • npm run build passes
  • npm test passes
  • Manually tested in the app (if UI changes)

Notes

Any additional context or trade-offs worth mentioning.

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>

@github-actions github-actions Bot left a comment

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.

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.tsbuildPluginContent():

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.tsbuildArgs():

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)

@github-actions github-actions Bot left a comment

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.

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.statususer_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.errortool_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.tspluginFilePath()

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.js to the project's .gitignore from installOpenCodeHooks() (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.ts matching the Codex pattern
  • Idempotent plugin install (byte-compare skip) avoids redundant disk writes on every spawn
  • _resetForTesting() / _resetCachedPath() pattern is consistent with other modules
  • dedupeByName and readSkillsFromDir re-use from provider-config-utils — good reuse
  • Binary resolution uses the shared resolveBinary / validateBinaryExists helpers
  • Platform path handling (replace(/\\/g, '/') in buildPluginContent) is correct

@github-actions github-actions Bot left a comment

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.

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 .sessionid write to the plugin when the session ID is available (e.g., from session.created payload if OpenCode includes one), or
  • Set sessionResume: false until 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 errorOccurredstatus: '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 readFileSafereadFileSync, 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).

lauferism and others added 3 commits April 12, 2026 19:46
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>
@lauferism

Copy link
Copy Markdown
Collaborator Author

PR Fix Summary

Reviewed and addressed comments from @github-actions[bot]:

Fixed Issues

  • permission_asked is not a valid InspectorEventType (HIGH) - Changed to permission_request to match the InspectorEventType union in shared/types.ts
  • Session ID never captured, sessionResume: true won't work (HIGH) - Set sessionResume: false until .sessionid writing is wired up in the plugin
  • session.error maps to wrong status and wrong event type (HIGH) - Changed status to 'completed' and event type to 'stop_failure'
  • session.status mapped to user_prompt is a semantic mismatch (MEDIUM) - Changed to 'status_update' event type
  • Plugin file written into project directory will show up in git status (MEDIUM) - installOpenCodeHooks now auto-adds .opencode/plugins/vibeyard-status.js to the project .gitignore
  • MCP override direction differs from agents/skills (MEDIUM) - Added comment in opencode-config.ts explaining the intentional asymmetry
  • Unnecessary mockStatSync in skills tests (LOW) - Removed dead mockStatSync.mockImplementation calls from skills tests

Deferred Issues

  • STATUS_DIR path not single-quote-escaped in plugin JS (LOW) - Practical risk is very low (OS temp dirs never contain single quotes); deferred as the same pattern exists throughout the codebase

Generated by gen-pr-resolver skill.

@github-actions github-actions Bot left a comment

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.

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 fallback

opencode-config.ts:229:

if (!fm.name) continue;                        // agents: skip if no name

This 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_EVENTS list in opencode-hooks.ts covers session.created / session.idle / tool.execute.*, but the plugin also handles session.error, session.status, and permission.asked. These extra events won't cause false-partial results, but they won't be validated either. If they matter for activity tracking, add them to the list.
  • cleanupOpenCodeHooks leaves lastProjectPath set after removing the plugin, unlike a full teardown. This is consistent with cleanupCopilotHooks, but it means a subsequent reinstallSettings() call after cleanup will re-create the plugin file. Probably fine given the call sites, just flagging for awareness.

lauferism and others added 3 commits April 12, 2026 20:00
…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>
@lauferism

Copy link
Copy Markdown
Collaborator Author

PR Fix Summary

Reviewed and addressed new comments from @github-actions[bot] (review #4095296830):

Fixed Issues

  • sessionResume: false contradicts the working resume implementation (HIGH) - Removed the --session code path and related tests from buildArgs. No contradiction: sessionResume stays false and there is no dead implementation. Can be restored together when OpenCode session ID writing is wired up in the plugin.
  • session.error writes 'completed' as its status field (MEDIUM) - Added a clarifying comment: hook-status.ts only accepts working/waiting/completed/input; there is no separate error terminal status. 'completed' correctly signals the session has terminated.
  • readCommandsFromDir uses filename as fallback; readAgentsFromDir does not (LOW) - Added a comment explaining the intentional asymmetry: agents without a name frontmatter key cannot be selected by the user, unlike commands where the filename is a reasonable display fallback.
  • User-level agent markdown dir ~/.config/opencode/agents/ — does OpenCode support this? (LOW) - Added a comment noting this follows OpenCode's project-level convention applied globally; the read is harmless when the dir is absent.
  • EXPECTED_HOOK_EVENTS doesn't cover session.error, session.status, permission.asked (LOW) - Expanded the list from 4 to 7 events so hook validation correctly reflects all handled events.

Deferred Issues

  • cleanupOpenCodeHooks leaves lastProjectPath set after cleanup (LOW) - Intentional; consistent with cleanupCopilotHooks behavior. Flagged for awareness only.

Generated by gen-pr-resolver skill.

@github-actions github-actions Bot left a comment

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.

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 5

These can be merged into one import. Not a functional issue.


Nits

  • planModeArg: '--agent plan' — the split-on-whitespace path in buildArgs will correctly yield ['--agent', 'plan']. Fine mechanically, but worth a manual smoke-test to confirm opencode --agent plan is 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.ts has 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).
  • dedupeByName correctly 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, .gitignore idempotency).
  • _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.

lauferism and others added 2 commits April 12, 2026 20:12
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>
@lauferism

Copy link
Copy Markdown
Collaborator Author

PR Fix Summary

Reviewed and addressed new comments from @github-actions[bot] (review #4095334323):

Fixed Issues

  • config-watcher.ts not updated for 'opencode' (BUG — HIGH) - Added setupOpenCodeWatchers() watching opencode.json and .opencode/{agents,skills,commands}/ at both user (~/.config/opencode/) and project levels. Wired into the provider dispatch chain with a dedicated 'opencode' case. Added 2 new tests.
  • Two separate fs-utils import lines in opencode-config.ts (MINOR) - Merged into a single import.
  • OPENCODE_CLIENT env var (NIT) - Added a comment explaining it identifies the client to OpenCode for telemetry/routing.

Acknowledged / No Code Change

  • Plugin ESM format (.js vs .mjs) - Noted for verification against the OpenCode plugin spec before merging. No code change until OpenCode's plugin loader behavior is confirmed.
  • --agent plan invocation - Needs a manual smoke test; no code change.

Generated by gen-pr-resolver skill.

@github-actions github-actions Bot left a comment

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.

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 setupGeminiWatchers

Two 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 / reinstall all delegate cleanly, the lastProjectPath pattern matches the existing Claude hooks pattern, and _resetForTesting() is properly exported.
  • .gitignore hygiene: The machine-specific plugin file is automatically excluded from version control; the logic handles missing/existing .gitignore correctly.
  • 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=vibeyard env var: Good integration touch for telemetry.
  • planModeArg: '--agent plan': Correctly set and tested; will split properly through the existing extraArgs whitespace-split path in buildArgs.
  • Config watcher coverage: Both watchFile (opencode.json) and watchDir (agents/skills/commands) are registered for both user and project scopes, consistent with Gemini/Copilot.

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.

Opencode support request

1 participant