Skip to content

Commit 76d814e

Browse files
authored
refactor(server): unify instance httpapi middleware routing
Unify declared instance HTTP API endpoints under typed middleware routing, including event streaming and PTY WebSocket connect handling.\n\nPreserve PTY connect compatibility by checking missing PTYs before parsing optional cursor and ticket query fields, with regression coverage.
1 parent 9941e70 commit 76d814e

18 files changed

Lines changed: 340 additions & 195 deletions

packages/opencode/src/server/routes/instance/httpapi/AGENTS.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,20 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session",
1414

1515
For SSE endpoints, stay in `HttpApiBuilder.group(...)` and return `HttpServerResponse.stream(...)` from the handler. Annotate the endpoint success schema with `HttpApiSchema.asText({ contentType: "text/event-stream" })` so OpenAPI documents the stream content type.
1616

17-
Use raw `HttpRouter.use(...)` only for routes that do not fit the request/response HttpApi model, such as WebSocket upgrade routes or catch-all fallback routes. Yield stable services at route-layer construction and close over them in `router.add(...)` callbacks.
17+
Use `HttpApiBuilder.group(...)` with `handleRaw(...)` for declared endpoints that need the raw request or response, including WebSocket upgrade routes. This keeps endpoint middleware, routing context, and OpenAPI metadata on one typed route tree.
1818

1919
```ts
20-
export const rawRoute = HttpRouter.use((router) =>
20+
export const ptyConnectHandlers = HttpApiBuilder.group(PtyConnectApi, "pty-connect", (handlers) =>
2121
Effect.gen(function* () {
2222
const pty = yield* Pty.Service
2323

24-
yield* router.add("GET", PtyPaths.connect, (request) => connectPty(request, pty))
24+
return handlers.handleRaw("connect", (ctx) => connectPty(ctx.request, pty))
2525
}),
2626
)
2727
```
2828

29+
Use raw `HttpRouter.use(...)` only for routes outside the declared API surface, such as a catch-all UI fallback.
30+
2931
Avoid `Effect.provide(SomeLayer)` inside request handlers or raw route callbacks. Stable layers should be provided once at the application/layer boundary, not rebuilt or scoped per request.
3032

3133
Avoid `HttpRouter.provideRequest(...)` unless the dependency is intentionally request-level. Prefer `HttpRouter.use(...)` for stable app services.
@@ -34,4 +36,4 @@ Use `Effect.provideService(...)` in middleware only for request-derived context,
3436

3537
Public JSON errors should be explicit `Schema.ErrorClass` contracts declared on each endpoint. Use built-in `HttpApiError.*` classes only when their empty/tagged body is the intended wire shape; for SDK-visible errors with messages, define an API error schema such as `ApiNotFoundError` and fail with that exact declared error. Keep domain and storage services free of HttpApi types, and translate expected domain errors at the handler boundary.
3638

37-
When adding middleware, compose it at the layer boundary and keep the route tree explicit in `server.ts`. Shared router middleware such as auth, workspace routing, and instance context should stay visible where routes are assembled.
39+
When adding middleware, declare endpoint-contract middleware on the owning `HttpApiGroup` and provide its implementation layer at the assembly boundary in `server.ts`. Keep router middleware for truly raw fallback routes or global transport policy.

packages/opencode/src/server/routes/instance/httpapi/groups/event.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { Schema } from "effect"
22
import { HttpApi, HttpApiEndpoint, HttpApiGroup, HttpApiSchema, OpenApi } from "effect/unstable/httpapi"
3-
import { WorkspaceRoutingQuery } from "../middleware/workspace-routing"
3+
import { Authorization } from "../middleware/authorization"
4+
import { InstanceContextMiddleware } from "../middleware/instance-context"
5+
import { WorkspaceRoutingMiddleware, WorkspaceRoutingQuery } from "../middleware/workspace-routing"
46

57
export const EventPaths = {
68
event: "/event",
@@ -20,5 +22,8 @@ export const EventApi = HttpApi.make("event").add(
2022
}),
2123
),
2224
)
25+
.middleware(InstanceContextMiddleware)
26+
.middleware(WorkspaceRoutingMiddleware)
27+
.middleware(Authorization)
2328
.annotateMerge(OpenApi.annotations({ title: "event", description: "Instance event stream route." })),
2429
)

