-
Notifications
You must be signed in to change notification settings - Fork 34
fix: make the plugin load and run outside Bun (#60) #97
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
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
00489a7
fix: defer bun:sqlite import so non-Bun loaders can resolve the plugin
vtemian c7db5d7
refactor: replace Bun process and file APIs with node: equivalents
vtemian bd00137
feat: run octto's server on node:http so the plugin works off Bun
vtemian 6ea91f3
fix: enforce the command timeout with an explicit timer
vtemian faf1b1f
fix: reject a timed-out command without waiting on its stdio
vtemian 7931040
fix: close the runtime and coverage gaps found in review
vtemian File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,125 +1,145 @@ | ||
| // src/octto/session/server.ts | ||
|
|
||
| import type { Server, ServerWebSocket } from "bun"; | ||
| import { createServer as createHttpServer, type IncomingMessage, type Server, type ServerResponse } from "node:http"; | ||
| import * as v from "valibot"; | ||
| import { type WebSocket, WebSocketServer } from "ws"; | ||
| import { getHtmlBundle } from "@/octto/ui"; | ||
| import { config } from "@/utils/config"; | ||
| import { extractErrorMessage } from "@/utils/errors"; | ||
| import { log } from "@/utils/logger"; | ||
| import { WsClientMessageSchema } from "./schemas"; | ||
| import type { SessionStore } from "./sessions"; | ||
| import type { WsClientMessage } from "./types"; | ||
| import type { SessionServer, SessionSocket, SocketRouter, WsClientMessage } from "./types"; | ||
|
|
||
| interface WsData { | ||
| sessionId: string; | ||
| const WS_PATH = "/ws"; | ||
| const HTML_PATHS = new Set(["/", "/index.html"]); | ||
| const LOOPBACK = "127.0.0.1"; | ||
| const LOG_MODULE = "octto"; | ||
| const ERR_NO_PORT = "Failed to get server port"; | ||
| const STATUS_OK = 200; | ||
| const STATUS_NOT_FOUND = 404; | ||
|
|
||
| function serveHttp(req: IncomingMessage, res: ServerResponse, htmlBundle: string): void { | ||
| const path = (req.url ?? "").split("?")[0]; | ||
|
|
||
| if (HTML_PATHS.has(path)) { | ||
| res.writeHead(STATUS_OK, { "Content-Type": "text/html; charset=utf-8" }); | ||
| res.end(htmlBundle); | ||
| return; | ||
| } | ||
|
|
||
| res.writeHead(STATUS_NOT_FOUND, { "Content-Type": "text/plain; charset=utf-8" }); | ||
| res.end("Not Found"); | ||
| } | ||
|
|
||
| export async function createServer( | ||
| sessionId: string, | ||
| store: SessionStore, | ||
| ): Promise<{ server: Server<WsData>; port: number }> { | ||
| const htmlBundle = getHtmlBundle(); | ||
| function sendError(socket: SessionSocket, error: string, details: string): void { | ||
| socket.send(JSON.stringify({ type: "error", error, details })); | ||
| } | ||
|
|
||
| const server = Bun.serve<WsData>({ | ||
| port: 0, // Random available port | ||
| hostname: config.octto.allowRemoteBind ? config.octto.bindAddress : "127.0.0.1", | ||
| fetch(req, server) { | ||
| return handleFetch(req, server, sessionId, htmlBundle); | ||
| }, | ||
| websocket: createWebSocketHandlers(store), | ||
| }); | ||
| function handleWsMessage(socket: SessionSocket, sessionId: string, raw: string, store: SocketRouter): void { | ||
| let parsed: unknown; | ||
| try { | ||
| parsed = JSON.parse(raw); | ||
| } catch (error) { | ||
| log.error(LOG_MODULE, "Failed to parse WebSocket message", error); | ||
| sendError(socket, "Invalid message format", extractErrorMessage(error)); | ||
| return; | ||
| } | ||
|
|
||
| // Port is always defined when using port: 0 | ||
| const port = server.port; | ||
| if (port === undefined) { | ||
| throw new Error("Failed to get server port"); | ||
| const result = v.safeParse(WsClientMessageSchema, parsed); | ||
| if (!result.success) { | ||
| log.error(LOG_MODULE, "Invalid WebSocket message schema", result.issues); | ||
| sendError(socket, "Invalid message schema", result.issues.map((issue) => issue.message).join("; ")); | ||
| return; | ||
| } | ||
|
|
||
| return { | ||
| server, | ||
| port, | ||
| }; | ||
| store.handleWsMessage(sessionId, result.output as WsClientMessage); | ||
| } | ||
|
|
||
| function handleFetch( | ||
| req: Request, | ||
| server: Server<WsData>, | ||
| sessionId: string, | ||
| htmlBundle: string, | ||
| ): Response | undefined { | ||
| const url = new URL(req.url); | ||
|
|
||
| // WebSocket upgrade | ||
| if (url.pathname === "/ws") { | ||
| const success = server.upgrade(req, { | ||
| data: { sessionId }, | ||
| function attachWebSockets(wss: WebSocketServer, sessionId: string, store: SocketRouter): void { | ||
| wss.on("connection", (socket: WebSocket) => { | ||
| store.handleWsConnect(sessionId, socket); | ||
|
|
||
| socket.on("message", (data: Buffer | string) => { | ||
| handleWsMessage(socket, sessionId, data.toString(), store); | ||
| }); | ||
| if (success) { | ||
| return undefined; | ||
| } | ||
| return new Response("WebSocket upgrade failed", { status: 400 }); | ||
| } | ||
| socket.on("close", () => { | ||
| store.handleWsDisconnect(sessionId); | ||
| }); | ||
| socket.on("error", (error: unknown) => { | ||
| log.error(LOG_MODULE, "WebSocket connection error", error); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| // Serve the bundled HTML app | ||
| if (url.pathname === "/" || url.pathname === "/index.html") { | ||
| return new Response(htmlBundle, { | ||
| headers: { | ||
| "Content-Type": "text/html; charset=utf-8", | ||
| }, | ||
| // ws attaches its own listener to the HTTP server and re-emits its errors on | ||
| // itself, so the only handler that sees a bind failure is one registered here. | ||
| // Without it an unhandled 'error' propagates out and takes the host process | ||
| // down with the session. | ||
| function listen(http: Server, wss: WebSocketServer, hostname: string): Promise<number> { | ||
| return new Promise((resolve, reject) => { | ||
| let settled = false; | ||
| const settle = (finish: () => void): void => { | ||
| if (settled) return; | ||
| settled = true; | ||
| finish(); | ||
| }; | ||
|
|
||
| wss.on("error", (error) => { | ||
| log.error(LOG_MODULE, "WebSocket server error", error); | ||
| settle(() => { | ||
| reject(error); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| return new Response("Not Found", { status: 404 }); | ||
| http.listen(0, hostname, () => { | ||
| const address = http.address(); | ||
| if (address === null || typeof address === "string") { | ||
| settle(() => { | ||
| reject(new Error(ERR_NO_PORT)); | ||
| }); | ||
| return; | ||
| } | ||
| settle(() => { | ||
| resolve(address.port); | ||
| }); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| function createWebSocketHandlers(store: SessionStore): { | ||
| open: (ws: ServerWebSocket<WsData>) => void; | ||
| close: (ws: ServerWebSocket<WsData>) => void; | ||
| message: (ws: ServerWebSocket<WsData>, message: string | Buffer) => void; | ||
| } { | ||
| return { | ||
| open(ws: ServerWebSocket<WsData>) { | ||
| store.handleWsConnect(ws.data.sessionId, ws); | ||
| }, | ||
| close(ws: ServerWebSocket<WsData>) { | ||
| store.handleWsDisconnect(ws.data.sessionId); | ||
| }, | ||
| message(ws: ServerWebSocket<WsData>, message: string | Buffer) { | ||
| handleWsMessage(ws, message, store); | ||
| }, | ||
| }; | ||
| // Live sockets keep node:http from closing, so drop them before waiting. | ||
| function stop(http: Server, wss: WebSocketServer): Promise<void> { | ||
| return new Promise((resolve) => { | ||
| for (const client of wss.clients) { | ||
| client.terminate(); | ||
| } | ||
| wss.close(() => { | ||
| // Absent before Node 18.2, which older Electron builds still ship. An | ||
| // unguarded call throws inside ws's close callback and strands stop(). | ||
| http.closeAllConnections?.(); | ||
| http.close(() => { | ||
| resolve(); | ||
| }); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| function handleWsMessage(ws: ServerWebSocket<WsData>, message: string | Buffer, store: SessionStore): void { | ||
| const { sessionId } = ws.data; | ||
| export async function createServer( | ||
| sessionId: string, | ||
| store: SocketRouter, | ||
| ): Promise<{ server: SessionServer; port: number }> { | ||
| const htmlBundle = getHtmlBundle(); | ||
| const hostname = config.octto.allowRemoteBind ? config.octto.bindAddress : LOOPBACK; | ||
|
|
||
| let raw: unknown; | ||
| try { | ||
| raw = JSON.parse(message.toString()); | ||
| } catch (error) { | ||
| log.error("octto", "Failed to parse WebSocket message", error); | ||
| ws.send( | ||
| JSON.stringify({ | ||
| type: "error", | ||
| error: "Invalid message format", | ||
| details: extractErrorMessage(error), | ||
| }), | ||
| ); | ||
| return; | ||
| } | ||
| const http = createHttpServer((req, res) => { | ||
| serveHttp(req, res, htmlBundle); | ||
| }); | ||
| const wss = new WebSocketServer({ server: http, path: WS_PATH, maxPayload: config.octto.maxFrameBytes }); | ||
| attachWebSockets(wss, sessionId, store); | ||
|
|
||
| const result = v.safeParse(WsClientMessageSchema, raw); | ||
| if (!result.success) { | ||
| log.error("octto", "Invalid WebSocket message schema", result.issues); | ||
| ws.send( | ||
| JSON.stringify({ | ||
| type: "error", | ||
| error: "Invalid message schema", | ||
| details: result.issues.map((i) => i.message).join("; "), | ||
| }), | ||
| ); | ||
| return; | ||
| } | ||
| const port = await listen(http, wss, hostname); | ||
|
|
||
| store.handleWsMessage(sessionId, result.output as WsClientMessage); | ||
| return { | ||
| port, | ||
| server: { port, hostname, stop: () => stop(http, wss) }, | ||
| }; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
P2: Stopping a session can hang on older Electron/Node runtimes after an HTTP client has used keep-alive. The optional call avoids the throw but leaves no fallback to destroy those HTTP sockets, so
http.close()may never invoke its callback; track HTTP connections and destroy them when this API is unavailable.Prompt for AI agents