diff --git a/packages/opencode/src/pty/index.ts b/packages/opencode/src/pty/index.ts index 6f18856fde..5cb4d716ff 100644 --- a/packages/opencode/src/pty/index.ts +++ b/packages/opencode/src/pty/index.ts @@ -87,6 +87,10 @@ export const UpdateInput = Schema.Struct({ export type UpdateInput = Types.DeepMutable> +export class NotFoundError extends Schema.TaggedErrorClass()("Pty.NotFoundError", { + ptyID: PtyID, +}) {} + export const Event = { Created: BusEvent.define("pty.created", Schema.Struct({ info: Info })), Updated: BusEvent.define("pty.updated", Schema.Struct({ info: Info })), @@ -96,17 +100,17 @@ export const Event = { export interface Interface { readonly list: () => Effect.Effect - readonly get: (id: PtyID) => Effect.Effect + readonly get: (id: PtyID) => Effect.Effect readonly create: (input: CreateInput) => Effect.Effect - readonly update: (id: PtyID, input: UpdateInput) => Effect.Effect - readonly remove: (id: PtyID) => Effect.Effect - readonly resize: (id: PtyID, cols: number, rows: number) => Effect.Effect - readonly write: (id: PtyID, data: string) => Effect.Effect + readonly update: (id: PtyID, input: UpdateInput) => Effect.Effect + readonly remove: (id: PtyID) => Effect.Effect + readonly resize: (id: PtyID, cols: number, rows: number) => Effect.Effect + readonly write: (id: PtyID, data: string) => Effect.Effect readonly connect: ( id: PtyID, ws: Socket, cursor?: number, - ) => Effect.Effect<{ onMessage: (message: string | ArrayBuffer) => void; onClose: () => void } | undefined> + ) => Effect.Effect<{ onMessage: (message: string | ArrayBuffer) => void; onClose: () => void } | undefined, NotFoundError> } export class Service extends Context.Service()("@opencode/Pty") {} @@ -150,10 +154,15 @@ export const layer = Layer.effect( }), ) + const requireSession = Effect.fn("Pty.requireSession")(function* (id: PtyID) { + const session = (yield* InstanceState.get(state)).sessions.get(id) + if (!session) return yield* new NotFoundError({ ptyID: id }) + return session + }) + const remove = Effect.fn("Pty.remove")(function* (id: PtyID) { const s = yield* InstanceState.get(state) - const session = s.sessions.get(id) - if (!session) return + const session = yield* requireSession(id) s.sessions.delete(id) log.info("removing session", { id }) teardown(session) @@ -166,8 +175,7 @@ export const layer = Layer.effect( }) const get = Effect.fn("Pty.get")(function* (id: PtyID) { - const s = yield* InstanceState.get(state) - return s.sessions.get(id)?.info + return (yield* requireSession(id)).info }) const create = Effect.fn("Pty.create")(function* (input: CreateInput) { @@ -262,9 +270,7 @@ export const layer = Layer.effect( }) const update = Effect.fn("Pty.update")(function* (id: PtyID, input: UpdateInput) { - const s = yield* InstanceState.get(state) - const session = s.sessions.get(id) - if (!session) return + const session = yield* requireSession(id) if (input.title) { session.info.title = input.title } @@ -276,28 +282,27 @@ export const layer = Layer.effect( }) const resize = Effect.fn("Pty.resize")(function* (id: PtyID, cols: number, rows: number) { - const s = yield* InstanceState.get(state) - const session = s.sessions.get(id) - if (session && session.info.status === "running") { + const session = yield* requireSession(id) + if (session.info.status === "running") { session.process.resize(cols, rows) } }) const write = Effect.fn("Pty.write")(function* (id: PtyID, data: string) { - const s = yield* InstanceState.get(state) - const session = s.sessions.get(id) - if (session && session.info.status === "running") { + const session = yield* requireSession(id) + if (session.info.status === "running") { session.process.write(data) } }) const connect = Effect.fn("Pty.connect")(function* (id: PtyID, ws: Socket, cursor?: number) { - const s = yield* InstanceState.get(state) - const session = s.sessions.get(id) - if (!session) { - ws.close() - return - } + const session = yield* requireSession(id).pipe( + Effect.tapError(() => + Effect.sync(() => { + ws.close() + }), + ), + ) log.info("client connected to session", { id }) const sub = sock(ws) 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 bcf6aef804..d439902a01 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/pty.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/pty.ts @@ -46,38 +46,50 @@ 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* new ApiError.PtyNotFoundError({ - ptyID: ctx.params.ptyID, - message: `PTY session not found: ${ctx.params.ptyID}`, - }) - return info + return yield* pty.get(ctx.params.ptyID).pipe( + Effect.catchTag("Pty.NotFoundError", (error) => + Effect.fail( + new ApiError.PtyNotFoundError({ + ptyID: error.ptyID, + message: `PTY session not found: ${error.ptyID}`, + }), + ), + ), + ) }) const update = Effect.fn("PtyHttpApi.update")(function* (ctx: { params: { ptyID: PtyID } payload: typeof Pty.UpdateInput.Type }) { - const info = yield* pty.update(ctx.params.ptyID, { - ...ctx.payload, - size: ctx.payload.size ? { ...ctx.payload.size } : undefined, - }) - if (!info) - return yield* new ApiError.PtyNotFoundError({ - ptyID: ctx.params.ptyID, - message: `PTY session not found: ${ctx.params.ptyID}`, + return yield* pty + .update(ctx.params.ptyID, { + ...ctx.payload, + size: ctx.payload.size ? { ...ctx.payload.size } : undefined, }) - return info + .pipe( + Effect.catchTag("Pty.NotFoundError", (error) => + Effect.fail( + new ApiError.PtyNotFoundError({ + ptyID: error.ptyID, + message: `PTY session not found: ${error.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).pipe( + Effect.catchTag("Pty.NotFoundError", (error) => + Effect.fail( + new ApiError.PtyNotFoundError({ + ptyID: error.ptyID, + message: `PTY session not found: ${error.ptyID}`, + }), + ), + ), + ) return true }) @@ -85,11 +97,16 @@ export const ptyHandlers = HttpApiBuilder.group(InstanceHttpApi, "pty", (handler const request = yield* HttpServerRequest.HttpServerRequest if (request.headers[PTY_CONNECT_TOKEN_HEADER] !== PTY_CONNECT_TOKEN_HEADER_VALUE || !validOrigin(request, cors)) 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}`, - }) + yield* pty.get(ctx.params.ptyID).pipe( + Effect.catchTag("Pty.NotFoundError", (error) => + Effect.fail( + new ApiError.PtyNotFoundError({ + ptyID: error.ptyID, + message: `PTY session not found: ${error.ptyID}`, + }), + ), + ), + ) return yield* tickets.issue({ ptyID: ctx.params.ptyID, ...(yield* PtyTicket.scope) }) }) @@ -114,7 +131,11 @@ export const ptyConnectRoute = HttpRouter.use((router) => PtyPaths.connect, Effect.gen(function* () { const params = yield* HttpRouter.schemaPathParams(Params) - if (!(yield* pty.get(params.ptyID))) return HttpServerResponse.empty({ status: 404 }) + const exists = yield* pty.get(params.ptyID).pipe( + Effect.as(true), + Effect.catchTag("Pty.NotFoundError", () => Effect.succeed(false)), + ) + if (!exists) return HttpServerResponse.empty({ status: 404 }) const query = yield* HttpServerRequest.schemaSearchParams(CursorQuery) const request = yield* HttpServerRequest.HttpServerRequest @@ -164,11 +185,12 @@ export const ptyConnectRoute = HttpRouter.use((router) => writeScoped(write(new Socket.CloseEvent(code, reason))) }, } - const handler = yield* pty.connect(params.ptyID, adapter, cursor) - if (!handler) { - yield* closeAccepted(new Socket.CloseEvent(4404, "session not found")) - return HttpServerResponse.empty() - } + const handler = yield* pty.connect(params.ptyID, adapter, cursor).pipe( + Effect.catchTag("Pty.NotFoundError", () => + closeAccepted(new Socket.CloseEvent(4404, "session not found")).pipe(Effect.as(undefined)), + ), + ) + if (!handler) return HttpServerResponse.empty() // No `pending[]`-style early-frame buffer (the legacy handler had one). // `request.upgrade` returns a Socket without running the WS handshake; the diff --git a/packages/opencode/test/pty/pty-session.test.ts b/packages/opencode/test/pty/pty-session.test.ts index 12784baf31..9fda48cc91 100644 --- a/packages/opencode/test/pty/pty-session.test.ts +++ b/packages/opencode/test/pty/pty-session.test.ts @@ -4,7 +4,7 @@ import { Config } from "../../src/config/config" import { Plugin } from "../../src/plugin" import { Pty } from "../../src/pty" import type { PtyID } from "../../src/pty/schema" -import { Effect, Layer, Queue } from "effect" +import { Cause, Effect, Exit, Layer, Queue } from "effect" import { testEffect } from "../lib/effect" type PtyEvent = { type: "created" | "exited" | "deleted"; id: PtyID } @@ -66,6 +66,54 @@ const waitForEvents = (events: Queue.Queue, id: PtyID, count: number) } describe("pty", () => { + it.instance( + "returns typed not found errors for missing sessions", + () => + Effect.gen(function* () { + const pty = yield* Pty.Service + const id = "pty_missing" as PtyID + let closed = false + const socket = { + readyState: 1, + send: () => {}, + close: () => { + closed = true + }, + } + + const get = yield* pty.get(id).pipe(Effect.exit) + expect(Exit.isFailure(get)).toBe(true) + if (Exit.isFailure(get)) expect(Cause.squash(get.cause)).toMatchObject({ _tag: "Pty.NotFoundError", ptyID: id }) + + const update = yield* pty.update(id, { title: "missing" }).pipe(Effect.exit) + expect(Exit.isFailure(update)).toBe(true) + if (Exit.isFailure(update)) + expect(Cause.squash(update.cause)).toMatchObject({ _tag: "Pty.NotFoundError", ptyID: id }) + + const remove = yield* pty.remove(id).pipe(Effect.exit) + expect(Exit.isFailure(remove)).toBe(true) + if (Exit.isFailure(remove)) + expect(Cause.squash(remove.cause)).toMatchObject({ _tag: "Pty.NotFoundError", ptyID: id }) + + const resize = yield* pty.resize(id, 80, 24).pipe(Effect.exit) + expect(Exit.isFailure(resize)).toBe(true) + if (Exit.isFailure(resize)) + expect(Cause.squash(resize.cause)).toMatchObject({ _tag: "Pty.NotFoundError", ptyID: id }) + + const write = yield* pty.write(id, "input").pipe(Effect.exit) + expect(Exit.isFailure(write)).toBe(true) + if (Exit.isFailure(write)) + expect(Cause.squash(write.cause)).toMatchObject({ _tag: "Pty.NotFoundError", ptyID: id }) + + const connect = yield* pty.connect(id, socket).pipe(Effect.exit) + expect(Exit.isFailure(connect)).toBe(true) + if (Exit.isFailure(connect)) + expect(Cause.squash(connect.cause)).toMatchObject({ _tag: "Pty.NotFoundError", ptyID: id }) + expect(closed).toBe(true) + }), + { git: true }, + ) + ptyTest( "publishes created, exited, deleted in order for a short-lived process", () => @@ -93,7 +141,7 @@ describe("pty", () => { expect(yield* waitForEvents(events, info.id, 1)).toEqual(["created"]) yield* pty.write(info.id, "exit\n") expect(yield* waitForEvents(events, info.id, 2)).toEqual(["exited", "deleted"]) - yield* pty.remove(info.id) + yield* pty.remove(info.id).pipe(Effect.ignore) }), { git: true }, ) diff --git a/packages/opencode/test/server/httpapi-pty.test.ts b/packages/opencode/test/server/httpapi-pty.test.ts index e28f38082a..029cdb9582 100644 --- a/packages/opencode/test/server/httpapi-pty.test.ts +++ b/packages/opencode/test/server/httpapi-pty.test.ts @@ -147,6 +147,33 @@ describe("pty HttpApi bridge", () => { expect(response.status).toBe(404) }) + test("returns typed not found errors for missing PTY HTTP resources", 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 expected = { + _tag: "PtyNotFoundError", + ptyID: missingID, + message: `PTY session not found: ${missingID}`, + } + + const found = await app().request(PtyPaths.get.replace(":ptyID", missingID), { headers }) + expect(found.status).toBe(404) + expect(await found.json()).toEqual(expected) + + const updated = await app().request(PtyPaths.update.replace(":ptyID", missingID), { + method: "PUT", + headers: { ...headers, "content-type": "application/json" }, + body: JSON.stringify({ title: "missing" }), + }) + expect(updated.status).toBe(404) + expect(await updated.json()).toEqual(expected) + + const removed = await app().request(PtyPaths.remove.replace(":ptyID", missingID), { method: "DELETE", headers }) + expect(removed.status).toBe(404) + expect(await removed.json()).toEqual(expected) + }) + 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 }