-
Notifications
You must be signed in to change notification settings - Fork 19
feat: support registering remote servers with auth #13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a persistent manual-server registry and verification, exposes POST/DELETE endpoints to register/unregister manual servers, extends discovery to include manual entries, and updates proxy and SSE routes to resolve manual-server overrides and inject authorization headers when proxying. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant ProxyRoute as Proxy Route
participant Registry as Manual Server Registry
participant Auth as Auth Logic
participant RemoteServer as Remote Manual Server
participant LocalServer as Local Server
Client->>ProxyRoute: Request /api/opencode/:port/...
activate ProxyRoute
ProxyRoute->>Registry: getManualServerByProxyPort(port)
activate Registry
Registry-->>ProxyRoute: ManualServer | undefined
deactivate Registry
alt manual server found
ProxyRoute->>Auth: createAuthorizationHeader(manualServer)
activate Auth
Auth-->>ProxyRoute: Authorization header?
deactivate Auth
ProxyRoute->>RemoteServer: fetch(manualServer.url + path, headers including Authorization)
activate RemoteServer
RemoteServer-->>ProxyRoute: Response
deactivate RemoteServer
else no manual server
ProxyRoute->>LocalServer: fetch(localhost:port + path)
activate LocalServer
LocalServer-->>ProxyRoute: Response
deactivate LocalServer
end
ProxyRoute-->>Client: proxied response
deactivate ProxyRoute
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (8)**/*.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.test.ts📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx,html,css}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{tsx,jsx,ts,js}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Files:
**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Files:
**/*.test.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (2)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/app/api/opencode/servers/route.ts (1)
73-79: Potential issue: AbortController reused after abort.The
controllerfrom line 53 may already be aborted (by the timeout at line 54) when used again for the session fetch at lines 76-78. After an AbortController is aborted, its signal remains in the aborted state and will immediately abort any new fetch using it.🐛 Proposed fix: Create a new AbortController for session fetch
// Fetch session list let sessions: string[] | undefined try { - const sessionTimeout = setTimeout(() => controller.abort(), 300) + const sessionController = new AbortController() + const sessionTimeout = setTimeout(() => sessionController.abort(), 300) const sessionRes = await fetch(`http://127.0.0.1:${candidate.port}/session`, { - signal: controller.signal, + signal: sessionController.signal, }) clearTimeout(sessionTimeout)
🧹 Nitpick comments (2)
apps/web/src/app/api/sse/[port]/route.ts (1)
7-17: Consider extracting shared port validation logic.The port validation logic (numeric check + range validation) is duplicated between this file and
apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts. Consider extracting to a shared utility to maintain consistency and reduce duplication.♻️ Example shared utility
Create a shared utility in
@/lib/port-validation.ts:export function validatePort( port: string ): { valid: true; port: number } | { valid: false; error: string } { if (!port || !/^\d+$/.test(port)) { return { valid: false, error: "Invalid port number" } } const portNum = parseInt(port, 10) if (portNum < 1024 || portNum > 65535) { return { valid: false, error: "Port out of valid range (1024-65535)" } } return { valid: true, port: portNum } }apps/web/src/app/api/opencode/servers/route.ts (1)
253-262: Error categorization via string matching is fragile.The error handling relies on checking if the error message includes specific substrings. This couples the route handler to the exact error message text in
addServer, which could break silently if messages change.♻️ Consider using custom error types
// In manual-server-registry.ts export class ServerAlreadyRegisteredError extends Error { constructor(url: string) { super(`Server already registered: ${url}`) this.name = "ServerAlreadyRegisteredError" } } export class InvalidUrlError extends Error { constructor(url: string) { super(`Invalid URL: ${url}`) this.name = "InvalidUrlError" } } // In route.ts } catch (error) { if (error instanceof ServerAlreadyRegisteredError || error instanceof InvalidUrlError) { return NextResponse.json({ error: error.message }, { status: 400 }) } // ... }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/app/api/opencode/[port]/[[...path]]/route.tsapps/web/src/app/api/opencode/servers/route.tsapps/web/src/app/api/sse/[port]/route.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Usebun <file>instead ofnode <file>orts-node <file>
Run TypeScript files withbun --hot <file.ts>for hot module reloading during development
Files:
apps/web/src/app/api/sse/[port]/route.tsapps/web/src/app/api/opencode/servers/route.tsapps/web/src/app/api/opencode/[port]/[[...path]]/route.ts
**/*.{ts,tsx,js,jsx,html,css}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
bun build <file.html|file.ts|file.css>instead ofwebpackoresbuildfor bundling
Files:
apps/web/src/app/api/sse/[port]/route.tsapps/web/src/app/api/opencode/servers/route.tsapps/web/src/app/api/opencode/[port]/[[...path]]/route.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Usebunx <package> <command>instead ofnpx <package> <command>
Bun automatically loads .env, so don't use dotenv
UseBun.serve()with WebSockets, HTTPS, and routes support instead ofexpress
Usebun:sqlitefor SQLite instead ofbetter-sqlite3
UseBun.redisfor Redis instead ofioredis
UseBun.sqlfor Postgres instead ofpgorpostgres.js
Use built-inWebSocketinstead ofwspackage
PreferBun.fileovernode:fsfor readFile/writeFile operations
UseBun.$shell execution (e.g.,Bun.$ls`) instead of execa
Import .css files directly in component files; Bun's CSS bundler will bundle automatically
Files:
apps/web/src/app/api/sse/[port]/route.tsapps/web/src/app/api/opencode/servers/route.tsapps/web/src/app/api/opencode/[port]/[[...path]]/route.ts
**/*.{tsx,jsx,ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
HTML files can import .tsx, .jsx, or .js files directly; Bun's bundler will transpile and bundle automatically
Files:
apps/web/src/app/api/sse/[port]/route.tsapps/web/src/app/api/opencode/servers/route.tsapps/web/src/app/api/opencode/[port]/[[...path]]/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
Use
interfacefor defining object shapes in TypeScript
**/*.{ts,tsx}: In React hooks and effects, useuseOpencodeStore.getState()for actions (stable reference) instead of using the hook return value in dependency arrays, which creates new references on every render causing infinite loops.
Use TypeScript 5+ with strict null checks. Use oxlint for linting and Biome for formatting. Configure tsconfig.json with strict mode enabled.
Files:
apps/web/src/app/api/sse/[port]/route.tsapps/web/src/app/api/opencode/servers/route.tsapps/web/src/app/api/opencode/[port]/[[...path]]/route.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/use-bun-instead-of-node-vite-npm-pnpm.mdc)
**/*.{js,jsx,ts,tsx}: Use camelCase for variable and function names
Use PascalCase for class and component names
Always use async/await instead of .then() chains for handling promises
Add JSDoc comments to all public functions and exported classes
Use const by default, let for loop variables, avoid var
Prefer arrow functions over function expressions
Use meaningful error messages in exceptions and validations
Implement proper null/undefined checks before using values
Files:
apps/web/src/app/api/sse/[port]/route.tsapps/web/src/app/api/opencode/servers/route.tsapps/web/src/app/api/opencode/[port]/[[...path]]/route.ts
🧠 Learnings (1)
📚 Learning: 2025-12-31T02:32:55.121Z
Learnt from: CR
Repo: joelhooks/opencode-vibe PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-31T02:32:55.121Z
Learning: Applies to apps/web/app/**/*.{ts,tsx} : For Next.js app files, use Server Components by default. Preserve AsyncLocalStorage DI pattern with Context.create() and Context.provide() for per-directory instance scoping.
Applied to files:
apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts
🧬 Code graph analysis (3)
apps/web/src/app/api/sse/[port]/route.ts (1)
apps/web/src/lib/manual-server-registry.ts (2)
getManualServerByProxyPort(111-114)createAuthorizationHeader(105-109)
apps/web/src/app/api/opencode/servers/route.ts (1)
apps/web/src/lib/manual-server-registry.ts (5)
ManualServer(12-19)verifyManualServer(188-249)readRegistry(116-133)addServer(142-166)removeServer(168-181)
apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts (1)
apps/web/src/lib/manual-server-registry.ts (2)
getManualServerByProxyPort(111-114)createAuthorizationHeader(105-109)
🔇 Additional comments (6)
apps/web/src/app/api/opencode/[port]/[[...path]]/route.ts (2)
87-94: LGTM! ThebuildTargetUrlfunction correctly handles both manual server URLs and local ports.The logic properly differentiates between full URLs (starting with "http") and numeric ports, constructing appropriate target URLs for both local and remote server scenarios.
109-136: LGTM! Manual server resolution and authorization injection are implemented correctly.The code properly:
- Resolves the manual server by proxy port
- Builds the target URL using either the manual server URL or localhost
- Conditionally injects the Authorization header only when credentials exist
This ensures credentials are forwarded to upstream servers without being exposed in responses.
apps/web/src/app/api/sse/[port]/route.ts (1)
19-34: LGTM! Manual server resolution and authorization for SSE are correctly implemented.The implementation mirrors the API proxy route, ensuring consistent handling of remote servers with proper authorization header injection.
apps/web/src/app/api/opencode/servers/route.ts (3)
105-128: LGTM! Clean transformation of manual servers to DiscoveredServer format.The parallel verification and filtering of unreachable servers is well-implemented. Setting
pid: 0for remote servers is a reasonable convention.
195-226: LGTM! Well-structured local server discovery with proper error handling.The function correctly handles
lsoffailures by checking for partial stdout, and uses appropriate timeouts and concurrency limits.
265-288: LGTM! DELETE handler is well-implemented with appropriate status codes.The handler correctly uses query parameters, returns proper HTTP status codes (204/404/400), and has comprehensive error handling.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Usecase
I want to use opencode-vibe with our team's internal opencode / sandbox orchestration system, which serves many opencode servers on different cloudflare sandboxes
this gives us a REST api endpoint in opencode-vibe to add/remove opencode servers frmo our orchestrator so users see all the sessions across sandboxes
And dead opencode server connections will still auto-prune!
Summary
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.