Skip to content

Add OpenCode AI client connect support and fix missing favicon#435

Merged
Dumbris merged 4 commits into
smart-mcp-proxy:mainfrom
JasonLandbridge:opencode-support
Apr 28, 2026
Merged

Add OpenCode AI client connect support and fix missing favicon#435
Dumbris merged 4 commits into
smart-mcp-proxy:mainfrom
JasonLandbridge:opencode-support

Conversation

@JasonLandbridge
Copy link
Copy Markdown
Contributor

@JasonLandbridge JasonLandbridge commented Apr 28, 2026

Description
This PR adds OpenCode as a first-class connect/disconnect client across backend, CLI, and web UI, and tightens related client UX.

What changed

image image image

Backend

  • Added opencode to the connect client registry
  • Mapped OpenCode config to:
    • config path: ~/.config/opencode/opencode.json on macOS/Linux
    • config section: mcp
  • Enforced existing-file policy for OpenCode config
  • Restricted OpenCode edits to the mcp subtree only
  • Added endpoint-equivalent adoption logic for OpenCode entries
  • Added alias-aware disconnect support via server_name
  • Added lenient parsing for OpenCode-style trailing-comma JSON configs

API

  • Added/covered connect handler behavior for:
    • adopted OpenCode entries returning idempotent success
    • missing OpenCode config returning 400
    • true conflicts still returning 409

CLI

  • Included OpenCode in connect command help/registry coverage
  • Added CLI test coverage for registry inclusion

Frontend

  • Added server_name?: string to connect status typing
  • Updated disconnect API calls to send optional alias server_name
  • Updated Connect modal to:
    • support alias-aware disconnect
    • refresh status after connect/disconnect
    • render OpenCode with a lightning icon
  • Added unit coverage for:
    • OpenCode row rendering
    • icon rendering
    • alias-aware disconnect

Favicon / browser tab icon

  • Replaced default Vite favicon usage with project favicon setup
  • Added frontend/public/favicon.svg based on assets/logo.svg
  • Updated frontend/index.html to use the normal Vite public-asset favicon path

Docs

  • Added OpenCode setup/connect documentation

Verification

Ran and tested locally

@Dumbris
Copy link
Copy Markdown
Member

Dumbris commented Apr 28, 2026

Thanks for this — really clean PR. The test coverage is the most thorough we've seen for a Connect client (HTTP handler + service + CLI + frontend), and the round-trip preservation logic in PreservesNonMCPRootKeys is exactly right.

Reviewed against four maintainer concerns and all four pass:

  • Backup before modify — every write path (connect.go:202, 285, 354, 438) routes through the existing backupFile() + atomicWriteFile() primitives. No new bypass.
  • Popularity ordering — sensible; tier-1 stays at top, OpenCode appended at the end of allClients reads naturally.
  • Per-client tests — service-level + HTTP handler + CLI registry + frontend unit. Most-tested client in the repo.
  • Scroll — at 8 clients the modal still fits; we'll add max-h-[60vh] overflow-y-auto on ConnectModal.vue:23 separately as we grow the catalog. Not a blocker for this PR.

Blocker — Windows CI: TestConnect_OpenCode_RequiresExistingConfigFile and TestHandleConnectClient_OpenCodeMissingConfigReturns400 fail on windows-amd64 (and the macOS jobs cancel from the same matrix). Root cause: ConfigPath("opencode", homeDir) reads %LOCALAPPDATA% from the real environment on Windows, ignoring the t.TempDir() injection.

Quickest fix is two t.Setenv calls — happy to push a fixup commit on top of your branch if that's easier:

 func TestConnect_OpenCode_RequiresExistingConfigFile(t *testing.T) {
-    svc, _ := testService(t)
+    svc, home := testService(t)
+    t.Setenv("LOCALAPPDATA", filepath.Join(home, "AppData", "Local"))
     _, err := svc.Connect("opencode", "", false)

Same t.Setenv("LOCALAPPDATA", filepath.Join(home, "AppData", "Local")) in internal/httpapi/connect_test.go:64 and TestConnect_OpenCode_PreservesNonMCPRootKeys. Two-test fix, no production code change.

Once Windows is green this is merge-ready — would like to land it ahead of our spec-046 stack (#433, #434) since it's a clean adapter addition. Let me know if you'd prefer to push the fix yourself or want me to do it.

The Windows-only `Build Binaries (windows-latest, …)` job failed because
ConfigPath("opencode", homeDir) reads %LOCALAPPDATA% from the real
environment on Windows and only falls back to homeDir/AppData/Local when
the env is unset. On the CI runner LOCALAPPDATA points at the runner's
real profile, so the test that asserts "config does not exist" hit a
path outside the t.TempDir() and broke the isolation contract.

Fix is two-fold:

- internal/connect/connect_test.go: t.Setenv LOCALAPPDATA in both
  testService and testServiceWithKey helpers so every test inherits the
  pinned path automatically (and any future opencode test will too).
- internal/httpapi/connect_test.go: same t.Setenv pattern in the two
  TestHandleConnectClient_OpenCode* tests that don't go through the
  service helper.

No production code change. Verified locally on macOS:
  go test ./internal/connect/... ./internal/httpapi/... — both PASS.
@Dumbris Dumbris merged commit e041087 into smart-mcp-proxy:main Apr 28, 2026
24 checks passed
@Dumbris
Copy link
Copy Markdown
Member

Dumbris commented Apr 28, 2026

Merged — thanks again Jason. Squash-merged into main; landed ahead of the spec-046 stack as planned.

@JasonLandbridge
Copy link
Copy Markdown
Contributor Author

@Dumbris Awesome, and thank you for the compliments, the fix and the awesome app!

I had some more ideas of things that might be cool to add, but I will make some issues out of them first to check with you

@Dumbris
Copy link
Copy Markdown
Member

Dumbris commented Apr 28, 2026

I had some more ideas of things that might be cool to add, but I will make some issues out of them first to check with you
@JasonLandbridge sounds great! I have send a request to you on LinkedIn. My profile https://www.linkedin.com/in/algis-dumbris/
Let me know if you'd like to have a call sometime.

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.

3 participants