packages/opencode/src/server/routes/instance/httpapi/groups/pty.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { Pty } from "@/pty"
22
import { PtyTicket } from "@/pty/ticket"
33
import { PtyID } from "@/pty/schema"
4+
import { PTY_CONNECT_TICKET_QUERY } from "@/server/shared/pty-ticket"
45
import { Schema } from "effect"
56
import { HttpApi, HttpApiEndpoint, HttpApiError, HttpApiGroup, OpenApi } from "effect/unstable/httpapi"
6-
import { Authorization } from "../middleware/authorization"
7+
import { Authorization, PtyConnectAuthorization } from "../middleware/authorization"
78
import { InstanceContextMiddleware } from "../middleware/instance-context"
89
import {
910
WorkspaceRoutingMiddleware,
@@ -138,9 +139,10 @@ export const PtyApi = HttpApi.make("pty")
138139
export const PtyConnectApi = HttpApi.make("pty-connect").add(
139140
HttpApiGroup.make("pty-connect")
140141
.add(
142+
// Decode PTY connection query fields in the raw handler after checking
143+
// existence, preserving the established empty-404 response ordering.
141144
HttpApiEndpoint.get("connect", PtyPaths.connect, {
142145
params: Params,
143-
query: WorkspaceRoutingQuery,
144146
success: described(Schema.Boolean, "Connected session"),
145147
error: [HttpApiError.Forbidden, HttpApiError.NotFound],
146148
}).annotateMerge(
@@ -149,8 +151,22 @@ export const PtyConnectApi = HttpApi.make("pty-connect").add(
149151
summary: "Connect to PTY session",
150152
description:
151153
"Establish a WebSocket connection to interact with a pseudo-terminal (PTY) session in real-time.",
154+
transform: (operation) => ({
155+
...operation,
156+
parameters: [
157+
...(operation.parameters ?? []),
158+
...["directory", "workspace", "cursor", PTY_CONNECT_TICKET_QUERY].map((name) => ({
159+
in: "query",
160+
name,
161+
schema: { type: "string" },
162+
})),
163+
],
164+
}),
152165
}),
153166
),
154167
)
155-
.annotateMerge(OpenApi.annotations({ title: "pty", description: "PTY websocket route." })),
168+
.annotateMerge(OpenApi.annotations({ title: "pty", description: "PTY websocket route." }))
169+
.middleware(InstanceContextMiddleware)
170+
.middleware(WorkspaceRoutingMiddleware)
171+
.middleware(PtyConnectAuthorization),
156172
)

