fix(httpapi): return pty error bodies (#28838)
This commit is contained in:
@@ -149,6 +149,23 @@ export class McpServerNotFoundError extends Schema.TaggedErrorClass<McpServerNot
|
|||||||
{ httpApiStatus: 404 },
|
{ httpApiStatus: 404 },
|
||||||
) {}
|
) {}
|
||||||
|
|
||||||
|
export class PtyNotFoundError extends Schema.TaggedErrorClass<PtyNotFoundError>()(
|
||||||
|
"PtyNotFoundError",
|
||||||
|
{
|
||||||
|
ptyID: Schema.String,
|
||||||
|
message: Schema.String,
|
||||||
|
},
|
||||||
|
{ httpApiStatus: 404 },
|
||||||
|
) {}
|
||||||
|
|
||||||
|
export class PtyForbiddenError extends Schema.TaggedErrorClass<PtyForbiddenError>()(
|
||||||
|
"PtyForbiddenError",
|
||||||
|
{
|
||||||
|
message: Schema.String,
|
||||||
|
},
|
||||||
|
{ httpApiStatus: 403 },
|
||||||
|
) {}
|
||||||
|
|
||||||
export class ApiNotFoundError extends Schema.ErrorClass<ApiNotFoundError>("NotFoundError")(
|
export class ApiNotFoundError extends Schema.ErrorClass<ApiNotFoundError>("NotFoundError")(
|
||||||
{
|
{
|
||||||
name: Schema.Literal("NotFoundError"),
|
name: Schema.Literal("NotFoundError"),
|
||||||
|
|||||||
@@ -10,7 +10,7 @@ import {
|
|||||||
WorkspaceRoutingQuery,
|
WorkspaceRoutingQuery,
|
||||||
WorkspaceRoutingQueryFields,
|
WorkspaceRoutingQueryFields,
|
||||||
} from "../middleware/workspace-routing"
|
} from "../middleware/workspace-routing"
|
||||||
import { ApiNotFoundError } from "../errors"
|
import { PtyForbiddenError, PtyNotFoundError } from "../errors"
|
||||||
import { described } from "./metadata"
|
import { described } from "./metadata"
|
||||||
|
|
||||||
const root = "/pty"
|
const root = "/pty"
|
||||||
@@ -76,7 +76,7 @@ export const PtyApi = HttpApi.make("pty")
|
|||||||
params: { ptyID: PtyID },
|
params: { ptyID: PtyID },
|
||||||
query: WorkspaceRoutingQuery,
|
query: WorkspaceRoutingQuery,
|
||||||
success: described(Pty.Info, "Session info"),
|
success: described(Pty.Info, "Session info"),
|
||||||
error: ApiNotFoundError,
|
error: PtyNotFoundError,
|
||||||
}).annotateMerge(
|
}).annotateMerge(
|
||||||
OpenApi.annotations({
|
OpenApi.annotations({
|
||||||
identifier: "pty.get",
|
identifier: "pty.get",
|
||||||
@@ -89,7 +89,7 @@ export const PtyApi = HttpApi.make("pty")
|
|||||||
query: WorkspaceRoutingQuery,
|
query: WorkspaceRoutingQuery,
|
||||||
payload: Pty.UpdateInput,
|
payload: Pty.UpdateInput,
|
||||||
success: described(Pty.Info, "Updated session"),
|
success: described(Pty.Info, "Updated session"),
|
||||||
error: [HttpApiError.BadRequest, ApiNotFoundError],
|
error: [PtyNotFoundError, HttpApiError.BadRequest],
|
||||||
}).annotateMerge(
|
}).annotateMerge(
|
||||||
OpenApi.annotations({
|
OpenApi.annotations({
|
||||||
identifier: "pty.update",
|
identifier: "pty.update",
|
||||||
@@ -101,7 +101,7 @@ export const PtyApi = HttpApi.make("pty")
|
|||||||
params: { ptyID: PtyID },
|
params: { ptyID: PtyID },
|
||||||
query: WorkspaceRoutingQuery,
|
query: WorkspaceRoutingQuery,
|
||||||
success: described(Schema.Boolean, "Session removed"),
|
success: described(Schema.Boolean, "Session removed"),
|
||||||
error: ApiNotFoundError,
|
error: PtyNotFoundError,
|
||||||
}).annotateMerge(
|
}).annotateMerge(
|
||||||
OpenApi.annotations({
|
OpenApi.annotations({
|
||||||
identifier: "pty.remove",
|
identifier: "pty.remove",
|
||||||
@@ -113,7 +113,7 @@ export const PtyApi = HttpApi.make("pty")
|
|||||||
params: { ptyID: PtyID },
|
params: { ptyID: PtyID },
|
||||||
query: WorkspaceRoutingQuery,
|
query: WorkspaceRoutingQuery,
|
||||||
success: described(PtyTicket.ConnectToken, "WebSocket connect token"),
|
success: described(PtyTicket.ConnectToken, "WebSocket connect token"),
|
||||||
error: [HttpApiError.Forbidden, ApiNotFoundError],
|
error: [PtyForbiddenError, PtyNotFoundError],
|
||||||
}).annotateMerge(
|
}).annotateMerge(
|
||||||
OpenApi.annotations({
|
OpenApi.annotations({
|
||||||
identifier: "pty.connectToken",
|
identifier: "pty.connectToken",
|
||||||
|
|||||||
@@ -12,7 +12,7 @@ import {
|
|||||||
} from "@/server/shared/pty-ticket"
|
} from "@/server/shared/pty-ticket"
|
||||||
import { Effect } from "effect"
|
import { Effect } from "effect"
|
||||||
import { HttpRouter, HttpServerRequest, HttpServerResponse } from "effect/unstable/http"
|
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 * as Socket from "effect/unstable/socket/Socket"
|
||||||
import { InstanceHttpApi } from "../api"
|
import { InstanceHttpApi } from "../api"
|
||||||
import * as ApiError from "../errors"
|
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 get = Effect.fn("PtyHttpApi.get")(function* (ctx: { params: { ptyID: PtyID } }) {
|
||||||
const info = yield* pty.get(ctx.params.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
|
return info
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -59,11 +63,20 @@ export const ptyHandlers = HttpApiBuilder.group(InstanceHttpApi, "pty", (handler
|
|||||||
...ctx.payload,
|
...ctx.payload,
|
||||||
size: ctx.payload.size ? { ...ctx.payload.size } : undefined,
|
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
|
return info
|
||||||
})
|
})
|
||||||
|
|
||||||
const remove = Effect.fn("PtyHttpApi.remove")(function* (ctx: { params: { ptyID: PtyID } }) {
|
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)
|
yield* pty.remove(ctx.params.ptyID)
|
||||||
return true
|
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 connectToken = Effect.fn("PtyHttpApi.connectToken")(function* (ctx: { params: { ptyID: PtyID } }) {
|
||||||
const request = yield* HttpServerRequest.HttpServerRequest
|
const request = yield* HttpServerRequest.HttpServerRequest
|
||||||
if (request.headers[PTY_CONNECT_TOKEN_HEADER] !== PTY_CONNECT_TOKEN_HEADER_VALUE || !validOrigin(request, cors))
|
if (request.headers[PTY_CONNECT_TOKEN_HEADER] !== PTY_CONNECT_TOKEN_HEADER_VALUE || !validOrigin(request, cors))
|
||||||
return yield* new HttpApiError.Forbidden({})
|
return yield* new ApiError.PtyForbiddenError({ message: "Invalid PTY connect token request" })
|
||||||
if (!(yield* pty.get(ctx.params.ptyID))) return yield* ApiError.notFound("Session not found")
|
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) })
|
return yield* tickets.issue({ ptyID: ctx.params.ptyID, ...(yield* PtyTicket.scope) })
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|||||||
@@ -371,7 +371,6 @@ function referencesComponent(input: unknown, name: string): boolean {
|
|||||||
|
|
||||||
function normalizeLegacyOperation(operation: OpenApiOperation, path: string, method: string) {
|
function normalizeLegacyOperation(operation: OpenApiOperation, path: string, method: string) {
|
||||||
if (path === "/experimental/console/switch" && method === "post") delete operation.responses?.["400"]
|
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
|
if ((path !== "/session/{sessionID}/message" && path !== "/session/{sessionID}/command") || method !== "post") return
|
||||||
const response = operation.responses?.["200"]?.content?.["application/json"]
|
const response = operation.responses?.["200"]?.content?.["application/json"]
|
||||||
if (!response) return
|
if (!response) return
|
||||||
|
|||||||
@@ -404,9 +404,7 @@ const scenarios: Scenario[] = [
|
|||||||
.delete("/pty/{ptyID}", "pty.remove")
|
.delete("/pty/{ptyID}", "pty.remove")
|
||||||
.mutating()
|
.mutating()
|
||||||
.at((ctx) => ({ path: route("/pty/{ptyID}", { ptyID: "pty_httpapi_missing" }), headers: ctx.headers() }))
|
.at((ctx) => ({ path: route("/pty/{ptyID}", { ptyID: "pty_httpapi_missing" }), headers: ctx.headers() }))
|
||||||
.json(200, (body) => {
|
.json(404, object, "status"),
|
||||||
check(body === true, "PTY remove should return true")
|
|
||||||
}),
|
|
||||||
http.protected
|
http.protected
|
||||||
.get("/pty/{ptyID}/connect", "pty.connect")
|
.get("/pty/{ptyID}/connect", "pty.connect")
|
||||||
.at((ctx) => ({ path: route("/pty/{ptyID}/connect", { ptyID: "pty_httpapi_missing" }), headers: ctx.headers() }))
|
.at((ctx) => ({ path: route("/pty/{ptyID}/connect", { ptyID: "pty_httpapi_missing" }), headers: ctx.headers() }))
|
||||||
|
|||||||
@@ -112,6 +112,31 @@ describe("pty HttpApi bridge", () => {
|
|||||||
|
|
||||||
const missing = await app().request(PtyPaths.get.replace(":ptyID", info.id), { headers })
|
const missing = await app().request(PtyPaths.get.replace(":ptyID", info.id), { headers })
|
||||||
expect(missing.status).toBe(404)
|
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 () => {
|
test("returns 404 for missing PTY websocket before upgrade", async () => {
|
||||||
@@ -121,6 +146,37 @@ describe("pty HttpApi bridge", () => {
|
|||||||
})
|
})
|
||||||
expect(response.status).toBe(404)
|
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)(
|
;(process.platform === "win32" ? effectIt.live.skip : effectIt.live)(
|
||||||
"serves PTY websocket output and input through Effect routes",
|
"serves PTY websocket output and input through Effect routes",
|
||||||
() =>
|
() =>
|
||||||
|
|||||||
@@ -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",
|
||||||
|
)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user