Skip to content

Fix OpenCode skill command refresh - #80

Merged
akemmanuel merged 1 commit into
masterfrom
fix/opencode-skill-commands
Jun 13, 2026
Merged

akemmanuel merged 1 commit into
masterfrom
fix/opencode-skill-commands

Conversation

@touch2be

Copy link
Copy Markdown
Collaborator

Summary

  • include OpenCode skills as slash commands when command.list omits them
  • restart local OpenCode and clear cached connections after skill install/update/remove
  • refresh workspace resources after plugin actions so slash commands update immediately

Testing

  • vp check

@github-actions

Copy link
Copy Markdown

Here is my review of the PR.


Confidence Score: 4/5


Issues

1. State inconsistency if server restart fails — refreshLocalOpenCodeAfterSkillsChange (opencode-bridge.ts:1077-1085)

All connections are torn down and project registries cleared before stopping/starting the server. If stopLocalOpenCodeServer() or startLocalOpenCodeServer() fails, the system is left with no active connections and a possibly dead server — but the original IPC handler still returns success: true (the caller already checked result.success before invoking this function).

async function refreshLocalOpenCodeAfterSkillsChange() {
  for (const state of windowStates.values()) {
    for (const conn of state.projectRegistry.values()) conn.teardown();
    state.projectRegistry.clear();
  }
  const stopped = await stopLocalOpenCodeServer();
  if (!stopped.success) return stopped;       // ← connections already destroyed
  return await startLocalOpenCodeServer();
}

Consider moving destructive work (teardown/clear) to only after confirming the restart succeeds, or adding recovery logic.


2. Unhandled throw in teardown loop (opencode-bridge.ts:1079)

If any single conn.teardown() throws (e.g. abortController.abort() on a disposed controller), the entire loop breaks, leaving remaining connections uncleaned and the registry only partially cleared.

for (const conn of state.projectRegistry.values()) conn.teardown();

Wrap each call in a try-catch or use Promise.allSettled if teardown were async.


3. Misleading variable name (opencode-bridge.ts:1082)

const stopped = await stopLocalOpenCodeServer();
if (!stopped.success) return stopped;

stopped is a result object { success, error? }, not a boolean. Readable but slightly confusing — consider naming it stopResult.


4. Silent catch swallowing errors across all frontend changes

In DiscoverPlugins.tsx:462 and PluginsSettings.tsx:166,178,196, all catch blocks are empty:

try {
  await action();
  await fetchInstalled();
  await refreshProviders();
} catch {}

If refreshProviders() fails (e.g. backend still starting up), the user gets no feedback. Consider at minimum a console.warn so failures are observable in devtools.


Summary

The PR correctly solves the described problem — including OpenCode skills as slash commands and refreshing state after skill operations. The backend restart and frontend provider refresh are well-coordinated. No correctness bugs found. The issues above are robustness/observability concerns rather than logic errors.

New%20session%20-%202026-06-11T12%3A51%3A50.181Z
opencode session  |  github run

@akemmanuel
akemmanuel merged commit 5a24ac3 into master Jun 13, 2026
1 check passed
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.

2 participants