packages/opencode/src/server/routes/instance/httpapi/handlers/pty.ts

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,13 @@ import {
1010
PTY_CONNECT_TOKEN_HEADER,
1111
PTY_CONNECT_TOKEN_HEADER_VALUE,
1212
} from "@/server/shared/pty-ticket"
13-
import { Effect } from "effect"
14-
import { HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/unstable/http"
13+
import { Effect, Option, Schema } from "effect"
14+
import { HttpServerRequest, HttpServerResponse } from "effect/unstable/http"
1515
import { HttpApiBuilder } from "effect/unstable/httpapi"
1616
import * as Socket from "effect/unstable/socket/Socket"
1717
import { InstanceHttpApi } from "../api"
1818
import * as ApiError from "../errors"
19-
import { CursorQuery, Params, PtyPaths } from "../groups/pty"
19+
import { CursorQuery, PtyConnectApi } from "../groups/pty"
2020
import { WebSocketTracker } from "../websocket-tracker"
2121

2222
function validOrigin(request: HttpServerRequest.HttpServerRequest, opts: CorsOptions | undefined) {
@@ -121,37 +121,39 @@ export const ptyHandlers = HttpApiBuilder.group(InstanceHttpApi, "pty", (handler
121121
}),
122122
)
123123

124-
export const ptyConnectRoute = HttpRouter.use((router) =>
124+
export const ptyConnectHandlers = HttpApiBuilder.group(PtyConnectApi, "pty-connect", (handlers) =>
125125
Effect.gen(function* () {
126126
const pty = yield* Pty.Service
127127
const tickets = yield* PtyTicket.Service
128128
const cors = yield* CorsConfig
129-
yield* router.add(
130-
"GET",
131-
PtyPaths.connect,
132-
Effect.gen(function* () {
133-
const params = yield* HttpRouter.schemaPathParams(Params)
134-
const exists = yield* pty.get(params.ptyID).pipe(
129+
130+
return handlers.handleRaw(
131+
"connect",
132+
Effect.fn("PtyHttpApi.connect")(function* (ctx: {
133+
params: { ptyID: PtyID }
134+
request: HttpServerRequest.HttpServerRequest
135+
}) {
136+
const exists = yield* pty.get(ctx.params.ptyID).pipe(
135137
Effect.as(true),
136138
Effect.catchTag("Pty.NotFoundError", () => Effect.succeed(false)),
137139
)
138140
if (!exists) return HttpServerResponse.empty({ status: 404 })
139141

140-
const query = yield* HttpServerRequest.schemaSearchParams(CursorQuery)
141-
const request = yield* HttpServerRequest.HttpServerRequest
142-
const ticket = new URL(request.url, "http://localhost").searchParams.get(PTY_CONNECT_TICKET_QUERY)
142+
const query = Schema.decodeUnknownOption(CursorQuery)(yield* HttpServerRequest.ParsedSearchParams)
143+
if (Option.isNone(query)) return HttpServerResponse.empty({ status: 400 })
144+
const ticket = new URL(ctx.request.url, "http://localhost").searchParams.get(PTY_CONNECT_TICKET_QUERY)
143145
if (ticket) {
144-
const valid = validOrigin(request, cors)
145-
? yield* tickets.consume({ ticket, ptyID: params.ptyID, ...(yield* PtyTicket.scope) })
146+
const valid = validOrigin(ctx.request, cors)
147+
? yield* tickets.consume({ ticket, ptyID: ctx.params.ptyID, ...(yield* PtyTicket.scope) })
146148
: false
147149
if (!valid) return HttpServerResponse.empty({ status: 403 })
148150
}
149-
const parsedCursor = query.cursor === undefined ? undefined : Number(query.cursor)
151+
const parsedCursor = query.value.cursor === undefined ? undefined : Number(query.value.cursor)
150152
const cursor =
151153
parsedCursor !== undefined && Number.isSafeInteger(parsedCursor) && parsedCursor >= -1
152154
? parsedCursor
153155
: undefined
154-
const socket = yield* Effect.orDie(request.upgrade)
156+
const socket = yield* Effect.orDie(ctx.request.upgrade)
155157
const write = yield* socket.writer
156158
const closeAccepted = (event: Socket.CloseEvent) =>
157159
socket
@@ -186,20 +188,16 @@ export const ptyConnectRoute = HttpRouter.use((router) =>
186188
},
187189
}
188190
const handler = yield* pty
189-
.connect(params.ptyID, adapter, cursor)
191+
.connect(ctx.params.ptyID, adapter, cursor)
190192
.pipe(
191193
Effect.catchTag("Pty.NotFoundError", () =>
192194
closeAccepted(new Socket.CloseEvent(4404, "session not found")).pipe(Effect.as(undefined)),
193195
),
194196
)
195197
if (!handler) return HttpServerResponse.empty()
196198

197-
// No `pending[]`-style early-frame buffer (the legacy handler had one).
198-
// `request.upgrade` returns a Socket without running the WS handshake; the
199-
// handshake fires inside `socket.runRaw` below, AFTER `pty.connect` resolves
200-
// and the message callback is registered. The client therefore can't fire
201-
// `open` and start sending until the listener is already wired. Don't move
202-
// `runRaw` ahead of `pty.connect` without re-introducing a buffer.
199+
// The handshake runs inside `socket.runRaw`, after the input callback is
200+
// registered, so the client cannot send frames before PTY input is wired.
203201
yield* socket
204202
.runRaw((message) => handlePtyInput(handler, message))
205203
.pipe(

packages/opencode/src/server/routes/instance/httpapi/middleware/authorization.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@ export class V2Authorization extends HttpApiMiddleware.Service<V2Authorization>(
2727
},
2828
) {}
2929

30+
export class PtyConnectAuthorization extends HttpApiMiddleware.Service<PtyConnectAuthorization>()(
31+
"@opencode/ExperimentalHttpApiPtyConnectAuthorization",
32+
{
33+
error: HttpApiError.UnauthorizedNoContent,
34+
},
35+
) {}
36+
3037
function emptyCredential() {
3138
return {
3239
username: "",
@@ -105,7 +112,6 @@ export const authorizationRouterMiddleware = HttpRouter.middleware()(
105112
const request = yield* HttpServerRequest.HttpServerRequest
106113
const url = new URL(request.url, "http://localhost")
107114
if (isPublicUIPath(request.method, url.pathname)) return yield* effect
108-
if (hasPtyConnectTicketURL(url)) return yield* effect
109115
return yield* credentialFromURL(url, request).pipe(
110116
Effect.flatMap((credential) => validateRawCredential(effect, credential, config)),
111117
)
@@ -129,6 +135,24 @@ export const authorizationLayer = Layer.effect(
129135
}),
130136
)
131137

138+
export const ptyConnectAuthorizationLayer = Layer.effect(
139+
PtyConnectAuthorization,
140+
Effect.gen(function* () {
141+
const config = yield* ServerAuth.Config
142+
if (!ServerAuth.required(config)) return PtyConnectAuthorization.of((effect) => effect)
143+
return PtyConnectAuthorization.of((effect) =>
144+
Effect.gen(function* () {
145+
const request = yield* HttpServerRequest.HttpServerRequest
146+
const url = new URL(request.url, "http://localhost")
147+
if (hasPtyConnectTicketURL(url)) return yield* effect
148+
return yield* credentialFromURL(url, request).pipe(
149+
Effect.flatMap((credential) => validateCredential(effect, credential, config)),
150+
)
151+
}),
152+
)
153+
}),
154+
)
155+
132156
export const v2AuthorizationLayer = Layer.effect(
133157
V2Authorization,
134158
Effect.gen(function* () {

packages/opencode/src/server/routes/instance/httpapi/middleware/instance-context.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { InstanceRef, WorkspaceRef } from "@/effect/instance-ref"
22
import { InstanceStore } from "@/project/instance-store"
33
import { Effect, Layer } from "effect"
4-
import { HttpRouter, HttpServerResponse } from "effect/unstable/http"
4+
import { HttpServerResponse } from "effect/unstable/http"
55
import { HttpApiMiddleware } from "effect/unstable/httpapi"
66
import { WorkspaceRouteContext } from "./workspace-routing"
77

@@ -41,10 +41,3 @@ export const instanceContextLayer = Layer.effect(
4141
return InstanceContextMiddleware.of((effect) => provideInstanceContext(effect, store))
4242
}),
4343
)
44-
45-
export const instanceRouterMiddleware = HttpRouter.middleware()(
46-
Effect.gen(function* () {
47-
const store = yield* InstanceStore.Service
48-
return (effect) => provideInstanceContext(effect, store)
49-
}),
50-
)

packages/opencode/src/server/routes/instance/httpapi/middleware/workspace-routing.ts

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { getWorkspaceRouteSessionID, isLocalWorkspaceRoute, workspaceProxyURL }
99
import { NotFoundError } from "@/storage/storage"
1010
import { Flag } from "@opencode-ai/core/flag/flag"
1111
import { Context, Data, Effect, Layer, Option, Schema } from "effect"
12-
import { HttpClient, HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/unstable/http"
12+
import { HttpClient, HttpServerRequest, HttpServerResponse } from "effect/unstable/http"
1313
import { HttpApiMiddleware } from "effect/unstable/httpapi"
1414
import * as Socket from "effect/unstable/socket/Socket"
1515
import { InvalidRequestError } from "../errors"
@@ -219,7 +219,10 @@ function routeHttpApiWorkspace<E>(
219219
const sessionID = getWorkspaceRouteSessionID(requestURL(request))
220220
const session = sessionID
221221
? yield* Session.Service.use((svc) => svc.get(sessionID)).pipe(
222-
Effect.catchIf(NotFoundError.isInstance, () => Effect.succeed(undefined)),
222+
Effect.catchIf(
223+
(error): error is NotFoundError => NotFoundError.isInstance(error),
224+
() => Effect.succeed(undefined),
225+
),
223226
Effect.catchDefect(() => Effect.succeed(undefined)),
224227
)
225228
: undefined
@@ -242,20 +245,3 @@ export const workspaceRoutingLayer = Layer.effect(
242245
)
243246
}),
244247
)
245-
246-
export const workspaceRouterMiddleware = HttpRouter.middleware<{ provides: WorkspaceRouteContext }>()(
247-
Effect.gen(function* () {
248-
const makeWebSocket = yield* Socket.WebSocketConstructor
249-
const workspace = yield* Workspace.Service
250-
const client = yield* HttpClient.HttpClient
251-
return (effect) =>
252-
Effect.gen(function* () {
253-
const request = yield* HttpServerRequest.HttpServerRequest
254-
const plan = yield* planRequest(request)
255-
return yield* routeWorkspace(client, effect, plan)
256-
}).pipe(
257-
Effect.provideService(Socket.WebSocketConstructor, makeWebSocket),
258-
Effect.provideService(Workspace.Service, workspace),
259-
)
260-
}),
261-
)

0 commit comments

Comments
 (0)