diff --git a/packages/opencode/src/server/routes/instance/httpapi/errors.ts b/packages/opencode/src/server/routes/instance/httpapi/errors.ts index d4fc232e3d..c1a0691c7f 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/errors.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/errors.ts @@ -149,6 +149,23 @@ export class McpServerNotFoundError extends Schema.TaggedErrorClass()( + "PtyNotFoundError", + { + ptyID: Schema.String, + message: Schema.String, + }, + { httpApiStatus: 404 }, +) {} + +export class PtyForbiddenError extends Schema.TaggedErrorClass()( + "PtyForbiddenError", + { + message: Schema.String, + }, + { httpApiStatus: 403 }, +) {} + export class ApiNotFoundError extends Schema.ErrorClass("NotFoundError")( { name: Schema.Literal("NotFoundError"), diff --git a/packages/opencode/src/server/routes/instance/httpapi/groups/pty.ts b/packages/opencode/src/server/routes/instance/httpapi/groups/pty.ts index 1391d2a919..3adc4e5c36 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/groups/pty.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/groups/pty.ts @@ -10,7 +10,7 @@ import { WorkspaceRoutingQuery, WorkspaceRoutingQueryFields, } from "../middleware/workspace-routing" -import { ApiNotFoundError } from "../errors" +import { PtyForbiddenError, PtyNotFoundError } from "../errors" import { described } from "./metadata" const root = "/pty" @@ -76,7 +76,7 @@ export const PtyApi = HttpApi.make("pty") params: { ptyID: PtyID }, query: WorkspaceRoutingQuery, success: described(Pty.Info, "Session info"), - error: ApiNotFoundError, + error: PtyNotFoundError, }).annotateMerge( OpenApi.annotations({ identifier: "pty.get", @@ -89,7 +89,7 @@ export const PtyApi = HttpApi.make("pty") query: WorkspaceRoutingQuery, payload: Pty.UpdateInput, success: described(Pty.Info, "Updated session"), - error: [HttpApiError.BadRequest, ApiNotFoundError], + error: [PtyNotFoundError, HttpApiError.BadRequest], }).annotateMerge( OpenApi.annotations({ identifier: "pty.update", @@ -101,7 +101,7 @@ export const PtyApi = HttpApi.make("pty") params: { ptyID: PtyID }, query: WorkspaceRoutingQuery, success: described(Schema.Boolean, "Session removed"), - error: ApiNotFoundError, + error: PtyNotFoundError, }).annotateMerge( OpenApi.annotations({ identifier: "pty.remove", @@ -113,7 +113,7 @@ export const PtyApi = HttpApi.make("pty") params: { ptyID: PtyID }, query: WorkspaceRoutingQuery, success: described(PtyTicket.ConnectToken, "WebSocket connect token"), - error: [HttpApiError.Forbidden, ApiNotFoundError], + error: [PtyForbiddenError, PtyNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "pty.connectToken", diff --git a/packages/opencode/src/server/routes/instance/httpapi/handlers/pty.ts b/packages/opencode/src/server/routes/instance/httpapi/handlers/pty.ts index f4d6adb935..bcf6aef804 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/pty.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/pty.ts @@ -12,7 +12,7 @@ import { } from "@/server/shared/pty-ticket" import { Effect } from "effect" import { HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/unstable/http" -import { HttpApiBuilder, HttpApiError } from "effect/unstable/httpapi" +import { HttpApiBuilder } from "effect/unstable/httpapi" import * as Socket from "effect/unstable/socket/Socket" import { InstanceHttpApi } from "../api" import * as ApiError from "../errors" @@ -47,7 +47,11 @@ export const ptyHandlers = HttpApiBuilder.group(InstanceHttpApi, "pty", (handler const get = Effect.fn("PtyHttpApi.get")(function* (ctx: { params: { ptyID: PtyID } }) { const info = yield* pty.get(ctx.params.ptyID) - if (!info) return yield* ApiError.notFound("Session not found") + if (!info) + return yield* new ApiError.PtyNotFoundError({ + ptyID: ctx.params.ptyID, + message: `PTY session not found: ${ctx.params.ptyID}`, + }) return info }) @@ -59,11 +63,20 @@ export const ptyHandlers = HttpApiBuilder.group(InstanceHttpApi, "pty", (handler ...ctx.payload, size: ctx.payload.size ? { ...ctx.payload.size } : undefined, }) - if (!info) return yield* ApiError.notFound("Session not found") + if (!info) + return yield* new ApiError.PtyNotFoundError({ + ptyID: ctx.params.ptyID, + message: `PTY session not found: ${ctx.params.ptyID}`, + }) return info }) const remove = Effect.fn("PtyHttpApi.remove")(function* (ctx: { params: { ptyID: PtyID } }) { + if (!(yield* pty.get(ctx.params.ptyID))) + return yield* new ApiError.PtyNotFoundError({ + ptyID: ctx.params.ptyID, + message: `PTY session not found: ${ctx.params.ptyID}`, + }) yield* pty.remove(ctx.params.ptyID) return true }) @@ -71,8 +84,12 @@ export const ptyHandlers = HttpApiBuilder.group(InstanceHttpApi, "pty", (handler const connectToken = Effect.fn("PtyHttpApi.connectToken")(function* (ctx: { params: { ptyID: PtyID } }) { const request = yield* HttpServerRequest.HttpServerRequest if (request.headers[PTY_CONNECT_TOKEN_HEADER] !== PTY_CONNECT_TOKEN_HEADER_VALUE || !validOrigin(request, cors)) - return yield* new HttpApiError.Forbidden({}) - if (!(yield* pty.get(ctx.params.ptyID))) return yield* ApiError.notFound("Session not found") + return yield* new ApiError.PtyForbiddenError({ message: "Invalid PTY connect token request" }) + if (!(yield* pty.get(ctx.params.ptyID))) + return yield* new ApiError.PtyNotFoundError({ + ptyID: ctx.params.ptyID, + message: `PTY session not found: ${ctx.params.ptyID}`, + }) return yield* tickets.issue({ ptyID: ctx.params.ptyID, ...(yield* PtyTicket.scope) }) }) diff --git a/packages/opencode/src/server/routes/instance/httpapi/public.ts b/packages/opencode/src/server/routes/instance/httpapi/public.ts index 7eb4497161..91a50c263a 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/public.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/public.ts @@ -371,7 +371,6 @@ function referencesComponent(input: unknown, name: string): boolean { function normalizeLegacyOperation(operation: OpenApiOperation, path: string, method: string) { if (path === "/experimental/console/switch" && method === "post") delete operation.responses?.["400"] - if (path === "/pty/{ptyID}" && method === "put") delete operation.responses?.["404"] if ((path !== "/session/{sessionID}/message" && path !== "/session/{sessionID}/command") || method !== "post") return const response = operation.responses?.["200"]?.content?.["application/json"] if (!response) return diff --git a/packages/opencode/test/server/httpapi-exercise/index.ts b/packages/opencode/test/server/httpapi-exercise/index.ts index 0c834e91b7..5c822c1109 100644 --- a/packages/opencode/test/server/httpapi-exercise/index.ts +++ b/packages/opencode/test/server/httpapi-exercise/index.ts @@ -404,9 +404,7 @@ const scenarios: Scenario[] = [ .delete("/pty/{ptyID}", "pty.remove") .mutating() .at((ctx) => ({ path: route("/pty/{ptyID}", { ptyID: "pty_httpapi_missing" }), headers: ctx.headers() })) - .json(200, (body) => { - check(body === true, "PTY remove should return true") - }), + .json(404, object, "status"), http.protected .get("/pty/{ptyID}/connect", "pty.connect") .at((ctx) => ({ path: route("/pty/{ptyID}/connect", { ptyID: "pty_httpapi_missing" }), headers: ctx.headers() })) diff --git a/packages/opencode/test/server/httpapi-pty.test.ts b/packages/opencode/test/server/httpapi-pty.test.ts index 0f10dbd3a7..d1e4660f2e 100644 --- a/packages/opencode/test/server/httpapi-pty.test.ts +++ b/packages/opencode/test/server/httpapi-pty.test.ts @@ -112,6 +112,31 @@ describe("pty HttpApi bridge", () => { const missing = await app().request(PtyPaths.get.replace(":ptyID", info.id), { headers }) expect(missing.status).toBe(404) + expect(await missing.json()).toEqual({ + _tag: "PtyNotFoundError", + ptyID: info.id, + message: `PTY session not found: ${info.id}`, + }) + + const missingUpdate = await app().request(PtyPaths.update.replace(":ptyID", info.id), { + method: "PUT", + headers: { ...headers, "content-type": "application/json" }, + body: JSON.stringify({ title: "missing" }), + }) + expect(missingUpdate.status).toBe(404) + expect(await missingUpdate.json()).toEqual({ + _tag: "PtyNotFoundError", + ptyID: info.id, + message: `PTY session not found: ${info.id}`, + }) + + const missingRemove = await app().request(PtyPaths.remove.replace(":ptyID", info.id), { method: "DELETE", headers }) + expect(missingRemove.status).toBe(404) + expect(await missingRemove.json()).toEqual({ + _tag: "PtyNotFoundError", + ptyID: info.id, + message: `PTY session not found: ${info.id}`, + }) }) test("returns 404 for missing PTY websocket before upgrade", async () => { @@ -121,6 +146,37 @@ describe("pty HttpApi bridge", () => { }) expect(response.status).toBe(404) }) + + test("returns typed errors for PTY connect token failures", async () => { + await using tmp = await tmpdir({ git: true, config: { formatter: false, lsp: false } }) + const headers = { "x-opencode-directory": tmp.path } + const missingID = String(PtyID.ascending()) + + const forbidden = await app().request(PtyPaths.connectToken.replace(":ptyID", missingID), { + method: "POST", + headers, + }) + expect(forbidden.status).toBe(403) + expect(await forbidden.json()).toEqual({ + _tag: "PtyForbiddenError", + message: "Invalid PTY connect token request", + }) + + const missing = await app().request(PtyPaths.connectToken.replace(":ptyID", missingID), { + method: "POST", + headers: { + ...headers, + "x-opencode-ticket": "1", + }, + }) + expect(missing.status).toBe(404) + expect(await missing.json()).toEqual({ + _tag: "PtyNotFoundError", + ptyID: missingID, + message: `PTY session not found: ${missingID}`, + }) + }) + ;(process.platform === "win32" ? effectIt.live.skip : effectIt.live)( "serves PTY websocket output and input through Effect routes", () => diff --git a/packages/opencode/test/server/httpapi-public-openapi.test.ts b/packages/opencode/test/server/httpapi-public-openapi.test.ts index afe3dd3f07..c8069bd312 100644 --- a/packages/opencode/test/server/httpapi-public-openapi.test.ts +++ b/packages/opencode/test/server/httpapi-public-openapi.test.ts @@ -190,4 +190,22 @@ describe("PublicApi OpenAPI v2 errors", () => { ) } }) + + test("documents PTY resource and ticket errors", () => { + const spec = OpenApi.fromApi(PublicApi) as OpenApiSpec + + for (const route of [ + ["get", "/pty/{ptyID}"], + ["put", "/pty/{ptyID}"], + ["delete", "/pty/{ptyID}"], + ["post", "/pty/{ptyID}/connect-token"], + ] as const) { + expect(componentName(responseRef(spec.paths[route[1]]?.[route[0]]?.responses?.["404"]) ?? "")).toBe( + "PtyNotFoundError", + ) + } + expect(componentName(responseRef(spec.paths["/pty/{ptyID}/connect-token"]?.post?.responses?.["403"]) ?? "")).toBe( + "PtyForbiddenError", + ) + }) })